From 7b504c4cace5ec7063fae08650610cde83dac2bc Mon Sep 17 00:00:00 2001 From: MSalopek Date: Thu, 21 Dec 2023 13:07:42 +0100 Subject: [PATCH 1/2] chore: update migration handling for provider consensus v2->v3 --- .../{migration.go => throttle_legacy.go} | 48 ++++--------------- x/ccv/provider/migrations/migrator.go | 32 +++++++++++++ .../v3}/migration_test.go | 36 +++++++------- x/ccv/provider/migrations/v3/migrations.go | 25 ++++++++++ x/ccv/provider/module.go | 3 +- 5 files changed, 87 insertions(+), 57 deletions(-) rename x/ccv/provider/keeper/{migration.go => throttle_legacy.go} (66%) create mode 100644 x/ccv/provider/migrations/migrator.go rename x/ccv/provider/{keeper => migrations/v3}/migration_test.go (63%) create mode 100644 x/ccv/provider/migrations/v3/migrations.go diff --git a/x/ccv/provider/keeper/migration.go b/x/ccv/provider/keeper/throttle_legacy.go similarity index 66% rename from x/ccv/provider/keeper/migration.go rename to x/ccv/provider/keeper/throttle_legacy.go index 5d739c8bef..331f303828 100644 --- a/x/ccv/provider/keeper/migration.go +++ b/x/ccv/provider/keeper/throttle_legacy.go @@ -4,42 +4,11 @@ import ( "fmt" sdktypes "github.com/cosmos/cosmos-sdk/types" - paramtypes "github.com/cosmos/cosmos-sdk/x/params/types" providertypes "github.com/cosmos/interchain-security/v3/x/ccv/provider/types" ccvtypes "github.com/cosmos/interchain-security/v3/x/ccv/types" ) -// Migrator is a struct for handling in-place store migrations. -type Migrator struct { - providerKeeper Keeper - paramSpace paramtypes.Subspace -} - -// NewMigrator returns a new Migrator. -func NewMigrator(providerKeeper Keeper, paramSpace paramtypes.Subspace) Migrator { - return Migrator{providerKeeper: providerKeeper, paramSpace: paramSpace} -} - -// Migrate2to3 migrates x/ccvprovider state from consensus version 2 to 3. -func (m Migrator) Migrate2to3(ctx sdktypes.Context) error { - return m.providerKeeper.MigrateQueuedPackets(ctx) -} - -func (k Keeper) MigrateQueuedPackets(ctx sdktypes.Context) error { - for _, consumer := range k.GetAllConsumerChains(ctx) { - slashData, vscmData := k.GetAllThrottledPacketData(ctx, consumer.ChainId) - if len(slashData) > 0 { - k.Logger(ctx).Error(fmt.Sprintf("slash data being dropped: %v", slashData)) - } - for _, data := range vscmData { - k.HandleVSCMaturedPacket(ctx, consumer.ChainId, data) - } - k.DeleteThrottledPacketDataForConsumer(ctx, consumer.ChainId) - } - return nil -} - // Pending packet data type enum, used to encode the type of packet data stored at each entry in the mutual queue. // Note this type is copy/pasted from throttle v1 code. const ( @@ -47,9 +16,9 @@ const ( vscMaturedPacketData ) -// GetAllThrottledPacketData returns all throttled packet data for a given consumer chain, only used for 2 -> 3 migration. -// Note this method is adapted from throttle v1 code. -func (k Keeper) GetAllThrottledPacketData(ctx sdktypes.Context, consumerChainID string) ( +// Deprecated: LegacyGetAllThrottledPacketData is deprecated is deprecated for ICS >= v4.0.0. +// LegacyGetAllThrottledPacketData returns all throttled packet data that was queued on the provider for a given consumer chain. +func (k Keeper) LegacyGetAllThrottledPacketData(ctx sdktypes.Context, consumerChainID string) ( slashData []ccvtypes.SlashPacketData, vscMaturedData []ccvtypes.VSCMaturedPacketData, ) { slashData = []ccvtypes.SlashPacketData{} @@ -86,8 +55,9 @@ func (k Keeper) GetAllThrottledPacketData(ctx sdktypes.Context, consumerChainID return slashData, vscMaturedData } -// Note this method is copy/pasted from throttle v1 code. -func (k Keeper) DeleteThrottledPacketDataForConsumer(ctx sdktypes.Context, consumerChainID string) { +// Deprecated: LegacyDeleteThrottledPacketDataForConsumer is deprecated for ICS >= v4.0.0. +// LegacyDeleteThrottledPacketDataForConsumer removes all throttled packet data that was queued on the provider for a given consumer chain. +func (k Keeper) LegacyDeleteThrottledPacketDataForConsumer(ctx sdktypes.Context, consumerChainID string) { store := ctx.KVStore(k.storeKey) iteratorPrefix := providertypes.ChainIdWithLenKey(providertypes.ThrottledPacketDataBytePrefix, consumerChainID) iterator := sdktypes.KVStorePrefixIterator(store, iteratorPrefix) @@ -106,8 +76,10 @@ func (k Keeper) DeleteThrottledPacketDataForConsumer(ctx sdktypes.Context, consu store.Delete(providertypes.ThrottledPacketDataSizeKey(consumerChainID)) } -// Note this method is adapted from throttle v1 code. -func (k Keeper) QueueThrottledPacketDataOnlyForTesting( +// Deprecated: LegacyQueueThrottledPacketData is deprecated for ICS >= v4.0.0. +// LegacyQueueThrottledPacketData queues throttled packet data for a given consumer chain on the provider. +// The method should not be used becase the provider does not process throttled packet data anymore. +func (k Keeper) LegacyQueueThrottledPacketData( ctx sdktypes.Context, consumerChainID string, ibcSeqNum uint64, packetData interface{}, ) error { store := ctx.KVStore(k.storeKey) diff --git a/x/ccv/provider/migrations/migrator.go b/x/ccv/provider/migrations/migrator.go new file mode 100644 index 0000000000..6669d1deb9 --- /dev/null +++ b/x/ccv/provider/migrations/migrator.go @@ -0,0 +1,32 @@ +package migrations + +import ( + sdktypes "github.com/cosmos/cosmos-sdk/types" + paramtypes "github.com/cosmos/cosmos-sdk/x/params/types" + + providerkeeper "github.com/cosmos/interchain-security/v3/x/ccv/provider/keeper" + v3 "github.com/cosmos/interchain-security/v3/x/ccv/provider/migrations/v3" +) + +// Migrator is a struct for handling in-place store migrations. +type Migrator struct { + providerKeeper providerkeeper.Keeper + paramSpace paramtypes.Subspace +} + +// NewMigrator returns a new Migrator. +func NewMigrator(providerKeeper providerkeeper.Keeper, paramSpace paramtypes.Subspace) Migrator { + return Migrator{providerKeeper: providerKeeper, paramSpace: paramSpace} +} + +// Migrating consensus version 1 to 2 is a no-op. +// v2 -> v3 will require multiple state breaking changes and migrations. +// First run provider@v2.x.y in production to migrate from v1 -> v2. Then use provider@v3.x.y to migrate from v2 -> v3. +func (m Migrator) Migrate1to2(ctx sdktypes.Context) error { + return nil +} + +// Migrate2to3 migrates x/ccvprovider state from consensus version 2 to 3. +func (m Migrator) Migrate2to3(ctx sdktypes.Context) error { + return v3.MigrateQueuedPackets(ctx, m.providerKeeper) +} diff --git a/x/ccv/provider/keeper/migration_test.go b/x/ccv/provider/migrations/v3/migration_test.go similarity index 63% rename from x/ccv/provider/keeper/migration_test.go rename to x/ccv/provider/migrations/v3/migration_test.go index a710e3979c..d82fb3c904 100644 --- a/x/ccv/provider/keeper/migration_test.go +++ b/x/ccv/provider/migrations/v3/migration_test.go @@ -1,4 +1,4 @@ -package keeper_test +package v3 import ( "testing" @@ -19,41 +19,41 @@ func TestMigrate2To3(t *testing.T) { providerKeeper.SetConsumerClientId(ctx, "chain-3", "client-3") // Queue some data for chain-1 - providerKeeper.QueueThrottledPacketDataOnlyForTesting( + providerKeeper.LegacyQueueThrottledPacketData( //nolint:staticcheck // SA1019: used in migration tests ctx, "chain-1", 66, testutil.GetNewSlashPacketData()) - providerKeeper.QueueThrottledPacketDataOnlyForTesting( + providerKeeper.LegacyQueueThrottledPacketData( //nolint:staticcheck // SA1019: used in migration tests ctx, "chain-1", 67, testutil.GetNewVSCMaturedPacketData()) - providerKeeper.QueueThrottledPacketDataOnlyForTesting( + providerKeeper.LegacyQueueThrottledPacketData( //nolint:staticcheck // SA1019: used in migration tests ctx, "chain-1", 68, testutil.GetNewSlashPacketData()) - providerKeeper.QueueThrottledPacketDataOnlyForTesting( + providerKeeper.LegacyQueueThrottledPacketData( //nolint:staticcheck // SA1019: used in migration tests ctx, "chain-1", 69, testutil.GetNewVSCMaturedPacketData()) // Queue some data for chain-2 - providerKeeper.QueueThrottledPacketDataOnlyForTesting( + providerKeeper.LegacyQueueThrottledPacketData( //nolint:staticcheck // SA1019: used in migration tests ctx, "chain-2", 789, testutil.GetNewVSCMaturedPacketData()) - providerKeeper.QueueThrottledPacketDataOnlyForTesting( + providerKeeper.LegacyQueueThrottledPacketData( //nolint:staticcheck // SA1019: used in migration tests ctx, "chain-2", 790, testutil.GetNewSlashPacketData()) - providerKeeper.QueueThrottledPacketDataOnlyForTesting( + providerKeeper.LegacyQueueThrottledPacketData( //nolint:staticcheck // SA1019: used in migration tests ctx, "chain-2", 791, testutil.GetNewVSCMaturedPacketData()) // Queue some data for chain-3 - providerKeeper.QueueThrottledPacketDataOnlyForTesting( + providerKeeper.LegacyQueueThrottledPacketData( //nolint:staticcheck // SA1019: used in migration tests ctx, "chain-3", 123, testutil.GetNewSlashPacketData()) - providerKeeper.QueueThrottledPacketDataOnlyForTesting( + providerKeeper.LegacyQueueThrottledPacketData( //nolint:staticcheck // SA1019: used in migration tests ctx, "chain-3", 124, testutil.GetNewVSCMaturedPacketData()) - providerKeeper.QueueThrottledPacketDataOnlyForTesting( + providerKeeper.LegacyQueueThrottledPacketData( //nolint:staticcheck // SA1019: used in migration tests ctx, "chain-3", 125, testutil.GetNewVSCMaturedPacketData()) // Confirm getter methods return expected values - slash1, vscm1 := providerKeeper.GetAllThrottledPacketData(ctx, "chain-1") + slash1, vscm1 := providerKeeper.LegacyGetAllThrottledPacketData(ctx, "chain-1") //nolint:staticcheck // SA1019: used in migration tests require.Len(t, slash1, 2) require.Len(t, vscm1, 2) - slash2, vscm2 := providerKeeper.GetAllThrottledPacketData(ctx, "chain-2") + slash2, vscm2 := providerKeeper.LegacyGetAllThrottledPacketData(ctx, "chain-2") //nolint:staticcheck // SA1019: used in migration tests require.Len(t, slash2, 1) require.Len(t, vscm2, 2) - slash3, vscm3 := providerKeeper.GetAllThrottledPacketData(ctx, "chain-3") + slash3, vscm3 := providerKeeper.LegacyGetAllThrottledPacketData(ctx, "chain-3") //nolint:staticcheck // SA1019: used in migration tests require.Len(t, slash3, 1) require.Len(t, vscm3, 2) @@ -87,17 +87,17 @@ func TestMigrate2To3(t *testing.T) { } // Run migration - err := providerKeeper.MigrateQueuedPackets(ctx) + err := MigrateQueuedPackets(ctx, providerKeeper) require.NoError(t, err) // Confirm throttled data is now deleted - slash1, vscm1 = providerKeeper.GetAllThrottledPacketData(ctx, "chain-1") + slash1, vscm1 = providerKeeper.LegacyGetAllThrottledPacketData(ctx, "chain-1") //nolint:staticcheck // SA1019: used in migration tests require.Empty(t, slash1) require.Empty(t, vscm1) - slash2, vscm2 = providerKeeper.GetAllThrottledPacketData(ctx, "chain-2") + slash2, vscm2 = providerKeeper.LegacyGetAllThrottledPacketData(ctx, "chain-2") //nolint:staticcheck // SA1019: used in migration tests require.Empty(t, slash2) require.Empty(t, vscm2) - slash3, vscm3 = providerKeeper.GetAllThrottledPacketData(ctx, "chain-3") + slash3, vscm3 = providerKeeper.LegacyGetAllThrottledPacketData(ctx, "chain-3") //nolint:staticcheck // SA1019: used in migration tests require.Empty(t, slash3) require.Empty(t, vscm3) diff --git a/x/ccv/provider/migrations/v3/migrations.go b/x/ccv/provider/migrations/v3/migrations.go new file mode 100644 index 0000000000..0022d4584a --- /dev/null +++ b/x/ccv/provider/migrations/v3/migrations.go @@ -0,0 +1,25 @@ +package v3 + +import ( + "fmt" + + sdk "github.com/cosmos/cosmos-sdk/types" + + providerkeeper "github.com/cosmos/interchain-security/v3/x/ccv/provider/keeper" +) + +// MigrateQueuedPackets processes all queued packet data for all consumer chains that were stored +// on the provider in the v2 consensus version (jail throttling v1). +func MigrateQueuedPackets(ctx sdk.Context, k providerkeeper.Keeper) error { + for _, consumer := range k.GetAllConsumerChains(ctx) { + slashData, vscmData := k.LegacyGetAllThrottledPacketData(ctx, consumer.ChainId) //nolint:staticcheck // SA1019: function used for migration + if len(slashData) > 0 { + k.Logger(ctx).Error(fmt.Sprintf("slash data being dropped: %v", slashData)) + } + for _, data := range vscmData { + k.HandleVSCMaturedPacket(ctx, consumer.ChainId, data) + } + k.LegacyDeleteThrottledPacketDataForConsumer(ctx, consumer.ChainId) //nolint:staticcheck // SA1019: function used for migration + } + return nil +} diff --git a/x/ccv/provider/module.go b/x/ccv/provider/module.go index c70fdeafa7..a4a96fc518 100644 --- a/x/ccv/provider/module.go +++ b/x/ccv/provider/module.go @@ -21,6 +21,7 @@ import ( "github.com/cosmos/interchain-security/v3/x/ccv/provider/client/cli" "github.com/cosmos/interchain-security/v3/x/ccv/provider/keeper" + "github.com/cosmos/interchain-security/v3/x/ccv/provider/migrations" providertypes "github.com/cosmos/interchain-security/v3/x/ccv/provider/types" ) @@ -107,7 +108,7 @@ func (AppModule) RegisterInvariants(ir sdk.InvariantRegistry) { func (am AppModule) RegisterServices(cfg module.Configurator) { providertypes.RegisterMsgServer(cfg.MsgServer(), keeper.NewMsgServerImpl(am.keeper)) providertypes.RegisterQueryServer(cfg.QueryServer(), am.keeper) - m := keeper.NewMigrator(*am.keeper, am.paramSpace) + m := migrations.NewMigrator(*am.keeper, am.paramSpace) if err := cfg.RegisterMigration(providertypes.ModuleName, 2, m.Migrate2to3); err != nil { panic(fmt.Sprintf("failed to register migrator for %s: %s", providertypes.ModuleName, err)) } From 0c75540bdef584a74c8653cc3b3f408ba7d3544c Mon Sep 17 00:00:00 2001 From: MSalopek Date: Thu, 21 Dec 2023 14:23:34 +0100 Subject: [PATCH 2/2] fix typos and nits --- x/ccv/provider/keeper/throttle_legacy.go | 2 +- x/ccv/provider/migrations/migrator.go | 5 +++-- 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/x/ccv/provider/keeper/throttle_legacy.go b/x/ccv/provider/keeper/throttle_legacy.go index 331f303828..44dd89905a 100644 --- a/x/ccv/provider/keeper/throttle_legacy.go +++ b/x/ccv/provider/keeper/throttle_legacy.go @@ -16,7 +16,7 @@ const ( vscMaturedPacketData ) -// Deprecated: LegacyGetAllThrottledPacketData is deprecated is deprecated for ICS >= v4.0.0. +// Deprecated: LegacyGetAllThrottledPacketData is deprecated for ICS >= v4.0.0. // LegacyGetAllThrottledPacketData returns all throttled packet data that was queued on the provider for a given consumer chain. func (k Keeper) LegacyGetAllThrottledPacketData(ctx sdktypes.Context, consumerChainID string) ( slashData []ccvtypes.SlashPacketData, vscMaturedData []ccvtypes.VSCMaturedPacketData, diff --git a/x/ccv/provider/migrations/migrator.go b/x/ccv/provider/migrations/migrator.go index 6669d1deb9..e4ea1d333a 100644 --- a/x/ccv/provider/migrations/migrator.go +++ b/x/ccv/provider/migrations/migrator.go @@ -20,8 +20,9 @@ func NewMigrator(providerKeeper providerkeeper.Keeper, paramSpace paramtypes.Sub } // Migrating consensus version 1 to 2 is a no-op. -// v2 -> v3 will require multiple state breaking changes and migrations. -// First run provider@v2.x.y in production to migrate from v1 -> v2. Then use provider@v3.x.y to migrate from v2 -> v3. +// Migrating from v1 -> v2 -> v3 will require multiple state breaking changes and migrations. +// First run provider@v2.x.y in production to migrate from consensus version 1 to 2. +// Then, in order to migrate to consensus version 3, first upgrade to provider@v3.x.y. func (m Migrator) Migrate1to2(ctx sdktypes.Context) error { return nil }