From ed29d26cc0bb0d8f8f48192a8ba9af78b94f270e Mon Sep 17 00:00:00 2001 From: technicallyty <48813565+tytech3@users.noreply.github.com> Date: Mon, 12 Apr 2021 09:33:10 -0700 Subject: [PATCH 1/3] update docs/code to utilize map access return values --- types/module/module.go | 21 ++++++++++----------- 1 file changed, 10 insertions(+), 11 deletions(-) diff --git a/types/module/module.go b/types/module/module.go index df62ea8f0d05..418795efb983 100644 --- a/types/module/module.go +++ b/types/module/module.go @@ -356,7 +356,7 @@ type VersionMap map[string]uint64 // - make a diff of `fromVM` and `udpatedVM`, and for each module: // - if the module's `fromVM` version is less than its `updatedVM` version, // then run in-place store migrations for that module between those versions. -// - if the module's `fromVM` is 0 (which means that it's a new module, +// - if the module does not exist in the `fromVM` (which means that it's a new module, // because it was not in the previous x/upgrade's store), then run // `InitGenesis` on that module. // - return the `updatedVM` to be persisted in the x/upgrade's store. @@ -372,7 +372,7 @@ type VersionMap map[string]uint64 // app.UpgradeKeeper.SetUpgradeHandler("my-plan", func(ctx sdk.Context, plan upgradetypes.Plan, fromVM module.VersionMap) (module.VersionMap, error) { // // Assume "foo" is a new module. // // `fromVM` is fetched from existing x/upgrade store. Since foo didn't exist -// // before this upgrade, `fromVM["foo"] == 0`, and RunMigration will by default +// // before this upgrade, `v, exists := fromVM["foo"]; exists == false`, and RunMigration will by default // // run InitGenesis on foo. // // To skip running foo's InitGenesis, you need set `fromVM`'s foo to its latest // // consensus version: @@ -390,19 +390,18 @@ func (m Manager) RunMigrations(ctx sdk.Context, cfg Configurator, fromVM Version updatedVM := make(VersionMap) for moduleName, module := range m.Modules { - fromVersion := fromVM[moduleName] + fromVersion, exists := fromVM[moduleName] toVersion := module.ConsensusVersion() - // Only run migrations when the fromVersion is > 0, or run InitGenesis - // if fromVersion == 0. + // Only run migrations when the module exists in the fromVM. + // Run InitGenesis otherwise. // - // fromVersion will be 0 in two cases: - // 1. If a new module is added. In this case we run InitGenesis with an + // the module won't exist in the fromVM in two cases: + // 1. A new module is added. In this case we run InitGenesis with an // empty genesis state. - // 2. If the app developer is running in-place store migrations for the - // first time. In this case, it is the app developer's responsibility - // to set their module's fromVersions to a version that suits them. - if fromVersion > 0 { + // 2. An existing chain is upgrading to v043 for the first time. In this case, + // all modules have yet to be added to x/upgrade's VersionMap store. + if exists { err := c.runModuleMigrations(ctx, moduleName, fromVersion, toVersion) if err != nil { return nil, err From 294f67b50379e5c90728d555e59104982c3f1cce Mon Sep 17 00:00:00 2001 From: technicallyty <48813565+tytech3@users.noreply.github.com> Date: Mon, 12 Apr 2021 10:06:36 -0700 Subject: [PATCH 2/3] remove mock module in fromVM to simulate truly non-existent module --- simapp/app_test.go | 1 - 1 file changed, 1 deletion(-) diff --git a/simapp/app_test.go b/simapp/app_test.go index 2f9c5e2c3d19..4773297bdef5 100644 --- a/simapp/app_test.go +++ b/simapp/app_test.go @@ -218,7 +218,6 @@ func TestInitGenesisOnMigration(t *testing.T) { // modules, we put their latest ConsensusVersion to skip migrations. _, err := app.mm.RunMigrations(ctx, app.configurator, module.VersionMap{ - "mock": 0, "bank": bank.AppModule{}.ConsensusVersion(), "auth": auth.AppModule{}.ConsensusVersion(), "authz": authz.AppModule{}.ConsensusVersion(), From 381705bdbc84a4daf9c50c56e41b5bbb552af7f9 Mon Sep 17 00:00:00 2001 From: technicallyty <48813565+tytech3@users.noreply.github.com> Date: Mon, 12 Apr 2021 10:08:51 -0700 Subject: [PATCH 3/3] update test docs to refelect change --- simapp/app_test.go | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/simapp/app_test.go b/simapp/app_test.go index 4773297bdef5..bfab92c05477 100644 --- a/simapp/app_test.go +++ b/simapp/app_test.go @@ -213,9 +213,8 @@ func TestInitGenesisOnMigration(t *testing.T) { app.mm.Modules["mock"] = mockModule - // Run migrations only for "mock" module. That's why we put the initial - // version for bank as 0 (to run its InitGenesis), and for all other - // modules, we put their latest ConsensusVersion to skip migrations. + // Run migrations only for "mock" module. We exclude it from + // the VersionMap to simulate upgrading with a new module. _, err := app.mm.RunMigrations(ctx, app.configurator, module.VersionMap{ "bank": bank.AppModule{}.ConsensusVersion(),