feat: improve genesis migration command (#15679)

## Description

Closes: #5041
ref: cosmos/gaia#1950 (comment)

---

### 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...

* [ ] 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/main/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/main/docs/docs/building-modules)
* [ ] included the necessary unit and integration [tests](https://github.com/cosmos/cosmos-sdk/blob/main/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

@@ -40,6 +40,7 @@ Ref: https://keepachangelog.com/en/1.0.0/
 
 ### Features
 
+* (x/genutil) [#15679](https://github.com/cosmos/cosmos-sdk/pull/15679) Allow applications to specify a custom genesis migration function for the `genesis migrate` command.
 * (client) [#15458](https://github.com/cosmos/cosmos-sdk/pull/15458) Add a `CmdContext` field to client.Context initialized to cobra command's context.
 * (core) [#15133](https://github.com/cosmos/cosmos-sdk/pull/15133) Implement RegisterServices in the module manager.
 * (x/gov) [#14373](https://github.com/cosmos/cosmos-sdk/pull/14373) Add new proto field `constitution` of type `string` to gov module genesis state, which allows chain builders to lay a strong foundation by specifying purpose.
@@ -99,6 +100,7 @@ Ref: https://keepachangelog.com/en/1.0.0/
 
 ### API Breaking Changes
 
+* (x/genutil) [#15679](https://github.com/cosmos/cosmos-sdk/pull/15679) `MigrateGenesisCmd` now takes a `MigrationMap` instead of having the SDK genesis migration hardcoded.
 * (client) [#15673](https://github.com/cosmos/cosmos-sdk/pull/15673) Move `client/keys.OutputFormatJSON` and `client/keys.OutputFormatText` to `client/flags` package.
 * (x/nft) [#15588](https://github.com/cosmos/cosmos-sdk/pull/15588) `NewKeeper` now takes a `KVStoreService` instead of a `StoreKey` and methods in the `Keeper` now take a `context.Context` instead of a `sdk.Context`. 
 * (x/auth) [#15520](https://github.com/cosmos/cosmos-sdk/pull/15520) `NewAccountKeeper` now takes a `KVStoreService` instead of a `StoreKey` and methods in the `Keeper` now take a `context.Context` instead of a `sdk.Context`. 

simapp/simd/cmd/root.go

@@ -219,7 +219,7 @@ func addModuleInitFlags(startCmd *cobra.Command) {
 
 // genesisCommand builds genesis-related `simd genesis` command. Users may provide application specific commands as a parameter
 func genesisCommand(encodingConfig params.EncodingConfig, cmds ...*cobra.Command) *cobra.Command {
-	cmd := genutilcli.GenesisCoreCommand(encodingConfig.TxConfig, simapp.ModuleBasics, simapp.DefaultNodeHome)
+	cmd := genutilcli.Commands(encodingConfig.TxConfig, simapp.ModuleBasics, simapp.DefaultNodeHome)
 
 	for _, subCmd := range cmds {
 		cmd.AddCommand(subCmd)

x/genutil/README.md

@@ -51,10 +51,19 @@ Migrate genesis to a specified target (SDK) version.
 simd genesis migrate [target-version]
 ```
 
+:::tip
+The `migrate` command is extensible and takes a `MigrationMap`. This map is a mapping of target versions to genesis migrations functions.
+When not using the default `MigrationMap`, it is recommended to still call the default `MigrationMap` corresponding the SDK version of the chain and prepend/append your own genesis migrations.
+:::
+
 #### validate-genesis
 
 Validates the genesis file at the default location or at the location passed as an argument.
 
 ```shell
 simd genesis validate-genesis
 ```
+
+:::warning
+Validate genesis only validates if the genesis is valid at the **current application binary**. For validating a genesis from a previous version of the application, use the `migrate` command to migrate the genesis to the current version.
+:::

x/genutil/client/cli/core_genesis_cmd.go → x/genutil/client/cli/commands.go

@@ -10,9 +10,20 @@ import (
 	"github.com/spf13/cobra"
 )
 
-// GenesisCoreCommand adds core sdk's sub-commands into genesis command:
-// -> gentx, migrate, collect-gentxs, validate-genesis, add-genesis-account
+// GenesisCoreCommand adds core sdk's sub-commands into genesis command.
+// Deprecated: use Commands instead.
 func GenesisCoreCommand(txConfig client.TxConfig, moduleBasics module.BasicManager, defaultNodeHome string) *cobra.Command {
+	return Commands(txConfig, moduleBasics, defaultNodeHome)
+}
+
+// Commands adds core sdk's sub-commands into genesis command.
+func Commands(txConfig client.TxConfig, moduleBasics module.BasicManager, defaultNodeHome string) *cobra.Command {
+	return CommandsWithCustomMigrationMap(txConfig, moduleBasics, defaultNodeHome, MigrationMap)
+}
+
+// CommandsWithCustomMigrationMap adds core sdk's sub-commands into genesis command with custom migration map.
+// This custom migration map can be used by the application to add its own migration map.
+func CommandsWithCustomMigrationMap(txConfig client.TxConfig, moduleBasics module.BasicManager, defaultNodeHome string, migrationMap genutiltypes.MigrationMap) *cobra.Command {
 	cmd := &cobra.Command{
 		Use:                        "genesis",
 		Short:                      "Application's genesis-related subcommands",
@@ -23,11 +34,9 @@ func GenesisCoreCommand(txConfig client.TxConfig, moduleBasics module.BasicManag
 	gentxModule := moduleBasics[genutiltypes.ModuleName].(genutil.AppModuleBasic)
 
 	cmd.AddCommand(
-		GenTxCmd(moduleBasics, txConfig,
-			banktypes.GenesisBalancesIterator{}, defaultNodeHome),
-		MigrateGenesisCmd(),
-		CollectGenTxsCmd(banktypes.GenesisBalancesIterator{}, defaultNodeHome,
-			gentxModule.GenTxValidator),
+		GenTxCmd(moduleBasics, txConfig, banktypes.GenesisBalancesIterator{}, defaultNodeHome),
+		MigrateGenesisCmd(migrationMap),
+		CollectGenTxsCmd(banktypes.GenesisBalancesIterator{}, defaultNodeHome, gentxModule.GenTxValidator),
 		ValidateGenesisCmd(moduleBasics),
 		AddGenesisAccountCmd(defaultNodeHome),
 	)

x/genutil/client/cli/migrate.go

@@ -4,6 +4,7 @@ import (
 	"encoding/json"
 	"fmt"
 	"sort"
+	"strings"
 	"time"
 
 	"github.com/spf13/cobra"
@@ -20,47 +21,35 @@ import (
 
 const flagGenesisTime = "genesis-time"
 
-// Allow applications to extend and modify the migration process.
-//
-// Ref: https://github.com/cosmos/cosmos-sdk/issues/5041
-var migrationMap = types.MigrationMap{
+// MigrationMap is a map of SDK versions to their respective genesis migration functions.
+var MigrationMap = types.MigrationMap{
 	"v0.43": v043.Migrate, // NOTE: v0.43, v0.44 and v0.45 are genesis compatible.
 	"v0.46": v046.Migrate,
 	"v0.47": v047.Migrate,
 }
 
-// GetMigrationCallback returns a MigrationCallback for a given version.
-func GetMigrationCallback(version string) types.MigrationCallback {
-	return migrationMap[version]
-}
-
-// GetMigrationVersions get all migration version in a sorted slice.
-func GetMigrationVersions() []string {
-	versions := maps.Keys(migrationMap)
-	sort.Strings(versions)
-
-	return versions
-}
-
 // MigrateGenesisCmd returns a command to execute genesis state migration.
-func MigrateGenesisCmd() *cobra.Command {
+// Applications should pass their own migration map to this function.
+// When the application migration includes a SDK migration, the Cosmos SDK migration function should as well be called.
+func MigrateGenesisCmd(migrations types.MigrationMap) *cobra.Command {
 	cmd := &cobra.Command{
-		Use:   "migrate [target-version] [genesis-file]",
-		Short: "Migrate genesis to a specified target version",
-		Long: fmt.Sprintf(`Migrate the source genesis into the target version and print to STDOUT.
-
-Example:
-$ %s migrate v0.36 /path/to/genesis.json --chain-id=cosmoshub-3 --genesis-time=2019-04-22T17:00:00Z
-`, version.AppName),
-		Args: cobra.ExactArgs(2),
+		Use:     "migrate [target-version] [genesis-file]",
+		Short:   "Migrate genesis to a specified target version",
+		Long:    "Migrate the source genesis into the target version and print to STDOUT",
+		Example: fmt.Sprintf("%s migrate v0.47 /path/to/genesis.json --chain-id=cosmoshub-3 --genesis-time=2019-04-22T17:00:00Z", version.AppName),
+		Args:    cobra.ExactArgs(2),
 		RunE: func(cmd *cobra.Command, args []string) error {
 			clientCtx := client.GetClientContextFromCmd(cmd)
 
-			var err error
-
 			target := args[0]
-			importGenesis := args[1]
+			migrationFunc, ok := migrations[target]
+			if !ok || migrationFunc == nil {
+				versions := maps.Keys(migrations)
+				sort.Strings(versions)
+				return fmt.Errorf("unknown migration function for version: %s (supported versions %s)", target, strings.Join(versions, ", "))
+			}
 
+			importGenesis := args[1]
 			appGenesis, err := types.AppGenesisFromFile(importGenesis)
 			if err != nil {
 				return err
@@ -83,14 +72,11 @@ $ %s migrate v0.36 /path/to/genesis.json --chain-id=cosmoshub-3 --genesis-time=2
 				return fmt.Errorf("failed to JSON unmarshal initial genesis state: %w", err)
 			}
 
-			migrationFunc := GetMigrationCallback(target)
-			if migrationFunc == nil {
-				return fmt.Errorf("unknown migration function for version: %s", target)
+			newGenState, err := migrationFunc(initialState, clientCtx)
+			if err != nil {
+				return fmt.Errorf("failed to migrate genesis state: %w", err)
 			}
 
-			// TODO: handler error from migrationFunc call
-			newGenState := migrationFunc(initialState, clientCtx)
-
 			appGenesis.AppState, err = json.Marshal(newGenState)
 			if err != nil {
 				return fmt.Errorf("failed to JSON marshal migrated genesis state: %w", err)

x/genutil/client/cli/migrate_test.go

@@ -9,15 +9,10 @@ import (
 	"github.com/cosmos/cosmos-sdk/client"
 	"github.com/cosmos/cosmos-sdk/testutil"
 	clitestutil "github.com/cosmos/cosmos-sdk/testutil/cli"
+	moduletestutil "github.com/cosmos/cosmos-sdk/types/module/testutil"
 	"github.com/cosmos/cosmos-sdk/x/genutil/client/cli"
 )
 
-func TestGetMigrationCallback(t *testing.T) {
-	for _, version := range cli.GetMigrationVersions() {
-		require.NotNil(t, cli.GetMigrationCallback(version))
-	}
-}
-
 func TestMigrateGenesis(t *testing.T) {
 	testCases := []struct {
 		name      string
@@ -28,9 +23,9 @@ func TestMigrateGenesis(t *testing.T) {
 		check     func(jsonOut string)
 	}{
 		{
-			"migrate 0.37 to 0.42",
+			"migrate 0.37 to 0.43",
 			v037Exported,
-			"v0.42",
+			"v0.43",
 			true, "make sure that you have correctly migrated all CometBFT consensus params", func(_ string) {},
 		},
 		{
@@ -42,7 +37,7 @@ func TestMigrateGenesis(t *testing.T) {
 				return string(bz)
 			}(),
 			"v0.10",
-			true, "unknown migration function for version: v0.10", func(_ string) {},
+			true, "unknown migration function for version: v0.10 (supported versions v0.43, v0.46, v0.47)", func(_ string) {},
 		},
 		{
 			"invalid target version",
@@ -53,15 +48,20 @@ func TestMigrateGenesis(t *testing.T) {
 				return string(bz)
 			}(),
 			"v0.10",
-			true, "unknown migration function for version: v0.10", func(_ string) {},
+			true, "unknown migration function for version: v0.10 (supported versions v0.43, v0.46, v0.47)", func(_ string) {},
 		},
 	}
 
 	for _, tc := range testCases {
 		tc := tc
 		t.Run(tc.name, func(t *testing.T) {
 			genesisFile := testutil.WriteToNewTempFile(t, tc.genesis)
-			jsonOutput, err := clitestutil.ExecTestCLICmd(client.Context{}, cli.MigrateGenesisCmd(), []string{tc.target, genesisFile.Name()})
+			jsonOutput, err := clitestutil.ExecTestCLICmd(
+				// the codec does not contain any modules so that genutil does not bring unnecessary dependencies in the test
+				client.Context{Codec: moduletestutil.MakeTestEncodingConfig().Codec},
+				cli.MigrateGenesisCmd(cli.MigrationMap),
+				[]string{tc.target, genesisFile.Name()},
+			)
 			if tc.expErr {
 				require.Contains(t, err.Error(), tc.expErrMsg)
 			} else {

x/genutil/client/cli/validate_genesis.go

@@ -53,7 +53,7 @@ func ValidateGenesisCmd(mbm module.BasicManager) *cobra.Command {
 				return fmt.Errorf("error validating genesis file %s: %s", genesis, err.Error())
 			}
 
-			fmt.Printf("File at %s is a valid genesis file\n", genesis)
+			fmt.Fprintf(cmd.OutOrStdout(), "File at %s is a valid genesis file\n", genesis)
 			return nil
 		},
 	}

x/genutil/migrations/v043/migrate.go

@@ -12,7 +12,7 @@ import (
 )
 
 // Migrate migrates exported state from v0.40 to a v0.43 genesis state.
-func Migrate(appState types.AppMap, clientCtx client.Context) types.AppMap {
+func Migrate(appState types.AppMap, clientCtx client.Context) (types.AppMap, error) {
 	// Migrate x/gov.
 	if appState[v1gov.ModuleName] != nil {
 		// unmarshal relative source genesis application state
@@ -40,5 +40,5 @@ func Migrate(appState types.AppMap, clientCtx client.Context) types.AppMap {
 		appState[v2bank.ModuleName] = clientCtx.Codec.MustMarshalJSON(v2bank.MigrateJSON(&oldBankState))
 	}
 
-	return appState
+	return appState, nil
 }

x/genutil/migrations/v046/migrate.go

@@ -12,7 +12,7 @@ import (
 )
 
 // Migrate migrates exported state from v0.43 to a v0.46 genesis state.
-func Migrate(appState types.AppMap, clientCtx client.Context) types.AppMap {
+func Migrate(appState types.AppMap, clientCtx client.Context) (types.AppMap, error) {
 	// Migrate x/gov.
 	if appState[v2gov.ModuleName] != nil {
 		// unmarshal relative source genesis application state
@@ -26,7 +26,7 @@ func Migrate(appState types.AppMap, clientCtx client.Context) types.AppMap {
 		// the respective key.
 		new, err := v3gov.MigrateJSON(&old)
 		if err != nil {
-			panic(err)
+			return nil, err
 		}
 		appState[v3gov.ModuleName] = clientCtx.Codec.MustMarshalJSON(new)
 	}
@@ -44,10 +44,10 @@ func Migrate(appState types.AppMap, clientCtx client.Context) types.AppMap {
 		// the respective key.
 		new, err := stakingv3.MigrateJSON(old)
 		if err != nil {
-			panic(err)
+			return nil, err
 		}
 		appState[stakingv3.ModuleName] = clientCtx.Codec.MustMarshalJSON(&new)
 	}
 
-	return appState
+	return appState, nil
 }

x/genutil/migrations/v047/migrate.go

@@ -16,7 +16,7 @@ import (
 )
 
 // Migrate migrates exported state from v0.46 to a v0.47 genesis state.
-func Migrate(appState types.AppMap, clientCtx client.Context) types.AppMap {
+func Migrate(appState types.AppMap, clientCtx client.Context) (types.AppMap, error) {
 	// Migrate x/bank.
 	bankState := appState[banktypes.ModuleName]
 	if len(bankState) > 0 {
@@ -37,7 +37,7 @@ func Migrate(appState types.AppMap, clientCtx client.Context) types.AppMap {
 		// set the x/gov genesis state with new state.
 		new, err := v4gov.MigrateJSON(&old)
 		if err != nil {
-			panic(err)
+			return nil, err
 		}
 		appState[v4gov.ModuleName] = clientCtx.Codec.MustMarshalJSON(new)
 	}
@@ -58,5 +58,5 @@ func Migrate(appState types.AppMap, clientCtx client.Context) types.AppMap {
 		appState[v1distr.ModuleName] = clientCtx.Codec.MustMarshalJSON(newDistState)
 	}
 
-	return appState
+	return appState, nil
 }

x/genutil/types/types.go

@@ -13,9 +13,7 @@ type (
 
 	// MigrationCallback converts a genesis map from the previous version to the
 	// targeted one.
-	//
-	// TODO: MigrationCallback should also return an error upon failure.
-	MigrationCallback func(AppMap, client.Context) AppMap
+	MigrationCallback func(AppMap, client.Context) (AppMap, error)
 
 	// MigrationMap defines a mapping from a version to a MigrationCallback.
 	MigrationMap map[string]MigrationCallback