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!: expedited proposals #240

Merged
merged 45 commits into from
Jun 16, 2022
Merged

feat!: expedited proposals #240

merged 45 commits into from
Jun 16, 2022

Conversation

p0mvn
Copy link
Member

@p0mvn p0mvn commented May 19, 2022

Closes: #XXX

What is the purpose of the change

Feature: 2/3 Majority Emergency Voting Periods
Original Spec: osmosis-labs/osmosis#1517

Description

This change introduces an option for any existing proposal to be expedited. If a proposal is expedited, it uses a shorter voting duration and a higher tally threshold. If a the higher threshold is not met under shorter voting duration, the proposal is converted as if it was a regular proposal and tallied / voted on under regular rules.

The distinction is achieved by having an IsExpedited flag on the proposal struct. When a proposal is converted from expedited to regular, this flag is flipped and VotingEndTime is extended. The rest of the fields, including the ProposalId remain the same. As a result, all existing votes are preserved.

Differences from the Spec:

  • ExpeditedDuration is implemented on the VotingParams as opposed to Proposal struct
  • Did not create a separate index for expedited proposals. This would require duplicating logic for expedited proposals. Instead, in end block we iterate over both expedited and regular proposals and handle them appropriately

Brief Changelog

  • add IsExpedited flag to protos of all proposal content types
  • add ExpeditedVotingPeriod to protos of gov voting parameters
  • add ExpeditedThreshold to protos of gov tally parameters
  • implement changes in EndBlocker of x/gov to move expedited proposals to regular (with extended voting period) once voting period ends with proposal not meeting ExpeditedThreshold
  • add unit and integration tests to cover all new logic related to expedited proposals
  • fix existing tests

Testing and Verifying

This change is a trivial rework / code cleanup without any test coverage.

This change is already covered by existing tests, such as :

  • unit, integration and simulation tests in x/gov

This change added tests and can be verified as follows:

  • go test -timeout 30s -run ^TestExpeditedProposal_PassAndConversionToRegular$ github.com/cosmos/cosmos-sdk/x/gov
  • go test -timeout 30s -run ^TestProposalPassedEndblocker$ github.com/cosmos/cosmos-sdk/x/gov
  • go test -timeout 30s -run ^TestTickPassedVotingPeriod$ github.com/cosmos/cosmos-sdk/x/gov

Documentation and Release Note

  • Does this pull request introduce a new feature or user-facing behavior changes? yes
  • Is a relevant changelog entry added to the Unreleased section in CHANGELOG.md? no
  • How is the feature or change documented? not documented

@p0mvn p0mvn changed the title feat: expedited proposals feat!: expedited proposals May 20, 2022
Copy link
Collaborator

@alexanderbez alexanderbez left a comment

Choose a reason for hiding this comment

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

So far I've just reviewed the proto changes, they look great apart from a few messed up field numbers (see comments). Will continue reviewing...

proto/cosmos/distribution/v1beta1/distribution.proto Outdated Show resolved Hide resolved
proto/cosmos/distribution/v1beta1/distribution.proto Outdated Show resolved Hide resolved
proto/cosmos/gov/v1beta1/gov.proto Outdated Show resolved Hide resolved
proto/cosmos/gov/v1beta1/gov.proto Outdated Show resolved Hide resolved
proto/cosmos/gov/v1beta1/gov.proto Outdated Show resolved Hide resolved
proto/cosmos/params/v1beta1/params.proto Outdated Show resolved Hide resolved
proto/cosmos/upgrade/v1beta1/upgrade.proto Outdated Show resolved Hide resolved
Copy link
Collaborator

@alexanderbez alexanderbez left a comment

Choose a reason for hiding this comment

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

utACK. Looks great! Can't see any major flaws. Let's fix the proto stuff.

x/gov/abci.go Outdated Show resolved Hide resolved
x/gov/types/genesis.go Outdated Show resolved Hide resolved
x/gov/types/params.go Outdated Show resolved Hide resolved
p0mvn and others added 5 commits June 3, 2022 12:32
Co-authored-by: Aleksandr Bezobchuk <alexanderbez@users.noreply.github.com>
Co-authored-by: Aleksandr Bezobchuk <alexanderbez@users.noreply.github.com>
Copy link
Collaborator

@alexanderbez alexanderbez left a comment

Choose a reason for hiding this comment

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

🚀

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.

Nice and clean implementation 🔥 Just some minor nit comments and should be good to merge!

proto/cosmos/distribution/v1beta1/distribution.proto Outdated Show resolved Hide resolved
proto/cosmos/params/v1beta1/params.proto Outdated Show resolved Hide resolved
x/gov/keeper/proposal.go Show resolved Hide resolved
x/gov/abci.go Outdated Show resolved Hide resolved
x/gov/types/keys.go Outdated Show resolved Hide resolved
x/gov/types/msgs_test.go Show resolved Hide resolved
x/gov/types/proposal.go Outdated Show resolved Hide resolved
x/gov/types/proposal.go Show resolved Hide resolved
x/params/types/proposal/proposal.go Show resolved Hide resolved
mattverse and others added 5 commits June 6, 2022 00:10
* Add simple changelog

* Update x/gov/spec/01_concepts.md

Co-authored-by: Aleksandr Bezobchuk <alexanderbez@users.noreply.github.com>

Co-authored-by: Aleksandr Bezobchuk <alexanderbez@users.noreply.github.com>
Co-authored-by: Matt, Park <45252226+mattverse@users.noreply.github.com>
Co-authored-by: Matt, Park <45252226+mattverse@users.noreply.github.com>
@github-actions github-actions bot removed the C:x/auth label Jun 7, 2022
johnletey added a commit to kyve-org/cosmos-sdk-old that referenced this pull request Jun 15, 2022
@p0mvn
Copy link
Member Author

p0mvn commented Jun 16, 2022

Merging this to avoid having to continuously sync it with osmosis-main. We should be careful not to release this before governance decides on the future of this feature.

As for the current status of this as a whole, #254 needs to be completed and merged before we create a gov prop. The gov prop is already written up and ready to go on-chain once #254 is in

@p0mvn p0mvn merged commit ae817b9 into osmosis-main Jun 16, 2022
@p0mvn p0mvn deleted the roman/emergency-voting branch June 16, 2022 21:29
@mattverse
Copy link
Member

Sweet! lmk once #254 is r4r!

johnletey added a commit to kyve-org/cosmos-sdk-old that referenced this pull request Jun 18, 2022
* feat: expedited proposals

h/t osmosis-labs#240

* fix: correctly generate random threshold params

* fix: update randomised genesis state tests
johnletey added a commit to kyve-org/cosmos-sdk-old that referenced this pull request Jun 27, 2022
* 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
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

None yet

4 participants