Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Update governance doc #83

Closed
wants to merge 7 commits into from
Closed

Update governance doc #83

wants to merge 7 commits into from

Conversation

gamarin2
Copy link
Contributor

@gamarin2 gamarin2 commented Feb 5, 2018

  • Typo fix
  • Fix language
  • Changed type of Threshold
  • Re-arrange implementation section according to bucky's directives

- Typo fix
- Fix language
- Changed type of `Threshold`
- Re-arrange implementation section according to bucky's directives

`Urgent` proposals also work with the aforementioned threshold, except there is another condition that can accelerate the acceptance of the proposal. Namely, if the ratio of `Yes` votes to `InitTotalVotingPower` exceeds 2/3, `UrgentProposal` will be immediately accepted, even if the `Voting period` is not finished. `InitTotalVotingPower` is the total voting power of all bonded Atom holders at the moment when the vote opens.
`Urgent` proposals also work with the aforementioned threshold, except there is another condition that can accelerate the acceptance of the proposal. Namely, if the ratio of `Yes` votes to `InitTotalVotingPower` exceeds 2:3, `UrgentProposal` will be immediately accepted, even if the `Voting period` is not finished. `InitTotalVotingPower` is the total voting power of all bonded Atom holders at the moment when the vote opens.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

maybe use 67% percent rather than 2:3? either way we should make this uniform across all specs

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sometimes it makes sense to use percentage and sometimes it makes sense to use ratio. I can't use 67% if it's a ratio.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's always use 67% then, 67% represents a ratio > 2:3 (by atleast 0.444444... %)

And the pseudocode for the `ProposalProcessingQueue`:

```
in BeginBlock do
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure we want pseudo code here, looks like it could just as easily be a series of bullet points (with sub bullet points) of procedure ordering... But if we do end up wanting pseudo code we should probably begin the code at func checkProposal() and still used curly brackets around funcs, if, and for statements like in golang.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Seeing that zarco did same formatting as me, I think we should keep it this way for now, so that it's uniform accross specs

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

okay well remove the first few lines before func checkProposal() - that is implicit. Also I don't see how having the function name helps... just scanning through your pseudo code here, it's called within itself... which means that this is a recursive function... Is this what you wanted? - just want to make sure... we should probably be using the queue system instead of recursion.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Talked with rigel, will use queue system+recursion. Fix to come


for each validator in validators
if validator is not in votersList
slash validator by ActiveProcedure.GovernancePenalty
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we accomplish this without a for loop? I guess it doesn't really matter here

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

don't think so

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I mean I'm sure there is a way we could accomplish that if we needed to - I'm just not convinced we really care because, unlike block processing, this process doesn't happen very often. We're good..

slash validator by ActiveProcedure.GovernancePenalty

ProposalProcessingQueueBeginning++ // ProposalProcessingQueue will have a new element
checkProposal()
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Like I mentioned we should really not be using recursion here, add pseudo code referencing a queue element

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Will add queue logic in recursive function

ProposalProcessingQueueBeginning++ // ProposalProcessingQueue will have a new element
checkProposal()

else
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

else return at the end of a function is always implicit, please remove

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ok

@ebuchman
Copy link
Member

Can we move this to the SDK? New home for gov spec: https://github.com/cosmos/cosmos-sdk/tree/develop/docs/spec/governance

@gamarin2
Copy link
Contributor Author

Yes. I'm currently finishing the consolidation of the spec. I will commit here to keep track of the different steps. Then I'll make a new branch on cosmos-sdk and submit a PR there (1 massive commit). Will close this one then.

@gamarin2
Copy link
Contributor Author

Moved to sdk repo @ebuchman cosmos/cosmos-sdk#495

@gamarin2 gamarin2 closed this Feb 21, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants