From ab372d6371c1f5f2e8ffdb2461575bd5694d7953 Mon Sep 17 00:00:00 2001 From: Shawn <44221603+smarshall-spitzbart@users.noreply.github.com> Date: Fri, 2 Jun 2023 09:46:32 -0700 Subject: [PATCH 1/8] Update keys.go --- x/ccv/consumer/types/keys.go | 33 ++++++++++++++++++--------------- 1 file changed, 18 insertions(+), 15 deletions(-) diff --git a/x/ccv/consumer/types/keys.go b/x/ccv/consumer/types/keys.go index d52c9c551e..60f12e3da7 100644 --- a/x/ccv/consumer/types/keys.go +++ b/x/ccv/consumer/types/keys.go @@ -49,6 +49,24 @@ const ( // received over CCV channel but not yet flushed over ABCI PendingChangesByteKey + // PendingDataPacketsByteKey is the byte key for storing + // a list of data packets that cannot be sent yet to the provider + // chain either because the CCV channel is not established or + // because the client is expired + PendingDataPacketsByteKey + + // PreCCVByteKey is the byte to store the consumer is running on democracy staking module without consumer + PreCCVByteKey + + // InitialValSetByteKey is the byte to store the initial validator set for a consumer + InitialValSetByteKey + + // LastStandaloneHeightByteKey is the byte that will store last standalone height + LastStandaloneHeightByteKey + + // SmallestNonOptOutPowerByteKey is the byte that will store the smallest val power that cannot opt out + SmallestNonOptOutPowerByteKey + // HistoricalInfoKey is the byte prefix that will store the historical info for a given height HistoricalInfoBytePrefix @@ -67,24 +85,9 @@ const ( // CrossChainValidatorPrefix is the byte prefix that will store cross-chain validators by consensus address CrossChainValidatorBytePrefix - // PendingDataPacketsByteKey is the byte key for storing - // a list of data packets that cannot be sent yet to the provider - // chain either because the CCV channel is not established or - // because the client is expired - PendingDataPacketsByteKey - - // PreCCVByteKey is the byte to store the consumer is running on democracy staking module without consumer - PreCCVByteKey - - // InitialValSetByteKey is the byte to store the initial validator set for a consumer - InitialValSetByteKey - // InitGenesisHeightByteKey is the byte that will store the init genesis height InitGenesisHeightByteKey - // SmallestNonOptOutPowerByteKey is the byte that will store the smallest val power that cannot opt out - SmallestNonOptOutPowerByteKey - // StandaloneTransferChannelIDByteKey is the byte storing the channelID of transfer channel // that existed from a standalone chain changing over to a consumer StandaloneTransferChannelIDByteKey From 88d7764a74e191926a3975a0199b6c7dbe5e083e Mon Sep 17 00:00:00 2001 From: Shawn <44221603+smarshall-spitzbart@users.noreply.github.com> Date: Fri, 2 Jun 2023 09:52:29 -0700 Subject: [PATCH 2/8] tests --- x/ccv/consumer/types/keys.go | 2 +- x/ccv/consumer/types/keys_test.go | 20 ++++++++++++-------- 2 files changed, 13 insertions(+), 9 deletions(-) diff --git a/x/ccv/consumer/types/keys.go b/x/ccv/consumer/types/keys.go index 60f12e3da7..b26d9ee723 100644 --- a/x/ccv/consumer/types/keys.go +++ b/x/ccv/consumer/types/keys.go @@ -61,7 +61,7 @@ const ( // InitialValSetByteKey is the byte to store the initial validator set for a consumer InitialValSetByteKey - // LastStandaloneHeightByteKey is the byte that will store last standalone height + // NOTE: This prefix is depreciated, but left in place to avoid consumer state migrations LastStandaloneHeightByteKey // SmallestNonOptOutPowerByteKey is the byte that will store the smallest val power that cannot opt out diff --git a/x/ccv/consumer/types/keys_test.go b/x/ccv/consumer/types/keys_test.go index c0ab13391a..f594517438 100644 --- a/x/ccv/consumer/types/keys_test.go +++ b/x/ccv/consumer/types/keys_test.go @@ -27,16 +27,18 @@ func getAllKeyPrefixes() []byte { ProviderClientByteKey, ProviderChannelByteKey, PendingChangesByteKey, + PendingDataPacketsByteKey, + PreCCVByteKey, + InitialValSetByteKey, + LastStandaloneHeightByteKey, + SmallestNonOptOutPowerByteKey, HistoricalInfoBytePrefix, PacketMaturityTimeBytePrefix, HeightValsetUpdateIDBytePrefix, OutstandingDowntimeBytePrefix, + PendingDataPacketsBytePrefix, CrossChainValidatorBytePrefix, - PendingDataPacketsByteKey, - PreCCVByteKey, - InitialValSetByteKey, InitGenesisHeightByteKey, - SmallestNonOptOutPowerByteKey, StandaloneTransferChannelIDByteKey, PrevStandaloneChainByteKey, } @@ -61,16 +63,18 @@ func getAllFullyDefinedKeys() [][]byte { ProviderClientIDKey(), ProviderChannelKey(), PendingChangesKey(), + PendingDataPacketsKey(), + PreCCVKey(), + InitialValSetKey(), + // LastStandaloneHeightKey() is depreciated + SmallestNonOptOutPowerKey(), HistoricalInfoKey(0), PacketMaturityTimeKey(0, time.Time{}), HeightValsetUpdateIDKey(0), OutstandingDowntimeKey([]byte{}), + // PendingDataPacketsKey() does not use depreciated prefix CrossChainValidatorKey([]byte{}), - PendingDataPacketsKey(), - PreCCVKey(), - InitialValSetKey(), InitGenesisHeightKey(), - SmallestNonOptOutPowerKey(), StandaloneTransferChannelIDKey(), PrevStandaloneChainKey(), } From fa0914b691888971da677dc231b131268ecfa501 Mon Sep 17 00:00:00 2001 From: Shawn <44221603+smarshall-spitzbart@users.noreply.github.com> Date: Fri, 2 Jun 2023 15:05:44 -0700 Subject: [PATCH 3/8] fix another bug --- x/ccv/consumer/types/keys.go | 2 +- x/ccv/consumer/types/keys_test.go | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/x/ccv/consumer/types/keys.go b/x/ccv/consumer/types/keys.go index b26d9ee723..e1c2194f12 100644 --- a/x/ccv/consumer/types/keys.go +++ b/x/ccv/consumer/types/keys.go @@ -173,7 +173,7 @@ func CrossChainValidatorKey(addr []byte) []byte { // that cannot be sent yet to the provider chain either because the CCV channel // is not established or because the client is expired. func PendingDataPacketsKey() []byte { - return []byte{PendingDataPacketsByteKey} + return []byte{PendingDataPacketsBytePrefix} } func PreCCVKey() []byte { diff --git a/x/ccv/consumer/types/keys_test.go b/x/ccv/consumer/types/keys_test.go index f594517438..a63da6f326 100644 --- a/x/ccv/consumer/types/keys_test.go +++ b/x/ccv/consumer/types/keys_test.go @@ -63,7 +63,7 @@ func getAllFullyDefinedKeys() [][]byte { ProviderClientIDKey(), ProviderChannelKey(), PendingChangesKey(), - PendingDataPacketsKey(), + // PendingDataPacketsKey() does not use duplicated prefix with value of 0x06 PreCCVKey(), InitialValSetKey(), // LastStandaloneHeightKey() is depreciated @@ -72,7 +72,7 @@ func getAllFullyDefinedKeys() [][]byte { PacketMaturityTimeKey(0, time.Time{}), HeightValsetUpdateIDKey(0), OutstandingDowntimeKey([]byte{}), - // PendingDataPacketsKey() does not use depreciated prefix + PendingDataPacketsKey(), CrossChainValidatorKey([]byte{}), InitGenesisHeightKey(), StandaloneTransferChannelIDKey(), From 69245ce5b438c35d64f0b6d5363907cab3b92661 Mon Sep 17 00:00:00 2001 From: Shawn <44221603+smarshall-spitzbart@users.noreply.github.com> Date: Wed, 7 Jun 2023 15:37:40 -0700 Subject: [PATCH 4/8] remove consumer genesis deletion, link to test --- x/ccv/provider/keeper/migration.go | 14 ++++---------- 1 file changed, 4 insertions(+), 10 deletions(-) diff --git a/x/ccv/provider/keeper/migration.go b/x/ccv/provider/keeper/migration.go index 4e899bcce2..d33fcb7968 100644 --- a/x/ccv/provider/keeper/migration.go +++ b/x/ccv/provider/keeper/migration.go @@ -33,8 +33,10 @@ func (m Migrator) Migratev1Tov2(ctx sdk.Context) error { sdk.NewCoin(m.ccvProviderKeeper.BondDenom(ctx), sdk.NewInt(10000000)), ) - // Delete select consumer genesis states for consumers that're launched - MigrateConsumerGenesisStatesv1Tov2(ctx, m.ccvProviderKeeper) + // Consumer genesis states persisted on the provider do not need to be migrated, + // as protobuf serialization is able to gracefully handle unpopulated fields when deserializing. + // See https://github.com/smarshall-spitzbart/ics-migration-tests/commit/b589e3982c26783ed66e954051f7da1ead16de68 + // which passes, proving the addition of preCCV will not break things. // Migrate keys to accommodate fix from https://github.com/cosmos/interchain-security/pull/786 MigrateKeysv1Tov2(ctx, m.ccvProviderKeeper) @@ -80,14 +82,6 @@ func MigrateParamsv1Tov2(ctx sdk.Context, paramsSubspace paramtypes.Subspace, co paramsSubspace.SetParamSet(ctx, &newParams) } -func MigrateConsumerGenesisStatesv1Tov2(ctx sdk.Context, providerKeeper Keeper) { - // We could try to migrate existing ConsumerGenesisStates, but they're not needed after consumer launch. - // Hence we delete them strategically. - providerKeeper.DeleteConsumerGenesis(ctx, "neutron-1") // See https://github.com/neutron-org/mainnet-assets#parameters - - // TODO: determine if any other ConsumerGenesisStates need to be deleted, or actually migrated! -} - // Due to https://github.com/cosmos/interchain-security/pull/786, // validators' slash logs are stored under the key prefix for slash acks. // This method will extract "slash logs" from the slash acks part of the store, and put the slash logs From f930eca428bade49a05368fe5dae2d96842659cc Mon Sep 17 00:00:00 2001 From: Shawn <44221603+smarshall-spitzbart@users.noreply.github.com> Date: Wed, 7 Jun 2023 15:39:02 -0700 Subject: [PATCH 5/8] remove unused bond denom method --- x/ccv/provider/keeper/keeper.go | 4 ---- 1 file changed, 4 deletions(-) diff --git a/x/ccv/provider/keeper/keeper.go b/x/ccv/provider/keeper/keeper.go index 7494057643..cc0013d7dc 100644 --- a/x/ccv/provider/keeper/keeper.go +++ b/x/ccv/provider/keeper/keeper.go @@ -1060,7 +1060,3 @@ func (k Keeper) GetSlashLog( bz := store.Get(types.SlashLogKey(providerAddr)) return bz != nil } - -func (k Keeper) BondDenom(ctx sdk.Context) string { - return k.stakingKeeper.BondDenom(ctx) -} From 484601f3122b3ee42e23d943ab21fccee29c48b0 Mon Sep 17 00:00:00 2001 From: Shawn <44221603+smarshall-spitzbart@users.noreply.github.com> Date: Wed, 7 Jun 2023 15:39:54 -0700 Subject: [PATCH 6/8] Revert "remove unused bond denom method" This reverts commit f930eca428bade49a05368fe5dae2d96842659cc. --- x/ccv/provider/keeper/keeper.go | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/x/ccv/provider/keeper/keeper.go b/x/ccv/provider/keeper/keeper.go index cc0013d7dc..7494057643 100644 --- a/x/ccv/provider/keeper/keeper.go +++ b/x/ccv/provider/keeper/keeper.go @@ -1060,3 +1060,7 @@ func (k Keeper) GetSlashLog( bz := store.Get(types.SlashLogKey(providerAddr)) return bz != nil } + +func (k Keeper) BondDenom(ctx sdk.Context) string { + return k.stakingKeeper.BondDenom(ctx) +} From 25c3fb0465e70792501a7245d02dbe46b231148b Mon Sep 17 00:00:00 2001 From: Shawn <44221603+smarshall-spitzbart@users.noreply.github.com> Date: Wed, 7 Jun 2023 15:47:46 -0700 Subject: [PATCH 7/8] remove test too --- x/ccv/provider/keeper/migration_test.go | 19 ------------------- 1 file changed, 19 deletions(-) diff --git a/x/ccv/provider/keeper/migration_test.go b/x/ccv/provider/keeper/migration_test.go index 858aa193e6..e488dd2d36 100644 --- a/x/ccv/provider/keeper/migration_test.go +++ b/x/ccv/provider/keeper/migration_test.go @@ -13,7 +13,6 @@ import ( types2 "github.com/cosmos/ibc-go/v4/modules/light-clients/07-tendermint/types" "github.com/cosmos/interchain-security/v2/testutil/crypto" testutil "github.com/cosmos/interchain-security/v2/testutil/keeper" - consumertypes "github.com/cosmos/interchain-security/v2/x/ccv/consumer/types" providerkeeper "github.com/cosmos/interchain-security/v2/x/ccv/provider/keeper" providertypes "github.com/cosmos/interchain-security/v2/x/ccv/provider/types" ccvtypes "github.com/cosmos/interchain-security/v2/x/ccv/types" @@ -124,24 +123,6 @@ type v1Params struct { MaxThrottledPackets int64 `protobuf:"varint,8,opt,name=max_throttled_packets,json=maxThrottledPackets,proto3" json:"max_throttled_packets,omitempty"` } -func TestMigrateConsumerGenesisv1Tov2(t *testing.T) { - providerKeeper, ctx, ctrl, _ := testutil.GetProviderKeeperAndCtx(t, testutil.NewInMemKeeperParams(t)) - defer ctrl.Finish() - - _, found := providerKeeper.GetConsumerGenesis(ctx, "neutron-1") - require.False(t, found) - - providerKeeper.SetConsumerGenesis(ctx, "neutron-1", consumertypes.GenesisState{}) - - _, found = providerKeeper.GetConsumerGenesis(ctx, "neutron-1") - require.True(t, found) - - providerkeeper.MigrateConsumerGenesisStatesv1Tov2(ctx, providerKeeper) - - _, found = providerKeeper.GetConsumerGenesis(ctx, "neutron-1") - require.False(t, found) -} - func TestMigrateKeysv1Tov2(t *testing.T) { providerKeeper, ctx, ctrl, _ := testutil.GetProviderKeeperAndCtx(t, testutil.NewInMemKeeperParams(t)) defer ctrl.Finish() From 22a17e687a3a7af0e7ef4e52fd1debd1ff8131f8 Mon Sep 17 00:00:00 2001 From: Shawn <44221603+smarshall-spitzbart@users.noreply.github.com> Date: Thu, 8 Jun 2023 11:08:07 -0700 Subject: [PATCH 8/8] update changelog --- CHANGELOG.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 5f946901f2..d2b0b754e8 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -25,11 +25,11 @@ Some PRs from v2.0.0 may reappear from other releases below. This is due to the ## Notable PRs included in v2.0.0 -* (feat) v1->v2 migrations to accommodate a bugfix having to do with store keys, introduce new params, and deal with consumer genesis state schema changes [#975](https://github.com/cosmos/interchain-security/pull/975) +* (fix) cosumer key prefix order to avoid complex migrations [#963](https://github.com/cosmos/interchain-security/pull/963) and [#991](https://github.com/cosmos/interchain-security/pull/991). The latter PR is the proper fix. +* (feat) v1->v2 migrations to accommodate a bugfix having to do with store keys, introduce new params, and deal with consumer genesis state schema changes [#975](https://github.com/cosmos/interchain-security/pull/975) and [#997](https://github.com/cosmos/interchain-security/pull/997) * (deps) Bump github.com/cosmos/ibc-go/v4 from 4.4.0 to 4.4.2 [#982](https://github.com/cosmos/interchain-security/pull/982) * (fix) partially revert key assignment type safety PR [#980](https://github.com/cosmos/interchain-security/pull/980) * (fix) Remove panics on failure to send IBC packets [#876](https://github.com/cosmos/interchain-security/pull/876) -* (fix) consumer key prefix order to avoid complex migrations [#963](https://github.com/cosmos/interchain-security/pull/963) * (fix) Prevent denom DOS [#931](https://github.com/cosmos/interchain-security/pull/931) * (fix) multisig for assigning consumer key, use json [#916](https://github.com/cosmos/interchain-security/pull/916) * (deps) Bump github.com/cosmos/ibc-go/v4 from 4.3.0 to 4.4.0 [#902](https://github.com/cosmos/interchain-security/pull/902)