From e77b2a8bc213fe08804618706eb438f6616527d9 Mon Sep 17 00:00:00 2001 From: Damian Nolan Date: Fri, 15 Jul 2022 22:23:55 +0100 Subject: [PATCH] chore: adding upgrade handler for 09-localhost removal (#1671) * adding localhost migration code with tests * updating test cases * renaming test func * updating migration docs and moving to v4-to-v5.md * fixing indentation of code snippet * Update docs/migrations/v4-to-v5.md * adding sample query check for localhost client to docs * updating changelog to provide info on localhost upgrade handler * updating migrations docs with nit * renaming upgrades to migrations * updating function namings, tests and docs * Update CHANGELOG.md Co-authored-by: Carlos Rodriguez Co-authored-by: Carlos Rodriguez --- CHANGELOG.md | 2 +- docs/migrations/v3-to-v4.md | 46 +------ docs/migrations/v4-to-v5.md | 101 +++++++++++++++ modules/core/migrations/v5/migrations.go | 44 +++++++ modules/core/migrations/v5/migrations_test.go | 116 ++++++++++++++++++ 5 files changed, 263 insertions(+), 46 deletions(-) create mode 100644 docs/migrations/v4-to-v5.md create mode 100644 modules/core/migrations/v5/migrations.go create mode 100644 modules/core/migrations/v5/migrations_test.go diff --git a/CHANGELOG.md b/CHANGELOG.md index e9c0c464864..db0f3cdace0 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -57,7 +57,7 @@ Ref: https://keepachangelog.com/en/1.0.0/ * (modules/light-clients/07-tendermint) [\#1118](https://github.com/cosmos/ibc-go/pull/1118) Deprecating `AllowUpdateAfterExpiry and AllowUpdateAfterMisbehaviour`. See ADR-026 for context. * (modules/core/02-client) [\#1188](https://github.com/cosmos/ibc-go/pull/1188/files) Routing `MsgSubmitMisbehaviour` to `UpdateClient` keeper function. Deprecating `SubmitMisbehaviour` endpoint. * (modules/core/02-client) [\#1208](https://github.com/cosmos/ibc-go/pull/1208) Replace `CheckHeaderAndUpdateState` usage in 02-client with calls to `VerifyClientMessage`, `CheckForMisbehaviour`, `UpdateStateOnMisbehaviour` and `UpdateState`. -* (modules/light-clients/09-localhost) [\#1187](https://github.com/cosmos/ibc-go/pull/1187/) Removing localhost light client implementation as it is not functional. +* (modules/light-clients/09-localhost) [\#1187](https://github.com/cosmos/ibc-go/pull/1187/) Removing localhost light client implementation as it is not functional. An upgrade handler is provided in `modules/migrations/v5` to prune `09-localhost` clients and consensus states from the store. * [\#1186](https://github.com/cosmos/ibc-go/pull/1186/files) Removing `GetRoot` function from ConsensusState interface in `02-client`. `GetRoot` is unused by core IBC. * (modules/core/02-client) [\#1196](https://github.com/cosmos/ibc-go/pull/1196) Adding VerifyClientMessage to ClientState interface. * (modules/core/02-client) [\#1198](https://github.com/cosmos/ibc-go/pull/1198) Adding UpdateStateOnMisbehaviour to ClientState interface. diff --git a/docs/migrations/v3-to-v4.md b/docs/migrations/v3-to-v4.md index f558e25ab60..df7dedabbae 100644 --- a/docs/migrations/v3-to-v4.md +++ b/docs/migrations/v3-to-v4.md @@ -1,4 +1,4 @@ -# Migrating from ibc-go v2 to v3 +# Migrating from ibc-go v3 to v4 This document is intended to highlight significant changes which may require more information than presented in the CHANGELOG. Any changes that must be done by a user of ibc-go should be documented here. @@ -24,47 +24,3 @@ The `WriteAcknowledgement` API now takes the `exported.Acknowledgement` type ins This is an API breaking change and as such IBC application developers will have to update any calls to `WriteAcknowledgement`. ## IBC Light Clients - -### `ClientState` interface changes - -The `VerifyUpgradeAndUpdateState` function has been modified. The client state and consensus state return values have been removed. - -Light clients **must** handle all management of client and consensus states including the setting of updated client state and consensus state in the client store. - -The `CheckHeaderAndUpdateState` function has been split into 4 new functions: - -- `VerifyClientMessage` verifies a `ClientMessage`. A `ClientMessage` could be a `Header`, `Misbehaviour`, or batch update. Calls to `CheckForMisbehaviour`, `UpdateState`, and `UpdateStateOnMisbehaviour` will assume that the content of the `ClientMessage` has been verified and can be trusted. An error should be returned if the `ClientMessage` fails to verify. - -- `CheckForMisbehaviour` checks for evidence of a misbehaviour in `Header` or `Misbehaviour` types. - -- `UpdateStateOnMisbehaviour` performs appropriate state changes on a `ClientState` given that misbehaviour has been detected and verified. - -- `UpdateState` updates and stores as necessary any associated information for an IBC client, such as the `ClientState` and corresponding `ConsensusState`. An error is returned if `ClientMessage` is of type `Misbehaviour`. Upon successful update, a list containing the updated consensus state height is returned. - -The `CheckMisbehaviourAndUpdateState` function has been removed from `ClientState` interface. This functionality is now encapsulated by the usage of `VerifyClientMessage`, `CheckForMisbehaviour`, `UpdateStateOnMisbehaviour`, `UpdateState`. - -The function `GetTimestampAtHeight` has been added to the `ClientState` interface. It should return the timestamp for a consensus state associated with the provided height. - -### `Header` and `Misbehaviour` - -`exported.Header` and `exported.Misbehaviour` interface types have been merged and renamed to `ClientMessage` interface. - -`GetHeight` function has been removed from `exported.Header` and thus is not included in the `ClientMessage` interface - -### `ConsensusState` - -The `GetRoot` function has been removed from consensus state interface since it was not used by core IBC. - -### Light client implementations - -09-localhost light client implementation has been removed because it is currently non-functional. - -### Client Keeper - -Keeper function `CheckMisbehaviourAndUpdateState` has been removed since function `UpdateClient` can now handle updating `ClientState` on `ClientMessage` type which can be any `Misbehaviour` implementations. - -### SDK Message - -`MsgSubmitMisbehaviour` is deprecated since `MsgUpdateClient` can now submit a `ClientMessage` type which can be any `Misbehaviour` implementations. - -The field `header` in `MsgUpdateClient` has been renamed to `client message`. diff --git a/docs/migrations/v4-to-v5.md b/docs/migrations/v4-to-v5.md new file mode 100644 index 00000000000..a02e953f984 --- /dev/null +++ b/docs/migrations/v4-to-v5.md @@ -0,0 +1,101 @@ +# Migrating from ibc-go v4 to v5 + +This document is intended to highlight significant changes which may require more information than presented in the CHANGELOG. +Any changes that must be done by a user of ibc-go should be documented here. + +There are four sections based on the four potential user groups of this document: +- Chains +- IBC Apps +- Relayers +- IBC Light Clients + +**Note:** ibc-go supports golang semantic versioning and therefore all imports must be updated to bump the version number on major releases. + +## Chains + +- No relevant changes were made in this release. + +## IBC Apps + +- No relevant changes were made in this release. + +## Relayers + +- No relevant changes were made in this release. + +## IBC Light Clients + +### `ClientState` interface changes + +The `VerifyUpgradeAndUpdateState` function has been modified. The client state and consensus state return values have been removed. + +Light clients **must** handle all management of client and consensus states including the setting of updated client state and consensus state in the client store. + +The `CheckHeaderAndUpdateState` function has been split into 4 new functions: + +- `VerifyClientMessage` verifies a `ClientMessage`. A `ClientMessage` could be a `Header`, `Misbehaviour`, or batch update. Calls to `CheckForMisbehaviour`, `UpdateState`, and `UpdateStateOnMisbehaviour` will assume that the content of the `ClientMessage` has been verified and can be trusted. An error should be returned if the `ClientMessage` fails to verify. + +- `CheckForMisbehaviour` checks for evidence of a misbehaviour in `Header` or `Misbehaviour` types. + +- `UpdateStateOnMisbehaviour` performs appropriate state changes on a `ClientState` given that misbehaviour has been detected and verified. + +- `UpdateState` updates and stores as necessary any associated information for an IBC client, such as the `ClientState` and corresponding `ConsensusState`. An error is returned if `ClientMessage` is of type `Misbehaviour`. Upon successful update, a list containing the updated consensus state height is returned. + +The `CheckMisbehaviourAndUpdateState` function has been removed from `ClientState` interface. This functionality is now encapsulated by the usage of `VerifyClientMessage`, `CheckForMisbehaviour`, `UpdateStateOnMisbehaviour`, `UpdateState`. + +The function `GetTimestampAtHeight` has been added to the `ClientState` interface. It should return the timestamp for a consensus state associated with the provided height. + +### `Header` and `Misbehaviour` + +`exported.Header` and `exported.Misbehaviour` interface types have been merged and renamed to `ClientMessage` interface. + +`GetHeight` function has been removed from `exported.Header` and thus is not included in the `ClientMessage` interface + +### `ConsensusState` + +The `GetRoot` function has been removed from consensus state interface since it was not used by core IBC. + +### Light client implementations + +The `09-localhost` light client implementation has been removed because it is currently non-functional. + +An upgrade handler has been added to supply chain developers with the logic needed to prune the ibc client store and successfully complete the removal of `09-localhost`. +Add the following to the application upgrade handler in `app/app.go`, calling `MigrateToV5` to perform store migration logic. + +```go +import ( + // ... + ibcv5 "github.com/cosmos/ibc-go/v5/modules/core/migrations/v5" +) + +// ... + +app.UpgradeKeeper.SetUpgradeHandler( + upgradeName, + func(ctx sdk.Context, _ upgradetypes.Plan, _ module.VersionMap) (module.VersionMap, error) { + // prune the 09-localhost client from the ibc client store + ibcv5.MigrateToV5(ctx, app.IBCKeeper.ClientKeeper) + + return app.mm.RunMigrations(ctx, app.configurator, fromVM) + }, +) +``` + +Please note the above upgrade handler is optional and should only be run if chains have an existing `09-localhost` client stored in state. +A simple query can be performed to check for a `09-localhost` client on chain. + +For example: + +``` +simd query ibc client states | grep 09-localhost +``` + +### Client Keeper + +Keeper function `CheckMisbehaviourAndUpdateState` has been removed since function `UpdateClient` can now handle updating `ClientState` on `ClientMessage` type which can be any `Misbehaviour` implementations. + +### SDK Message + +`MsgSubmitMisbehaviour` is deprecated since `MsgUpdateClient` can now submit a `ClientMessage` type which can be any `Misbehaviour` implementations. + +The field `header` in `MsgUpdateClient` has been renamed to `client_message`. diff --git a/modules/core/migrations/v5/migrations.go b/modules/core/migrations/v5/migrations.go new file mode 100644 index 00000000000..c08992cf7d3 --- /dev/null +++ b/modules/core/migrations/v5/migrations.go @@ -0,0 +1,44 @@ +package v5 + +import ( + "strings" + + sdk "github.com/cosmos/cosmos-sdk/types" + + clientkeeper "github.com/cosmos/ibc-go/v3/modules/core/02-client/keeper" + clienttypes "github.com/cosmos/ibc-go/v3/modules/core/02-client/types" + host "github.com/cosmos/ibc-go/v3/modules/core/24-host" + "github.com/cosmos/ibc-go/v3/modules/core/exported" +) + +// Localhost is the client type for a localhost client. It is also used as the clientID +// for the localhost client. +const Localhost string = "09-localhost" + +// MigrateToV5 prunes the 09-Localhost client and associated consensus states from the ibc store +func MigrateToV5(ctx sdk.Context, clientKeeper clientkeeper.Keeper) { + clientStore := clientKeeper.ClientStore(ctx, Localhost) + + iterator := sdk.KVStorePrefixIterator(clientStore, []byte(host.KeyConsensusStatePrefix)) + var heights []exported.Height + + defer iterator.Close() + for ; iterator.Valid(); iterator.Next() { + keySplit := strings.Split(string(iterator.Key()), "/") + // key is in the format "consensusStates/" + if len(keySplit) != 2 || keySplit[0] != string(host.KeyConsensusStatePrefix) { + continue + } + + // collect consensus states to be pruned + heights = append(heights, clienttypes.MustParseHeight(keySplit[1])) + } + + // delete all consensus states + for _, height := range heights { + clientStore.Delete(host.ConsensusStateKey(height)) + } + + // delete the client state + clientStore.Delete(host.ClientStateKey()) +} diff --git a/modules/core/migrations/v5/migrations_test.go b/modules/core/migrations/v5/migrations_test.go new file mode 100644 index 00000000000..0c81e5c973d --- /dev/null +++ b/modules/core/migrations/v5/migrations_test.go @@ -0,0 +1,116 @@ +package v5_test + +import ( + "testing" + + sdk "github.com/cosmos/cosmos-sdk/types" + "github.com/stretchr/testify/suite" + + clienttypes "github.com/cosmos/ibc-go/v3/modules/core/02-client/types" + host "github.com/cosmos/ibc-go/v3/modules/core/24-host" + "github.com/cosmos/ibc-go/v3/modules/core/exported" + v5 "github.com/cosmos/ibc-go/v3/modules/core/migrations/v5" + ibctesting "github.com/cosmos/ibc-go/v3/testing" +) + +type MigrationsV5TestSuite struct { + suite.Suite + + coordinator *ibctesting.Coordinator + + chainA *ibctesting.TestChain + chainB *ibctesting.TestChain +} + +func (suite *MigrationsV5TestSuite) SetupTest() { + suite.coordinator = ibctesting.NewCoordinator(suite.T(), 2) + + suite.chainA = suite.coordinator.GetChain(ibctesting.GetChainID(1)) + suite.chainB = suite.coordinator.GetChain(ibctesting.GetChainID(2)) +} + +func TestIBCTestSuite(t *testing.T) { + suite.Run(t, new(MigrationsV5TestSuite)) +} + +func (suite *MigrationsV5TestSuite) TestMigrateToV5() { + var clientStore sdk.KVStore + + testCases := []struct { + name string + malleate func() + expPass bool + }{ + { + "success: prune localhost client state", + func() { + clientStore.Set(host.ClientStateKey(), []byte("clientState")) + }, + true, + }, + { + "success: prune localhost client state and consensus states", + func() { + clientStore.Set(host.ClientStateKey(), []byte("clientState")) + + for i := 0; i < 10; i++ { + clientStore.Set(host.ConsensusStateKey(clienttypes.NewHeight(1, uint64(i))), []byte("consensusState")) + } + }, + true, + }, + { + "07-tendermint client state and consensus states remain in client store", + func() { + clientStore = suite.chainA.GetSimApp().IBCKeeper.ClientKeeper.ClientStore(suite.chainA.GetContext(), clienttypes.FormatClientIdentifier(exported.Tendermint, 0)) + clientStore.Set(host.ClientStateKey(), []byte("clientState")) + + for i := 0; i < 10; i++ { + clientStore.Set(host.ConsensusStateKey(clienttypes.NewHeight(1, uint64(i))), []byte("consensusState")) + } + }, + false, + }, + { + "06-solomachine client state and consensus states remain in client store", + func() { + clientStore = suite.chainA.GetSimApp().IBCKeeper.ClientKeeper.ClientStore(suite.chainA.GetContext(), clienttypes.FormatClientIdentifier(exported.Solomachine, 0)) + clientStore.Set(host.ClientStateKey(), []byte("clientState")) + + for i := 0; i < 10; i++ { + clientStore.Set(host.ConsensusStateKey(clienttypes.NewHeight(1, uint64(i))), []byte("consensusState")) + } + }, + false, + }, + } + + for _, tc := range testCases { + tc := tc + + suite.Run(tc.name, func() { + suite.SetupTest() // reset + + ctx := suite.chainA.GetContext() + clientStore = suite.chainA.GetSimApp().IBCKeeper.ClientKeeper.ClientStore(suite.chainA.GetContext(), v5.Localhost) + + tc.malleate() + + v5.MigrateToV5(ctx, suite.chainA.GetSimApp().IBCKeeper.ClientKeeper) + + if tc.expPass { + suite.Require().False(clientStore.Has(host.ClientStateKey())) + + for i := 0; i < 10; i++ { + suite.Require().False(clientStore.Has(host.ConsensusStateKey(clienttypes.NewHeight(1, uint64(i))))) + } + } else { + suite.Require().True(clientStore.Has(host.ClientStateKey())) + + for i := 0; i < 10; i++ { + suite.Require().True(clientStore.Has(host.ConsensusStateKey(clienttypes.NewHeight(1, uint64(i))))) + } + } + }) + } +}