Skip to content

Commit

Permalink
chore: adding upgrade handler for 09-localhost removal (#1671)
Browse files Browse the repository at this point in the history
* 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 <carlos@interchain.io>

Co-authored-by: Carlos Rodriguez <carlos@interchain.io>
  • Loading branch information
damiannolan and crodriguezvega authored Jul 15, 2022
1 parent 549c181 commit 04df7cd
Show file tree
Hide file tree
Showing 5 changed files with 263 additions and 46 deletions.
2 changes: 1 addition & 1 deletion CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
46 changes: 1 addition & 45 deletions docs/migrations/v3-to-v4.md
Original file line number Diff line number Diff line change
@@ -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.
Expand All @@ -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`.
101 changes: 101 additions & 0 deletions docs/migrations/v4-to-v5.md
Original file line number Diff line number Diff line change
@@ -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`.
44 changes: 44 additions & 0 deletions modules/core/migrations/v5/migrations.go
Original file line number Diff line number Diff line change
@@ -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/<height>"
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())
}
116 changes: 116 additions & 0 deletions modules/core/migrations/v5/migrations_test.go
Original file line number Diff line number Diff line change
@@ -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)))))
}
}
})
}
}

0 comments on commit 04df7cd

Please sign in to comment.