From c7a8a2cf6a505d4a61f0611cfae893d518825362 Mon Sep 17 00:00:00 2001 From: Daniel T <30197399+danwt@users.noreply.github.com> Date: Mon, 11 Nov 2024 11:47:38 +0000 Subject: [PATCH] fix(migration): properly migrates params (#1459) Co-authored-by: omritoptix --- app/keepers/modules.go | 5 +-- app/upgrades/v4/delayedack_params.go | 2 +- app/upgrades/v4/upgrade.go | 37 ++++++++++------------ app/upgrades/v4/upgrade_test.go | 2 -- x/incentives/keeper/params.go | 5 +++ x/sequencer/module.go | 12 +------ x/sequencer/params_migrations.go | 45 --------------------------- x/sequencer/params_migrations_test.go | 44 -------------------------- x/sequencer/types/keys.go | 1 + x/sequencer/types/params_legacy.go | 38 ---------------------- 10 files changed, 27 insertions(+), 164 deletions(-) delete mode 100644 x/sequencer/params_migrations.go delete mode 100644 x/sequencer/params_migrations_test.go delete mode 100644 x/sequencer/types/params_legacy.go diff --git a/app/keepers/modules.go b/app/keepers/modules.go index 43ce2f1c8..be3cd11b3 100644 --- a/app/keepers/modules.go +++ b/app/keepers/modules.go @@ -205,7 +205,8 @@ func (a *AppKeepers) SetupModules( ibctransfer.NewAppModule(a.TransferKeeper), rollappmodule.NewAppModule(appCodec, a.RollappKeeper, a.AccountKeeper, a.BankKeeper), iro.NewAppModule(appCodec, *a.IROKeeper), - sequencermodule.NewAppModule(appCodec, a.SequencerKeeper, a.AccountKeeper, a.BankKeeper, a.GetSubspace(sequencertypes.ModuleName)), + + sequencermodule.NewAppModule(appCodec, a.SequencerKeeper, a.AccountKeeper, a.BankKeeper), sponsorship.NewAppModule(a.SponsorshipKeeper), streamermodule.NewAppModule(a.StreamerKeeper, a.AccountKeeper, a.BankKeeper, a.EpochsKeeper), delayedackmodule.NewAppModule(appCodec, a.DelayedAckKeeper, a.delayedAckMiddleware), @@ -268,8 +269,8 @@ var maccPerms = map[string][]string{ } var BeginBlockers = []string{ - epochstypes.ModuleName, upgradetypes.ModuleName, + epochstypes.ModuleName, capabilitytypes.ModuleName, minttypes.ModuleName, distrtypes.ModuleName, diff --git a/app/upgrades/v4/delayedack_params.go b/app/upgrades/v4/delayedack_params.go index 81974ff55..64a6a7d50 100644 --- a/app/upgrades/v4/delayedack_params.go +++ b/app/upgrades/v4/delayedack_params.go @@ -12,7 +12,7 @@ func migrateDelayedAckParams(ctx sdk.Context, delayedAckKeeper delayedackkeeper. params := delayedacktypes.DefaultParams() // EpochIdentifier is the only one that hasn't changed - params.EpochIdentifier = delayedAckKeeper.GetParams(ctx).EpochIdentifier + params.EpochIdentifier = delayedAckKeeper.EpochIdentifier(ctx) delayedAckKeeper.SetParams(ctx, params) } diff --git a/app/upgrades/v4/upgrade.go b/app/upgrades/v4/upgrade.go index ed41571da..8b6f08942 100644 --- a/app/upgrades/v4/upgrade.go +++ b/app/upgrades/v4/upgrade.go @@ -51,7 +51,7 @@ func CreateUpgradeHandler( return func(ctx sdk.Context, _ upgradetypes.Plan, fromVM module.VersionMap) (module.VersionMap, error) { logger := ctx.Logger().With("upgrade", UpgradeName) - LoadDeprecatedParamsSubspaces(keepers) + setKeyTables(keepers) // Run migrations before applying any other state changes. // NOTE: DO NOT PUT ANY STATE CHANGES BEFORE RunMigrations(). @@ -60,12 +60,12 @@ func CreateUpgradeHandler( if err != nil { return nil, err } + migrateModuleParams(ctx, keepers) if err := deprecateCrisisModule(ctx, keepers.CrisisKeeper); err != nil { return nil, err } - migrateModuleParams(ctx, keepers) migrateDelayedAckParams(ctx, keepers.DelayedAckKeeper) migrateRollappParams(ctx, keepers.RollappKeeper) if err := migrateRollapps(ctx, keepers.RollappKeeper); err != nil { @@ -106,16 +106,7 @@ func CreateUpgradeHandler( } } -//nolint:staticcheck -func migrateModuleParams(ctx sdk.Context, keepers *keepers.AppKeepers) { - // Migrate Tendermint consensus parameters from x/params module to a dedicated x/consensus module. - baseAppLegacySS := keepers.ParamsKeeper.Subspace(baseapp.Paramspace).WithKeyTable(paramstypes.ConsensusParamsKeyTable()) - baseapp.MigrateParams(ctx, baseAppLegacySS, &keepers.ConsensusParamsKeeper) -} - -// LoadDeprecatedParamsSubspaces loads the deprecated param subspaces for each module -// used to support the migration from x/params to each module's own store -func LoadDeprecatedParamsSubspaces(keepers *keepers.AppKeepers) { +func setKeyTables(keepers *keepers.AppKeepers) { for _, subspace := range keepers.ParamsKeeper.GetSubspaces() { var keyTable paramstypes.KeyTable switch subspace.Name() { @@ -141,7 +132,7 @@ func LoadDeprecatedParamsSubspaces(keepers *keepers.AppKeepers) { case rollapptypes.ModuleName: keyTable = rollapptypes.ParamKeyTable() case sequencertypes.ModuleName: - keyTable = sequencertypes.ParamKeyTable() + continue // Ethermint modules case evmtypes.ModuleName: @@ -158,6 +149,13 @@ func LoadDeprecatedParamsSubspaces(keepers *keepers.AppKeepers) { } } +//nolint:staticcheck +func migrateModuleParams(ctx sdk.Context, keepers *keepers.AppKeepers) { + // Migrate Tendermint consensus parameters from x/params module to a dedicated x/consensus module. + baseAppLegacySS := keepers.ParamsKeeper.Subspace(baseapp.Paramspace).WithKeyTable(paramstypes.ConsensusParamsKeyTable()) + baseapp.MigrateParams(ctx, baseAppLegacySS, &keepers.ConsensusParamsKeeper) +} + // migrateRollappGauges creates a gauge for each rollapp in the store func migrateRollappGauges(ctx sdk.Context, rollappkeeper *rollappkeeper.Keeper, incentivizeKeeper *incentiveskeeper.Keeper) error { rollapps := rollappkeeper.GetAllRollapps(ctx) @@ -205,9 +203,8 @@ func migrateRollappLightClients(ctx sdk.Context, rollappkeeper *rollappkeeper.Ke // migrateStreamer creates epoch pointers for all epoch infos and updates module params func migrateStreamer(ctx sdk.Context, sk streamerkeeper.Keeper, ek *epochskeeper.Keeper) error { // update module params - oldParams := sk.GetParams(ctx) - oldParams.MaxIterationsPerBlock = streamertypes.DefaultMaxIterationsPerBlock - sk.SetParams(ctx, oldParams) + params := streamertypes.DefaultParams() + sk.SetParams(ctx, params) // create epoch pointers for all epoch infos for _, epoch := range ek.AllEpochInfos(ctx) { @@ -271,11 +268,8 @@ func ReformatFinalizationQueue(queue rollapptypes.BlockHeightToFinalizationQueue } func migrateIncentivesParams(ctx sdk.Context, ik *incentiveskeeper.Keeper) { - params := ik.GetParams(ctx) - defaultParams := incentivestypes.DefaultParams() - params.CreateGaugeBaseFee = defaultParams.CreateGaugeBaseFee - params.AddToGaugeBaseFee = defaultParams.AddToGaugeBaseFee - params.AddDenomFee = defaultParams.AddDenomFee + params := incentivestypes.DefaultParams() + params.DistrEpochIdentifier = ik.DistrEpochIdentifier(ctx) ik.SetParams(ctx, params) } @@ -355,6 +349,7 @@ func ConvertOldSequencerToNew(old sequencertypes.Sequencer) sequencertypes.Seque Status: old.Status, Tokens: old.Tokens, OptedIn: true, + Proposer: old.Proposer, Metadata: sequencertypes.SequencerMetadata{ Moniker: old.Metadata.Moniker, Details: old.Metadata.Details, diff --git a/app/upgrades/v4/upgrade_test.go b/app/upgrades/v4/upgrade_test.go index d4c0f3449..e03b4978e 100644 --- a/app/upgrades/v4/upgrade_test.go +++ b/app/upgrades/v4/upgrade_test.go @@ -71,8 +71,6 @@ func (s *UpgradeTestSuite) TestUpgrade() { msg: "Test that upgrade does not panic and sets correct parameters and migrates rollapp module", numRollapps: 5, preUpgrade: func(numRollapps int) error { - v4.LoadDeprecatedParamsSubspaces(&s.App.AppKeepers) - // Create and store rollapps s.seedAndStoreRollapps(numRollapps) diff --git a/x/incentives/keeper/params.go b/x/incentives/keeper/params.go index a3661f65c..2a19f8a45 100644 --- a/x/incentives/keeper/params.go +++ b/x/incentives/keeper/params.go @@ -16,3 +16,8 @@ func (k Keeper) GetParams(ctx sdk.Context) (params types.Params) { func (k Keeper) SetParams(ctx sdk.Context, params types.Params) { k.paramSpace.SetParamSet(ctx, ¶ms) } + +func (k Keeper) DistrEpochIdentifier(ctx sdk.Context) (res string) { + k.paramSpace.Get(ctx, types.KeyDistrEpochIdentifier, &res) + return +} diff --git a/x/sequencer/module.go b/x/sequencer/module.go index 171755e03..3092c7a94 100644 --- a/x/sequencer/module.go +++ b/x/sequencer/module.go @@ -101,9 +101,6 @@ type AppModule struct { keeper *keeper.Keeper accountKeeper types.AccountKeeper bankKeeper types.BankKeeper - - // legacySubspace is used solely for migration of x/params managed parameters - legacySubspace Subspace } func NewAppModule( @@ -111,14 +108,12 @@ func NewAppModule( keeper *keeper.Keeper, accountKeeper types.AccountKeeper, bankKeeper types.BankKeeper, - ss Subspace, ) AppModule { return AppModule{ AppModuleBasic: NewAppModuleBasic(cdc), keeper: keeper, accountKeeper: accountKeeper, bankKeeper: bankKeeper, - legacySubspace: ss, } } @@ -132,11 +127,6 @@ func (am AppModule) Name() string { func (am AppModule) RegisterServices(cfg module.Configurator) { types.RegisterMsgServer(cfg.MsgServer(), keeper.NewMsgServerImpl(am.keeper)) types.RegisterQueryServer(cfg.QueryServer(), am.keeper) - - m := NewMigrator(am.keeper, am.legacySubspace) - if err := cfg.RegisterMigration(types.ModuleName, 2, m.Migrate2to3); err != nil { - panic(fmt.Sprintf("failed to migrate x/%s from version 2 to 3: %v", types.ModuleName, err)) - } } // RegisterInvariants registers the module's invariants. @@ -163,7 +153,7 @@ func (am AppModule) ExportGenesis(ctx sdk.Context, cdc codec.JSONCodec) json.Raw } // ConsensusVersion implements ConsensusVersion. -func (AppModule) ConsensusVersion() uint64 { return 3 } +func (AppModule) ConsensusVersion() uint64 { return 2 } // BeginBlock executes all ABCI BeginBlock logic respective to the capability module. func (am AppModule) BeginBlock(ctx sdk.Context, _ abci.RequestBeginBlock) { diff --git a/x/sequencer/params_migrations.go b/x/sequencer/params_migrations.go deleted file mode 100644 index 4d88febb4..000000000 --- a/x/sequencer/params_migrations.go +++ /dev/null @@ -1,45 +0,0 @@ -package sequencer - -import ( - sdk "github.com/cosmos/cosmos-sdk/types" - paramtypes "github.com/cosmos/cosmos-sdk/x/params/types" - - seqkeeper "github.com/dymensionxyz/dymension/v3/x/sequencer/keeper" - "github.com/dymensionxyz/dymension/v3/x/sequencer/types" -) - -type ( - ParamSet = paramtypes.ParamSet - - // Subspace defines an interface that implements the legacy x/params Subspace - // type. - // - // NOTE: This is used solely for migration of x/params managed parameters. - Subspace interface { - GetParamSet(ctx sdk.Context, ps ParamSet) - } -) - -// Migrator is a struct for handling in-place store migrations. -type Migrator struct { - keeper *seqkeeper.Keeper - legacySubspace Subspace -} - -// NewMigrator returns a new Migrator. -func NewMigrator(keeper *seqkeeper.Keeper, ss Subspace) Migrator { - return Migrator{keeper: keeper, legacySubspace: ss} -} - -// Migrate2to3 migrates from version 2 to 3. -func (m Migrator) Migrate2to3(ctx sdk.Context) error { - var currParams types.Params - m.legacySubspace.GetParamSet(ctx, &currParams) - - if err := currParams.ValidateBasic(); err != nil { - return err - } - - m.keeper.SetParams(ctx, currParams) - return nil -} diff --git a/x/sequencer/params_migrations_test.go b/x/sequencer/params_migrations_test.go deleted file mode 100644 index 53afb31d9..000000000 --- a/x/sequencer/params_migrations_test.go +++ /dev/null @@ -1,44 +0,0 @@ -package sequencer_test - -import ( - "testing" - "time" - - cometbftproto "github.com/cometbft/cometbft/proto/tendermint/types" - - "github.com/dymensionxyz/dymension/v3/app/apptesting" - seq "github.com/dymensionxyz/dymension/v3/x/sequencer" - "github.com/dymensionxyz/dymension/v3/x/sequencer/types" -) - -func TestMigrate2to3(t *testing.T) { - app := apptesting.Setup(t, false) - ctx := app.BaseApp.NewContext(false, cometbftproto.Header{Height: 1, ChainID: "dymension_100-1", Time: time.Now().UTC()}) - - // create legacy subspace - params := types.DefaultParams() - - seqSubspace, ok := app.AppKeepers.ParamsKeeper.GetSubspace(types.ModuleName) - if !ok { - t.Fatalf("sequencer subspace not found") - } - - // set KeyTable if it has not already been set - if !seqSubspace.HasKeyTable() { - seqSubspace = seqSubspace.WithKeyTable(types.ParamKeyTable()) - } - seqSubspace.SetParamSet(ctx, ¶ms) - - // Create a migrator instance - migrator := seq.NewMigrator(app.SequencerKeeper, seqSubspace) - - // Call the Migrate2to3 function - err := migrator.Migrate2to3(ctx) - // Check if there was an error - if err != nil { - t.Errorf("Migrate2to3 returned an error: %s", err) - } - - // Check if the value was migrated correctly - params = app.SequencerKeeper.GetParams(ctx) -} diff --git a/x/sequencer/types/keys.go b/x/sequencer/types/keys.go index 56679891b..dc8e81284 100644 --- a/x/sequencer/types/keys.go +++ b/x/sequencer/types/keys.go @@ -58,6 +58,7 @@ var ( // These keys were already used on mainnet. Don't reuse _ = []byte{0xa3} _ = []byte{0x41} + _ = []byte("MinBond") ) /* --------------------- specific sequencer address keys -------------------- */ diff --git a/x/sequencer/types/params_legacy.go b/x/sequencer/types/params_legacy.go deleted file mode 100644 index 8f32f7c31..000000000 --- a/x/sequencer/types/params_legacy.go +++ /dev/null @@ -1,38 +0,0 @@ -/* -NOTE: Usage of x/params to manage parameters is deprecated in favor of x/gov -controlled execution of MsgUpdateParams messages. These types remains solely -for migration purposes and will be removed in a future release. -*/ -package types - -import ( - paramtypes "github.com/cosmos/cosmos-sdk/x/params/types" - "github.com/dymensionxyz/sdk-utils/utils/uparam" -) - -var _ paramtypes.ParamSet = &Params{} - -var ( - KeyMinBond = []byte("MinBond") - KeyKickThreshold = []byte("KickThreshold") - KeyNoticePeriod = []byte("NoticePeriod") - keyLivenessSlashMinMultiplier = []byte("LivenessSlashMultiplier") - KeyLivenessSlashMinAbsolute = []byte("LivenessSlashMinAbsolute") -) - -// Deprecated: ParamKeyTable for module -func ParamKeyTable() paramtypes.KeyTable { - return paramtypes.NewKeyTable().RegisterParamSet(&Params{}) -} - -// ParamSetPairs implements the ParamSet interface and returns all the key/value pairs -// pairs of module's parameters. -func (p *Params) ParamSetPairs() paramtypes.ParamSetPairs { - return paramtypes.ParamSetPairs{ - paramtypes.NewParamSetPair(KeyMinBond, &p.MinBond, validateMinBond), - paramtypes.NewParamSetPair(KeyKickThreshold, &p.KickThreshold, uparam.ValidateCoin), - paramtypes.NewParamSetPair(KeyNoticePeriod, &p.NoticePeriod, validateTime), - paramtypes.NewParamSetPair(keyLivenessSlashMinMultiplier, &p.LivenessSlashMinMultiplier, validateLivenessSlashMultiplier), - paramtypes.NewParamSetPair(KeyLivenessSlashMinAbsolute, &p.LivenessSlashMinAbsolute, uparam.ValidateCoin), - } -}