From f0a1898b089e52d68d242a0f08f42ed99dc3eaa8 Mon Sep 17 00:00:00 2001 From: Giuseppe Natale <12249307+giunatale@users.noreply.github.com> Date: Wed, 21 Feb 2024 14:41:12 +0100 Subject: [PATCH] feat(x/gov): validators can only vote with own stake (#20) Change #7 so that validators are allowed to vote, but cannot use delegations in governance to increase their voting power, they can only vote with their own stake. This seems more sensible wrt the previous solution of removing their ability to vote entirely. This also addresses #19 as I added comments in the README, and also che changelog should be good. Therefore with this PR we should be able to also merge the feat branch back into main and close #6 --- CHANGELOG.md | 4 +- README.md | 3 + app/modules.go | 4 +- x/gov/keeper/common_test.go | 2 +- x/gov/keeper/grpc_query_test.go | 13 +-- x/gov/keeper/tally.go | 4 + x/gov/keeper/tally_test.go | 159 +++++++--------------------- x/gov/keeper/vote.go | 6 -- x/gov/keeper/vote_test.go | 6 -- x/gov/module.go | 6 +- x/gov/simulation/operations.go | 36 +++---- x/gov/simulation/operations_test.go | 8 +- x/gov/types/errors.go | 1 - x/gov/types/expected_keepers.go | 2 - 14 files changed, 72 insertions(+), 182 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 8997510..53ef2fd 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -14,8 +14,8 @@ * Forked `x/gov` module from Cosmos SDK v0.46.16 for custom modifications ([#4](https://github.com/atomone-hub/govgen/pull/4)). -* Removed ability for validators to vote on proposals - ([#10](https://github.com/atomone-hub/govgen/pull/10)). +* Removed ability for validators to vote on proposals with delegations, they can only use their own stake + ([#20](https://github.com/atomone-hub/govgen/pull/20)). * Remove community spend proposal ([#13](https://github.com/atomone-hub/govgen/pull/13)). * Adapt voting period to proposal type. diff --git a/README.md b/README.md index cae0247..8fc15d9 100644 --- a/README.md +++ b/README.md @@ -11,3 +11,6 @@ The following modifications have been made to the Cosmos Hub software to create 4. Revert to standard Cosmos SDK v0.46.16 without the Liquid Staking Module (LSM) 5. Changed Bech32 prefixes to `govgen` (see `cmd/govgend/cmd/config.go`) 6. Reduced hard-coded ante min-deposit percentage to 1% (see `ante/gov_ante.go:minInitialDepositFraction`) + 7. Removed ability for validators to vote on proposals with delegations, they can only use their own stake + 8. Remove community spend proposal + 9. Allow selecting different voting periods for different proposal types diff --git a/app/modules.go b/app/modules.go index ed375ad..23993a9 100644 --- a/app/modules.go +++ b/app/modules.go @@ -95,7 +95,7 @@ func appModules( bank.NewAppModule(appCodec, app.BankKeeper, app.AccountKeeper), capability.NewAppModule(appCodec, *app.CapabilityKeeper), crisis.NewAppModule(&app.CrisisKeeper, skipGenesisInvariants), - gov.NewAppModule(appCodec, app.GovKeeper, app.AccountKeeper, app.BankKeeper, app.StakingKeeper), + gov.NewAppModule(appCodec, app.GovKeeper, app.AccountKeeper, app.BankKeeper), mint.NewAppModule(appCodec, app.MintKeeper, app.AccountKeeper), slashing.NewAppModule(appCodec, app.SlashingKeeper, app.AccountKeeper, app.BankKeeper, app.StakingKeeper), distr.NewAppModule(appCodec, app.DistrKeeper, app.AccountKeeper, app.BankKeeper, app.StakingKeeper), @@ -122,7 +122,7 @@ func simulationModules( bank.NewAppModule(appCodec, app.BankKeeper, app.AccountKeeper), capability.NewAppModule(appCodec, *app.CapabilityKeeper), feegrantmodule.NewAppModule(appCodec, app.AccountKeeper, app.BankKeeper, app.FeeGrantKeeper, app.interfaceRegistry), - gov.NewAppModule(appCodec, app.GovKeeper, app.AccountKeeper, app.BankKeeper, app.StakingKeeper), + gov.NewAppModule(appCodec, app.GovKeeper, app.AccountKeeper, app.BankKeeper), mint.NewAppModule(appCodec, app.MintKeeper, app.AccountKeeper), staking.NewAppModule(appCodec, app.StakingKeeper, app.AccountKeeper, app.BankKeeper), distr.NewAppModule(appCodec, app.DistrKeeper, app.AccountKeeper, app.BankKeeper, app.StakingKeeper), diff --git a/x/gov/keeper/common_test.go b/x/gov/keeper/common_test.go index 2cd0954..ab456a1 100644 --- a/x/gov/keeper/common_test.go +++ b/x/gov/keeper/common_test.go @@ -14,7 +14,7 @@ import ( govgenhelpers "github.com/atomone-hub/govgen/app/helpers" ) -func createValidators(t *testing.T, ctx sdk.Context, app *govgenapp.GovGenApp, powers []int64) ([]sdk.AccAddress, []sdk.ValAddress) { //nolint: thelper,unparam +func createValidators(t *testing.T, ctx sdk.Context, app *govgenapp.GovGenApp, powers []int64) ([]sdk.AccAddress, []sdk.ValAddress) { //nolint: thelper addrs := govgenhelpers.AddTestAddrsIncremental(app, ctx, 5, sdk.NewInt(30000000)) valAddrs := govgenhelpers.ConvertAddrsToValAddrs(addrs) pks := govgenhelpers.CreateTestPubKeys(5) diff --git a/x/gov/keeper/grpc_query_test.go b/x/gov/keeper/grpc_query_test.go index 221d618..0a32782 100644 --- a/x/gov/keeper/grpc_query_test.go +++ b/x/gov/keeper/grpc_query_test.go @@ -7,7 +7,6 @@ import ( sdk "github.com/cosmos/cosmos-sdk/types" "github.com/cosmos/cosmos-sdk/types/query" - stakingtypes "github.com/cosmos/cosmos-sdk/x/staking/types" govgenhelpers "github.com/atomone-hub/govgen/app/helpers" "github.com/atomone-hub/govgen/x/gov/types" @@ -715,17 +714,7 @@ func (suite *KeeperTestSuite) TestGRPCQueryDeposits() { func (suite *KeeperTestSuite) TestGRPCQueryTally() { app, ctx, queryClient := suite.app, suite.ctx, suite.queryClient - _, valAddrs := createValidators(suite.T(), ctx, app, []int64{5, 5, 5}) - addrs := govgenhelpers.AddTestAddrs(app, ctx, 3, sdk.NewInt(50000000)) - - delTokens := app.StakingKeeper.TokensFromConsensusPower(ctx, 5) - val1, found := app.StakingKeeper.GetValidator(ctx, valAddrs[0]) - suite.Require().True(found) - - for _, addr := range addrs { - _, err := app.StakingKeeper.Delegate(ctx, addr, delTokens, stakingtypes.Unbonded, val1, true) - suite.Require().NoError(err) - } + addrs, _ := createValidators(suite.T(), ctx, app, []int64{5, 5, 5}) var ( req *types.QueryTallyResultRequest diff --git a/x/gov/keeper/tally.go b/x/gov/keeper/tally.go index bb129d5..e07ca08 100644 --- a/x/gov/keeper/tally.go +++ b/x/gov/keeper/tally.go @@ -71,6 +71,8 @@ func (keeper Keeper) Tally(ctx sdk.Context, proposal types.Proposal) (passes boo return false }) + /* DISABLED on GovGen - Voting can only be done with your own stake + // iterate over the validators again to tally their voting power for _, val := range currValidators { if len(val.Vote) == 0 { @@ -87,6 +89,8 @@ func (keeper Keeper) Tally(ctx sdk.Context, proposal types.Proposal) (passes boo totalVotingPower = totalVotingPower.Add(votingPower) } + */ + tallyParams := keeper.GetTallyParams(ctx) tallyResults = types.NewTallyResultFromMap(results) diff --git a/x/gov/keeper/tally_test.go b/x/gov/keeper/tally_test.go index f1adf10..1e33f63 100644 --- a/x/gov/keeper/tally_test.go +++ b/x/gov/keeper/tally_test.go @@ -7,6 +7,7 @@ import ( tmproto "github.com/tendermint/tendermint/proto/tendermint/types" sdk "github.com/cosmos/cosmos-sdk/types" + "github.com/cosmos/cosmos-sdk/x/staking" stakingtypes "github.com/cosmos/cosmos-sdk/x/staking/types" govgenhelpers "github.com/atomone-hub/govgen/app/helpers" @@ -59,21 +60,11 @@ func TestTallyNoQuorum(t *testing.T) { require.True(t, burnDeposits) } -func TestTallyAllYes(t *testing.T) { +func TestTallyOnlyValidatorsAllYes(t *testing.T) { app := govgenhelpers.SetupNoValset(false) ctx := app.BaseApp.NewContext(false, tmproto.Header{}) - _, valAddrs := createValidators(t, ctx, app, []int64{1, 1, 1}) - addrs := govgenhelpers.AddTestAddrs(app, ctx, 3, sdk.NewInt(50000000)) - - delTokens := app.StakingKeeper.TokensFromConsensusPower(ctx, 5) - val1, found := app.StakingKeeper.GetValidator(ctx, valAddrs[0]) - require.True(t, found) - - for _, addr := range addrs { - _, err := app.StakingKeeper.Delegate(ctx, addr, delTokens, stakingtypes.Unbonded, val1, true) - require.NoError(t, err) - } + addrs, _ := createValidators(t, ctx, app, []int64{1, 1, 1}) tp := govgenhelpers.TestTextProposal @@ -96,22 +87,11 @@ func TestTallyAllYes(t *testing.T) { require.False(t, tallyResults.Equals(types.EmptyTallyResult())) } -func TestTally51No(t *testing.T) { +func TestTallyOnlyValidators51No(t *testing.T) { app := govgenhelpers.SetupNoValset(false) ctx := app.BaseApp.NewContext(false, tmproto.Header{}) - power := []int64{5, 6, 0} - _, valAddrs := createValidators(t, ctx, app, []int64{1, 1, 1}) - addrs := govgenhelpers.AddTestAddrs(app, ctx, 3, sdk.NewInt(50000000)) - - val1, found := app.StakingKeeper.GetValidator(ctx, valAddrs[0]) - require.True(t, found) - - for i := range addrs { - delTokens := app.StakingKeeper.TokensFromConsensusPower(ctx, power[i]) - _, err := app.StakingKeeper.Delegate(ctx, addrs[i], delTokens, stakingtypes.Unbonded, val1, true) - require.NoError(t, err) - } + valAccAddrs, _ := createValidators(t, ctx, app, []int64{5, 6, 0}) tp := govgenhelpers.TestTextProposal proposal, err := app.GovKeeper.SubmitProposal(ctx, tp) @@ -120,8 +100,8 @@ func TestTally51No(t *testing.T) { proposal.Status = types.StatusVotingPeriod app.GovKeeper.SetProposal(ctx, proposal) - require.NoError(t, app.GovKeeper.AddVote(ctx, proposalID, addrs[0], types.NewNonSplitVoteOption(types.OptionYes))) - require.NoError(t, app.GovKeeper.AddVote(ctx, proposalID, addrs[1], types.NewNonSplitVoteOption(types.OptionNo))) + require.NoError(t, app.GovKeeper.AddVote(ctx, proposalID, valAccAddrs[0], types.NewNonSplitVoteOption(types.OptionYes))) + require.NoError(t, app.GovKeeper.AddVote(ctx, proposalID, valAccAddrs[1], types.NewNonSplitVoteOption(types.OptionNo))) proposal, ok := app.GovKeeper.GetProposal(ctx, proposalID) require.True(t, ok) @@ -131,22 +111,11 @@ func TestTally51No(t *testing.T) { require.False(t, burnDeposits) } -func TestTally51Yes(t *testing.T) { +func TestTallyOnlyValidators51Yes(t *testing.T) { app := govgenhelpers.SetupNoValset(false) ctx := app.BaseApp.NewContext(false, tmproto.Header{}) - power := []int64{5, 6, 0} - _, valAddrs := createValidators(t, ctx, app, []int64{1, 1, 1}) - addrs := govgenhelpers.AddTestAddrs(app, ctx, 3, sdk.NewInt(50000000)) - - val1, found := app.StakingKeeper.GetValidator(ctx, valAddrs[0]) - require.True(t, found) - - for i := range addrs { - delTokens := app.StakingKeeper.TokensFromConsensusPower(ctx, power[i]) - _, err := app.StakingKeeper.Delegate(ctx, addrs[i], delTokens, stakingtypes.Unbonded, val1, true) - require.NoError(t, err) - } + valAccAddrs, _ := createValidators(t, ctx, app, []int64{5, 6, 0}) tp := govgenhelpers.TestTextProposal proposal, err := app.GovKeeper.SubmitProposal(ctx, tp) @@ -155,8 +124,8 @@ func TestTally51Yes(t *testing.T) { proposal.Status = types.StatusVotingPeriod app.GovKeeper.SetProposal(ctx, proposal) - require.NoError(t, app.GovKeeper.AddVote(ctx, proposalID, addrs[0], types.NewNonSplitVoteOption(types.OptionNo))) - require.NoError(t, app.GovKeeper.AddVote(ctx, proposalID, addrs[1], types.NewNonSplitVoteOption(types.OptionYes))) + require.NoError(t, app.GovKeeper.AddVote(ctx, proposalID, valAccAddrs[0], types.NewNonSplitVoteOption(types.OptionNo))) + require.NoError(t, app.GovKeeper.AddVote(ctx, proposalID, valAccAddrs[1], types.NewNonSplitVoteOption(types.OptionYes))) proposal, ok := app.GovKeeper.GetProposal(ctx, proposalID) require.True(t, ok) @@ -167,22 +136,11 @@ func TestTally51Yes(t *testing.T) { require.False(t, tallyResults.Equals(types.EmptyTallyResult())) } -func TestTallyVetoed(t *testing.T) { +func TestTallyOnlyValidatorsVetoed(t *testing.T) { app := govgenhelpers.SetupNoValset(false) ctx := app.BaseApp.NewContext(false, tmproto.Header{}) - power := []int64{6, 6, 7} - _, valAddrs := createValidators(t, ctx, app, []int64{1, 1, 1}) - addrs := govgenhelpers.AddTestAddrs(app, ctx, 3, sdk.NewInt(50000000)) - - val1, found := app.StakingKeeper.GetValidator(ctx, valAddrs[0]) - require.True(t, found) - - for i := range addrs { - delTokens := app.StakingKeeper.TokensFromConsensusPower(ctx, power[i]) - _, err := app.StakingKeeper.Delegate(ctx, addrs[i], delTokens, stakingtypes.Unbonded, val1, true) - require.NoError(t, err) - } + valAccAddrs, _ := createValidators(t, ctx, app, []int64{6, 6, 7}) tp := govgenhelpers.TestTextProposal proposal, err := app.GovKeeper.SubmitProposal(ctx, tp) @@ -191,9 +149,9 @@ func TestTallyVetoed(t *testing.T) { proposal.Status = types.StatusVotingPeriod app.GovKeeper.SetProposal(ctx, proposal) - require.NoError(t, app.GovKeeper.AddVote(ctx, proposalID, addrs[0], types.NewNonSplitVoteOption(types.OptionYes))) - require.NoError(t, app.GovKeeper.AddVote(ctx, proposalID, addrs[1], types.NewNonSplitVoteOption(types.OptionYes))) - require.NoError(t, app.GovKeeper.AddVote(ctx, proposalID, addrs[2], types.NewNonSplitVoteOption(types.OptionNoWithVeto))) + require.NoError(t, app.GovKeeper.AddVote(ctx, proposalID, valAccAddrs[0], types.NewNonSplitVoteOption(types.OptionYes))) + require.NoError(t, app.GovKeeper.AddVote(ctx, proposalID, valAccAddrs[1], types.NewNonSplitVoteOption(types.OptionYes))) + require.NoError(t, app.GovKeeper.AddVote(ctx, proposalID, valAccAddrs[2], types.NewNonSplitVoteOption(types.OptionNoWithVeto))) proposal, ok := app.GovKeeper.GetProposal(ctx, proposalID) require.True(t, ok) @@ -204,22 +162,11 @@ func TestTallyVetoed(t *testing.T) { require.False(t, tallyResults.Equals(types.EmptyTallyResult())) } -func TestTallyAbstainPasses(t *testing.T) { +func TestTallyOnlyValidatorsAbstainPasses(t *testing.T) { app := govgenhelpers.SetupNoValset(false) ctx := app.BaseApp.NewContext(false, tmproto.Header{}) - power := []int64{6, 6, 7} - _, valAddrs := createValidators(t, ctx, app, []int64{1, 1, 1}) - addrs := govgenhelpers.AddTestAddrs(app, ctx, 3, sdk.NewInt(50000000)) - - val1, found := app.StakingKeeper.GetValidator(ctx, valAddrs[0]) - require.True(t, found) - - for i := range addrs { - delTokens := app.StakingKeeper.TokensFromConsensusPower(ctx, power[i]) - _, err := app.StakingKeeper.Delegate(ctx, addrs[i], delTokens, stakingtypes.Unbonded, val1, true) - require.NoError(t, err) - } + valAccAddrs, _ := createValidators(t, ctx, app, []int64{6, 6, 7}) tp := govgenhelpers.TestTextProposal proposal, err := app.GovKeeper.SubmitProposal(ctx, tp) @@ -228,9 +175,9 @@ func TestTallyAbstainPasses(t *testing.T) { proposal.Status = types.StatusVotingPeriod app.GovKeeper.SetProposal(ctx, proposal) - require.NoError(t, app.GovKeeper.AddVote(ctx, proposalID, addrs[0], types.NewNonSplitVoteOption(types.OptionAbstain))) - require.NoError(t, app.GovKeeper.AddVote(ctx, proposalID, addrs[1], types.NewNonSplitVoteOption(types.OptionNo))) - require.NoError(t, app.GovKeeper.AddVote(ctx, proposalID, addrs[2], types.NewNonSplitVoteOption(types.OptionYes))) + require.NoError(t, app.GovKeeper.AddVote(ctx, proposalID, valAccAddrs[0], types.NewNonSplitVoteOption(types.OptionAbstain))) + require.NoError(t, app.GovKeeper.AddVote(ctx, proposalID, valAccAddrs[1], types.NewNonSplitVoteOption(types.OptionNo))) + require.NoError(t, app.GovKeeper.AddVote(ctx, proposalID, valAccAddrs[2], types.NewNonSplitVoteOption(types.OptionYes))) proposal, ok := app.GovKeeper.GetProposal(ctx, proposalID) require.True(t, ok) @@ -245,18 +192,7 @@ func TestTallyAbstainFails(t *testing.T) { app := govgenhelpers.SetupNoValset(false) ctx := app.BaseApp.NewContext(false, tmproto.Header{}) - power := []int64{6, 6, 7} - _, valAddrs := createValidators(t, ctx, app, []int64{1, 1, 1}) - addrs := govgenhelpers.AddTestAddrs(app, ctx, 3, sdk.NewInt(50000000)) - - val1, found := app.StakingKeeper.GetValidator(ctx, valAddrs[0]) - require.True(t, found) - - for i := range addrs { - delTokens := app.StakingKeeper.TokensFromConsensusPower(ctx, power[i]) - _, err := app.StakingKeeper.Delegate(ctx, addrs[i], delTokens, stakingtypes.Unbonded, val1, true) - require.NoError(t, err) - } + valAccAddrs, _ := createValidators(t, ctx, app, []int64{6, 6, 7}) tp := govgenhelpers.TestTextProposal proposal, err := app.GovKeeper.SubmitProposal(ctx, tp) @@ -265,9 +201,9 @@ func TestTallyAbstainFails(t *testing.T) { proposal.Status = types.StatusVotingPeriod app.GovKeeper.SetProposal(ctx, proposal) - require.NoError(t, app.GovKeeper.AddVote(ctx, proposalID, addrs[0], types.NewNonSplitVoteOption(types.OptionAbstain))) - require.NoError(t, app.GovKeeper.AddVote(ctx, proposalID, addrs[1], types.NewNonSplitVoteOption(types.OptionYes))) - require.NoError(t, app.GovKeeper.AddVote(ctx, proposalID, addrs[2], types.NewNonSplitVoteOption(types.OptionNo))) + require.NoError(t, app.GovKeeper.AddVote(ctx, proposalID, valAccAddrs[0], types.NewNonSplitVoteOption(types.OptionAbstain))) + require.NoError(t, app.GovKeeper.AddVote(ctx, proposalID, valAccAddrs[1], types.NewNonSplitVoteOption(types.OptionYes))) + require.NoError(t, app.GovKeeper.AddVote(ctx, proposalID, valAccAddrs[2], types.NewNonSplitVoteOption(types.OptionNo))) proposal, ok := app.GovKeeper.GetProposal(ctx, proposalID) require.True(t, ok) @@ -278,22 +214,11 @@ func TestTallyAbstainFails(t *testing.T) { require.False(t, tallyResults.Equals(types.EmptyTallyResult())) } -func TestTallyNonVoter(t *testing.T) { +func TestTallyOnlyValidatorsNonVoter(t *testing.T) { app := govgenhelpers.SetupNoValset(false) ctx := app.BaseApp.NewContext(false, tmproto.Header{}) - power := []int64{6, 6, 7} - _, valAddrs := createValidators(t, ctx, app, []int64{1, 1, 1}) - addrs := govgenhelpers.AddTestAddrs(app, ctx, 3, sdk.NewInt(50000000)) - - val1, found := app.StakingKeeper.GetValidator(ctx, valAddrs[0]) - require.True(t, found) - - for i := range addrs { - delTokens := app.StakingKeeper.TokensFromConsensusPower(ctx, power[i]) - _, err := app.StakingKeeper.Delegate(ctx, addrs[i], delTokens, stakingtypes.Unbonded, val1, true) - require.NoError(t, err) - } + valAccAddrs, _ := createValidators(t, ctx, app, []int64{6, 6, 7}) tp := govgenhelpers.TestTextProposal proposal, err := app.GovKeeper.SubmitProposal(ctx, tp) @@ -302,8 +227,8 @@ func TestTallyNonVoter(t *testing.T) { proposal.Status = types.StatusVotingPeriod app.GovKeeper.SetProposal(ctx, proposal) - require.NoError(t, app.GovKeeper.AddVote(ctx, proposalID, addrs[0], types.NewNonSplitVoteOption(types.OptionYes))) - require.NoError(t, app.GovKeeper.AddVote(ctx, proposalID, addrs[1], types.NewNonSplitVoteOption(types.OptionNo))) + require.NoError(t, app.GovKeeper.AddVote(ctx, proposalID, valAccAddrs[0], types.NewNonSplitVoteOption(types.OptionYes))) + require.NoError(t, app.GovKeeper.AddVote(ctx, proposalID, valAccAddrs[1], types.NewNonSplitVoteOption(types.OptionNo))) proposal, ok := app.GovKeeper.GetProposal(ctx, proposalID) require.True(t, ok) @@ -314,10 +239,6 @@ func TestTallyNonVoter(t *testing.T) { require.False(t, tallyResults.Equals(types.EmptyTallyResult())) } -/* -// NOTE: these tests are disabled as voting for validators is not available and therefore -// all features around voting and delegations will not work - func TestTallyDelgatorOverride(t *testing.T) { app := govgenhelpers.SetupNoValset(false) ctx := app.BaseApp.NewContext(false, tmproto.Header{}) @@ -333,7 +254,7 @@ func TestTallyDelgatorOverride(t *testing.T) { _ = staking.EndBlocker(ctx, app.StakingKeeper) - tp := TestProposal + tp := govgenhelpers.TestTextProposal proposal, err := app.GovKeeper.SubmitProposal(ctx, tp) require.NoError(t, err) proposalID := proposal.ProposalId @@ -354,6 +275,8 @@ func TestTallyDelgatorOverride(t *testing.T) { require.False(t, tallyResults.Equals(types.EmptyTallyResult())) } +// As validators can only vote with their own stake, delegators don't inherit votes from validators +// so the proposal fails func TestTallyDelgatorInherit(t *testing.T) { app := govgenhelpers.SetupNoValset(false) ctx := app.BaseApp.NewContext(false, tmproto.Header{}) @@ -369,7 +292,7 @@ func TestTallyDelgatorInherit(t *testing.T) { _ = staking.EndBlocker(ctx, app.StakingKeeper) - tp := TestProposal + tp := govgenhelpers.TestTextProposal proposal, err := app.GovKeeper.SubmitProposal(ctx, tp) require.NoError(t, err) proposalID := proposal.ProposalId @@ -384,7 +307,7 @@ func TestTallyDelgatorInherit(t *testing.T) { require.True(t, ok) passes, burnDeposits, tallyResults := app.GovKeeper.Tally(ctx, proposal) - require.True(t, passes) + require.False(t, passes) require.False(t, burnDeposits) require.False(t, tallyResults.Equals(types.EmptyTallyResult())) } @@ -408,7 +331,7 @@ func TestTallyDelgatorMultipleOverride(t *testing.T) { _ = staking.EndBlocker(ctx, app.StakingKeeper) - tp := TestProposal + tp := govgenhelpers.TestTextProposal proposal, err := app.GovKeeper.SubmitProposal(ctx, tp) require.NoError(t, err) proposalID := proposal.ProposalId @@ -429,6 +352,8 @@ func TestTallyDelgatorMultipleOverride(t *testing.T) { require.False(t, tallyResults.Equals(types.EmptyTallyResult())) } +// As validators can only vote with their own stake, delegators don't inherit votes from validators +// so the proposal passes func TestTallyDelgatorMultipleInherit(t *testing.T) { app := govgenhelpers.SetupNoValset(false) ctx := app.BaseApp.NewContext(false, tmproto.Header{}) @@ -450,7 +375,7 @@ func TestTallyDelgatorMultipleInherit(t *testing.T) { _ = staking.EndBlocker(ctx, app.StakingKeeper) - tp := TestProposal + tp := govgenhelpers.TestTextProposal proposal, err := app.GovKeeper.SubmitProposal(ctx, tp) require.NoError(t, err) proposalID := proposal.ProposalId @@ -465,7 +390,7 @@ func TestTallyDelgatorMultipleInherit(t *testing.T) { require.True(t, ok) passes, burnDeposits, tallyResults := app.GovKeeper.Tally(ctx, proposal) - require.False(t, passes) + require.True(t, passes) require.False(t, burnDeposits) require.False(t, tallyResults.Equals(types.EmptyTallyResult())) } @@ -493,7 +418,7 @@ func TestTallyJailedValidator(t *testing.T) { require.NoError(t, err) app.StakingKeeper.Jail(ctx, sdk.ConsAddress(consAddr.Bytes())) - tp := TestProposal + tp := govgenhelpers.TestTextProposal proposal, err := app.GovKeeper.SubmitProposal(ctx, tp) require.NoError(t, err) proposalID := proposal.ProposalId @@ -526,7 +451,7 @@ func TestTallyValidatorMultipleDelegations(t *testing.T) { _, err := app.StakingKeeper.Delegate(ctx, addrs[0], delTokens, stakingtypes.Unbonded, val2, true) require.NoError(t, err) - tp := TestProposal + tp := govgenhelpers.TestTextProposal proposal, err := app.GovKeeper.SubmitProposal(ctx, tp) require.NoError(t, err) proposalID := proposal.ProposalId @@ -552,5 +477,3 @@ func TestTallyValidatorMultipleDelegations(t *testing.T) { require.True(t, tallyResults.Equals(expectedTallyResult)) } - -*/ diff --git a/x/gov/keeper/vote.go b/x/gov/keeper/vote.go index 259ec63..1187e1b 100644 --- a/x/gov/keeper/vote.go +++ b/x/gov/keeper/vote.go @@ -19,12 +19,6 @@ func (keeper Keeper) AddVote(ctx sdk.Context, proposalID uint64, voterAddr sdk.A return sdkerrors.Wrapf(types.ErrInactiveProposal, "%d", proposalID) } - // if account is validator it cannot vote - valAddr := sdk.ValAddress(voterAddr.Bytes()) - if _, found := keeper.sk.GetValidator(ctx, valAddr); found { - return types.ErrValidatorCannotVote - } - for _, option := range options { if !types.ValidWeightedVoteOption(option) { return sdkerrors.Wrap(types.ErrInvalidVote, option.String()) diff --git a/x/gov/keeper/vote_test.go b/x/gov/keeper/vote_test.go index 0177961..dfbd370 100644 --- a/x/gov/keeper/vote_test.go +++ b/x/gov/keeper/vote_test.go @@ -33,12 +33,6 @@ func TestVotes(t *testing.T) { require.Error(t, app.GovKeeper.AddVote(ctx, proposalID, addrs[0], types.NewNonSplitVoteOption(invalidOption)), "invalid option") - // Test vote from validator - vals := app.StakingKeeper.GetAllValidators(ctx) - valAddr, err := sdk.ValAddressFromBech32(vals[0].OperatorAddress) - require.NoError(t, err) - require.Error(t, app.GovKeeper.AddVote(ctx, proposalID, sdk.AccAddress(valAddr.Bytes()), types.NewNonSplitVoteOption(types.OptionAbstain)), "voting for validators is disabled") - // Test first vote require.NoError(t, app.GovKeeper.AddVote(ctx, proposalID, addrs[0], types.NewNonSplitVoteOption(types.OptionAbstain))) vote, found := app.GovKeeper.GetVote(ctx, proposalID, addrs[0]) diff --git a/x/gov/module.go b/x/gov/module.go index 4c621a8..a0e2ee0 100644 --- a/x/gov/module.go +++ b/x/gov/module.go @@ -120,17 +120,15 @@ type AppModule struct { keeper keeper.Keeper accountKeeper types.AccountKeeper bankKeeper types.BankKeeper - stakingKeeper types.StakingKeeper } // NewAppModule creates a new AppModule object -func NewAppModule(cdc codec.Codec, keeper keeper.Keeper, ak types.AccountKeeper, bk types.BankKeeper, sk types.StakingKeeper) AppModule { +func NewAppModule(cdc codec.Codec, keeper keeper.Keeper, ak types.AccountKeeper, bk types.BankKeeper) AppModule { return AppModule{ AppModuleBasic: AppModuleBasic{cdc: cdc}, keeper: keeper, accountKeeper: ak, bankKeeper: bk, - stakingKeeper: sk, } } @@ -218,6 +216,6 @@ func (am AppModule) RegisterStoreDecoder(sdr sdk.StoreDecoderRegistry) { func (am AppModule) WeightedOperations(simState module.SimulationState) []simtypes.WeightedOperation { return simulation.WeightedOperations( simState.AppParams, simState.Cdc, - am.accountKeeper, am.bankKeeper, am.stakingKeeper, am.keeper, simState.Contents, + am.accountKeeper, am.bankKeeper, am.keeper, simState.Contents, ) } diff --git a/x/gov/simulation/operations.go b/x/gov/simulation/operations.go index a80ed1f..d51bd37 100644 --- a/x/gov/simulation/operations.go +++ b/x/gov/simulation/operations.go @@ -29,7 +29,7 @@ const ( // WeightedOperations returns all the operations from the module with their respective weights func WeightedOperations( appParams simtypes.AppParams, cdc codec.JSONCodec, ak types.AccountKeeper, - bk types.BankKeeper, sk types.StakingKeeper, k keeper.Keeper, + bk types.BankKeeper, k keeper.Keeper, wContents []simtypes.WeightedProposalContent, ) simulation.WeightedOperations { var ( @@ -69,7 +69,7 @@ func WeightedOperations( wProposalOps, simulation.NewWeightedOperation( weight, - SimulateMsgSubmitProposal(ak, bk, sk, k, wContent.ContentSimulatorFn()), + SimulateMsgSubmitProposal(ak, bk, k, wContent.ContentSimulatorFn()), ), ) } @@ -81,11 +81,11 @@ func WeightedOperations( ), simulation.NewWeightedOperation( weightMsgVote, - SimulateMsgVote(ak, bk, sk, k), + SimulateMsgVote(ak, bk, k), ), simulation.NewWeightedOperation( weightMsgVoteWeighted, - SimulateMsgVoteWeighted(ak, bk, sk, k), + SimulateMsgVoteWeighted(ak, bk, k), ), } @@ -96,7 +96,7 @@ func WeightedOperations( // voting on the proposal, and subsequently slashing the proposal. It is implemented using // future operations. func SimulateMsgSubmitProposal( - ak types.AccountKeeper, bk types.BankKeeper, sk types.StakingKeeper, + ak types.AccountKeeper, bk types.BankKeeper, k keeper.Keeper, contentSim simtypes.ContentSimulatorFn, ) simtypes.Operation { // The states are: @@ -202,7 +202,7 @@ func SimulateMsgSubmitProposal( whenVote := ctx.BlockHeader().Time.Add(time.Duration(r.Int63n(int64(votingPeriod.Seconds()))) * time.Second) fops[i] = simtypes.FutureOperation{ BlockTime: whenVote, - Op: operationSimulateMsgVote(ak, bk, sk, k, accs[whoVotes[i]], int64(proposalID)), + Op: operationSimulateMsgVote(ak, bk, k, accs[whoVotes[i]], int64(proposalID)), } } @@ -262,11 +262,11 @@ func SimulateMsgDeposit(ak types.AccountKeeper, bk types.BankKeeper, k keeper.Ke } // SimulateMsgVote generates a MsgVote with random values. -func SimulateMsgVote(ak types.AccountKeeper, bk types.BankKeeper, sk types.StakingKeeper, k keeper.Keeper) simtypes.Operation { - return operationSimulateMsgVote(ak, bk, sk, k, simtypes.Account{}, -1) +func SimulateMsgVote(ak types.AccountKeeper, bk types.BankKeeper, k keeper.Keeper) simtypes.Operation { + return operationSimulateMsgVote(ak, bk, k, simtypes.Account{}, -1) } -func operationSimulateMsgVote(ak types.AccountKeeper, bk types.BankKeeper, sk types.StakingKeeper, +func operationSimulateMsgVote(ak types.AccountKeeper, bk types.BankKeeper, k keeper.Keeper, simAccount simtypes.Account, proposalIDInt int64, ) simtypes.Operation { return func( @@ -290,12 +290,6 @@ func operationSimulateMsgVote(ak types.AccountKeeper, bk types.BankKeeper, sk ty proposalID = uint64(proposalIDInt) } - // if account is validator it cannot vote - valAddr := sdk.ValAddress(simAccount.Address.Bytes()) - if _, found := sk.GetValidator(ctx, valAddr); found { - return simtypes.NoOpMsg(types.ModuleName, types.TypeMsgVote, "account is a validator"), nil, nil - } - option := randomVotingOption(r) msg := types.NewMsgVote(simAccount.Address, proposalID, option) @@ -322,11 +316,11 @@ func operationSimulateMsgVote(ak types.AccountKeeper, bk types.BankKeeper, sk ty } // SimulateMsgVoteWeighted generates a MsgVoteWeighted with random values. -func SimulateMsgVoteWeighted(ak types.AccountKeeper, bk types.BankKeeper, sk types.StakingKeeper, k keeper.Keeper) simtypes.Operation { - return operationSimulateMsgVoteWeighted(ak, bk, sk, k, simtypes.Account{}, -1) +func SimulateMsgVoteWeighted(ak types.AccountKeeper, bk types.BankKeeper, k keeper.Keeper) simtypes.Operation { + return operationSimulateMsgVoteWeighted(ak, bk, k, simtypes.Account{}, -1) } -func operationSimulateMsgVoteWeighted(ak types.AccountKeeper, bk types.BankKeeper, sk types.StakingKeeper, +func operationSimulateMsgVoteWeighted(ak types.AccountKeeper, bk types.BankKeeper, k keeper.Keeper, simAccount simtypes.Account, proposalIDInt int64, ) simtypes.Operation { return func( @@ -350,12 +344,6 @@ func operationSimulateMsgVoteWeighted(ak types.AccountKeeper, bk types.BankKeepe proposalID = uint64(proposalIDInt) } - // if account is validator it cannot vote - valAddr := sdk.ValAddress(simAccount.Address.Bytes()) - if _, found := sk.GetValidator(ctx, valAddr); found { - return simtypes.NoOpMsg(types.ModuleName, types.TypeMsgVote, "account is a validator"), nil, nil - } - options := randomWeightedVotingOptions(r) msg := types.NewMsgVoteWeighted(simAccount.Address, proposalID, options) diff --git a/x/gov/simulation/operations_test.go b/x/gov/simulation/operations_test.go index dc86c1c..dc254c3 100644 --- a/x/gov/simulation/operations_test.go +++ b/x/gov/simulation/operations_test.go @@ -62,7 +62,7 @@ func TestWeightedOperations(t *testing.T) { appParams := make(simtypes.AppParams) weightesOps := simulation.WeightedOperations(appParams, cdc, app.AccountKeeper, - app.BankKeeper, app.StakingKeeper, app.GovKeeper, mockWeightedProposalContent(3), + app.BankKeeper, app.GovKeeper, mockWeightedProposalContent(3), ) // setup 3 accounts @@ -108,7 +108,7 @@ func TestSimulateMsgSubmitProposal(t *testing.T) { app.BeginBlock(abci.RequestBeginBlock{Header: tmproto.Header{Height: app.LastBlockHeight() + 1, AppHash: app.LastCommitID().Hash}}) // execute operation - op := simulation.SimulateMsgSubmitProposal(app.AccountKeeper, app.BankKeeper, app.StakingKeeper, app.GovKeeper, MockWeightedProposalContent{3}.ContentSimulatorFn()) + op := simulation.SimulateMsgSubmitProposal(app.AccountKeeper, app.BankKeeper, app.GovKeeper, MockWeightedProposalContent{3}.ContentSimulatorFn()) operationMsg, _, err := op(r, app.BaseApp, ctx, accounts, "") require.NoError(t, err) @@ -193,7 +193,7 @@ func TestSimulateMsgVote(t *testing.T) { app.BeginBlock(abci.RequestBeginBlock{Header: tmproto.Header{Height: app.LastBlockHeight() + 1, AppHash: app.LastCommitID().Hash, Time: blockTime}}) // execute operation - op := simulation.SimulateMsgVote(app.AccountKeeper, app.BankKeeper, app.StakingKeeper, app.GovKeeper) + op := simulation.SimulateMsgVote(app.AccountKeeper, app.BankKeeper, app.GovKeeper) operationMsg, _, err := op(r, app.BaseApp, ctx, accounts, "") require.NoError(t, err) @@ -235,7 +235,7 @@ func TestSimulateMsgVoteWeighted(t *testing.T) { app.BeginBlock(abci.RequestBeginBlock{Header: tmproto.Header{Height: app.LastBlockHeight() + 1, AppHash: app.LastCommitID().Hash, Time: blockTime}}) // execute operation - op := simulation.SimulateMsgVoteWeighted(app.AccountKeeper, app.BankKeeper, app.StakingKeeper, app.GovKeeper) + op := simulation.SimulateMsgVoteWeighted(app.AccountKeeper, app.BankKeeper, app.GovKeeper) operationMsg, _, err := op(r, app.BaseApp, ctx, accounts, "") require.NoError(t, err) diff --git a/x/gov/types/errors.go b/x/gov/types/errors.go index 7172de2..49fbf2e 100644 --- a/x/gov/types/errors.go +++ b/x/gov/types/errors.go @@ -14,5 +14,4 @@ var ( ErrInvalidVote = sdkerrors.Register(ModuleName, 70, "invalid vote option") ErrInvalidGenesis = sdkerrors.Register(ModuleName, 80, "invalid genesis state") ErrNoProposalHandlerExists = sdkerrors.Register(ModuleName, 90, "no handler exists for proposal type") - ErrValidatorCannotVote = sdkerrors.Register(ModuleName, 100, "voting for validators is disabled") ) diff --git a/x/gov/types/expected_keepers.go b/x/gov/types/expected_keepers.go index 863bc73..a6e5216 100644 --- a/x/gov/types/expected_keepers.go +++ b/x/gov/types/expected_keepers.go @@ -24,8 +24,6 @@ type StakingKeeper interface { ctx sdk.Context, delegator sdk.AccAddress, fn func(index int64, delegation stakingtypes.DelegationI) (stop bool), ) - // GetValidator gets a single validator - GetValidator(ctx sdk.Context, addr sdk.ValAddress) (validator stakingtypes.Validator, found bool) } // AccountKeeper defines the expected account keeper (noalias)