Deterministic connection and channel identifiers (#7993)

* add generate identifier functions for connection and channel

* update proto types and begin open init changes

remove unnecessary field from open init msgs. Update connectionopeninit handshake, begin updating channel handshake

* finish connection handshake chanes and some channel handshake changes

Finish updating connection handshake changes, update genesis for connection and channel, some channel handshake changes, fix build for tests but still failing

* finish channel handshake changes

* fix more tests

* fix connection handshake tests

* fix more tests

* fix build

* changes from self review

* address @fedekunze review suggestions

* update spec

* fix build

* fix tests

* Update x/ibc/testing/chain.go

Co-authored-by: Aditya <adityasripal@gmail.com>

* Update x/ibc/core/03-connection/types/msgs.go

Co-authored-by: Aditya <adityasripal@gmail.com>

* address @AdityaSripal comments

* reflect spec changes

* address @AdityaSripal review suggestions

* nit

* add connection/channel identifier parsing and validation as per @AdityaSripal suggestion, cc @fedekunze

* Fix auth rest test

Change dummy channel id to a valid channel id given the updated stricter channel identifier checks cc @amaurymartiny

Co-authored-by: Aditya <adityasripal@gmail.com>

proto/ibc/core/channel/v1/genesis.proto

@@ -18,6 +18,8 @@ message GenesisState {
       [(gogoproto.nullable) = false, (gogoproto.moretags) = "yaml:\"recv_sequences\""];
   repeated PacketSequence ack_sequences = 7
       [(gogoproto.nullable) = false, (gogoproto.moretags) = "yaml:\"ack_sequences\""];
+  // the sequence for the next generated channel identifier
+  uint64 next_channel_sequence = 8 [(gogoproto.moretags) = "yaml:\"next_channel_sequence\""];
 }
 
 // PacketSequence defines the genesis type necessary to retrieve and store

proto/ibc/core/channel/v1/tx.proto

@@ -46,10 +46,9 @@ message MsgChannelOpenInit {
   option (gogoproto.equal)           = false;
   option (gogoproto.goproto_getters) = false;
 
-  string  port_id    = 1 [(gogoproto.moretags) = "yaml:\"port_id\""];
-  string  channel_id = 2 [(gogoproto.moretags) = "yaml:\"channel_id\""];
-  Channel channel    = 3 [(gogoproto.nullable) = false];
-  string  signer     = 4;
+  string  port_id = 1 [(gogoproto.moretags) = "yaml:\"port_id\""];
+  Channel channel = 2 [(gogoproto.nullable) = false];
+  string  signer  = 3;
 }
 
 // MsgChannelOpenInitResponse defines the Msg/ChannelOpenInit response type.
@@ -61,15 +60,16 @@ message MsgChannelOpenTry {
   option (gogoproto.equal)           = false;
   option (gogoproto.goproto_getters) = false;
 
-  string  port_id                        = 1 [(gogoproto.moretags) = "yaml:\"port_id\""];
-  string  desired_channel_id             = 2 [(gogoproto.moretags) = "yaml:\"desired_channel_id\""];
-  string  counterparty_chosen_channel_id = 3 [(gogoproto.moretags) = "yaml:\"counterparty_chosen_channel_id\""];
-  Channel channel                        = 4 [(gogoproto.nullable) = false];
-  string  counterparty_version           = 5 [(gogoproto.moretags) = "yaml:\"counterparty_version\""];
-  bytes   proof_init                     = 6 [(gogoproto.moretags) = "yaml:\"proof_init\""];
-  ibc.core.client.v1.Height proof_height = 7
+  string port_id = 1 [(gogoproto.moretags) = "yaml:\"port_id\""];
+  // in the case of crossing hello's, when both chains call OpenInit, we need the channel identifier
+  // of the previous channel in state INIT
+  string                    previous_channel_id  = 2 [(gogoproto.moretags) = "yaml:\"previous_channel_id\""];
+  Channel                   channel              = 3 [(gogoproto.nullable) = false];
+  string                    counterparty_version = 4 [(gogoproto.moretags) = "yaml:\"counterparty_version\""];
+  bytes                     proof_init           = 5 [(gogoproto.moretags) = "yaml:\"proof_init\""];
+  ibc.core.client.v1.Height proof_height         = 6
       [(gogoproto.moretags) = "yaml:\"proof_height\"", (gogoproto.nullable) = false];
-  string signer = 8;
+  string signer = 7;
 }
 
 // MsgChannelOpenTryResponse defines the Msg/ChannelOpenTry response type.
@@ -147,9 +147,9 @@ message MsgRecvPacket {
   option (gogoproto.equal)           = false;
   option (gogoproto.goproto_getters) = false;
 
-  Packet                    packet       = 1 [(gogoproto.nullable) = false];
-  bytes                     proof_commitment        = 2 [(gogoproto.moretags) = "yaml:\"proof_commitment\""];
-  ibc.core.client.v1.Height proof_height = 3
+  Packet                    packet           = 1 [(gogoproto.nullable) = false];
+  bytes                     proof_commitment = 2 [(gogoproto.moretags) = "yaml:\"proof_commitment\""];
+  ibc.core.client.v1.Height proof_height     = 3
       [(gogoproto.moretags) = "yaml:\"proof_height\"", (gogoproto.nullable) = false];
   string signer = 4;
 }
@@ -162,9 +162,9 @@ message MsgTimeout {
   option (gogoproto.equal)           = false;
   option (gogoproto.goproto_getters) = false;
 
-  Packet                    packet       = 1 [(gogoproto.nullable) = false];
-  bytes                     proof_unreceived        = 2 [(gogoproto.moretags) = "yaml:\"proof_unreceived\""];
-  ibc.core.client.v1.Height proof_height = 3
+  Packet                    packet           = 1 [(gogoproto.nullable) = false];
+  bytes                     proof_unreceived = 2 [(gogoproto.moretags) = "yaml:\"proof_unreceived\""];
+  ibc.core.client.v1.Height proof_height     = 3
       [(gogoproto.moretags) = "yaml:\"proof_height\"", (gogoproto.nullable) = false];
   uint64 next_sequence_recv = 4 [(gogoproto.moretags) = "yaml:\"next_sequence_recv\""];
   string signer             = 5;
@@ -178,10 +178,10 @@ message MsgTimeoutOnClose {
   option (gogoproto.equal)           = false;
   option (gogoproto.goproto_getters) = false;
 
-  Packet                    packet       = 1 [(gogoproto.nullable) = false];
-  bytes                     proof_unreceived        = 2 [(gogoproto.moretags) = "yaml:\"proof_unreceived\""];
-  bytes                     proof_close  = 3 [(gogoproto.moretags) = "yaml:\"proof_close\""];
-  ibc.core.client.v1.Height proof_height = 4
+  Packet                    packet           = 1 [(gogoproto.nullable) = false];
+  bytes                     proof_unreceived = 2 [(gogoproto.moretags) = "yaml:\"proof_unreceived\""];
+  bytes                     proof_close      = 3 [(gogoproto.moretags) = "yaml:\"proof_close\""];
+  ibc.core.client.v1.Height proof_height     = 4
       [(gogoproto.moretags) = "yaml:\"proof_height\"", (gogoproto.nullable) = false];
   uint64 next_sequence_recv = 5 [(gogoproto.moretags) = "yaml:\"next_sequence_recv\""];
   string signer             = 6;
@@ -197,7 +197,7 @@ message MsgAcknowledgement {
 
   Packet                    packet          = 1 [(gogoproto.nullable) = false];
   bytes                     acknowledgement = 2;
-  bytes                     proof_acked           = 3 [(gogoproto.moretags) = "yaml:\"proof_acked\""];
+  bytes                     proof_acked     = 3 [(gogoproto.moretags) = "yaml:\"proof_acked\""];
   ibc.core.client.v1.Height proof_height    = 4
       [(gogoproto.moretags) = "yaml:\"proof_height\"", (gogoproto.nullable) = false];
   string signer = 5;

proto/ibc/core/connection/v1/genesis.proto

@@ -11,4 +11,6 @@ message GenesisState {
   repeated IdentifiedConnection connections             = 1 [(gogoproto.nullable) = false];
   repeated ConnectionPaths      client_connection_paths = 2
       [(gogoproto.nullable) = false, (gogoproto.moretags) = "yaml:\"client_connection_paths\""];
+  // the sequence for the next generated connection identifier
+  uint64 next_connection_sequence = 3 [(gogoproto.moretags) = "yaml:\"next_connection_sequence\""];
 }

proto/ibc/core/connection/v1/tx.proto

@@ -29,11 +29,10 @@ message MsgConnectionOpenInit {
   option (gogoproto.equal)           = false;
   option (gogoproto.goproto_getters) = false;
 
-  string       client_id     = 1 [(gogoproto.moretags) = "yaml:\"client_id\""];
-  string       connection_id = 2 [(gogoproto.moretags) = "yaml:\"connection_id\""];
-  Counterparty counterparty  = 3 [(gogoproto.nullable) = false];
-  Version      version       = 4;
-  string       signer        = 5;
+  string       client_id    = 1 [(gogoproto.moretags) = "yaml:\"client_id\""];
+  Counterparty counterparty = 2 [(gogoproto.nullable) = false];
+  Version      version      = 3;
+  string       signer       = 4;
 }
 
 // MsgConnectionOpenInitResponse defines the Msg/ConnectionOpenInit response type.
@@ -45,24 +44,25 @@ message MsgConnectionOpenTry {
   option (gogoproto.equal)           = false;
   option (gogoproto.goproto_getters) = false;
 
-  string client_id                         = 1 [(gogoproto.moretags) = "yaml:\"client_id\""];
-  string desired_connection_id             = 2 [(gogoproto.moretags) = "yaml:\"desired_connection_id\""];
-  string counterparty_chosen_connection_id = 3 [(gogoproto.moretags) = "yaml:\"counterparty_chosen_connection_id\""];
-  google.protobuf.Any       client_state   = 4 [(gogoproto.moretags) = "yaml:\"client_state\""];
-  Counterparty              counterparty   = 5 [(gogoproto.nullable) = false];
-  repeated Version          counterparty_versions = 6 [(gogoproto.moretags) = "yaml:\"counterparty_versions\""];
-  ibc.core.client.v1.Height proof_height          = 7
+  string client_id = 1 [(gogoproto.moretags) = "yaml:\"client_id\""];
+  // in the case of crossing hello's, when both chains call OpenInit, we need the connection identifier
+  // of the previous connection in state INIT
+  string                    previous_connection_id = 2 [(gogoproto.moretags) = "yaml:\"previous_connection_id\""];
+  google.protobuf.Any       client_state           = 3 [(gogoproto.moretags) = "yaml:\"client_state\""];
+  Counterparty              counterparty           = 4 [(gogoproto.nullable) = false];
+  repeated Version          counterparty_versions  = 5 [(gogoproto.moretags) = "yaml:\"counterparty_versions\""];
+  ibc.core.client.v1.Height proof_height           = 6
       [(gogoproto.moretags) = "yaml:\"proof_height\"", (gogoproto.nullable) = false];
   // proof of the initialization the connection on Chain A: `UNITIALIZED ->
   // INIT`
-  bytes proof_init = 8 [(gogoproto.moretags) = "yaml:\"proof_init\""];
+  bytes proof_init = 7 [(gogoproto.moretags) = "yaml:\"proof_init\""];
   // proof of client state included in message
-  bytes proof_client = 9 [(gogoproto.moretags) = "yaml:\"proof_client\""];
+  bytes proof_client = 8 [(gogoproto.moretags) = "yaml:\"proof_client\""];
   // proof of client consensus state
-  bytes                     proof_consensus  = 10 [(gogoproto.moretags) = "yaml:\"proof_consensus\""];
-  ibc.core.client.v1.Height consensus_height = 11
+  bytes                     proof_consensus  = 9 [(gogoproto.moretags) = "yaml:\"proof_consensus\""];
+  ibc.core.client.v1.Height consensus_height = 10
       [(gogoproto.moretags) = "yaml:\"consensus_height\"", (gogoproto.nullable) = false];
-  string signer = 12;
+  string signer = 11;
 }
 
 // MsgConnectionOpenTryResponse defines the Msg/ConnectionOpenTry response type.

proto/ibc/lightclients/solomachine/v1/solomachine.proto

@@ -48,11 +48,11 @@ message Header {
 // Misbehaviour defines misbehaviour for a solo machine which consists
 // of a sequence and two signatures over different messages at that sequence.
 message Misbehaviour {
-  option (gogoproto.goproto_getters)  = false;
-  string           client_id          = 1 [(gogoproto.moretags) = "yaml:\"client_id\""];
-  uint64           sequence           = 2;
-  SignatureAndData signature_one      = 3 [(gogoproto.moretags) = "yaml:\"signature_one\""];
-  SignatureAndData signature_two      = 4 [(gogoproto.moretags) = "yaml:\"signature_two\""];
+  option (gogoproto.goproto_getters) = false;
+  string           client_id         = 1 [(gogoproto.moretags) = "yaml:\"client_id\""];
+  uint64           sequence          = 2;
+  SignatureAndData signature_one     = 3 [(gogoproto.moretags) = "yaml:\"signature_one\""];
+  SignatureAndData signature_two     = 4 [(gogoproto.moretags) = "yaml:\"signature_two\""];
 }
 
 // SignatureAndData contains a signature and the data signed over to create that

proto/ibc/lightclients/tendermint/v1/tendermint.proto

@@ -75,7 +75,7 @@ message ConsensusState {
 // Misbehaviour is a wrapper over two conflicting Headers
 // that implements Misbehaviour interface expected by ICS-02
 message Misbehaviour {
-  option (gogoproto.goproto_getters)  = false;
+  option (gogoproto.goproto_getters) = false;
 
   string client_id = 1 [(gogoproto.moretags) = "yaml:\"client_id\""];
   Header header_1  = 2 [(gogoproto.customname) = "Header1", (gogoproto.moretags) = "yaml:\"header_1\""];

x/auth/client/rest/rest_test.go

@@ -307,8 +307,8 @@ func (s *IntegrationTestSuite) TestLegacyRestErrMessages() {
 	val := s.network.Validators[0]
 
 	args := []string{
-		"121",         // dummy port-id
-		"21212121212", // dummy channel-id
+		"121",       // dummy port-id
+		"channel-0", // dummy channel-id
 		fmt.Sprintf("--%s=true", flags.FlagSkipConfirmation),
 		fmt.Sprintf("--%s=%s", flags.FlagBroadcastMode, flags.BroadcastBlock),
 		fmt.Sprintf("--%s=%s", flags.FlagFees, sdk.NewCoins(sdk.NewCoin(s.cfg.BondDenom, sdk.NewInt(10))).String()),

x/ibc/applications/transfer/keeper/relay_test.go

@@ -51,8 +51,8 @@ func (suite *KeeperTestSuite) TestSendTransfer() {
 		{"next seq send not found",
 			func() {
 				_, _, connA, connB := suite.coordinator.SetupClientConnections(suite.chainA, suite.chainB, exported.Tendermint)
-				channelA = connA.NextTestChannel(ibctesting.TransferPort)
-				channelB = connB.NextTestChannel(ibctesting.TransferPort)
+				channelA = suite.chainA.NextTestChannel(connA, ibctesting.TransferPort)
+				channelB = suite.chainB.NextTestChannel(connB, ibctesting.TransferPort)
 				// manually create channel so next seq send is never set
 				suite.chainA.App.IBCKeeper.ChannelKeeper.SetChannel(
 					suite.chainA.GetContext(),

x/ibc/applications/transfer/module_test.go

@@ -33,7 +33,7 @@ func (suite *TransferTestSuite) TestOnChanOpenInit() {
 		},
 		{
 			"invalid port ID", func() {
-				testChannel = connA.NextTestChannel(ibctesting.MockPort)
+				testChannel = suite.chainA.NextTestChannel(connA, ibctesting.MockPort)
 			}, false,
 		},
 		{
@@ -56,7 +56,7 @@ func (suite *TransferTestSuite) TestOnChanOpenInit() {
 			suite.SetupTest() // reset
 
 			_, _, connA, _ = suite.coordinator.SetupClientConnections(suite.chainA, suite.chainB, exported.Tendermint)
-			testChannel = connA.NextTestChannel(ibctesting.TransferPort)
+			testChannel = suite.chainA.NextTestChannel(connA, ibctesting.TransferPort)
 			counterparty := channeltypes.NewCounterparty(testChannel.PortID, testChannel.ID)
 			channel = &channeltypes.Channel{
 				State:          channeltypes.INIT,
@@ -122,7 +122,7 @@ func (suite *TransferTestSuite) TestOnChanOpenTry() {
 		},
 		{
 			"invalid port ID", func() {
-				testChannel = connA.NextTestChannel(ibctesting.MockPort)
+				testChannel = suite.chainA.NextTestChannel(connA, ibctesting.MockPort)
 			}, false,
 		},
 		{
@@ -144,7 +144,7 @@ func (suite *TransferTestSuite) TestOnChanOpenTry() {
 			suite.SetupTest() // reset
 
 			_, _, connA, _ = suite.coordinator.SetupClientConnections(suite.chainA, suite.chainB, exported.Tendermint)
-			testChannel = connA.NextTestChannel(ibctesting.TransferPort)
+			testChannel = suite.chainA.NextTestChannel(connA, ibctesting.TransferPort)
 			counterparty := channeltypes.NewCounterparty(testChannel.PortID, testChannel.ID)
 			channel = &channeltypes.Channel{
 				State:          channeltypes.TRYOPEN,
@@ -210,7 +210,7 @@ func (suite *TransferTestSuite) TestOnChanOpenAck() {
 			suite.SetupTest() // reset
 
 			_, _, connA, _ = suite.coordinator.SetupClientConnections(suite.chainA, suite.chainB, exported.Tendermint)
-			testChannel = connA.NextTestChannel(ibctesting.TransferPort)
+			testChannel = suite.chainA.NextTestChannel(connA, ibctesting.TransferPort)
 			counterpartyVersion = types.Version
 
 			module, _, err := suite.chainA.App.IBCKeeper.PortKeeper.LookupModuleByPort(suite.chainA.GetContext(), ibctesting.TransferPort)

x/ibc/applications/transfer/types/msgs_test.go

@@ -19,7 +19,7 @@ const (
 
 	validChannel        = "testchannel"
 	invalidChannel      = "(invalidchannel1)"
-	invalidShortChannel = "invalidch"
+	invalidShortChannel = "invalid"
 	invalidLongChannel  = "invalidlongchannelinvalidlongchannelinvalidlongchannelinvalidlongchannel"
 )
 

x/ibc/core/03-connection/client/cli/tx.go

@@ -22,36 +22,33 @@ import (
 const (
 	flagVersionIdentifier = "version-identifier"
 	flagVersionFeatures   = "version-features"
-	flagProvedID          = "proved-id"
 )
 
 // NewConnectionOpenInitCmd defines the command to initialize a connection on
 // chain A with a given counterparty chain B
 func NewConnectionOpenInitCmd() *cobra.Command {
 	cmd := &cobra.Command{
-		Use:   "open-init [connection-id] [client-id] [counterparty-connection-id] [counterparty-client-id] [path/to/counterparty_prefix.json]",
+		Use:   "open-init [client-id] [counterparty-client-id] [path/to/counterparty_prefix.json]",
 		Short: "Initialize connection on chain A",
 		Long: `Initialize a connection on chain A with a given counterparty chain B.
 	- 'version-identifier' flag can be a single pre-selected version identifier to be used in the handshake.
 	- 'version-features' flag can be a list of features separated by commas to accompany the version identifier.`,
 		Example: fmt.Sprintf(
-			"%s tx %s %s open-init [connection-id] [client-id] [counterparty-connection-id] [counterparty-client-id] [path/to/counterparty_prefix.json] --version-identifier=\"1.0\" --version-features=\"ORDER_UNORDERED\"",
+			"%s tx %s %s open-init [client-id] [counterparty-client-id] [path/to/counterparty_prefix.json] --version-identifier=\"1.0\" --version-features=\"ORDER_UNORDERED\"",
 			version.AppName, host.ModuleName, types.SubModuleName,
 		),
-		Args: cobra.ExactArgs(5),
+		Args: cobra.ExactArgs(3),
 		RunE: func(cmd *cobra.Command, args []string) error {
 			clientCtx := client.GetClientContextFromCmd(cmd)
 			clientCtx, err := client.ReadTxCommandFlags(clientCtx, cmd.Flags())
 			if err != nil {
 				return err
 			}
 
-			connectionID := args[0]
-			clientID := args[1]
-			counterpartyConnectionID := args[2]
-			counterpartyClientID := args[3]
+			clientID := args[0]
+			counterpartyClientID := args[1]
 
-			counterpartyPrefix, err := utils.ParsePrefix(clientCtx.LegacyAmino, args[4])
+			counterpartyPrefix, err := utils.ParsePrefix(clientCtx.LegacyAmino, args[2])
 			if err != nil {
 				return err
 			}
@@ -71,7 +68,7 @@ func NewConnectionOpenInitCmd() *cobra.Command {
 			}
 
 			msg := types.NewMsgConnectionOpenInit(
-				connectionID, clientID, counterpartyConnectionID, counterpartyClientID,
+				clientID, counterpartyClientID,
 				counterpartyPrefix, version, clientCtx.GetFromAddress(),
 			)
 
@@ -116,7 +113,6 @@ func NewConnectionOpenTryCmd() *cobra.Command {
 			}
 
 			connectionID := args[0]
-			provedID, _ := cmd.Flags().GetString(flagProvedID)
 			clientID := args[1]
 			counterpartyConnectionID := args[2]
 			counterpartyClientID := args[3]
@@ -179,7 +175,7 @@ func NewConnectionOpenTryCmd() *cobra.Command {
 			}
 
 			msg := types.NewMsgConnectionOpenTry(
-				connectionID, provedID, clientID, counterpartyConnectionID, counterpartyClientID,
+				connectionID, clientID, counterpartyConnectionID, counterpartyClientID,
 				counterpartyClient, counterpartyPrefix, counterpartyVersions,
 				proofInit, proofClient, proofConsensus, proofHeight,
 				consensusHeight, clientCtx.GetFromAddress(),
@@ -193,7 +189,6 @@ func NewConnectionOpenTryCmd() *cobra.Command {
 		},
 	}
 
-	cmd.Flags().String(flagProvedID, "", "identifier set by the counterparty chain")
 	flags.AddTxFlagsToCmd(cmd)
 
 	return cmd

x/ibc/core/03-connection/genesis.go

@@ -16,6 +16,7 @@ func InitGenesis(ctx sdk.Context, k keeper.Keeper, gs types.GenesisState) {
 	for _, connPaths := range gs.ClientConnectionPaths {
 		k.SetClientConnectionPaths(ctx, connPaths.ClientId, connPaths.Paths)
 	}
+	k.SetNextConnectionSequence(ctx, gs.NextConnectionSequence)
 }
 
 // ExportGenesis returns the ibc connection submodule's exported genesis.