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.

Sign up for GitHub

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

Jump to bottom

feat!: support for custom proposal-based voting periods #239

Merged
merged 15 commits into from
May 20, 2022
Merged

feat!: support for custom proposal-based voting periods #239

merged 15 commits into from
May 20, 2022

Conversation

alexanderbez
Copy link

@alexanderbez alexanderbez commented May 19, 2022 •
edited
Loading

Add support for custom voting periods:

  • Define ProposalVotingPeriod
  • Add []ProposalVotingPeriod to the existing VotingParams (on-chain upgrade should be fine)
  • Update Keeper#ActivateVotingPeriod to account for the new []ProposalVotingPeriod

NOTE: I did NOT add/implement CustomVotingPeriodQuorum as I just didn't see that as necessary.

ref: osmosis-labs/osmosis#1517

@alexanderbez alexanderbez marked this pull request as ready for review May 20, 2022 13:30
@alexanderbez alexanderbez requested a review from a team May 20, 2022 13:30
@alexanderbez
Copy link
Author

Will fix tests shortly.

@github-actions github-actions bot added the C:CLI label May 20, 2022
mattverse
mattverse approved these changes May 20, 2022
View reviewed changes
Copy link
Member

@mattverse mattverse left a comment

Choose a reason for hiding this comment

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

LGTM!

Nit(we can also create an issue and set this as a chore): I wish there was more test cases added, other than simply comparing the two proposals, but don't want to block merging for this!

x/gov/keeper/grpc_query.go Show resolved Hide resolved
@alexanderbez
Copy link
Author

LGTM!

Nit(we can also create an issue and set this as a chore): I wish there was more test cases added, other than simply comparing the two proposals, but don't want to block merging for this!

@mattverse I added TestGetVotingPeriod, which is the core change in this PR (apart from adding the new types). That should be enough to cover things.

ValarDragon
ValarDragon approved these changes May 20, 2022
View reviewed changes
Copy link
Member

@ValarDragon ValarDragon left a comment

Choose a reason for hiding this comment

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

Awesome, looks good to me!

We shouldn't need to initialize the new param in our app.go right, since this doesn't update the paramset, just the concrete struct at VotingParams?

@alexanderbez
Copy link
Author

Awesome, looks good to me!

We shouldn't need to initialize the new param in our app.go right, since this doesn't update the paramset, just the concrete struct at VotingParams?

That's correct!

@alexanderbez alexanderbez merged commit a2fcb7a into osmosis-main May 20, 2022
@alexanderbez alexanderbez deleted the bez/prop-based-vp branch May 20, 2022 18:59
@assafmo
Copy link

assafmo commented Jun 9, 2022

This is super cool guys!

johnletey added a commit to kyve-org/cosmos-sdk-old that referenced this pull request Jun 13, 2022
@johnletey
feat: custom proposal-based voting periods
5feda1e 
h/t osmosis-labs#239
@johnletey johnletey mentioned this pull request Jun 13, 2022
Merged
johnletey added a commit to kyve-org/cosmos-sdk-old that referenced this pull request Jun 15, 2022
@johnletey
feat: custom proposal-based voting periods (#1)
379239e 
h/t osmosis-labs#239
@tac0turtle tac0turtle mentioned this pull request Jun 16, 2022
Closed
9 tasks
johnletey added a commit to kyve-org/cosmos-sdk-old that referenced this pull request Jun 27, 2022
@johnletey
feat: custom gov (#5)
b295ae1 
* feat: custom proposal-based voting periods (#1)

h/t osmosis-labs#239

* feat: expedited proposals (#2)

* feat: expedited proposals

h/t osmosis-labs#240

* fix: correctly generate random threshold params

* fix: update randomised genesis state tests

* feat: allow protocol stakers and delegators to vote (#3)

* feat: expedited deposits (#4)

h/t osmosis-labs#254

* fix: still include custom voting periods

* fix: fallback to custom voting periods
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@mattverse mattverse mattverse approved these changes

@ValarDragon ValarDragon ValarDragon approved these changes

Assignees
No one assigned
Labels
C:CLI C:Simulations C:x/gov C:x/staking
Projects
Osmosis Chain Development
Archived in project
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@alexanderbez @assafmo @ValarDragon @mattverse