From 27453b3be3717753bb07cafe3d30756946a23e47 Mon Sep 17 00:00:00 2001 From: MSalopek Date: Wed, 24 Apr 2024 22:17:42 +0200 Subject: [PATCH 1/3] chore: add consumer migration 2 -> 3 --- x/ccv/consumer/keeper/migrations.go | 7 +- .../v3}/legacy_params.go | 42 ++++---- x/ccv/consumer/migrations/v3/migration.go | 17 ++-- .../consumer/migrations/v3/migration_test.go | 97 +++++++++++++++++++ x/ccv/consumer/module.go | 5 +- x/ccv/provider/module.go | 2 +- 6 files changed, 141 insertions(+), 29 deletions(-) rename x/ccv/consumer/{keeper => migrations/v3}/legacy_params.go (67%) create mode 100644 x/ccv/consumer/migrations/v3/migration_test.go diff --git a/x/ccv/consumer/keeper/migrations.go b/x/ccv/consumer/keeper/migrations.go index ba4746b3d4..145d595087 100644 --- a/x/ccv/consumer/keeper/migrations.go +++ b/x/ccv/consumer/keeper/migrations.go @@ -5,6 +5,7 @@ import ( paramtypes "github.com/cosmos/cosmos-sdk/x/params/types" v2 "github.com/cosmos/interchain-security/v5/x/ccv/consumer/migrations/v2" + v3 "github.com/cosmos/interchain-security/v5/x/ccv/consumer/migrations/v3" ) // Migrator is a struct for handling in-place store migrations. @@ -24,8 +25,10 @@ func (m Migrator) Migrate1to2(ctx sdk.Context) error { return v2.MigrateConsumerPacketData(ctx, store) } -// Migrate2to3 migrates x/ccvconsumer state from consensus version 2 to 3. +// Migrate2to3 migrates x/ccvconsumer from consensus version 2 to 3. +// This migration is necessary to move the consumer module legacy params. func (m Migrator) Migrate2to3(ctx sdk.Context) error { store := ctx.KVStore(m.keeper.storeKey) - return v2.MigrateConsumerPacketData(ctx, store) + cdc := m.keeper.cdc + return v3.MigrateLegacyParams(ctx, cdc, store, m.paramSpace) } diff --git a/x/ccv/consumer/keeper/legacy_params.go b/x/ccv/consumer/migrations/v3/legacy_params.go similarity index 67% rename from x/ccv/consumer/keeper/legacy_params.go rename to x/ccv/consumer/migrations/v3/legacy_params.go index 9bf1618dcc..a7b4e5fed5 100644 --- a/x/ccv/consumer/keeper/legacy_params.go +++ b/x/ccv/consumer/migrations/v3/legacy_params.go @@ -1,18 +1,24 @@ -package keeper +package v3 import ( "time" sdk "github.com/cosmos/cosmos-sdk/types" - paramtypes "github.com/cosmos/cosmos-sdk/x/params/types" ccvtypes "github.com/cosmos/interchain-security/v5/x/ccv/types" ) +// helper interface +// paramtypes.ParamSpace implements this interface because it +// implements the Get(ctx sdk.Context, key []byte, ptr interface{}) +// since only Get(...) is needed to migrate params we can ignore the other methods on paramtypes.ParamSpace. +type ParamSubspace interface { + Get(ctx sdk.Context, key []byte, ptr interface{}) +} + // Legacy: used for migration only! -// GetConsumerParamsLegacy returns the params for the consumer ccv module from x/param subspace -// which will be deprecated soon -func GetConsumerParamsLegacy(ctx sdk.Context, keeper Keeper, paramSpace paramtypes.Subspace) ccvtypes.ConsumerParams { +// GetConsumerParamsLegacy returns the params for the consumer ccv module from legacy subspace +func GetConsumerParamsLegacy(ctx sdk.Context, paramSpace ParamSubspace) ccvtypes.ConsumerParams { return ccvtypes.NewParams( getEnabled(ctx, paramSpace), getBlocksPerDistributionTransmission(ctx, paramSpace), @@ -31,39 +37,39 @@ func GetConsumerParamsLegacy(ctx sdk.Context, keeper Keeper, paramSpace paramtyp } // getEnabled returns the enabled flag for the consumer module -func getEnabled(ctx sdk.Context, paramStore paramtypes.Subspace) bool { +func getEnabled(ctx sdk.Context, paramStore ParamSubspace) bool { var enabled bool paramStore.Get(ctx, ccvtypes.KeyEnabled, &enabled) return enabled } -func getBlocksPerDistributionTransmission(ctx sdk.Context, paramStore paramtypes.Subspace) int64 { +func getBlocksPerDistributionTransmission(ctx sdk.Context, paramStore ParamSubspace) int64 { var bpdt int64 paramStore.Get(ctx, ccvtypes.KeyBlocksPerDistributionTransmission, &bpdt) return bpdt } -func getDistributionTransmissionChannel(ctx sdk.Context, paramStore paramtypes.Subspace) string { +func getDistributionTransmissionChannel(ctx sdk.Context, paramStore ParamSubspace) string { var s string paramStore.Get(ctx, ccvtypes.KeyDistributionTransmissionChannel, &s) return s } -func getProviderFeePoolAddrStr(ctx sdk.Context, paramStore paramtypes.Subspace) string { +func getProviderFeePoolAddrStr(ctx sdk.Context, paramStore ParamSubspace) string { var s string paramStore.Get(ctx, ccvtypes.KeyProviderFeePoolAddrStr, &s) return s } // getCCVTimeoutPeriod returns the timeout period for sent ccv related ibc packets -func getCCVTimeoutPeriod(ctx sdk.Context, paramStore paramtypes.Subspace) time.Duration { +func getCCVTimeoutPeriod(ctx sdk.Context, paramStore ParamSubspace) time.Duration { var p time.Duration paramStore.Get(ctx, ccvtypes.KeyCCVTimeoutPeriod, &p) return p } // getTransferTimeoutPeriod returns the timeout period for sent transfer related ibc packets -func getTransferTimeoutPeriod(ctx sdk.Context, paramStore paramtypes.Subspace) time.Duration { +func getTransferTimeoutPeriod(ctx sdk.Context, paramStore ParamSubspace) time.Duration { var p time.Duration paramStore.Get(ctx, ccvtypes.KeyTransferTimeoutPeriod, &p) return p @@ -72,20 +78,20 @@ func getTransferTimeoutPeriod(ctx sdk.Context, paramStore paramtypes.Subspace) t // getConsumerRedistributionFrac returns the fraction of tokens allocated to the consumer redistribution // address during distribution events. The fraction is a string representing a // decimal number. For example "0.75" would represent 75%. -func getConsumerRedistributionFrac(ctx sdk.Context, paramStore paramtypes.Subspace) string { +func getConsumerRedistributionFrac(ctx sdk.Context, paramStore ParamSubspace) string { var str string paramStore.Get(ctx, ccvtypes.KeyConsumerRedistributionFrac, &str) return str } // getHistoricalEntries returns the number of historical info entries to persist in store -func getHistoricalEntries(ctx sdk.Context, paramStore paramtypes.Subspace) int64 { +func getHistoricalEntries(ctx sdk.Context, paramStore ParamSubspace) int64 { var n int64 paramStore.Get(ctx, ccvtypes.KeyHistoricalEntries, &n) return n } -func getUnbondingPeriod(ctx sdk.Context, paramStore paramtypes.Subspace) time.Duration { +func getUnbondingPeriod(ctx sdk.Context, paramStore ParamSubspace) time.Duration { var period time.Duration paramStore.Get(ctx, ccvtypes.KeyConsumerUnbondingPeriod, &period) return period @@ -93,25 +99,25 @@ func getUnbondingPeriod(ctx sdk.Context, paramStore paramtypes.Subspace) time.Du // getSoftOptOutThreshold returns the percentage of validators at the bottom of the set // that can opt out of running the consumer chain -func getSoftOptOutThreshold(ctx sdk.Context, paramStore paramtypes.Subspace) string { +func getSoftOptOutThreshold(ctx sdk.Context, paramStore ParamSubspace) string { var str string paramStore.Get(ctx, ccvtypes.KeySoftOptOutThreshold, &str) return str } -func getRewardDenoms(ctx sdk.Context, paramStore paramtypes.Subspace) []string { +func getRewardDenoms(ctx sdk.Context, paramStore ParamSubspace) []string { var denoms []string paramStore.Get(ctx, ccvtypes.KeyRewardDenoms, &denoms) return denoms } -func getProviderRewardDenoms(ctx sdk.Context, paramStore paramtypes.Subspace) []string { +func getProviderRewardDenoms(ctx sdk.Context, paramStore ParamSubspace) []string { var denoms []string paramStore.Get(ctx, ccvtypes.KeyProviderRewardDenoms, &denoms) return denoms } -func getRetryDelayPeriod(ctx sdk.Context, paramStore paramtypes.Subspace) time.Duration { +func getRetryDelayPeriod(ctx sdk.Context, paramStore ParamSubspace) time.Duration { var period time.Duration paramStore.Get(ctx, ccvtypes.KeyRetryDelayPeriod, &period) return period diff --git a/x/ccv/consumer/migrations/v3/migration.go b/x/ccv/consumer/migrations/v3/migration.go index aa2bd21b3a..63d4187127 100644 --- a/x/ccv/consumer/migrations/v3/migration.go +++ b/x/ccv/consumer/migrations/v3/migration.go @@ -1,22 +1,25 @@ package v3 import ( + "github.com/cosmos/cosmos-sdk/codec" sdk "github.com/cosmos/cosmos-sdk/types" - paramtypes "github.com/cosmos/cosmos-sdk/x/params/types" + storetypes "cosmossdk.io/store/types" - consumerKeeper "github.com/cosmos/interchain-security/v5/x/ccv/consumer/keeper" + "github.com/cosmos/interchain-security/v5/x/ccv/consumer/types" ) -// MigrateParams migrates the consumers module's parameters from the x/params subspace to the +// MigrateLegacyParams migrates the consumers module's parameters from the x/params subspace to the // consumer modules store. -func MigrateParams(ctx sdk.Context, keeper consumerKeeper.Keeper, legacyParamspace paramtypes.Subspace) error { - params := consumerKeeper.GetConsumerParamsLegacy(ctx, keeper, legacyParamspace) +func MigrateLegacyParams(ctx sdk.Context, cdc codec.BinaryCodec, store storetypes.KVStore, legacyParamspace ParamSubspace) error { + params := GetConsumerParamsLegacy(ctx, legacyParamspace) err := params.Validate() if err != nil { return err } - keeper.SetParams(ctx, params) - keeper.Logger(ctx).Info("successfully migrated provider parameters") + + bz := cdc.MustMarshal(¶ms) + store.Set(types.ParametersKey(), bz) + ctx.Logger().Info("successfully migrated consumer parameters") return nil } diff --git a/x/ccv/consumer/migrations/v3/migration_test.go b/x/ccv/consumer/migrations/v3/migration_test.go new file mode 100644 index 0000000000..69593516ce --- /dev/null +++ b/x/ccv/consumer/migrations/v3/migration_test.go @@ -0,0 +1,97 @@ +package v3 + +import ( + "fmt" + "testing" + "time" + + storetypes "cosmossdk.io/store/types" + "github.com/cosmos/cosmos-sdk/testutil" + sdk "github.com/cosmos/cosmos-sdk/types" + + "github.com/cosmos/interchain-security/v5/app/encoding" + consumertypes "github.com/cosmos/interchain-security/v5/x/ccv/consumer/types" + ccvtypes "github.com/cosmos/interchain-security/v5/x/ccv/types" + + "github.com/stretchr/testify/require" +) + +type testLegacyParamSubspace struct { + *ccvtypes.ConsumerParams +} + +func newTestLegacyParamsSubspace(p ccvtypes.ConsumerParams) testLegacyParamSubspace { + return testLegacyParamSubspace{ + &p, + } +} + +func (ps testLegacyParamSubspace) Get(ctx sdk.Context, key []byte, ptr interface{}) { + switch string(key) { + case string(ccvtypes.KeyEnabled): + *ptr.(*bool) = ps.Enabled + case string(ccvtypes.KeyBlocksPerDistributionTransmission): + *ptr.(*int64) = ps.BlocksPerDistributionTransmission + case string(ccvtypes.KeyDistributionTransmissionChannel): + *ptr.(*string) = ps.DistributionTransmissionChannel + case string(ccvtypes.KeyProviderFeePoolAddrStr): + *ptr.(*string) = ps.ProviderFeePoolAddrStr + case string(ccvtypes.KeyCCVTimeoutPeriod): + *ptr.(*time.Duration) = ps.CcvTimeoutPeriod + case string(ccvtypes.KeyTransferTimeoutPeriod): + *ptr.(*time.Duration) = ps.TransferTimeoutPeriod + case string(ccvtypes.KeyConsumerRedistributionFrac): + *ptr.(*string) = ps.ConsumerRedistributionFraction + case string(ccvtypes.KeyHistoricalEntries): + *ptr.(*int64) = ps.HistoricalEntries + case string(ccvtypes.KeyConsumerUnbondingPeriod): + *ptr.(*time.Duration) = ps.UnbondingPeriod + case string(ccvtypes.KeySoftOptOutThreshold): + *ptr.(*string) = ps.SoftOptOutThreshold + case string(ccvtypes.KeyRewardDenoms): + *ptr.(*[]string) = ps.RewardDenoms + case string(ccvtypes.KeyProviderRewardDenoms): + *ptr.(*[]string) = ps.ProviderRewardDenoms + case string(ccvtypes.KeyRetryDelayPeriod): + *ptr.(*time.Duration) = ps.RetryDelayPeriod + default: + panic(fmt.Sprintf("invalid paramspace key: %s", string(key))) + + } +} + +// MigrateParams migrates the consumers module's parameters from the x/params subspace to the +// consumer modules store. +func TestMigrateParams(t *testing.T) { + cdc := encoding.MakeTestEncodingConfig().Codec + govKey := storetypes.NewKVStoreKey("provider") + ctx := testutil.DefaultContext(govKey, storetypes.NewTransientStoreKey("transient_test")) + store := ctx.KVStore(govKey) + + defaultParams := ccvtypes.DefaultParams() + legacyParamSubspace := newTestLegacyParamsSubspace(defaultParams) + // confirms that testLegacyParamSubspace works as expected + require.NotPanics(t, func() { + GetConsumerParamsLegacy(ctx, legacyParamSubspace) + }) + + emptyParams := ccvtypes.ConsumerParams{} + bz := store.Get(consumertypes.ParametersKey()) + require.NoError(t, cdc.Unmarshal(bz, &emptyParams)) + require.NotNil(t, emptyParams) + require.Empty(t, emptyParams) + require.NotEqual(t, defaultParams, emptyParams) + + err := MigrateLegacyParams(ctx, cdc, store, legacyParamSubspace) + require.NoError(t, err) + + // check that new params are available after migration and equal to defaults + // legacyParamSubspace was set to match defaultParams + migratedParams := ccvtypes.ConsumerParams{} + paramsBz := store.Get(consumertypes.ParametersKey()) + require.NotEqual(t, bz, paramsBz) + require.NoError(t, cdc.Unmarshal(paramsBz, &migratedParams)) + + require.Equal(t, defaultParams, migratedParams) + require.NotEqual(t, emptyParams, migratedParams) +} diff --git a/x/ccv/consumer/module.go b/x/ccv/consumer/module.go index fba9d19093..49e19542be 100644 --- a/x/ccv/consumer/module.go +++ b/x/ccv/consumer/module.go @@ -127,6 +127,9 @@ func (am AppModule) RegisterServices(cfg module.Configurator) { if err := cfg.RegisterMigration(consumertypes.ModuleName, 1, m.Migrate1to2); err != nil { panic(fmt.Sprintf("failed to register migrator for %s: %s", consumertypes.ModuleName, err)) } + if err := cfg.RegisterMigration(consumertypes.ModuleName, 2, m.Migrate2to3); err != nil { + panic(fmt.Sprintf("failed to register migrator for %s: %s -- from 2 -> 3", consumertypes.ModuleName, err)) + } } // InitGenesis performs genesis initialization for the consumer module. It returns @@ -146,7 +149,7 @@ func (am AppModule) ExportGenesis(ctx sdk.Context, cdc codec.JSONCodec) json.Raw // ConsensusVersion implements AppModule/ConsensusVersion. func (AppModule) ConsensusVersion() uint64 { - return 2 + return 3 } // BeginBlock implements the AppModule interface diff --git a/x/ccv/provider/module.go b/x/ccv/provider/module.go index ef35716f8c..959672b88a 100644 --- a/x/ccv/provider/module.go +++ b/x/ccv/provider/module.go @@ -130,7 +130,7 @@ func (am AppModule) RegisterServices(cfg module.Configurator) { panic(fmt.Sprintf("failed to register migrator for %s: %s -- from 3 -> 4", providertypes.ModuleName, err)) } if err := cfg.RegisterMigration(providertypes.ModuleName, 4, migrator.Migrate4to5); err != nil { - panic(fmt.Sprintf("failed to register migrator for %s: %s -- from 4 -> 4", providertypes.ModuleName, err)) + panic(fmt.Sprintf("failed to register migrator for %s: %s -- from 4 -> 5", providertypes.ModuleName, err)) } } From 87ad996c553d9f463b5306ca52596b1460fd94ad Mon Sep 17 00:00:00 2001 From: MSalopek Date: Wed, 24 Apr 2024 23:15:55 +0200 Subject: [PATCH 2/3] chore: add provider migration; update params --- x/ccv/consumer/migrations/v3/legacy_params.go | 36 +++++------- x/ccv/consumer/migrations/v3/migration.go | 8 ++- .../consumer/migrations/v3/migration_test.go | 8 +-- x/ccv/provider/migrations/migrator.go | 4 ++ .../v5}/legacy_params.go | 23 ++++---- x/ccv/provider/migrations/v5/migrations.go | 10 ++-- .../provider/migrations/v5/migrations_test.go | 57 +++++++++++++++++++ x/ccv/types/params.go | 8 +++ 8 files changed, 107 insertions(+), 47 deletions(-) rename x/ccv/provider/{keeper => migrations/v5}/legacy_params.go (74%) create mode 100644 x/ccv/provider/migrations/v5/migrations_test.go diff --git a/x/ccv/consumer/migrations/v3/legacy_params.go b/x/ccv/consumer/migrations/v3/legacy_params.go index a7b4e5fed5..0906455e21 100644 --- a/x/ccv/consumer/migrations/v3/legacy_params.go +++ b/x/ccv/consumer/migrations/v3/legacy_params.go @@ -8,17 +8,9 @@ import ( ccvtypes "github.com/cosmos/interchain-security/v5/x/ccv/types" ) -// helper interface -// paramtypes.ParamSpace implements this interface because it -// implements the Get(ctx sdk.Context, key []byte, ptr interface{}) -// since only Get(...) is needed to migrate params we can ignore the other methods on paramtypes.ParamSpace. -type ParamSubspace interface { - Get(ctx sdk.Context, key []byte, ptr interface{}) -} - // Legacy: used for migration only! // GetConsumerParamsLegacy returns the params for the consumer ccv module from legacy subspace -func GetConsumerParamsLegacy(ctx sdk.Context, paramSpace ParamSubspace) ccvtypes.ConsumerParams { +func GetConsumerParamsLegacy(ctx sdk.Context, paramSpace ccvtypes.LegacyParamSubspace) ccvtypes.ConsumerParams { return ccvtypes.NewParams( getEnabled(ctx, paramSpace), getBlocksPerDistributionTransmission(ctx, paramSpace), @@ -37,39 +29,39 @@ func GetConsumerParamsLegacy(ctx sdk.Context, paramSpace ParamSubspace) ccvtypes } // getEnabled returns the enabled flag for the consumer module -func getEnabled(ctx sdk.Context, paramStore ParamSubspace) bool { +func getEnabled(ctx sdk.Context, paramStore ccvtypes.LegacyParamSubspace) bool { var enabled bool paramStore.Get(ctx, ccvtypes.KeyEnabled, &enabled) return enabled } -func getBlocksPerDistributionTransmission(ctx sdk.Context, paramStore ParamSubspace) int64 { +func getBlocksPerDistributionTransmission(ctx sdk.Context, paramStore ccvtypes.LegacyParamSubspace) int64 { var bpdt int64 paramStore.Get(ctx, ccvtypes.KeyBlocksPerDistributionTransmission, &bpdt) return bpdt } -func getDistributionTransmissionChannel(ctx sdk.Context, paramStore ParamSubspace) string { +func getDistributionTransmissionChannel(ctx sdk.Context, paramStore ccvtypes.LegacyParamSubspace) string { var s string paramStore.Get(ctx, ccvtypes.KeyDistributionTransmissionChannel, &s) return s } -func getProviderFeePoolAddrStr(ctx sdk.Context, paramStore ParamSubspace) string { +func getProviderFeePoolAddrStr(ctx sdk.Context, paramStore ccvtypes.LegacyParamSubspace) string { var s string paramStore.Get(ctx, ccvtypes.KeyProviderFeePoolAddrStr, &s) return s } // getCCVTimeoutPeriod returns the timeout period for sent ccv related ibc packets -func getCCVTimeoutPeriod(ctx sdk.Context, paramStore ParamSubspace) time.Duration { +func getCCVTimeoutPeriod(ctx sdk.Context, paramStore ccvtypes.LegacyParamSubspace) time.Duration { var p time.Duration paramStore.Get(ctx, ccvtypes.KeyCCVTimeoutPeriod, &p) return p } // getTransferTimeoutPeriod returns the timeout period for sent transfer related ibc packets -func getTransferTimeoutPeriod(ctx sdk.Context, paramStore ParamSubspace) time.Duration { +func getTransferTimeoutPeriod(ctx sdk.Context, paramStore ccvtypes.LegacyParamSubspace) time.Duration { var p time.Duration paramStore.Get(ctx, ccvtypes.KeyTransferTimeoutPeriod, &p) return p @@ -78,20 +70,20 @@ func getTransferTimeoutPeriod(ctx sdk.Context, paramStore ParamSubspace) time.Du // getConsumerRedistributionFrac returns the fraction of tokens allocated to the consumer redistribution // address during distribution events. The fraction is a string representing a // decimal number. For example "0.75" would represent 75%. -func getConsumerRedistributionFrac(ctx sdk.Context, paramStore ParamSubspace) string { +func getConsumerRedistributionFrac(ctx sdk.Context, paramStore ccvtypes.LegacyParamSubspace) string { var str string paramStore.Get(ctx, ccvtypes.KeyConsumerRedistributionFrac, &str) return str } // getHistoricalEntries returns the number of historical info entries to persist in store -func getHistoricalEntries(ctx sdk.Context, paramStore ParamSubspace) int64 { +func getHistoricalEntries(ctx sdk.Context, paramStore ccvtypes.LegacyParamSubspace) int64 { var n int64 paramStore.Get(ctx, ccvtypes.KeyHistoricalEntries, &n) return n } -func getUnbondingPeriod(ctx sdk.Context, paramStore ParamSubspace) time.Duration { +func getUnbondingPeriod(ctx sdk.Context, paramStore ccvtypes.LegacyParamSubspace) time.Duration { var period time.Duration paramStore.Get(ctx, ccvtypes.KeyConsumerUnbondingPeriod, &period) return period @@ -99,25 +91,25 @@ func getUnbondingPeriod(ctx sdk.Context, paramStore ParamSubspace) time.Duration // getSoftOptOutThreshold returns the percentage of validators at the bottom of the set // that can opt out of running the consumer chain -func getSoftOptOutThreshold(ctx sdk.Context, paramStore ParamSubspace) string { +func getSoftOptOutThreshold(ctx sdk.Context, paramStore ccvtypes.LegacyParamSubspace) string { var str string paramStore.Get(ctx, ccvtypes.KeySoftOptOutThreshold, &str) return str } -func getRewardDenoms(ctx sdk.Context, paramStore ParamSubspace) []string { +func getRewardDenoms(ctx sdk.Context, paramStore ccvtypes.LegacyParamSubspace) []string { var denoms []string paramStore.Get(ctx, ccvtypes.KeyRewardDenoms, &denoms) return denoms } -func getProviderRewardDenoms(ctx sdk.Context, paramStore ParamSubspace) []string { +func getProviderRewardDenoms(ctx sdk.Context, paramStore ccvtypes.LegacyParamSubspace) []string { var denoms []string paramStore.Get(ctx, ccvtypes.KeyProviderRewardDenoms, &denoms) return denoms } -func getRetryDelayPeriod(ctx sdk.Context, paramStore ParamSubspace) time.Duration { +func getRetryDelayPeriod(ctx sdk.Context, paramStore ccvtypes.LegacyParamSubspace) time.Duration { var period time.Duration paramStore.Get(ctx, ccvtypes.KeyRetryDelayPeriod, &period) return period diff --git a/x/ccv/consumer/migrations/v3/migration.go b/x/ccv/consumer/migrations/v3/migration.go index 63d4187127..aadc94bc43 100644 --- a/x/ccv/consumer/migrations/v3/migration.go +++ b/x/ccv/consumer/migrations/v3/migration.go @@ -6,12 +6,14 @@ import ( storetypes "cosmossdk.io/store/types" - "github.com/cosmos/interchain-security/v5/x/ccv/consumer/types" + consumertypes "github.com/cosmos/interchain-security/v5/x/ccv/consumer/types" + ccvtypes "github.com/cosmos/interchain-security/v5/x/ccv/types" ) // MigrateLegacyParams migrates the consumers module's parameters from the x/params subspace to the // consumer modules store. -func MigrateLegacyParams(ctx sdk.Context, cdc codec.BinaryCodec, store storetypes.KVStore, legacyParamspace ParamSubspace) error { +func MigrateLegacyParams(ctx sdk.Context, cdc codec.BinaryCodec, store storetypes.KVStore, legacyParamspace ccvtypes.LegacyParamSubspace) error { + ctx.Logger().Info("starting consumer legacy params migration") params := GetConsumerParamsLegacy(ctx, legacyParamspace) err := params.Validate() if err != nil { @@ -19,7 +21,7 @@ func MigrateLegacyParams(ctx sdk.Context, cdc codec.BinaryCodec, store storetype } bz := cdc.MustMarshal(¶ms) - store.Set(types.ParametersKey(), bz) + store.Set(consumertypes.ParametersKey(), bz) ctx.Logger().Info("successfully migrated consumer parameters") return nil } diff --git a/x/ccv/consumer/migrations/v3/migration_test.go b/x/ccv/consumer/migrations/v3/migration_test.go index 69593516ce..bdc3f59f70 100644 --- a/x/ccv/consumer/migrations/v3/migration_test.go +++ b/x/ccv/consumer/migrations/v3/migration_test.go @@ -60,13 +60,11 @@ func (ps testLegacyParamSubspace) Get(ctx sdk.Context, key []byte, ptr interface } } -// MigrateParams migrates the consumers module's parameters from the x/params subspace to the -// consumer modules store. func TestMigrateParams(t *testing.T) { cdc := encoding.MakeTestEncodingConfig().Codec - govKey := storetypes.NewKVStoreKey("provider") - ctx := testutil.DefaultContext(govKey, storetypes.NewTransientStoreKey("transient_test")) - store := ctx.KVStore(govKey) + storeKey := storetypes.NewKVStoreKey("ccvconsumer") + ctx := testutil.DefaultContext(storeKey, storetypes.NewTransientStoreKey("transient_test")) + store := ctx.KVStore(storeKey) defaultParams := ccvtypes.DefaultParams() legacyParamSubspace := newTestLegacyParamsSubspace(defaultParams) diff --git a/x/ccv/provider/migrations/migrator.go b/x/ccv/provider/migrations/migrator.go index fc99bc45c7..83812b6eb5 100644 --- a/x/ccv/provider/migrations/migrator.go +++ b/x/ccv/provider/migrations/migrator.go @@ -1,6 +1,8 @@ package migrations import ( + storetypes "cosmossdk.io/store/types" + "github.com/cosmos/cosmos-sdk/codec" sdktypes "github.com/cosmos/cosmos-sdk/types" paramtypes "github.com/cosmos/cosmos-sdk/x/params/types" @@ -14,6 +16,8 @@ import ( type Migrator struct { providerKeeper providerkeeper.Keeper paramSpace paramtypes.Subspace + cdc codec.BinaryCodec + storeKey storetypes.StoreKey } // NewMigrator returns a new Migrator. diff --git a/x/ccv/provider/keeper/legacy_params.go b/x/ccv/provider/migrations/v5/legacy_params.go similarity index 74% rename from x/ccv/provider/keeper/legacy_params.go rename to x/ccv/provider/migrations/v5/legacy_params.go index def88809ac..146339a131 100644 --- a/x/ccv/provider/keeper/legacy_params.go +++ b/x/ccv/provider/migrations/v5/legacy_params.go @@ -1,10 +1,9 @@ -package keeper +package v5 import ( "time" sdk "github.com/cosmos/cosmos-sdk/types" - paramtypes "github.com/cosmos/cosmos-sdk/x/params/types" ibctmtypes "github.com/cosmos/ibc-go/v8/modules/light-clients/07-tendermint" "github.com/cosmos/interchain-security/v5/x/ccv/provider/types" @@ -12,7 +11,7 @@ import ( ) // getTemplateClient returns the template client for provider proposals -func getTemplateClient(ctx sdk.Context, paramSpace paramtypes.Subspace) *ibctmtypes.ClientState { +func getTemplateClient(ctx sdk.Context, paramSpace ccvtypes.LegacyParamSubspace) *ibctmtypes.ClientState { var cs ibctmtypes.ClientState paramSpace.Get(ctx, types.KeyTemplateClient, &cs) return &cs @@ -20,28 +19,28 @@ func getTemplateClient(ctx sdk.Context, paramSpace paramtypes.Subspace) *ibctmty // getTrustingPeriodFraction returns a TrustingPeriodFraction // used to compute the provider IBC client's TrustingPeriod as UnbondingPeriod / TrustingPeriodFraction -func getTrustingPeriodFraction(ctx sdk.Context, paramSpace paramtypes.Subspace) string { +func getTrustingPeriodFraction(ctx sdk.Context, paramSpace ccvtypes.LegacyParamSubspace) string { var f string paramSpace.Get(ctx, types.KeyTrustingPeriodFraction, &f) return f } // getCCVTimeoutPeriod returns the timeout period for sent ibc packets -func getCCVTimeoutPeriod(ctx sdk.Context, paramSpace paramtypes.Subspace) time.Duration { +func getCCVTimeoutPeriod(ctx sdk.Context, paramSpace ccvtypes.LegacyParamSubspace) time.Duration { var p time.Duration paramSpace.Get(ctx, ccvtypes.KeyCCVTimeoutPeriod, &p) return p } // getInitTimeoutPeriod returns the init timeout period -func getInitTimeoutPeriod(ctx sdk.Context, paramSpace paramtypes.Subspace) time.Duration { +func getInitTimeoutPeriod(ctx sdk.Context, paramSpace ccvtypes.LegacyParamSubspace) time.Duration { var p time.Duration paramSpace.Get(ctx, types.KeyInitTimeoutPeriod, &p) return p } // getVscTimeoutPeriod returns the vsc timeout period -func getVscTimeoutPeriod(ctx sdk.Context, paramSpace paramtypes.Subspace) time.Duration { +func getVscTimeoutPeriod(ctx sdk.Context, paramSpace ccvtypes.LegacyParamSubspace) time.Duration { var p time.Duration paramSpace.Get(ctx, types.KeyVscTimeoutPeriod, &p) return p @@ -49,7 +48,7 @@ func getVscTimeoutPeriod(ctx sdk.Context, paramSpace paramtypes.Subspace) time.D // getSlashMeterReplenishPeriod returns the period in which: // Once the slash meter becomes not-full, the slash meter is replenished after this period. -func getSlashMeterReplenishPeriod(ctx sdk.Context, paramSpace paramtypes.Subspace) time.Duration { +func getSlashMeterReplenishPeriod(ctx sdk.Context, paramSpace ccvtypes.LegacyParamSubspace) time.Duration { var p time.Duration paramSpace.Get(ctx, types.KeySlashMeterReplenishPeriod, &p) return p @@ -58,19 +57,19 @@ func getSlashMeterReplenishPeriod(ctx sdk.Context, paramSpace paramtypes.Subspac // getSlashMeterReplenishFraction returns the string fraction of total voting power that is replenished // to the slash meter every replenish period. This param also serves as a maximum fraction of total // voting power that the slash meter can hold. -func getSlashMeterReplenishFraction(ctx sdk.Context, paramSpace paramtypes.Subspace) string { +func getSlashMeterReplenishFraction(ctx sdk.Context, paramSpace ccvtypes.LegacyParamSubspace) string { var f string paramSpace.Get(ctx, types.KeySlashMeterReplenishFraction, &f) return f } -func getConsumerRewardDenomRegistrationFee(ctx sdk.Context, paramSpace paramtypes.Subspace) sdk.Coin { +func getConsumerRewardDenomRegistrationFee(ctx sdk.Context, paramSpace ccvtypes.LegacyParamSubspace) sdk.Coin { var c sdk.Coin paramSpace.Get(ctx, types.KeyConsumerRewardDenomRegistrationFee, &c) return c } -func getBlocksPerEpoch(ctx sdk.Context, paramSpace paramtypes.Subspace) int64 { +func getBlocksPerEpoch(ctx sdk.Context, paramSpace ccvtypes.LegacyParamSubspace) int64 { var b int64 paramSpace.Get(ctx, types.KeyBlocksPerEpoch, &b) return b @@ -78,7 +77,7 @@ func getBlocksPerEpoch(ctx sdk.Context, paramSpace paramtypes.Subspace) int64 { // Legacy: Only for migration purposes. GetParamsLegacy returns the paramset for the provider // module from a given param subspace -func GetParamsLegacy(ctx sdk.Context, paramspace paramtypes.Subspace) types.Params { +func GetParamsLegacy(ctx sdk.Context, paramspace ccvtypes.LegacyParamSubspace) types.Params { return types.NewParams( getTemplateClient(ctx, paramspace), getTrustingPeriodFraction(ctx, paramspace), diff --git a/x/ccv/provider/migrations/v5/migrations.go b/x/ccv/provider/migrations/v5/migrations.go index face264ddc..f61428e48a 100644 --- a/x/ccv/provider/migrations/v5/migrations.go +++ b/x/ccv/provider/migrations/v5/migrations.go @@ -1,16 +1,16 @@ -package v4 +package v5 import ( sdk "github.com/cosmos/cosmos-sdk/types" - paramtypes "github.com/cosmos/cosmos-sdk/x/params/types" providerkeeper "github.com/cosmos/interchain-security/v5/x/ccv/provider/keeper" + ccvtypes "github.com/cosmos/interchain-security/v5/x/ccv/types" ) // MigrateParams migrates the provider module's parameters from the x/params to self store. -func MigrateLegacyParams(ctx sdk.Context, keeper providerkeeper.Keeper, legacySubspace paramtypes.Subspace) error { - keeper.Logger(ctx).Info("starting provider legacy params migration") - params := providerkeeper.GetParamsLegacy(ctx, legacySubspace) +func MigrateLegacyParams(ctx sdk.Context, keeper providerkeeper.Keeper, legacyParamspace ccvtypes.LegacyParamSubspace) error { + ctx.Logger().Info("starting provider legacy params migration") + params := GetParamsLegacy(ctx, legacyParamspace) err := params.Validate() if err != nil { return err diff --git a/x/ccv/provider/migrations/v5/migrations_test.go b/x/ccv/provider/migrations/v5/migrations_test.go new file mode 100644 index 0000000000..a7705da8b0 --- /dev/null +++ b/x/ccv/provider/migrations/v5/migrations_test.go @@ -0,0 +1,57 @@ +package v5 + +import ( + "testing" + + "github.com/stretchr/testify/require" + + testutil "github.com/cosmos/interchain-security/v5/testutil/keeper" + providertypes "github.com/cosmos/interchain-security/v5/x/ccv/provider/types" + ccvtypes "github.com/cosmos/interchain-security/v5/x/ccv/types" +) + +func TestMigrateParams(t *testing.T) { + t.Helper() + inMemParams := testutil.NewInMemKeeperParams(t) + k, ctx, ctrl, _ := testutil.GetProviderKeeperAndCtx(t, inMemParams) + defer ctrl.Finish() + + if !inMemParams.ParamsSubspace.HasKeyTable() { + inMemParams.ParamsSubspace.WithKeyTable(providertypes.ParamKeyTable()) + } + + defaultParams := providertypes.DefaultParams() + inMemParams.ParamsSubspace.Set(ctx, providertypes.KeyTemplateClient, defaultParams.TemplateClient) + inMemParams.ParamsSubspace.Set(ctx, providertypes.KeyTrustingPeriodFraction, defaultParams.TrustingPeriodFraction) + inMemParams.ParamsSubspace.Set(ctx, ccvtypes.KeyCCVTimeoutPeriod, defaultParams.CcvTimeoutPeriod) + inMemParams.ParamsSubspace.Set(ctx, providertypes.KeyInitTimeoutPeriod, defaultParams.InitTimeoutPeriod) + inMemParams.ParamsSubspace.Set(ctx, providertypes.KeyVscTimeoutPeriod, defaultParams.VscTimeoutPeriod) + inMemParams.ParamsSubspace.Set(ctx, providertypes.KeySlashMeterReplenishPeriod, defaultParams.SlashMeterReplenishPeriod) + inMemParams.ParamsSubspace.Set(ctx, providertypes.KeySlashMeterReplenishFraction, defaultParams.SlashMeterReplenishFraction) + inMemParams.ParamsSubspace.Set(ctx, providertypes.KeyConsumerRewardDenomRegistrationFee, defaultParams.ConsumerRewardDenomRegistrationFee) + inMemParams.ParamsSubspace.Set(ctx, providertypes.KeyBlocksPerEpoch, defaultParams.BlocksPerEpoch) + + // confirms that inMemParams.ParamsSubspace works as expected + require.NotPanics(t, func() { + GetParamsLegacy(ctx, inMemParams.ParamsSubspace) + }) + + // no "new" params should be available before migration + // "new" params are stored under providertypes.ParametersKey() + emptyParams := k.GetParams(ctx) + require.Empty(t, emptyParams) + + // make sure that the legacy params are equal to the default params (they were set using inMemParams.ParamsSubspace.Set()) + legacyParams := GetParamsLegacy(ctx, inMemParams.ParamsSubspace) + require.NotNil(t, legacyParams) + require.Equal(t, defaultParams, legacyParams) + + err := MigrateLegacyParams(ctx, k, inMemParams.ParamsSubspace) + require.NoError(t, err) + + // check that "new" params are available after migration and equal to defaults + migratedParams := k.GetParams(ctx) + require.NotEmpty(t, migratedParams) + require.Equal(t, defaultParams, migratedParams) + require.NotEqual(t, emptyParams, migratedParams) +} diff --git a/x/ccv/types/params.go b/x/ccv/types/params.go index 2bcbc41717..72a95ae7dd 100644 --- a/x/ccv/types/params.go +++ b/x/ccv/types/params.go @@ -61,6 +61,14 @@ var ( KeyRetryDelayPeriod = []byte("RetryDelayPeriod") ) +// helper interface +// sdk::paramtypes.ParamSpace implicitly implements this interface because it +// implements the Get(ctx sdk.Context, key []byte, ptr interface{}) +// since only Get(...) is needed to migrate params we can ignore the other methods on paramtypes.ParamSpace. +type LegacyParamSubspace interface { + Get(ctx sdktypes.Context, key []byte, ptr interface{}) +} + // ParamKeyTable type declaration for parameters func ParamKeyTable() paramtypes.KeyTable { return paramtypes.NewKeyTable().RegisterParamSet(&ConsumerParams{}) From 61abbf49cc07dc8d519e9eceba360d94605f9bb9 Mon Sep 17 00:00:00 2001 From: MSalopek Date: Thu, 25 Apr 2024 13:09:21 +0200 Subject: [PATCH 3/3] fix: resolve review nits --- x/ccv/provider/module.go | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/x/ccv/provider/module.go b/x/ccv/provider/module.go index 959672b88a..63e26431fc 100644 --- a/x/ccv/provider/module.go +++ b/x/ccv/provider/module.go @@ -122,9 +122,8 @@ func (am AppModule) RegisterServices(cfg module.Configurator) { providertypes.RegisterQueryServer(cfg.QueryServer(), am.keeper) migrator := migrations.NewMigrator(*am.keeper, am.paramSpace) - err := cfg.RegisterMigration(am.Name(), 2, migrator.Migrate2to3) - if err != nil { - panic(err) + if err := cfg.RegisterMigration(providertypes.ModuleName, 2, migrator.Migrate2to3); err != nil { + panic(fmt.Sprintf("failed to register migrator for %s: %s -- from 2 -> 3", providertypes.ModuleName, err)) } if err := cfg.RegisterMigration(providertypes.ModuleName, 3, migrator.Migrate3to4); err != nil { panic(fmt.Sprintf("failed to register migrator for %s: %s -- from 3 -> 4", providertypes.ModuleName, err))