From 3ebf06ce0d9d04a1d94a769583fd0b58d2c63ec0 Mon Sep 17 00:00:00 2001
From: technicallyty <48813565+technicallyty@users.noreply.github.com>
Date: Tue, 13 Apr 2021 12:00:04 -0700
Subject: [PATCH] module/types code hygiene - utilize map access 2nd return
 value (#9098)

* update docs/code to utilize map access return values

* remove mock module in fromVM to simulate truly non-existent module

* update test docs to refelect change

Co-authored-by: technicallyty <48813565+tytech3@users.noreply.github.com>
Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com>
---
 simapp/app_test.go     |  6 ++----
 types/module/module.go | 21 ++++++++++-----------
 2 files changed, 12 insertions(+), 15 deletions(-)

diff --git a/simapp/app_test.go b/simapp/app_test.go
index 2f9c5e2c3d..bfab92c054 100644
--- a/simapp/app_test.go
+++ b/simapp/app_test.go
@@ -213,12 +213,10 @@ 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{
-			"mock":         0,
 			"bank":         bank.AppModule{}.ConsensusVersion(),
 			"auth":         auth.AppModule{}.ConsensusVersion(),
 			"authz":        authz.AppModule{}.ConsensusVersion(),
diff --git a/types/module/module.go b/types/module/module.go
index df62ea8f0d..418795efb9 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