From c76a99b66f3453b42e29c3156747edb4bc5e416e Mon Sep 17 00:00:00 2001 From: Christopher Goes Date: Thu, 19 Nov 2020 15:35:45 +0100 Subject: [PATCH 01/10] Remove checks for identifier equality --- spec/ics-003-connection-semantics/README.md | 13 ++----------- .../ics-004-channel-and-packet-semantics/README.md | 14 ++------------ 2 files changed, 4 insertions(+), 23 deletions(-) diff --git a/spec/ics-003-connection-semantics/README.md b/spec/ics-003-connection-semantics/README.md index 7f955b292..937f41037 100644 --- a/spec/ics-003-connection-semantics/README.md +++ b/spec/ics-003-connection-semantics/README.md @@ -294,7 +294,6 @@ 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, @@ -309,7 +308,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) @@ -334,11 +333,7 @@ function connOpenTry( abortTransactionUnless(validateConnectionIdentifier(desiredIdentifier)) 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( @@ -374,10 +369,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)) diff --git a/spec/ics-004-channel-and-packet-semantics/README.md b/spec/ics-004-channel-and-packet-semantics/README.md index dafa2ea6c..dad332569 100644 --- a/spec/ics-004-channel-and-packet-semantics/README.md +++ b/spec/ics-004-channel-and-packet-semantics/README.md @@ -282,7 +282,6 @@ function chanOpenInit( portIdentifier: Identifier, channelIdentifier: Identifier, counterpartyPortIdentifier: Identifier, - counterpartyChannelIdentifier: Identifier, version: string): CapabilityKey { abortTransactionUnless(validateChannelIdentifier(portIdentifier, channelIdentifier)) @@ -295,7 +294,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) @@ -323,10 +322,6 @@ function chanOpenTry( 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) || @@ -342,7 +337,7 @@ function chanOpenTry( 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 +373,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) From 78b2d8c79166ea4315d3a7f21bfebdeeeff34779 Mon Sep 17 00:00:00 2001 From: Christopher Goes Date: Thu, 19 Nov 2020 16:13:57 +0100 Subject: [PATCH 02/10] Only reuse existing identifier if compatible --- spec/ics-003-connection-semantics/README.md | 23 +++++++++-------- .../README.md | 25 +++++++++++-------- 2 files changed, 28 insertions(+), 20 deletions(-) diff --git a/spec/ics-003-connection-semantics/README.md b/spec/ics-003-connection-semantics/README.md index 937f41037..3c7aaa218 100644 --- a/spec/ics-003-connection-semantics/README.md +++ b/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. @@ -336,13 +336,17 @@ function connOpenTry( 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)) + // use the provided identifier only if the handshake can progress with it + if ((previous === null) || + !(previous.state === INIT && + previous.counterpartyConnectionIdentifier === counterpartyConnectionIdentifier && + previous.counterpartyPrefix === counterpartyPrefix && + previous.clientIdentifier === clientIdentifier && + previous.counterpartyClientIdentifier === counterpartyClientIdentifier)) { + identifier = generateIdentifier() + } else { + identifier = desiredIdentifier + } versionsIntersection = intersection(counterpartyVersions, previous !== null ? previous.version : getCompatibleVersions()) version = pickVersion(versionsIntersection) // throws if there is no intersection connection = ConnectionEnd{TRYOPEN, counterpartyConnectionIdentifier, counterpartyPrefix, @@ -350,7 +354,6 @@ function connOpenTry( 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) } diff --git a/spec/ics-004-channel-and-packet-semantics/README.md b/spec/ics-004-channel-and-packet-semantics/README.md index dad332569..e62181d07 100644 --- a/spec/ics-004-channel-and-packet-semantics/README.md +++ b/spec/ics-004-channel-and-packet-semantics/README.md @@ -273,7 +273,11 @@ 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( @@ -323,15 +327,16 @@ function chanOpenTry( abortTransactionUnless(connectionHops.length === 1) // for v1 of the IBC protocol // empty-string is a sentinel value for "allow any identifier" 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) - ) + // use the provided identifier only if the handshake can progress with it + if ((previous === null) || + !(previous.state === INIT && + previous.order === order && + previous.counterpartyPortIdentifier === counterpartyPortIdentifier && + previous.counterpartyChannelIdentifier === counterpartyChannelIdentifier && + previous.connectionHops === connectionHops && + previous.version === version)) { + channelIdentifier = generateIdentifier() + } abortTransactionUnless(authenticateCapability(portPath(portIdentifier), portCapability)) connection = provableStore.get(connectionPath(connectionHops[0])) abortTransactionUnless(connection !== null) From 5a42e18f2b54773ee5e06c36b7ad82def267fd99 Mon Sep 17 00:00:00 2001 From: Christopher Goes Date: Thu, 19 Nov 2020 16:47:13 +0100 Subject: [PATCH 03/10] Apply suggestions from code review MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: colin axnér <25233464+colin-axner@users.noreply.github.com> --- spec/ics-003-connection-semantics/README.md | 2 +- spec/ics-004-channel-and-packet-semantics/README.md | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/spec/ics-003-connection-semantics/README.md b/spec/ics-003-connection-semantics/README.md index 3c7aaa218..c60c2cd23 100644 --- a/spec/ics-003-connection-semantics/README.md +++ b/spec/ics-003-connection-semantics/README.md @@ -339,7 +339,7 @@ function connOpenTry( // use the provided identifier only if the handshake can progress with it if ((previous === null) || !(previous.state === INIT && - previous.counterpartyConnectionIdentifier === counterpartyConnectionIdentifier && + previous.counterpartyConnectionIdentifier === "" && previous.counterpartyPrefix === counterpartyPrefix && previous.clientIdentifier === clientIdentifier && previous.counterpartyClientIdentifier === counterpartyClientIdentifier)) { diff --git a/spec/ics-004-channel-and-packet-semantics/README.md b/spec/ics-004-channel-and-packet-semantics/README.md index e62181d07..e164d5a57 100644 --- a/spec/ics-004-channel-and-packet-semantics/README.md +++ b/spec/ics-004-channel-and-packet-semantics/README.md @@ -332,7 +332,7 @@ function chanOpenTry( !(previous.state === INIT && previous.order === order && previous.counterpartyPortIdentifier === counterpartyPortIdentifier && - previous.counterpartyChannelIdentifier === counterpartyChannelIdentifier && + previous.counterpartyChannelIdentifier === "" && previous.connectionHops === connectionHops && previous.version === version)) { channelIdentifier = generateIdentifier() From 6e6b6cea208b52e6b0b4a485dff1ffd916159438 Mon Sep 17 00:00:00 2001 From: Christopher Goes Date: Tue, 24 Nov 2020 02:46:44 +0100 Subject: [PATCH 04/10] Update spec/ics-003-connection-semantics/README.md Co-authored-by: Anca Zamfir --- spec/ics-003-connection-semantics/README.md | 25 +++++++++++++-------- 1 file changed, 16 insertions(+), 9 deletions(-) diff --git a/spec/ics-003-connection-semantics/README.md b/spec/ics-003-connection-semantics/README.md index c60c2cd23..975862a6f 100644 --- a/spec/ics-003-connection-semantics/README.md +++ b/spec/ics-003-connection-semantics/README.md @@ -336,16 +336,23 @@ function connOpenTry( expected = ConnectionEnd{INIT, "", getCommitmentPrefix(), counterpartyClientIdentifier, clientIdentifier, counterpartyVersions} previous = provableStore.get(connectionPath(desiredIdentifier)) - // use the provided identifier only if the handshake can progress with it - if ((previous === null) || - !(previous.state === INIT && - previous.counterpartyConnectionIdentifier === "" && - previous.counterpartyPrefix === counterpartyPrefix && - previous.clientIdentifier === clientIdentifier && - previous.counterpartyClientIdentifier === counterpartyClientIdentifier)) { - identifier = generateIdentifier() + abortTransactionUnless( + (previous === null) || + (previous.state === INIT && + previous.counterpartyConnectionIdentifier === "" && // unless there's some state corruption this should always be true since connOpenInit always stores "" + previous.counterpartyPrefix === counterpartyPrefix && + previous.clientIdentifier === clientIdentifier && + previous.counterpartyClientIdentifier === counterpartyClientIdentifier)) + + // we are here either with null previous or one stored under desiredIdentifier. + // previous can be null in two cases: + // 1. desiredIdentifier === "" or + // 2. desiredIdentifier != "" and there is no stored connection with that id. + // We only want to generate the identifier in case 1 + if (previous === null) && (desiredIdentifier === "") { + identifier = generateIdentifier() } else { - identifier = desiredIdentifier + identifier = desiredIdentifier } versionsIntersection = intersection(counterpartyVersions, previous !== null ? previous.version : getCompatibleVersions()) version = pickVersion(versionsIntersection) // throws if there is no intersection From 52e3e488a62861515a2da55c3f48695d21fb5854 Mon Sep 17 00:00:00 2001 From: Christopher Goes Date: Mon, 23 Nov 2020 17:56:40 -0800 Subject: [PATCH 05/10] Updates from comments, for new logic --- spec/ics-003-connection-semantics/README.md | 35 +++++++------------ .../README.md | 27 +++++++------- 2 files changed, 28 insertions(+), 34 deletions(-) diff --git a/spec/ics-003-connection-semantics/README.md b/spec/ics-003-connection-semantics/README.md index 975862a6f..9eb775f71 100644 --- a/spec/ics-003-connection-semantics/README.md +++ b/spec/ics-003-connection-semantics/README.md @@ -293,12 +293,11 @@ A specific version can optionally be passed as `version` to ensure that the hand ```typescript function connOpenInit( - identifier: Identifier, counterpartyPrefix: CommitmentPrefix, clientIdentifier: Identifier, counterpartyClientIdentifier: Identifier, version: string) { - abortTransactionUnless(validateConnectionIdentifier(identifier)) + identifier = generateIdentifier() abortTransactionUnless(provableStore.get(connectionPath(identifier)) == null) state = INIT if version != "" { @@ -320,7 +319,6 @@ function connOpenInit( ```typescript function connOpenTry( desiredIdentifier: Identifier, - counterpartyChosenConnectionIdentifer: Identifier, counterpartyConnectionIdentifier: Identifier, counterpartyPrefix: CommitmentPrefix, counterpartyClientIdentifier: Identifier, @@ -330,30 +328,23 @@ function connOpenTry( proofConsensus: CommitmentProof, proofHeight: Height, consensusHeight: Height) { - abortTransactionUnless(validateConnectionIdentifier(desiredIdentifier)) + // generate a new identifier if the passed identifier was the sentinel empty-string + if (desiredIdentifier === "") { + identifier = generateIdentifier() + } abortTransactionUnless(consensusHeight < getCurrentHeight()) expectedConsensusState = getConsensusState(consensusHeight) expected = ConnectionEnd{INIT, "", getCommitmentPrefix(), counterpartyClientIdentifier, clientIdentifier, counterpartyVersions} - previous = provableStore.get(connectionPath(desiredIdentifier)) + previous = provableStore.get(connectionPath(identifier)) abortTransactionUnless( - (previous === null) || - (previous.state === INIT && - previous.counterpartyConnectionIdentifier === "" && // unless there's some state corruption this should always be true since connOpenInit always stores "" - previous.counterpartyPrefix === counterpartyPrefix && - previous.clientIdentifier === clientIdentifier && - previous.counterpartyClientIdentifier === counterpartyClientIdentifier)) - - // we are here either with null previous or one stored under desiredIdentifier. - // previous can be null in two cases: - // 1. desiredIdentifier === "" or - // 2. desiredIdentifier != "" and there is no stored connection with that id. - // We only want to generate the identifier in case 1 - if (previous === null) && (desiredIdentifier === "") { - identifier = generateIdentifier() - } else { - identifier = desiredIdentifier - } + (previous === null) || + (previous.state === INIT && + // unless there's some state corruption this should always be true since connOpenInit always stores "" + previous.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, diff --git a/spec/ics-004-channel-and-packet-semantics/README.md b/spec/ics-004-channel-and-packet-semantics/README.md index e164d5a57..7a297aa5a 100644 --- a/spec/ics-004-channel-and-packet-semantics/README.md +++ b/spec/ics-004-channel-and-packet-semantics/README.md @@ -284,9 +284,9 @@ function chanOpenInit( order: ChannelOrder, connectionHops: [Identifier], portIdentifier: Identifier, - channelIdentifier: Identifier, counterpartyPortIdentifier: Identifier, version: string): CapabilityKey { + channelIdentifier = generateIdentifier() abortTransactionUnless(validateChannelIdentifier(portIdentifier, channelIdentifier)) abortTransactionUnless(connectionHops.length === 1) // for v1 of the IBC protocol @@ -323,21 +323,24 @@ function chanOpenTry( counterpartyVersion: string, proofInit: CommitmentProof, proofHeight: Height): CapabilityKey { + // generate a new identifier if the provided identifier was the sentinel empty-string + if (channelIdentifier === "") { + channelIdentifier = generateIdentifier() + } abortTransactionUnless(validateChannelIdentifier(portIdentifier, channelIdentifier)) + abortTransactionUnless(authenticateCapability(portPath(portIdentifier), portCapability)) abortTransactionUnless(connectionHops.length === 1) // for v1 of the IBC protocol // empty-string is a sentinel value for "allow any identifier" previous = provableStore.get(channelPath(portIdentifier, channelIdentifier)) - // use the provided identifier only if the handshake can progress with it - if ((previous === null) || - !(previous.state === INIT && - previous.order === order && - previous.counterpartyPortIdentifier === counterpartyPortIdentifier && - previous.counterpartyChannelIdentifier === "" && - previous.connectionHops === connectionHops && - previous.version === version)) { - channelIdentifier = generateIdentifier() - } - abortTransactionUnless(authenticateCapability(portPath(portIdentifier), portCapability)) + abortTransactionUnless( + (previous === null) || + (previous.state === INIT && + previous.order === order && + previous.counterpartyPortIdentifier === counterpartyPortIdentifier && + previous.counterpartyChannelIdentifier === "" && + previous.connectionHops === connectionHops && + previous.version === version) + ) connection = provableStore.get(connectionPath(connectionHops[0])) abortTransactionUnless(connection !== null) abortTransactionUnless(connection.state === OPEN) From a46ca9c6c295a9354073988232e328754c4b1058 Mon Sep 17 00:00:00 2001 From: Christopher Goes Date: Mon, 23 Nov 2020 18:00:28 -0800 Subject: [PATCH 06/10] Remove unnecessary diffs --- spec/ics-003-connection-semantics/README.md | 5 ++--- spec/ics-004-channel-and-packet-semantics/README.md | 2 +- 2 files changed, 3 insertions(+), 4 deletions(-) diff --git a/spec/ics-003-connection-semantics/README.md b/spec/ics-003-connection-semantics/README.md index 9eb775f71..06f982888 100644 --- a/spec/ics-003-connection-semantics/README.md +++ b/spec/ics-003-connection-semantics/README.md @@ -338,12 +338,11 @@ function connOpenTry( clientIdentifier, counterpartyVersions} previous = provableStore.get(connectionPath(identifier)) abortTransactionUnless( - (previous === null) || + (previous === null) || (previous.state === INIT && - // unless there's some state corruption this should always be true since connOpenInit always stores "" previous.counterpartyConnectionIdentifier === "" && previous.counterpartyPrefix === counterpartyPrefix && - previous.clientIdentifier === clientIdentifier && + previous.clientIdentifier === clientIdentifier && previous.counterpartyClientIdentifier === counterpartyClientIdentifier)) versionsIntersection = intersection(counterpartyVersions, previous !== null ? previous.version : getCompatibleVersions()) version = pickVersion(versionsIntersection) // throws if there is no intersection diff --git a/spec/ics-004-channel-and-packet-semantics/README.md b/spec/ics-004-channel-and-packet-semantics/README.md index 7a297aa5a..df9d5dab3 100644 --- a/spec/ics-004-channel-and-packet-semantics/README.md +++ b/spec/ics-004-channel-and-packet-semantics/README.md @@ -328,7 +328,6 @@ function chanOpenTry( channelIdentifier = generateIdentifier() } abortTransactionUnless(validateChannelIdentifier(portIdentifier, channelIdentifier)) - abortTransactionUnless(authenticateCapability(portPath(portIdentifier), portCapability)) abortTransactionUnless(connectionHops.length === 1) // for v1 of the IBC protocol // empty-string is a sentinel value for "allow any identifier" previous = provableStore.get(channelPath(portIdentifier, channelIdentifier)) @@ -341,6 +340,7 @@ function chanOpenTry( previous.connectionHops === connectionHops && previous.version === version) ) + abortTransactionUnless(authenticateCapability(portPath(portIdentifier), portCapability)) connection = provableStore.get(connectionPath(connectionHops[0])) abortTransactionUnless(connection !== null) abortTransactionUnless(connection.state === OPEN) From d60d01e8eff1733155825e8365bd7da40dbc4f21 Mon Sep 17 00:00:00 2001 From: Christopher Goes Date: Mon, 23 Nov 2020 18:01:41 -0800 Subject: [PATCH 07/10] Remove unnecessary diffs part II --- spec/ics-003-connection-semantics/README.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/spec/ics-003-connection-semantics/README.md b/spec/ics-003-connection-semantics/README.md index 06f982888..a86dcc855 100644 --- a/spec/ics-003-connection-semantics/README.md +++ b/spec/ics-003-connection-semantics/README.md @@ -338,8 +338,8 @@ function connOpenTry( clientIdentifier, counterpartyVersions} previous = provableStore.get(connectionPath(identifier)) abortTransactionUnless( - (previous === null) || - (previous.state === INIT && + (previous === null) || + (previous.state === INIT && previous.counterpartyConnectionIdentifier === "" && previous.counterpartyPrefix === counterpartyPrefix && previous.clientIdentifier === clientIdentifier && From 43bbbb32fe7b7c2ae0fd641e647a16e4c954f8ed Mon Sep 17 00:00:00 2001 From: Christopher Goes Date: Tue, 24 Nov 2020 06:49:06 -0800 Subject: [PATCH 08/10] Switch around the conditionals --- spec/ics-003-connection-semantics/README.md | 24 +++++++++-------- .../README.md | 27 ++++++++++--------- 2 files changed, 27 insertions(+), 24 deletions(-) diff --git a/spec/ics-003-connection-semantics/README.md b/spec/ics-003-connection-semantics/README.md index a86dcc855..264b5442e 100644 --- a/spec/ics-003-connection-semantics/README.md +++ b/spec/ics-003-connection-semantics/README.md @@ -318,7 +318,7 @@ function connOpenInit( ```typescript function connOpenTry( - desiredIdentifier: Identifier, + previousIdentifier: Identifier, counterpartyConnectionIdentifier: Identifier, counterpartyPrefix: CommitmentPrefix, counterpartyClientIdentifier: Identifier, @@ -328,22 +328,24 @@ function connOpenTry( proofConsensus: CommitmentProof, proofHeight: Height, consensusHeight: Height) { - // generate a new identifier if the passed identifier was the sentinel empty-string - if (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) expected = ConnectionEnd{INIT, "", getCommitmentPrefix(), counterpartyClientIdentifier, clientIdentifier, counterpartyVersions} - previous = provableStore.get(connectionPath(identifier)) - abortTransactionUnless( - (previous === null) || - (previous.state === INIT && - previous.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, diff --git a/spec/ics-004-channel-and-packet-semantics/README.md b/spec/ics-004-channel-and-packet-semantics/README.md index df9d5dab3..981815e36 100644 --- a/spec/ics-004-channel-and-packet-semantics/README.md +++ b/spec/ics-004-channel-and-packet-semantics/README.md @@ -323,23 +323,24 @@ function chanOpenTry( counterpartyVersion: string, proofInit: CommitmentProof, proofHeight: Height): CapabilityKey { - // generate a new identifier if the provided identifier was the sentinel empty-string - if (channelIdentifier === "") { + 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" - 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) - ) abortTransactionUnless(authenticateCapability(portPath(portIdentifier), portCapability)) connection = provableStore.get(connectionPath(connectionHops[0])) abortTransactionUnless(connection !== null) From b16a7721bba47d9c5534139a6e27237f15a4eeba Mon Sep 17 00:00:00 2001 From: Christopher Goes Date: Tue, 24 Nov 2020 06:53:26 -0800 Subject: [PATCH 09/10] Change argument name --- spec/ics-004-channel-and-packet-semantics/README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/spec/ics-004-channel-and-packet-semantics/README.md b/spec/ics-004-channel-and-packet-semantics/README.md index 981815e36..63a383f44 100644 --- a/spec/ics-004-channel-and-packet-semantics/README.md +++ b/spec/ics-004-channel-and-packet-semantics/README.md @@ -315,7 +315,7 @@ function chanOpenTry( order: ChannelOrder, connectionHops: [Identifier], portIdentifier: Identifier, - channelIdentifier: Identifier, + previousIdentifier: Identifier, counterpartyChosenChannelIdentifer: Identifier, counterpartyPortIdentifier: Identifier, counterpartyChannelIdentifier: Identifier, From a99e6099ab56832db91f1034e395a790883576d5 Mon Sep 17 00:00:00 2001 From: Christopher Goes Date: Mon, 30 Nov 2020 16:24:38 +0100 Subject: [PATCH 10/10] Apply suggestions from code review MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: colin axnér <25233464+colin-axner@users.noreply.github.com> --- spec/ics-003-connection-semantics/README.md | 2 +- spec/ics-004-channel-and-packet-semantics/README.md | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/spec/ics-003-connection-semantics/README.md b/spec/ics-003-connection-semantics/README.md index 264b5442e..23ac4e72f 100644 --- a/spec/ics-003-connection-semantics/README.md +++ b/spec/ics-003-connection-semantics/README.md @@ -331,7 +331,7 @@ function connOpenTry( if (previousIdentifier !== "") { previous = provableStore.get(connectionPath(identifier)) abortTransactionUnless( - (previous === null) || + (previous !== null) && (previous.state === INIT && previous.counterpartyConnectionIdentifier === "" && previous.counterpartyPrefix === counterpartyPrefix && diff --git a/spec/ics-004-channel-and-packet-semantics/README.md b/spec/ics-004-channel-and-packet-semantics/README.md index 63a383f44..84122074b 100644 --- a/spec/ics-004-channel-and-packet-semantics/README.md +++ b/spec/ics-004-channel-and-packet-semantics/README.md @@ -326,7 +326,7 @@ function chanOpenTry( if (previousIdentifier !== "") { previous = provableStore.get(channelPath(portIdentifier, channelIdentifier)) abortTransactionUnless( - (previous === null) || + (previous !== null) && (previous.state === INIT && previous.order === order && previous.counterpartyPortIdentifier === counterpartyPortIdentifier &&