diff --git a/x/ibc/03-connection/types/version.go b/x/ibc/03-connection/types/version.go index ee79267457..a6d60d803e 100644 --- a/x/ibc/03-connection/types/version.go +++ b/x/ibc/03-connection/types/version.go @@ -11,7 +11,7 @@ import ( var ( // DefaultIBCVersion represents the latest supported version of IBC. // The current version supports only ORDERED and UNORDERED channels. - DefaultIBCVersion = CreateVersionString("1", []string{"ORDERED", "UNORDERED"}) + DefaultIBCVersion = CreateVersionString("1", []string{"ORDER_ORDERED", "ORDER_UNORDERED"}) ) // GetCompatibleVersions returns a descending ordered set of compatible IBC @@ -161,6 +161,22 @@ func VerifyProposedFeatureSet(proposedVersion, supportedVersion string) bool { return true } +// VerifySupportedFeature takes in a version string and feature string and returns +// true if the feature is supported by the version and false otherwise. +func VerifySupportedFeature(version, feature string) bool { + _, featureSet, err := UnpackVersion(version) + if err != nil { + return false + } + + for _, f := range featureSet { + if f == feature { + return true + } + } + return false +} + // contains returns true if the provided string element exists within the // string set. func contains(elem string, set []string) bool { diff --git a/x/ibc/03-connection/types/version_test.go b/x/ibc/03-connection/types/version_test.go index 611c5c9bab..ed7ce8257c 100644 --- a/x/ibc/03-connection/types/version_test.go +++ b/x/ibc/03-connection/types/version_test.go @@ -99,10 +99,10 @@ func TestVerifyProposedFeatureSet(t *testing.T) { supportedVersion string expPass bool }{ - {"entire feature set supported", types.DefaultIBCVersion, types.CreateVersionString("1", []string{"ORDERED", "UNORDERED", "DAG"}), true}, + {"entire feature set supported", types.DefaultIBCVersion, types.CreateVersionString("1", []string{"ORDER_ORDERED", "ORDER_UNORDERED", "ORDER_DAG"}), true}, {"empty feature sets", types.CreateVersionString("1", []string{}), types.DefaultIBCVersion, true}, - {"one feature missing", types.DefaultIBCVersion, types.CreateVersionString("1", []string{"UNORDERED", "DAG"}), false}, - {"both features missing", types.DefaultIBCVersion, types.CreateVersionString("1", []string{"DAG"}), false}, + {"one feature missing", types.DefaultIBCVersion, types.CreateVersionString("1", []string{"ORDER_UNORDERED", "ORDER_DAG"}), false}, + {"both features missing", types.DefaultIBCVersion, types.CreateVersionString("1", []string{"ORDER_DAG"}), false}, } for i, tc := range testCases { @@ -112,3 +112,23 @@ func TestVerifyProposedFeatureSet(t *testing.T) { } } + +func TestVerifySupportedFeature(t *testing.T) { + testCases := []struct { + name string + version string + feature string + expPass bool + }{ + {"check ORDERED supported", types.DefaultIBCVersion, "ORDER_ORDERED", true}, + {"check UNORDERED supported", types.DefaultIBCVersion, "ORDER_UNORDERED", true}, + {"check DAG unsupported", types.DefaultIBCVersion, "ORDER_DAG", false}, + {"check empty feature set returns false", types.CreateVersionString("1", []string{}), "ORDER_ORDERED", false}, + } + + for i, tc := range testCases { + supported := types.VerifySupportedFeature(tc.version, tc.feature) + + require.Equal(t, tc.expPass, supported, "test case %d: %s", i, tc.name) + } +} diff --git a/x/ibc/04-channel/keeper/handshake.go b/x/ibc/04-channel/keeper/handshake.go index 46b922fe96..4288f1bd2f 100644 --- a/x/ibc/04-channel/keeper/handshake.go +++ b/x/ibc/04-channel/keeper/handshake.go @@ -59,6 +59,22 @@ func (k Keeper) ChanOpenInit( ) } + if len(connectionEnd.GetVersions()) != 1 { + return nil, sdkerrors.Wrapf( + connectiontypes.ErrInvalidVersion, + "single version must be negotiated on connection before opening channel, got: %v", + connectionEnd.GetVersions(), + ) + } + + if !connectiontypes.VerifySupportedFeature(connectionEnd.GetVersions()[0], order.String()) { + return nil, sdkerrors.Wrapf( + connectiontypes.ErrInvalidVersion, + "connection version %s does not support channel ordering: %s", + connectionEnd.GetVersions()[0], order.String(), + ) + } + if !k.portKeeper.Authenticate(ctx, portCap, portID) { return nil, sdkerrors.Wrap(porttypes.ErrInvalidPort, "caller does not own port capability") } @@ -121,6 +137,22 @@ func (k Keeper) ChanOpenTry( ) } + if len(connectionEnd.GetVersions()) != 1 { + return nil, sdkerrors.Wrapf( + connectiontypes.ErrInvalidVersion, + "single version must be negotiated on connection before opening channel, got: %v", + connectionEnd.GetVersions(), + ) + } + + if !connectiontypes.VerifySupportedFeature(connectionEnd.GetVersions()[0], order.String()) { + return nil, sdkerrors.Wrapf( + connectiontypes.ErrInvalidVersion, + "connection version %s does not support channel ordering: %s", + connectionEnd.GetVersions()[0], order.String(), + ) + } + // NOTE: this step has been switched with the one below to reverse the connection // hops channel := types.NewChannel(types.TRYOPEN, order, counterparty, connectionHops, version) diff --git a/x/ibc/04-channel/keeper/handshake_test.go b/x/ibc/04-channel/keeper/handshake_test.go index cc48cf4ab9..9f9c5891fd 100644 --- a/x/ibc/04-channel/keeper/handshake_test.go +++ b/x/ibc/04-channel/keeper/handshake_test.go @@ -23,14 +23,16 @@ type testCase = struct { // can succeed. func (suite *KeeperTestSuite) TestChanOpenInit() { var ( - connA *ibctesting.TestConnection - connB *ibctesting.TestConnection - portCap *capabilitytypes.Capability + connA *ibctesting.TestConnection + connB *ibctesting.TestConnection + features []string + portCap *capabilitytypes.Capability ) testCases := []testCase{ {"success", func() { _, _, connA, connB = suite.coordinator.SetupClientConnections(suite.chainA, suite.chainB, clientexported.Tendermint) + features = []string{"ORDER_ORDERED", "ORDER_UNORDERED"} suite.chainA.CreatePortCapability(connA.NextTestChannel().PortID) portCap = suite.chainA.GetPortCapability(connA.NextTestChannel().PortID) }, true}, @@ -56,8 +58,38 @@ func (suite *KeeperTestSuite) TestChanOpenInit() { }, false}, {"capability is incorrect", func() { _, _, connA, connB = suite.coordinator.SetupClientConnections(suite.chainA, suite.chainB, clientexported.Tendermint) + features = []string{"ORDER_ORDERED", "ORDER_UNORDERED"} portCap = capabilitytypes.NewCapability(3) }, false}, + {"connection version not negotiated", func() { + _, _, connA, connB = suite.coordinator.SetupClientConnections(suite.chainA, suite.chainB, clientexported.Tendermint) + + // modify connA versions + conn := suite.chainA.GetConnection(connA) + conn.Versions = append(conn.Versions, connectiontypes.CreateVersionString("2", []string{"ORDER_ORDERED", "ORDER_UNORDERED"})) + suite.chainA.App.IBCKeeper.ConnectionKeeper.SetConnection( + suite.chainA.GetContext(), + connA.ID, conn, + ) + features = []string{"ORDER_ORDERED", "ORDER_UNORDERED"} + suite.chainA.CreatePortCapability(connA.NextTestChannel().PortID) + portCap = suite.chainA.GetPortCapability(connA.NextTestChannel().PortID) + }, false}, + {"connection does not support ORDERED channels", func() { + _, _, connA, connB = suite.coordinator.SetupClientConnections(suite.chainA, suite.chainB, clientexported.Tendermint) + + // modify connA versions to only support UNORDERED channels + conn := suite.chainA.GetConnection(connA) + conn.Versions = []string{connectiontypes.CreateVersionString("1", []string{"ORDER_UNORDERED"})} + suite.chainA.App.IBCKeeper.ConnectionKeeper.SetConnection( + suite.chainA.GetContext(), + connA.ID, conn, + ) + // NOTE: Opening UNORDERED channels is still expected to pass but ORDERED channels should fail + features = []string{"ORDER_UNORDERED"} + suite.chainA.CreatePortCapability(connA.NextTestChannel().PortID) + portCap = suite.chainA.GetPortCapability(connA.NextTestChannel().PortID) + }, true}, } for _, tc := range testCases { @@ -76,7 +108,17 @@ func (suite *KeeperTestSuite) TestChanOpenInit() { channelA.PortID, channelA.ID, portCap, counterparty, ibctesting.ChannelVersion, ) - if tc.expPass { + // check if order is supported by channel to determine expected behaviour + orderSupported := false + for _, f := range features { + if f == order.String() { + orderSupported = true + } + } + + // Testcase must have expectedPass = true AND channel order supported before + // asserting the channel handshake initiation succeeded + if tc.expPass && orderSupported { suite.Require().NoError(err) suite.Require().NotNil(cap) @@ -159,6 +201,34 @@ func (suite *KeeperTestSuite) TestChanOpenTry() { portCap = capabilitytypes.NewCapability(3) }, false}, + {"connection version not negotiated", func() { + _, _, connA, connB = suite.coordinator.SetupClientConnections(suite.chainA, suite.chainB, clientexported.Tendermint) + suite.coordinator.ChanOpenInit(suite.chainA, suite.chainB, connA, connB, types.ORDERED) + + // modify connB versions + conn := suite.chainB.GetConnection(connB) + conn.Versions = append(conn.Versions, connectiontypes.CreateVersionString("2", []string{"ORDER_ORDERED", "ORDER_UNORDERED"})) + suite.chainB.App.IBCKeeper.ConnectionKeeper.SetConnection( + suite.chainB.GetContext(), + connB.ID, conn, + ) + suite.chainB.CreatePortCapability(connB.NextTestChannel().PortID) + portCap = suite.chainB.GetPortCapability(connB.NextTestChannel().PortID) + }, false}, + {"connection does not support ORDERED channels", func() { + _, _, connA, connB = suite.coordinator.SetupClientConnections(suite.chainA, suite.chainB, clientexported.Tendermint) + suite.coordinator.ChanOpenInit(suite.chainA, suite.chainB, connA, connB, types.ORDERED) + + // modify connA versions to only support UNORDERED channels + conn := suite.chainA.GetConnection(connA) + conn.Versions = []string{connectiontypes.CreateVersionString("1", []string{"ORDER_UNORDERED"})} + suite.chainA.App.IBCKeeper.ConnectionKeeper.SetConnection( + suite.chainA.GetContext(), + connA.ID, conn, + ) + suite.chainA.CreatePortCapability(connA.NextTestChannel().PortID) + portCap = suite.chainA.GetPortCapability(connA.NextTestChannel().PortID) + }, false}, } for _, tc := range testCases {