From ea3718b23883897ad55154856e0abe9bbc7759e6 Mon Sep 17 00:00:00 2001 From: Thomas Bruyelle Date: Fri, 21 Jul 2023 11:50:35 +0200 Subject: [PATCH] test(gov): add test for `Tally` function Although rather complex, the `govKeeper.Tally` function has no unit tests. This change adds a test that covers around 91% of the code, only some unexpected errors are not covered. If this change is accepted, another will follow to refactor the function into smaller parts (without changing the test). This will address the TODO on top, reduce complexity, improve readability and reusability. This test should also help for issues like #11422 and #10353. It's more comfortable to improve performance or completely rewrite the implementation with a high code coverage. The `setupGovKeeper` had to be modified because it was registering some mocks expectations that cannot be overridden. It now takes an additional variadic argument that can be used to better customize the mocks expectations. Also, to improve readability, the mocks are all gathered in a new specific `mocks` struct. --- x/gov/keeper/common_test.go | 72 +++--- x/gov/keeper/deposit_test.go | 8 +- x/gov/keeper/hooks_test.go | 3 +- x/gov/keeper/keeper_test.go | 14 +- x/gov/keeper/tally_test.go | 417 +++++++++++++++++++++++++++++++++++ x/gov/keeper/vote_test.go | 3 +- 6 files changed, 480 insertions(+), 37 deletions(-) create mode 100644 x/gov/keeper/tally_test.go diff --git a/x/gov/keeper/common_test.go b/x/gov/keeper/common_test.go index 74247f7b5730..0b307b9bd806 100644 --- a/x/gov/keeper/common_test.go +++ b/x/gov/keeper/common_test.go @@ -54,13 +54,38 @@ func getTestProposal() []sdk.Msg { } } +type mocks struct { + acctKeeper *govtestutil.MockAccountKeeper + bankKeeper *govtestutil.MockBankKeeper + stakingKeeper *govtestutil.MockStakingKeeper + distributionKeeper *govtestutil.MockDistributionKeeper +} + +func mockAccountKeeperExpectations(ctx sdk.Context, m mocks) { + m.acctKeeper.EXPECT().GetModuleAddress(types.ModuleName).Return(govAcct).AnyTimes() + m.acctKeeper.EXPECT().GetModuleAddress(disttypes.ModuleName).Return(distAcct).AnyTimes() + m.acctKeeper.EXPECT().GetModuleAccount(gomock.Any(), types.ModuleName).Return(authtypes.NewEmptyModuleAccount(types.ModuleName)).AnyTimes() + m.acctKeeper.EXPECT().AddressCodec().Return(address.NewBech32Codec("cosmos")).AnyTimes() +} + +func mockDefaultExpectations(ctx sdk.Context, m mocks) { + mockAccountKeeperExpectations(ctx, m) + trackMockBalances(m.bankKeeper, m.distributionKeeper) + m.stakingKeeper.EXPECT().TokensFromConsensusPower(ctx, gomock.Any()).DoAndReturn(func(ctx sdk.Context, power int64) math.Int { + return sdk.TokensFromConsensusPower(power, math.NewIntFromUint64(1000000)) + }).AnyTimes() + + m.stakingKeeper.EXPECT().BondDenom(ctx).Return("stake", nil).AnyTimes() + m.stakingKeeper.EXPECT().IterateBondedValidatorsByPower(gomock.Any(), gomock.Any()).AnyTimes() + m.stakingKeeper.EXPECT().IterateDelegations(gomock.Any(), gomock.Any(), gomock.Any()).AnyTimes() + m.stakingKeeper.EXPECT().TotalBondedTokens(gomock.Any()).Return(math.NewInt(10000000), nil).AnyTimes() + m.distributionKeeper.EXPECT().FundCommunityPool(gomock.Any(), gomock.Any(), gomock.Any()).Return(nil).AnyTimes() +} + // setupGovKeeper creates a govKeeper as well as all its dependencies. -func setupGovKeeper(t *testing.T) ( +func setupGovKeeper(t *testing.T, expectations ...func(sdk.Context, mocks)) ( *keeper.Keeper, - *govtestutil.MockAccountKeeper, - *govtestutil.MockBankKeeper, - *govtestutil.MockStakingKeeper, - *govtestutil.MockDistributionKeeper, + mocks, moduletestutil.TestEncodingConfig, sdk.Context, ) { @@ -80,30 +105,23 @@ func setupGovKeeper(t *testing.T) ( // gomock initializations ctrl := gomock.NewController(t) - acctKeeper := govtestutil.NewMockAccountKeeper(ctrl) - bankKeeper := govtestutil.NewMockBankKeeper(ctrl) - stakingKeeper := govtestutil.NewMockStakingKeeper(ctrl) - distributionKeeper := govtestutil.NewMockDistributionKeeper(ctrl) - - acctKeeper.EXPECT().GetModuleAddress(types.ModuleName).Return(govAcct).AnyTimes() - acctKeeper.EXPECT().GetModuleAddress(disttypes.ModuleName).Return(distAcct).AnyTimes() - acctKeeper.EXPECT().GetModuleAccount(gomock.Any(), types.ModuleName).Return(authtypes.NewEmptyModuleAccount(types.ModuleName)).AnyTimes() - acctKeeper.EXPECT().AddressCodec().Return(address.NewBech32Codec("cosmos")).AnyTimes() - - trackMockBalances(bankKeeper, distributionKeeper) - stakingKeeper.EXPECT().TokensFromConsensusPower(ctx, gomock.Any()).DoAndReturn(func(ctx sdk.Context, power int64) math.Int { - return sdk.TokensFromConsensusPower(power, math.NewIntFromUint64(1000000)) - }).AnyTimes() - - stakingKeeper.EXPECT().BondDenom(ctx).Return("stake", nil).AnyTimes() - stakingKeeper.EXPECT().IterateBondedValidatorsByPower(gomock.Any(), gomock.Any()).AnyTimes() - stakingKeeper.EXPECT().IterateDelegations(gomock.Any(), gomock.Any(), gomock.Any()).AnyTimes() - stakingKeeper.EXPECT().TotalBondedTokens(gomock.Any()).Return(math.NewInt(10000000), nil).AnyTimes() - distributionKeeper.EXPECT().FundCommunityPool(gomock.Any(), gomock.Any(), gomock.Any()).Return(nil).AnyTimes() + m := mocks{ + acctKeeper: govtestutil.NewMockAccountKeeper(ctrl), + bankKeeper: govtestutil.NewMockBankKeeper(ctrl), + stakingKeeper: govtestutil.NewMockStakingKeeper(ctrl), + distributionKeeper: govtestutil.NewMockDistributionKeeper(ctrl), + } + if len(expectations) == 0 { + mockDefaultExpectations(ctx, m) + } else { + for _, exp := range expectations { + exp(ctx, m) + } + } // Gov keeper initializations - govKeeper := keeper.NewKeeper(encCfg.Codec, storeService, acctKeeper, bankKeeper, stakingKeeper, distributionKeeper, msr, types.DefaultConfig(), govAcct.String()) + govKeeper := keeper.NewKeeper(encCfg.Codec, storeService, m.acctKeeper, m.bankKeeper, m.stakingKeeper, m.distributionKeeper, msr, types.DefaultConfig(), govAcct.String()) require.NoError(t, govKeeper.ProposalID.Set(ctx, 1)) govRouter := v1beta1.NewRouter() // Also register legacy gov handlers to test them too. govRouter.AddRoute(types.RouterKey, v1beta1.ProposalHandler) @@ -118,7 +136,7 @@ func setupGovKeeper(t *testing.T) ( v1.RegisterMsgServer(msr, keeper.NewMsgServerImpl(govKeeper)) banktypes.RegisterMsgServer(msr, nil) // Nil is fine here as long as we never execute the proposal's Msgs. - return govKeeper, acctKeeper, bankKeeper, stakingKeeper, distributionKeeper, encCfg, ctx + return govKeeper, m, encCfg, ctx } // trackMockBalances sets up expected calls on the Mock BankKeeper, and also diff --git a/x/gov/keeper/deposit_test.go b/x/gov/keeper/deposit_test.go index 420b4aee4c09..97baf54321a5 100644 --- a/x/gov/keeper/deposit_test.go +++ b/x/gov/keeper/deposit_test.go @@ -38,7 +38,8 @@ func TestDeposits(t *testing.T) { for _, tc := range testcases { t.Run(tc.name, func(t *testing.T) { - govKeeper, authKeeper, bankKeeper, stakingKeeper, distKeeper, _, ctx := setupGovKeeper(t) + govKeeper, mocks, _, ctx := setupGovKeeper(t) + authKeeper, bankKeeper, stakingKeeper, distKeeper := mocks.acctKeeper, mocks.bankKeeper, mocks.stakingKeeper, mocks.distributionKeeper trackMockBalances(bankKeeper, distKeeper) // With expedited proposals the minimum deposit is higher, so we must @@ -249,7 +250,7 @@ func TestValidateInitialDeposit(t *testing.T) { for name, tc := range testcases { t.Run(name, func(t *testing.T) { - govKeeper, _, _, _, _, _, ctx := setupGovKeeper(t) + govKeeper, _, _, ctx := setupGovKeeper(t) params := v1.DefaultParams() if tc.expedited { @@ -312,7 +313,8 @@ func TestChargeDeposit(t *testing.T) { } t.Run(testName(i), func(t *testing.T) { - govKeeper, authKeeper, bankKeeper, stakingKeeper, _, _, ctx := setupGovKeeper(t) + govKeeper, mocks, _, ctx := setupGovKeeper(t) + authKeeper, bankKeeper, stakingKeeper := mocks.acctKeeper, mocks.bankKeeper, mocks.stakingKeeper params := v1.DefaultParams() params.ProposalCancelRatio = tc.proposalCancelRatio TestAddrs := simtestutil.AddTestAddrsIncremental(bankKeeper, stakingKeeper, ctx, 2, sdkmath.NewInt(10000000000)) diff --git a/x/gov/keeper/hooks_test.go b/x/gov/keeper/hooks_test.go index 59f1d73c43f5..8d0f0d9e3b47 100644 --- a/x/gov/keeper/hooks_test.go +++ b/x/gov/keeper/hooks_test.go @@ -49,7 +49,8 @@ func (h *MockGovHooksReceiver) AfterProposalVotingPeriodEnded(ctx context.Contex func TestHooks(t *testing.T) { minDeposit := v1.DefaultParams().MinDeposit - govKeeper, authKeeper, bankKeeper, stakingKeeper, _, _, ctx := setupGovKeeper(t) + govKeeper, mocks, _, ctx := setupGovKeeper(t) + authKeeper, bankKeeper, stakingKeeper := mocks.acctKeeper, mocks.bankKeeper, mocks.stakingKeeper addrs := simtestutil.AddTestAddrs(bankKeeper, stakingKeeper, ctx, 1, minDeposit[0].Amount) authKeeper.EXPECT().AddressCodec().Return(address.NewBech32Codec("cosmos")).AnyTimes() diff --git a/x/gov/keeper/keeper_test.go b/x/gov/keeper/keeper_test.go index 51e8737f0c84..6d4413b3a43d 100644 --- a/x/gov/keeper/keeper_test.go +++ b/x/gov/keeper/keeper_test.go @@ -45,7 +45,8 @@ func (suite *KeeperTestSuite) SetupSuite() { } func (suite *KeeperTestSuite) reset() { - govKeeper, acctKeeper, bankKeeper, stakingKeeper, distKeeper, encCfg, ctx := setupGovKeeper(suite.T()) + govKeeper, mocks, encCfg, ctx := setupGovKeeper(suite.T()) + acctKeeper, bankKeeper, stakingKeeper, distKeeper := mocks.acctKeeper, mocks.bankKeeper, mocks.stakingKeeper, mocks.distributionKeeper // Populate the gov account with some coins, as the TestProposal we have // is a MsgSend from the gov account. @@ -80,7 +81,8 @@ func (suite *KeeperTestSuite) reset() { } func TestIncrementProposalNumber(t *testing.T) { - govKeeper, authKeeper, _, _, _, _, ctx := setupGovKeeper(t) + govKeeper, mocks, _, ctx := setupGovKeeper(t) + authKeeper := mocks.acctKeeper authKeeper.EXPECT().AddressCodec().Return(address.NewBech32Codec("cosmos")).AnyTimes() @@ -106,7 +108,8 @@ func TestIncrementProposalNumber(t *testing.T) { } func TestProposalQueues(t *testing.T) { - govKeeper, authKeeper, _, _, _, _, ctx := setupGovKeeper(t) + govKeeper, mocks, _, ctx := setupGovKeeper(t) + authKeeper := mocks.acctKeeper ac := address.NewBech32Codec("cosmos") addrBz, err := ac.StringToBytes(address1) @@ -133,7 +136,7 @@ func TestProposalQueues(t *testing.T) { } func TestSetHooks(t *testing.T) { - govKeeper, _, _, _, _, _, _ := setupGovKeeper(t) + govKeeper, _, _, _ := setupGovKeeper(t) require.Empty(t, govKeeper.Hooks()) govHooksReceiver := MockGovHooksReceiver{} @@ -145,7 +148,8 @@ func TestSetHooks(t *testing.T) { } func TestGetGovGovernanceAndModuleAccountAddress(t *testing.T) { - govKeeper, authKeeper, _, _, _, _, ctx := setupGovKeeper(t) + govKeeper, mocks, _, ctx := setupGovKeeper(t) + authKeeper := mocks.acctKeeper mAcc := authKeeper.GetModuleAccount(ctx, "gov") require.Equal(t, mAcc, govKeeper.GetGovernanceAccount(ctx)) mAddr := authKeeper.GetModuleAddress("gov") diff --git a/x/gov/keeper/tally_test.go b/x/gov/keeper/tally_test.go new file mode 100644 index 000000000000..eb4a196690de --- /dev/null +++ b/x/gov/keeper/tally_test.go @@ -0,0 +1,417 @@ +package keeper_test + +import ( + "context" + "testing" + + "github.com/golang/mock/gomock" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + + sdkmath "cosmossdk.io/math" + + simtestutil "github.com/cosmos/cosmos-sdk/testutil/sims" + sdk "github.com/cosmos/cosmos-sdk/types" + "github.com/cosmos/cosmos-sdk/x/gov/keeper" + v1 "github.com/cosmos/cosmos-sdk/x/gov/types/v1" + stakingtypes "github.com/cosmos/cosmos-sdk/x/staking/types" +) + +func TestTally(t *testing.T) { + type suite struct { + t *testing.T + proposal v1.Proposal + valAddrs []sdk.ValAddress + delAddrs []sdk.AccAddress + keeper *keeper.Keeper + ctx sdk.Context + mocks mocks + } + + var ( + // handy functions + setTotalBonded = func(s suite, n int64) { + s.mocks.stakingKeeper.EXPECT().TotalBondedTokens(gomock.Any()). + Return(sdkmath.NewInt(n), nil) + } + delegatorVote = func(s suite, voter sdk.AccAddress, delegations []stakingtypes.Delegation, vote v1.VoteOption) { + err := s.keeper.AddVote(s.ctx, s.proposal.Id, voter, v1.NewNonSplitVoteOption(vote), "") + require.NoError(s.t, err) + s.mocks.stakingKeeper.EXPECT(). + IterateDelegations(s.ctx, voter, gomock.Any()). + DoAndReturn( + func(ctx context.Context, voter sdk.AccAddress, fn func(index int64, d stakingtypes.DelegationI) bool) error { + for i, d := range delegations { + fn(int64(i), d) + } + return nil + }) + } + validatorVote = func(s suite, voter sdk.ValAddress, vote v1.VoteOption) { + // validatorVote is like delegatorVote but without delegations + delegatorVote(s, sdk.AccAddress(voter), nil, vote) + } + ) + tests := []struct { + name string + expedited bool + setup func(suite) + expectedPass bool + expectedBurn bool + expectedTally v1.TallyResult + expectedError string + }{ + { + name: "no votes, no bonded tokens: prop fails", + setup: func(s suite) { + setTotalBonded(s, 0) + }, + expectedPass: false, + expectedBurn: false, + expectedTally: v1.TallyResult{ + YesCount: "0", + AbstainCount: "0", + NoCount: "0", + NoWithVetoCount: "0", + }, + }, + { + name: "no votes: prop fails/burn deposit", + setup: func(s suite) { + setTotalBonded(s, 10000000) + }, + expectedPass: false, + expectedBurn: true, // burn because quorum not reached + expectedTally: v1.TallyResult{ + YesCount: "0", + AbstainCount: "0", + NoCount: "0", + NoWithVetoCount: "0", + }, + }, + { + name: "one validator votes: prop fails/burn deposit", + setup: func(s suite) { + setTotalBonded(s, 10000000) + validatorVote(s, s.valAddrs[0], v1.VoteOption_VOTE_OPTION_NO) + }, + expectedPass: false, + expectedBurn: true, // burn because quorum not reached + expectedTally: v1.TallyResult{ + YesCount: "0", + AbstainCount: "0", + NoCount: "1000000", + NoWithVetoCount: "0", + }, + }, + { + name: "one account votes without delegation: prop fails/burn deposit", + setup: func(s suite) { + setTotalBonded(s, 10000000) + delegatorVote(s, s.delAddrs[0], nil, v1.VoteOption_VOTE_OPTION_YES) + }, + expectedPass: false, + expectedBurn: true, // burn because quorum not reached + expectedTally: v1.TallyResult{ + YesCount: "0", + AbstainCount: "0", + NoCount: "0", + NoWithVetoCount: "0", + }, + }, + { + name: "one delegator votes: prop fails/burn deposit", + setup: func(s suite) { + setTotalBonded(s, 10000000) + delegations := []stakingtypes.Delegation{{ + DelegatorAddress: s.delAddrs[0].String(), + ValidatorAddress: s.valAddrs[0].String(), + Shares: sdkmath.LegacyNewDec(42), + }} + delegatorVote(s, s.delAddrs[0], delegations, v1.VoteOption_VOTE_OPTION_YES) + }, + expectedPass: false, + expectedBurn: true, // burn because quorum not reached + expectedTally: v1.TallyResult{ + YesCount: "42", + AbstainCount: "0", + NoCount: "0", + NoWithVetoCount: "0", + }, + }, + { + name: "one delegator votes yes, validator votes also yes: prop fails/burn deposit", + setup: func(s suite) { + setTotalBonded(s, 10000000) + delegations := []stakingtypes.Delegation{{ + DelegatorAddress: s.delAddrs[0].String(), + ValidatorAddress: s.valAddrs[0].String(), + Shares: sdkmath.LegacyNewDec(42), + }} + delegatorVote(s, s.delAddrs[0], delegations, v1.VoteOption_VOTE_OPTION_YES) + validatorVote(s, s.valAddrs[0], v1.VoteOption_VOTE_OPTION_YES) + }, + expectedPass: false, + expectedBurn: true, // burn because quorum not reached + expectedTally: v1.TallyResult{ + YesCount: "1000000", + AbstainCount: "0", + NoCount: "0", + NoWithVetoCount: "0", + }, + }, + { + name: "one delegator votes yes, validator votes no: prop fails/burn deposit", + setup: func(s suite) { + setTotalBonded(s, 10000000) + delegations := []stakingtypes.Delegation{{ + DelegatorAddress: s.delAddrs[0].String(), + ValidatorAddress: s.valAddrs[0].String(), + Shares: sdkmath.LegacyNewDec(42), + }} + delegatorVote(s, s.delAddrs[0], delegations, v1.VoteOption_VOTE_OPTION_YES) + validatorVote(s, s.valAddrs[0], v1.VoteOption_VOTE_OPTION_NO) + }, + expectedPass: false, + expectedBurn: true, // burn because quorum not reached + expectedTally: v1.TallyResult{ + YesCount: "42", + AbstainCount: "0", + NoCount: "999958", + NoWithVetoCount: "0", + }, + }, + { + // one delegator delegates 42 shares to 2 different validators (21 each) + // delegator votes yes + // first validator votes yes + // second validator votes no + // third validator (no delegation) votes abstain + name: "delegator with mixed delegations: prop fails/burn deposit", + setup: func(s suite) { + setTotalBonded(s, 10000000) + delegations := []stakingtypes.Delegation{ + { + DelegatorAddress: s.delAddrs[0].String(), + ValidatorAddress: s.valAddrs[0].String(), + Shares: sdkmath.LegacyNewDec(21), + }, + { + DelegatorAddress: s.delAddrs[0].String(), + ValidatorAddress: s.valAddrs[1].String(), + Shares: sdkmath.LegacyNewDec(21), + }, + } + delegatorVote(s, s.delAddrs[0], delegations, v1.VoteOption_VOTE_OPTION_YES) + validatorVote(s, s.valAddrs[0], v1.VoteOption_VOTE_OPTION_NO) + validatorVote(s, s.valAddrs[1], v1.VoteOption_VOTE_OPTION_YES) + validatorVote(s, s.valAddrs[2], v1.VoteOption_VOTE_OPTION_ABSTAIN) + }, + expectedPass: false, + expectedBurn: true, // burn because quorum not reached + expectedTally: v1.TallyResult{ + YesCount: "1000021", + AbstainCount: "1000000", + NoCount: "999979", + NoWithVetoCount: "0", + }, + }, + { + name: "quorum reached with only abstain: prop fails", + setup: func(s suite) { + setTotalBonded(s, 10000000) + validatorVote(s, s.valAddrs[0], v1.VoteOption_VOTE_OPTION_ABSTAIN) + validatorVote(s, s.valAddrs[1], v1.VoteOption_VOTE_OPTION_ABSTAIN) + validatorVote(s, s.valAddrs[2], v1.VoteOption_VOTE_OPTION_ABSTAIN) + validatorVote(s, s.valAddrs[3], v1.VoteOption_VOTE_OPTION_ABSTAIN) + }, + expectedPass: false, + expectedBurn: false, + expectedTally: v1.TallyResult{ + YesCount: "0", + AbstainCount: "4000000", + NoCount: "0", + NoWithVetoCount: "0", + }, + }, + { + name: "quorum reached with veto>1/3: prop fails/burn deposit", + setup: func(s suite) { + setTotalBonded(s, 10000000) + validatorVote(s, s.valAddrs[0], v1.VoteOption_VOTE_OPTION_YES) + validatorVote(s, s.valAddrs[1], v1.VoteOption_VOTE_OPTION_YES) + validatorVote(s, s.valAddrs[2], v1.VoteOption_VOTE_OPTION_YES) + validatorVote(s, s.valAddrs[3], v1.VoteOption_VOTE_OPTION_YES) + validatorVote(s, s.valAddrs[4], v1.VoteOption_VOTE_OPTION_NO_WITH_VETO) + validatorVote(s, s.valAddrs[5], v1.VoteOption_VOTE_OPTION_NO_WITH_VETO) + validatorVote(s, s.valAddrs[6], v1.VoteOption_VOTE_OPTION_NO_WITH_VETO) + }, + expectedPass: false, + expectedBurn: true, + expectedTally: v1.TallyResult{ + YesCount: "4000000", + AbstainCount: "0", + NoCount: "0", + NoWithVetoCount: "3000000", + }, + }, + { + name: "quorum reached with yes<=.5: prop fails", + setup: func(s suite) { + setTotalBonded(s, 10000000) + validatorVote(s, s.valAddrs[0], v1.VoteOption_VOTE_OPTION_YES) + validatorVote(s, s.valAddrs[1], v1.VoteOption_VOTE_OPTION_YES) + validatorVote(s, s.valAddrs[2], v1.VoteOption_VOTE_OPTION_NO) + validatorVote(s, s.valAddrs[3], v1.VoteOption_VOTE_OPTION_NO) + }, + expectedPass: false, + expectedBurn: false, + expectedTally: v1.TallyResult{ + YesCount: "2000000", + AbstainCount: "0", + NoCount: "2000000", + NoWithVetoCount: "0", + }, + }, + { + name: "quorum reached with yes>.5: prop succeeds", + setup: func(s suite) { + setTotalBonded(s, 10000000) + validatorVote(s, s.valAddrs[0], v1.VoteOption_VOTE_OPTION_YES) + validatorVote(s, s.valAddrs[1], v1.VoteOption_VOTE_OPTION_YES) + validatorVote(s, s.valAddrs[2], v1.VoteOption_VOTE_OPTION_YES) + validatorVote(s, s.valAddrs[3], v1.VoteOption_VOTE_OPTION_YES) + validatorVote(s, s.valAddrs[4], v1.VoteOption_VOTE_OPTION_NO) + validatorVote(s, s.valAddrs[5], v1.VoteOption_VOTE_OPTION_NO) + validatorVote(s, s.valAddrs[6], v1.VoteOption_VOTE_OPTION_NO_WITH_VETO) + }, + expectedPass: true, + expectedBurn: false, + expectedTally: v1.TallyResult{ + YesCount: "4000000", + AbstainCount: "0", + NoCount: "2000000", + NoWithVetoCount: "1000000", + }, + }, + { + name: "quorum reached thanks to abstain, yes>.5: prop succeeds", + setup: func(s suite) { + setTotalBonded(s, 10000000) + validatorVote(s, s.valAddrs[0], v1.VoteOption_VOTE_OPTION_YES) + validatorVote(s, s.valAddrs[1], v1.VoteOption_VOTE_OPTION_YES) + validatorVote(s, s.valAddrs[2], v1.VoteOption_VOTE_OPTION_NO) + validatorVote(s, s.valAddrs[3], v1.VoteOption_VOTE_OPTION_ABSTAIN) + validatorVote(s, s.valAddrs[4], v1.VoteOption_VOTE_OPTION_ABSTAIN) + validatorVote(s, s.valAddrs[5], v1.VoteOption_VOTE_OPTION_ABSTAIN) + }, + expectedPass: true, + expectedBurn: false, + expectedTally: v1.TallyResult{ + YesCount: "2000000", + AbstainCount: "3000000", + NoCount: "1000000", + NoWithVetoCount: "0", + }, + }, + { + name: "quorum reached with yes<=.667: expedited prop fails", + expedited: true, + setup: func(s suite) { + setTotalBonded(s, 10000000) + validatorVote(s, s.valAddrs[0], v1.VoteOption_VOTE_OPTION_YES) + validatorVote(s, s.valAddrs[1], v1.VoteOption_VOTE_OPTION_YES) + validatorVote(s, s.valAddrs[2], v1.VoteOption_VOTE_OPTION_YES) + validatorVote(s, s.valAddrs[3], v1.VoteOption_VOTE_OPTION_YES) + validatorVote(s, s.valAddrs[4], v1.VoteOption_VOTE_OPTION_NO) + validatorVote(s, s.valAddrs[5], v1.VoteOption_VOTE_OPTION_NO) + validatorVote(s, s.valAddrs[6], v1.VoteOption_VOTE_OPTION_NO_WITH_VETO) + }, + expectedPass: false, + expectedBurn: false, + expectedTally: v1.TallyResult{ + YesCount: "4000000", + AbstainCount: "0", + NoCount: "2000000", + NoWithVetoCount: "1000000", + }, + }, + { + name: "quorum reached with yes>.667: expedited prop succeeds", + expedited: true, + setup: func(s suite) { + setTotalBonded(s, 10000000) + validatorVote(s, s.valAddrs[0], v1.VoteOption_VOTE_OPTION_YES) + validatorVote(s, s.valAddrs[1], v1.VoteOption_VOTE_OPTION_YES) + validatorVote(s, s.valAddrs[2], v1.VoteOption_VOTE_OPTION_YES) + validatorVote(s, s.valAddrs[3], v1.VoteOption_VOTE_OPTION_YES) + validatorVote(s, s.valAddrs[4], v1.VoteOption_VOTE_OPTION_YES) + validatorVote(s, s.valAddrs[5], v1.VoteOption_VOTE_OPTION_NO) + validatorVote(s, s.valAddrs[6], v1.VoteOption_VOTE_OPTION_NO_WITH_VETO) + }, + expectedPass: true, + expectedBurn: false, + expectedTally: v1.TallyResult{ + YesCount: "5000000", + AbstainCount: "0", + NoCount: "1000000", + NoWithVetoCount: "1000000", + }, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + govKeeper, mocks, _, ctx := setupGovKeeper(t, mockAccountKeeperExpectations) + params := v1.DefaultParams() + // Ensure params value are different than false + params.BurnVoteQuorum = true + params.BurnVoteVeto = true + err := govKeeper.Params.Set(ctx, params) + require.NoError(t, err) + var ( + numVals = 10 + numDelegators = 5 + addrs = simtestutil.CreateRandomAccounts(numVals + numDelegators) + valAddrs = simtestutil.ConvertAddrsToValAddrs(addrs[:numVals]) + delAddrs = addrs[numVals:] + ) + // Mocks a bunch of validators + mocks.stakingKeeper.EXPECT(). + IterateBondedValidatorsByPower(ctx, gomock.Any()). + DoAndReturn( + func(ctx context.Context, fn func(index int64, validator stakingtypes.ValidatorI) bool) error { + for i := int64(0); i < int64(numVals); i++ { + fn(i, stakingtypes.Validator{ + OperatorAddress: valAddrs[i].String(), + Status: stakingtypes.Bonded, + Tokens: sdkmath.NewInt(1000000), + DelegatorShares: sdkmath.LegacyNewDec(1000000), + }) + } + return nil + }) + // Submit and activate a proposal + proposal, err := govKeeper.SubmitProposal(ctx, TestProposal, "", "title", "summary", delAddrs[0], tt.expedited) + require.NoError(t, err) + err = govKeeper.ActivateVotingPeriod(ctx, proposal) + require.NoError(t, err) + suite := suite{ + t: t, + proposal: proposal, + valAddrs: valAddrs, + delAddrs: delAddrs, + ctx: ctx, + keeper: govKeeper, + mocks: mocks, + } + tt.setup(suite) + + pass, burn, tally, err := govKeeper.Tally(ctx, proposal) + + require.NoError(t, err) + assert.Equal(t, tt.expectedPass, pass, "wrong pass") + assert.Equal(t, tt.expectedBurn, burn, "wrong burn") + assert.Equal(t, tt.expectedTally, tally) + }) + } +} diff --git a/x/gov/keeper/vote_test.go b/x/gov/keeper/vote_test.go index fc969f01a771..634c06156048 100644 --- a/x/gov/keeper/vote_test.go +++ b/x/gov/keeper/vote_test.go @@ -15,7 +15,8 @@ import ( ) func TestVotes(t *testing.T) { - govKeeper, authKeeper, bankKeeper, stakingKeeper, _, _, ctx := setupGovKeeper(t) + govKeeper, mocks, _, ctx := setupGovKeeper(t) + authKeeper, bankKeeper, stakingKeeper := mocks.acctKeeper, mocks.bankKeeper, mocks.stakingKeeper addrs := simtestutil.AddTestAddrsIncremental(bankKeeper, stakingKeeper, ctx, 2, sdkmath.NewInt(10000000)) authKeeper.EXPECT().AddressCodec().Return(address.NewBech32Codec("cosmos")).AnyTimes()