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

chore: adding upgrade handler for 09-localhost removal #1671

Merged
Merged
Show file tree
Hide file tree
Changes from 7 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
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`.
92 changes: 92 additions & 0 deletions docs/migrations/v4-to-v5.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,92 @@
# 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`.
colin-axner marked this conversation as resolved.
Show resolved Hide resolved
Add the following to the application upgrade handler in `app/app.go`, calling `UpgradeToV5` to perform store migration logic.

```go
import (
// ...
ibcv5 "github.com/cosmos/ibc-go/v5/modules/core/upgrades/v5"
)

// ...

app.UpgradeKeeper.SetUpgradeHandler(
upgradeName,
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: upgradeName can be MigrateToV5 or MigrateLocalHostClient?

func(ctx sdk.Context, _ upgradetypes.Plan, _ module.VersionMap) (module.VersionMap, error) {
// prune the 09-localhost client from the ibc client store
ibcv5.UpgradeToV5(ctx, app.IBCKeeper.ClientKeeper)

return app.mm.RunMigrations(ctx, app.configurator, fromVM)
},
)
```

### 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`.
damiannolan marked this conversation as resolved.
Show resolved Hide resolved
44 changes: 44 additions & 0 deletions modules/core/upgrades/v5/upgrades.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"

// UpgradeToV5 prunes the 09-Localhost client and associated consensus states from the ibc store
func UpgradeToV5(ctx sdk.Context, clientKeeper clientkeeper.Keeper) {
clientStore := clientKeeper.ClientStore(ctx, Localhost)
colin-axner marked this conversation as resolved.
Show resolved Hide resolved

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/upgrades/v5/upgrades_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/upgrades/v5"
ibctesting "github.com/cosmos/ibc-go/v3/testing"
)

type UpgradeV5TestSuite struct {
suite.Suite

coordinator *ibctesting.Coordinator

chainA *ibctesting.TestChain
chainB *ibctesting.TestChain
}

func (suite *UpgradeV5TestSuite) 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(UpgradeV5TestSuite))
}

func (suite *UpgradeV5TestSuite) TestUpgradeToV5() {
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.UpgradeToV5(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)))))
}
}
})
}
}