Skip to content

Commit

Permalink
refactor(x/**)!: genesis return errors (#19740)
Browse files Browse the repository at this point in the history
Co-authored-by: chixiaoxiao <[email protected]>
  • Loading branch information
chixiaowen and chixiaoxiao authored Mar 13, 2024
1 parent cdc3291 commit db82004
Show file tree
Hide file tree
Showing 32 changed files with 167 additions and 112 deletions.
6 changes: 4 additions & 2 deletions tests/integration/gov/genesis_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
45 changes: 22 additions & 23 deletions tests/integration/staking/keeper/genesis_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)

Expand Down Expand Up @@ -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) {
Expand Down Expand Up @@ -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] {
Expand Down
2 changes: 1 addition & 1 deletion x/authz/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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.
3 changes: 2 additions & 1 deletion x/authz/keeper/genesis.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ package keeper

import (
"context"
"errors"

"cosmossdk.io/x/authz"

Expand All @@ -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)
Expand Down
1 change: 1 addition & 0 deletions x/bank/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
6 changes: 3 additions & 3 deletions x/bank/keeper/genesis.go
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand All @@ -70,5 +70,5 @@ func (k BaseKeeper) ExportGenesis(ctx context.Context) *types.GenesisState {
k.GetAllDenomMetaData(ctx),
k.GetAllSendEnabledEntries(ctx),
)
return rv
return rv, nil
}
3 changes: 2 additions & 1 deletion x/bank/keeper/genesis_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
2 changes: 1 addition & 1 deletion x/bank/keeper/keeper.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
5 changes: 4 additions & 1 deletion x/bank/module.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}

Expand Down
6 changes: 3 additions & 3 deletions x/crisis/keeper/genesis.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
6 changes: 4 additions & 2 deletions x/crisis/keeper/genesis_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -55,15 +55,17 @@ 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))
err = s.keeper.ConstantFee.Set(s.sdkCtx, constantFee)
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)
}

Expand Down
5 changes: 4 additions & 1 deletion x/crisis/module.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}

Expand Down
1 change: 1 addition & 0 deletions x/distribution/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
4 changes: 2 additions & 2 deletions x/distribution/keeper/genesis.go
Original file line number Diff line number Diff line change
Expand Up @@ -132,15 +132,15 @@ 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())
if balances.IsZero() {
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
}
Expand Down
1 change: 1 addition & 0 deletions x/gov/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
6 changes: 3 additions & 3 deletions x/gov/genesis.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
}
Expand Down
1 change: 1 addition & 0 deletions x/group/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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.
14 changes: 7 additions & 7 deletions x/group/keeper/genesis.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -54,40 +54,40 @@ 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

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)

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

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
}
3 changes: 2 additions & 1 deletion x/group/keeper/genesis_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)

Expand Down
5 changes: 4 additions & 1 deletion x/group/module/module.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}

Expand Down
4 changes: 4 additions & 0 deletions x/nft/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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


6 changes: 3 additions & 3 deletions x/nft/keeper/genesis.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand All @@ -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 {
Expand All @@ -66,5 +66,5 @@ func (k Keeper) ExportGenesis(ctx context.Context) *nft.GenesisState {
return &nft.GenesisState{
Classes: classes,
Entries: entries,
}
}, nil
}
3 changes: 2 additions & 1 deletion x/nft/keeper/keeper_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}

Expand Down
Loading

0 comments on commit db82004

Please sign in to comment.