Standardize connection versioning + channel versioning docs

docs/ibc/custom.md

@@ -142,6 +142,21 @@ OnChanCloseConfirm(
 }
 ```
 
+#### Channel Handshake Version Negotiation
+
+Application modules are expected to verify versioning used during the channel handshake procedure.
+
+* `ChanOpenInit` callback should verify that the `MsgChanOpenInit.Version` is valid
+* `ChanOpenTry` callback should verify that the `MsgChanOpenTry.Version` is valid and that `MsgChanOpenTry.CounterpartyVersion` is valid.
+* `ChanOpenAck` callback should verify that the `MsgChanOpenAck.CounterpartyVersion` is valid and supported.
+
+Versions must be strings but can implement any versioning structure. If your application plans to
+have linear releases then semantic versioning is recommended. If your application plans to release
+various features in between major releases then it is advised to use the same versioning scheme
+as IBC. This versioning scheme specifies a version identifier and compatible feature set with
+that identifier. Valid version selection includes selecting a compatible version identifier with
+a subset of features supported by your application for that version.
+
 ### Bind Ports
 
 Currently, ports must be bound on app initialization. A module may bind to ports in `InitGenesis`

proto/ibc/connection/connection.proto

@@ -9,70 +9,97 @@ import "ibc/commitment/commitment.proto";
 // MsgConnectionOpenInit defines the msg sent by an account on Chain A to
 // initialize a connection with Chain B.
 message MsgConnectionOpenInit {
-  string       client_id     = 1 [(gogoproto.customname) = "ClientID", (gogoproto.moretags) = "yaml:\"client_id\""];
-  string       connection_id = 2 [(gogoproto.customname) = "ConnectionID", (gogoproto.moretags) = "yaml:\"connection_id\""];
-  Counterparty counterparty  = 3 [(gogoproto.nullable) = false];
-  bytes        signer        = 4 [(gogoproto.casttype) = "github.com/cosmos/cosmos-sdk/types.AccAddress"];
+  string client_id = 1 [
+    (gogoproto.customname) = "ClientID",
+    (gogoproto.moretags) = "yaml:\"client_id\""
+  ];
+  string connection_id = 2 [
+    (gogoproto.customname) = "ConnectionID",
+    (gogoproto.moretags) = "yaml:\"connection_id\""
+  ];
+  Counterparty counterparty = 3 [(gogoproto.nullable) = false];
+  bytes signer = 4
+      [(gogoproto.casttype) = "github.com/cosmos/cosmos-sdk/types.AccAddress"];
 }
 
-// MsgConnectionOpenTry defines a msg sent by a Relayer to try to open a connection
-// on Chain B.
+// MsgConnectionOpenTry defines a msg sent by a Relayer to try to open a
+// connection on Chain B.
 message MsgConnectionOpenTry {
-  string          client_id             = 1 [(gogoproto.customname) = "ClientID", (gogoproto.moretags) = "yaml:\"client_id\""];
-  string          connection_id         = 2 [(gogoproto.customname) = "ConnectionID", (gogoproto.moretags) = "yaml:\"connection_id\""];
-  Counterparty    counterparty          = 3 [(gogoproto.nullable) = false];
-  repeated string counterparty_versions = 4 [(gogoproto.moretags) = "yaml:\"counterparty_versions\""];
-  // proof of the initialization the connection on Chain A: `UNITIALIZED -> INIT`
-  bytes  proof_init   = 5 [(gogoproto.moretags) = "yaml:\"proof_init\""];
+  string client_id = 1 [
+    (gogoproto.customname) = "ClientID",
+    (gogoproto.moretags) = "yaml:\"client_id\""
+  ];
+  string connection_id = 2 [
+    (gogoproto.customname) = "ConnectionID",
+    (gogoproto.moretags) = "yaml:\"connection_id\""
+  ];
+  Counterparty counterparty = 3 [(gogoproto.nullable) = false];
+  repeated string counterparty_versions = 4 [
+    (gogoproto.moretags) = "yaml:\"counterparty_versions\""
+  ];
+  // proof of the initialization the connection on Chain A: `UNITIALIZED ->
+  // INIT`
+  bytes proof_init = 5 [(gogoproto.moretags) = "yaml:\"proof_init\""];
   uint64 proof_height = 6;
   // proof of client consensus state
