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!: migrate to core appmodule.AppModule extension interfaces #13794

Merged
merged 25 commits into from
Nov 14, 2022
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
25 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
7 changes: 2 additions & 5 deletions runtime/module.go
Original file line number Diff line number Diff line change
Expand Up @@ -81,15 +81,12 @@ type AppInputs struct {
AppConfig *appv1alpha1.Config
Config *runtimev1alpha1.Module
AppBuilder *AppBuilder
Modules map[string]AppModuleWrapper
Copy link
Contributor

Choose a reason for hiding this comment

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

We can remove AppModuleWrapper in runtime/wrappers.go, right?

Copy link
Contributor

Choose a reason for hiding this comment

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

TBH I would even remove WrapAppModuleBasic and all ProvideModuleBasics. Does depinject need the AppModuleBasics? Maybe only provide the appmodule.AppModules?

Copy link
Member Author

Choose a reason for hiding this comment

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

I removed AppModuleWrapper. I'll look into removing WrapAppModuleBasic. It's needed for codec instantiation, but maybe I can find a workaround.

Copy link
Member Author

Choose a reason for hiding this comment

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

I removed WrapModuleBasic. A bunch of boilerplate gone now. Should I go ahead and make the rest of AppModuleBasic extension interfaces? In particular HasInterfaces and HasAmino?

Copy link
Contributor

Choose a reason for hiding this comment

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

Yes, I think that's a good idea. Totally fine to do in a follow-up too, as this already has approvals. It's not breaking anyways, right?

Modules map[string]appmodule.AppModule
BaseAppOptions []BaseAppOption
}

