Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

feat: Remove consumer genesis migration on provider #997

Merged
merged 9 commits into from
Jun 8, 2023
Merged

feat: Remove consumer genesis migration on provider #997

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
ab372d6
Update keys.go
shaspitz Jun 2, 2023
88d7764
tests
shaspitz Jun 2, 2023
fa0914b
fix another bug
shaspitz Jun 2, 2023
69245ce
remove consumer genesis deletion, link to test
shaspitz Jun 7, 2023
f930eca
remove unused bond denom method
shaspitz Jun 7, 2023
484601f
Revert "remove unused bond denom method"
shaspitz Jun 7, 2023
25c3fb0
remove test too
shaspitz Jun 7, 2023
8499e73
Merge branch 'main' into shawn/cleanups-following-migration-audit
shaspitz Jun 8, 2023
22a17e6
update changelog
shaspitz Jun 8, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Updating changelog in this PR, including entry for #991

* (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)
Expand Down
14 changes: 4 additions & 10 deletions x/ccv/provider/keeper/migration.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -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
Expand Down
19 changes: 0 additions & 19 deletions x/ccv/provider/keeper/migration_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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()
Expand Down