-  bytes proof_consensus = 7
-      [(gogoproto.moretags) = "yaml:\"proof_consensus\""];
-  uint64 consensus_height = 8 [(gogoproto.moretags) = "yaml:\"consensus_height\""];
-  bytes  signer           = 9 [(gogoproto.casttype) = "github.com/cosmos/cosmos-sdk/types.AccAddress"];
+  bytes proof_consensus = 7 [(gogoproto.moretags) = "yaml:\"proof_consensus\""];
+  uint64 consensus_height = 8
+      [(gogoproto.moretags) = "yaml:\"consensus_height\""];
+  bytes signer = 9
+      [(gogoproto.casttype) = "github.com/cosmos/cosmos-sdk/types.AccAddress"];
 }
 
-// MsgConnectionOpenAck defines a msg sent by a Relayer to Chain A to acknowledge
-// the change of connection state to TRYOPEN on Chain B.
+// MsgConnectionOpenAck defines a msg sent by a Relayer to Chain A to
+// acknowledge the change of connection state to TRYOPEN on Chain B.
 message MsgConnectionOpenAck {
-  string connection_id = 1 [(gogoproto.customname) = "ConnectionID", (gogoproto.moretags) = "yaml:\"connection_id\""];
-  string version       = 2;
-  // proof of the initialization the connection on Chain B: `UNITIALIZED -> TRYOPEN`
-  bytes proof_try = 3
-      [(gogoproto.moretags) = "yaml:\"proof_try\""];
+  string connection_id = 1 [
+    (gogoproto.customname) = "ConnectionID",
+    (gogoproto.moretags) = "yaml:\"connection_id\""
+  ];
+  string version = 2;
+  // proof of the initialization the connection on Chain B: `UNITIALIZED ->
+  // TRYOPEN`
+  bytes proof_try = 3 [(gogoproto.moretags) = "yaml:\"proof_try\""];
   uint64 proof_height = 4 [(gogoproto.moretags) = "yaml:\"proof_height\""];
   // proof of client consensus state
-  bytes proof_consensus = 5
-      [(gogoproto.moretags) = "yaml:\"proof_consensus\""];
-  uint64 consensus_height = 6 [(gogoproto.moretags) = "yaml:\"consensus_height\""];
-  bytes  signer           = 7 [(gogoproto.casttype) = "github.com/cosmos/cosmos-sdk/types.AccAddress"];
+  bytes proof_consensus = 5 [(gogoproto.moretags) = "yaml:\"proof_consensus\""];
+  uint64 consensus_height = 6
+      [(gogoproto.moretags) = "yaml:\"consensus_height\""];
+  bytes signer = 7
+      [(gogoproto.casttype) = "github.com/cosmos/cosmos-sdk/types.AccAddress"];
 }
 
-// MsgConnectionOpenConfirm defines a msg sent by a Relayer to Chain B to acknowledge
-// the change of connection state to OPEN on Chain A.
+// MsgConnectionOpenConfirm defines a msg sent by a Relayer to Chain B to
+// acknowledge the change of connection state to OPEN on Chain A.
 message MsgConnectionOpenConfirm {
-  string connection_id = 1 [(gogoproto.customname) = "ConnectionID", (gogoproto.moretags) = "yaml:\"connection_id\""];
+  string connection_id = 1 [
+    (gogoproto.customname) = "ConnectionID",
+    (gogoproto.moretags) = "yaml:\"connection_id\""
+  ];
   // proof for the change of the connection state on Chain A: `INIT -> OPEN`
-  bytes proof_ack = 2
-      [(gogoproto.moretags) = "yaml:\"proof_ack\""];
+  bytes proof_ack = 2 [(gogoproto.moretags) = "yaml:\"proof_ack\""];
   uint64 proof_height = 3 [(gogoproto.moretags) = "yaml:\"proof_height\""];
-  bytes  signer       = 4 [(gogoproto.casttype) = "github.com/cosmos/cosmos-sdk/types.AccAddress"];
+  bytes signer = 4
+      [(gogoproto.casttype) = "github.com/cosmos/cosmos-sdk/types.AccAddress"];
 }
 
 // ICS03 - Connection Data Structures as defined in
 // https://github.com/cosmos/ics/tree/master/spec/ics-003-connection-semantics#data-structures
 
