Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

refactor(x/**)!: genesis return errors #19740

Merged
merged 8 commits into from
Mar 13, 2024
Merged
Show file tree
Hide file tree
Changes from 7 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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
14 changes: 10 additions & 4 deletions types/module/core_module.go
julienrbrt marked this conversation as resolved.
Show resolved Hide resolved
Original file line number Diff line number Diff line change
Expand Up @@ -112,15 +112,18 @@ func (c coreAppModuleAdaptor) ExportGenesis(ctx context.Context) (json.RawMessag
}

if mod, ok := c.module.(HasABCIGenesis); ok {
return mod.ExportGenesis(ctx)
eg, err := mod.ExportGenesis(ctx)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

not needed, we can just return as it was

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think is needed , because staking module need.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

your snippet does the same thing as previously but is just more verbose, this is why it can be reverted.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

o, i see, i will do it

if err != nil {
return nil, err
}
return eg, nil
}

if mod, ok := c.module.(HasGenesis); ok {
eg, err := mod.ExportGenesis(ctx)
if err != nil {
return nil, err
}

return eg, nil
}

Expand All @@ -142,15 +145,18 @@ func (c coreAppModuleAdaptor) InitGenesis(ctx context.Context, bz json.RawMessag
}

if mod, ok := c.module.(HasABCIGenesis); ok {
return mod.InitGenesis(ctx, bz)
moduleValUpdates, err := mod.InitGenesis(ctx, bz)
julienrbrt marked this conversation as resolved.
Show resolved Hide resolved
if err != nil {
return nil, err
}
return moduleValUpdates, nil
}

if mod, ok := c.module.(HasGenesis); ok {
if err := mod.InitGenesis(ctx, bz); err != nil {
return nil, err
}
}

return nil, nil
}

Expand Down
8 changes: 5 additions & 3 deletions types/module/module.go
julienrbrt marked this conversation as resolved.
Show resolved Hide resolved
julienrbrt marked this conversation as resolved.
Show resolved Hide resolved
Original file line number Diff line number Diff line change
Expand Up @@ -565,12 +565,14 @@ func (m *Manager) ExportGenesisForModules(ctx sdk.Context, modulesToExport []str
} else if module, ok := mod.(HasABCIGenesis); ok {
channels[moduleName] = make(chan genesisResult)
go func(module HasABCIGenesis, ch chan genesisResult) {
ctx := ctx.WithGasMeter(storetypes.NewInfiniteGasMeter()) // avoid race conditions
jm, err := module.ExportGenesis(ctx)
ctx := ctx.WithGasMeter(storetypes.NewInfiniteGasMeter())
exportGenesis, err := module.ExportGenesis(ctx)
if err != nil {
ch <- genesisResult{nil, err}
} else {
// avoid race conditions
ch <- genesisResult{exportGenesis, nil}
}
ch <- genesisResult{jm, nil}
}(module, channels[moduleName])
}
}
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.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The changelog entry is clear and adheres to the changelog standards. However, consider revising the verb form for better grammatical agreement in the summary of other entries.

For example, in the entry for [#19637], the phrase "doesn't take a message router anymore" could be improved for clarity.

- doesn't take a message router anymore
+ no longer requires a message router

Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation.

Suggested change
* [#19740](https://github.com/cosmos/cosmos-sdk/pull/19740) Verify `InitGenesis` and `ExportGenesis` module code and keeper code do not panic.
* [#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.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The changelog entry is clear and adheres to the changelog standards. However, consider revising the verb form for better grammatical agreement in the summary of other entries.

For example, in the entry for [#18636], the phrase "methods now returns an error" could be improved for grammatical accuracy.

- methods now returns an error
+ methods now return an error

Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation.

Suggested change
* [#19740](https://github.com/cosmos/cosmos-sdk/pull/19740) Verify `InitGenesis` and `ExportGenesis` module code and keeper code do not panic.
* [#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.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The entry for PR #19740 could be enhanced for clarity. Consider specifying the modules affected by the changes to InitGenesis and ExportGenesis to provide more context to the reader.

- * [#19740](https://github.com/cosmos/cosmos-sdk/pull/19740) Verify `InitGenesis` and `ExportGenesis` module code and keeper code do not panic.
+ * [#19740](https://github.com/cosmos/cosmos-sdk/pull/19740) Updated `InitGenesis` and `ExportGenesis` in various modules (e.g., authz, bank, crisis) to return errors instead of panicking, enhancing error handling.

Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation.

Suggested change
* [#19740](https://github.com/cosmos/cosmos-sdk/pull/19740) Verify `InitGenesis` and `ExportGenesis` module code and keeper code do not panic.
* [#19740](https://github.com/cosmos/cosmos-sdk/pull/19740) Updated `InitGenesis` and `ExportGenesis` in various modules (e.g., authz, bank, crisis) to return errors instead of panicking, enhancing error handling.


### 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.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The changelog entry is clear and adheres to the changelog standards. However, consider revising the verb form for better grammatical agreement in the summary of other entries.

For example, in the entry for [#19101], the phrase "Add message based params configuration" could be improved for clarity.

- Add message based params configuration
+ Added message-based params configuration

Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation.

Suggested change
* [#19740](https://github.com/cosmos/cosmos-sdk/pull/19740) Verify `InitGenesis` and `ExportGenesis` module code and keeper code do not panic.
* [#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.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The changelog entry is clear and adheres to the changelog standards. However, consider revising the verb form for better grammatical agreement in the summary of other entries.

For example, in the entry for [#19638], the phrase "so no baseapp.MessageRouter is required is NewKeeper anymore" could be improved for clarity and grammatical accuracy.

- so no `baseapp.MessageRouter` is required is `NewKeeper` anymore
+ so that `NewKeeper` no longer requires a `baseapp.MessageRouter`.

Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation.

Suggested change
* [#19740](https://github.com/cosmos/cosmos-sdk/pull/19740) Verify `InitGenesis` and `ExportGenesis` module code and keeper code do not panic.
* [#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
Loading
Loading