fix(x/gov,x/distribution): Balance assertions on genesis import shouldn't be exact (#22832)

This commit is contained in:
violet 2025-01-06 03:57:57 -05:00 committed by GitHub
parent da19b360f5
commit 4ff445ca6a
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
6 changed files with 266 additions and 6 deletions

View File

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

View File

@ -60,6 +60,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.AuthKeeper.ExportGenesis(ctx)
require.NoError(t, err)
bankGenState, err := s1.BankKeeper.ExportGenesis(ctx)
@ -119,7 +123,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)
@ -160,3 +164,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")
}

View File

@ -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.

View File

@ -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
}

View File

@ -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.

View File

@ -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
}