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

feat: create a x/gov tutorial #1217

Merged
merged 13 commits into from
Sep 19, 2022
Merged

feat: create a x/gov tutorial #1217

merged 13 commits into from
Sep 19, 2022

Conversation

julienrbrt
Copy link
Member

Closes #1135. Requires cosmos/cosmos-sdk#13304 to be merged.

@julienrbrt julienrbrt marked this pull request as ready for review September 19, 2022 06:15
README.md Outdated Show resolved Hide resolved
tutorials/understanding-gov/index.md Outdated Show resolved Hide resolved
tutorials/understanding-gov/index.md Show resolved Hide resolved
tutorials/understanding-gov/index.md Show resolved Hide resolved
tutorials/understanding-gov/index.md Outdated Show resolved Hide resolved
tutorials/understanding-gov/index.md Outdated Show resolved Hide resolved
tutorials/understanding-gov/index.md Outdated Show resolved Hide resolved
tutorials/understanding-gov/index.md Outdated Show resolved Hide resolved
tutorials/understanding-gov/index.md Outdated Show resolved Hide resolved
tutorials/understanding-gov/index.md Outdated Show resolved Hide resolved
julienrbrt and others added 2 commits September 19, 2022 14:04
Co-authored-by: Xavier Leprêtre <xavierlepretre@users.noreply.github.com>
Co-authored-by: Xavier Leprêtre <xavierlepretre@users.noreply.github.com>
tutorials/understanding-gov/index.md Outdated Show resolved Hide resolved
$ simd query gov proposal 1 --output json | jq .status
```

This is because the governance proposal weight the votes given the amount of token staked. Alice had staked tokens, while Bob had no token staked. So Bob's vote was not took into consideration in the tally of the result.
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
This is because the governance proposal weight the votes given the amount of token staked. Alice had staked tokens, while Bob had no token staked. So Bob's vote was not took into consideration in the tally of the result.
This is because the governance proposal weights each vote by the amount of tokens staked. Alice owns staked tokens, while Bob has no staked tokens. So Bob's vote was not taken into consideration in the tally of the result.

Do you want to put a word about what Alice can do with her staked tokens after voting? In particular if the voting period is longer than the unstaking period?

Copy link
Member Author

Choose a reason for hiding this comment

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

I'm not sure what you mean by this.

Copy link
Contributor

Choose a reason for hiding this comment

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

  • Alice votes Yes with 1 million ATOM.
  • Alice unstakes her million ATOM and waits 3 weeks.
  • Alice sends her million to Bob.
  • Bob votes Yes with 1 million ATOM.

This should not be allowed. It was possible because the voting period was longer than the unsticking period.

Copy link
Member Author

@julienrbrt julienrbrt Sep 19, 2022

Choose a reason for hiding this comment

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

I get it! However, this is a non issue because the vote weight is calculated at vote tally. So only Bob's vote will be counted in that case (if he staked).

Copy link
Contributor

Choose a reason for hiding this comment

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

Mmh, I don't like O(n) actions in blockchain 🤣
Are you sure there is nothing like intermediate tally, and when unstaking, your vote is updated on all proposals?

Copy link
Member Author

Choose a reason for hiding this comment

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

Yeah, that is why tally takes some time (and sometimes there are no blocks for x seconds due to that): https://github.com/cosmos/cosmos-sdk/blob/main/x/gov/keeper/tally.go#L37-L73

Copy link
Contributor

Choose a reason for hiding this comment

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

Lol

tutorials/understanding-gov/index.md Outdated Show resolved Hide resolved
tutorials/understanding-gov/index.md Show resolved Hide resolved
tutorials/understanding-gov/index.md Outdated Show resolved Hide resolved
tutorials/understanding-gov/index.md Outdated Show resolved Hide resolved
tutorials/understanding-gov/index.md Outdated Show resolved Hide resolved
tutorials/understanding-gov/index.md Show resolved Hide resolved
tutorials/understanding-gov/index.md Outdated Show resolved Hide resolved
tutorials/understanding-gov/index.md Outdated Show resolved Hide resolved
julienrbrt and others added 4 commits September 19, 2022 16:36
Co-authored-by: Xavier Leprêtre <xavierlepretre@users.noreply.github.com>
tutorials/understanding-gov/index.md Show resolved Hide resolved
tutorials/understanding-gov/index.md Outdated Show resolved Hide resolved
tutorials/understanding-gov/index.md Outdated Show resolved Hide resolved
Co-authored-by: Xavier Leprêtre <xavierlepretre@users.noreply.github.com>
@julienrbrt
Copy link
Member Author

Thank you very much again @xavierlepretre for all the great feedback!

tutorials/understanding-gov/index.md Outdated Show resolved Hide resolved
Co-authored-by: Xavier Leprêtre <xavierlepretre@users.noreply.github.com>
@julienrbrt julienrbrt merged commit 88e6c00 into master Sep 19, 2022
@julienrbrt julienrbrt deleted the julien/gov branch September 19, 2022 15:37
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.

Create a x/gov tutorial
2 participants