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(modules): adopt appmodulev2.Hasgenesis #19627

Merged
merged 28 commits into from
Mar 11, 2024
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
28 commits
Select commit Hold shift + click to select a range
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
2 changes: 1 addition & 1 deletion core/appmodule/v2/genesis.go
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ import (
// migration of existing modules to the new app module API. It is intended to be replaced by collections
type HasGenesis interface {
AppModule
DefaultGenesis() Message
tac0turtle marked this conversation as resolved.
Show resolved Hide resolved
DefaultGenesis() json.RawMessage
ValidateGenesis(data json.RawMessage) error
InitGenesis(ctx context.Context, data json.RawMessage) error
ExportGenesis(ctx context.Context) (json.RawMessage, error)
Expand Down
2 changes: 1 addition & 1 deletion simapp/app.go
Original file line number Diff line number Diff line change
Expand Up @@ -420,7 +420,7 @@ func NewSimApp(
distr.NewAppModule(appCodec, app.DistrKeeper, app.AuthKeeper, app.BankKeeper, app.StakingKeeper, app.PoolKeeper),
staking.NewAppModule(appCodec, app.StakingKeeper, app.AuthKeeper, app.BankKeeper),
upgrade.NewAppModule(app.UpgradeKeeper),
evidence.NewAppModule(app.EvidenceKeeper),
evidence.NewAppModule(appCodec, app.EvidenceKeeper),
authzmodule.NewAppModule(appCodec, app.AuthzKeeper, app.AuthKeeper, app.BankKeeper, app.interfaceRegistry),
groupmodule.NewAppModule(appCodec, app.GroupKeeper, app.AuthKeeper, app.BankKeeper, app.interfaceRegistry),
nftmodule.NewAppModule(appCodec, app.NFTKeeper, app.AuthKeeper, app.BankKeeper, app.interfaceRegistry),
Expand Down
2 changes: 1 addition & 1 deletion tests/integration/evidence/keeper/infraction_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -139,7 +139,7 @@ func initFixture(tb testing.TB) *fixture {
bankModule := bank.NewAppModule(cdc, bankKeeper, accountKeeper)
stakingModule := staking.NewAppModule(cdc, stakingKeeper, accountKeeper, bankKeeper)
slashingModule := slashing.NewAppModule(cdc, slashingKeeper, accountKeeper, bankKeeper, stakingKeeper, cdc.InterfaceRegistry())
evidenceModule := evidence.NewAppModule(*evidenceKeeper)
evidenceModule := evidence.NewAppModule(cdc, *evidenceKeeper)

integrationApp := integration.NewIntegrationApp(newCtx, logger, keys, cdc, map[string]appmodule.AppModule{
authtypes.ModuleName: authModule,
Expand Down
2 changes: 1 addition & 1 deletion tests/integration/staking/keeper/vote_extensions_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -86,7 +86,7 @@ func TestValidateVoteExtensions(t *testing.T) {
votes = append(votes, ve)
}

err := baseapp.ValidateVoteExtensions(f.sdkCtx, f.stakingKeeper, f.sdkCtx.BlockHeight(), "chain-id-123", abci.ExtendedCommitInfo{Round: 0, Votes: votes})
err := baseapp.ValidateVoteExtensions(f.sdkCtx, f.stakingKeeper, abci.ExtendedCommitInfo{Round: 0, Votes: votes})
Copy link
Contributor

Choose a reason for hiding this comment

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

Tip

Codebase Verification

The simplification in the call to baseapp.ValidateVoteExtensions observed in vote_extensions_test.go does not appear to be consistently applied across the codebase, as calls in other test files and the implementation show different parameters being used. It's recommended to ensure that the function's usage is consistent with its intended signature and application across all relevant parts of the codebase.

Analysis chain

The simplification in the call to baseapp.ValidateVoteExtensions in TestValidateVoteExtensions is a positive change. Please ensure this new signature is consistently used across the codebase.

Scripts Executed

The following scripts were executed for the analysis:


Script:

#!/bin/bash
# Verify that all calls to ValidateVoteExtensions have been updated.
rg "ValidateVoteExtensions" --type go

Length of output: 2834

assert.NilError(t, err)
}

Expand Down
5 changes: 5 additions & 0 deletions types/module/module.go
julienrbrt marked this conversation as resolved.
Show resolved Hide resolved
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@ import (
"golang.org/x/exp/maps"

"cosmossdk.io/core/appmodule"
appmodulev2 "cosmossdk.io/core/appmodule/v2"
"cosmossdk.io/core/genesis"
errorsmod "cosmossdk.io/errors"
storetypes "cosmossdk.io/store/types"
Expand Down Expand Up @@ -217,6 +218,10 @@ func (m *Manager) SetOrderInitGenesis(moduleNames ...string) {
return !hasABCIGenesis
}

if _, hasGenesis := module.(appmodulev2.HasGenesis); hasGenesis {
return !hasGenesis
}

Copy link
Contributor

Choose a reason for hiding this comment

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

The addition of the appmodulev2.HasGenesis check in the SetOrderInitGenesis method is correctly implemented and aligns with the PR's objectives. This ensures that modules implementing the new interface are correctly recognized during the initialization order setup.

One minor suggestion for improvement is to add a comment explaining the purpose of this new check, especially for developers who might be unfamiliar with the transition to appmodulev2. This can enhance code readability and maintainability.

+ // Check if the module implements the appmodulev2.HasGenesis interface
  if _, hasGenesis := module.(appmodulev2.HasGenesis); hasGenesis {
      return !hasGenesis
  }

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
if _, hasGenesis := module.(appmodulev2.HasGenesis); hasGenesis {
return !hasGenesis
}
// Check if the module implements the appmodulev2.HasGenesis interface
if _, hasGenesis := module.(appmodulev2.HasGenesis); hasGenesis {
return !hasGenesis
}

_, hasGenesis := module.(HasGenesis)
tac0turtle marked this conversation as resolved.
Show resolved Hide resolved
return !hasGenesis
})
Expand Down
27 changes: 14 additions & 13 deletions x/auth/module.go
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import (
"google.golang.org/grpc"

"cosmossdk.io/core/appmodule"
appmodulev2 "cosmossdk.io/core/appmodule/v2"
"cosmossdk.io/x/auth/keeper"
"cosmossdk.io/x/auth/simulation"
"cosmossdk.io/x/auth/types"
Expand All @@ -29,7 +30,7 @@ const (
var (
_ module.AppModuleSimulation = AppModule{}
_ module.HasName = AppModule{}
_ module.HasGenesis = AppModule{}
_ appmodulev2.HasGenesis = AppModule{}

_ appmodule.AppModule = AppModule{}
_ appmodule.HasServices = AppModule{}
Expand All @@ -40,6 +41,7 @@ var (
type AppModule struct {
accountKeeper keeper.AccountKeeper
randGenAccountsFn types.RandomGenesisAccountsFn
cdc codec.Codec
}

// IsAppModule implements the appmodule.AppModule interface.
Expand Down Expand Up @@ -104,38 +106,37 @@ func (am AppModule) RegisterMigrations(mr appmodule.MigrationRegistrar) error {
}

// DefaultGenesis returns default genesis state as raw bytes for the auth module.
func (AppModule) DefaultGenesis(cdc codec.JSONCodec) json.RawMessage {
return cdc.MustMarshalJSON(types.DefaultGenesisState())
func (am AppModule) DefaultGenesis() json.RawMessage {
return am.cdc.MustMarshalJSON(types.DefaultGenesisState())
}

// ValidateGenesis performs genesis state validation for the auth module.
func (AppModule) ValidateGenesis(cdc codec.JSONCodec, config client.TxEncodingConfig, bz json.RawMessage) error {
func (am AppModule) ValidateGenesis(bz json.RawMessage) error {
var data types.GenesisState
if err := cdc.UnmarshalJSON(bz, &data); err != nil {
if err := am.cdc.UnmarshalJSON(bz, &data); err != nil {
return fmt.Errorf("failed to unmarshal %s genesis state: %w", types.ModuleName, err)
}

return types.ValidateGenesis(data)
}

// InitGenesis performs genesis initialization for the auth module.
func (am AppModule) InitGenesis(ctx context.Context, cdc codec.JSONCodec, data json.RawMessage) {
func (am AppModule) InitGenesis(ctx context.Context, data json.RawMessage) error {
var genesisState types.GenesisState
cdc.MustUnmarshalJSON(data, &genesisState)
err := am.accountKeeper.InitGenesis(ctx, genesisState)
if err != nil {
panic(err)
if err := am.cdc.UnmarshalJSON(data, &genesisState); err != nil {
return err
}
return am.accountKeeper.InitGenesis(ctx, genesisState)
}

// ExportGenesis returns the exported genesis state as raw bytes for the auth
// module.
func (am AppModule) ExportGenesis(ctx context.Context, cdc codec.JSONCodec) json.RawMessage {
func (am AppModule) ExportGenesis(ctx context.Context) (json.RawMessage, error) {
gs, err := am.accountKeeper.ExportGenesis(ctx)
if err != nil {
panic(err)
return nil, err
}
return cdc.MustMarshalJSON(gs)
return am.cdc.MarshalJSON(gs)
}

// ConsensusVersion implements HasConsensusVersion
Expand Down
9 changes: 5 additions & 4 deletions x/authz/keeper/genesis.go
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ import (
)

// InitGenesis initializes new authz genesis
func (k Keeper) InitGenesis(ctx context.Context, data *authz.GenesisState) {
func (k Keeper) InitGenesis(ctx context.Context, data *authz.GenesisState) error {
now := k.environment.HeaderService.GetHeaderInfo(ctx).Time
for _, entry := range data.Authorization {
// ignore expired authorizations
Expand All @@ -19,11 +19,11 @@ func (k Keeper) InitGenesis(ctx context.Context, data *authz.GenesisState) {

grantee, err := k.authKeeper.AddressCodec().StringToBytes(entry.Grantee)
if err != nil {
panic(err)
return err
}
granter, err := k.authKeeper.AddressCodec().StringToBytes(entry.Granter)
if err != nil {
panic(err)
return err
}

a, ok := entry.Authorization.GetCachedValue().(authz.Authorization)
Expand All @@ -33,9 +33,10 @@ func (k Keeper) InitGenesis(ctx context.Context, data *authz.GenesisState) {

err = k.SaveGrant(ctx, grantee, granter, a, entry.Expiration)
if err != nil {
panic(err)
return err
}
}
return nil
}

// ExportGenesis returns a GenesisState for a given context.
Expand Down
23 changes: 13 additions & 10 deletions x/authz/module/module.go
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import (
"google.golang.org/grpc"

"cosmossdk.io/core/appmodule"
appmodulev2 "cosmossdk.io/core/appmodule/v2"
"cosmossdk.io/errors"
"cosmossdk.io/x/authz"
"cosmossdk.io/x/authz/client/cli"
Expand All @@ -31,7 +32,7 @@ var (
_ module.HasGRPCGateway = AppModule{}
_ module.HasRegisterInterfaces = AppModule{}
_ module.AppModuleSimulation = AppModule{}
_ module.HasGenesis = AppModule{}
_ appmodulev2.HasGenesis = AppModule{}

_ appmodule.AppModule = AppModule{}
_ appmodule.HasBeginBlocker = AppModule{}
Expand Down Expand Up @@ -108,31 +109,33 @@ func (AppModule) GetTxCmd() *cobra.Command {
}

// DefaultGenesis returns default genesis state as raw bytes for the authz module.
func (AppModule) DefaultGenesis(cdc codec.JSONCodec) json.RawMessage {
return cdc.MustMarshalJSON(authz.DefaultGenesisState())
func (am AppModule) DefaultGenesis() json.RawMessage {
return am.cdc.MustMarshalJSON(authz.DefaultGenesisState())
}

// ValidateGenesis performs genesis state validation for the authz module.
func (AppModule) ValidateGenesis(cdc codec.JSONCodec, config sdkclient.TxEncodingConfig, bz json.RawMessage) error {
func (am AppModule) ValidateGenesis(bz json.RawMessage) error {
var data authz.GenesisState
if err := cdc.UnmarshalJSON(bz, &data); err != nil {
if err := am.cdc.UnmarshalJSON(bz, &data); err != nil {
return errors.Wrapf(err, "failed to unmarshal %s genesis state", authz.ModuleName)
}

return authz.ValidateGenesis(data)
}

// InitGenesis performs genesis initialization for the authz module.
func (am AppModule) InitGenesis(ctx context.Context, cdc codec.JSONCodec, data json.RawMessage) {
func (am AppModule) InitGenesis(ctx context.Context, data json.RawMessage) error {
var genesisState authz.GenesisState
cdc.MustUnmarshalJSON(data, &genesisState)
am.keeper.InitGenesis(ctx, &genesisState)
if err := am.cdc.UnmarshalJSON(data, &genesisState); err != nil {
return err
}
return am.keeper.InitGenesis(ctx, &genesisState)
}

// ExportGenesis returns the exported genesis state as raw bytes for the authz module.
func (am AppModule) ExportGenesis(ctx context.Context, cdc codec.JSONCodec) json.RawMessage {
func (am AppModule) ExportGenesis(ctx context.Context) (json.RawMessage, error) {
gs := am.keeper.ExportGenesis(ctx)
return cdc.MustMarshalJSON(gs)
return am.cdc.MarshalJSON(gs)
}

// ConsensusVersion implements HasConsensusVersion.
Expand Down
9 changes: 5 additions & 4 deletions x/bank/keeper/genesis.go
Original file line number Diff line number Diff line change
Expand Up @@ -12,9 +12,9 @@ import (
)

// InitGenesis initializes the bank module's state from a given genesis state.
func (k BaseKeeper) InitGenesis(ctx context.Context, genState *types.GenesisState) {
func (k BaseKeeper) InitGenesis(ctx context.Context, genState *types.GenesisState) error {
if err := k.SetParams(ctx, genState.Params); err != nil {
panic(err)
return err
}

for _, se := range genState.GetAllSendEnabled() {
Expand All @@ -28,13 +28,13 @@ func (k BaseKeeper) InitGenesis(ctx context.Context, genState *types.GenesisStat
addr := balance.GetAddress()
bz, err := k.ak.AddressCodec().StringToBytes(addr)
if err != nil {
panic(err)
return err
}

for _, coin := range balance.Coins {
err := k.Balances.Set(ctx, collections.Join(sdk.AccAddress(bz), coin.Denom), coin.Amount)
if err != nil {
panic(err)
return err
}
}

Expand All @@ -53,6 +53,7 @@ func (k BaseKeeper) InitGenesis(ctx context.Context, genState *types.GenesisStat
for _, meta := range genState.DenomMetadata {
k.SetDenomMetaData(ctx, meta)
}
return nil
}

// ExportGenesis returns the bank module's genesis state.
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 @@ -26,7 +26,7 @@ type Keeper interface {
SendKeeper
WithMintCoinsRestriction(types.MintingRestrictionFn) BaseKeeper

InitGenesis(context.Context, *types.GenesisState)
InitGenesis(context.Context, *types.GenesisState) error
ExportGenesis(context.Context) *types.GenesisState

GetSupply(ctx context.Context, denom string) sdk.Coin
Expand Down
27 changes: 13 additions & 14 deletions x/bank/module.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,13 +4,13 @@ import (
"context"
"encoding/json"
"fmt"
"time"

gwruntime "github.com/grpc-ecosystem/grpc-gateway/runtime"
"github.com/spf13/cobra"
"google.golang.org/grpc"

"cosmossdk.io/core/appmodule"
appmodulev2 "cosmossdk.io/core/appmodule/v2"
"cosmossdk.io/x/bank/client/cli"
"cosmossdk.io/x/bank/keeper"
"cosmossdk.io/x/bank/simulation"
Expand All @@ -19,7 +19,6 @@ import (
"github.com/cosmos/cosmos-sdk/client"
"github.com/cosmos/cosmos-sdk/codec"
codectypes "github.com/cosmos/cosmos-sdk/codec/types"
"github.com/cosmos/cosmos-sdk/telemetry"
sdk "github.com/cosmos/cosmos-sdk/types"
"github.com/cosmos/cosmos-sdk/types/module"
simtypes "github.com/cosmos/cosmos-sdk/types/simulation"
Expand All @@ -34,7 +33,7 @@ var (
_ module.HasGRPCGateway = AppModule{}
_ module.HasRegisterInterfaces = AppModule{}
_ module.AppModuleSimulation = AppModule{}
_ module.HasGenesis = AppModule{}
_ appmodulev2.HasGenesis = AppModule{}
_ module.HasInvariants = AppModule{}

_ appmodule.AppModule = AppModule{}
Expand Down Expand Up @@ -119,35 +118,35 @@ func (am AppModule) RegisterInvariants(ir sdk.InvariantRegistry) {
}

// DefaultGenesis returns default genesis state as raw bytes for the bank module.
func (AppModule) DefaultGenesis(cdc codec.JSONCodec) json.RawMessage {
return cdc.MustMarshalJSON(types.DefaultGenesisState())
func (am AppModule) DefaultGenesis() json.RawMessage {
return am.cdc.MustMarshalJSON(types.DefaultGenesisState())
}

// ValidateGenesis performs genesis state validation for the bank module.
func (AppModule) ValidateGenesis(cdc codec.JSONCodec, _ client.TxEncodingConfig, bz json.RawMessage) error {
func (am AppModule) ValidateGenesis(bz json.RawMessage) error {
var data types.GenesisState
if err := cdc.UnmarshalJSON(bz, &data); err != nil {
if err := am.cdc.UnmarshalJSON(bz, &data); err != nil {
return fmt.Errorf("failed to unmarshal %s genesis state: %w", types.ModuleName, err)
}

return data.Validate()
}

// InitGenesis performs genesis initialization for the bank module.
func (am AppModule) InitGenesis(ctx context.Context, cdc codec.JSONCodec, data json.RawMessage) {
start := time.Now()
func (am AppModule) InitGenesis(ctx context.Context, data json.RawMessage) error {
var genesisState types.GenesisState
cdc.MustUnmarshalJSON(data, &genesisState)
telemetry.MeasureSince(start, "InitGenesis", "crisis", "unmarshal")
if err := am.cdc.UnmarshalJSON(data, &genesisState); err != nil {
return err
}

am.keeper.InitGenesis(ctx, &genesisState)
return am.keeper.InitGenesis(ctx, &genesisState)
}

// ExportGenesis returns the exported genesis state as raw bytes for the bank
// module.
func (am AppModule) ExportGenesis(ctx context.Context, cdc codec.JSONCodec) json.RawMessage {
func (am AppModule) ExportGenesis(ctx context.Context) (json.RawMessage, error) {
gs := am.keeper.ExportGenesis(ctx)
return cdc.MustMarshalJSON(gs)
return am.cdc.MarshalJSON(gs)
}

// ConsensusVersion implements HasConsensusVersion
Expand Down
Loading
Loading