Skip to content

Commit

Permalink
feat(x/gov): add max cancel voting period param (#18856)
Browse files Browse the repository at this point in the history
  • Loading branch information
julienrbrt authored Dec 28, 2023
1 parent 77b2366 commit e1e8c46
Show file tree
Hide file tree
Showing 14 changed files with 488 additions and 262 deletions.
257 changes: 173 additions & 84 deletions api/cosmos/gov/v1/gov.pulsar.go

Large diffs are not rendered by default.

16 changes: 14 additions & 2 deletions proto/cosmos/gov/v1/gov.proto
Original file line number Diff line number Diff line change
Expand Up @@ -289,12 +289,18 @@ message Params {
[(gogoproto.nullable) = false, (amino.dont_omitempty) = true];

// burn deposits if a proposal does not meet quorum
//
// Since: cosmos-sdk 0.47
bool burn_vote_quorum = 13;

// burn deposits if the proposal does not enter voting period
//
// Since: cosmos-sdk 0.47
bool burn_proposal_deposit_prevote = 14;

// burn deposits if quorum with vote type no_veto is met
//
// Since: cosmos-sdk 0.47
bool burn_vote_veto = 15;

// The ratio representing the proportion of the deposit value minimum that must be met when making a deposit.
Expand All @@ -304,15 +310,21 @@ message Params {
// Since: cosmos-sdk 0.50
string min_deposit_ratio = 16 [(cosmos_proto.scalar) = "cosmos.Dec"];

// proposal_cancel_max_period defines how far in the voting period a proposer can cancel a proposal.
// If the proposal is cancelled before the max cancel period, the deposit will be returned/burn to the
// depositors, according to the proposal_cancel_ratio and proposal_cancel_dest parameters.
// After the max cancel period, the proposal cannot be cancelled anymore.
string proposal_cancel_max_period = 17 [(cosmos_proto.scalar) = "cosmos.Dec"];

// optimistic_authorized_addresses is an optional governance parameter that limits the authorized accounts than can
// submit optimistic proposals
//
// Since: x/gov v1.0.0
repeated string optimistic_authorized_addresses = 17 [(cosmos_proto.scalar) = "cosmos.AddressString"];
repeated string optimistic_authorized_addresses = 18 [(cosmos_proto.scalar) = "cosmos.AddressString"];

// optimistic rejected threshold defines at which percentage of NO votes, the optimistic proposal should fail and be
// converted to a standard proposal. The threshold is expressed as a percentage of the total bonded tokens.
//
// Since: x/gov v1.0.0
string optimistic_rejected_threshold = 18 [(cosmos_proto.scalar) = "cosmos.Dec"];
string optimistic_rejected_threshold = 19 [(cosmos_proto.scalar) = "cosmos.Dec"];
}
3 changes: 2 additions & 1 deletion x/gov/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,8 @@ Ref: https://keepachangelog.com/en/1.0.0/

### Improvements

* [#18445](https://github.com/cosmos/cosmos-sdk/pull/18445) Extend gov config
* [#18856](https://github.com/cosmos/cosmos-sdk/pull/18856) Add `ProposalCancelMaxPeriod` for modifying how long a proposal can be cancelled after it has been submitted.
* [#18445](https://github.com/cosmos/cosmos-sdk/pull/18445) Extend gov config.
* [#18532](https://github.com/cosmos/cosmos-sdk/pull/18532) Repurpose `govcliutils.NormalizeProposalType` to work for gov v1 proposal types.

### API Breaking Changes
Expand Down
37 changes: 20 additions & 17 deletions x/gov/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -591,23 +591,26 @@ The governance module emits the following events:

The governance module contains the following parameters:

| Key | Type | Example |
| ------------------------------- | ---------------------- | --------------------------------------- |
| min_deposit | array (coins) | [{"denom":"uatom","amount":"10000000"}] |
| max_deposit_period | string (time ns) | "172800000000000" (17280s) |
| voting_period | string (time ns) | "172800000000000" (17280s) |
| quorum | string (dec) | "0.334000000000000000" |
| threshold | string (dec) | "0.500000000000000000" |
| veto | string (dec) | "0.334000000000000000" |
| expedited_threshold | string (time ns) | "0.667000000000000000" |
| expedited_voting_period | string (time ns) | "86400000000000" (8600s) |
| expedited_min_deposit | array (coins) | [{"denom":"uatom","amount":"50000000"}] |
| burn_proposal_deposit_prevote | bool | false |
| burn_vote_quorum | bool | false |
| burn_vote_veto | bool | true |
| min_initial_deposit_ratio | string | "0.1" |
| optimistic_rejected_threshold | string (dec) | "0.1" |
| optimistic_authorized_addresses | bytes array (addresses) | [][] |
| Key | Type | Example |
| ------------------------------- | ----------------- | --------------------------------------- |
| min_deposit | array (coins) | [{"denom":"uatom","amount":"10000000"}] |
| max_deposit_period | string (time ns) | "172800000000000" (17280s) |
| voting_period | string (time ns) | "172800000000000" (17280s) |
| quorum | string (dec) | "0.334000000000000000" |
| threshold | string (dec) | "0.500000000000000000" |
| veto | string (dec) | "0.334000000000000000" |
| expedited_threshold | string (time ns) | "0.667000000000000000" |
| expedited_voting_period | string (time ns) | "86400000000000" (8600s) |
| expedited_min_deposit | array (coins) | [{"denom":"uatom","amount":"50000000"}] |
| burn_proposal_deposit_prevote | bool | false |
| burn_vote_quorum | bool | false |
| burn_vote_veto | bool | true |
| min_initial_deposit_ratio | string | "0.1" |
| proposal_cancel_ratio | string (dec) | "0.5" |
| proposal_cancel_dest | string (address) | "cosmos1.." or empty for burn |
| proposal_cancel_max_period | string (dec) | "0.5" |
| optimistic_rejected_threshold | string (dec) | "0.1" |
| optimistic_authorized_addresses | array (addresses) | [] |

**NOTE**: The governance module contains parameters that are objects unlike other
modules. If only a subset of parameters are desired to be changed, only they need
Expand Down
4 changes: 2 additions & 2 deletions x/gov/keeper/msg_server_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -337,11 +337,11 @@ func (suite *KeeperTestSuite) TestMsgCancelProposal() {
},
depositor: proposer,
expErr: true,
expErrMsg: "not found",
expErrMsg: "proposal 0 doesn't exist",
},
"valid proposal but invalid proposer": {
preRun: func() uint64 {
return proposalID
return res.ProposalId
},
depositor: addrs[1],
expErr: true,
Expand Down
29 changes: 21 additions & 8 deletions x/gov/keeper/proposal.go
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import (

"cosmossdk.io/collections"
errorsmod "cosmossdk.io/errors"
sdkmath "cosmossdk.io/math"
"cosmossdk.io/x/gov/types"
v1 "cosmossdk.io/x/gov/types/v1"

Expand Down Expand Up @@ -126,6 +127,14 @@ func (keeper Keeper) SubmitProposal(ctx context.Context, messages []sdk.Msg, met
func (keeper Keeper) CancelProposal(ctx context.Context, proposalID uint64, proposer string) error {
sdkCtx := sdk.UnwrapSDKContext(ctx)
proposal, err := keeper.Proposals.Get(ctx, proposalID)
if err != nil {
if errors.Is(err, collections.ErrNotFound) {
return types.ErrInvalidProposal.Wrapf("proposal %d doesn't exist", proposalID)
}
return err
}

params, err := keeper.Params.Get(ctx)
if err != nil {
return err
}
Expand All @@ -146,18 +155,22 @@ func (keeper Keeper) CancelProposal(ctx context.Context, proposalID uint64, prop
return types.ErrInvalidProposal.Wrap("proposal should be in the deposit or voting period")
}

// Check proposal voting period is ended.
if proposal.VotingEndTime != nil && proposal.VotingEndTime.Before(sdkCtx.HeaderInfo().Time) {
return types.ErrVotingPeriodEnded.Wrapf("voting period is already ended for this proposal %d", proposalID)
// Check proposal is not too far in voting period to be canceled
if proposal.VotingEndTime != nil {
currentTime := sdkCtx.HeaderInfo().Time

maxCancelPeriodRate := sdkmath.LegacyMustNewDecFromStr(params.ProposalCancelMaxPeriod)
maxCancelPeriod := time.Duration(float64(proposal.VotingEndTime.Sub(*proposal.VotingStartTime)) * maxCancelPeriodRate.MustFloat64()).Round(time.Second)

if proposal.VotingEndTime.Before(currentTime) {
return types.ErrVotingPeriodEnded.Wrapf("voting period is already ended for this proposal %d", proposalID)
} else if proposal.VotingEndTime.Add(-maxCancelPeriod).Before(currentTime) {
return types.ErrTooLateToCancel.Wrapf("proposal %d is too late to cancel", proposalID)
}
}

// burn the (deposits * proposal_cancel_rate) amount or sent to cancellation destination address.
// and deposits * (1 - proposal_cancel_rate) will be sent to depositors.
params, err := keeper.Params.Get(ctx)
if err != nil {
return err
}

err = keeper.ChargeDeposit(ctx, proposal.Id, params.ProposalCancelDest, params.ProposalCancelRatio)
if err != nil {
return err
Expand Down
48 changes: 33 additions & 15 deletions x/gov/keeper/proposal_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import (
"fmt"
"strings"
"testing"
"time"

"github.com/stretchr/testify/require"

Expand Down Expand Up @@ -216,32 +217,32 @@ func (suite *KeeperTestSuite) TestCancelProposal() {
suite.Require().NoError(err)

testCases := []struct {
name string
malleate func() (proposalID uint64, proposer string)
proposalID uint64
proposer string
expectedErr bool
name string
malleate func() (proposalID uint64, proposer string)
proposalID uint64
proposer string
expectedErrMsg string
}{
{
name: "without proposer",
malleate: func() (uint64, string) {
return 1, ""
return proposalID, ""
},
expectedErr: true,
expectedErrMsg: "invalid proposer",
},
{
name: "invalid proposal id",
malleate: func() (uint64, string) {
return 1, suite.addrs[1].String()
return 10, suite.addrs[1].String()
},
expectedErr: true,
expectedErrMsg: "proposal 10 doesn't exist",
},
{
name: "valid proposalID but invalid proposer",
malleate: func() (uint64, string) {
return proposalID, suite.addrs[1].String()
},
expectedErr: true,
expectedErrMsg: "invalid proposer",
},
{
name: "valid proposalID but invalid proposal which has already passed",
Expand All @@ -255,14 +256,32 @@ func (suite *KeeperTestSuite) TestCancelProposal() {
suite.Require().NoError(err)
return proposal2ID, suite.addrs[1].String()
},
expectedErr: true,
expectedErrMsg: "proposal should be in the deposit or voting period",
},
{
name: "proposal canceled too late",
malleate: func() (uint64, string) {
suite.Require().NoError(suite.govKeeper.ActivateVotingPeriod(suite.ctx, proposal2))

proposal2, err = suite.govKeeper.Proposals.Get(suite.ctx, proposal2.Id)
suite.Require().Nil(err)

headerInfo := suite.ctx.HeaderInfo()
// try to cancel 1min before the end of the voting period
// this should fail, as we allow to cancel proposal (by default) up to 1/2 of the voting period
headerInfo.Time = proposal2.VotingEndTime.Add(-1 * time.Minute)
suite.ctx = suite.ctx.WithHeaderInfo(headerInfo)

suite.Require().NoError(err)
return proposal2ID, suite.addrs[1].String()
},
expectedErrMsg: "too late",
},
{
name: "valid proposer and proposal id",
malleate: func() (uint64, string) {
return proposalID, suite.addrs[0].String()
},
expectedErr: false,
},
{
name: "valid case with deletion of votes",
Expand All @@ -283,16 +302,15 @@ func (suite *KeeperTestSuite) TestCancelProposal() {

return proposalID, suite.addrs[0].String()
},
expectedErr: false,
},
}

for _, tc := range testCases {
suite.Run(tc.name, func() {
pID, proposer := tc.malleate()
err = suite.govKeeper.CancelProposal(suite.ctx, pID, proposer)
if tc.expectedErr {
suite.Require().Error(err)
if tc.expectedErrMsg != "" {
suite.Require().ErrorContains(err, tc.expectedErrMsg)
} else {
suite.Require().NoError(err)
}
Expand Down
1 change: 1 addition & 0 deletions x/gov/migrations/v6/store.go
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,7 @@ func MigrateStore(ctx sdk.Context, paramsCollection collections.Item[v1.Params],
defaultParams := v1.DefaultParams()
govParams.OptimisticAuthorizedAddresses = defaultParams.OptimisticAuthorizedAddresses
govParams.OptimisticRejectedThreshold = defaultParams.OptimisticRejectedThreshold
govParams.ProposalCancelMaxPeriod = defaultParams.ProposalCancelMaxPeriod

return paramsCollection.Set(ctx, govParams)
}
36 changes: 22 additions & 14 deletions x/gov/simulation/genesis.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,19 +17,20 @@ import (

// Simulation parameter constants
const (
MinDeposit = "min_deposit"
ExpeditedMinDeposit = "expedited_min_deposit"
DepositPeriod = "deposit_period"
MinInitialRatio = "min_initial_ratio"
VotingPeriod = "voting_period"
ExpeditedVotingPeriod = "expedited_voting_period"
Quorum = "quorum"
Threshold = "threshold"
ExpeditedThreshold = "expedited_threshold"
Veto = "veto"
OptimisticRejectedThreshold = "optimistic_rejected_threshold"
ProposalCancelRate = "proposal_cancel_rate"
MinDepositRatio = "min_deposit_ratio"
MinDeposit = "min_deposit"
ExpeditedMinDeposit = "expedited_min_deposit"
DepositPeriod = "deposit_period"
MinInitialRatio = "min_initial_ratio"
VotingPeriod = "voting_period"
ExpeditedVotingPeriod = "expedited_voting_period"
Quorum = "quorum"
Threshold = "threshold"
ExpeditedThreshold = "expedited_threshold"
Veto = "veto"
OptimisticRejectedThreshold = "optimistic_rejected_threshold"
ProposalCancelRate = "proposal_cancel_rate"
ProposalMaxCancelVotingPeriod = "proposal_max_cancel_voting_period"
MinDepositRatio = "min_deposit_ratio"

// ExpeditedThreshold must be at least as large as the regular Threshold
// Therefore, we use this break out point in randomization.
Expand Down Expand Up @@ -66,6 +67,10 @@ func GenProposalCancelRate(r *rand.Rand) sdkmath.LegacyDec {
return sdkmath.LegacyNewDec(int64(simulation.RandIntBetween(r, 0, 99))).Quo(sdkmath.LegacyNewDec(100))
}

func GenProposalMaxCancelVotingPeriod(r *rand.Rand) sdkmath.LegacyDec {
return sdkmath.LegacyNewDec(int64(simulation.RandIntBetween(r, 0, 99))).Quo(sdkmath.LegacyNewDec(100))
}

// GenVotingPeriod returns randomized VotingPeriod
func GenVotingPeriod(r *rand.Rand) time.Duration {
return time.Duration(simulation.RandIntBetween(r, expeditedMaxVotingPeriod, 2*expeditedMaxVotingPeriod)) * time.Second
Expand Down Expand Up @@ -125,6 +130,9 @@ func RandomizedGenState(simState *module.SimulationState) {
var proposalCancelRate sdkmath.LegacyDec
simState.AppParams.GetOrGenerate(ProposalCancelRate, &proposalCancelRate, simState.Rand, func(r *rand.Rand) { proposalCancelRate = GenProposalCancelRate(r) })

var proposalMaxCancelVotingPeriod sdkmath.LegacyDec
simState.AppParams.GetOrGenerate(ProposalMaxCancelVotingPeriod, &proposalMaxCancelVotingPeriod, simState.Rand, func(r *rand.Rand) { proposalMaxCancelVotingPeriod = GenProposalMaxCancelVotingPeriod(r) })

var votingPeriod time.Duration
simState.AppParams.GetOrGenerate(VotingPeriod, &votingPeriod, simState.Rand, func(r *rand.Rand) { votingPeriod = GenVotingPeriod(r) })

Expand All @@ -151,7 +159,7 @@ func RandomizedGenState(simState *module.SimulationState) {

govGenesis := v1.NewGenesisState(
startingProposalID,
v1.NewParams(minDeposit, expeditedMinDeposit, depositPeriod, votingPeriod, expeditedVotingPeriod, quorum.String(), threshold.String(), expitedVotingThreshold.String(), veto.String(), minInitialDepositRatio.String(), proposalCancelRate.String(), "", simState.Rand.Intn(2) == 0, simState.Rand.Intn(2) == 0, simState.Rand.Intn(2) == 0, minDepositRatio.String(), optimisticRejectedThreshold.String(), []string{}),
v1.NewParams(minDeposit, expeditedMinDeposit, depositPeriod, votingPeriod, expeditedVotingPeriod, quorum.String(), threshold.String(), expitedVotingThreshold.String(), veto.String(), minInitialDepositRatio.String(), proposalCancelRate.String(), "", proposalMaxCancelVotingPeriod.String(), simState.Rand.Intn(2) == 0, simState.Rand.Intn(2) == 0, simState.Rand.Intn(2) == 0, minDepositRatio.String(), optimisticRejectedThreshold.String(), []string{}),
)

bz, err := json.MarshalIndent(&govGenesis, "", " ")
Expand Down
14 changes: 8 additions & 6 deletions x/gov/simulation/genesis_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -46,22 +46,24 @@ func TestRandomizedGenState(t *testing.T) {
simState.Cdc.MustUnmarshalJSON(simState.GenState[types.ModuleName], &govGenesis)

const (
tallyQuorum = "0.350000000000000000"
tallyThreshold = "0.495000000000000000"
tallyExpeditedThreshold = "0.545000000000000000"
tallyVetoThreshold = "0.327000000000000000"
tallyQuorum = "0.387000000000000000"
tallyThreshold = "0.452000000000000000"
tallyExpeditedThreshold = "0.537000000000000000"
tallyVetoThreshold = "0.276000000000000000"
minInitialDepositDec = "0.880000000000000000"
proposalCancelMaxPeriod = "0.110000000000000000"
)

assert.Equal(t, "272stake", govGenesis.Params.MinDeposit[0].String())
assert.Equal(t, "800stake", govGenesis.Params.ExpeditedMinDeposit[0].String())
assert.Equal(t, "41h11m36s", govGenesis.Params.MaxDepositPeriod.String())
assert.Equal(t, float64(283889), govGenesis.Params.VotingPeriod.Seconds())
assert.Equal(t, float64(123081), govGenesis.Params.ExpeditedVotingPeriod.Seconds())
assert.Equal(t, float64(291928), govGenesis.Params.VotingPeriod.Seconds())
assert.Equal(t, float64(33502), govGenesis.Params.ExpeditedVotingPeriod.Seconds())
assert.Equal(t, tallyQuorum, govGenesis.Params.Quorum)
assert.Equal(t, tallyThreshold, govGenesis.Params.Threshold)
assert.Equal(t, tallyExpeditedThreshold, govGenesis.Params.ExpeditedThreshold)
assert.Equal(t, tallyVetoThreshold, govGenesis.Params.VetoThreshold)
assert.Equal(t, proposalCancelMaxPeriod, govGenesis.Params.ProposalCancelMaxPeriod)
assert.Equal(t, uint64(0x28), govGenesis.StartingProposalId)
assert.DeepEqual(t, []*v1.Deposit{}, govGenesis.Deposits)
assert.DeepEqual(t, []*v1.Vote{}, govGenesis.Votes)
Expand Down
1 change: 1 addition & 0 deletions x/gov/types/errors.go
Original file line number Diff line number Diff line change
Expand Up @@ -26,4 +26,5 @@ var (
ErrSummaryTooLong = errors.Register(ModuleName, 22, "summary too long")
ErrInvalidDepositDenom = errors.Register(ModuleName, 23, "invalid deposit denom")
ErrTitleTooLong = errors.Register(ModuleName, 24, "title too long")
ErrTooLateToCancel = errors.Register(ModuleName, 25, "too late to cancel proposal")
)
Loading

0 comments on commit e1e8c46

Please sign in to comment.