From da929211d6f78014e3b40de3dc01e0ffcf0104cb Mon Sep 17 00:00:00 2001 From: Robert Zaremba Date: Wed, 5 Jan 2022 23:32:10 +0100 Subject: [PATCH] feat: support in-place migration ordering (#10614) ## Description Closes: #10604 --- ### Author Checklist *All items are required. Please add a note to the item if the item is not applicable and please add links to any relevant follow up issues.* I have... - [x] included the correct [type prefix](https://github.com/commitizen/conventional-commit-types/blob/v3.0.0/index.json) in the PR title - [ ] added `!` to the type prefix if API or client breaking change - [ ] targeted the correct branch (see [PR Targeting](https://github.com/cosmos/cosmos-sdk/blob/master/CONTRIBUTING.md#pr-targeting)) - [ ] provided a link to the relevant issue or specification - [ ] followed the guidelines for [building modules](https://github.com/cosmos/cosmos-sdk/blob/master/docs/building-modules) - [ ] included the necessary unit and integration [tests](https://github.com/cosmos/cosmos-sdk/blob/master/CONTRIBUTING.md#testing) - [ ] added a changelog entry to `CHANGELOG.md` - [ ] included comments for [documenting Go code](https://blog.golang.org/godoc) - [ ] updated the relevant documentation or specification - [ ] reviewed "Files changed" and left comments if necessary - [ ] confirmed all CI checks have passed ### Reviewers Checklist *All items are required. Please add a note if the item is not applicable and please add your handle next to the items reviewed if you only reviewed selected items.* I have... - [ ] confirmed the correct [type prefix](https://github.com/commitizen/conventional-commit-types/blob/v3.0.0/index.json) in the PR title - [ ] confirmed `!` in the type prefix if API or client breaking change - [ ] confirmed all author checklist items have been addressed - [ ] reviewed state machine logic - [ ] reviewed API design and naming - [ ] reviewed documentation is accurate - [ ] reviewed tests and test coverage - [ ] manually tested (if applicable) --- CHANGELOG.md | 1 + docs/core/upgrade.md | 84 +++++++++++++++--------------- simapp/app.go | 3 ++ types/errors/errors.go | 6 +-- types/module/module.go | 90 ++++++++++++++++++++++++--------- types/module/module_int_test.go | 57 +++++++++++++++++++++ 6 files changed, 174 insertions(+), 67 deletions(-) create mode 100644 types/module/module_int_test.go diff --git a/CHANGELOG.md b/CHANGELOG.md index ee19e9cc4..c92685869 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -58,6 +58,7 @@ Ref: https://keepachangelog.com/en/1.0.0/ * [\#10507](https://github.com/cosmos/cosmos-sdk/pull/10507) Add middleware for tx priority. * [\#10311](https://github.com/cosmos/cosmos-sdk/pull/10311) Adds cli to use tips transactions. It adds an `--aux` flag to all CLI tx commands to generate the aux signer data (with optional tip), and a new `tx aux-to-fee` subcommand to let the fee payer gather aux signer data and broadcast the tx * [\#10430](https://github.com/cosmos/cosmos-sdk/pull/10430) ADR-040: Add store/v2 `MultiStore` implementation +* [\#10614](https://github.com/cosmos/cosmos-sdk/pull/10614) Support in-place migration ordering ### API Breaking Changes diff --git a/docs/core/upgrade.md b/docs/core/upgrade.md index 2a3844161..2782785e7 100644 --- a/docs/core/upgrade.md +++ b/docs/core/upgrade.md @@ -22,27 +22,13 @@ This document provides steps to use the In-Place Store Migrations upgrade method Each module gets assigned a consensus version by the module developer. The consensus version serves as the breaking change version of the module. The Cosmos SDK keeps track of all module consensus versions in the x/upgrade `VersionMap` store. During an upgrade, the difference between the old `VersionMap` stored in state and the new `VersionMap` is calculated by the Cosmos SDK. For each identified difference, the module-specific migrations are run and the respective consensus version of each upgraded module is incremented. -## Genesis State - -When starting a new chain, the consensus version of each module must be saved to state during the application's genesis. To save the consensus version, add the following line to the `InitChainer` method in `app.go`: - -```diff -func (app *MyApp) InitChainer(ctx sdk.Context, req abci.RequestInitChain) abci.ResponseInitChain { - ... -+ app.UpgradeKeeper.SetModuleVersionMap(ctx, app.mm.GetVersionMap()) - ... -} -``` - -This information is used by the Cosmos SDK to detect when modules with newer versions are introduced to the app. - ### Consensus Version The consensus version is defined on each app module by the module developer and serves as the breaking change version of the module. The consensus version informs the Cosmos SDK on which modules need to be upgraded. For example, if the bank module was version 2 and an upgrade introduces bank module 3, the Cosmos SDK upgrades the bank module and runs the "version 2 to 3" migration script. ### Version Map -The version map is a mapping of module names to consensus versions. The map is persisted to x/upgrade's state for use during in-place migrations. When migrations finish, the updated version map is persisted to state. +The version map is a mapping of module names to consensus versions. The map is persisted to x/upgrade's state for use during in-place migrations. When migrations finish, the updated version map is persisted in the state. ## Upgrade Handlers @@ -60,24 +46,29 @@ Inside these functions, you must perform any upgrade logic to include in the pro ## Running Migrations -Migrations are run inside of an `UpgradeHandler` using `app.mm.RunMigrations(ctx, cfg, vm)`. The `UpgradeHandler` functions describe the functionality to occur during an upgrade. The `RunMigration` function loops through the `VersionMap` argument and runs the migration scripts for all versions that are less than the versions of the new binary app module. After the migrations are finished, a new `VersionMap` is returned to persist the upgraded module versions to state. +Migrations are run inside of an `UpgradeHandler` using `app.mm.RunMigrations(ctx, cfg, vm)`. The `UpgradeHandler` functions describe the functionality to occur during an upgrade. The `RunMigration` function loops through the `VersionMap` argument and runs the migration scripts for all versions that are less than the versions of the new binary app module. After the migrations are finished, a new `VersionMap` is returned to persist the upgraded module versions to state. ```go cfg := module.NewConfigurator(...) -app.UpgradeKeeper.SetUpgradeHandler("my-plan", func(ctx sdk.Context, plan upgradetypes.Plan, vm module.VersionMap) (module.VersionMap, error) { +app.UpgradeKeeper.SetUpgradeHandler("my-plan", func(ctx sdk.Context, plan upgradetypes.Plan, fromVM module.VersionMap) (module.VersionMap, error) { // ... - // do upgrade logic + // additional upgrade logic // ... - // RunMigrations returns the VersionMap - // with the updated module ConsensusVersions - return app.mm.RunMigrations(ctx, vm) + // returns a VersionMap with the updated module ConsensusVersions + return app.mm.RunMigrations(ctx, fromVM) }) ``` To learn more about configuring migration scripts for your modules, see the [Module Upgrade Guide](../building-modules/upgrade.md). +### Order Of Migrations + +By default, all migrations are run in module name alphabetical ascending order, except `x/auth` which is run last. The reason is state dependencies between x/auth and other modules (you can read more in [issue #10606](https://github.com/cosmos/cosmos-sdk/issues/10606)). + +If you want to change the order of migration then you should call `app.mm.SetOrderMigrations(module1, module2, ...)` in your app.go file. The function will panic if you forget to include a module in the argument list. + ## Adding New Modules During Upgrades You can introduce entirely new modules to the application during an upgrade. New modules are recognized because they have not yet been registered in `x/upgrade`'s `VersionMap` store. In this case, `RunMigrations` calls the `InitGenesis` function from the corresponding module to set up its initial state. @@ -105,7 +96,35 @@ if upgradeInfo.Name == "my-plan" && !app.UpgradeKeeper.IsSkipHeight(upgradeInfo. } ``` -## Overwriting Genesis Functions +## Genesis State + +When starting a new chain, the consensus version of each module MUST be saved to state during the application's genesis. To save the consensus version, add the following line to the `InitChainer` method in `app.go`: + +```diff +func (app *MyApp) InitChainer(ctx sdk.Context, req abci.RequestInitChain) abci.ResponseInitChain { + ... ++ app.UpgradeKeeper.SetModuleVersionMap(ctx, app.mm.GetVersionMap()) + ... +} +``` + +This information is used by the Cosmos SDK to detect when modules with newer versions are introduced to the app. + +For a new module `foo`, `InitGenesis` is called by `RunMigration` only when `foo` is registered in the module manager but it's not set in the `fromVM`. Therefore, if you want to skip `InitGenesis` when a new module is added to the app, then you should set its module version in `fromVM` to the module consensus version: + +```go +app.UpgradeKeeper.SetUpgradeHandler("my-plan", func(ctx sdk.Context, plan upgradetypes.Plan, fromVM module.VersionMap) (module.VersionMap, error) { + // ... + + // Set foo's version to the latest ConsensusVersion in the VersionMap. + // This will skip running InitGenesis on Foo + fromVM[foo.ModuleName] = foo.AppModule{}.ConsensusVersion() + + return app.mm.RunMigrations(ctx, fromVM) +}) +``` + +### Overwriting Genesis Functions The Cosmos SDK offers modules that the application developer can import in their app. These modules often have an `InitGenesis` function already defined. @@ -118,32 +137,17 @@ You MUST manually set the consensus version in the version map passed to the `Up ```go import foo "github.com/my/module/foo" -app.UpgradeKeeper.SetUpgradeHandler("my-plan", func(ctx sdk.Context, plan upgradetypes.Plan, vm module.VersionMap) (module.VersionMap, error) { +app.UpgradeKeeper.SetUpgradeHandler("my-plan", func(ctx sdk.Context, plan upgradetypes.Plan, fromVM module.VersionMap) (module.VersionMap, error) { // Register the consensus version in the version map // to avoid the SDK from triggering the default // InitGenesis function. - vm["foo"] = foo.AppModule{}.ConsensusVersion() + fromVM["foo"] = foo.AppModule{}.ConsensusVersion() // Run custom InitGenesis for foo app.mm["foo"].InitGenesis(ctx, app.appCodec, myCustomGenesisState) - return app.mm.RunMigrations(ctx, cfg, vm) -}) -``` - -If you do not have a custom genesis function and want to skip the module's default genesis function, you can simply register the module with the version map in the `UpgradeHandler` as shown in the example: - -```go -import foo "github.com/my/module/foo" - -app.UpgradeKeeper.SetUpgradeHandler("my-plan", func(ctx sdk.Context, plan upgradetypes.Plan, vm module.VersionMap) (module.VersionMap, error) { - - // Set foo's version to the latest ConsensusVersion in the VersionMap. - // This will skip running InitGenesis on Foo - vm["foo"] = foo.AppModule{}.ConsensusVersion() - - return app.mm.RunMigrations(ctx, cfg, vm) + return app.mm.RunMigrations(ctx, cfg, fromVM) }) ``` diff --git a/simapp/app.go b/simapp/app.go index af060d7b5..edc6a6333 100644 --- a/simapp/app.go +++ b/simapp/app.go @@ -389,6 +389,9 @@ func NewSimApp( group.ModuleName, ) + // Uncomment if you want to set a custom migration order here. + // app.mm.SetOrderMigrations(custom order) + app.mm.RegisterInvariants(&app.CrisisKeeper) app.mm.RegisterRoutes(app.legacyRouter, app.QueryRouter(), encodingConfig.Amino) app.configurator = module.NewConfigurator(app.appCodec, app.msgSvcRouter, app.GRPCQueryRouter()) diff --git a/types/errors/errors.go b/types/errors/errors.go index 2aa53c1de..8f98a1910 100644 --- a/types/errors/errors.go +++ b/types/errors/errors.go @@ -160,10 +160,10 @@ var ( // Examples: not DB domain error, file writing etc... ErrIO = Register(RootCodespace, 39, "Internal IO error") + // ErrAppConfig defines an error occurred if min-gas-prices field in BaseConfig is empty. + ErrAppConfig = Register(RootCodespace, 40, "error in app.toml") + // ErrPanic is only set when we recover from a panic, so we know to // redact potentially sensitive system info ErrPanic = errorsmod.ErrPanic - - // ErrAppConfig defines an error occurred if min-gas-prices field in BaseConfig is empty. - ErrAppConfig = Register(RootCodespace, 40, "error in app.toml") ) diff --git a/types/module/module.go b/types/module/module.go index 2c4754d5c..4128d781a 100644 --- a/types/module/module.go +++ b/types/module/module.go @@ -230,6 +230,7 @@ type Manager struct { OrderExportGenesis []string OrderBeginBlockers []string OrderEndBlockers []string + OrderMigrations []string } // NewManager creates a new Manager object @@ -253,28 +254,35 @@ func NewManager(modules ...AppModule) *Manager { // SetOrderInitGenesis sets the order of init genesis calls func (m *Manager) SetOrderInitGenesis(moduleNames ...string) { - m.checkForgottenModules("SetOrderInitGenesis", moduleNames) + m.assertNoForgottenModules("SetOrderInitGenesis", moduleNames) m.OrderInitGenesis = moduleNames } // SetOrderExportGenesis sets the order of export genesis calls func (m *Manager) SetOrderExportGenesis(moduleNames ...string) { - m.checkForgottenModules("SetOrderExportGenesis", moduleNames) + m.assertNoForgottenModules("SetOrderExportGenesis", moduleNames) m.OrderExportGenesis = moduleNames } // SetOrderBeginBlockers sets the order of set begin-blocker calls func (m *Manager) SetOrderBeginBlockers(moduleNames ...string) { - m.checkForgottenModules("SetOrderBeginBlockers", moduleNames) + m.assertNoForgottenModules("SetOrderBeginBlockers", moduleNames) m.OrderBeginBlockers = moduleNames } // SetOrderEndBlockers sets the order of set end-blocker calls func (m *Manager) SetOrderEndBlockers(moduleNames ...string) { - m.checkForgottenModules("SetOrderEndBlockers", moduleNames) + m.assertNoForgottenModules("SetOrderEndBlockers", moduleNames) m.OrderEndBlockers = moduleNames } +// SetOrderMigrations sets the order of migrations to be run. If not set +// then migrations will be run with an order defined in `DefaultMigrationsOrder`. +func (m *Manager) SetOrderMigrations(moduleNames ...string) { + m.assertNoForgottenModules("SetOrderMigrations", moduleNames) + m.OrderMigrations = moduleNames +} + // RegisterInvariants registers all module invariants func (m *Manager) RegisterInvariants(ir sdk.InvariantRegistry) { for _, module := range m.Modules { @@ -345,24 +353,29 @@ func (m *Manager) ExportGenesis(ctx sdk.Context, cdc codec.JSONCodec) map[string return genesisData } -// checkForgottenModules checks that we didn't forget any modules in the +// assertNoForgottenModules checks that we didn't forget any modules in the // SetOrder* functions. -func (m *Manager) checkForgottenModules(setOrderFnName string, moduleNames []string) { - setOrderMap := map[string]struct{}{} +func (m *Manager) assertNoForgottenModules(setOrderFnName string, moduleNames []string) { + ms := make(map[string]bool) for _, m := range moduleNames { - setOrderMap[m] = struct{}{} + ms[m] = true } - - if len(setOrderMap) != len(m.Modules) { - panic(fmt.Sprintf("got %d modules in the module manager, but %d modules in %s", len(m.Modules), len(setOrderMap), setOrderFnName)) + var missing []string + for m := range m.Modules { + if !ms[m] { + missing = append(missing, m) + } + } + if len(missing) != 0 { + panic(fmt.Sprintf( + "%s: all modules must be defined when setting SetOrderMigrations, missing: %v", setOrderFnName, missing)) } } // MigrationHandler is the migration function that each module registers. type MigrationHandler func(sdk.Context) error -// VersionMap is a map of moduleName -> version, where version denotes the -// version from which we should perform the migration for each module. +// VersionMap is a map of moduleName -> version type VersionMap map[string]uint64 // RunMigrations performs in-place store migrations for all modules. This @@ -389,6 +402,9 @@ type VersionMap map[string]uint64 // `InitGenesis` on that module. // - return the `updatedVM` to be persisted in the x/upgrade's store. // +// Migrations are run in an order defined by `Manager.OrderMigrations` or (if not set) defined by +// `DefaultMigrationsOrder` function. +// // As an app developer, if you wish to skip running InitGenesis for your new // module "foo", you need to manually pass a `fromVM` argument to this function // foo's module version set to its latest ConsensusVersion. That way, the diff @@ -415,19 +431,13 @@ func (m Manager) RunMigrations(ctx sdk.Context, cfg Configurator, fromVM Version if !ok { return nil, sdkerrors.Wrapf(sdkerrors.ErrInvalidType, "expected %T, got %T", configurator{}, cfg) } - - updatedVM := make(VersionMap) - // for deterministic iteration order - // (as some migrations depend on other modules - // and the order of executing migrations matters) - // TODO: make the order user-configurable? - sortedModNames := make([]string, 0, len(m.Modules)) - for key := range m.Modules { - sortedModNames = append(sortedModNames, key) + var modules = m.OrderMigrations + if modules == nil { + modules = DefaultMigrationsOrder(m.ModuleNames()) } - sort.Strings(sortedModNames) - for _, moduleName := range sortedModNames { + updatedVM := VersionMap{} + for _, moduleName := range modules { module := m.Modules[moduleName] fromVersion, exists := fromVM[moduleName] toVersion := module.ConsensusVersion() @@ -514,3 +524,35 @@ func (m *Manager) GetVersionMap() VersionMap { return vermap } + +// ModuleNames returns list of all module names, without any particular order. +func (m *Manager) ModuleNames() []string { + ms := make([]string, len(m.Modules)) + i := 0 + for m := range m.Modules { + ms[i] = m + i++ + } + return ms +} + +// DefaultMigrationsOrder returns a default migrations order: ascending alphabetical by module name, +// except x/auth which will run last, see: +// https://github.com/cosmos/cosmos-sdk/issues/10591 +func DefaultMigrationsOrder(modules []string) []string { + const authName = "auth" + out := make([]string, 0, len(modules)) + hasAuth := false + for _, m := range modules { + if m == authName { + hasAuth = true + } else { + out = append(out, m) + } + } + sort.Strings(out) + if hasAuth { + out = append(out, authName) + } + return out +} diff --git a/types/module/module_int_test.go b/types/module/module_int_test.go new file mode 100644 index 000000000..3301a9926 --- /dev/null +++ b/types/module/module_int_test.go @@ -0,0 +1,57 @@ +package module + +import ( + "sort" + "testing" + + "github.com/stretchr/testify/suite" +) + +func TestModuleIntSuite(t *testing.T) { + suite.Run(t, new(TestSuite)) +} + +type TestSuite struct { + suite.Suite +} + +func (s TestSuite) TestAssertNoForgottenModules() { + m := Manager{ + Modules: map[string]AppModule{"a": nil, "b": nil}, + } + tcs := []struct { + name string + positive bool + modules []string + }{ + {"same modules", true, []string{"a", "b"}}, + {"more modules", true, []string{"a", "b", "c"}}, + } + + for _, tc := range tcs { + if tc.positive { + m.assertNoForgottenModules("x", tc.modules) + } else { + s.Panics(func() { m.assertNoForgottenModules("x", tc.modules) }) + } + } +} + +func (s TestSuite) TestModuleNames() { + m := Manager{ + Modules: map[string]AppModule{"a": nil, "b": nil}, + } + ms := m.ModuleNames() + sort.Strings(ms) + s.Require().Equal([]string{"a", "b"}, ms) +} + +func (s TestSuite) TestDefaultMigrationsOrder() { + require := s.Require() + require.Equal( + []string{"auth2", "d", "z", "auth"}, + DefaultMigrationsOrder([]string{"d", "auth", "auth2", "z"}), "alphabetical, but auth should be last") + require.Equal( + []string{"auth2", "d", "z"}, + DefaultMigrationsOrder([]string{"d", "auth2", "z"}), "alphabetical") +}