Add in-place store migrations (cosmos#8485)

* Add 1st version of migrate

* Put migration logic into Configurator

* add test to bank store migration

* add test for configurator

* Error if no migration found

* Remove RunMigrations from Configurator interface

* Update spec

* Rename folders

* copy-paste from keys.go

* Fix nil map

* rename function

* Update simapp/app.go

Co-authored-by: Robert Zaremba <robert@zaremba.ch>

* Update simapp/app_test.go

Co-authored-by: Robert Zaremba <robert@zaremba.ch>

* Adderss reviews

* Fix tests

* Update testutil/context.go

Co-authored-by: Robert Zaremba <robert@zaremba.ch>

* Update docs for ConsensusVersion

* Rename to forVersion

* Fix tests

* Check error early

* Return 1 for intiial version

* Use MigrationKeeper

* Fix test

* Revert adding marshaler to Configurator

* Godoc updates

* Update docs

Co-authored-by: Robert Zaremba <robert@zaremba.ch>
Co-authored-by: Aaron Craelius <aaron@regen.network>

app.go

@@ -198,6 +198,9 @@ type SimApp struct {
 
 	// simulation manager
 	sm *module.SimulationManager
+
+	// the configurator
+	configurator module.Configurator
 }
 
 func init() {
@@ -393,7 +396,8 @@ func NewSimApp(
 
 	app.mm.RegisterInvariants(&app.CrisisKeeper)
 	app.mm.RegisterRoutes(app.Router(), app.QueryRouter(), encodingConfig.Amino)
-	app.mm.RegisterServices(module.NewConfigurator(app.MsgServiceRouter(), app.GRPCQueryRouter()))
+	app.configurator = module.NewConfigurator(app.MsgServiceRouter(), app.GRPCQueryRouter())
+	app.mm.RegisterServices(app.configurator)
 
 	// add test gRPC service for testing gRPC queries in isolation
 	testdata.RegisterQueryServer(app.GRPCQueryRouter(), testdata.QueryImpl{})
@@ -598,6 +602,28 @@ 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) {
+//       err := app.RunMigrations(ctx, module.MigrationMap{
+//           "bank": 1,     // Migrate x/bank from v1 to current x/bank's ConsensusVersion
+//           "staking": 8,  // Migrate x/staking from v8 to current x/staking's ConsensusVersion
+//      })
+//      if err != nil {
+//           panic(err)
+//      }
+//   })
+func (app *SimApp) RunMigrations(ctx sdk.Context, migrateFromVersions module.MigrationMap) 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()

app_test.go

@@ -6,10 +6,13 @@ import (
 	"testing"
 
 	"github.com/stretchr/testify/require"
+	abci "github.com/tendermint/tendermint/abci/types"
 	"github.com/tendermint/tendermint/libs/log"
+	tmproto "github.com/tendermint/tendermint/proto/tendermint/types"
 	dbm "github.com/tendermint/tm-db"
 
-	abci "github.com/tendermint/tendermint/abci/types"
+	sdk "github.com/cosmos/cosmos-sdk/types"
+	"github.com/cosmos/cosmos-sdk/types/module"
 )
 
 func TestSimAppExportAndBlockedAddrs(t *testing.T) {
@@ -45,3 +48,87 @@ func TestGetMaccPerms(t *testing.T) {
 	dup := GetMaccPerms()
 	require.Equal(t, maccPerms, dup, "duplicated module account permissions differed from actual module account permissions")
 }
+
+func TestRunMigrations(t *testing.T) {
+	db := dbm.NewMemDB()
+	encCfg := MakeTestEncodingConfig()
+	app := NewSimApp(log.NewTMLogger(log.NewSyncWriter(os.Stdout)), db, nil, true, map[int64]bool{}, DefaultNodeHome, 0, encCfg, EmptyAppOptions{})
+
+	// Create a new configurator for the purpose of this test.
+	app.configurator = module.NewConfigurator(app.MsgServiceRouter(), app.GRPCQueryRouter())
+
+	testCases := []struct {
+		name         string
+		moduleName   string
+		forVersion   uint64
+		expRegErr    bool // errors while registering migration
+		expRegErrMsg string
+		expRunErr    bool // errors while running migration
+		expRunErrMsg string
+		expCalled    int
+	}{
+		{
+			"cannot register migration for version 0",
+			"bank", 0,
+			true, "module migration versions should start at 1: invalid version", false, "", 0,
+		},
+		{
+			"throws error on RunMigrations if no migration registered for bank",
+			"", 1,
+			false, "", true, "no migrations found for module bank: not found", 0,
+		},
+		{
+			"can register and run migration handler for x/bank",
+			"bank", 1,
+			false, "", false, "", 1,
+		},
+		{
+			"cannot register migration handler for same module & forVersion",
+			"bank", 1,
+			true, "another migration for module bank and version 1 already exists: internal logic error", false, "", 0,
+		},
+	}
+
+	for _, tc := range testCases {
+		t.Run(tc.name, func(t *testing.T) {
+			var err error
+
+			// Since it's very hard to test actual in-place store migrations in
+			// tests (due to the difficulty of maintaing multiple versions of a
+			// module), we're just testing here that the migration logic is
+			// called.
+			called := 0
+
+			if tc.moduleName != "" {
+				// Register migration for module from version `forVersion` to `forVersion+1`.
+				err = app.configurator.RegisterMigration(tc.moduleName, tc.forVersion, func(sdk.Context) error {
+					called++
+
+					return nil
+				})
+
+				if tc.expRegErr {
+					require.EqualError(t, err, tc.expRegErrMsg)
+
+					return
+				}
+			}
+			require.NoError(t, err)
+
+			err = app.RunMigrations(
+				app.NewContext(true, tmproto.Header{Height: app.LastBlockHeight()}),
+				module.MigrationMap{
+					"auth": 1, "authz": 1, "bank": 1, "staking": 1, "mint": 1, "distribution": 1,
+					"slashing": 1, "gov": 1, "params": 1, "ibc": 1, "upgrade": 1, "vesting": 1,
+					"feegrant": 1, "transfer": 1, "evidence": 1, "crisis": 1, "genutil": 1, "capability": 1,
+				},
+			)
+			if tc.expRunErr {
+				require.EqualError(t, err, tc.expRunErrMsg)
+			} else {
+				require.NoError(t, err)
+				require.Equal(t, tc.expCalled, called)
+			}
+		})
+	}
+}