From efc8aa34296233c5a9fc7fe1326ce8d80dcc2224 Mon Sep 17 00:00:00 2001 From: mmsqe Date: Fri, 13 Oct 2023 12:11:02 +0800 Subject: [PATCH 1/6] fix(x/bank): miss keypair of SendEnabled to restore legacy param set before migration this change was introduced in https://github.com/cosmos/cosmos-sdk/pull/11977/files, but accidently get removed in https://github.com/cosmos/cosmos-sdk/pull/12630/files, since bank send_enabled refactor before params migration --- CHANGELOG.md | 1 + tests/integration/bank/keeper/keeper_test.go | 2 ++ x/bank/exported/exported.go | 1 + x/bank/keeper/keeper_test.go | 2 ++ x/bank/keeper/migrations.go | 4 +++ x/bank/migrations/v4/store.go | 3 ++ x/bank/migrations/v4/store_test.go | 2 ++ x/bank/types/params_legacy.go | 38 +++++++++++++++++++- 8 files changed, 52 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 8eca4b873b9c..3bd9c4f94e6f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -48,6 +48,7 @@ Ref: https://keepachangelog.com/en/1.0.0/ * (config) [#17649](https://github.com/cosmos/cosmos-sdk/pull/17649) Fix `mempool.max-txs` configuration is invalid in `app.config`. * (mempool) [#17668](https://github.com/cosmos/cosmos-sdk/pull/17668) Fix `PriorityNonceIterator.Next()` nil pointer ref for min priority at the end of iteration. * (x/auth) [#17902](https://github.com/cosmos/cosmos-sdk/pull/17902) Remove tip posthandler. +* (x/bank) [#18107](https://github.com/cosmos/cosmos-sdk/pull/18107) Add missing keypair of SendEnabled to restore legacy param set before migration. ### Client Breaking Changes diff --git a/tests/integration/bank/keeper/keeper_test.go b/tests/integration/bank/keeper/keeper_test.go index 7b37a19be0b2..493103ef3ea2 100644 --- a/tests/integration/bank/keeper/keeper_test.go +++ b/tests/integration/bank/keeper/keeper_test.go @@ -1642,6 +1642,8 @@ func (ms mockSubspace) GetParamSet(ctx sdk.Context, ps exported.ParamSet) { *ps.(*types.Params) = ms.ps } +func (ms mockSubspace) Get(ctx sdk.Context, key []byte, ptr interface{}) {} + func (suite *IntegrationTestSuite) TestMigrator_Migrate3to4() { ctx, bankKeeper := suite.ctx, suite.bankKeeper diff --git a/x/bank/exported/exported.go b/x/bank/exported/exported.go index 7ab9136e896d..9a518025968c 100644 --- a/x/bank/exported/exported.go +++ b/x/bank/exported/exported.go @@ -21,5 +21,6 @@ type ( // NOTE: This is used solely for migration of x/params managed parameters. Subspace interface { GetParamSet(ctx sdk.Context, ps ParamSet) + Get(ctx sdk.Context, key []byte, ptr interface{}) } ) diff --git a/x/bank/keeper/keeper_test.go b/x/bank/keeper/keeper_test.go index 9cb428322b82..cd1daabe086c 100644 --- a/x/bank/keeper/keeper_test.go +++ b/x/bank/keeper/keeper_test.go @@ -1697,6 +1697,8 @@ func (ms mockSubspace) GetParamSet(ctx sdk.Context, ps exported.ParamSet) { *ps.(*banktypes.Params) = ms.ps } +func (ms mockSubspace) Get(ctx sdk.Context, key []byte, ptr interface{}) {} + func (suite *KeeperTestSuite) TestMigrator_Migrate3to4() { ctx, bankKeeper := suite.ctx, suite.bankKeeper require := suite.Require() diff --git a/x/bank/keeper/migrations.go b/x/bank/keeper/migrations.go index 2e2d0ee7ad7a..32880532c7f4 100644 --- a/x/bank/keeper/migrations.go +++ b/x/bank/keeper/migrations.go @@ -6,6 +6,7 @@ import ( v2 "github.com/cosmos/cosmos-sdk/x/bank/migrations/v2" v3 "github.com/cosmos/cosmos-sdk/x/bank/migrations/v3" v4 "github.com/cosmos/cosmos-sdk/x/bank/migrations/v4" + "github.com/cosmos/cosmos-sdk/x/bank/types" ) // Migrator is a struct for handling in-place store migrations. @@ -31,5 +32,8 @@ func (m Migrator) Migrate2to3(ctx sdk.Context) error { // Migrate3to4 migrates x/bank storage from version 3 to 4. func (m Migrator) Migrate3to4(ctx sdk.Context) error { + var sendEnabled []*types.SendEnabled + m.legacySubspace.Get(ctx, types.KeySendEnabled, &sendEnabled) + m.keeper.SetAllSendEnabled(ctx, sendEnabled) return v4.MigrateStore(ctx, m.keeper.storeKey, m.legacySubspace, m.keeper.cdc) } diff --git a/x/bank/migrations/v4/store.go b/x/bank/migrations/v4/store.go index 03c5b02f43bc..2a0482838fa0 100644 --- a/x/bank/migrations/v4/store.go +++ b/x/bank/migrations/v4/store.go @@ -21,6 +21,9 @@ func MigrateStore(ctx sdk.Context, storeKey storetypes.StoreKey, legacySubspace var currParams types.Params legacySubspace.GetParamSet(ctx, &currParams) + // it's migrated to the x/bank module store, so delete from the params + currParams.SendEnabled = nil + if err := currParams.Validate(); err != nil { return err } diff --git a/x/bank/migrations/v4/store_test.go b/x/bank/migrations/v4/store_test.go index b9d2c035c25b..537d2a39c854 100644 --- a/x/bank/migrations/v4/store_test.go +++ b/x/bank/migrations/v4/store_test.go @@ -25,6 +25,8 @@ func (ms mockSubspace) GetParamSet(ctx sdk.Context, ps exported.ParamSet) { *ps.(*types.Params) = ms.ps } +func (ms mockSubspace) Get(ctx sdk.Context, key []byte, ptr interface{}) {} + func TestMigrate(t *testing.T) { encCfg := moduletestutil.MakeTestEncodingConfig(bank.AppModuleBasic{}) cdc := encCfg.Codec diff --git a/x/bank/types/params_legacy.go b/x/bank/types/params_legacy.go index 668358b4df7a..d88d15218938 100644 --- a/x/bank/types/params_legacy.go +++ b/x/bank/types/params_legacy.go @@ -1,6 +1,11 @@ package types -import paramtypes "github.com/cosmos/cosmos-sdk/x/params/types" +import ( + fmt "fmt" + + sdk "github.com/cosmos/cosmos-sdk/types" + paramtypes "github.com/cosmos/cosmos-sdk/x/params/types" +) var ( // KeySendEnabled is store's key for SendEnabled Params @@ -18,6 +23,37 @@ func ParamKeyTable() paramtypes.KeyTable { // Deprecated: ParamSetPairs implements params.ParamSet func (p *Params) ParamSetPairs() paramtypes.ParamSetPairs { return paramtypes.ParamSetPairs{ + paramtypes.NewParamSetPair(KeySendEnabled, &p.SendEnabled, validateSendEnabledParams), paramtypes.NewParamSetPair(KeyDefaultSendEnabled, &p.DefaultSendEnabled, validateIsBool), } } + +// SendEnabledParams is a collection of parameters indicating if a coin denom is enabled for sending +type SendEnabledParams []*SendEnabled + +func validateSendEnabledParams(i interface{}) error { + params, ok := i.([]*SendEnabled) + if !ok { + return fmt.Errorf("invalid parameter type: %T", i) + } + // ensure each denom is only registered one time. + registered := make(map[string]bool) + for _, p := range params { + if _, exists := registered[p.Denom]; exists { + return fmt.Errorf("duplicate send enabled parameter found: '%s'", p.Denom) + } + if err := validateSendEnabled(*p); err != nil { + return err + } + registered[p.Denom] = true + } + return nil +} + +func validateSendEnabled(i interface{}) error { + param, ok := i.(SendEnabled) + if !ok { + return fmt.Errorf("invalid parameter type: %T", i) + } + return sdk.ValidateDenom(param.Denom) +} From c886f4f579b173893385de98f057d5ff4e1a3348 Mon Sep 17 00:00:00 2001 From: mmsqe Date: Fri, 13 Oct 2023 18:35:37 +0800 Subject: [PATCH 2/6] clone without SendEnabled and avoid touch deprecated --- x/bank/migrations/v4/store.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/x/bank/migrations/v4/store.go b/x/bank/migrations/v4/store.go index 2a0482838fa0..9d8eb41289f3 100644 --- a/x/bank/migrations/v4/store.go +++ b/x/bank/migrations/v4/store.go @@ -21,8 +21,8 @@ func MigrateStore(ctx sdk.Context, storeKey storetypes.StoreKey, legacySubspace var currParams types.Params legacySubspace.GetParamSet(ctx, &currParams) - // it's migrated to the x/bank module store, so delete from the params - currParams.SendEnabled = nil + // SendEnabled is migrated to the x/bank module store, so delete from the params + currParams = types.NewParams(currParams.DefaultSendEnabled) if err := currParams.Validate(); err != nil { return err From f9e0c5926eeef2f01045e7d1b89ba7929a110e6d Mon Sep 17 00:00:00 2001 From: mmsqe Date: Fri, 13 Oct 2023 18:36:58 +0800 Subject: [PATCH 3/6] Apply suggestions from code review --- x/bank/keeper/migrations.go | 3 +-- x/bank/types/params_legacy.go | 8 ++++++++ 2 files changed, 9 insertions(+), 2 deletions(-) diff --git a/x/bank/keeper/migrations.go b/x/bank/keeper/migrations.go index 32880532c7f4..8feda69a2a50 100644 --- a/x/bank/keeper/migrations.go +++ b/x/bank/keeper/migrations.go @@ -32,8 +32,7 @@ func (m Migrator) Migrate2to3(ctx sdk.Context) error { // Migrate3to4 migrates x/bank storage from version 3 to 4. func (m Migrator) Migrate3to4(ctx sdk.Context) error { - var sendEnabled []*types.SendEnabled - m.legacySubspace.Get(ctx, types.KeySendEnabled, &sendEnabled) + sendEnabled := types.GetSendEnabledParams(ctx, m.legacySubspace) m.keeper.SetAllSendEnabled(ctx, sendEnabled) return v4.MigrateStore(ctx, m.keeper.storeKey, m.legacySubspace, m.keeper.cdc) } diff --git a/x/bank/types/params_legacy.go b/x/bank/types/params_legacy.go index d88d15218938..6f6a25a06999 100644 --- a/x/bank/types/params_legacy.go +++ b/x/bank/types/params_legacy.go @@ -4,6 +4,7 @@ import ( fmt "fmt" sdk "github.com/cosmos/cosmos-sdk/types" + "github.com/cosmos/cosmos-sdk/x/bank/exported" paramtypes "github.com/cosmos/cosmos-sdk/x/params/types" ) @@ -31,6 +32,13 @@ func (p *Params) ParamSetPairs() paramtypes.ParamSetPairs { // SendEnabledParams is a collection of parameters indicating if a coin denom is enabled for sending type SendEnabledParams []*SendEnabled +// GetSendEnabledParams retrieves the send enabled parameters from the provided context and legacy subspace. +func GetSendEnabledParams(ctx sdk.Context, legacySubspace exported.Subspace) []*SendEnabled { + var sendEnabled []*SendEnabled + legacySubspace.Get(ctx, KeySendEnabled, &sendEnabled) + return sendEnabled +} + func validateSendEnabledParams(i interface{}) error { params, ok := i.([]*SendEnabled) if !ok { From 63ae12440cadb0e8888e6eeda413c9bf73b81e94 Mon Sep 17 00:00:00 2001 From: mmsqe Date: Fri, 13 Oct 2023 21:20:08 +0800 Subject: [PATCH 4/6] Apply suggestions from code review --- x/bank/keeper/migrations.go | 6 ++++++ x/bank/module.go | 8 +++++++- 2 files changed, 13 insertions(+), 1 deletion(-) diff --git a/x/bank/keeper/migrations.go b/x/bank/keeper/migrations.go index 8feda69a2a50..9ab026250c97 100644 --- a/x/bank/keeper/migrations.go +++ b/x/bank/keeper/migrations.go @@ -32,6 +32,12 @@ func (m Migrator) Migrate2to3(ctx sdk.Context) error { // Migrate3to4 migrates x/bank storage from version 3 to 4. func (m Migrator) Migrate3to4(ctx sdk.Context) error { + return v4.MigrateStore(ctx, m.keeper.storeKey, m.legacySubspace, m.keeper.cdc) +} + +// Migrate3to4WithSendEnabledParams migrates x/bank storage from version 3 to 4 with +// the send enabled params get from x/params and update the bank params. +func (m Migrator) Migrate3to4WithSendEnabledParams(ctx sdk.Context) error { sendEnabled := types.GetSendEnabledParams(ctx, m.legacySubspace) m.keeper.SetAllSendEnabled(ctx, sendEnabled) return v4.MigrateStore(ctx, m.keeper.storeKey, m.legacySubspace, m.keeper.cdc) diff --git a/x/bank/module.go b/x/bank/module.go index 45ff753f2545..e8c7cdbf2932 100644 --- a/x/bank/module.go +++ b/x/bank/module.go @@ -127,7 +127,7 @@ func (am AppModule) RegisterServices(cfg module.Configurator) { panic(fmt.Sprintf("failed to migrate x/bank from version 2 to 3: %v", err)) } - if err := cfg.RegisterMigration(types.ModuleName, 3, m.Migrate3to4); err != nil { + if err := cfg.RegisterMigration(types.ModuleName, 3, m.Migrate3to4WithSendEnabledParams); err != nil { panic(fmt.Sprintf("failed to migrate x/bank from version 3 to 4: %v", err)) } } @@ -197,6 +197,12 @@ func (am AppModule) WeightedOperations(simState module.SimulationState) []simtyp ) } +// MigrateSubspaceParams get send enabled params from x/params and update the bank params. +func (am AppModule) MigrateSubspaceParams(ctx sdk.Context) { + sendEnabled := types.GetSendEnabledParams(ctx, am.legacySubspace) + am.keeper.SetAllSendEnabled(ctx, sendEnabled) +} + // App Wiring Setup func init() { From 142bc2ea7e79c94f52c5775be9b077df033cde39 Mon Sep 17 00:00:00 2001 From: mmsqe Date: Fri, 13 Oct 2023 23:39:58 +0800 Subject: [PATCH 5/6] Apply suggestions from code review --- x/bank/keeper/migrations.go | 7 ++++--- x/bank/module.go | 8 +------- 2 files changed, 5 insertions(+), 10 deletions(-) diff --git a/x/bank/keeper/migrations.go b/x/bank/keeper/migrations.go index 9ab026250c97..80afd55fdbfd 100644 --- a/x/bank/keeper/migrations.go +++ b/x/bank/keeper/migrations.go @@ -32,13 +32,14 @@ func (m Migrator) Migrate2to3(ctx sdk.Context) error { // Migrate3to4 migrates x/bank storage from version 3 to 4. func (m Migrator) Migrate3to4(ctx sdk.Context) error { + m.MigrateSendEnabledParams(ctx) return v4.MigrateStore(ctx, m.keeper.storeKey, m.legacySubspace, m.keeper.cdc) } -// Migrate3to4WithSendEnabledParams migrates x/bank storage from version 3 to 4 with +// MigrateSendEnabledParams migrates x/bank storage from version 3 to 4 with // the send enabled params get from x/params and update the bank params. -func (m Migrator) Migrate3to4WithSendEnabledParams(ctx sdk.Context) error { +// This function is only needed for chains having migrated from <= v0.47 to v0.47.0-5 +func (m Migrator) MigrateSendEnabledParams(ctx sdk.Context) { sendEnabled := types.GetSendEnabledParams(ctx, m.legacySubspace) m.keeper.SetAllSendEnabled(ctx, sendEnabled) - return v4.MigrateStore(ctx, m.keeper.storeKey, m.legacySubspace, m.keeper.cdc) } diff --git a/x/bank/module.go b/x/bank/module.go index e8c7cdbf2932..45ff753f2545 100644 --- a/x/bank/module.go +++ b/x/bank/module.go @@ -127,7 +127,7 @@ func (am AppModule) RegisterServices(cfg module.Configurator) { panic(fmt.Sprintf("failed to migrate x/bank from version 2 to 3: %v", err)) } - if err := cfg.RegisterMigration(types.ModuleName, 3, m.Migrate3to4WithSendEnabledParams); err != nil { + if err := cfg.RegisterMigration(types.ModuleName, 3, m.Migrate3to4); err != nil { panic(fmt.Sprintf("failed to migrate x/bank from version 3 to 4: %v", err)) } } @@ -197,12 +197,6 @@ func (am AppModule) WeightedOperations(simState module.SimulationState) []simtyp ) } -// MigrateSubspaceParams get send enabled params from x/params and update the bank params. -func (am AppModule) MigrateSubspaceParams(ctx sdk.Context) { - sendEnabled := types.GetSendEnabledParams(ctx, am.legacySubspace) - am.keeper.SetAllSendEnabled(ctx, sendEnabled) -} - // App Wiring Setup func init() { From 861e9b80b53b8b57d8fb625254c224f72976d4ee Mon Sep 17 00:00:00 2001 From: mmsqe Date: Sat, 14 Oct 2023 00:08:31 +0800 Subject: [PATCH 6/6] fix doc --- x/bank/keeper/migrations.go | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/x/bank/keeper/migrations.go b/x/bank/keeper/migrations.go index 80afd55fdbfd..7371989a7593 100644 --- a/x/bank/keeper/migrations.go +++ b/x/bank/keeper/migrations.go @@ -36,8 +36,7 @@ func (m Migrator) Migrate3to4(ctx sdk.Context) error { return v4.MigrateStore(ctx, m.keeper.storeKey, m.legacySubspace, m.keeper.cdc) } -// MigrateSendEnabledParams migrates x/bank storage from version 3 to 4 with -// the send enabled params get from x/params and update the bank params. +// MigrateSendEnabledParams get params from x/params and update the bank params. // This function is only needed for chains having migrated from <= v0.47 to v0.47.0-5 func (m Migrator) MigrateSendEnabledParams(ctx sdk.Context) { sendEnabled := types.GetSendEnabledParams(ctx, m.legacySubspace)