From d0d1f5c056a66ba2ca39bbcf130972593cb49c66 Mon Sep 17 00:00:00 2001 From: Julien Robert Date: Tue, 4 Apr 2023 19:31:42 +0200 Subject: [PATCH] feat: improve genesis migration command (#15679) ## Description Closes: https://github.com/cosmos/cosmos-sdk/issues/5041 ref: https://github.com/cosmos/gaia/issues/1950#issuecomment-1400097306 --- ### 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 | 2 + simapp/simd/cmd/root.go | 2 +- x/genutil/README.md | 9 +++ .../cli/{core_genesis_cmd.go => commands.go} | 23 +++++--- x/genutil/client/cli/migrate.go | 56 +++++++------------ x/genutil/client/cli/migrate_test.go | 22 ++++---- x/genutil/client/cli/validate_genesis.go | 2 +- x/genutil/migrations/v043/migrate.go | 4 +- x/genutil/migrations/v046/migrate.go | 8 +-- x/genutil/migrations/v047/migrate.go | 6 +- x/genutil/types/types.go | 4 +- 11 files changed, 71 insertions(+), 67 deletions(-) rename x/genutil/client/cli/{core_genesis_cmd.go => commands.go} (52%) diff --git a/CHANGELOG.md b/CHANGELOG.md index 716314bba60d..3710cb8e778f 100644 --- a/CHANGELOG.md +++ b/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`. diff --git a/simapp/simd/cmd/root.go b/simapp/simd/cmd/root.go index cbd64162fbb0..23e4707be0c5 100644 --- a/simapp/simd/cmd/root.go +++ b/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) diff --git a/x/genutil/README.md b/x/genutil/README.md index 90db9f494212..c534b8b0d7ac 100644 --- a/x/genutil/README.md +++ b/x/genutil/README.md @@ -51,6 +51,11 @@ 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. @@ -58,3 +63,7 @@ Validates the genesis file at the default location or at the location passed as ```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. +::: diff --git a/x/genutil/client/cli/core_genesis_cmd.go b/x/genutil/client/cli/commands.go similarity index 52% rename from x/genutil/client/cli/core_genesis_cmd.go rename to x/genutil/client/cli/commands.go index 384ddc9c782c..22d69dc6d17b 100644 --- a/x/genutil/client/cli/core_genesis_cmd.go +++ b/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), ) diff --git a/x/genutil/client/cli/migrate.go b/x/genutil/client/cli/migrate.go index 6ab54b8e9689..56c8d3638335 100644 --- a/x/genutil/client/cli/migrate.go +++ b/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) diff --git a/x/genutil/client/cli/migrate_test.go b/x/genutil/client/cli/migrate_test.go index 9a569a8b6ede..ab89ed5429fc 100644 --- a/x/genutil/client/cli/migrate_test.go +++ b/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,7 +48,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) {}, }, } @@ -61,7 +56,12 @@ func TestMigrateGenesis(t *testing.T) { 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 { diff --git a/x/genutil/client/cli/validate_genesis.go b/x/genutil/client/cli/validate_genesis.go index a565a37f368b..f3a57320ac16 100644 --- a/x/genutil/client/cli/validate_genesis.go +++ b/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 }, } diff --git a/x/genutil/migrations/v043/migrate.go b/x/genutil/migrations/v043/migrate.go index bbb78e5f0769..93ed56989472 100644 --- a/x/genutil/migrations/v043/migrate.go +++ b/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 } diff --git a/x/genutil/migrations/v046/migrate.go b/x/genutil/migrations/v046/migrate.go index c13a24746627..b6d6774feb41 100644 --- a/x/genutil/migrations/v046/migrate.go +++ b/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 } diff --git a/x/genutil/migrations/v047/migrate.go b/x/genutil/migrations/v047/migrate.go index 0e3512257aae..359bb1e0439d 100644 --- a/x/genutil/migrations/v047/migrate.go +++ b/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 } diff --git a/x/genutil/types/types.go b/x/genutil/types/types.go index 89f63554869a..e0995f978377 100644 --- a/x/genutil/types/types.go +++ b/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