Skip to content

Commit

Permalink
backport-v7.8.x: self client/consensus state removal from connection …
Browse files Browse the repository at this point in the history 
…handshake (#7128)

* refactor: remove self validation of client and consensus states in connection handshake

* fix: tests + additional removal in msg validation

* changelog

* chore: add deprecation notice to protos
  • Loading branch information
@colin-axner
colin-axner authored Aug 13, 2024
1 parent 88fd95e commit e49ed07
Show file tree
Hide file tree
Showing 9 changed files with 96 additions and 378 deletions.
6 changes: 6 additions & 0 deletions CHANGELOG.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,12 @@ Ref: https://keepachangelog.com/en/1.0.0/

# Changelog

## [v7.8.0]()

### State Machine Breaking

* (core/03-connection)[\#7128](https://github.com/cosmos/ibc-go/pull/7128) Remove verification of self client and consensus state from connection handshake.

## [v7.7.0](https://github.com/cosmos/ibc-go/releases/tag/v7.7.0) - 2024-07-29

### Dependencies
Expand Down
81 changes: 8 additions & 73 deletions modules/core/03-connection/keeper/handshake.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -72,37 +72,17 @@ func (k Keeper) ConnOpenTry(
counterparty types.Counterparty, // counterpartyConnectionIdentifier, counterpartyPrefix and counterpartyClientIdentifier
delayPeriod uint64,
clientID string, // clientID of chainA
clientState exported.ClientState, // clientState that chainA has for chainB
_ exported.ClientState, // clientState that chainA has for chainB
counterpartyVersions []exported.Version, // supported versions of chain A
proofInit []byte, // proof that chainA stored connectionEnd in state (on ConnOpenInit)
proofClient []byte, // proof that chainA stored a light client of chainB
proofConsensus []byte, // proof that chainA stored chainB's consensus state at consensus height
_ []byte, // proof that chainA stored a light client of chainB
_ []byte, // proof that chainA stored chainB's consensus state at consensus height
proofHeight exported.Height, // height at which relayer constructs proof of A storing connectionEnd in state
consensusHeight exported.Height, // latest height of chain B which chain A has stored in its chain B client
_ exported.Height, // latest height of chain B which chain A has stored in its chain B client
) (string, error) {
// generate a new connection
connectionID := k.GenerateConnectionIdentifier(ctx)

// check that the consensus height the counterparty chain is using to store a representation
// of this chain's consensus state is at a height in the past
selfHeight := clienttypes.GetSelfHeight(ctx)
if consensusHeight.GTE(selfHeight) {
return "", sdkerrors.Wrapf(
sdkerrors.ErrInvalidHeight,
"consensus height is greater than or equal to the current block height (%s >= %s)", consensusHeight, selfHeight,
)
}

// validate client parameters of a chainB client stored on chainA
if err := k.clientKeeper.ValidateSelfClient(ctx, clientState); err != nil {
return "", err
}

expectedConsensusState, err := k.clientKeeper.GetSelfConsensusState(ctx, consensusHeight)
if err != nil {
return "", sdkerrors.Wrapf(err, "self consensus state not found for height %s", consensusHeight.String())
}

// expectedConnection defines Chain A's ConnectionEnd
// NOTE: chain A's counterparty is chain B (i.e where this code is executed)
// NOTE: chainA and chainB must have the same delay period
Expand All @@ -129,18 +109,6 @@ func (k Keeper) ConnOpenTry(
return "", err
}

// Check that ChainA stored the clientState provided in the msg
if err := k.VerifyClientState(ctx, connection, proofHeight, proofClient, clientState); err != nil {
return "", err
}

// Check that ChainA stored the correct ConsensusState of chainB at the given consensusHeight
if err := k.VerifyClientConsensusState(
ctx, connection, proofHeight, consensusHeight, proofConsensus, expectedConsensusState,
); err != nil {
return "", err
}

// store connection in chainB state
if err := k.addConnectionToClient(ctx, clientID, connectionID); err != nil {
return "", sdkerrors.Wrapf(err, "failed to add connection with ID %s to client with ID %s", connectionID, clientID)
Expand All @@ -165,25 +133,15 @@ func (k Keeper) ConnOpenTry(
func (k Keeper) ConnOpenAck(
ctx sdk.Context,
connectionID string,
clientState exported.ClientState, // client state for chainA on chainB
_ exported.ClientState, // client state for chainA on chainB
version *types.Version, // version that ChainB chose in ConnOpenTry
counterpartyConnectionID string,
proofTry []byte, // proof that connectionEnd was added to ChainB state in ConnOpenTry
proofClient []byte, // proof of client state on chainB for chainA
proofConsensus []byte, // proof that chainB has stored ConsensusState of chainA on its client
_ []byte, // proof of client state on chainB for chainA
_ []byte, // proof that chainB has stored ConsensusState of chainA on its client
proofHeight exported.Height, // height that relayer constructed proofTry
consensusHeight exported.Height, // latest height of chainA that chainB has stored on its chainA client
_ exported.Height, // latest height of chainA that chainB has stored on its chainA client
) error {
// check that the consensus height the counterparty chain is using to store a representation
// of this chain's consensus state is at a height in the past
selfHeight := clienttypes.GetSelfHeight(ctx)
if consensusHeight.GTE(selfHeight) {
return sdkerrors.Wrapf(
sdkerrors.ErrInvalidHeight,
"consensus height is greater than or equal to the current block height (%s >= %s)", consensusHeight, selfHeight,
)
}

// Retrieve connection
connection, found := k.GetConnection(ctx, connectionID)
if !found {
Expand All @@ -206,17 +164,6 @@ func (k Keeper) ConnOpenAck(
)
}

// validate client parameters of a chainA client stored on chainB
if err := k.clientKeeper.ValidateSelfClient(ctx, clientState); err != nil {
return err
}

// Retrieve chainA's consensus state at consensusheight
expectedConsensusState, err := k.clientKeeper.GetSelfConsensusState(ctx, consensusHeight)
if err != nil {
return sdkerrors.Wrapf(err, "self consensus state not found for height %s", consensusHeight.String())
}

prefix := k.GetCommitmentPrefix()
expectedCounterparty := types.NewCounterparty(connection.ClientId, connectionID, commitmenttypes.NewMerklePrefix(prefix.Bytes()))
expectedConnection := types.NewConnectionEnd(types.TRYOPEN, connection.Counterparty.ClientId, expectedCounterparty, []*types.Version{version}, connection.DelayPeriod)
Expand All @@ -229,18 +176,6 @@ func (k Keeper) ConnOpenAck(
return err
}

// Check that ChainB stored the clientState provided in the msg
if err := k.VerifyClientState(ctx, connection, proofHeight, proofClient, clientState); err != nil {
return err
}

// Ensure that ChainB has stored the correct ConsensusState for chainA at the consensusHeight
if err := k.VerifyClientConsensusState(
ctx, connection, proofHeight, consensusHeight, proofConsensus, expectedConsensusState,
); err != nil {
return err
}

k.Logger(ctx).Info("connection state updated", "connection-id", connectionID, "previous-state", "INIT", "new-state", "OPEN")

defer func() {
Expand Down
142 changes: 1 addition & 141 deletions modules/core/03-connection/keeper/handshake_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@ import (
"github.com/cosmos/ibc-go/v7/modules/core/03-connection/types"
host "github.com/cosmos/ibc-go/v7/modules/core/24-host"
"github.com/cosmos/ibc-go/v7/modules/core/exported"
ibctm "github.com/cosmos/ibc-go/v7/modules/light-clients/07-tendermint"
ibctesting "github.com/cosmos/ibc-go/v7/testing"
)

Expand Down Expand Up @@ -132,38 +131,6 @@ func (suite *KeeperTestSuite) TestConnOpenTry() {
// retrieve client state of chainA to pass as counterpartyClient
counterpartyClient = suite.chainA.GetClientState(path.EndpointA.ClientID)
}, true},
{"invalid counterparty client", func() {
err := path.EndpointA.ConnOpenInit()
suite.Require().NoError(err)

// retrieve client state of chainB to pass as counterpartyClient
counterpartyClient = suite.chainA.GetClientState(path.EndpointA.ClientID)

// Set an invalid client of chainA on chainB
tmClient, ok := counterpartyClient.(*ibctm.ClientState)
suite.Require().True(ok)
tmClient.ChainId = "wrongchainid"

suite.chainA.App.GetIBCKeeper().ClientKeeper.SetClientState(suite.chainA.GetContext(), path.EndpointA.ClientID, tmClient)
}, false},
{"consensus height >= latest height", func() {
err := path.EndpointA.ConnOpenInit()
suite.Require().NoError(err)

// retrieve client state of chainA to pass as counterpartyClient
counterpartyClient = suite.chainA.GetClientState(path.EndpointA.ClientID)

consensusHeight = clienttypes.GetSelfHeight(suite.chainB.GetContext())
}, false},
{"self consensus state not found", func() {
err := path.EndpointA.ConnOpenInit()
suite.Require().NoError(err)

// retrieve client state of chainA to pass as counterpartyClient
counterpartyClient = suite.chainA.GetClientState(path.EndpointA.ClientID)

consensusHeight = clienttypes.NewHeight(0, 1)
}, false},
{"counterparty versions is empty", func() {
err := path.EndpointA.ConnOpenInit()
suite.Require().NoError(err)
Expand All @@ -189,35 +156,6 @@ func (suite *KeeperTestSuite) TestConnOpenTry() {
// retrieve client state of chainA to pass as counterpartyClient
counterpartyClient = suite.chainA.GetClientState(path.EndpointA.ClientID)
}, false},
{"client state verification failed", func() {
err := path.EndpointA.ConnOpenInit()
suite.Require().NoError(err)

// retrieve client state of chainA to pass as counterpartyClient
counterpartyClient = suite.chainA.GetClientState(path.EndpointA.ClientID)

// modify counterparty client without setting in store so it still passes validate but fails proof verification
tmClient, ok := counterpartyClient.(*ibctm.ClientState)
suite.Require().True(ok)
tmClient.LatestHeight = tmClient.LatestHeight.Increment().(clienttypes.Height)
}, false},
{"consensus state verification failed", func() {
// retrieve client state of chainA to pass as counterpartyClient
counterpartyClient = suite.chainA.GetClientState(path.EndpointA.ClientID)

// give chainA wrong consensus state for chainB
consState, found := suite.chainA.App.GetIBCKeeper().ClientKeeper.GetLatestClientConsensusState(suite.chainA.GetContext(), path.EndpointA.ClientID)
suite.Require().True(found)

tmConsState, ok := consState.(*ibctm.ConsensusState)
suite.Require().True(ok)

tmConsState.Timestamp = time.Now()
suite.chainA.App.GetIBCKeeper().ClientKeeper.SetClientConsensusState(suite.chainA.GetContext(), path.EndpointA.ClientID, counterpartyClient.GetLatestHeight(), tmConsState)

err := path.EndpointA.ConnOpenInit()
suite.Require().NoError(err)
}, false},
}

for _, tc := range testCases {
Expand Down Expand Up @@ -295,35 +233,6 @@ func (suite *KeeperTestSuite) TestConnOpenAck() {
// retrieve client state of chainB to pass as counterpartyClient
counterpartyClient = suite.chainB.GetClientState(path.EndpointB.ClientID)
}, true},
{"invalid counterparty client", func() {
err := path.EndpointA.ConnOpenInit()
suite.Require().NoError(err)

err = path.EndpointB.ConnOpenTry()
suite.Require().NoError(err)

// retrieve client state of chainB to pass as counterpartyClient
counterpartyClient = suite.chainB.GetClientState(path.EndpointB.ClientID)

// Set an invalid client of chainA on chainB
tmClient, ok := counterpartyClient.(*ibctm.ClientState)
suite.Require().True(ok)
tmClient.ChainId = "wrongchainid"

suite.chainB.App.GetIBCKeeper().ClientKeeper.SetClientState(suite.chainB.GetContext(), path.EndpointB.ClientID, tmClient)
}, false},
{"consensus height >= latest height", func() {
err := path.EndpointA.ConnOpenInit()
suite.Require().NoError(err)

// retrieve client state of chainB to pass as counterpartyClient
counterpartyClient = suite.chainB.GetClientState(path.EndpointB.ClientID)

err = path.EndpointB.ConnOpenTry()
suite.Require().NoError(err)

consensusHeight = clienttypes.GetSelfHeight(suite.chainA.GetContext())
}, false},
{"connection not found", func() {
// connections are never created

Expand All @@ -334,9 +243,6 @@ func (suite *KeeperTestSuite) TestConnOpenAck() {
err := path.EndpointA.ConnOpenInit()
suite.Require().NoError(err)

// retrieve client state of chainB to pass as counterpartyClient
counterpartyClient = suite.chainB.GetClientState(path.EndpointB.ClientID)

err = path.EndpointB.ConnOpenTry()
suite.Require().NoError(err)

Expand All @@ -345,6 +251,7 @@ func (suite *KeeperTestSuite) TestConnOpenAck() {
suite.Require().True(found)

connection.Counterparty.ConnectionId = "badconnectionid"
path.EndpointB.ConnectionID = "badconnectionid"

suite.chainA.App.GetIBCKeeper().ConnectionKeeper.SetConnection(suite.chainA.GetContext(), path.EndpointA.ConnectionID, connection)

Expand Down Expand Up @@ -418,18 +325,6 @@ func (suite *KeeperTestSuite) TestConnOpenAck() {

version = types.NewVersion(types.DefaultIBCVersionIdentifier, []string{"ORDER_ORDERED", "ORDER_UNORDERED", "ORDER_DAG"})
}, false},
{"self consensus state not found", func() {
err := path.EndpointA.ConnOpenInit()
suite.Require().NoError(err)

// retrieve client state of chainB to pass as counterpartyClient
counterpartyClient = suite.chainB.GetClientState(path.EndpointB.ClientID)

err = path.EndpointB.ConnOpenTry()
suite.Require().NoError(err)

consensusHeight = clienttypes.NewHeight(0, 1)
}, false},
{"connection state verification failed", func() {
// chainB connection is not in INIT
err := path.EndpointA.ConnOpenInit()
Expand All @@ -438,41 +333,6 @@ func (suite *KeeperTestSuite) TestConnOpenAck() {
// retrieve client state of chainB to pass as counterpartyClient
counterpartyClient = suite.chainB.GetClientState(path.EndpointB.ClientID)
}, false},
{"client state verification failed", func() {
err := path.EndpointA.ConnOpenInit()
suite.Require().NoError(err)

// retrieve client state of chainB to pass as counterpartyClient
counterpartyClient = suite.chainB.GetClientState(path.EndpointB.ClientID)

// modify counterparty client without setting in store so it still passes validate but fails proof verification
tmClient, ok := counterpartyClient.(*ibctm.ClientState)
suite.Require().True(ok)
tmClient.LatestHeight = tmClient.LatestHeight.Increment().(clienttypes.Height)

err = path.EndpointB.ConnOpenTry()
suite.Require().NoError(err)
}, false},
{"consensus state verification failed", func() {
err := path.EndpointA.ConnOpenInit()
suite.Require().NoError(err)

// retrieve client state of chainB to pass as counterpartyClient
counterpartyClient = suite.chainB.GetClientState(path.EndpointB.ClientID)

// give chainB wrong consensus state for chainA
consState, found := suite.chainB.App.GetIBCKeeper().ClientKeeper.GetLatestClientConsensusState(suite.chainB.GetContext(), path.EndpointB.ClientID)
suite.Require().True(found)

tmConsState, ok := consState.(*ibctm.ConsensusState)
suite.Require().True(ok)

tmConsState.Timestamp = tmConsState.Timestamp.Add(time.Second)
suite.chainB.App.GetIBCKeeper().ClientKeeper.SetClientConsensusState(suite.chainB.GetContext(), path.EndpointB.ClientID, counterpartyClient.GetLatestHeight(), tmConsState)

err = path.EndpointB.ConnOpenTry()
suite.Require().NoError(err)
}, false},
}

for _, tc := range testCases {
Expand Down
Loading

0 comments on commit e49ed07

Please sign in to comment.