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.

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

feat: v2 migrations #975

Merged
merged 25 commits into from
Jun 1, 2023
Merged
Show file tree
Hide file tree
Changes from 22 commits
Commits
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
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ Some PRs from v2.0.0 may reappear from other releases below. This is due to the

## 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)
* (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)
Expand Down
6 changes: 4 additions & 2 deletions app/consumer-democracy/app.go
Original file line number Diff line number Diff line change
Expand Up @@ -117,7 +117,7 @@ import (

const (
AppName = "interchain-security-cd"
upgradeName = "v07-Theta" // arbitrary name, define your own appropriately named upgrade
upgradeName = "ics-v1-to-v2" // arbitrary name, define your own appropriately named upgrade
AccountAddressPrefix = "cosmos"
)

Expand Down Expand Up @@ -441,7 +441,7 @@ func New(

// register slashing module StakingHooks to the consumer keeper
app.ConsumerKeeper = *app.ConsumerKeeper.SetHooks(app.SlashingKeeper.Hooks())
consumerModule := consumer.NewAppModule(app.ConsumerKeeper)
consumerModule := consumer.NewAppModule(app.ConsumerKeeper, app.GetSubspace(consumertypes.ModuleName))
Copy link
Contributor Author

Choose a reason for hiding this comment

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

we now have to pass in param subspace to the app module constructor so the subspace can be used for migrations


app.TransferKeeper = ibctransferkeeper.NewKeeper(
appCodec,
Expand Down Expand Up @@ -628,6 +628,8 @@ func New(
app.SetBeginBlocker(app.BeginBlocker)
app.SetEndBlocker(app.EndBlocker)

// Note this upgrade handler is just an example and may not be exactly what you need to implement.
// See https://docs.cosmos.network/v0.45/building-modules/upgrade.html
app.UpgradeKeeper.SetUpgradeHandler(
upgradeName,
func(ctx sdk.Context, _ upgradetypes.Plan, _ module.VersionMap) (module.VersionMap, error) {
Expand Down
6 changes: 4 additions & 2 deletions app/consumer/app.go
Original file line number Diff line number Diff line change
Expand Up @@ -90,7 +90,7 @@ import (

const (
AppName = "interchain-security-c"
upgradeName = "v07-Theta"
upgradeName = "ics-v1-to-v2"
AccountAddressPrefix = "cosmos"
)

Expand Down Expand Up @@ -343,7 +343,7 @@ func New(

// register slashing module Slashing hooks to the consumer keeper
app.ConsumerKeeper = *app.ConsumerKeeper.SetHooks(app.SlashingKeeper.Hooks())
consumerModule := ibcconsumer.NewAppModule(app.ConsumerKeeper)
consumerModule := ibcconsumer.NewAppModule(app.ConsumerKeeper, app.GetSubspace(ibcconsumertypes.ModuleName))

app.TransferKeeper = ibctransferkeeper.NewKeeper(
appCodec,
Expand Down Expand Up @@ -510,6 +510,8 @@ func New(
app.SetBeginBlocker(app.BeginBlocker)
app.SetEndBlocker(app.EndBlocker)

// Note this upgrade handler is just an example and may not be exactly what you need to implement.
// See https://docs.cosmos.network/v0.45/building-modules/upgrade.html
app.UpgradeKeeper.SetUpgradeHandler(
upgradeName,
func(ctx sdk.Context, _ upgradetypes.Plan, _ module.VersionMap) (module.VersionMap, error) {
Expand Down
6 changes: 4 additions & 2 deletions app/provider/app.go
Original file line number Diff line number Diff line change
Expand Up @@ -109,7 +109,7 @@ import (

const (
AppName = "interchain-security-p"
upgradeName = "v07-Theta"
upgradeName = "ics-v1-to-v2"
AccountAddressPrefix = "cosmos"
)

Expand Down Expand Up @@ -415,7 +415,7 @@ func New(
authtypes.FeeCollectorName,
)

providerModule := ibcprovider.NewAppModule(&app.ProviderKeeper)
providerModule := ibcprovider.NewAppModule(&app.ProviderKeeper, app.GetSubspace(providertypes.ModuleName))

// register the proposal types
govRouter := govtypes.NewRouter()
Expand Down Expand Up @@ -616,6 +616,8 @@ func New(
app.SetBeginBlocker(app.BeginBlocker)
app.SetEndBlocker(app.EndBlocker)

// Note this upgrade handler is just an example and may not be exactly what you need to implement.
// See https://docs.cosmos.network/v0.45/building-modules/upgrade.html
app.UpgradeKeeper.SetUpgradeHandler(
upgradeName,
func(ctx sdk.Context, _ upgradetypes.Plan, _ module.VersionMap) (module.VersionMap, error) {
Expand Down
30 changes: 18 additions & 12 deletions x/ccv/consumer/ibc_module_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -121,9 +121,10 @@ func TestOnChanOpenInit(t *testing.T) {
for _, tc := range testCases {

// Common setup
keeperParams := testkeeper.NewInMemKeeperParams(t)
consumerKeeper, ctx, ctrl, mocks := testkeeper.GetConsumerKeeperAndCtx(
t, testkeeper.NewInMemKeeperParams(t))
consumerModule := consumer.NewAppModule(consumerKeeper)
t, keeperParams)
consumerModule := consumer.NewAppModule(consumerKeeper, *keeperParams.ParamsSubspace)

consumerKeeper.SetPort(ctx, ccv.ConsumerPortID)
consumerKeeper.SetProviderClientID(ctx, "clientIDToProvider")
Expand Down Expand Up @@ -172,10 +173,11 @@ func TestOnChanOpenInit(t *testing.T) {
// See: https://github.com/cosmos/ibc/blob/main/spec/app/ics-028-cross-chain-validation/methods.md#ccv-ccf-cotry1
// Spec tag: [CCV-CCF-COTRY.1]
func TestOnChanOpenTry(t *testing.T) {
consumerKeeper, ctx, ctrl, _ := testkeeper.GetConsumerKeeperAndCtx(t, testkeeper.NewInMemKeeperParams(t))
keeperParams := testkeeper.NewInMemKeeperParams(t)
consumerKeeper, ctx, ctrl, _ := testkeeper.GetConsumerKeeperAndCtx(t, keeperParams)
// No external keeper methods should be called
defer ctrl.Finish()
consumerModule := consumer.NewAppModule(consumerKeeper)
consumerModule := consumer.NewAppModule(consumerKeeper, *keeperParams.ParamsSubspace)

// OnOpenTry must error even with correct arguments
_, err := consumerModule.OnChanOpenTry(
Expand Down Expand Up @@ -266,9 +268,10 @@ func TestOnChanOpenAck(t *testing.T) {

for _, tc := range testCases {
// Common setup
keeperParams := testkeeper.NewInMemKeeperParams(t)
consumerKeeper, ctx, ctrl, mocks := testkeeper.GetConsumerKeeperAndCtx(
t, testkeeper.NewInMemKeeperParams(t))
consumerModule := consumer.NewAppModule(consumerKeeper)
t, keeperParams)
consumerModule := consumer.NewAppModule(consumerKeeper, *keeperParams.ParamsSubspace)

// Instantiate valid params as default. Individual test cases mutate these as needed.
params := params{
Expand Down Expand Up @@ -316,9 +319,10 @@ func TestOnChanOpenAck(t *testing.T) {
// See: https://github.com/cosmos/ibc/blob/main/spec/app/ics-028-cross-chain-validation/methods.md#ccv-ccf-coconfirm1
// Spec tag: [CCV-CCF-COCONFIRM.1]
func TestOnChanOpenConfirm(t *testing.T) {
consumerKeeper, ctx, ctrl, _ := testkeeper.GetConsumerKeeperAndCtx(t, testkeeper.NewInMemKeeperParams(t))
keeperParams := testkeeper.NewInMemKeeperParams(t)
consumerKeeper, ctx, ctrl, _ := testkeeper.GetConsumerKeeperAndCtx(t, keeperParams)
defer ctrl.Finish()
consumerModule := consumer.NewAppModule(consumerKeeper)
consumerModule := consumer.NewAppModule(consumerKeeper, *keeperParams.ParamsSubspace)

err := consumerModule.OnChanOpenConfirm(ctx, ccv.ConsumerPortID, "channel-1")
require.Error(t, err, "OnChanOpenConfirm callback must error on consumer chain")
Expand Down Expand Up @@ -356,8 +360,9 @@ func TestOnChanCloseInit(t *testing.T) {
}

for _, tc := range testCases {
consumerKeeper, ctx, ctrl, _ := testkeeper.GetConsumerKeeperAndCtx(t, testkeeper.NewInMemKeeperParams(t))
consumerModule := consumer.NewAppModule(consumerKeeper)
keeperParams := testkeeper.NewInMemKeeperParams(t)
consumerKeeper, ctx, ctrl, _ := testkeeper.GetConsumerKeeperAndCtx(t, keeperParams)
consumerModule := consumer.NewAppModule(consumerKeeper, *keeperParams.ParamsSubspace)

if tc.establishedProviderExists {
consumerKeeper.SetProviderChannel(ctx, "provider")
Expand All @@ -379,12 +384,13 @@ func TestOnChanCloseInit(t *testing.T) {
// See: https://github.com/cosmos/ibc/blob/main/spec/app/ics-028-cross-chain-validation/methods.md#ccv-pcf-ccconfirm1// Spec tag: [CCV-CCF-CCINIT.1]
// Spec tag: [CCV-PCF-CCCONFIRM.1]
func TestOnChanCloseConfirm(t *testing.T) {
consumerKeeper, ctx, ctrl, _ := testkeeper.GetConsumerKeeperAndCtx(t, testkeeper.NewInMemKeeperParams(t))
keeperParams := testkeeper.NewInMemKeeperParams(t)
consumerKeeper, ctx, ctrl, _ := testkeeper.GetConsumerKeeperAndCtx(t, keeperParams)

// No external keeper methods should be called
defer ctrl.Finish()

consumerModule := consumer.NewAppModule(consumerKeeper)
consumerModule := consumer.NewAppModule(consumerKeeper, *keeperParams.ParamsSubspace)

// Nothing happens, no error returned
err := consumerModule.OnChanCloseConfirm(ctx, "portID", "channelID")
Expand Down
6 changes: 6 additions & 0 deletions x/ccv/consumer/keeper/keeper.go
Original file line number Diff line number Diff line change
Expand Up @@ -92,6 +92,12 @@ func (k *Keeper) SetStandaloneStakingKeeper(sk ccv.StakingKeeper) {
k.standaloneStakingKeeper = sk
}

// SetParamSpace sets the param space for the consumer keeper.
// Note: this is only used for testing!
func (k *Keeper) SetParamSpace(ctx sdk.Context, ps paramtypes.Subspace) {
k.paramStore = ps
}

// Validates that the consumer keeper is initialized with non-zero and
// non-nil values for all its fields. Otherwise this method will panic.
func (k Keeper) mustValidateFields() {
Expand Down
83 changes: 83 additions & 0 deletions x/ccv/consumer/keeper/migration.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,83 @@
package keeper

import (
"time"

sdk "github.com/cosmos/cosmos-sdk/types"
paramtypes "github.com/cosmos/cosmos-sdk/x/params/types"
consumertypes "github.com/cosmos/interchain-security/v2/x/ccv/consumer/types"
ccvtypes "github.com/cosmos/interchain-security/v2/x/ccv/types"
)

// Migrator is a struct for handling in-place store migrations.
type Migrator struct {
ccvConsumerKeeper Keeper
ccvConsumerParamSpace paramtypes.Subspace
}

// NewMigrator returns a new Migrator.
func NewMigrator(ccvConsumerKeeper Keeper, ccvConsumerParamSpace paramtypes.Subspace) Migrator {
return Migrator{ccvConsumerKeeper: ccvConsumerKeeper, ccvConsumerParamSpace: ccvConsumerParamSpace}
}

// Migratev1p2p0MultidenTov2 migrates a consumer from v1.2.0-multiden to v2.0.0.
func (m Migrator) Migratev1p2p0MultidenTov2(ctx sdk.Context) error {
Copy link
Contributor

Choose a reason for hiding this comment

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

Remove this function as is not needed. State migration cares about consensus versions and not semver. As both v1.2.0-multiden and v1.0.0 are on the same consensus version (i.e., 1), we just need Migratev1Tov2

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@mpoke see a235a1a. I think this is the best way to deal with consensus version for just the consumer. Moving forward we'll know to increment that number when migrations are required

Copy link
Contributor Author

Choose a reason for hiding this comment

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

But as far as your original comment, the methods were removed 👍

// Note: If migrating from v1.2.0-multiden to v2.0.0, there are no migrations required.
// This is due to the fact that the former version includes both of:
// - https://github.com/cosmos/interchain-security/commit/54e9852d3c89a2513cd0170a56c6eec894fc878d
// - https://github.com/cosmos/interchain-security/pull/833
// both of which handle the introduction of new params.
return nil
}

// Migratev1Tov2 migrates a consumer from v1.0.0 to v2.0.0.
func (m Migrator) Migratev1Tov2(ctx sdk.Context) error {
// Migrate params
MigrateParamsv1Tov2(ctx, m.ccvConsumerParamSpace)

return nil
}

// MigrateParamsv1Tov2 migrates the consumer CCV module params from v1.0.0 to v2.0.0,
// setting default values for new params.
func MigrateParamsv1Tov2(ctx sdk.Context, paramsSubspace paramtypes.Subspace) {
// Get old params
var enabled bool
paramsSubspace.Get(ctx, consumertypes.KeyEnabled, &enabled)
var blocksPerDistributionTransmission int64
paramsSubspace.Get(ctx, consumertypes.KeyBlocksPerDistributionTransmission, &blocksPerDistributionTransmission)
var distributionTransmissionChannel string
paramsSubspace.Get(ctx, consumertypes.KeyDistributionTransmissionChannel, &distributionTransmissionChannel)
var providerFeePoolAddrStr string
paramsSubspace.Get(ctx, consumertypes.KeyProviderFeePoolAddrStr, &providerFeePoolAddrStr)
var ccvTimeoutPeriod time.Duration
paramsSubspace.Get(ctx, ccvtypes.KeyCCVTimeoutPeriod, &ccvTimeoutPeriod)
var transferTimeoutPeriod time.Duration
paramsSubspace.Get(ctx, consumertypes.KeyTransferTimeoutPeriod, &transferTimeoutPeriod)
var consumerRedistributionFrac string
paramsSubspace.Get(ctx, consumertypes.KeyConsumerRedistributionFrac, &consumerRedistributionFrac)
var historicalEntries int64
paramsSubspace.Get(ctx, consumertypes.KeyHistoricalEntries, &historicalEntries)
var unbondingPeriod time.Duration
paramsSubspace.Get(ctx, consumertypes.KeyConsumerUnbondingPeriod, &unbondingPeriod)

// Recycle old params, set new params to default values
defaultParams := consumertypes.DefaultParams()
newParams := consumertypes.NewParams(
enabled,
blocksPerDistributionTransmission,
distributionTransmissionChannel,
providerFeePoolAddrStr,
ccvTimeoutPeriod,
transferTimeoutPeriod,
consumerRedistributionFrac,
historicalEntries,
unbondingPeriod,
defaultParams.SoftOptOutThreshold,
defaultParams.RewardDenoms,
defaultParams.ProviderRewardDenoms,
)

// Persist new params
paramsSubspace.SetParamSet(ctx, &newParams)
}
Loading