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

imp!: removed 'ConsensusHost' interface #6937

Merged
merged 43 commits into from
Aug 6, 2024
Merged
Show file tree
Hide file tree
Changes from 36 commits
Commits
Show all changes
43 commits
Select commit Hold shift + click to select a range
dce76d2
imp(02-client,03-connection)!: remove `ValidateSelfClient` (#6853)
srdtrk Jul 19, 2024
9af4a22
imp: deleted consensus host from core
srdtrk Jul 24, 2024
0632278
imp(08-wasm): removed consensus host
srdtrk Jul 24, 2024
5bfa510
imp: fix linter
srdtrk Jul 24, 2024
dfbbac0
imp: fixed linter
srdtrk Jul 24, 2024
d0c97d8
imp: fixed simapp
srdtrk Jul 24, 2024
9e624da
imp: updated docs
srdtrk Jul 24, 2024
47f5c84
imp: removed more code
srdtrk Jul 24, 2024
e7465f5
revert
srdtrk Jul 24, 2024
147e774
imp: removed unneeded proto fields
srdtrk Jul 24, 2024
14c1449
imp: lint
srdtrk Jul 24, 2024
702cc7c
lint
srdtrk Jul 24, 2024
3694cd3
imp: auto generated code
srdtrk Jul 24, 2024
041bbfe
imp: removed conflicts
srdtrk Jul 24, 2024
3d8c8cc
imp: removed more code
srdtrk Jul 24, 2024
6f45ddf
fix: tests
srdtrk Jul 24, 2024
ab3d3b0
feat: all tests passing
srdtrk Jul 24, 2024
4166a01
merge: branch 'main' into feat/rm-cs-validation
srdtrk Jul 24, 2024
f9132f3
fix: added the reserved proto fields back with deprecation notice
srdtrk Jul 24, 2024
de6244a
style: linted
srdtrk Jul 24, 2024
90c8394
imp: regenerated proto
srdtrk Jul 24, 2024
3252e55
imp: review item
srdtrk Jul 24, 2024
fb1ccec
revert: conn name change
srdtrk Jul 24, 2024
ee974fa
Merge branch 'main' into feat/rm-cs-validation
srdtrk Jul 24, 2024
d9228c6
docs: added changelog
srdtrk Jul 24, 2024
0f29805
Merge branch 'main' into feat/rm-cs-validation
srdtrk Jul 25, 2024
74832b5
Merge branch 'main' into feat/rm-cs-validation
crodriguezvega Jul 25, 2024
956420a
add godoc string of QueryConnectionHandshakeProof
crodriguezvega Jul 25, 2024
f596f0d
add migration docs for ibc-go
crodriguezvega Jul 25, 2024
f397284
Merge branch 'main' into feat/rm-cs-validation
crodriguezvega Jul 25, 2024
f509aa5
Merge branch 'main' into feat/rm-cs-validation
crodriguezvega Jul 25, 2024
5bec21b
Update CHANGELOG.md
crodriguezvega Jul 25, 2024
1113d22
update changelog
crodriguezvega Jul 25, 2024
7bae8b8
imp(proto): added deprecation notice to field
srdtrk Aug 1, 2024
61333b5
imp: ran 'make proto-all'
srdtrk Aug 1, 2024
5304bad
merge: branch 'main' into feat/rm-cs-validation
srdtrk Aug 1, 2024
84b92b0
imp: removed unused keeper
srdtrk Aug 1, 2024
4e7a4f6
Update CHANGELOG.md
srdtrk Aug 2, 2024
01eb933
Update docs/docs/05-migrations/13-v8-to-v9.md
srdtrk Aug 2, 2024
4153f6f
Update docs/docs/05-migrations/13-v8-to-v9.md
srdtrk Aug 2, 2024
ca3c141
merge: branch 'main' into feat/rm-cs-validation
srdtrk Aug 2, 2024
934f8e8
Merge branch 'main' into feat/rm-cs-validation
srdtrk Aug 5, 2024
7059e27
Merge branch 'main' into feat/rm-cs-validation
colin-axner Aug 5, 2024
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 @@ -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.
srdtrk marked this conversation as resolved.
Show resolved Hide resolved

### 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 @@ -55,26 +56,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 introspects 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:
srdtrk marked this conversation as resolved.
Show resolved Hide resolved

```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 @@ -312,6 +365,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,
connectioProof []byte, proofHeight clienttypes.Height,
srdtrk marked this conversation as resolved.
Show resolved Hide resolved
)
```

- 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
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
Loading