From e2ac9743e9a309b29e6e92ab64be2e4e79503f75 Mon Sep 17 00:00:00 2001 From: Shawn <44221603+smarshall-spitzbart@users.noreply.github.com> Date: Thu, 8 Jun 2023 11:46:35 -0700 Subject: [PATCH] feat: Remove consumer genesis migration on provider (#997) * Update keys.go * tests * fix another bug * remove consumer genesis deletion, link to test * remove unused bond denom method * Revert "remove unused bond denom method" This reverts commit f930eca428bade49a05368fe5dae2d96842659cc. * remove test too * update changelog --- CHANGELOG.md | 4 ++-- x/ccv/provider/keeper/migration.go | 14 ++++---------- x/ccv/provider/keeper/migration_test.go | 19 ------------------- 3 files changed, 6 insertions(+), 31 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) 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 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()