-// ConnectionEnd defines a stateful object on a chain connected to another separate
-// one.
-// NOTE: there must only be 2 defined ConnectionEnds to establish a connection
-// between two chains.
+// ConnectionEnd defines a stateful object on a chain connected to another
+// separate one. NOTE: there must only be 2 defined ConnectionEnds to establish
+// a connection between two chains.
 message ConnectionEnd {
   option (gogoproto.goproto_getters) = false;
   // connection identifier.
-  string id = 1 [(gogoproto.customname) = "ID", (gogoproto.moretags) = "yaml:\"id\""];
+  string id = 1
+      [(gogoproto.customname) = "ID", (gogoproto.moretags) = "yaml:\"id\""];
   // client associated with this connection.
-  string client_id = 2 [(gogoproto.customname) = "ClientID", (gogoproto.moretags) = "yaml:\"client_id\""];
-  // opaque string which can be utilised to determine encodings or protocols for
+  string client_id = 2 [
+    (gogoproto.customname) = "ClientID",
+    (gogoproto.moretags) = "yaml:\"client_id\""
+  ];
+  // IBC version which can be utilised to determine encodings or protocols for
   // channels or packets utilising this connection
   repeated string versions = 3;
   // current state of the connection end.
@@ -87,10 +114,12 @@ enum State {
   option (gogoproto.goproto_enum_prefix) = false;
 
   // Default State
-  STATE_UNINITIALIZED_UNSPECIFIED = 0 [(gogoproto.enumvalue_customname) = "UNINITIALIZED"];
+  STATE_UNINITIALIZED_UNSPECIFIED = 0
+      [(gogoproto.enumvalue_customname) = "UNINITIALIZED"];
   // A connection end has just started the opening handshake.
   STATE_INIT = 1 [(gogoproto.enumvalue_customname) = "INIT"];
-  // A connection end has acknowledged the handshake step on the counterparty chain.
+  // A connection end has acknowledged the handshake step on the counterparty
+  // chain.
   STATE_TRYOPEN = 2 [(gogoproto.enumvalue_customname) = "TRYOPEN"];
   // A connection end has completed the handshake.
   STATE_OPEN = 3 [(gogoproto.enumvalue_customname) = "OPEN"];
@@ -100,10 +129,18 @@ enum State {
 message Counterparty {
   option (gogoproto.goproto_getters) = false;
 
-  // identifies the client on the counterparty chain associated with a given connection.
-  string client_id = 1 [(gogoproto.customname) = "ClientID", (gogoproto.moretags) = "yaml:\"client_id\""];
-  // identifies the connection end on the counterparty chain associated with a given connection.
-  string connection_id = 2 [(gogoproto.customname) = "ConnectionID", (gogoproto.moretags) = "yaml:\"connection_id\""];
+  // identifies the client on the counterparty chain associated with a given
+  // connection.
+  string client_id = 1 [
+    (gogoproto.customname) = "ClientID",
+    (gogoproto.moretags) = "yaml:\"client_id\""
+  ];
+  // identifies the connection end on the counterparty chain associated with a
+  // given connection.
+  string connection_id = 2 [
+    (gogoproto.customname) = "ConnectionID",
+    (gogoproto.moretags) = "yaml:\"connection_id\""
+  ];
   // commitment merkle prefix of the counterparty chain
   ibc.commitment.MerklePrefix prefix = 3 [(gogoproto.nullable) = false];
 }
@@ -117,7 +154,21 @@ message ClientPaths {
 // ConnectionPaths define all the connection paths for a given client state.
 message ConnectionPaths {
   // client state unique identifier
-  string client_id = 1 [(gogoproto.customname) = "ClientID", (gogoproto.moretags) = "yaml:\"client_id\""];
+  string client_id = 1 [
+    (gogoproto.customname) = "ClientID",
+    (gogoproto.moretags) = "yaml:\"client_id\""
+  ];
   // list of connection paths
   repeated string paths = 2;
 }
+
+// Version defines the versioning scheme used to negotiate the IBC verison in
+// the connection handshake.
+message Version {
+  option (gogoproto.goproto_getters) = false;
+
+  // unique version identifier
+  string identifier = 1;
+  // list of features compatible with the specified identifier
+  repeated string features = 2;
+}

x/ibc-transfer/handler_test.go

@@ -284,7 +284,7 @@ func (chain *TestChain) createConnection(
 		State:        state,
 		ClientID:     clientID,
 		Counterparty: counterparty,
-		Versions:     connectiontypes.GetCompatibleVersions(),
+		Versions:     connectiontypes.GetCompatibleVersionStrings(),
 	}
 	ctx := chain.GetContext()
 	chain.App.IBCKeeper.ConnectionKeeper.SetConnection(ctx, connID, connection)

x/ibc-transfer/keeper/keeper_test.go

@@ -259,7 +259,7 @@ func (chain *TestChain) createConnection(
 		State:        state,
 		ClientID:     clientID,
 		Counterparty: counterparty,
-		Versions:     connectiontypes.GetCompatibleVersions(),
+		Versions:     connectiontypes.GetCompatibleVersionStrings(),
 	}
 	ctx := chain.GetContext()
 	chain.App.IBCKeeper.ConnectionKeeper.SetConnection(ctx, connID, connection)

x/ibc-transfer/module.go

@@ -195,7 +195,7 @@ func (am AppModule) OnChanOpenInit(
 	}
 
 	if version != types.Version {
-		return sdkerrors.Wrapf(types.ErrInvalidVersion, "got: %s, expected %s", version, types.Version)
+		return sdkerrors.Wrapf(types.ErrInvalidVersion, "got %s, expected %s", version, types.Version)
 	}
 
 	// Claim channel capability passed back by IBC module

x/ibc-transfer/types/errors.go

@@ -9,5 +9,5 @@ var (
 	ErrInvalidPacketTimeout    = sdkerrors.Register(ModuleName, 2, "invalid packet timeout")
 	ErrOnlyOneDenomAllowed     = sdkerrors.Register(ModuleName, 3, "only one denom allowed")
 	ErrInvalidDenomForTransfer = sdkerrors.Register(ModuleName, 4, "invalid denomination for cross-chain transfer")
-	ErrInvalidVersion          = sdkerrors.Register(ModuleName, 5, "invalid version")
+	ErrInvalidVersion          = sdkerrors.Register(ModuleName, 5, "invalid ICS20 version")
 )

x/ibc/03-connection/keeper/grpc_query_test.go

@@ -50,7 +50,7 @@ func (suite *KeeperTestSuite) TestQueryConnection() {
 				connB := suite.chainB.GetFirstTestConnection(clientB, clientA)
 
 				counterparty := types.NewCounterparty(clientB, connB.ID, suite.chainB.GetPrefix())
-				expConnection = types.NewConnectionEnd(types.INIT, connA.ID, clientA, counterparty, types.GetCompatibleVersions())
+				expConnection = types.NewConnectionEnd(types.INIT, connA.ID, clientA, counterparty, types.GetCompatibleVersionStrings())
 				suite.chainA.App.IBCKeeper.ConnectionKeeper.SetConnection(suite.chainA.GetContext(), connA.ID, expConnection)
 
 				req = &types.QueryConnectionRequest{
@@ -119,9 +119,9 @@ func (suite *KeeperTestSuite) TestQueryConnections() {
 				counterparty2 := types.NewCounterparty(clientB, connB1.ID, suite.chainB.GetPrefix())
 				counterparty3 := types.NewCounterparty(clientB1, connB2.ID, suite.chainB.GetPrefix())
 
-				conn1 := types.NewConnectionEnd(types.OPEN, connA0.ID, clientA, counterparty1, types.GetCompatibleVersions())
-				conn2 := types.NewConnectionEnd(types.INIT, connA1.ID, clientA, counterparty2, types.GetCompatibleVersions())
-				conn3 := types.NewConnectionEnd(types.OPEN, connA2.ID, clientA1, counterparty3, types.GetCompatibleVersions())
+				conn1 := types.NewConnectionEnd(types.OPEN, connA0.ID, clientA, counterparty1, types.GetCompatibleVersionStrings())
+				conn2 := types.NewConnectionEnd(types.INIT, connA1.ID, clientA, counterparty2, types.GetCompatibleVersionStrings())
+				conn3 := types.NewConnectionEnd(types.OPEN, connA2.ID, clientA1, counterparty3, types.GetCompatibleVersionStrings())
 
 				expConnections = []*types.ConnectionEnd{&conn1, &conn2, &conn3}
 

x/ibc/03-connection/keeper/handshake.go

@@ -26,7 +26,7 @@ func (k Keeper) ConnOpenInit(
 	}
 
 	// connection defines chain A's ConnectionEnd
-	connection := types.NewConnectionEnd(types.INIT, connectionID, clientID, counterparty, types.GetCompatibleVersions())
+	connection := types.NewConnectionEnd(types.INIT, connectionID, clientID, counterparty, types.GetCompatibleVersionStrings())
 	k.SetConnection(ctx, connectionID, connection)
 
 	if err := k.addConnectionToClient(ctx, clientID, connectionID); err != nil {
@@ -128,7 +128,7 @@ func (k Keeper) ConnOpenTry(
 func (k Keeper) ConnOpenAck(
 	ctx sdk.Context,
 	connectionID string,
-	version string, // version that ChainB chose in ConnOpenTry
+	encodedVersion string, // version that ChainB chose in ConnOpenTry
 	proofTry []byte, // proof that connectionEnd was added to ChainB state in ConnOpenTry
 	proofConsensus []byte, // proof that chainB has stored ConsensusState of chainA on its client
 	proofHeight uint64, // height that relayer constructed proofTry
@@ -156,7 +156,12 @@ func (k Keeper) ConnOpenAck(
 		)
 	}
 
-	// Check that ChainB's proposed version number is supported by chainA
+	// Check that ChainB's proposed version identifier is supported by chainA
+	version, err := types.StringToVersion(encodedVersion)
+	if err != nil {
+		return sdkerrors.Wrap(err, "version negotiation failed")
+	}
+
 	supportedVersion, found := types.FindSupportedVersion(version, types.GetCompatibleVersions())
 	if !found {
 		return sdkerrors.Wrapf(
@@ -166,11 +171,8 @@ func (k Keeper) ConnOpenAck(
 	}
 
 	// Check that ChainB's proposed feature set is supported by chainA
-	if !types.VerifyProposedFeatureSet(version, supportedVersion) {
-		return sdkerrors.Wrapf(
-			types.ErrVersionNegotiationFailed,
-			"connection version feature set provided (%s) is not supported (%s)", version, types.GetCompatibleVersions(),
-		)
+	if err := types.VerifyProposedVersion(version, supportedVersion); err != nil {
+		return err
 	}
 
 	// Retrieve chainA's consensus state at consensusheight
@@ -181,7 +183,7 @@ func (k Keeper) ConnOpenAck(
 
 	prefix := k.GetCommitmentPrefix()
 	expectedCounterparty := types.NewCounterparty(connection.ClientID, connectionID, commitmenttypes.NewMerklePrefix(prefix.Bytes()))
-	expectedConnection := types.NewConnectionEnd(types.TRYOPEN, connection.Counterparty.ConnectionID, connection.Counterparty.ClientID, expectedCounterparty, []string{version})
+	expectedConnection := types.NewConnectionEnd(types.TRYOPEN, connection.Counterparty.ConnectionID, connection.Counterparty.ClientID, expectedCounterparty, []string{encodedVersion})
 
 	// Ensure that ChainB stored expected connectionEnd in its state during ConnOpenTry
 	if err := k.VerifyConnectionState(
@@ -202,7 +204,7 @@ func (k Keeper) ConnOpenAck(
 
 	// Update connection state to Open
 	connection.State = types.OPEN
-	connection.Versions = []string{version}
+	connection.Versions = []string{encodedVersion}
 	k.SetConnection(ctx, connectionID, connection)
 	return nil
 }

x/ibc/03-connection/keeper/handshake_test.go

@@ -102,7 +102,9 @@ func (suite *KeeperTestSuite) TestConnOpenTry() {
 			_, _, err := suite.coordinator.ConnOpenInit(suite.chainA, suite.chainB, clientA, clientB)
 			suite.Require().NoError(err)
 
-			versions = []string{"(version won't match,[])"}
+			version, err := types.NewVersion("0.0", nil).ToString()
+			suite.Require().NoError(err)
+			versions = []string{version}
 		}, false},
 		{"connection state verification failed", func() {
 			clientA, clientB = suite.coordinator.SetupClients(suite.chainA, suite.chainB, clientexported.Tendermint)
@@ -140,9 +142,9 @@ func (suite *KeeperTestSuite) TestConnOpenTry() {
 		tc := tc
 
 		suite.Run(tc.msg, func() {
-			suite.SetupTest()                        // reset
-			consensusHeight = 0                      // must be explicitly changed in malleate
-			versions = types.GetCompatibleVersions() // must be explicitly changed in malleate
+			suite.SetupTest()                              // reset
+			consensusHeight = 0                            // must be explicitly changed in malleate
+			versions = types.GetCompatibleVersionStrings() // must be explicitly changed in malleate
 
 			tc.malleate()
 
@@ -266,6 +268,17 @@ func (suite *KeeperTestSuite) TestConnOpenAck() {
 
 			version = ""
 		}, false},
+		{"feature set verification failed - unsupported feature", func() {
+			clientA, clientB = suite.coordinator.SetupClients(suite.chainA, suite.chainB, clientexported.Tendermint)
+			connA, connB, err := suite.coordinator.ConnOpenInit(suite.chainA, suite.chainB, clientA, clientB)
+			suite.Require().NoError(err)
+
+			err = suite.coordinator.ConnOpenTry(suite.chainB, suite.chainA, connB, connA)
+			suite.Require().NoError(err)
+
+			version, err = types.NewVersion(types.DefaultIBCVersionIdentifier, []string{"ORDER_ORDERED", "ORDER_UNORDERED", "ORDER_DAG"}).ToString()
+			suite.Require().NoError(err)
+		}, false},
 		{"self consensus state not found", func() {
 			clientA, clientB = suite.coordinator.SetupClients(suite.chainA, suite.chainB, clientexported.Tendermint)
 			connA, connB, err := suite.coordinator.ConnOpenInit(suite.chainA, suite.chainB, clientA, clientB)
@@ -305,9 +318,9 @@ func (suite *KeeperTestSuite) TestConnOpenAck() {
 	for _, tc := range testCases {
 		tc := tc
 		suite.Run(tc.msg, func() {
-			suite.SetupTest()                          // reset
-			version = types.GetCompatibleVersions()[0] // must be explicitly changed in malleate
-			consensusHeight = 0                        // must be explicitly changed in malleate
+			suite.SetupTest()                                // reset
+			version = types.GetCompatibleVersionStrings()[0] // must be explicitly changed in malleate
+			consensusHeight = 0                              // must be explicitly changed in malleate
 
 			tc.malleate()