Skip to content

Commit

Permalink
imp!: removed 'ConsensusHost' interface (#6937)
Browse files Browse the repository at this point in the history
* imp(02-client,03-connection)!: remove `ValidateSelfClient` (#6853)

* imp: removed ValidateSelfClient

* docs: updated a godoc

* imp: deleted consensus host from core

* imp(08-wasm): removed consensus host

* imp: fix linter

* imp: fixed linter

* imp: fixed simapp

* imp: updated docs

* imp: removed more code

* revert

* imp: removed unneeded proto fields

* imp: lint

* lint

* imp: auto generated code

* imp: removed conflicts

* imp: removed more code

* fix: tests

* feat: all tests passing

* fix: added the reserved proto fields back with deprecation notice

* style: linted

* imp: regenerated proto

* imp: review item

* revert: conn name change

* docs: added changelog

* add godoc string of QueryConnectionHandshakeProof

* add migration docs for ibc-go

* Update CHANGELOG.md

* update changelog

* imp(proto): added deprecation notice to field

* imp: ran 'make proto-all'

* imp: removed unused keeper

* Update CHANGELOG.md

Co-authored-by: colin axnér <[email protected]>

* Update docs/docs/05-migrations/13-v8-to-v9.md

Co-authored-by: colin axnér <[email protected]>

* Update docs/docs/05-migrations/13-v8-to-v9.md

Co-authored-by: colin axnér <[email protected]>

---------

Co-authored-by: Carlos Rodriguez <[email protected]>
Co-authored-by: colin axnér <[email protected]>
  • Loading branch information
3 people authored Aug 6, 2024
1 parent 4ace83d commit 2028e9a
Show file tree
Hide file tree
Showing 32 changed files with 285 additions and 1,643 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -76,6 +76,7 @@ Ref: https://keepachangelog.com/en/1.0.0/
* (light-clients/06-solomachine) [\#6888](https://github.com/cosmos/ibc-go/pull/6888) Remove `TypeClientMisbehaviour` constant and the `Type` method on `Misbehaviour`.
* (light-clients/06-solomachine, light-clients/07-tendermint) [\#6891](https://github.com/cosmos/ibc-go/pull/6891) The `VerifyMembership` and `VerifyNonMembership` functions of solomachine's `ClientState` have been made private. The `VerifyMembership`, `VerifyNonMembership`, `GetTimestampAtHeight`, `Status` and `Initialize` functions of tendermint's `ClientState` have been made private.
* (core/04-channel) [\#6902](https://github.com/cosmos/ibc-go/pull/6902) Add channel version to core application callbacks.
* (core/03-connection, core/02-client) [\#6937](https://github.com/cosmos/ibc-go/pull/6937) Remove 'ConsensusHost' interface, also removing self client and consensus state validation in the connection handshake.
* (core/24-host) [\#6882](https://github.com/cosmos/ibc-go/issues/6882) All functions ending in `Path` have been removed from 24-host in favour of their sybling functions ending in `Key`.

### State Machine Breaking
Expand Down
1 change: 0 additions & 1 deletion docs/docs/01-ibc/02-integration.md
Original file line number Diff line number Diff line change
Expand Up @@ -98,7 +98,6 @@ func NewApp(...args) *App {
appCodec,
keys[ibcexported.StoreKey],
app.GetSubspace(ibcexported.ModuleName),
ibctm.NewConsensusHost(app.StakingKeeper),
app.UpgradeKeeper,
scopedIBCKeeper,
authtypes.NewModuleAddress(govtypes.ModuleName).String(),
Expand Down
66 changes: 66 additions & 0 deletions docs/docs/05-migrations/13-v8-to-v9.md
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ There are four sections based on the four potential user groups of this document
- [API removals](#api-removals)
- [02-client](#02-client)
- [03-connection](#03-connection)
- [Removal of self client and consensus state from connection handshake](#removal-of-self-client-and-consensus-state-from-connection-handshake)
- [04-channel](#04-channel)
- [05-port](#05-port)
- [23-commitment](#23-commitment)
Expand Down Expand Up @@ -57,26 +58,78 @@ govRouter.AddRoute(govtypes.RouterKey, govv1beta1.ProposalHandler).

## IBC core

- Because the self client and consensus state validation has been removed from the connection handshake (see section [Removal of self client and consensus state from connection handshake](#removal-of-self-client-and-consensus-state-from-connection-handshake) for more details), the IBC core keeper does not need the staking keeper anymore to introspect the (self) past historical info at a given height and construct the expected consensus state at that height. Thus, the signature of IBC core keeper constructor function `NewKeeper` has been updated:

```diff
func NewKeeper(
cdc codec.BinaryCodec, key storetypes.StoreKey, paramSpace types.ParamSubspace,
- stakingKeeper clienttypes.StakingKeeper,
upgradeKeeper clienttypes.UpgradeKeeper,
scopedKeeper capabilitykeeper.ScopedKeeper, authority string,
) *Keeper
```

### API removals

- The [`exported.ChannelI`](https://github.com/cosmos/ibc-go/blob/v8.0.0/modules/core/exported/channel.go#L3-L11) and [`exported.CounterpartyChannelI`](https://github.com/cosmos/ibc-go/blob/v8.0.0/modules/core/exported/channel.go#L13-L19) interfaces have been removed. Please use the concrete types.
- The [`exported.ConnectionI`](https://github.com/cosmos/ibc-go/blob/v8.0.0/modules/core/exported/connection.go#L6-L13) and [`exported.CounterpartyConnectionI`](https://github.com/cosmos/ibc-go/blob/v8.0.0/modules/core/exported/connection.go#L15-L21) interfaces have been removed. Please use the concrete types.
- The [`Router` reference](https://github.com/cosmos/ibc-go/blob/v8.0.0/modules/core/keeper/keeper.go#L35) has been removed from the IBC core keeper in [#6138](https://github.com/cosmos/ibc-go/pull/6138). Please use `PortKeeper.Router` instead.
- The [composite interface `QueryServer`](https://github.com/cosmos/ibc-go/blob/v8.0.0/modules/core/types/query.go#L14-L19) has been removed from package `core/types`. Please use the granular `QueryServer` interfaces for IBC submodules directly.
- The [`TypeClientMisbehaviour` constant](https://github.com/cosmos/ibc-go/blob/v8.0.0/modules/core/exported/client.go#L17) has been removed.
- The function [`SetConsensusHost`](https://github.com/cosmos/ibc-go/blob/v8.3.0/modules/core/keeper/keeper.go#L88-L96) has been removed because the self client and consensus state validation has been removed from the connection handshake. See section [Removal of self client and consensus state from connection handshake](#removal-of-self-client-and-consensus-state-from-connection-handshake) for more details.

### 02-client

- The `QueryVerifyMembershipRequest` protobuf message has been modified to include `commitment.v2.MerklePath`. The deprecated `commitment.v1.MerklePath` field has been `reserved`. [See 23-commitment](#23-commitment).
- The function [`CreateLocalhostClient`](https://github.com/cosmos/ibc-go/blob/v8.0.0/modules/core/02-client/keeper/keeper.go#L56) has been removed. The localhost client is now stateless.
- The function [`NewClientProposalHandler`](https://github.com/cosmos/ibc-go/blob/v8.0.0/modules/core/02-client/proposal_handler.go#L18) has been removed in [#6777](https://github.com/cosmos/ibc-go/pull/6777).
- The deprecated [`ClientUpdateProposal` and `UpgradeProposal` messages](https://github.com/cosmos/ibc-go/blob/v8.0.0/proto/ibc/core/client/v1/client.proto#L67-L113) have been removed in [\#6782](https://github.com/cosmos/ibc-go/pull/6782). Please use [`MsgRecoverClient`](https://github.com/cosmos/ibc-go/blob/release/v9.0.x/proto/ibc/core/client/v1/tx.proto#L125-L138) and [`MsgIBCSoftwareUpgrade`](https://github.com/cosmos/ibc-go/blob/release/v9.0.x/proto/ibc/core/client/v1/tx.proto#L143-L158) respectively instead.
- Because the self client and consensus state validation has been removed from the connection handshake (see section [Removal of self client and consensus state from connection handshake](#removal-of-self-client-and-consensus-state-from-connection-handshake) for more details):
- The [ConsensusHost interface](https://github.com/cosmos/ibc-go/blob/v8.3.0/modules/core/02-client/types/client.go#L25-L29) has been removed.
- The function [`SetConsensusHost`](https://github.com/cosmos/ibc-go/blob/v8.3.0/modules/core/02-client/keeper/keeper.go#L61-L68) has been removed.
- The functions [`GetSelfConsensusState` and `ValidateSelfClient`](https://github.com/cosmos/ibc-go/blob/v8.3.0/modules/core/02-client/keeper/keeper.go#L256-L269) have been removed.

### 03-connection

- The [functions `GetState()`, `GetClientID()`, `GetCounterparty()`, `GetVersions()`, and `GetDelayPeriod()`](https://github.com/cosmos/ibc-go/blob/v8.0.0/modules/core/03-connection/types/connection.go#L25-L48) of the `Connection` type have been removed.
- The [functions `GetClientID()`, `GetConnectionID()`, and `GetPrefix()`](https://github.com/cosmos/ibc-go/blob/v8.0.0/modules/core/03-connection/types/connection.go#L79-L92) of the `Counterparty` type have been removed.

#### Removal of self client and consensus state from connection handshake

The `ConnectionOpenTry` and `ConnectionOpenAck` handlers no longer validate that the light client on counterparty chain has a valid representation of the executing chain's consensus protocol (please see [#1128](https://github.com/cosmos/ibc/pull/1128) in cosmos/ibc repository for an exhaustive explanation of the reasoning).

- The fields `client_state`, `proof_client`, `proof_consensus`, `consensus_height` and `host_consensus_state_proof` of `MsgConnectionOpenTry` and `MsgConnectionOpenAck` have been deprecated, and the signature of the constructor functions [`NewMsgConnectionOpenTry`](https://github.com/cosmos/ibc-go/blob/release/v9.0.x/modules/core/03-connection/types/msgs.go#L78) and [`NewMsgConnectionOpenTry`](https://github.com/cosmos/ibc-go/blob/release/v9.0.x/modules/core/03-connection/types/msgs.go#L165) has been accordingly updated:

```diff
func NewMsgConnectionOpenTry(
clientID, counterpartyConnectionID, counterpartyClientID string,
- counterpartyClient exported.ClientState,
counterpartyPrefix commitmenttypes.MerklePrefix,
counterpartyVersions []*Version, delayPeriod uint64,
initProof []byte,
- clientProof []byte,
- consensusProof []byte,
proofHeight lienttypes.Height,
- consensusHeight clienttypes.Height,
signer string,
) *MsgConnectionOpenTry

func NewMsgConnectionOpenAck(
connectionID, counterpartyConnectionID string,
- counterpartyClient exported.ClientState,
tryProof []byte,
- clientProof []byte,
- consensusProof []byte,
proofHeight clienttypes.Height,
- consensusHeight clienttypes.Height,
version *Version,
signer string,
) *MsgConnectionOpenAck
```

- The functions [`VerifyClientState` and `VerifyClientConsensusState`](https://github.com/cosmos/ibc-go/blob/v8.3.0/modules/core/03-connection/keeper/verify.go#L20-L101) have been removed.
- The function [`UnpackInterfaces`](https://github.com/cosmos/ibc-go/blob/v8.0.0/modules/core/03-connection/types/msgs.go#L166) has been removed.

### 04-channel

- The utility function [`QueryLatestConsensusState`](https://github.com/cosmos/ibc-go/blob/v8.0.0/modules/core/04-channel/client/utils/utils.go#L130) of the CLI has been removed.
Expand Down Expand Up @@ -375,6 +428,19 @@ func AssertEvents(
clientQueryServer := clientkeeper.NewQueryServer(app.IBCKeeper.ClientKeeper)
```

- The signature of the function `QueryConnectionHandshakeProof` has changed, since the validation of self client and consensus state has been remove from the connection handshake:

```diff
func (endpoint *Endpoint) QueryConnectionHandshakeProof() (
- clientState exported.ClientState, clientProof,
- consensusProof []byte, consensusHeight clienttypes.Height,
connectionProof []byte, proofHeight clienttypes.Height,
)
```

- The functions [`GenerateClientStateProof` and `GenerateConsensusStateProof`](https://github.com/cosmos/ibc-go/blob/v8.0.0/testing/solomachine.go#L513-L547)
have been removed.

### API deprecation notice

- The testing package functions `Setup`, `SetupClients`, `SetupConnections`, `CreateConnections`, and `CreateChannels` of the `Coordinator` type have been deprecated and will be removed in v10. Please use the new functions `Setup`, `SetupClients`, `SetupConnections`, `CreateConnections`, `CreateChannels` of the `Path` type.
Expand Down
2 changes: 1 addition & 1 deletion modules/apps/callbacks/testing/simapp/app.go
Original file line number Diff line number Diff line change
Expand Up @@ -405,7 +405,7 @@ func NewSimApp(
app.UpgradeKeeper = upgradekeeper.NewKeeper(skipUpgradeHeights, runtime.NewKVStoreService(keys[upgradetypes.StoreKey]), appCodec, homePath, app.BaseApp, authtypes.NewModuleAddress(govtypes.ModuleName).String())

app.IBCKeeper = ibckeeper.NewKeeper(
appCodec, keys[ibcexported.StoreKey], app.GetSubspace(ibcexported.ModuleName), ibctm.NewConsensusHost(app.StakingKeeper), app.UpgradeKeeper, scopedIBCKeeper, authtypes.NewModuleAddress(govtypes.ModuleName).String(),
appCodec, keys[ibcexported.StoreKey], app.GetSubspace(ibcexported.ModuleName), app.UpgradeKeeper, scopedIBCKeeper, authtypes.NewModuleAddress(govtypes.ModuleName).String(),
)

// NOTE: The mock ContractKeeper is only created for testing.
Expand Down
28 changes: 1 addition & 27 deletions modules/core/02-client/keeper/keeper.go
Original file line number Diff line number Diff line change
Expand Up @@ -27,13 +27,12 @@ type Keeper struct {
storeKey storetypes.StoreKey
cdc codec.BinaryCodec
router *types.Router
consensusHost types.ConsensusHost
legacySubspace types.ParamSubspace
upgradeKeeper types.UpgradeKeeper
}

// NewKeeper creates a new NewKeeper instance
func NewKeeper(cdc codec.BinaryCodec, key storetypes.StoreKey, legacySubspace types.ParamSubspace, consensusHost types.ConsensusHost, uk types.UpgradeKeeper) *Keeper {
func NewKeeper(cdc codec.BinaryCodec, key storetypes.StoreKey, legacySubspace types.ParamSubspace, uk types.UpgradeKeeper) *Keeper {
router := types.NewRouter()
localhostModule := localhost.NewLightClientModule(cdc, key)
router.AddRoute(exported.Localhost, localhostModule)
Expand All @@ -42,7 +41,6 @@ func NewKeeper(cdc codec.BinaryCodec, key storetypes.StoreKey, legacySubspace ty
storeKey: key,
cdc: cdc,
router: router,
consensusHost: consensusHost,
legacySubspace: legacySubspace,
upgradeKeeper: uk,
}
Expand Down Expand Up @@ -90,15 +88,6 @@ func (k *Keeper) Route(ctx sdk.Context, clientID string) (exported.LightClientMo
return clientModule, nil
}

// SetConsensusHost sets a custom ConsensusHost for self client state and consensus state validation.
func (k *Keeper) SetConsensusHost(consensusHost types.ConsensusHost) {
if consensusHost == nil {
panic(errors.New("cannot set a nil self consensus host"))
}

k.consensusHost = consensusHost
}

// GenerateClientIdentifier returns the next client identifier.
func (k *Keeper) GenerateClientIdentifier(ctx sdk.Context, clientType string) string {
nextClientSeq := k.GetNextClientSequence(ctx)
Expand Down Expand Up @@ -315,21 +304,6 @@ func (k *Keeper) GetLatestClientConsensusState(ctx sdk.Context, clientID string)
return k.GetClientConsensusState(ctx, clientID, clientModule.LatestHeight(ctx, clientID))
}

// GetSelfConsensusState introspects the (self) past historical info at a given height
// and returns the expected consensus state at that height.
// For now, can only retrieve self consensus states for the current revision
func (k *Keeper) GetSelfConsensusState(ctx sdk.Context, height exported.Height) (exported.ConsensusState, error) {
return k.consensusHost.GetSelfConsensusState(ctx, height)
}

// ValidateSelfClient validates the client parameters for a client of the running chain.
// This function is only used to validate the client state the counterparty stores for this chain.
// NOTE: If the client type is not of type Tendermint then delegate to a custom client validator function.
// This allows support for non-Tendermint clients, for example 08-wasm clients.
func (k *Keeper) ValidateSelfClient(ctx sdk.Context, clientState exported.ClientState) error {
return k.consensusHost.ValidateSelfClient(ctx, clientState)
}

// VerifyMembership retrieves the light client module for the clientID and verifies the proof of the existence of a key-value pair at a specified height.
func (k *Keeper) VerifyMembership(ctx sdk.Context, clientID string, height exported.Height, delayTimePeriod uint64, delayBlockPeriod uint64, proof []byte, path exported.Path, value []byte) error {
clientModule, err := k.Route(ctx, clientID)
Expand Down
7 changes: 0 additions & 7 deletions modules/core/02-client/types/client.go
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,6 @@ import (
errorsmod "cosmossdk.io/errors"

codectypes "github.com/cosmos/cosmos-sdk/codec/types"
sdk "github.com/cosmos/cosmos-sdk/types"

host "github.com/cosmos/ibc-go/v9/modules/core/24-host"
"github.com/cosmos/ibc-go/v9/modules/core/exported"
Expand All @@ -22,12 +21,6 @@ var (
_ codectypes.UnpackInterfacesMessage = (*ConsensusStateWithHeight)(nil)
)

// ConsensusHost defines an interface used to validate an IBC ClientState and ConsensusState against the host chain's underlying consensus parameters.
type ConsensusHost interface {
GetSelfConsensusState(ctx sdk.Context, height exported.Height) (exported.ConsensusState, error)
ValidateSelfClient(ctx sdk.Context, clientState exported.ClientState) error
}

// NewIdentifiedClientState creates a new IdentifiedClientState instance
func NewIdentifiedClientState(clientID string, clientState exported.ClientState) IdentifiedClientState {
msg, ok := clientState.(proto.Message)
Expand Down
8 changes: 0 additions & 8 deletions modules/core/02-client/types/expected_keepers.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,21 +2,13 @@ package types

import (
"context"
"time"

upgradetypes "cosmossdk.io/x/upgrade/types"

sdk "github.com/cosmos/cosmos-sdk/types"
paramtypes "github.com/cosmos/cosmos-sdk/x/params/types"
stakingtypes "github.com/cosmos/cosmos-sdk/x/staking/types"
)

// StakingKeeper expected staking keeper
type StakingKeeper interface {
GetHistoricalInfo(ctx context.Context, height int64) (stakingtypes.HistoricalInfo, error)
UnbondingTime(ctx context.Context) (time.Duration, error)
}

// UpgradeKeeper expected upgrade keeper
type UpgradeKeeper interface {
GetUpgradePlan(ctx context.Context) (plan upgradetypes.Plan, err error)
Expand Down
15 changes: 6 additions & 9 deletions modules/core/03-connection/keeper/events_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -48,13 +48,12 @@ func (suite *KeeperTestSuite) TestMsgConnectionOpenTryEvents() {

suite.Require().NoError(path.EndpointB.UpdateClient())

counterpartyClient, clientProof, consensusProof, consensusHeight, initProof, proofHeight := path.EndpointB.QueryConnectionHandshakeProof()
initProof, proofHeight := path.EndpointB.QueryConnectionHandshakeProof()

msg := types.NewMsgConnectionOpenTry(
path.EndpointB.ClientID, path.EndpointB.Counterparty.ConnectionID, path.EndpointB.Counterparty.ClientID,
counterpartyClient, path.EndpointB.Counterparty.Chain.GetPrefix(), []*types.Version{ibctesting.ConnectionVersion}, path.EndpointB.ConnectionConfig.DelayPeriod,
initProof, clientProof, consensusProof,
proofHeight, consensusHeight,
path.EndpointB.Counterparty.Chain.GetPrefix(), []*types.Version{ibctesting.ConnectionVersion},
path.EndpointB.ConnectionConfig.DelayPeriod, initProof, proofHeight,
path.EndpointB.Chain.SenderAccount.GetAddress().String(),
)

Expand Down Expand Up @@ -88,13 +87,11 @@ func (suite *KeeperTestSuite) TestMsgConnectionOpenAckEvents() {

suite.Require().NoError(path.EndpointA.UpdateClient())

counterpartyClient, clientProof, consensusProof, consensusHeight, tryProof, proofHeight := path.EndpointA.QueryConnectionHandshakeProof()
tryProof, proofHeight := path.EndpointA.QueryConnectionHandshakeProof()

msg := types.NewMsgConnectionOpenAck(
path.EndpointA.ConnectionID, path.EndpointA.Counterparty.ConnectionID, counterpartyClient,
tryProof, clientProof, consensusProof,
proofHeight, consensusHeight,
ibctesting.ConnectionVersion,
path.EndpointA.ConnectionID, path.EndpointA.Counterparty.ConnectionID,
tryProof, proofHeight, ibctesting.ConnectionVersion,
path.EndpointA.Chain.SenderAccount.GetAddress().String(),
)

Expand Down
Loading

0 comments on commit 2028e9a

Please sign in to comment.