From 6699ce5e1a4015eb905e88f8588aac4f07d2149a Mon Sep 17 00:00:00 2001
From: Amaury M <1293565+amaurym@users.noreply.github.com>
Date: Fri, 26 Mar 2021 13:23:47 +0100
Subject: [PATCH 01/12] InitGenesis in migrations when fromVersion==0

---
 simapp/app.go                |  2 +-
 simapp/app_test.go           |  2 +-
 types/module/configurator.go | 14 +++++++++++++-
 types/module/module.go       | 18 ++++++++++++++++--
 types/module/module_test.go  |  4 +++-
 5 files changed, 34 insertions(+), 6 deletions(-)

diff --git a/simapp/app.go b/simapp/app.go
index e0c4316f30a0..9f1ce5ba2f3c 100644
--- a/simapp/app.go
+++ b/simapp/app.go
@@ -339,7 +339,7 @@ func NewSimApp(
 
 	app.mm.RegisterInvariants(&app.CrisisKeeper)
 	app.mm.RegisterRoutes(app.Router(), app.QueryRouter(), encodingConfig.Amino)
-	app.configurator = module.NewConfigurator(app.MsgServiceRouter(), app.GRPCQueryRouter())
+	app.configurator = module.NewConfigurator(app.appCodec, app.MsgServiceRouter(), app.GRPCQueryRouter())
 	app.mm.RegisterServices(app.configurator)
 
 	// add test gRPC service for testing gRPC queries in isolation
diff --git a/simapp/app_test.go b/simapp/app_test.go
index c4aa9885b5d8..c5d6562a9281 100644
--- a/simapp/app_test.go
+++ b/simapp/app_test.go
@@ -80,7 +80,7 @@ func TestRunMigrations(t *testing.T) {
 	bApp.SetCommitMultiStoreTracer(nil)
 	bApp.SetInterfaceRegistry(encCfg.InterfaceRegistry)
 	app.BaseApp = bApp
-	app.configurator = module.NewConfigurator(app.MsgServiceRouter(), app.GRPCQueryRouter())
+	app.configurator = module.NewConfigurator(app.appCodec, app.MsgServiceRouter(), app.GRPCQueryRouter())
 
 	// We register all modules on the Configurator, except x/bank. x/bank will
 	// serve as the test subject on which we run the migration tests.
diff --git a/types/module/configurator.go b/types/module/configurator.go
index 0e766df2a13d..22a9b31ad614 100644
--- a/types/module/configurator.go
+++ b/types/module/configurator.go
@@ -3,6 +3,7 @@ package module
 import (
 	"github.com/gogo/protobuf/grpc"
 
+	"github.com/cosmos/cosmos-sdk/codec"
 	sdk "github.com/cosmos/cosmos-sdk/types"
 	sdkerrors "github.com/cosmos/cosmos-sdk/types/errors"
 )
@@ -31,9 +32,14 @@ type Configurator interface {
 	// will panic. If the ConsensusVersion bump does not introduce any store
 	// changes, then a no-op function must be registered here.
 	RegisterMigration(moduleName string, forVersion uint64, handler MigrationHandler) error
+
+	// Cdc defines the app-wide codec interface used for serialization and
+	// deserialization.
+	Cdc() codec.Marshaler
 }
 
 type configurator struct {
+	cdc         codec.Marshaler
 	msgServer   grpc.Server
 	queryServer grpc.Server
 
@@ -42,8 +48,9 @@ type configurator struct {
 }
 
 // NewConfigurator returns a new Configurator instance
-func NewConfigurator(msgServer grpc.Server, queryServer grpc.Server) Configurator {
+func NewConfigurator(cdc codec.Marshaler, msgServer grpc.Server, queryServer grpc.Server) Configurator {
 	return configurator{
+		cdc:         cdc,
 		msgServer:   msgServer,
 		queryServer: queryServer,
 		migrations:  map[string]map[uint64]MigrationHandler{},
@@ -62,6 +69,11 @@ func (c configurator) QueryServer() grpc.Server {
 	return c.queryServer
 }
 
+// QueryServer implements the Configurator.Cdc method
+func (c configurator) Cdc() codec.Marshaler {
+	return c.cdc
+}
+
 // RegisterMigration implements the Configurator.RegisterMigration method
 func (c configurator) RegisterMigration(moduleName string, forVersion uint64, handler MigrationHandler) error {
 	if forVersion == 0 {
diff --git a/types/module/module.go b/types/module/module.go
index d123a05d192b..197ab589b8ec 100644
--- a/types/module/module.go
+++ b/types/module/module.go
@@ -349,13 +349,27 @@ func (m Manager) RunMigrations(ctx sdk.Context, cfg Configurator, fromVM Version
 		fromVersion := fromVM[moduleName]
 		toVersion := module.ConsensusVersion()
 
-		// only run migrations when the from version is > 0
-		// from version will be 0 when a new module is added and migrations shouldn't be run in this case
+		// Only run migrations when the fromVersion is > 0, or run InitGenesis
+		// if fromVersion == 0.
+		//
+		// fromVersion will be 0 in two cases:
+		// 1. If 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 {
 			err := c.runModuleMigrations(ctx, moduleName, fromVersion, toVersion)
 			if err != nil {
 				return nil, err
 			}
+		} else {
+			moduleValUpdates := module.InitGenesis(ctx, cfg.Cdc(), nil)
+			// The module manager assumes only one module will update the
+			// validator set, and that it will not be by a new module.
+			if len(moduleValUpdates) > 0 {
+				return nil, sdkerrors.Wrapf(sdkerrors.ErrLogic, "validator InitGenesis updates already set by a previous module")
+			}
 		}
 
 		updatedVM[moduleName] = toVersion
diff --git a/types/module/module_test.go b/types/module/module_test.go
index 630c57619245..9f7a21f5c50c 100644
--- a/types/module/module_test.go
+++ b/types/module/module_test.go
@@ -179,7 +179,9 @@ func TestManager_RegisterQueryServices(t *testing.T) {
 
 	msgRouter := mocks.NewMockServer(mockCtrl)
 	queryRouter := mocks.NewMockServer(mockCtrl)
-	cfg := module.NewConfigurator(msgRouter, queryRouter)
+	interfaceRegistry := types.NewInterfaceRegistry()
+	cdc := codec.NewProtoCodec(interfaceRegistry)
+	cfg := module.NewConfigurator(cdc, msgRouter, queryRouter)
 	mockAppModule1.EXPECT().RegisterServices(cfg).Times(1)
 	mockAppModule2.EXPECT().RegisterServices(cfg).Times(1)
 

From 8dab2b75f6a7228593d137a419f1da2a5008ddbe Mon Sep 17 00:00:00 2001
From: Amaury M <1293565+amaurym@users.noreply.github.com>
Date: Fri, 26 Mar 2021 13:57:14 +0100
Subject: [PATCH 02/12] Add test

---
 simapp/app_test.go     | 49 ++++++++++++++++++++++++++++++++++++++++++
 types/module/module.go |  2 +-
 2 files changed, 50 insertions(+), 1 deletion(-)

diff --git a/simapp/app_test.go b/simapp/app_test.go
index c5d6562a9281..4f261853d57d 100644
--- a/simapp/app_test.go
+++ b/simapp/app_test.go
@@ -5,6 +5,7 @@ import (
 	"os"
 	"testing"
 
+	"github.com/golang/mock/gomock"
 	"github.com/stretchr/testify/require"
 	abci "github.com/tendermint/tendermint/abci/types"
 	"github.com/tendermint/tendermint/libs/log"
@@ -12,11 +13,13 @@ import (
 	dbm "github.com/tendermint/tm-db"
 
 	"github.com/cosmos/cosmos-sdk/baseapp"
+	"github.com/cosmos/cosmos-sdk/tests/mocks"
 	sdk "github.com/cosmos/cosmos-sdk/types"
 	"github.com/cosmos/cosmos-sdk/types/module"
 	"github.com/cosmos/cosmos-sdk/x/auth"
 	"github.com/cosmos/cosmos-sdk/x/auth/vesting"
 	"github.com/cosmos/cosmos-sdk/x/authz"
+	"github.com/cosmos/cosmos-sdk/x/bank"
 	banktypes "github.com/cosmos/cosmos-sdk/x/bank/types"
 	"github.com/cosmos/cosmos-sdk/x/capability"
 	"github.com/cosmos/cosmos-sdk/x/crisis"
@@ -185,12 +188,58 @@ func TestRunMigrations(t *testing.T) {
 				require.EqualError(t, err, tc.expRunErrMsg)
 			} else {
 				require.NoError(t, err)
+				// Make sure bank's migration is called.
 				require.Equal(t, tc.expCalled, called)
 			}
 		})
 	}
 }
 
+func TestInitGenesisOnMigration(t *testing.T) {
+	db := dbm.NewMemDB()
+	encCfg := MakeTestEncodingConfig()
+	logger := log.NewTMLogger(log.NewSyncWriter(os.Stdout))
+	app := NewSimApp(logger, db, nil, true, map[int64]bool{}, DefaultNodeHome, 0, encCfg, EmptyAppOptions{})
+
+	// Create a mock module. This module will serve as the new module we're
+	// adding during a migration.
+	mockCtrl := gomock.NewController(t)
+	t.Cleanup(mockCtrl.Finish)
+	mockModule := mocks.NewMockAppModuleGenesis(mockCtrl)
+	goam := module.NewGenesisOnlyAppModule(mockModule)
+
+	app.mm.Modules["mock"] = goam
+
+	// 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.
+	_, err := app.RunMigrations(
+		app.NewContext(true, tmproto.Header{Height: app.LastBlockHeight()}),
+		module.VersionMap{
+			"mock":         0,
+			"bank":         bank.AppModule{}.ConsensusVersion(),
+			"auth":         auth.AppModule{}.ConsensusVersion(),
+			"authz":        authz.AppModule{}.ConsensusVersion(),
+			"staking":      staking.AppModule{}.ConsensusVersion(),
+			"mint":         mint.AppModule{}.ConsensusVersion(),
+			"distribution": distribution.AppModule{}.ConsensusVersion(),
+			"slashing":     slashing.AppModule{}.ConsensusVersion(),
+			"gov":          gov.AppModule{}.ConsensusVersion(),
+			"params":       params.AppModule{}.ConsensusVersion(),
+			"upgrade":      upgrade.AppModule{}.ConsensusVersion(),
+			"vesting":      vesting.AppModule{}.ConsensusVersion(),
+			"feegrant":     feegrant.AppModule{}.ConsensusVersion(),
+			"evidence":     evidence.AppModule{}.ConsensusVersion(),
+			"crisis":       crisis.AppModule{}.ConsensusVersion(),
+			"genutil":      genutil.AppModule{}.ConsensusVersion(),
+			"capability":   capability.AppModule{}.ConsensusVersion(),
+		},
+	)
+	require.NoError(t, err)
+
+	// TODO FInd a way to test that InitGenesis has been called.
+}
+
 func TestUpgradeStateOnGenesis(t *testing.T) {
 	encCfg := MakeTestEncodingConfig()
 	db := dbm.NewMemDB()
diff --git a/types/module/module.go b/types/module/module.go
index 197ab589b8ec..2bc9677ffc07 100644
--- a/types/module/module.go
+++ b/types/module/module.go
@@ -364,7 +364,7 @@ func (m Manager) RunMigrations(ctx sdk.Context, cfg Configurator, fromVM Version
 				return nil, err
 			}
 		} else {
-			moduleValUpdates := module.InitGenesis(ctx, cfg.Cdc(), nil)
+			moduleValUpdates := module.InitGenesis(ctx, cfg.Cdc(), module.DefaultGenesis(cfg.Cdc()))
 			// The module manager assumes only one module will update the
 			// validator set, and that it will not be by a new module.
 			if len(moduleValUpdates) > 0 {

From 5850cae4d52b2496a204c0b3c32be8fee98362dd Mon Sep 17 00:00:00 2001
From: Amaury M <1293565+amaurym@users.noreply.github.com>
Date: Fri, 26 Mar 2021 14:19:49 +0100
Subject: [PATCH 03/12] Fix test

---
 simapp/app_test.go | 14 +++++++-------
 1 file changed, 7 insertions(+), 7 deletions(-)

diff --git a/simapp/app_test.go b/simapp/app_test.go
index 4f261853d57d..20f321beb12f 100644
--- a/simapp/app_test.go
+++ b/simapp/app_test.go
@@ -200,21 +200,23 @@ func TestInitGenesisOnMigration(t *testing.T) {
 	encCfg := MakeTestEncodingConfig()
 	logger := log.NewTMLogger(log.NewSyncWriter(os.Stdout))
 	app := NewSimApp(logger, db, nil, true, map[int64]bool{}, DefaultNodeHome, 0, encCfg, EmptyAppOptions{})
+	ctx := app.NewContext(true, tmproto.Header{Height: app.LastBlockHeight()})
 
 	// Create a mock module. This module will serve as the new module we're
 	// adding during a migration.
 	mockCtrl := gomock.NewController(t)
 	t.Cleanup(mockCtrl.Finish)
-	mockModule := mocks.NewMockAppModuleGenesis(mockCtrl)
-	goam := module.NewGenesisOnlyAppModule(mockModule)
+	mockModule := mocks.NewMockAppModule(mockCtrl)
+	mockDefaultGenesis := json.RawMessage(`{"key": "value"}`)
+	mockModule.EXPECT().DefaultGenesis(gomock.Eq(app.appCodec)).Times(1).Return(mockDefaultGenesis)
+	mockModule.EXPECT().InitGenesis(gomock.Eq(ctx), gomock.Eq(app.appCodec), gomock.Eq(mockDefaultGenesis)).Times(1).Return(nil)
 
-	app.mm.Modules["mock"] = goam
+	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.
-	_, err := app.RunMigrations(
-		app.NewContext(true, tmproto.Header{Height: app.LastBlockHeight()}),
+	_, err := app.RunMigrations(ctx,
 		module.VersionMap{
 			"mock":         0,
 			"bank":         bank.AppModule{}.ConsensusVersion(),
@@ -236,8 +238,6 @@ func TestInitGenesisOnMigration(t *testing.T) {
 		},
 	)
 	require.NoError(t, err)
-
-	// TODO FInd a way to test that InitGenesis has been called.
 }
 
 func TestUpgradeStateOnGenesis(t *testing.T) {

From 86e02a4c72701f435fc46ccec458739221a26408 Mon Sep 17 00:00:00 2001
From: Amaury M <1293565+amaurym@users.noreply.github.com>
Date: Fri, 26 Mar 2021 14:20:31 +0100
Subject: [PATCH 04/12] Fix comment

---
 types/module/configurator.go | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/types/module/configurator.go b/types/module/configurator.go
index 22a9b31ad614..0435c2d88070 100644
--- a/types/module/configurator.go
+++ b/types/module/configurator.go
@@ -69,7 +69,7 @@ func (c configurator) QueryServer() grpc.Server {
 	return c.queryServer
 }
 
-// QueryServer implements the Configurator.Cdc method
+// Cdc implements the Configurator.Cdc method
 func (c configurator) Cdc() codec.Marshaler {
 	return c.cdc
 }

From a0e1bc85cbb558d3aa505d4ea0d801156e1888ff Mon Sep 17 00:00:00 2001
From: Amaury M <1293565+amaurym@users.noreply.github.com>
Date: Fri, 26 Mar 2021 16:30:23 +0100
Subject: [PATCH 05/12] cdc=>codec

---
 types/module/configurator.go | 6 +++---
 types/module/module.go       | 2 +-
 2 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/types/module/configurator.go b/types/module/configurator.go
index 0435c2d88070..4fafa948a75e 100644
--- a/types/module/configurator.go
+++ b/types/module/configurator.go
@@ -33,9 +33,9 @@ type Configurator interface {
 	// changes, then a no-op function must be registered here.
 	RegisterMigration(moduleName string, forVersion uint64, handler MigrationHandler) error
 
-	// Cdc defines the app-wide codec interface used for serialization and
+	// Codec defines the app-wide codec interface used for serialization and
 	// deserialization.
-	Cdc() codec.Marshaler
+	Codec() codec.Marshaler
 }
 
 type configurator struct {
@@ -70,7 +70,7 @@ func (c configurator) QueryServer() grpc.Server {
 }
 
 // Cdc implements the Configurator.Cdc method
-func (c configurator) Cdc() codec.Marshaler {
+func (c configurator) Codec() codec.Marshaler {
 	return c.cdc
 }
 
diff --git a/types/module/module.go b/types/module/module.go
index 2bc9677ffc07..932d71de5fcd 100644
--- a/types/module/module.go
+++ b/types/module/module.go
@@ -364,7 +364,7 @@ func (m Manager) RunMigrations(ctx sdk.Context, cfg Configurator, fromVM Version
 				return nil, err
 			}
 		} else {
-			moduleValUpdates := module.InitGenesis(ctx, cfg.Cdc(), module.DefaultGenesis(cfg.Cdc()))
+			moduleValUpdates := module.InitGenesis(ctx, cfg.Codec(), module.DefaultGenesis(cfg.Codec()))
 			// The module manager assumes only one module will update the
 			// validator set, and that it will not be by a new module.
 			if len(moduleValUpdates) > 0 {

From 3fd6130e114ec90dee3e2964e5840c12f9707a13 Mon Sep 17 00:00:00 2001
From: Amaury M <1293565+amaurym@users.noreply.github.com>
Date: Wed, 31 Mar 2021 19:34:24 +0200
Subject: [PATCH 06/12] Don't break Configurator

---
 types/module/configurator.go | 6 +-----
 types/module/module.go       | 9 ++++++++-
 2 files changed, 9 insertions(+), 6 deletions(-)

diff --git a/types/module/configurator.go b/types/module/configurator.go
index 4fafa948a75e..97f7b711c6c6 100644
--- a/types/module/configurator.go
+++ b/types/module/configurator.go
@@ -32,10 +32,6 @@ type Configurator interface {
 	// will panic. If the ConsensusVersion bump does not introduce any store
 	// changes, then a no-op function must be registered here.
 	RegisterMigration(moduleName string, forVersion uint64, handler MigrationHandler) error
-
-	// Codec defines the app-wide codec interface used for serialization and
-	// deserialization.
-	Codec() codec.Marshaler
 }
 
 type configurator struct {
@@ -70,7 +66,7 @@ func (c configurator) QueryServer() grpc.Server {
 }
 
 // Cdc implements the Configurator.Cdc method
-func (c configurator) Codec() codec.Marshaler {
+func (c configurator) codec() codec.Marshaler {
 	return c.cdc
 }
 
diff --git a/types/module/module.go b/types/module/module.go
index 932d71de5fcd..6554a29aa6cd 100644
--- a/types/module/module.go
+++ b/types/module/module.go
@@ -364,7 +364,14 @@ func (m Manager) RunMigrations(ctx sdk.Context, cfg Configurator, fromVM Version
 				return nil, err
 			}
 		} else {
-			moduleValUpdates := module.InitGenesis(ctx, cfg.Codec(), module.DefaultGenesis(cfg.Codec()))
+			cfgtor, ok := cfg.(configurator)
+			if !ok {
+				// Currently, the only implementator of Configurator (the interface)
+				// is configurator (the struct).
+				return nil, sdkerrors.Wrapf(sdkerrors.ErrInvalidType, "expected %T, got %T", configurator{}, cfg)
+			}
+
+			moduleValUpdates := module.InitGenesis(ctx, cfgtor.cdc, module.DefaultGenesis(cfgtor.cdc))
 			// The module manager assumes only one module will update the
 			// validator set, and that it will not be by a new module.
 			if len(moduleValUpdates) > 0 {

From 735e553587166e49ff802dd6922b7cab334a2602 Mon Sep 17 00:00:00 2001
From: Amaury M <1293565+amaurym@users.noreply.github.com>
Date: Wed, 31 Mar 2021 19:35:15 +0200
Subject: [PATCH 07/12] Remove method

---
 types/module/configurator.go | 5 -----
 1 file changed, 5 deletions(-)

diff --git a/types/module/configurator.go b/types/module/configurator.go
index 97f7b711c6c6..f1e16f7e798a 100644
--- a/types/module/configurator.go
+++ b/types/module/configurator.go
@@ -65,11 +65,6 @@ func (c configurator) QueryServer() grpc.Server {
 	return c.queryServer
 }
 
-// Cdc implements the Configurator.Cdc method
-func (c configurator) codec() codec.Marshaler {
-	return c.cdc
-}
-
 // RegisterMigration implements the Configurator.RegisterMigration method
 func (c configurator) RegisterMigration(moduleName string, forVersion uint64, handler MigrationHandler) error {
 	if forVersion == 0 {

From c33f820ef806ea27b3db78750d5b558f630097e3 Mon Sep 17 00:00:00 2001
From: Amaury M <1293565+amaurym@users.noreply.github.com>
Date: Thu, 1 Apr 2021 11:33:18 +0200
Subject: [PATCH 08/12] Add comments

---
 simapp/app.go              | 16 ----------------
 simapp/app_test.go         |  6 +++---
 types/module/module.go     | 26 +++++++++++++++++++++++++-
 x/upgrade/types/handler.go | 15 +++++++++++++--
 4 files changed, 41 insertions(+), 22 deletions(-)

diff --git a/simapp/app.go b/simapp/app.go
index 9f1ce5ba2f3c..4c3805bc1bca 100644
--- a/simapp/app.go
+++ b/simapp/app.go
@@ -536,22 +536,6 @@ func (app *SimApp) RegisterTendermintService(clientCtx client.Context) {
 	tmservice.RegisterTendermintService(app.BaseApp.GRPCQueryRouter(), clientCtx, app.interfaceRegistry)
 }
 
-// RunMigrations performs in-place store migrations for all modules. This
-// function MUST be only called by x/upgrade UpgradeHandler.
-//
-// `migrateFromVersions` is a map of moduleName to fromVersion (unit64), where
-// fromVersion denotes the version from which we should migrate the module, the
-// target version being the module's latest ConsensusVersion.
-//
-// Example:
-//   cfg := module.NewConfigurator(...)
-//   app.UpgradeKeeper.SetUpgradeHandler("store-migration", func(ctx sdk.Context, plan upgradetypes.Plan, vm module.VersionMap) {
-//       return app.RunMigrations(ctx, vm)
-//   })
-func (app *SimApp) RunMigrations(ctx sdk.Context, migrateFromVersions module.VersionMap) (module.VersionMap, error) {
-	return app.mm.RunMigrations(ctx, app.configurator, migrateFromVersions)
-}
-
 // RegisterSwaggerAPI registers swagger route with API Server
 func RegisterSwaggerAPI(ctx client.Context, rtr *mux.Router) {
 	statikFS, err := fs.New()
diff --git a/simapp/app_test.go b/simapp/app_test.go
index 20f321beb12f..2f9c5e2c3d19 100644
--- a/simapp/app_test.go
+++ b/simapp/app_test.go
@@ -163,8 +163,8 @@ func TestRunMigrations(t *testing.T) {
 			// Run migrations only for bank. That's why we put the initial
 			// version for bank as 1, and for all other modules, we put as
 			// their latest ConsensusVersion.
-			_, err = app.RunMigrations(
-				app.NewContext(true, tmproto.Header{Height: app.LastBlockHeight()}),
+			_, err = app.mm.RunMigrations(
+				app.NewContext(true, tmproto.Header{Height: app.LastBlockHeight()}), app.configurator,
 				module.VersionMap{
 					"bank":         1,
 					"auth":         auth.AppModule{}.ConsensusVersion(),
@@ -216,7 +216,7 @@ func TestInitGenesisOnMigration(t *testing.T) {
 	// 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.
-	_, err := app.RunMigrations(ctx,
+	_, err := app.mm.RunMigrations(ctx, app.configurator,
 		module.VersionMap{
 			"mock":         0,
 			"bank":         bank.AppModule{}.ConsensusVersion(),
diff --git a/types/module/module.go b/types/module/module.go
index 6554a29aa6cd..2aa26013ccb8 100644
--- a/types/module/module.go
+++ b/types/module/module.go
@@ -337,7 +337,31 @@ type MigrationHandler func(sdk.Context) error
 // version from which we should perform the migration for each module.
 type VersionMap map[string]uint64
 
-// RunMigrations performs in-place store migrations for all modules.
+// RunMigrations performs in-place store migrations for all modules. This
+// function MUST be called insde an x/upgrade UpgradeHandler.
+//
+// Recall that in an upgrade handler, the `fromVM` VersionMap is retrieved from
+// x/upgrade's store, and the function needs to return to target VersionMap
+// that will in turn be persisted to the x/upgrade's store. In general,
+// returning RunMigrations should be enough:
+//
+// Example:
+//   cfg := module.NewConfigurator(...)
+//   app.UpgradeKeeper.SetUpgradeHandler("my-plan", func(ctx sdk.Context, plan upgradetypes.Plan, fromVM module.VersionMap) {
+//       return app.mm.RunMigrations(ctx, cfg, fromVM)
+//   })
+//
+// Internally, RunMigrations will perform the following steps:
+// - create an `updatedVM` VersionMap of module with their latest ConsensusVersion
+// - 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,
+//      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.
+//
+// Please also refer to docs/core/upgrade.md for more information.
 func (m Manager) RunMigrations(ctx sdk.Context, cfg Configurator, fromVM VersionMap) (VersionMap, error) {
 	c, ok := cfg.(configurator)
 	if !ok {
diff --git a/x/upgrade/types/handler.go b/x/upgrade/types/handler.go
index 0dccaefebde8..451ba8825b93 100644
--- a/x/upgrade/types/handler.go
+++ b/x/upgrade/types/handler.go
@@ -5,5 +5,16 @@ import (
 	"github.com/cosmos/cosmos-sdk/types/module"
 )
 
-// UpgradeHandler specifies the type of function that is called when an upgrade is applied
-type UpgradeHandler func(ctx sdk.Context, plan Plan, vm module.VersionMap) (module.VersionMap, error)
+// UpgradeHandler specifies the type of function that is called when an upgrade
+// is applied.
+//
+// `fromVM` is a VersionMap of moduleName to fromVersion (unit64), where
+// fromVersion denotes the version from which we should migrate the module, the
+// target version being the module's latest version in the return VersionMap,
+// let's call it `toVM`.
+//
+// fromVM is retrieved from x/upgrade's store, whereas `toVM` is chosen
+// arbitrarily by the app developer (and persisted to x/upgrade's store right
+// after the upgrade handler runs). In general, `toVM` should map module names
+// to their latest ConsensusVersion.
+type UpgradeHandler func(ctx sdk.Context, plan Plan, fromVM module.VersionMap) (module.VersionMap, error)

From 19995913096959477f08258266d72ebea20fcff2 Mon Sep 17 00:00:00 2001
From: Amaury M <1293565+amaurym@users.noreply.github.com>
Date: Fri, 2 Apr 2021 17:06:05 +0200
Subject: [PATCH 09/12] Add more comments

---
 types/module/module.go     | 20 ++++++++++++++++++++
 x/upgrade/types/handler.go | 12 +++++++++---
 2 files changed, 29 insertions(+), 3 deletions(-)

diff --git a/types/module/module.go b/types/module/module.go
index 2aa26013ccb8..a4b0b84d6893 100644
--- a/types/module/module.go
+++ b/types/module/module.go
@@ -361,6 +361,26 @@ type VersionMap map[string]uint64
 //      `InitGenesis` on that module.
 // - return the `updatedVM` to be persisted in the x/upgrade's store.
 //
+// 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
+// between the function's `fromVM` and `udpatedVM` will be empty, hence not
+// running anything for foo.
+//
+// Example:
+//   cfg := module.NewConfigurator(...)
+//   app.UpgradeKeeper.SetUpgradeHandler("my-plan", func(ctx sdk.Context, plan upgradetypes.Plan, fromVM module.VersionMap) {
+//       // 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
+//       // run InitGenesis on foo.
+//       // To skip running foo's InitGenesis, you need set `fromVM`'s foo to its latest
+//       // consensus version:
+//       fromVM["foo"] = foo.AppModule{}.ConsensusVersion()
+//
+//       return app.mm.RunMigrations(ctx, cfg, fromVM)
+//   })
+//
 // Please also refer to docs/core/upgrade.md for more information.
 func (m Manager) RunMigrations(ctx sdk.Context, cfg Configurator, fromVM VersionMap) (VersionMap, error) {
 	c, ok := cfg.(configurator)
diff --git a/x/upgrade/types/handler.go b/x/upgrade/types/handler.go
index 451ba8825b93..0543b6bbeb2d 100644
--- a/x/upgrade/types/handler.go
+++ b/x/upgrade/types/handler.go
@@ -13,8 +13,14 @@ import (
 // target version being the module's latest version in the return VersionMap,
 // let's call it `toVM`.
 //
-// fromVM is retrieved from x/upgrade's store, whereas `toVM` is chosen
+// `fromVM` is retrieved from x/upgrade's store, whereas `toVM` is chosen
 // arbitrarily by the app developer (and persisted to x/upgrade's store right
-// after the upgrade handler runs). In general, `toVM` should map module names
-// to their latest ConsensusVersion.
+// after the upgrade handler runs). In general, `toVM` should map all modules
+// to their latest ConsensusVersion so that x/upgrade can track each module's
+// latest ConsensusVersion; `fromVM` can be left as-is, but can also be
+// modified inside the upgrade handler, e.g. to skip running InitGenesis or
+// migrations for certain modules when calling the `module.Manager#RunMigrations`
+// function.
+//
+// Please also refer to docs/core/upgrade.md for more information.
 type UpgradeHandler func(ctx sdk.Context, plan Plan, fromVM module.VersionMap) (module.VersionMap, error)

From 16232c1e1b24540aae47f18531957fa3bce3032e Mon Sep 17 00:00:00 2001
From: Amaury <1293565+amaurym@users.noreply.github.com>
Date: Fri, 2 Apr 2021 17:23:57 +0200
Subject: [PATCH 10/12] Update types/module/module.go

Co-authored-by: technicallyty <48813565+technicallyty@users.noreply.github.com>
---
 types/module/module.go | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/types/module/module.go b/types/module/module.go
index a4b0b84d6893..d6f10831b7b5 100644
--- a/types/module/module.go
+++ b/types/module/module.go
@@ -341,7 +341,7 @@ type VersionMap map[string]uint64
 // function MUST be called insde an x/upgrade UpgradeHandler.
 //
 // Recall that in an upgrade handler, the `fromVM` VersionMap is retrieved from
-// x/upgrade's store, and the function needs to return to target VersionMap
+// x/upgrade's store, and the function needs to return the target VersionMap
 // that will in turn be persisted to the x/upgrade's store. In general,
 // returning RunMigrations should be enough:
 //

From c1e6b74ab34e70c6ba2ca59a382e38ad9eafb97d Mon Sep 17 00:00:00 2001
From: Amaury <1293565+amaurym@users.noreply.github.com>
Date: Fri, 2 Apr 2021 17:24:07 +0200
Subject: [PATCH 11/12] Update types/module/module.go

Co-authored-by: technicallyty <48813565+technicallyty@users.noreply.github.com>
---
 types/module/module.go | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/types/module/module.go b/types/module/module.go
index d6f10831b7b5..9ceaf4777684 100644
--- a/types/module/module.go
+++ b/types/module/module.go
@@ -347,7 +347,7 @@ type VersionMap map[string]uint64
 //
 // Example:
 //   cfg := module.NewConfigurator(...)
-//   app.UpgradeKeeper.SetUpgradeHandler("my-plan", func(ctx sdk.Context, plan upgradetypes.Plan, fromVM module.VersionMap) {
+//   app.UpgradeKeeper.SetUpgradeHandler("my-plan", func(ctx sdk.Context, plan upgradetypes.Plan, fromVM module.VersionMap) (module.VersionMap, error) {
 //       return app.mm.RunMigrations(ctx, cfg, fromVM)
 //   })
 //

From 625b89258e4463a18f770291434114751b8a2a7f Mon Sep 17 00:00:00 2001
From: Amaury <1293565+amaurym@users.noreply.github.com>
Date: Fri, 2 Apr 2021 17:24:17 +0200
Subject: [PATCH 12/12] Update types/module/module.go

Co-authored-by: technicallyty <48813565+technicallyty@users.noreply.github.com>
---
 types/module/module.go | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/types/module/module.go b/types/module/module.go
index 9ceaf4777684..df62ea8f0d05 100644
--- a/types/module/module.go
+++ b/types/module/module.go
@@ -369,7 +369,7 @@ type VersionMap map[string]uint64
 //
 // Example:
 //   cfg := module.NewConfigurator(...)
-//   app.UpgradeKeeper.SetUpgradeHandler("my-plan", func(ctx sdk.Context, plan upgradetypes.Plan, fromVM module.VersionMap) {
+//   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