certusone
/
cosmos-sdk
mirror of https://github.com/certusone/cosmos-sdk.git
1
0
Fork 0
Code Issues Projects Releases Wiki Activity

connection handshake updates for versioning (#7328) 

* update handshake to match spec

* add test for version

* add test for open try

* add more tests

Add a test for supplied versions that do not function with previous connection states in INIT and TRYOPEN

* update godoc

* update version checks to switch

Update the version checks in ConnOpenAck to use a switch to increase readability of the code as well as provide custom error messages for each possible case that may occur. Slighly modified version to review suggestion by @fedekunze

* add intersection comments to handshake and pick version

* remove old code
This commit is contained in:
colin axnér 2020-09-22 15:47:43 +02:00 committed by GitHub
parent 7cd25abb87
commit 83f2c75f3d
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
4 changed files with 149 additions and 24 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

45
x/ibc/03-connection/keeper/handshake.go
View File

@ -3,6 +3,7 @@ package keeper
import (
"bytes"
"fmt"
"reflect"
sdk "github.com/cosmos/cosmos-sdk/types"
sdkerrors "github.com/cosmos/cosmos-sdk/types/errors"
@ -82,7 +83,8 @@ func (k Keeper) ConnOpenTry(
expectedConnection := types.NewConnectionEnd(types.INIT, counterparty.ClientId, expectedCounterparty, counterpartyVersions)
// chain B picks a version from Chain A's available versions that is compatible
// with the supported IBC versions
// with Chain B's supported IBC versions. PickVersion will select the intersection
// of the supported versions and the counterparty versions.
version, err := types.PickVersion(counterpartyVersions)
if err != nil {
return err
@ -111,16 +113,17 @@ func (k Keeper) ConnOpenTry(
return err
}
// If connection already exists for connectionID, ensure that the existing connection's counterparty
// is chainA and connection is on INIT stage
// Check that existing connection version is on desired version of current handshake
// If connection already exists for connectionID, ensure that the existing connection's
// counterparty is chainA and connection is on INIT stage.
// Check that existing connection versions for initialized connection is equal to compatible
// versions for this chain.
previousConnection, found := k.GetConnection(ctx, connectionID)
if found && !(previousConnection.State == types.INIT &&
previousConnection.Counterparty.ConnectionId == counterparty.ConnectionId &&
bytes.Equal(previousConnection.Counterparty.Prefix.Bytes(), counterparty.Prefix.Bytes()) &&
previousConnection.ClientId == clientID &&
previousConnection.Counterparty.ClientId == counterparty.ClientId &&
previousConnection.Versions[0] == version) {
reflect.DeepEqual(previousConnection.Versions, types.GetCompatibleEncodedVersions())) {
return sdkerrors.Wrap(types.ErrInvalidConnection, "cannot relay connection attempt")
}
@ -164,31 +167,29 @@ func (k Keeper) ConnOpenAck(
return sdkerrors.Wrap(types.ErrConnectionNotFound, connectionID)
}
// Check connection on ChainA is on correct state: INIT or TRYOPEN
if connection.State != types.INIT && connection.State != types.TRYOPEN {
// Verify the provided version against the previously set connection state
switch {
// connection on ChainA must be in INIT or TRYOPEN
case connection.State != types.INIT && connection.State != types.TRYOPEN:
return sdkerrors.Wrapf(
types.ErrInvalidConnectionState,
"connection state is not INIT or TRYOPEN (got %s)", connection.State.String(),
)
}
version, err := types.DecodeVersion(encodedVersion)
if err != nil {
return sdkerrors.Wrap(err, "version negotiation failed")
}
// Check that ChainB's proposed version identifier is supported by chainA
supportedVersion, found := types.FindSupportedVersion(version, types.GetCompatibleVersions())
if !found {
// if the connection is INIT then the provided version must be supproted
case connection.State == types.INIT && !types.IsSupportedVersion(encodedVersion):
return sdkerrors.Wrapf(
types.ErrVersionNegotiationFailed,
"connection version provided (%s) is not supported (%s)", version, types.GetCompatibleVersions(),
types.ErrInvalidConnectionState,
"connection state is in INIT but the provided encoded version is not supported %s", encodedVersion,
)
}
// Check that ChainB's proposed feature set is supported by chainA
if err := supportedVersion.VerifyProposedVersion(version); err != nil {
return err
// if the connection is in TRYOPEN then the encoded version must be the only set version in the
// retreived connection state.
case connection.State == types.TRYOPEN && (len(connection.Versions) != 1 || connection.Versions[0] != encodedVersion):
return sdkerrors.Wrapf(
types.ErrInvalidConnectionState,
"connection state is in TRYOPEN but the provided encoded version (%s) is not set in the previous connection %s", encodedVersion, connection,
)
}
// validate client parameters of a chainA client stored on chainB

70
x/ibc/03-connection/keeper/handshake_test.go
View File

@ -180,9 +180,24 @@ func (suite *KeeperTestSuite) TestConnOpenTry() {
{"invalid previous connection is in TRYOPEN", func() {
clientA, clientB = suite.coordinator.SetupClients(suite.chainA, suite.chainB, exported.Tendermint)
// open init chainA
connA, connB, err := suite.coordinator.ConnOpenInit(suite.chainA, suite.chainB, clientA, clientB)
suite.Require().NoError(err)
// open try chainB
err = suite.coordinator.ConnOpenTry(suite.chainB, suite.chainA, connB, connA)
suite.Require().NoError(err)
err = suite.coordinator.UpdateClient(suite.chainB, suite.chainA, clientB, exported.Tendermint)
suite.Require().NoError(err)
// retrieve client state of chainA to pass as counterpartyClient
counterpartyClient = suite.chainA.GetClientState(clientA)
}, false},
{"invalid previous connection has invalid versions", func() {
clientA, clientB = suite.coordinator.SetupClients(suite.chainA, suite.chainB, exported.Tendermint)
// open init chainA
connA, connB, err := suite.coordinator.ConnOpenInit(suite.chainA, suite.chainB, clientA, clientB)
suite.Require().NoError(err)
@ -190,6 +205,21 @@ func (suite *KeeperTestSuite) TestConnOpenTry() {
// open try chainB
err = suite.coordinator.ConnOpenTry(suite.chainB, suite.chainA, connB, connA)
suite.Require().NoError(err)
// modify connB to be in INIT with incorrect versions
connection, found := suite.chainB.App.IBCKeeper.ConnectionKeeper.GetConnection(suite.chainB.GetContext(), connB.ID)
suite.Require().True(found)
connection.State = types.INIT
connection.Versions = []string{"invalid version"}
suite.chainB.App.IBCKeeper.ConnectionKeeper.SetConnection(suite.chainB.GetContext(), connB.ID, connection)
err = suite.coordinator.UpdateClient(suite.chainB, suite.chainA, clientB, exported.Tendermint)
suite.Require().NoError(err)
// retrieve client state of chainA to pass as counterpartyClient
counterpartyClient = suite.chainA.GetClientState(clientA)
}, false},
}
@ -212,8 +242,7 @@ func (suite *KeeperTestSuite) TestConnOpenTry() {
if consensusHeight.IsZero() {
// retrieve consensus state height to provide proof for
clientState := suite.chainA.GetClientState(clientA)
consensusHeight = clientState.GetLatestHeight()
consensusHeight = counterpartyClient.GetLatestHeight()
}
consensusKey := host.FullKeyClientPath(clientA, host.KeyConsensusState(consensusHeight))
proofConsensus, _ := suite.chainA.QueryProof(consensusKey)
@ -338,6 +367,43 @@ func (suite *KeeperTestSuite) TestConnOpenAck() {
err = suite.coordinator.ConnOpenAck(suite.chainA, suite.chainB, connA, connB)
suite.Require().NoError(err)
}, false},
{"connection is in INIT but the proposed version is invalid", func() {
// chainA is in INIT, chainB is in TRYOPEN
clientA, clientB = suite.coordinator.SetupClients(suite.chainA, suite.chainB, exported.Tendermint)
connA, connB, err := suite.coordinator.ConnOpenInit(suite.chainA, suite.chainB, clientA, clientB)
suite.Require().NoError(err)
// retrieve client state of chainB to pass as counterpartyClient
counterpartyClient = suite.chainB.GetClientState(clientB)
err = suite.coordinator.ConnOpenTry(suite.chainB, suite.chainA, connB, connA)
suite.Require().NoError(err)
version = "2.0"
}, false},
{"connection is in TRYOPEN but the set version in the connection is invalid", func() {
// chainA is in TRYOPEN, chainB is in TRYOPEN
clientA, clientB = suite.coordinator.SetupClients(suite.chainA, suite.chainB, exported.Tendermint)
connB, connA, err := suite.coordinator.ConnOpenInit(suite.chainB, suite.chainA, clientB, clientA)
suite.Require().NoError(err)
err = suite.coordinator.ConnOpenTry(suite.chainA, suite.chainB, connA, connB)
suite.Require().NoError(err)
// set chainB to TRYOPEN
connection := suite.chainB.GetConnection(connB)
connection.State = types.TRYOPEN
suite.chainB.App.IBCKeeper.ConnectionKeeper.SetConnection(suite.chainB.GetContext(), connB.ID, connection)
// update clientB so state change is committed
suite.coordinator.UpdateClient(suite.chainB, suite.chainA, clientB, exported.Tendermint)
suite.coordinator.UpdateClient(suite.chainA, suite.chainB, clientA, exported.Tendermint)
// retrieve client state of chainB to pass as counterpartyClient
counterpartyClient = suite.chainB.GetClientState(clientB)
version = "2.0"
}, false},
{"incompatible IBC versions", func() {
clientA, clientB = suite.coordinator.SetupClients(suite.chainA, suite.chainB, exported.Tendermint)
connA, connB, err := suite.coordinator.ConnOpenInit(suite.chainA, suite.chainB, clientA, clientB)

24
x/ibc/03-connection/types/version.go
View File

@ -136,6 +136,27 @@ func GetCompatibleEncodedVersions() []string {
return versions
}
// IsSupportedVersion returns true if the proposed version has a matching version
// identifier and its entire feature set is supported or the version identifier
// supports an empty feature set.
func IsSupportedVersion(encodedProposedVersion string) bool {
proposedVersion, err := DecodeVersion(encodedProposedVersion)
if err != nil {
return false
}
supportedVersion, found := FindSupportedVersion(proposedVersion, GetCompatibleVersions())
if !found {
return false
}
if err := supportedVersion.VerifyProposedVersion(proposedVersion); err != nil {
return false
}
return true
}
// FindSupportedVersion returns the version with a matching version identifier
// if it exists. The returned boolean is true if the version is found and
// false otherwise.
@ -156,6 +177,9 @@ func FindSupportedVersion(version Version, supportedVersions []Version) (Version
// not allowed for the chosen version identifier then the search for a
// compatible version continues. This function is called in the ConnOpenTry
// handshake procedure.
//
// CONTRACT: PickVersion must only provide a version that is in the
// intersection of the supported versions and the counterparty versions.
func PickVersion(encodedCounterpartyVersions []string) (string, error) {
counterpartyVersions, err := DecodeVersions(encodedCounterpartyVersions)
if err != nil {

34
x/ibc/03-connection/types/version_test.go
View File

@ -59,6 +59,40 @@ func TestDecodeVersion(t *testing.T) {
}
}
func TestIsSupportedVersion(t *testing.T) {
testCases := []struct {
name string
version types.Version
expPass bool
}{
{
"version is supported",
types.GetCompatibleVersions()[0],
true,
},
{
"version is not supported",
types.Version{},
false,
},
{
"version feature is not supported",
types.NewVersion(types.DefaultIBCVersionIdentifier, []string{"ORDER_DAG"}),
false,
},
}
// test that a version that cannot be decoded does not pass
require.False(t, types.IsSupportedVersion("1.0"))
for _, tc := range testCases {
encodedVersion, err := tc.version.Encode()
require.NoError(t, err)
require.Equal(t, tc.expPass, types.IsSupportedVersion(encodedVersion))
}
}
func TestFindSupportedVersion(t *testing.T) {
testCases := []struct {
name string