fix(migration): properly migrates params (#1459)

Co-authored-by: omritoptix <[email protected]>

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,

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)
 }

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,

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)
 

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, &params)
 }
+
+func (k Keeper) DistrEpochIdentifier(ctx sdk.Context) (res string) {
+	k.paramSpace.Get(ctx, types.KeyDistrEpochIdentifier, &res)
+	return
+}

x/sequencer/module.go

@@ -101,24 +101,19 @@ 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(
 	cdc codec.Codec,
 	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) {

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 -------------------- */