diff --git a/tests/integration/distribution/genesis_test.go b/tests/integration/distribution/genesis_test.go new file mode 100644 index 000000000000..a113341496ae --- /dev/null +++ b/tests/integration/distribution/genesis_test.go @@ -0,0 +1,190 @@ +package distribution_test + +import ( + "encoding/json" + "testing" + + corestore "cosmossdk.io/core/store" + coretesting "cosmossdk.io/core/testing" + "cosmossdk.io/depinject" + "cosmossdk.io/log" + sdkmath "cosmossdk.io/math" + "cosmossdk.io/x/distribution/keeper" + "cosmossdk.io/x/distribution/types" + stakingkeeper "cosmossdk.io/x/staking/keeper" + abci "github.com/cometbft/cometbft/api/cometbft/abci/v1" + "github.com/cosmos/cosmos-sdk/codec" + "github.com/cosmos/cosmos-sdk/runtime" + simtestutil "github.com/cosmos/cosmos-sdk/testutil/sims" + sdk "github.com/cosmos/cosmos-sdk/types" + "github.com/stretchr/testify/suite" + + bankkeeper "cosmossdk.io/x/bank/keeper" + _ "github.com/cosmos/cosmos-sdk/x/auth" + authkeeper "github.com/cosmos/cosmos-sdk/x/auth/keeper" +) + +type ImportExportSuite struct { + suite.Suite + + cdc codec.Codec + app *runtime.App + addrs []sdk.AccAddress + AccountKeeper authkeeper.AccountKeeper + BankKeeper bankkeeper.Keeper + DistributionKeeper keeper.Keeper + StakingKeeper *stakingkeeper.Keeper + appBuilder *runtime.AppBuilder +} + +func TestDistributionImportExport(t *testing.T) { + suite.Run(t, new(ImportExportSuite)) +} + +func (s *ImportExportSuite) SetupTest() { + var err error + valTokens := sdk.TokensFromConsensusPower(42, sdk.DefaultPowerReduction) + s.app, err = simtestutil.SetupWithConfiguration( + depinject.Configs( + AppConfig, + depinject.Supply(log.NewNopLogger()), + ), + simtestutil.DefaultStartUpConfig(), + &s.AccountKeeper, &s.BankKeeper, &s.DistributionKeeper, &s.StakingKeeper, + &s.cdc, &s.appBuilder, + ) + s.Require().NoError(err) + + ctx := s.app.BaseApp.NewContext(false) + s.addrs = simtestutil.AddTestAddrs(s.BankKeeper, s.StakingKeeper, ctx, 1, valTokens) + + _, err = s.app.FinalizeBlock(&abci.FinalizeBlockRequest{ + Height: s.app.LastBlockHeight() + 1, + }) + s.Require().NoError(err) +} + +func (s *ImportExportSuite) TestHappyPath() { + ctx := s.app.NewContext(true) + // Imagine a situation where rewards were, e.g. 100 / 3 = 33, but the fee collector sent 100 to the distribution module. + // There're 99 tokens in rewards, but 100 in the module; let's simulate a situation where there are 34 tokens left in the module, + // and a single validator has 33 tokens of rewards. + rewards := sdk.NewDecCoinsFromCoins(sdk.NewCoin("stake", sdkmath.NewInt(33))) + + // We'll pretend s.addrs[0] is the fee collector module. + err := s.BankKeeper.SendCoinsFromAccountToModule(ctx, s.addrs[0], types.ModuleName, sdk.NewCoins(sdk.NewCoin("stake", sdkmath.NewInt(34)))) + s.Require().NoError(err) + + validators, err := s.StakingKeeper.GetAllValidators(ctx) + s.Require().NoError(err) + val := validators[0] + + err = s.DistributionKeeper.AllocateTokensToValidator(ctx, val, rewards) + s.Require().NoError(err) + + _, err = s.app.FinalizeBlock(&abci.FinalizeBlockRequest{ + Height: s.app.LastBlockHeight() + 1, + }) + s.Require().NoError(err) + + valBz, err := s.StakingKeeper.ValidatorAddressCodec().StringToBytes(val.GetOperator()) + s.Require().NoError(err) + outstanding, err := s.DistributionKeeper.ValidatorOutstandingRewards.Get(ctx, valBz) + s.Require().NoError(err) + s.Require().Equal(rewards, outstanding.Rewards) + + genesisState, err := s.app.ModuleManager.ExportGenesis(ctx) + s.Require().NoError(err) + stateBytes, err := json.MarshalIndent(genesisState, "", " ") + s.Require().NoError(err) + + db := coretesting.NewMemDB() + conf2 := simtestutil.DefaultStartUpConfig() + conf2.DB = db + app2, err := simtestutil.SetupWithConfiguration( + depinject.Configs( + AppConfig, + depinject.Supply(log.NewNopLogger()), + ), + conf2, + ) + s.Require().NoError(err) + + s.clearDB(db) + err = app2.CommitMultiStore().LoadLatestVersion() + s.Require().NoError(err) + + _, err = app2.InitChain( + &abci.InitChainRequest{ + Validators: []abci.ValidatorUpdate{}, + ConsensusParams: simtestutil.DefaultConsensusParams, + AppStateBytes: stateBytes, + }, + ) + s.Require().NoError(err) +} + +func (s *ImportExportSuite) TestInsufficientFunds() { + ctx := s.app.NewContext(true) + rewards := sdk.NewCoin("stake", sdkmath.NewInt(35)) + + validators, err := s.StakingKeeper.GetAllValidators(ctx) + s.Require().NoError(err) + + err = s.DistributionKeeper.AllocateTokensToValidator(ctx, validators[0], sdk.NewDecCoinsFromCoins(rewards)) + s.Require().NoError(err) + + // We'll pretend s.addrs[0] is the fee collector module. + err = s.BankKeeper.SendCoinsFromAccountToModule(ctx, s.addrs[0], types.ModuleName, sdk.NewCoins(sdk.NewCoin("stake", sdkmath.NewInt(34)))) + s.Require().NoError(err) + + _, err = s.app.FinalizeBlock(&abci.FinalizeBlockRequest{ + Height: s.app.LastBlockHeight() + 1, + }) + s.Require().NoError(err) + + genesisState, err := s.app.ModuleManager.ExportGenesis(ctx) + s.Require().NoError(err) + stateBytes, err := json.MarshalIndent(genesisState, "", " ") + s.Require().NoError(err) + + db := coretesting.NewMemDB() + conf2 := simtestutil.DefaultStartUpConfig() + conf2.DB = db + app2, err := simtestutil.SetupWithConfiguration( + depinject.Configs( + AppConfig, + depinject.Supply(log.NewNopLogger()), + ), + conf2, + ) + s.Require().NoError(err) + + s.clearDB(db) + err = app2.CommitMultiStore().LoadLatestVersion() + s.Require().NoError(err) + + _, err = app2.InitChain( + &abci.InitChainRequest{ + Validators: []abci.ValidatorUpdate{}, + ConsensusParams: simtestutil.DefaultConsensusParams, + AppStateBytes: stateBytes, + }, + ) + s.Require().ErrorContains(err, "distribution module balance is less than module holdings") +} + +func (s *ImportExportSuite) clearDB(db corestore.KVStoreWithBatch) { + iter, err := db.Iterator(nil, nil) + s.Require().NoError(err) + defer iter.Close() + + var keys [][]byte + for ; iter.Valid(); iter.Next() { + keys = append(keys, iter.Key()) + } + + for _, k := range keys { + s.Require().NoError(db.Delete(k)) + } +} diff --git a/tests/integration/gov/genesis_test.go b/tests/integration/gov/genesis_test.go index 5f512be92a85..2e3393480838 100644 --- a/tests/integration/gov/genesis_test.go +++ b/tests/integration/gov/genesis_test.go @@ -103,6 +103,10 @@ func TestImportExportQueues(t *testing.T) { assert.Assert(t, proposal1.Status == v1.StatusDepositPeriod) assert.Assert(t, proposal2.Status == v1.StatusVotingPeriod) + // transfer some tokens to the governance account to simulate more money being there than deposits would require + err = s1.BankKeeper.SendCoinsFromAccountToModule(ctx, addrs[0], types.ModuleName, params.MinDeposit) + require.NoError(t, err) + authGenState, err := s1.AccountKeeper.ExportGenesis(ctx) require.NoError(t, err) bankGenState, err := s1.BankKeeper.ExportGenesis(ctx) @@ -175,7 +179,7 @@ func TestImportExportQueues(t *testing.T) { assert.Assert(t, proposal2.Status == v1.StatusVotingPeriod) macc := s2.GovKeeper.GetGovernanceAccount(ctx2) - assert.DeepEqual(t, sdk.Coins(params.MinDeposit), s2.BankKeeper.GetAllBalances(ctx2, macc.GetAddress())) + assert.DeepEqual(t, sdk.Coins(params.MinDeposit).MulInt(sdkmath.NewInt(2)), s2.BankKeeper.GetAllBalances(ctx2, macc.GetAddress())) // Run the endblocker. Check to make sure that proposal1 is removed from state, and proposal2 is finished VotingPeriod. err = s2.GovKeeper.EndBlocker(ctx2) @@ -233,3 +237,91 @@ func TestImportExportQueues_ErrorUnconsistentState(t *testing.T) { require.NoError(t, err) require.Equal(t, genState, v1.DefaultGenesisState()) } + +func TestImportExportQueues_ErrorInsufficientBalance(t *testing.T) { + var err error + s1 := suite{} + s1.app, err = simtestutil.SetupWithConfiguration( + depinject.Configs( + appConfig, + depinject.Supply(log.NewNopLogger()), + ), + simtestutil.DefaultStartUpConfig(), + &s1.AccountKeeper, &s1.BankKeeper, &s1.GovKeeper, &s1.StakingKeeper, &s1.cdc, &s1.appBuilder, + ) + assert.NilError(t, err) + + ctx := s1.app.BaseApp.NewContext(false) + addrs := simtestutil.AddTestAddrs(s1.BankKeeper, s1.StakingKeeper, ctx, 1, valTokens) + + _, err = s1.app.FinalizeBlock(&abci.FinalizeBlockRequest{ + Height: s1.app.LastBlockHeight() + 1, + }) + assert.NilError(t, err) + + ctx = s1.app.BaseApp.NewContext(false) + // Create a proposal and put it into the deposit period + proposal1, err := s1.GovKeeper.SubmitProposal(ctx, []sdk.Msg{mkTestLegacyContent(t)}, "", "test", "description", addrs[0], v1.ProposalType_PROPOSAL_TYPE_STANDARD) + assert.NilError(t, err) + proposalID1 := proposal1.Id + + params, err := s1.GovKeeper.Params.Get(ctx) + assert.NilError(t, err) + votingStarted, err := s1.GovKeeper.AddDeposit(ctx, proposalID1, addrs[0], params.MinDeposit) + assert.NilError(t, err) + assert.Assert(t, votingStarted) + + proposal1, err = s1.GovKeeper.Proposals.Get(ctx, proposalID1) + assert.NilError(t, err) + assert.Assert(t, proposal1.Status == v1.StatusVotingPeriod) + + // transfer some tokens from the governance account to the user account to simulate less money being there than deposits would require + err = s1.BankKeeper.SendCoinsFromModuleToAccount(ctx, types.ModuleName, addrs[0], sdk.Coins(params.MinDeposit).QuoInt(sdkmath.NewInt(2))) + require.NoError(t, err) + + authGenState, err := s1.AccountKeeper.ExportGenesis(ctx) + require.NoError(t, err) + bankGenState, err := s1.BankKeeper.ExportGenesis(ctx) + require.NoError(t, err) + stakingGenState, err := s1.StakingKeeper.ExportGenesis(ctx) + require.NoError(t, err) + + // export the state and import it into a new app + govGenState, _ := gov.ExportGenesis(ctx, s1.GovKeeper) + genesisState := s1.appBuilder.DefaultGenesis() + + genesisState[authtypes.ModuleName] = s1.cdc.MustMarshalJSON(authGenState) + genesisState[banktypes.ModuleName] = s1.cdc.MustMarshalJSON(bankGenState) + genesisState[types.ModuleName] = s1.cdc.MustMarshalJSON(govGenState) + genesisState[stakingtypes.ModuleName] = s1.cdc.MustMarshalJSON(stakingGenState) + + stateBytes, err := json.MarshalIndent(genesisState, "", " ") + assert.NilError(t, err) + + s2 := suite{} + db := coretesting.NewMemDB() + conf2 := simtestutil.DefaultStartUpConfig() + conf2.DB = db + s2.app, err = simtestutil.SetupWithConfiguration( + depinject.Configs( + appConfig, + depinject.Supply(log.NewNopLogger()), + ), + conf2, + &s2.AccountKeeper, &s2.BankKeeper, &s2.GovKeeper, &s2.StakingKeeper, &s2.cdc, &s2.appBuilder, + ) + assert.NilError(t, err) + + clearDB(t, db) + err = s2.app.CommitMultiStore().LoadLatestVersion() + assert.NilError(t, err) + + _, err = s2.app.InitChain( + &abci.InitChainRequest{ + Validators: []abci.ValidatorUpdate{}, + ConsensusParams: simtestutil.DefaultConsensusParams, + AppStateBytes: stateBytes, + }, + ) + require.ErrorContains(t, err, "expected gov module to hold at least") +} diff --git a/x/distribution/CHANGELOG.md b/x/distribution/CHANGELOG.md index 0a341876bc94..ae69c4d3d800 100644 --- a/x/distribution/CHANGELOG.md +++ b/x/distribution/CHANGELOG.md @@ -31,6 +31,7 @@ Ref: https://keepachangelog.com/en/1.0.0/ ### Improvements +* [#22832](https://github.com/cosmos/cosmos-sdk/pull/22832) Ensure the distribution module has at least as many tokens as outstanding rewards at genesis import * [#20790](https://github.com/cosmos/cosmos-sdk/pull/20790) `x/distribution` does not depend on `x/protocolpool` anymore, now `x/distribution` only does token transfers and `x/protocolpool` does the rest. ### API Breaking Changes diff --git a/x/distribution/keeper/genesis.go b/x/distribution/keeper/genesis.go index 41f8a715c163..75024e2d8e29 100644 --- a/x/distribution/keeper/genesis.go +++ b/x/distribution/keeper/genesis.go @@ -123,8 +123,8 @@ func (k Keeper) InitGenesis(ctx context.Context, data types.GenesisState) error } balances := k.bankKeeper.GetAllBalances(ctx, moduleAcc.GetAddress()) - if !balances.Equal(moduleHoldingsInt) { - return fmt.Errorf("distribution module balance does not match the module holdings: %s <-> %s", balances, moduleHoldingsInt) + if balances.IsAllLT(moduleHoldingsInt) { + return fmt.Errorf("distribution module balance is less than module holdings: %s < %s", balances, moduleHoldingsInt) } return nil } diff --git a/x/gov/CHANGELOG.md b/x/gov/CHANGELOG.md index 4743c45d6571..42130ee83d94 100644 --- a/x/gov/CHANGELOG.md +++ b/x/gov/CHANGELOG.md @@ -38,6 +38,7 @@ Ref: https://keepachangelog.com/en/1.0.0/ ### Improvements +* [#22832](https://github.com/cosmos/cosmos-sdk/pull/22832) Ensure the governance module has at least as many tokens as are deposited at genesis import. * [#20521](https://github.com/cosmos/cosmos-sdk/pull/20521) Legacy proposals can now access the `appmodule.Environment` present in the `context.Context` of the handler. This is useful when migrating to server/v2 and removing the sdk context dependency. * [#19741](https://github.com/cosmos/cosmos-sdk/pull/19741) Add `ExpeditedQuorum` parameter specifying a minimum quorum for expedited proposals, that can differ from the regular quorum. * [#19352](https://github.com/cosmos/cosmos-sdk/pull/19352) `TallyResult` include vote options counts. Those counts replicates the now deprecated (but not removed) yes, no, abstain and veto count fields. diff --git a/x/gov/genesis.go b/x/gov/genesis.go index dd9e42b18017..75cad6e1d5f8 100644 --- a/x/gov/genesis.go +++ b/x/gov/genesis.go @@ -79,9 +79,9 @@ func InitGenesis(ctx context.Context, ak types.AccountKeeper, bk types.BankKeepe ak.SetModuleAccount(ctx, moduleAcc) } - // check if total deposits equals balance, if it doesn't return an error - if !balance.Equal(totalDeposits) { - return fmt.Errorf("expected module account was %s but we got %s", balance.String(), totalDeposits.String()) + // check if the module account can cover the total deposits + if !balance.IsAllGTE(totalDeposits) { + return fmt.Errorf("expected gov module to hold at least %s, but it holds %s", totalDeposits, balance) } return nil }