fix(x/gov,x/distribution): Balance assertions on genesis import shoul…

…dn't be exact (#22832)

(cherry picked from commit 4ff445c)

# Conflicts:
#	tests/integration/gov/genesis_test.go
#	x/distribution/keeper/genesis.go

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))
+	}
+}

tests/integration/gov/genesis_test.go

@@ -103,7 +103,15 @@ func TestImportExportQueues(t *testing.T) {
 	assert.Assert(t, proposal1.Status == v1.StatusDepositPeriod)
 	assert.Assert(t, proposal2.Status == v1.StatusVotingPeriod)
 
+<<<<<<< HEAD:tests/integration/gov/genesis_test.go
 	authGenState, err := s1.AccountKeeper.ExportGenesis(ctx)
+=======
+	// 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.AuthKeeper.ExportGenesis(ctx)
+>>>>>>> 4ff445ca6 (fix(x/gov,x/distribution): Balance assertions on genesis import shouldn't be exact (#22832)):tests/integration/v2/gov/genesis_test.go
 	require.NoError(t, err)
 	bankGenState, err := s1.BankKeeper.ExportGenesis(ctx)
 	require.NoError(t, err)
@@ -175,7 +183,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 +241,67 @@ 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 := createTestSuite(t, integration.Genesis_COMMIT)
+
+	ctx := s1.ctx
+	addrs := simtestutil.AddTestAddrs(s1.BankKeeper, s1.StakingKeeper, ctx, 1, valTokens)
+
+	// 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.AuthKeeper.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, err := gov.ExportGenesis(ctx, s1.GovKeeper)
+	require.NoError(t, err)
+	genesisState := s1.app.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 := createTestSuite(t, integration.Genesis_SKIP)
+	emptyHash := sha256.Sum256(nil)
+	_, _, err = s2.app.InitGenesis(
+		ctx,
+		&server.BlockRequest[transaction.Tx]{
+			Height:    1,
+			Time:      time.Now(),
+			Hash:      emptyHash[:],
+			ChainId:   "test-chain",
+			AppHash:   emptyHash[:],
+			IsGenesis: true,
+		},
+		stateBytes,
+		integration.NewGenesisTxCodec(s2.txConfigOptions),
+	)
+	require.ErrorContains(t, err, "expected gov module to hold at least")
+}

x/distribution/CHANGELOG.md

@@ -29,6 +29,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.
 * [#18636](https://github.com/cosmos/cosmos-sdk/pull/18636) `CalculateDelegationRewards` and `DelegationTotalRewards` methods no longer panics on any sanity checks and instead returns appropriate errors.
 

x/distribution/keeper/genesis.go

@@ -123,11 +123,16 @@ func (k Keeper) InitGenesis(ctx context.Context, data types.GenesisState) error
 	}
 
 	balances := k.bankKeeper.GetAllBalances(ctx, moduleAcc.GetAddress())
+<<<<<<< HEAD
 	if balances.IsZero() {
 		k.authKeeper.SetModuleAccount(ctx, moduleAcc)
 	}
 	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)
+>>>>>>> 4ff445ca6 (fix(x/gov,x/distribution): Balance assertions on genesis import shouldn't be exact (#22832))
 	}
 	return nil
 }

x/gov/CHANGELOG.md

@@ -40,6 +40,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.

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
 }