func SetupAppBuilder(inputs AppInputs) {
mm := &module.Manager{Modules: map[string]module.AppModule{}}
for name, wrapper := range inputs.Modules {
mm.Modules[name] = wrapper.AppModule
}
mm := module.NewManagerFromMap(inputs.Modules)
app := inputs.AppBuilder.app
app.baseAppOptions = inputs.BaseAppOptions
app.config = inputs.Config
Expand Down
5 changes: 2 additions & 3 deletions runtime/services/autocli.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,8 +4,7 @@ import (
"context"

autocliv1 "cosmossdk.io/api/cosmos/autocli/v1"

"github.com/cosmos/cosmos-sdk/types/module"
"cosmossdk.io/core/appmodule"
)

// AutoCLIQueryService implements the cosmos.autocli.v1.Query service.
Expand All @@ -15,7 +14,7 @@ type AutoCLIQueryService struct {
moduleOptions map[string]*autocliv1.ModuleOptions
}

func NewAutoCLIQueryService(appModules map[string]module.AppModule) *AutoCLIQueryService {
func NewAutoCLIQueryService(appModules map[string]appmodule.AppModule) *AutoCLIQueryService {
moduleOptions := map[string]*autocliv1.ModuleOptions{}
for modName, mod := range appModules {
if autoCliMod, ok := mod.(interface {
Expand Down
125 changes: 96 additions & 29 deletions types/module/module.go
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@ import (
"fmt"
"sort"

"cosmossdk.io/core/appmodule"
"github.com/grpc-ecosystem/grpc-gateway/runtime"
"github.com/spf13/cobra"
abci "github.com/tendermint/tendermint/abci/types"
Expand All @@ -47,19 +48,30 @@ import (

// AppModuleBasic is the standard form for basic non-dependant elements of an application module.
type AppModuleBasic interface {
Name() string
HasName
RegisterLegacyAminoCodec(*codec.LegacyAmino)
RegisterInterfaces(codectypes.InterfaceRegistry)

DefaultGenesis(codec.JSONCodec) json.RawMessage
ValidateGenesis(codec.JSONCodec, client.TxEncodingConfig, json.RawMessage) error
HasGenesisBasics

// client functionality
RegisterGRPCGatewayRoutes(client.Context, *runtime.ServeMux)
GetTxCmd() *cobra.Command
GetQueryCmd() *cobra.Command
}

// HasName allows the module to provide its own name for legacy purposes.
// Newer apps should specify the name for their modules using a map
tac0turtle marked this conversation as resolved.
Show resolved Hide resolved
// (see NewManagerFromMap).
type HasName interface {
Name() string
}

type HasGenesisBasics interface {
DefaultGenesis(codec.JSONCodec) json.RawMessage
ValidateGenesis(codec.JSONCodec, client.TxEncodingConfig, json.RawMessage) error
}

// BasicManager is a collection of AppModuleBasic
type BasicManager map[string]AppModuleBasic

Expand Down Expand Up @@ -143,21 +155,35 @@ func (bm BasicManager) AddQueryCommands(rootQueryCmd *cobra.Command) {
// AppModuleGenesis is the standard form for an application module genesis functions
type AppModuleGenesis interface {
AppModuleBasic
HasGenesis
}

type HasGenesis interface {
HasGenesisBasics
InitGenesis(sdk.Context, codec.JSONCodec, json.RawMessage) []abci.ValidatorUpdate
ExportGenesis(sdk.Context, codec.JSONCodec) json.RawMessage
}

// AppModule is the standard form for an application module
// Deprecated: AppModule is the legacy form for an application module. Most of
// its functionality has been moved to extension interfaces based off of
// appmodule.AppModule.
type AppModule interface {
AppModuleGenesis
appmodule.AppModule

AppModuleBasic
}

type HasRegisterInvariants interface {
aaronc marked this conversation as resolved.
Show resolved Hide resolved
// registers
RegisterInvariants(sdk.InvariantRegistry)
}

type HasRegisterServices interface {
// RegisterServices allows a module to register services
RegisterServices(Configurator)
}

type HasConsensusVersion interface {
// ConsensusVersion is a sequence number for state-breaking change of the
// module. It should be incremented on each consensus-breaking change
// introduced by the module. To avoid wrong/empty versions, the initial version
Expand Down Expand Up @@ -189,6 +215,12 @@ func NewGenesisOnlyAppModule(amg AppModuleGenesis) AppModule {
}
}

// IsOnePerModuleType implements the depinject.OnePerModuleType interface.
func (GenesisOnlyAppModule) IsOnePerModuleType() {}

// IsAppModule implements the appmodule.AppModule interface.
func (GenesisOnlyAppModule) IsAppModule() {}

// RegisterInvariants is a placeholder function register no invariants
func (GenesisOnlyAppModule) RegisterInvariants(_ sdk.InvariantRegistry) {}

Expand All @@ -212,17 +244,17 @@ func (GenesisOnlyAppModule) EndBlock(_ sdk.Context, _ abci.RequestEndBlock) []ab
// Manager defines a module manager that provides the high level utility for managing and executing
// operations for a group of modules
type Manager struct {
Modules map[string]AppModule
Modules map[string]appmodule.AppModule
OrderInitGenesis []string
OrderExportGenesis []string
OrderBeginBlockers []string
OrderEndBlockers []string
OrderMigrations []string
}

// NewManager creates a new Manager object
// Deprecated: NewManager creates a new Manager object
func NewManager(modules ...AppModule) *Manager {
moduleMap := make(map[string]AppModule)
moduleMap := make(map[string]appmodule.AppModule)
modulesStr := make([]string, 0, len(modules))
for _, module := range modules {
moduleMap[module.Name()] = module
Expand All @@ -238,6 +270,22 @@ func NewManager(modules ...AppModule) *Manager {
}
}

// NewManager creates a new Manager object
func NewManagerFromMap(moduleMap map[string]appmodule.AppModule) *Manager {
modulesStr := make([]string, 0, len(moduleMap))
for name := range moduleMap {
modulesStr = append(modulesStr, name)
}
Fixed Show fixed Hide fixed

return &Manager{
Modules: moduleMap,
OrderInitGenesis: modulesStr,
OrderExportGenesis: modulesStr,
OrderBeginBlockers: modulesStr,
OrderEndBlockers: modulesStr,
}
}

// SetOrderInitGenesis sets the order of init genesis calls
func (m *Manager) SetOrderInitGenesis(moduleNames ...string) {
m.assertNoForgottenModules("SetOrderInitGenesis", moduleNames)
Expand Down Expand Up @@ -273,15 +321,19 @@ func (m *Manager) SetOrderMigrations(moduleNames ...string) {
func (m *Manager) RegisterInvariants(ir sdk.InvariantRegistry) {
modules := maps.Values(m.Modules)
for _, module := range modules {
module.RegisterInvariants(ir)
if module, ok := module.(HasRegisterInvariants); ok {
module.RegisterInvariants(ir)
}
}
}

// RegisterServices registers all module services
func (m *Manager) RegisterServices(cfg Configurator) {
modules := maps.Values(m.Modules)
for _, module := range modules {
module.RegisterServices(cfg)
if module, ok := module.(HasRegisterServices); ok {
module.RegisterServices(cfg)
}
}
}

Expand All @@ -295,17 +347,20 @@ func (m *Manager) InitGenesis(ctx sdk.Context, cdc codec.JSONCodec, genesisData
if genesisData[moduleName] == nil {
continue
}
ctx.Logger().Debug("running initialization for module", "module", moduleName)

moduleValUpdates := m.Modules[moduleName].InitGenesis(ctx, cdc, genesisData[moduleName])
if module, ok := m.Modules[moduleName].(HasGenesis); ok {
ctx.Logger().Debug("running initialization for module", "module", moduleName)

// use these validator updates if provided, the module manager assumes
// only one module will update the validator set
if len(moduleValUpdates) > 0 {
if len(validatorUpdates) > 0 {
panic("validator InitGenesis updates already set by a previous module")
moduleValUpdates := module.InitGenesis(ctx, cdc, genesisData[moduleName])

// use these validator updates if provided, the module manager assumes
// only one module will update the validator set
if len(moduleValUpdates) > 0 {
if len(validatorUpdates) > 0 {
panic("validator InitGenesis updates already set by a previous module")
}
validatorUpdates = moduleValUpdates
}
validatorUpdates = moduleValUpdates
}
}

Expand All @@ -329,7 +384,9 @@ func (m *Manager) ExportGenesisForModules(ctx sdk.Context, cdc codec.JSONCodec,
genesisData := make(map[string]json.RawMessage)
if len(modulesToExport) == 0 {
for _, moduleName := range m.OrderExportGenesis {
genesisData[moduleName] = m.Modules[moduleName].ExportGenesis(ctx, cdc)
if module, ok := m.Modules[moduleName].(HasGenesis); ok {
genesisData[moduleName] = module.ExportGenesis(ctx, cdc)
}
}

return genesisData
Expand All @@ -341,7 +398,9 @@ func (m *Manager) ExportGenesisForModules(ctx sdk.Context, cdc codec.JSONCodec,
}

for _, moduleName := range modulesToExport {
genesisData[moduleName] = m.Modules[moduleName].ExportGenesis(ctx, cdc)
if module, ok := m.Modules[moduleName].(HasGenesis); ok {
genesisData[moduleName] = module.ExportGenesis(ctx, cdc)
}
}

return genesisData
Expand Down Expand Up @@ -450,7 +509,10 @@ func (m Manager) RunMigrations(ctx sdk.Context, cfg Configurator, fromVM Version
for _, moduleName := range modules {
module := m.Modules[moduleName]
fromVersion, exists := fromVM[moduleName]
toVersion := module.ConsensusVersion()
toVersion := uint64(0)
if module, ok := module.(HasConsensusVersion); ok {
toVersion = module.ConsensusVersion()
}

// We run migration if the module is specified in `fromVM`.
// Otherwise we run InitGenesis.
Expand All @@ -467,11 +529,13 @@ func (m Manager) RunMigrations(ctx sdk.Context, cfg Configurator, fromVM Version
}
} else {
ctx.Logger().Info(fmt.Sprintf("adding a new module: %s", moduleName))
moduleValUpdates := module.InitGenesis(ctx, c.cdc, module.DefaultGenesis(c.cdc))
// The module manager assumes only one module will update the
// validator set, and it can't be a new module.
if len(moduleValUpdates) > 0 {
return nil, sdkerrors.Wrapf(sdkerrors.ErrLogic, "validator InitGenesis update is already set by another module")
if module, ok := m.Modules[moduleName].(HasGenesis); ok {
moduleValUpdates := module.InitGenesis(ctx, c.cdc, module.DefaultGenesis(c.cdc))
// The module manager assumes only one module will update the
// validator set, and it can't be a new module.
if len(moduleValUpdates) > 0 {
return nil, sdkerrors.Wrapf(sdkerrors.ErrLogic, "validator InitGenesis update is already set by another module")
}
}
}

Expand Down Expand Up @@ -533,9 +597,12 @@ func (m *Manager) EndBlock(ctx sdk.Context, req abci.RequestEndBlock) abci.Respo
// GetVersionMap gets consensus version from all modules
func (m *Manager) GetVersionMap() VersionMap {
vermap := make(VersionMap)
for _, v := range m.Modules {
version := v.ConsensusVersion()
name := v.Name()
for name, v := range m.Modules {
version := uint64(0)
if v, ok := v.(HasConsensusVersion); ok {
version = v.ConsensusVersion()
}
name := name
vermap[name] = version
}

Expand Down
4 changes: 3 additions & 1 deletion types/module/simulation.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,9 @@ import (
"sort"
"time"

"cosmossdk.io/core/appmodule"
sdkmath "cosmossdk.io/math"

"github.com/cosmos/cosmos-sdk/codec"
sdk "github.com/cosmos/cosmos-sdk/types"
"github.com/cosmos/cosmos-sdk/types/simulation"
Expand Down Expand Up @@ -51,7 +53,7 @@ func NewSimulationManager(modules ...AppModuleSimulation) *SimulationManager {
// with the same moduleName.
// Then it attempts to cast every provided AppModule into an AppModuleSimulation.
// If the cast succeeds, its included, otherwise it is excluded.
func NewSimulationManagerFromAppModules(modules map[string]AppModule, overrideModules map[string]AppModuleSimulation) *SimulationManager {
func NewSimulationManagerFromAppModules(modules map[string]appmodule.AppModule, overrideModules map[string]AppModuleSimulation) *SimulationManager {
simModules := []AppModuleSimulation{}
appModuleNamesSorted := make([]string, 0, len(modules))
for moduleName := range modules {
Expand Down
5 changes: 2 additions & 3 deletions x/auth/module.go
Original file line number Diff line number Diff line change
Expand Up @@ -230,8 +230,7 @@ type AuthOutputs struct {
depinject.Out

AccountKeeper keeper.AccountKeeper
Module runtime.AppModuleWrapper
NewAppModule appmodule.AppModule
Module appmodule.AppModule
}

func ProvideModule(in AuthInputs) AuthOutputs {
Expand All @@ -257,5 +256,5 @@ func ProvideModule(in AuthInputs) AuthOutputs {
k := keeper.NewAccountKeeper(in.Cdc, in.Key, in.AccountI, maccPerms, in.Config.Bech32Prefix, authority.String())
m := NewAppModule(in.Cdc, k, in.RandomGenesisAccountsFn, in.LegacySubspace)

return AuthOutputs{AccountKeeper: k, Module: runtime.WrapAppModule(m), NewAppModule: m}
return AuthOutputs{AccountKeeper: k, Module: m}
}
12 changes: 10 additions & 2 deletions x/auth/vesting/module.go
Original file line number Diff line number Diff line change
Expand Up @@ -91,6 +91,14 @@ func NewAppModule(ak keeper.AccountKeeper, bk types.BankKeeper) AppModule {
}
}

var _ appmodule.AppModule = AppModule{}

// IsOnePerModuleType implements the depinject.OnePerModuleType interface.
func (am AppModule) IsOnePerModuleType() {}

// IsAppModule implements the appmodule.AppModule interface.
func (am AppModule) IsAppModule() {}

// RegisterInvariants performs a no-op; there are no invariants to enforce.
func (AppModule) RegisterInvariants(_ sdk.InvariantRegistry) {}

Expand Down Expand Up @@ -136,11 +144,11 @@ type VestingInputs struct {
type VestingOutputs struct {
depinject.Out

Module runtime.AppModuleWrapper
Module appmodule.AppModule
}

func ProvideModule(in VestingInputs) VestingOutputs {
m := NewAppModule(in.AccountKeeper, in.BankKeeper)

return VestingOutputs{Module: runtime.WrapAppModule(m)}
return VestingOutputs{Module: m}
}
13 changes: 11 additions & 2 deletions x/authz/module/module.go
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ import (
"cosmossdk.io/core/appmodule"

"cosmossdk.io/depinject"

"github.com/cosmos/cosmos-sdk/baseapp"
sdkclient "github.com/cosmos/cosmos-sdk/client"
"github.com/cosmos/cosmos-sdk/codec"
Expand Down Expand Up @@ -120,6 +121,14 @@ func NewAppModule(cdc codec.Codec, keeper keeper.Keeper, ak authz.AccountKeeper,
}
}

var _ appmodule.AppModule = AppModule{}

// IsOnePerModuleType implements the depinject.OnePerModuleType interface.
func (am AppModule) IsOnePerModuleType() {}

// IsAppModule implements the appmodule.AppModule interface.
func (am AppModule) IsAppModule() {}

// Name returns the authz module's name.
func (AppModule) Name() string {
return authz.ModuleName
Expand Down Expand Up @@ -185,13 +194,13 @@ type AuthzOutputs struct {
depinject.Out

AuthzKeeper keeper.Keeper
Module runtime.AppModuleWrapper
Module appmodule.AppModule
}

func ProvideModule(in AuthzInputs) AuthzOutputs {
k := keeper.NewKeeper(in.Key, in.Cdc, in.MsgServiceRouter, in.AccountKeeper)
m := NewAppModule(in.Cdc, k, in.AccountKeeper, in.BankKeeper, in.Registry)
return AuthzOutputs{AuthzKeeper: k, Module: runtime.WrapAppModule(m)}
return AuthzOutputs{AuthzKeeper: k, Module: m}
}

// ____________________________________________________________________________
Expand Down
Loading