Merge PR #493: Always use flexible identifier selection

spec/ics-003-connection-semantics/README.md

@@ -281,10 +281,10 @@ At the end of an opening handshake between two chains implementing the sub-proto
 
 This sub-protocol need not be permissioned, modulo anti-spam measures.
 
-In `connOpenInit`, a sentinel empty-string identifier can be used to allow the recipient chain to choose its own connection identifier. Chains may implement a function `desiredIdentifier` which chooses an identifier, e.g. by incrementing a counter:
+Chains MUST implement a function `generateIdentifier` which chooses an identifier, e.g. by incrementing a counter:
 
 ```typescript
-type desiredIdentifier = (counterpartyChosenConnectionIdentifer: Identifier) -> Identifier
+type generateIdentifier = () -> Identifier
 ```
 
 A specific version can optionally be passed as `version` to ensure that the handshake will either complete with that version or fail.
@@ -293,13 +293,11 @@ A specific version can optionally be passed as `version` to ensure that the hand
 
 ```typescript
 function connOpenInit(
-  identifier: Identifier,
-  desiredCounterpartyConnectionIdentifier: Identifier,
   counterpartyPrefix: CommitmentPrefix,
   clientIdentifier: Identifier,
   counterpartyClientIdentifier: Identifier,
   version: string) {
-    abortTransactionUnless(validateConnectionIdentifier(identifier))
+    identifier = generateIdentifier()
     abortTransactionUnless(provableStore.get(connectionPath(identifier)) == null)
     state = INIT
     if version != "" {
@@ -309,7 +307,7 @@ function connOpenInit(
     } else {
       versions = getCompatibleVersions()
     }
-    connection = ConnectionEnd{state, desiredCounterpartyConnectionIdentifier, counterpartyPrefix,
+    connection = ConnectionEnd{state, "", counterpartyPrefix,
       clientIdentifier, counterpartyClientIdentifier, versions}
     provableStore.set(connectionPath(identifier), connection)
     addConnectionToClient(clientIdentifier, identifier)
@@ -320,8 +318,7 @@ function connOpenInit(
 
 ```typescript
 function connOpenTry(
-  desiredIdentifier: Identifier,
-  counterpartyChosenConnectionIdentifer: Identifier,
+  previousIdentifier: Identifier,
   counterpartyConnectionIdentifier: Identifier,
   counterpartyPrefix: CommitmentPrefix,
   counterpartyClientIdentifier: Identifier,
@@ -331,31 +328,31 @@ function connOpenTry(
   proofConsensus: CommitmentProof,
   proofHeight: Height,
   consensusHeight: Height) {
-    abortTransactionUnless(validateConnectionIdentifier(desiredIdentifier))
+    if (previousIdentifier !== "") {
+      previous = provableStore.get(connectionPath(identifier))
+      abortTransactionUnless(
+        (previous !== null) &&
+        (previous.state === INIT &&
+         previous.counterpartyConnectionIdentifier === "" &&
+         previous.counterpartyPrefix === counterpartyPrefix &&
+         previous.clientIdentifier === clientIdentifier &&
+         previous.counterpartyClientIdentifier === counterpartyClientIdentifier))
+      identifier = previousIdentifier
+    } else {
+      // generate a new identifier if the passed identifier was the sentinel empty-string
+      identifier = generateIdentifier()
+    }
     abortTransactionUnless(consensusHeight < getCurrentHeight())
     expectedConsensusState = getConsensusState(consensusHeight)
-    abortTransationUnless(
-      counterpartyChosenConnectionIdentifer === "" ||
-      counterpartyChosenConnectionIdentifer === desiredIdentifier
-      )
-    expected = ConnectionEnd{INIT, counterpartyChosenConnectionIdentifer, getCommitmentPrefix(), counterpartyClientIdentifier,
+    expected = ConnectionEnd{INIT, "", getCommitmentPrefix(), counterpartyClientIdentifier,
                              clientIdentifier, counterpartyVersions}
-    previous = provableStore.get(connectionPath(desiredIdentifier))
-    abortTransactionUnless(
-      (previous === null) ||
-      (previous.state === INIT &&
-        previous.counterpartyConnectionIdentifier === counterpartyConnectionIdentifier &&
-        previous.counterpartyPrefix === counterpartyPrefix &&
-        previous.clientIdentifier === clientIdentifier &&
-        previous.counterpartyClientIdentifier === counterpartyClientIdentifier))
     versionsIntersection = intersection(counterpartyVersions, previous !== null ? previous.version : getCompatibleVersions())
     version = pickVersion(versionsIntersection) // throws if there is no intersection
     connection = ConnectionEnd{TRYOPEN, counterpartyConnectionIdentifier, counterpartyPrefix,
                                clientIdentifier, counterpartyClientIdentifier, version}
     abortTransactionUnless(connection.verifyConnectionState(proofHeight, proofInit, counterpartyConnectionIdentifier, expected))
     abortTransactionUnless(connection.verifyClientConsensusState(
       proofHeight, proofConsensus, counterpartyClientIdentifier, consensusHeight, expectedConsensusState))
-    identifier = desiredIdentifier
     provableStore.set(connectionPath(identifier), connection)
     addConnectionToClient(clientIdentifier, identifier)
 }
@@ -374,10 +371,6 @@ function connOpenAck(
   consensusHeight: Height) {
     abortTransactionUnless(consensusHeight < getCurrentHeight())
     connection = provableStore.get(connectionPath(identifier))
-    abortTransactionUnless(
-      connection.counterpartyConnectionIdentifier === "" ||
-      counterpartyIdentifier === connection.counterpartyConnectionIdentifier
-      )
     abortTransactionUnless(
         (connection.state === INIT && connection.version.indexOf(version) !== -1)
         || (connection.state === TRYOPEN && connection.version === version))

spec/ics-004-channel-and-packet-semantics/README.md

@@ -273,17 +273,20 @@ When the opening handshake is complete, the module which initiates the handshake
 it specifies will own the other end of the created channel on the counterparty chain. Once a channel is created, ownership cannot be changed (although higher-level abstractions
 could be implemented to provide this).
 
-A sentinel empty-string identifier can be used to allow the recipient chain to choose its own channel identifier.
+Chains MUST implement a function `generateIdentifier` which chooses an identifier, e.g. by incrementing a counter:
+
+```typescript
+type generateIdentifier = () -> Identifier
+```
 
 ```typescript
 function chanOpenInit(
   order: ChannelOrder,
   connectionHops: [Identifier],
   portIdentifier: Identifier,
-  channelIdentifier: Identifier,
   counterpartyPortIdentifier: Identifier,
-  counterpartyChannelIdentifier: Identifier,
   version: string): CapabilityKey {
+    channelIdentifier = generateIdentifier()
     abortTransactionUnless(validateChannelIdentifier(portIdentifier, channelIdentifier))
 
     abortTransactionUnless(connectionHops.length === 1) // for v1 of the IBC protocol
@@ -295,7 +298,7 @@ function chanOpenInit(
     abortTransactionUnless(connection !== null)
     abortTransactionUnless(authenticateCapability(portPath(portIdentifier), portCapability))
     channel = ChannelEnd{INIT, order, counterpartyPortIdentifier,
-                         counterpartyChannelIdentifier, connectionHops, version}
+                         "", connectionHops, version}
     provableStore.set(channelPath(portIdentifier, channelIdentifier), channel)
     channelCapability = newCapability(channelCapabilityPath(portIdentifier, channelIdentifier))
     provableStore.set(nextSequenceSendPath(portIdentifier, channelIdentifier), 1)
@@ -312,37 +315,38 @@ function chanOpenTry(
   order: ChannelOrder,
   connectionHops: [Identifier],
   portIdentifier: Identifier,
-  channelIdentifier: Identifier,
+  previousIdentifier: Identifier,
   counterpartyChosenChannelIdentifer: Identifier,
   counterpartyPortIdentifier: Identifier,
   counterpartyChannelIdentifier: Identifier,
   version: string,
   counterpartyVersion: string,
   proofInit: CommitmentProof,
   proofHeight: Height): CapabilityKey {
+    if (previousIdentifier !== "") {
+      previous = provableStore.get(channelPath(portIdentifier, channelIdentifier))
+      abortTransactionUnless(
+        (previous !== null) &&
+        (previous.state === INIT &&
+         previous.order === order &&
+         previous.counterpartyPortIdentifier === counterpartyPortIdentifier &&
+         previous.counterpartyChannelIdentifier === "" &&
+         previous.connectionHops === connectionHops &&
+         previous.version === version)
+        )
+      channelIdentifier = previousIdentifier
+    } else {
+      // generate a new identifier if the provided identifier was the sentinel empty-string
+      channelIdentifier = generateIdentifier()
+    }
     abortTransactionUnless(validateChannelIdentifier(portIdentifier, channelIdentifier))
     abortTransactionUnless(connectionHops.length === 1) // for v1 of the IBC protocol
-    // empty-string is a sentinel value for "allow any identifier"
-    abortTransationUnless(
-      counterpartyChosenChannelIdentifer === "" ||
-      counterpartyChosenChannelIdentifer === channelIdentifier
-      )
-    previous = provableStore.get(channelPath(portIdentifier, channelIdentifier))
-    abortTransactionUnless(
-      (previous === null) ||
-      (previous.state === INIT &&
-       previous.order === order &&
-       previous.counterpartyPortIdentifier === counterpartyPortIdentifier &&
-       previous.counterpartyChannelIdentifier === counterpartyChannelIdentifier &&
-       previous.connectionHops === connectionHops &&
-       previous.version === version)
-      )
     abortTransactionUnless(authenticateCapability(portPath(portIdentifier), portCapability))
     connection = provableStore.get(connectionPath(connectionHops[0]))
     abortTransactionUnless(connection !== null)
     abortTransactionUnless(connection.state === OPEN)
     expected = ChannelEnd{INIT, order, portIdentifier,
-                          counterpartyChosenChannelIdentifer, [connection.counterpartyConnectionIdentifier], counterpartyVersion}
+                          "", [connection.counterpartyConnectionIdentifier], counterpartyVersion}
     abortTransactionUnless(connection.verifyChannelState(
       proofHeight,
       proofInit,
@@ -378,11 +382,6 @@ function chanOpenAck(
     channel = provableStore.get(channelPath(portIdentifier, channelIdentifier))
     abortTransactionUnless(channel.state === INIT || channel.state === TRYOPEN)
     abortTransactionUnless(authenticateCapability(channelCapabilityPath(portIdentifier, channelIdentifier), capability))
-    // empty-string is a sentinel value for "allow any identifier"
-    abortTransactionUnless(
-      channel.counterpartyChannelIdentifier === "" ||
-      counterpartyChannelIdentifier === channel.counterpartyChannelIdentifier
-      )
     connection = provableStore.get(connectionPath(channel.connectionHops[0]))
     abortTransactionUnless(connection !== null)
     abortTransactionUnless(connection.state === OPEN)