diff --git a/tests/integration/gov/genesis_test.go b/tests/integration/gov/genesis_test.go index 86599ff3073c..6cbe8e7be48c 100644 --- a/tests/integration/gov/genesis_test.go +++ b/tests/integration/gov/genesis_test.go @@ -102,8 +102,10 @@ func TestImportExportQueues(t *testing.T) { authGenState, err := s1.AccountKeeper.ExportGenesis(ctx) require.NoError(t, err) - bankGenState := s1.BankKeeper.ExportGenesis(ctx) - stakingGenState := s1.StakingKeeper.ExportGenesis(ctx) + 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) diff --git a/tests/integration/staking/keeper/genesis_test.go b/tests/integration/staking/keeper/genesis_test.go index 96527dd20942..128af43cbc60 100644 --- a/tests/integration/staking/keeper/genesis_test.go +++ b/tests/integration/staking/keeper/genesis_test.go @@ -98,9 +98,11 @@ func TestInitGenesis(t *testing.T) { delegations = append(delegations, genesisDelegations...) genesisState := types.NewGenesisState(params, validators, delegations) - vals := (f.stakingKeeper.InitGenesis(f.sdkCtx, genesisState)) + vals, err := (f.stakingKeeper.InitGenesis(f.sdkCtx, genesisState)) + assert.NilError(t, err) - actualGenesis := (f.stakingKeeper.ExportGenesis(f.sdkCtx)) + actualGenesis, err := (f.stakingKeeper.ExportGenesis(f.sdkCtx)) + assert.NilError(t, err) assert.DeepEqual(t, genesisState.Params, actualGenesis.Params) assert.DeepEqual(t, genesisState.Delegations, actualGenesis.Delegations) @@ -157,27 +159,23 @@ func TestInitGenesis_PoolsBalanceMismatch(t *testing.T) { BondDenom: "stake", } - require.Panics(t, func() { - // setting validator status to bonded so the balance counts towards bonded pool - validator.Status = types.Bonded - f.stakingKeeper.InitGenesis(f.sdkCtx, &types.GenesisState{ - Params: params, - Validators: []types.Validator{validator}, - }) - }, - // "should panic because bonded pool balance is different from bonded pool coins", - ) - - require.Panics(t, func() { - // setting validator status to unbonded so the balance counts towards not bonded pool - validator.Status = types.Unbonded - f.stakingKeeper.InitGenesis(f.sdkCtx, &types.GenesisState{ - Params: params, - Validators: []types.Validator{validator}, - }) - }, + // setting validator status to bonded so the balance counts towards bonded pool + validator.Status = types.Bonded + _, err = f.stakingKeeper.InitGenesis(f.sdkCtx, &types.GenesisState{ + Params: params, + Validators: []types.Validator{validator}, + }) + // "should error because bonded pool balance is different from bonded pool coins", + require.NotNil(t, err) + + // setting validator status to unbonded so the balance counts towards not bonded pool + validator.Status = types.Unbonded + _, err = f.stakingKeeper.InitGenesis(f.sdkCtx, &types.GenesisState{ + Params: params, + Validators: []types.Validator{validator}, + }) // "should panic because not bonded pool balance is different from not bonded pool coins", - ) + require.NotNil(t, err) } func TestInitGenesisLargeValidatorSet(t *testing.T) { @@ -228,7 +226,8 @@ func TestInitGenesisLargeValidatorSet(t *testing.T) { ), ) - vals := f.stakingKeeper.InitGenesis(f.sdkCtx, genesisState) + vals, err := f.stakingKeeper.InitGenesis(f.sdkCtx, genesisState) + assert.NilError(t, err) abcivals := make([]abci.ValidatorUpdate, 100) for i, val := range validators[:100] { diff --git a/x/authz/CHANGELOG.md b/x/authz/CHANGELOG.md index 0e4e5795d1c1..e6de2c18baca 100644 --- a/x/authz/CHANGELOG.md +++ b/x/authz/CHANGELOG.md @@ -35,7 +35,7 @@ Ref: https://keepachangelog.com/en/1.0.0/ * [#19490](https://github.com/cosmos/cosmos-sdk/pull/19490) `appmodule.Environment` is received on the Keeper to get access to different application services. * [#18737](https://github.com/cosmos/cosmos-sdk/pull/18737) Update the keeper method `DequeueAndDeleteExpiredGrants` to take a limit argument for the number of grants to prune. * [#16509](https://github.com/cosmos/cosmos-sdk/pull/16509) `AcceptResponse` has been moved to sdk/types/authz and the `Updated` field is now of the type `sdk.Msg` instead of `authz.Authorization`. - +* [#19740](https://github.com/cosmos/cosmos-sdk/pull/19740) Verify `InitGenesis` and `ExportGenesis` module code and keeper code do not panic. ### Consensus Breaking Changes * [#19188](https://github.com/cosmos/cosmos-sdk/pull/19188) Remove creation of `BaseAccount` when sending a message to an account that does not exist. diff --git a/x/authz/keeper/genesis.go b/x/authz/keeper/genesis.go index e014f5265748..d5de1d741b09 100644 --- a/x/authz/keeper/genesis.go +++ b/x/authz/keeper/genesis.go @@ -2,6 +2,7 @@ package keeper import ( "context" + "errors" "cosmossdk.io/x/authz" @@ -28,7 +29,7 @@ func (k Keeper) InitGenesis(ctx context.Context, data *authz.GenesisState) error a, ok := entry.Authorization.GetCachedValue().(authz.Authorization) if !ok { - panic("expected authorization") + return errors.New("expected authorization") } err = k.SaveGrant(ctx, grantee, granter, a, entry.Expiration) diff --git a/x/bank/CHANGELOG.md b/x/bank/CHANGELOG.md index fc76c3c85efd..3d1944d2c446 100644 --- a/x/bank/CHANGELOG.md +++ b/x/bank/CHANGELOG.md @@ -38,6 +38,7 @@ Ref: https://keepachangelog.com/en/1.0.0/ * [#17569](https://github.com/cosmos/cosmos-sdk/pull/17569) `BurnCoins` takes an address instead of a module name * [#19477](https://github.com/cosmos/cosmos-sdk/pull/19477) `appmodule.Environment` is passed to bank `NewKeeper` * [#19627](https://github.com/cosmos/cosmos-sdk/pull/19627) The genesis api has been updated to match `appmodule.HasGenesis`. +* [#19740](https://github.com/cosmos/cosmos-sdk/pull/19740) Verify `InitGenesis` and `ExportGenesis` module code and keeper code do not panic. ### Consensus Breaking Changes diff --git a/x/bank/keeper/genesis.go b/x/bank/keeper/genesis.go index 61a0729becab..dd094055c79a 100644 --- a/x/bank/keeper/genesis.go +++ b/x/bank/keeper/genesis.go @@ -57,10 +57,10 @@ func (k BaseKeeper) InitGenesis(ctx context.Context, genState *types.GenesisStat } // ExportGenesis returns the bank module's genesis state. -func (k BaseKeeper) ExportGenesis(ctx context.Context) *types.GenesisState { +func (k BaseKeeper) ExportGenesis(ctx context.Context) (*types.GenesisState, error) { totalSupply, _, err := k.GetPaginatedTotalSupply(ctx, &query.PageRequest{Limit: query.PaginationMaxLimit}) if err != nil { - panic(fmt.Errorf("unable to fetch total supply %v", err)) + return nil, fmt.Errorf("unable to fetch total supply %v", err) } rv := types.NewGenesisState( @@ -70,5 +70,5 @@ func (k BaseKeeper) ExportGenesis(ctx context.Context) *types.GenesisState { k.GetAllDenomMetaData(ctx), k.GetAllSendEnabledEntries(ctx), ) - return rv + return rv, nil } diff --git a/x/bank/keeper/genesis_test.go b/x/bank/keeper/genesis_test.go index 8798c51d5044..5462ca52802f 100644 --- a/x/bank/keeper/genesis_test.go +++ b/x/bank/keeper/genesis_test.go @@ -38,7 +38,8 @@ func (suite *KeeperTestSuite) TestExportGenesis() { suite.Require().NoError(suite.bankKeeper.SetParams(ctx, types.DefaultParams())) - exportGenesis := suite.bankKeeper.ExportGenesis(ctx) + exportGenesis, err := suite.bankKeeper.ExportGenesis(ctx) + suite.Require().NoError(err) suite.Require().Len(exportGenesis.Params.SendEnabled, 0) //nolint:staticcheck // we're testing the old way here suite.Require().Equal(types.DefaultParams().DefaultSendEnabled, exportGenesis.Params.DefaultSendEnabled) diff --git a/x/bank/keeper/keeper.go b/x/bank/keeper/keeper.go index e4d76312a419..c643eb01489b 100644 --- a/x/bank/keeper/keeper.go +++ b/x/bank/keeper/keeper.go @@ -27,7 +27,7 @@ type Keeper interface { WithMintCoinsRestriction(types.MintingRestrictionFn) BaseKeeper InitGenesis(context.Context, *types.GenesisState) error - ExportGenesis(context.Context) *types.GenesisState + ExportGenesis(context.Context) (*types.GenesisState, error) GetSupply(ctx context.Context, denom string) sdk.Coin HasSupply(ctx context.Context, denom string) bool diff --git a/x/bank/module.go b/x/bank/module.go index 62bafcb41778..6390360f4b0f 100644 --- a/x/bank/module.go +++ b/x/bank/module.go @@ -144,7 +144,10 @@ func (am AppModule) InitGenesis(ctx context.Context, data json.RawMessage) error // ExportGenesis returns the exported genesis state as raw bytes for the bank // module. func (am AppModule) ExportGenesis(ctx context.Context) (json.RawMessage, error) { - gs := am.keeper.ExportGenesis(ctx) + gs, err := am.keeper.ExportGenesis(ctx) + if err != nil { + return nil, err + } return am.cdc.MarshalJSON(gs) } diff --git a/x/crisis/keeper/genesis.go b/x/crisis/keeper/genesis.go index b2bd54af50df..f95f28ad937a 100644 --- a/x/crisis/keeper/genesis.go +++ b/x/crisis/keeper/genesis.go @@ -14,10 +14,10 @@ func (k *Keeper) InitGenesis(ctx context.Context, data *types.GenesisState) { } // ExportGenesis returns a GenesisState for a given context and keeper. -func (k *Keeper) ExportGenesis(ctx context.Context) *types.GenesisState { +func (k *Keeper) ExportGenesis(ctx context.Context) (*types.GenesisState, error) { constantFee, err := k.ConstantFee.Get(ctx) if err != nil { - panic(err) + return nil, err } - return types.NewGenesisState(constantFee) + return types.NewGenesisState(constantFee), nil } diff --git a/x/crisis/keeper/genesis_test.go b/x/crisis/keeper/genesis_test.go index a2b392c3a6a0..c49754d8561b 100644 --- a/x/crisis/keeper/genesis_test.go +++ b/x/crisis/keeper/genesis_test.go @@ -55,7 +55,8 @@ func (s *GenesisTestSuite) TestImportExportGenesis() { constantFee := sdk.NewCoin(sdk.DefaultBondDenom, sdkmath.NewInt(1000)) err := s.keeper.ConstantFee.Set(s.sdkCtx, constantFee) s.Require().NoError(err) - genesis := s.keeper.ExportGenesis(s.sdkCtx) + genesis, err := s.keeper.ExportGenesis(s.sdkCtx) + s.Require().NoError(err) // set constant fee to zero constantFee = sdk.NewCoin(sdk.DefaultBondDenom, sdkmath.NewInt(0)) @@ -63,7 +64,8 @@ func (s *GenesisTestSuite) TestImportExportGenesis() { s.Require().NoError(err) s.keeper.InitGenesis(s.sdkCtx, genesis) - newGenesis := s.keeper.ExportGenesis(s.sdkCtx) + newGenesis, err := s.keeper.ExportGenesis(s.sdkCtx) + s.Require().NoError(err) s.Require().Equal(genesis, newGenesis) } diff --git a/x/crisis/module.go b/x/crisis/module.go index bc981e86cea2..ad7c10e6e894 100644 --- a/x/crisis/module.go +++ b/x/crisis/module.go @@ -134,7 +134,10 @@ func (am AppModule) InitGenesis(ctx context.Context, data json.RawMessage) error // ExportGenesis returns the exported genesis state as raw bytes for the crisis module. func (am AppModule) ExportGenesis(ctx context.Context) (json.RawMessage, error) { - gs := am.keeper.ExportGenesis(ctx) + gs, err := am.keeper.ExportGenesis(ctx) + if err != nil { + return nil, err + } return am.cdc.MarshalJSON(gs) } diff --git a/x/distribution/CHANGELOG.md b/x/distribution/CHANGELOG.md index f1c3f4c59689..610ac8103bce 100644 --- a/x/distribution/CHANGELOG.md +++ b/x/distribution/CHANGELOG.md @@ -49,6 +49,7 @@ Ref: https://keepachangelog.com/en/1.0.0/ * remove `Keeper`: `IterateValidatorCurrentRewards`, `GetValidatorCurrentRewards`, `SetValidatorCurrentRewards`, `DeleteValidatorCurrentRewards` * [#17657](https://github.com/cosmos/cosmos-sdk/pull/17657) The distribution module keeper now takes a new argument `PoolKeeper` in addition. * [#17670](https://github.com/cosmos/cosmos-sdk/pull/17670) `AllocateTokens` takes `comet.VoteInfos` instead of `[]abci.VoteInfo` +* [#19740](https://github.com/cosmos/cosmos-sdk/pull/19740) Verify `InitGenesis` and `ExportGenesis` module code and keeper code do not panic. ### Improvements diff --git a/x/distribution/keeper/genesis.go b/x/distribution/keeper/genesis.go index e77a1475551f..527ef35ba952 100644 --- a/x/distribution/keeper/genesis.go +++ b/x/distribution/keeper/genesis.go @@ -132,7 +132,7 @@ func (k Keeper) InitGenesis(ctx context.Context, data types.GenesisState) error // check if the module account exists moduleAcc := k.GetDistributionAccount(ctx) if moduleAcc == nil { - panic(fmt.Sprintf("%s module account has not been set", types.ModuleName)) + return fmt.Errorf("%s module account has not been set", types.ModuleName) } balances := k.bankKeeper.GetAllBalances(ctx, moduleAcc.GetAddress()) @@ -140,7 +140,7 @@ func (k Keeper) InitGenesis(ctx context.Context, data types.GenesisState) error k.authKeeper.SetModuleAccount(ctx, moduleAcc) } if !balances.Equal(moduleHoldingsInt) { - panic(fmt.Sprintf("distribution module balance does not match the module holdings: %s <-> %s", balances, moduleHoldingsInt)) + return fmt.Errorf("distribution module balance does not match the module holdings: %s <-> %s", balances, moduleHoldingsInt) } return nil } diff --git a/x/gov/CHANGELOG.md b/x/gov/CHANGELOG.md index 116e87afc38c..b1d565a9a4d4 100644 --- a/x/gov/CHANGELOG.md +++ b/x/gov/CHANGELOG.md @@ -67,6 +67,7 @@ Ref: https://keepachangelog.com/en/1.0.0/ * [#18532](https://github.com/cosmos/cosmos-sdk/pull/18532) All functions that were taking an expedited bool parameter now take a `ProposalType` parameter instead. * [#17496](https://github.com/cosmos/cosmos-sdk/pull/17496) in `x/gov/types/v1beta1/vote.go` `NewVote` was removed, constructing the struct is required for this type. * [#19101](https://github.com/cosmos/cosmos-sdk/pull/19101) Move `QueryProposalVotesParams` and `QueryVoteParams` from the `types/v1` package to `utils` and remove unused `querier.go` file. +* [#19740](https://github.com/cosmos/cosmos-sdk/pull/19740) Verify `InitGenesis` and `ExportGenesis` module code and keeper code do not panic. ### Deprecated diff --git a/x/gov/genesis.go b/x/gov/genesis.go index 09975f458bf4..dd9e42b18017 100644 --- a/x/gov/genesis.go +++ b/x/gov/genesis.go @@ -32,7 +32,7 @@ func InitGenesis(ctx context.Context, ak types.AccountKeeper, bk types.BankKeepe // check if the deposits pool account exists moduleAcc := k.GetGovernanceAccount(ctx) if moduleAcc == nil { - panic(fmt.Sprintf("%s module account has not been set", types.ModuleName)) + return fmt.Errorf("%s module account has not been set", types.ModuleName) } var totalDeposits sdk.Coins @@ -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 panic because there were export/import errors + // check if total deposits equals balance, if it doesn't return an error if !balance.Equal(totalDeposits) { - panic(fmt.Sprintf("expected module account was %s but we got %s", balance.String(), totalDeposits.String())) + return fmt.Errorf("expected module account was %s but we got %s", balance.String(), totalDeposits.String()) } return nil } diff --git a/x/group/CHANGELOG.md b/x/group/CHANGELOG.md index 9e67e2a664d1..0f6182d12bda 100644 --- a/x/group/CHANGELOG.md +++ b/x/group/CHANGELOG.md @@ -35,3 +35,4 @@ Ref: https://keepachangelog.com/en/1.0.0/ * [#19638](https://github.com/cosmos/cosmos-sdk/pull/19638) Migrate module to use `appmodule.Environment` router service so no `baseapp.MessageRouter` is required is `NewKeeper` anymore. * [#19489](https://github.com/cosmos/cosmos-sdk/pull/19489) `appmodule.Environment` is received on the Keeper to get access to different application services. * [#19410](https://github.com/cosmos/cosmos-sdk/pull/19410) Migrate to Store Service. +* [#19740](https://github.com/cosmos/cosmos-sdk/pull/19740) Verify `InitGenesis` and `ExportGenesis` module code and keeper code do not panic. diff --git a/x/group/keeper/genesis.go b/x/group/keeper/genesis.go index 70d61ff69230..a119a24bc32c 100644 --- a/x/group/keeper/genesis.go +++ b/x/group/keeper/genesis.go @@ -45,7 +45,7 @@ func (k Keeper) InitGenesis(ctx context.Context, cdc codec.JSONCodec, data json. } // ExportGenesis returns the group module's exported genesis. -func (k Keeper) ExportGenesis(ctx context.Context, _ codec.JSONCodec) *group.GenesisState { +func (k Keeper) ExportGenesis(ctx context.Context, _ codec.JSONCodec) (*group.GenesisState, error) { genesisState := group.NewGenesisState() var groups []*group.GroupInfo @@ -54,7 +54,7 @@ func (k Keeper) ExportGenesis(ctx context.Context, _ codec.JSONCodec) *group.Gen groupSeq, err := k.groupTable.Export(store, &groups) if err != nil { - panic(errors.Wrap(err, "groups")) + return nil, errors.Wrap(err, "groups") } genesisState.Groups = groups genesisState.GroupSeq = groupSeq @@ -62,14 +62,14 @@ func (k Keeper) ExportGenesis(ctx context.Context, _ codec.JSONCodec) *group.Gen var groupMembers []*group.GroupMember _, err = k.groupMemberTable.Export(store, &groupMembers) if err != nil { - panic(errors.Wrap(err, "group members")) + return nil, errors.Wrap(err, "group members") } genesisState.GroupMembers = groupMembers var groupPolicies []*group.GroupPolicyInfo _, err = k.groupPolicyTable.Export(store, &groupPolicies) if err != nil { - panic(errors.Wrap(err, "group policies")) + return nil, errors.Wrap(err, "group policies") } genesisState.GroupPolicies = groupPolicies genesisState.GroupPolicySeq = k.groupPolicySeq.CurVal(store) @@ -77,7 +77,7 @@ func (k Keeper) ExportGenesis(ctx context.Context, _ codec.JSONCodec) *group.Gen var proposals []*group.Proposal proposalSeq, err := k.proposalTable.Export(store, &proposals) if err != nil { - panic(errors.Wrap(err, "proposals")) + return nil, errors.Wrap(err, "proposals") } genesisState.Proposals = proposals genesisState.ProposalSeq = proposalSeq @@ -85,9 +85,9 @@ func (k Keeper) ExportGenesis(ctx context.Context, _ codec.JSONCodec) *group.Gen var votes []*group.Vote _, err = k.voteTable.Export(store, &votes) if err != nil { - panic(errors.Wrap(err, "votes")) + return nil, errors.Wrap(err, "votes") } genesisState.Votes = votes - return genesisState + return genesisState, nil } diff --git a/x/group/keeper/genesis_test.go b/x/group/keeper/genesis_test.go index 967867a1f271..ee92089f8c22 100644 --- a/x/group/keeper/genesis_test.go +++ b/x/group/keeper/genesis_test.go @@ -186,7 +186,8 @@ func (s *GenesisTestSuite) TestInitExportGenesis() { s.Require().Equal(votesRes.Votes[0], genesisState.Votes[0]) } - exported := s.keeper.ExportGenesis(sdkCtx, cdc) + exported, err := s.keeper.ExportGenesis(sdkCtx, cdc) + s.Require().NoError(err) bz, err := cdc.MarshalJSON(exported) s.Require().NoError(err) diff --git a/x/group/module/module.go b/x/group/module/module.go index a1f00cd9f1fa..9a044274ec7c 100644 --- a/x/group/module/module.go +++ b/x/group/module/module.go @@ -144,7 +144,10 @@ func (am AppModule) InitGenesis(ctx context.Context, data json.RawMessage) error // ExportGenesis returns the exported genesis state as raw bytes for the group module. func (am AppModule) ExportGenesis(ctx context.Context) (json.RawMessage, error) { - gs := am.keeper.ExportGenesis(ctx, am.cdc) + gs, err := am.keeper.ExportGenesis(ctx, am.cdc) + if err != nil { + return nil, err + } return am.cdc.MarshalJSON(gs) } diff --git a/x/nft/CHANGELOG.md b/x/nft/CHANGELOG.md index 8e939e4fe3d2..4c8cc88e930d 100644 --- a/x/nft/CHANGELOG.md +++ b/x/nft/CHANGELOG.md @@ -37,6 +37,10 @@ Ref: https://keepachangelog.com/en/1.0.0/ * [#19367](https://github.com/cosmos/cosmos-sdk/pull/19367) `appmodule.Environment` is received on the Keeper to get access to different application services +### API Breaking Changes + +* [#19740](https://github.com/cosmos/cosmos-sdk/pull/19740) Verify `InitGenesis` and `ExportGenesis` module code and keeper code do not panic. + ## [v0.1.0](https://github.com/cosmos/cosmos-sdk/releases/tag/x/nft/v0.1.0) - 2023-11-07 diff --git a/x/nft/keeper/genesis.go b/x/nft/keeper/genesis.go index 5e5dcae184fd..1c9aae22d73b 100644 --- a/x/nft/keeper/genesis.go +++ b/x/nft/keeper/genesis.go @@ -31,7 +31,7 @@ func (k Keeper) InitGenesis(ctx context.Context, data *nft.GenesisState) error { } // ExportGenesis returns a GenesisState for a given context. -func (k Keeper) ExportGenesis(ctx context.Context) *nft.GenesisState { +func (k Keeper) ExportGenesis(ctx context.Context) (*nft.GenesisState, error) { classes := k.GetClasses(ctx) nftMap := make(map[string][]*nft.NFT) for _, class := range classes { @@ -40,7 +40,7 @@ func (k Keeper) ExportGenesis(ctx context.Context) *nft.GenesisState { owner := k.GetOwner(ctx, n.ClassId, n.Id) ownerStr, err := k.ac.BytesToString(owner.Bytes()) if err != nil { - panic(err) + return nil, err } nftArr, ok := nftMap[ownerStr] if !ok { @@ -66,5 +66,5 @@ func (k Keeper) ExportGenesis(ctx context.Context) *nft.GenesisState { return &nft.GenesisState{ Classes: classes, Entries: entries, - } + }, nil } diff --git a/x/nft/keeper/keeper_test.go b/x/nft/keeper/keeper_test.go index e3ac2dfd0563..27c929bba25e 100644 --- a/x/nft/keeper/keeper_test.go +++ b/x/nft/keeper/keeper_test.go @@ -361,7 +361,8 @@ func (s *TestSuite) TestExportGenesis() { Nfts: []*nft.NFT{&expNFT}, }}, } - genesis := s.nftKeeper.ExportGenesis(s.ctx) + genesis, err := s.nftKeeper.ExportGenesis(s.ctx) + s.Require().NoError(err) s.Require().Equal(expGenesis, genesis) } diff --git a/x/nft/keeper/msg_server_test.go b/x/nft/keeper/msg_server_test.go index ae70d60b52d9..5209a7d4010f 100644 --- a/x/nft/keeper/msg_server_test.go +++ b/x/nft/keeper/msg_server_test.go @@ -41,7 +41,8 @@ func (s *TestSuite) TestSend() { Nfts: []*nft.NFT{&ExpNFT}, }}, } - genesis := s.nftKeeper.ExportGenesis(s.ctx) + genesis, err := s.nftKeeper.ExportGenesis(s.ctx) + s.Require().NoError(err) s.Require().Equal(expGenesis, genesis) testCases := []struct { diff --git a/x/nft/module/module.go b/x/nft/module/module.go index c9a8e8b5b484..1c61a048f53d 100644 --- a/x/nft/module/module.go +++ b/x/nft/module/module.go @@ -108,7 +108,10 @@ func (am AppModule) InitGenesis(ctx context.Context, data json.RawMessage) error // ExportGenesis returns the exported genesis state as raw bytes for the nft module. func (am AppModule) ExportGenesis(ctx context.Context) (json.RawMessage, error) { - gs := am.keeper.ExportGenesis(ctx) + gs, err := am.keeper.ExportGenesis(ctx) + if err != nil { + return nil, err + } return am.cdc.MarshalJSON(gs) } diff --git a/x/slashing/CHANGELOG.md b/x/slashing/CHANGELOG.md index c5e3852773ef..81a7018057d4 100644 --- a/x/slashing/CHANGELOG.md +++ b/x/slashing/CHANGELOG.md @@ -43,5 +43,6 @@ Ref: https://keepachangelog.com/en/1.0.0/ * remove from `Keeper`: `AddPubkey` * [#19440](https://github.com/cosmos/cosmos-sdk/pull/19440) Slashing Module creation takes `appmodule.Environment` instead of individual services * [#19458](https://github.com/cosmos/cosmos-sdk/pull/19458) ValidatorSigningInfo.IndexOffset is deprecated, and no longer used. The index is now derived using just the StartHeight. +* [#19740](https://github.com/cosmos/cosmos-sdk/pull/19740) Verify `InitGenesis` and `ExportGenesis` module code and keeper code do not panic. ### Bug Fixes diff --git a/x/slashing/keeper/genesis.go b/x/slashing/keeper/genesis.go index ac4af63f0054..f551fe748551 100644 --- a/x/slashing/keeper/genesis.go +++ b/x/slashing/keeper/genesis.go @@ -11,16 +11,19 @@ import ( // InitGenesis initializes default parameters and the keeper's address to // pubkey map. func (keeper Keeper) InitGenesis(ctx context.Context, stakingKeeper types.StakingKeeper, data *types.GenesisState) error { + var fnErr error err := stakingKeeper.IterateValidators(ctx, func(index int64, validator sdk.ValidatorI) bool { consPk, err := validator.ConsPubKey() if err != nil { - panic(err) + fnErr = err + return true } err = keeper.AddrPubkeyRelation.Set(ctx, consPk.Address(), consPk) if err != nil { - panic(err) + fnErr = err + return true } return false }, @@ -28,6 +31,9 @@ func (keeper Keeper) InitGenesis(ctx context.Context, stakingKeeper types.Stakin if err != nil { return err } + if fnErr != nil { + return fnErr + } for _, info := range data.SigningInfos { address, err := keeper.sk.ConsensusAddressCodec().StringToBytes(info.Address) @@ -62,17 +68,17 @@ func (keeper Keeper) InitGenesis(ctx context.Context, stakingKeeper types.Stakin // ExportGenesis writes the current store values // to a genesis file, which can be imported again // with InitGenesis -func (keeper Keeper) ExportGenesis(ctx context.Context) (data *types.GenesisState) { +func (keeper Keeper) ExportGenesis(ctx context.Context) (*types.GenesisState, error) { params, err := keeper.Params.Get(ctx) if err != nil { - panic(err) + return nil, err } signingInfos := make([]types.SigningInfo, 0) missedBlocks := make([]types.ValidatorMissedBlocks, 0) err = keeper.ValidatorSigningInfo.Walk(ctx, nil, func(address sdk.ConsAddress, info types.ValidatorSigningInfo) (stop bool, err error) { bechAddr, err := keeper.sk.ConsensusAddressCodec().BytesToString(address) if err != nil { - panic(err) + return true, err } signingInfos = append(signingInfos, types.SigningInfo{ Address: bechAddr, @@ -81,7 +87,7 @@ func (keeper Keeper) ExportGenesis(ctx context.Context) (data *types.GenesisStat localMissedBlocks, err := keeper.GetValidatorMissedBlocks(ctx, address) if err != nil { - panic(err) + return true, err } missedBlocks = append(missedBlocks, types.ValidatorMissedBlocks{ @@ -92,7 +98,7 @@ func (keeper Keeper) ExportGenesis(ctx context.Context) (data *types.GenesisStat return false, nil }) if err != nil { - panic(err) + return nil, err } - return types.NewGenesisState(params, signingInfos, missedBlocks) + return types.NewGenesisState(params, signingInfos, missedBlocks), nil } diff --git a/x/slashing/keeper/genesis_test.go b/x/slashing/keeper/genesis_test.go index c8ba1131da7c..0f5485c01bcf 100644 --- a/x/slashing/keeper/genesis_test.go +++ b/x/slashing/keeper/genesis_test.go @@ -30,7 +30,8 @@ func (s *KeeperTestSuite) TestExportAndInitGenesis() { s.Require().NoError(keeper.ValidatorSigningInfo.Set(ctx, consAddr1, info1)) s.Require().NoError(keeper.ValidatorSigningInfo.Set(ctx, consAddr2, info2)) - genesisState := keeper.ExportGenesis(ctx) + genesisState, err := keeper.ExportGenesis(ctx) + require.NoError(err) require.Equal(genesisState.Params, testutil.TestParams()) require.Len(genesisState.SigningInfos, 2) diff --git a/x/slashing/module.go b/x/slashing/module.go index 7d8499629a3c..1371ecdcee8e 100644 --- a/x/slashing/module.go +++ b/x/slashing/module.go @@ -146,7 +146,10 @@ func (am AppModule) InitGenesis(ctx context.Context, data json.RawMessage) error // ExportGenesis returns the exported genesis state as raw bytes for the slashing module. func (am AppModule) ExportGenesis(ctx context.Context) (json.RawMessage, error) { - gs := am.keeper.ExportGenesis(ctx) + gs, err := am.keeper.ExportGenesis(ctx) + if err != nil { + return nil, err + } return am.cdc.MarshalJSON(gs) } diff --git a/x/staking/CHANGELOG.md b/x/staking/CHANGELOG.md index 522fc2dce0e3..1cf49509062e 100644 --- a/x/staking/CHANGELOG.md +++ b/x/staking/CHANGELOG.md @@ -95,3 +95,4 @@ Ref: https://keepachangelog.com/en/1.0.0/ * [#18841](https://github.com/cosmos/cosmos-sdk/pull/18841) In a undelegation or redelegation if the shares being left delegated correspond to less than 1 token (in base denom) the entire delegation gets removed. * [#18142](https://github.com/cosmos/cosmos-sdk/pull/18142) Introduce `key_rotation_fee` param to calculate fees while rotating the keys * [#17655](https://github.com/cosmos/cosmos-sdk/pull/17655) `HistoricalInfo` was replaced with `HistoricalRecord`, it removes the validator set and comet header and only keep what is needed for IBC. +* [#19740](https://github.com/cosmos/cosmos-sdk/pull/19740) Verify `InitGenesis` and `ExportGenesis` module code and keeper code do not panic. \ No newline at end of file diff --git a/x/staking/keeper/genesis.go b/x/staking/keeper/genesis.go index f41e214219bd..662f56972e28 100644 --- a/x/staking/keeper/genesis.go +++ b/x/staking/keeper/genesis.go @@ -18,7 +18,7 @@ import ( // setting the indexes. In addition, it also sets any delegations found in // data. Finally, it updates the bonded validators. // Returns final validator set after applying all declaration and delegations -func (k Keeper) InitGenesis(ctx context.Context, data *types.GenesisState) (res []abci.ValidatorUpdate) { +func (k Keeper) InitGenesis(ctx context.Context, data *types.GenesisState) (res []abci.ValidatorUpdate, err error) { bondedTokens := math.ZeroInt() notBondedTokens := math.ZeroInt() @@ -32,42 +32,42 @@ func (k Keeper) InitGenesis(ctx context.Context, data *types.GenesisState) (res ctx = sdkCtx if err := k.Params.Set(ctx, data.Params); err != nil { - panic(err) + return nil, err } if err := k.LastTotalPower.Set(ctx, data.LastTotalPower); err != nil { - panic(err) + return nil, err } for _, validator := range data.Validators { if err := k.SetValidator(ctx, validator); err != nil { - panic(err) + return nil, err } // Manually set indices for the first time if err := k.SetValidatorByConsAddr(ctx, validator); err != nil { - panic(err) + return nil, err } if err := k.SetValidatorByPowerIndex(ctx, validator); err != nil { - panic(err) + return nil, err } // Call the creation hook if not exported if !data.Exported { valbz, err := k.ValidatorAddressCodec().StringToBytes(validator.GetOperator()) if err != nil { - panic(err) + return nil, err } if err := k.Hooks().AfterValidatorCreated(ctx, valbz); err != nil { - panic(err) + return nil, err } } // update timeslice if necessary if validator.IsUnbonding() { if err := k.InsertUnbondingValidatorQueue(ctx, validator); err != nil { - panic(err) + return nil, err } } @@ -79,48 +79,48 @@ func (k Keeper) InitGenesis(ctx context.Context, data *types.GenesisState) (res notBondedTokens = notBondedTokens.Add(validator.GetTokens()) default: - panic(fmt.Sprintf("invalid validator status: %v", validator.GetStatus())) + return nil, fmt.Errorf("invalid validator status: %v", validator.GetStatus()) } } for _, delegation := range data.Delegations { delegatorAddress, err := k.authKeeper.AddressCodec().StringToBytes(delegation.DelegatorAddress) if err != nil { - panic(fmt.Errorf("invalid delegator address: %s", err)) + return nil, fmt.Errorf("invalid delegator address: %s", err) } valAddr, err := k.validatorAddressCodec.StringToBytes(delegation.GetValidatorAddr()) if err != nil { - panic(err) + return nil, err } // Call the before-creation hook if not exported if !data.Exported { if err := k.Hooks().BeforeDelegationCreated(ctx, delegatorAddress, valAddr); err != nil { - panic(err) + return nil, err } } if err := k.SetDelegation(ctx, delegation); err != nil { - panic(err) + return nil, err } // Call the after-modification hook if not exported if !data.Exported { if err := k.Hooks().AfterDelegationModified(ctx, delegatorAddress, valAddr); err != nil { - panic(err) + return nil, err } } } for _, ubd := range data.UnbondingDelegations { if err := k.SetUnbondingDelegation(ctx, ubd); err != nil { - panic(err) + return nil, err } for _, entry := range ubd.Entries { if err := k.InsertUBDQueue(ctx, ubd, entry.CompletionTime); err != nil { - panic(err) + return nil, err } notBondedTokens = notBondedTokens.Add(entry.Balance) } @@ -128,12 +128,12 @@ func (k Keeper) InitGenesis(ctx context.Context, data *types.GenesisState) (res for _, red := range data.Redelegations { if err := k.SetRedelegation(ctx, red); err != nil { - panic(err) + return nil, err } for _, entry := range red.Entries { if err := k.InsertRedelegationQueue(ctx, red, entry.CompletionTime); err != nil { - panic(err) + return nil, err } } } @@ -144,7 +144,7 @@ func (k Keeper) InitGenesis(ctx context.Context, data *types.GenesisState) (res // check if the unbonded and bonded pools accounts exists bondedPool := k.GetBondedPool(ctx) if bondedPool == nil { - panic(fmt.Sprintf("%s module account has not been set", types.BondedPoolName)) + return nil, fmt.Errorf("%s module account has not been set", types.BondedPoolName) } // TODO: remove with genesis 2-phases refactor https://github.com/cosmos/cosmos-sdk/issues/2862 @@ -154,14 +154,14 @@ func (k Keeper) InitGenesis(ctx context.Context, data *types.GenesisState) (res k.authKeeper.SetModuleAccount(ctx, bondedPool) } - // if balance is different from bonded coins panic because genesis is most likely malformed + // if balance is different from bonded coins error because genesis is most likely malformed if !bondedBalance.Equal(bondedCoins) { - panic(fmt.Sprintf("bonded pool balance is different from bonded coins: %s <-> %s", bondedBalance, bondedCoins)) + return nil, fmt.Errorf("bonded pool balance is different from bonded coins: %s <-> %s", bondedBalance, bondedCoins) } notBondedPool := k.GetNotBondedPool(ctx) if notBondedPool == nil { - panic(fmt.Sprintf("%s module account has not been set", types.NotBondedPoolName)) + return nil, fmt.Errorf("%s module account has not been set", types.NotBondedPoolName) } notBondedBalance := k.bankKeeper.GetAllBalances(ctx, notBondedPool.GetAddress()) @@ -169,10 +169,10 @@ func (k Keeper) InitGenesis(ctx context.Context, data *types.GenesisState) (res k.authKeeper.SetModuleAccount(ctx, notBondedPool) } - // If balance is different from non bonded coins panic because genesis is most + // If balance is different from non bonded coins error because genesis is most // likely malformed. if !notBondedBalance.Equal(notBondedCoins) { - panic(fmt.Sprintf("not bonded pool balance is different from not bonded coins: %s <-> %s", notBondedBalance, notBondedCoins)) + return nil, fmt.Errorf("not bonded pool balance is different from not bonded coins: %s <-> %s", notBondedBalance, notBondedCoins) } // don't need to run CometBFT updates if we exported @@ -180,17 +180,17 @@ func (k Keeper) InitGenesis(ctx context.Context, data *types.GenesisState) (res for _, lv := range data.LastValidatorPowers { valAddr, err := k.validatorAddressCodec.StringToBytes(lv.Address) if err != nil { - panic(err) + return nil, err } err = k.SetLastValidatorPower(ctx, valAddr, lv.Power) if err != nil { - panic(err) + return nil, err } validator, err := k.GetValidator(ctx, valAddr) if err != nil { - panic(fmt.Sprintf("validator %s not found", lv.Address)) + return nil, fmt.Errorf("validator %s not found", lv.Address) } update := validator.ABCIValidatorUpdate(k.PowerReduction(ctx)) @@ -202,19 +202,19 @@ func (k Keeper) InitGenesis(ctx context.Context, data *types.GenesisState) (res res, err = k.ApplyAndReturnValidatorSetUpdates(ctx) if err != nil { - panic(err) + return nil, err } } - return res + return res, nil } // ExportGenesis returns a GenesisState for a given context and keeper. The // GenesisState will contain the pool, params, validators, and bonds found in // the keeper. -func (k Keeper) ExportGenesis(ctx context.Context) *types.GenesisState { +func (k Keeper) ExportGenesis(ctx context.Context) (*types.GenesisState, error) { var unbondingDelegations []types.UnbondingDelegation - + var fnErr error err := k.UnbondingDelegations.Walk( ctx, nil, @@ -224,7 +224,7 @@ func (k Keeper) ExportGenesis(ctx context.Context) *types.GenesisState { }, ) if err != nil { - panic(err) + return nil, err } var redelegations []types.Redelegation @@ -234,7 +234,7 @@ func (k Keeper) ExportGenesis(ctx context.Context) *types.GenesisState { return false }) if err != nil { - panic(err) + return nil, err } var lastValidatorPowers []types.LastValidatorPower @@ -242,33 +242,37 @@ func (k Keeper) ExportGenesis(ctx context.Context) *types.GenesisState { err = k.IterateLastValidatorPowers(ctx, func(addr sdk.ValAddress, power int64) (stop bool) { addrStr, err := k.validatorAddressCodec.BytesToString(addr) if err != nil { - panic(err) + fnErr = err + return true } lastValidatorPowers = append(lastValidatorPowers, types.LastValidatorPower{Address: addrStr, Power: power}) return false }) if err != nil { - panic(err) + return nil, err + } + if fnErr != nil { + return nil, fnErr } params, err := k.Params.Get(ctx) if err != nil { - panic(err) + return nil, err } totalPower, err := k.LastTotalPower.Get(ctx) if err != nil { - panic(err) + return nil, err } allDelegations, err := k.GetAllDelegations(ctx) if err != nil { - panic(err) + return nil, err } allValidators, err := k.GetAllValidators(ctx) if err != nil { - panic(err) + return nil, err } return &types.GenesisState{ @@ -280,5 +284,5 @@ func (k Keeper) ExportGenesis(ctx context.Context) *types.GenesisState { UnbondingDelegations: unbondingDelegations, Redelegations: redelegations, Exported: true, - } + }, nil } diff --git a/x/staking/module.go b/x/staking/module.go index c841b70546f3..359b0beb83ac 100644 --- a/x/staking/module.go +++ b/x/staking/module.go @@ -150,7 +150,10 @@ func (am AppModule) InitGenesis(ctx context.Context, data json.RawMessage) ([]mo am.cdc.MustUnmarshalJSON(data, &genesisState) - cometValidatorUpdates := am.keeper.InitGenesis(ctx, &genesisState) // TODO: refactor to return ValidatorUpdate higher up the stack + cometValidatorUpdates, err := am.keeper.InitGenesis(ctx, &genesisState) // TODO: refactor to return ValidatorUpdate higher up the stack + if err != nil { + return nil, err + } validatorUpdates := make([]module.ValidatorUpdate, len(cometValidatorUpdates)) for i, v := range cometValidatorUpdates { if ed25519 := v.PubKey.GetEd25519(); len(ed25519) > 0 { @@ -175,7 +178,15 @@ func (am AppModule) InitGenesis(ctx context.Context, data json.RawMessage) ([]mo // ExportGenesis returns the exported genesis state as raw bytes for the staking module. func (am AppModule) ExportGenesis(ctx context.Context) (json.RawMessage, error) { - return am.cdc.MarshalJSON(am.keeper.ExportGenesis(ctx)) + genesis, err := am.keeper.ExportGenesis(ctx) + if err != nil { + return nil, err + } + marshalJSON, err := am.cdc.MarshalJSON(genesis) + if err != nil { + return nil, err + } + return marshalJSON, nil } // ConsensusVersion implements HasConsensusVersion