diff --git a/CHANGELOG.md b/CHANGELOG.md index 2cdf5fcf38e6..b81c79292e25 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -62,6 +62,8 @@ Ref: https://keepachangelog.com/en/1.0.0/ * (x/group) [#17146](https://github.com/cosmos/cosmos-sdk/pull/17146) Rename x/group legacy ORM package's error codespace from "orm" to "legacy_orm", preventing collisions with ORM's error codespace "orm". * (x/bank) [#17170](https://github.com/cosmos/cosmos-sdk/pull/17170) Avoid empty spendable error message on send coins. * (x/distribution) [#17236](https://github.com/cosmos/cosmos-sdk/pull/17236) Using "validateCommunityTax" in "Params.ValidateBasic", preventing panic when field "CommunityTax" is nil. +* (types) [#16583](https://github.com/cosmos/cosmos-sdk/pull/16583) Add `MigrationModuleManager` to handle migration of upgrade module before other modules, ensuring access to the updated context with consensus parameters within the same block that executes the migration. +* (baseapp) [#17372](https://github.com/cosmos/cosmos-sdk/pull/17372) Stop state-machine when `RunMigrationBeginBlock` has error. ### API Breaking Changes diff --git a/UPGRADING.md b/UPGRADING.md index 515a3b37df77..1668638a10d2 100644 --- a/UPGRADING.md +++ b/UPGRADING.md @@ -122,7 +122,7 @@ In this section we describe the changes made in Cosmos SDK' SimApp. #### Module Assertions -Previously, all modules were required to be set in `OrderBeginBlockers`, `OrderEndBlockers` and `OrderInitGenesis / OrderExportGenesis` in `app.go` / `app_config.go`. This is no longer the case, the assertion has been loosened to only require modules implementing, respectively, the `module.BeginBlockAppModule`, `module.EndBlockAppModule` and `module.HasGenesis` interfaces. +Previously, all modules were required to be set in `OrderBeginBlockers`, `OrderEndBlockers` and `OrderInitGenesis / OrderExportGenesis` in `app.go` / `app_config.go`. This is no longer the case, the assertion has been loosened to only require modules implementing, respectively, the `appmodule.HasBeginBlocker`, `appmodule.HasEndBlocker` and `appmodule.HasGenesis` / `module.HasGenesis` interfaces. #### Module wiring diff --git a/baseapp/baseapp.go b/baseapp/baseapp.go index 7db78a04d2bb..84fb00a066f7 100644 --- a/baseapp/baseapp.go +++ b/baseapp/baseapp.go @@ -43,6 +43,12 @@ type ( StoreLoader func(ms storetypes.CommitMultiStore) error ) +// MigrationModuleManager is the interface that a migration module manager should implement to handle +// the execution of migration logic during the beginning of a block. +type MigrationModuleManager interface { + RunMigrationBeginBlock(ctx sdk.Context) (bool, error) +} + const ( execModeCheck execMode = iota // Check a transaction execModeReCheck // Recheck a (pending) transaction after a commit @@ -92,6 +98,9 @@ type BaseApp struct { // manages snapshots, i.e. dumps of app state at certain intervals snapshotManager *snapshots.Manager + // manages migrate module + migrationModuleManager MigrationModuleManager + // volatile states: // // - checkState is set on InitChain and reset on Commit @@ -267,6 +276,11 @@ func (app *BaseApp) SetMsgServiceRouter(msgServiceRouter *MsgServiceRouter) { app.msgServiceRouter = msgServiceRouter } +// SetMigrationModuleManager sets the MigrationModuleManager of a BaseApp. +func (app *BaseApp) SetMigrationModuleManager(migrationModuleManager MigrationModuleManager) { + app.migrationModuleManager = migrationModuleManager +} + // MountStores mounts all IAVL or DB stores to the provided keys in the BaseApp // multistore. func (app *BaseApp) MountStores(keys ...storetypes.StoreKey) { @@ -671,7 +685,22 @@ func (app *BaseApp) beginBlock(req *abci.RequestFinalizeBlock) (sdk.BeginBlock, ) if app.beginBlocker != nil { - resp, err = app.beginBlocker(app.finalizeBlockState.ctx) + ctx := app.finalizeBlockState.ctx + if app.migrationModuleManager != nil { + if success, err := app.migrationModuleManager.RunMigrationBeginBlock(ctx); success { + cp := ctx.ConsensusParams() + // Manager skips this step if Block is non-nil since upgrade module is expected to set this params + // and consensus parameters should not be overwritten. + if cp.Block == nil { + if cp = app.GetConsensusParams(ctx); cp.Block != nil { + ctx = ctx.WithConsensusParams(cp) + } + } + } else if err != nil { + return sdk.BeginBlock{}, err + } + } + resp, err = app.beginBlocker(ctx) if err != nil { return resp, err } diff --git a/docs/docs/building-modules/01-module-manager.md b/docs/docs/building-modules/01-module-manager.md index 6f28829ce1d1..b1e88d59639d 100644 --- a/docs/docs/building-modules/01-module-manager.md +++ b/docs/docs/building-modules/01-module-manager.md @@ -265,7 +265,8 @@ The module manager is used throughout the application whenever an action on a co * `InitGenesis(ctx context.Context, cdc codec.JSONCodec, genesisData map[string]json.RawMessage)`: Calls the [`InitGenesis`](./08-genesis.md#initgenesis) function of each module when the application is first started, in the order defined in `OrderInitGenesis`. Returns an `abci.ResponseInitChain` to the underlying consensus engine, which can contain validator updates. * `ExportGenesis(ctx context.Context, cdc codec.JSONCodec)`: Calls the [`ExportGenesis`](./08-genesis.md#exportgenesis) function of each module, in the order defined in `OrderExportGenesis`. The export constructs a genesis file from a previously existing state, and is mainly used when a hard-fork upgrade of the chain is required. * `ExportGenesisForModules(ctx context.Context, cdc codec.JSONCodec, modulesToExport []string)`: Behaves the same as `ExportGenesis`, except takes a list of modules to export. -* `BeginBlock(ctx context.Context) error`: At the beginning of each block, this function is called from [`BaseApp`](../core/00-baseapp.md#beginblock) and, in turn, calls the [`BeginBlock`](./05-beginblock-endblock.md) function of each modules implementing the `BeginBlockAppModule` interface, in the order defined in `OrderBeginBlockers`. It creates a child [context](../core/02-context.md) with an event manager to aggregate [events](../core/08-events.md) emitted from all modules. +* `RunMigrationBeginBlock(ctx sdk.Context) (bool, error)`: At the beginning of each block, this function is called from [`BaseApp`](../core/00-baseapp.md#beginblock) and, in turn, calls the [`BeginBlock`](./05-beginblock-endblock.md) function of the upgrade module implementing the `HasBeginBlocker` interface. The function returns a boolean value indicating whether the migration was executed or not and an error if fails. +* `BeginBlock(ctx context.Context) error`: At the beginning of each block, this function is called from [`BaseApp`](../core/00-baseapp.md#beginblock) and, in turn, calls the [`BeginBlock`](./05-beginblock-endblock.md) function of each non-upgrade modules implementing the `HasBeginBlocker` interface, in the order defined in `OrderBeginBlockers`. It creates a child [context](../core/02-context.md) with an event manager to aggregate [events](../core/08-events.md) emitted from non-upgrade modules. * `EndBlock(ctx context.Context) error`: At the end of each block, this function is called from [`BaseApp`](../core/00-baseapp.md#endblock) and, in turn, calls the [`EndBlock`](./05-beginblock-endblock.md) function of each modules implementing the `HasEndBlocker` interface, in the order defined in `OrderEndBlockers`. It creates a child [context](../core/02-context.md) with an event manager to aggregate [events](../core/08-events.md) emitted from all modules. The function returns an `abci` which contains the aforementioned events, as well as validator set updates (if any). * `EndBlock(context.Context) ([]abci.ValidatorUpdate, error)`: At the end of each block, this function is called from [`BaseApp`](../core/00-baseapp.md#endblock) and, in turn, calls the [`EndBlock`](./05-beginblock-endblock.md) function of each modules implementing the `HasEndBlocker` interface, in the order defined in `OrderEndBlockers`. It creates a child [context](../core/02-context.md) with an event manager to aggregate [events](../core/08-events.md) emitted from all modules. The function returns an `abci` which contains the aforementioned events, as well as validator set updates (if any). * `Precommit(ctx context.Context)`: During [`Commit`](../core/00-baseapp.md#commit), this function is called from `BaseApp` immediately before the [`deliverState`](../core/00-baseapp.md#state-updates) is written to the underlying [`rootMultiStore`](../core/04-store.md#commitmultistore) and, in turn calls the `Precommit` function of each modules implementing the `HasPrecommit` interface, in the order defined in `OrderPrecommiters`. It creates a child [context](../core/02-context.md) where the underlying `CacheMultiStore` is that of the newly committed block's [`finalizeblockstate`](../core/00-baseapp.md#state-updates). diff --git a/docs/docs/core/00-baseapp.md b/docs/docs/core/00-baseapp.md index 3a0aa2651d5b..20a89fb11924 100644 --- a/docs/docs/core/00-baseapp.md +++ b/docs/docs/core/00-baseapp.md @@ -455,7 +455,7 @@ https://github.com/cosmos/cosmos-sdk/blob/v0.50.0-alpha.0/baseapp/abci.go#L623 This function also resets the [main gas meter](../basics/04-gas-fees.md#main-gas-meter). * Initialize the [block gas meter](../basics/04-gas-fees.md#block-gas-meter) with the `maxGas` limit. The `gas` consumed within the block cannot go above `maxGas`. This parameter is defined in the application's consensus parameters. -* Run the application's [`beginBlocker()`](../basics/00-app-anatomy.md#beginblocker-and-endblock), which mainly runs the [`BeginBlocker()`](../building-modules/05-beginblock-endblock.md#beginblock) method of each of the application's modules. +* Run the application's [`beginBlocker()`](../basics/00-app-anatomy.md#beginblocker-and-endblock), which mainly runs the [`BeginBlocker()`](../building-modules/05-beginblock-endblock.md#beginblock) method of each of the non-upgrade modules. * Set the [`VoteInfos`](https://github.com/cometbft/cometbft/blob/v0.37.x/spec/abci/abci++_methods.md#voteinfo) of the application, i.e. the list of validators whose _precommit_ for the previous block was included by the proposer of the current block. This information is carried into the [`Context`](./02-context.md) so that it can be used during transaction execution and EndBlock. #### Transaction Execution diff --git a/runtime/builder.go b/runtime/builder.go index 72bf965f7082..07e16bfcc20d 100644 --- a/runtime/builder.go +++ b/runtime/builder.go @@ -35,6 +35,7 @@ func (a *AppBuilder) Build(db dbm.DB, traceStore io.Writer, baseAppOptions ...fu bApp.SetVersion(version.Version) bApp.SetInterfaceRegistry(a.app.interfaceRegistry) bApp.MountStores(a.app.storeKeys...) + bApp.SetMigrationModuleManager(a.app.ModuleManager) a.app.BaseApp = bApp a.app.configurator = module.NewConfigurator(a.app.cdc, a.app.MsgServiceRouter(), a.app.GRPCQueryRouter()) diff --git a/simapp/app.go b/simapp/app.go index 6caa015cf1eb..20ff245460c1 100644 --- a/simapp/app.go +++ b/simapp/app.go @@ -401,6 +401,7 @@ func NewSimApp( consensus.NewAppModule(appCodec, app.ConsensusParamsKeeper), circuit.NewAppModule(appCodec, app.CircuitKeeper), ) + bApp.SetMigrationModuleManager(app.ModuleManager) // BasicModuleManager defines the module BasicManager is in charge of setting up basic, // non-dependant module elements, such as codec registration and genesis verification. diff --git a/testutil/mock/types_mock_appmodule.go b/testutil/mock/types_mock_appmodule.go index b5cab1121f0e..da310e69d4e7 100644 --- a/testutil/mock/types_mock_appmodule.go +++ b/testutil/mock/types_mock_appmodule.go @@ -360,3 +360,40 @@ func (mr *MockCoreAppModuleMockRecorder) ValidateGenesis(arg0 interface{}) *gomo mr.mock.ctrl.T.Helper() return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "ValidateGenesis", reflect.TypeOf((*MockCoreAppModule)(nil).ValidateGenesis), arg0) } + +// MockUpgradeModule is a mock of UpgradeModule interface. +type MockUpgradeModule struct { + *MockCoreAppModule + recorder *MockUpgradeModuleMockRecorder +} + +// MockUpgradeModuleMockRecorder is the mock recorder for MockUpgradeModule. +type MockUpgradeModuleMockRecorder struct { + *MockCoreAppModuleMockRecorder +} + +// NewMockUpgradeModule creates a new mock instance. +func NewMockUpgradeModule(ctrl *gomock.Controller) *MockUpgradeModule { + mock := &MockUpgradeModule{ + MockCoreAppModule: NewMockCoreAppModule(ctrl), + } + mock.recorder = &MockUpgradeModuleMockRecorder{mock.MockCoreAppModule.recorder} + return mock +} + +// EXPECT returns an object that allows the caller to indicate expected use. +func (m *MockUpgradeModule) EXPECT() *MockUpgradeModuleMockRecorder { + return m.recorder +} + +// IsUpgradeModule mocks base method. +func (m *MockUpgradeModule) IsUpgradeModule() { + m.ctrl.T.Helper() + m.ctrl.Call(m, "IsUpgradeModule") +} + +// IsUpgradeModule indicates an expected call of IsUpgradeModule. +func (mr *MockUpgradeModuleMockRecorder) IsUpgradeModule() *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "IsUpgradeModule", reflect.TypeOf((*MockUpgradeModule)(nil).IsUpgradeModule)) +} diff --git a/types/module/module.go b/types/module/module.go index 0ba4fa6c5a1d..f985d0d73792 100644 --- a/types/module/module.go +++ b/types/module/module.go @@ -67,6 +67,13 @@ type HasName interface { Name() string } +// UpgradeModule is the extension interface that upgrade module should implement to differentiate +// it from other modules, migration handler need ensure the upgrade module's migration is executed +// before the rest of the modules. +type UpgradeModule interface { + IsUpgradeModule() +} + // HasGenesisBasics is the legacy interface for stateless genesis methods. type HasGenesisBasics interface { DefaultGenesis(codec.JSONCodec) json.RawMessage @@ -692,17 +699,32 @@ func (m Manager) RunMigrations(ctx context.Context, cfg Configurator, fromVM Ver return updatedVM, nil } -// BeginBlock performs begin block functionality for all modules. It creates a -// child context with an event manager to aggregate events emitted from all +// RunMigrationBeginBlock performs begin block functionality for upgrade module. +// It takes the current context as a parameter and returns a boolean value +// indicating whether the migration was executed or not and an error if fails. +func (m *Manager) RunMigrationBeginBlock(ctx sdk.Context) (bool, error) { + for _, moduleName := range m.OrderBeginBlockers { + if mod, ok := m.Modules[moduleName].(appmodule.HasBeginBlocker); ok { + if _, ok := mod.(UpgradeModule); ok { + err := mod.BeginBlock(ctx) + return err == nil, err + } + } + } + return false, nil +} + +// BeginBlock performs begin block functionality for non-upgrade modules. It creates a +// child context with an event manager to aggregate events emitted from non-upgrade // modules. func (m *Manager) BeginBlock(ctx sdk.Context) (sdk.BeginBlock, error) { ctx = ctx.WithEventManager(sdk.NewEventManager()) - for _, moduleName := range m.OrderBeginBlockers { if module, ok := m.Modules[moduleName].(appmodule.HasBeginBlocker); ok { - err := module.BeginBlock(ctx) - if err != nil { - return sdk.BeginBlock{}, err + if _, ok := module.(UpgradeModule); !ok { + if err := module.BeginBlock(ctx); err != nil { + return sdk.BeginBlock{}, err + } } } } diff --git a/types/module/module_test.go b/types/module/module_test.go index 3cde79f6a6fe..aaf4c8b3b2de 100644 --- a/types/module/module_test.go +++ b/types/module/module_test.go @@ -462,6 +462,37 @@ func TestCoreAPIManagerOrderSetters(t *testing.T) { require.Equal(t, []string{"module3", "module2", "module1"}, mm.OrderPrecommiters) } +func TestCoreAPIManager_RunMigrationBeginBlock(t *testing.T) { + mockCtrl := gomock.NewController(t) + t.Cleanup(mockCtrl.Finish) + + mockAppModule1 := mock.NewMockCoreAppModule(mockCtrl) + mockAppModule2 := mock.NewMockUpgradeModule(mockCtrl) + mm := module.NewManagerFromMap(map[string]appmodule.AppModule{ + "module1": mockAppModule1, + "module2": mockAppModule2, + }) + require.NotNil(t, mm) + require.Equal(t, 2, len(mm.Modules)) + + mockAppModule1.EXPECT().BeginBlock(gomock.Any()).Times(0) + mockAppModule2.EXPECT().BeginBlock(gomock.Any()).Times(1).Return(nil) + success, err := mm.RunMigrationBeginBlock(sdk.Context{}) + require.Equal(t, true, success) + require.NoError(t, err) + + // test false + success, err = module.NewManager().RunMigrationBeginBlock(sdk.Context{}) + require.Equal(t, false, success) + require.NoError(t, err) + + // test panic + mockAppModule2.EXPECT().BeginBlock(gomock.Any()).Times(1).Return(errors.New("some error")) + success, err = mm.RunMigrationBeginBlock(sdk.Context{}) + require.Equal(t, false, success) + require.Error(t, err) +} + func TestCoreAPIManager_BeginBlock(t *testing.T) { mockCtrl := gomock.NewController(t) t.Cleanup(mockCtrl.Finish) diff --git a/x/upgrade/module.go b/x/upgrade/module.go index 9bb9618df7a2..918dc893aa9d 100644 --- a/x/upgrade/module.go +++ b/x/upgrade/module.go @@ -74,6 +74,19 @@ func (AppModuleBasic) RegisterInterfaces(registry codectypes.InterfaceRegistry) types.RegisterInterfaces(registry) } +// DefaultGenesis is an empty object +func (AppModuleBasic) DefaultGenesis(_ codec.JSONCodec) json.RawMessage { + return []byte("{}") +} + +// ValidateGenesis is always successful, as we ignore the value +func (AppModuleBasic) ValidateGenesis(_ codec.JSONCodec, config client.TxEncodingConfig, _ json.RawMessage) error { + return nil +} + +// IsUpgradeModule implements the module.UpgradeModule interface. +func (ab AppModuleBasic) IsUpgradeModule() {} + // AppModule implements the sdk.AppModule interface type AppModule struct { AppModuleBasic @@ -137,16 +150,6 @@ func (am AppModule) InitGenesis(ctx sdk.Context, _ codec.JSONCodec, _ json.RawMe return []abci.ValidatorUpdate{} } -// DefaultGenesis is an empty object -func (AppModuleBasic) DefaultGenesis(_ codec.JSONCodec) json.RawMessage { - return []byte("{}") -} - -// ValidateGenesis is always successful, as we ignore the value -func (AppModuleBasic) ValidateGenesis(_ codec.JSONCodec, config client.TxEncodingConfig, _ json.RawMessage) error { - return nil -} - // ExportGenesis is always empty, as InitGenesis does nothing either func (am AppModule) ExportGenesis(_ sdk.Context, cdc codec.JSONCodec) json.RawMessage { return am.DefaultGenesis(cdc)