Enforce Connection Features on Channel Opening (#6620)

* add channel open checks for supported feature

* add tests and fix bugs

* fix version tests

Co-authored-by: Federico Kunze <31522760+fedekunze@users.noreply.github.com>

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 {

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},
+		{"one feature missing", types.DefaultIBCVersion, types.CreateVersionString("1", []string{"ORDER_UNORDERED", "ORDER_DAG"}), false},
-		{"both features missing", types.DefaultIBCVersion, types.CreateVersionString("1", []string{"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)
+	}
+}

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)

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
+		connA    *ibctesting.TestConnection
-		connB   *ibctesting.TestConnection
+		connB    *ibctesting.TestConnection
-		portCap *capabilitytypes.Capability
+		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 {