From d9f275c6190661a4fbe4991f69d63d75c6fcd6b5 Mon Sep 17 00:00:00 2001 From: Aditya Sripal Date: Thu, 16 Dec 2021 19:33:42 +0530 Subject: [PATCH 1/4] improve handshake and callback documentation --- .../README.md | 4 +- spec/core/ics-026-routing-module/README.md | 73 +++++++++++++++---- 2 files changed, 62 insertions(+), 15 deletions(-) diff --git a/spec/core/ics-004-channel-and-packet-semantics/README.md b/spec/core/ics-004-channel-and-packet-semantics/README.md index b5e773cc1..bc05f89ba 100644 --- a/spec/core/ics-004-channel-and-packet-semantics/README.md +++ b/spec/core/ics-004-channel-and-packet-semantics/README.md @@ -319,7 +319,7 @@ function chanOpenTry( counterpartyChosenChannelIdentifer: Identifier, counterpartyPortIdentifier: Identifier, counterpartyChannelIdentifier: Identifier, - version: string, + version: string, // deprecated counterpartyVersion: string, proofInit: CommitmentProof, proofHeight: Height): CapabilityKey { @@ -332,7 +332,7 @@ function chanOpenTry( previous.counterpartyPortIdentifier === counterpartyPortIdentifier && previous.counterpartyChannelIdentifier === "" && previous.connectionHops === connectionHops && - previous.version === version) + previous.version === counterpartyVersion ) channelIdentifier = previousIdentifier } else { diff --git a/spec/core/ics-026-routing-module/README.md b/spec/core/ics-026-routing-module/README.md index f63a1668f..0bd2db37b 100644 --- a/spec/core/ics-026-routing-module/README.md +++ b/spec/core/ics-026-routing-module/README.md @@ -41,6 +41,20 @@ The functions `newCapability` & `authenticateCapability` are defined as in [ICS Modules must expose the following function signatures to the routing module, which are called upon the receipt of various datagrams: +#### **ChanOpenInit** + +ChanOpenInit will verify that the relayer-chosen parameters +are valid and perform any custom INIT logic. +It may return an error if the chosen parameters are invalid +in which case the handshake is aborted. +If the provided version string is non-empty, ChanOpenInit should return +the version string if valid or an error if the provided version is invalid. +If the version string is empty, ChanOpenInit is expected to +return a default version string representing the version(s) +it supports. +If there is no default version string for the application, +it should return an error if provided version is empty string. + ```typescript function onChanOpenInit( order: ChannelOrder, @@ -49,10 +63,24 @@ function onChanOpenInit( channelIdentifier: Identifier, counterpartyPortIdentifier: Identifier, counterpartyChannelIdentifier: Identifier, - version: string) { + version: string) => (version: string, err: Error) { // defined by the module } +``` + +#### **ChanOpenTry** + +ChanOpenTry will verify the relayer-chosen parameters along with the +counterparty-chosen version string and perform custom TRY logic. +If the relayer-chosen parameters +are invalid, the callback must return an error to abort the handshake. +If the counterparty-chosen version is not compatible with this modules +supported versions, the callback must return an error to abort the handshake. +If the versions are compatible, the try callback must select the final version +string and return it to core IBC. +ChanOpenTry may also perform custom initialization logic +```typescript function onChanOpenTry( order: ChannelOrder, connectionHops: [Identifier], @@ -60,24 +88,38 @@ function onChanOpenTry( channelIdentifier: Identifier, counterpartyPortIdentifier: Identifier, counterpartyChannelIdentifier: Identifier, - version: string, - counterpartyVersion: string) { + counterpartyVersion: string) => (version: string, err: Error) { // defined by the module } +``` + +#### **OnChanOpenAck** +OnChanOpenAck will error if the counterparty selected version string +is invalid to abort the handshake. It may also perform custom ACK logic. + +```typescript function onChanOpenAck( portIdentifier: Identifier, channelIdentifier: Identifier, version: string) { // defined by the module } +``` + +#### **OnChanOpenConfirm** + +OnChanOpenConfirm will perform custom CONFIRM logic and may error to abort the handshake. +```typescript function onChanOpenConfirm( portIdentifier: Identifier, channelIdentifier: Identifier) { // defined by the module } +``` +```typescript function onChanCloseInit( portIdentifier: Identifier, channelIdentifier: Identifier) { @@ -382,7 +424,7 @@ interface ChanOpenInit { ```typescript function handleChanOpenInit(datagram: ChanOpenInit) { module = lookupModule(datagram.portIdentifier) - module.onChanOpenInit( + version, err = module.onChanOpenInit( datagram.order, datagram.connectionHops, datagram.portIdentifier, @@ -391,6 +433,7 @@ function handleChanOpenInit(datagram: ChanOpenInit) { datagram.counterpartyChannelIdentifier, datagram.version ) + abortTransactionUnless(err === nil) handler.chanOpenInit( datagram.order, datagram.connectionHops, @@ -398,7 +441,7 @@ function handleChanOpenInit(datagram: ChanOpenInit) { datagram.channelIdentifier, datagram.counterpartyPortIdentifier, datagram.counterpartyChannelIdentifier, - datagram.version + version // pass in version returned from callback ) } ``` @@ -411,7 +454,7 @@ interface ChanOpenTry { channelIdentifier: Identifier counterpartyPortIdentifier: Identifier counterpartyChannelIdentifier: Identifier - version: string + version: string // deprecated counterpartyVersion: string proofInit: CommitmentProof proofHeight: Height @@ -421,16 +464,16 @@ interface ChanOpenTry { ```typescript function handleChanOpenTry(datagram: ChanOpenTry) { module = lookupModule(datagram.portIdentifier) - module.onChanOpenTry( + version, err = module.onChanOpenTry( datagram.order, datagram.connectionHops, datagram.portIdentifier, datagram.channelIdentifier, datagram.counterpartyPortIdentifier, datagram.counterpartyChannelIdentifier, - datagram.version, datagram.counterpartyVersion ) + abortTransactionUnless(err === nil) handler.chanOpenTry( datagram.order, datagram.connectionHops, @@ -438,7 +481,7 @@ function handleChanOpenTry(datagram: ChanOpenTry) { datagram.channelIdentifier, datagram.counterpartyPortIdentifier, datagram.counterpartyChannelIdentifier, - datagram.version, + version, // pass in version returned by callback datagram.counterpartyVersion, datagram.proofInit, datagram.proofHeight @@ -458,11 +501,12 @@ interface ChanOpenAck { ```typescript function handleChanOpenAck(datagram: ChanOpenAck) { - module.onChanOpenAck( + err = module.onChanOpenAck( datagram.portIdentifier, datagram.channelIdentifier, datagram.version ) + abortTransactionUnless(err === nil) handler.chanOpenAck( datagram.portIdentifier, datagram.channelIdentifier, @@ -485,10 +529,11 @@ interface ChanOpenConfirm { ```typescript function handleChanOpenConfirm(datagram: ChanOpenConfirm) { module = lookupModule(datagram.portIdentifier) - module.onChanOpenConfirm( + err = module.onChanOpenConfirm( datagram.portIdentifier, datagram.channelIdentifier ) + abortTransactionUnless(err === nil) handler.chanOpenConfirm( datagram.portIdentifier, datagram.channelIdentifier, @@ -508,10 +553,11 @@ interface ChanCloseInit { ```typescript function handleChanCloseInit(datagram: ChanCloseInit) { module = lookupModule(datagram.portIdentifier) - module.onChanCloseInit( + err = module.onChanCloseInit( datagram.portIdentifier, datagram.channelIdentifier ) + abortTransactionUnless(err === nil) handler.chanCloseInit( datagram.portIdentifier, datagram.channelIdentifier @@ -531,10 +577,11 @@ interface ChanCloseConfirm { ```typescript function handleChanCloseConfirm(datagram: ChanCloseConfirm) { module = lookupModule(datagram.portIdentifier) - module.onChanCloseConfirm( + err = module.onChanCloseConfirm( datagram.portIdentifier, datagram.channelIdentifier ) + abortTransactionUnless(err === nil) handler.chanCloseConfirm( datagram.portIdentifier, datagram.channelIdentifier, From 735aa7342a0148ed33bd05fba39224fac8f5f086 Mon Sep 17 00:00:00 2001 From: Aditya Sripal Date: Tue, 11 Jan 2022 17:08:30 +0530 Subject: [PATCH 2/4] remove crossing hello case --- .../README.md | 19 ++----------------- 1 file changed, 2 insertions(+), 17 deletions(-) diff --git a/spec/core/ics-004-channel-and-packet-semantics/README.md b/spec/core/ics-004-channel-and-packet-semantics/README.md index bc05f89ba..9e9bd150e 100644 --- a/spec/core/ics-004-channel-and-packet-semantics/README.md +++ b/spec/core/ics-004-channel-and-packet-semantics/README.md @@ -315,7 +315,6 @@ function chanOpenTry( order: ChannelOrder, connectionHops: [Identifier], portIdentifier: Identifier, - previousIdentifier: Identifier, counterpartyChosenChannelIdentifer: Identifier, counterpartyPortIdentifier: Identifier, counterpartyChannelIdentifier: Identifier, @@ -323,22 +322,8 @@ function chanOpenTry( 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 === counterpartyVersion - ) - channelIdentifier = previousIdentifier - } else { - // generate a new identifier if the provided identifier was the sentinel empty-string - channelIdentifier = generateIdentifier() - } + channelIdentifier = generateIdentifier() + abortTransactionUnless(validateChannelIdentifier(portIdentifier, channelIdentifier)) abortTransactionUnless(connectionHops.length === 1) // for v1 of the IBC protocol abortTransactionUnless(authenticateCapability(portPath(portIdentifier), portCapability)) From 8566d52ed6ff9dd18079595842462c60f13b11a3 Mon Sep 17 00:00:00 2001 From: Aditya Date: Tue, 11 Jan 2022 17:14:58 +0530 Subject: [PATCH 3/4] 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> Co-authored-by: Carlos Rodriguez --- spec/core/ics-026-routing-module/README.md | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/spec/core/ics-026-routing-module/README.md b/spec/core/ics-026-routing-module/README.md index 0bd2db37b..6de20da2c 100644 --- a/spec/core/ics-026-routing-module/README.md +++ b/spec/core/ics-026-routing-module/README.md @@ -43,13 +43,13 @@ Modules must expose the following function signatures to the routing module, whi #### **ChanOpenInit** -ChanOpenInit will verify that the relayer-chosen parameters -are valid and perform any custom INIT logic. +`onChanOpenInit` will verify that the relayer-chosen parameters +are valid and perform any custom `INIT` logic. It may return an error if the chosen parameters are invalid in which case the handshake is aborted. -If the provided version string is non-empty, ChanOpenInit should return +If the provided version string is non-empty, `onChanOpenInit` should return the version string if valid or an error if the provided version is invalid. -If the version string is empty, ChanOpenInit is expected to +If the version string is empty, `onChanOpenInit` is expected to return a default version string representing the version(s) it supports. If there is no default version string for the application, @@ -70,15 +70,15 @@ function onChanOpenInit( #### **ChanOpenTry** -ChanOpenTry will verify the relayer-chosen parameters along with the -counterparty-chosen version string and perform custom TRY logic. +`onChanOpenTry` will verify the relayer-chosen parameters along with the +counterparty-chosen version string and perform custom `TRY` logic. If the relayer-chosen parameters are invalid, the callback must return an error to abort the handshake. If the counterparty-chosen version is not compatible with this modules supported versions, the callback must return an error to abort the handshake. If the versions are compatible, the try callback must select the final version string and return it to core IBC. -ChanOpenTry may also perform custom initialization logic +`onChanOpenTry` may also perform custom initialization logic ```typescript function onChanOpenTry( @@ -95,7 +95,7 @@ function onChanOpenTry( #### **OnChanOpenAck** -OnChanOpenAck will error if the counterparty selected version string +`onChanOpenAck` will error if the counterparty selected version string is invalid to abort the handshake. It may also perform custom ACK logic. ```typescript @@ -109,7 +109,7 @@ function onChanOpenAck( #### **OnChanOpenConfirm** -OnChanOpenConfirm will perform custom CONFIRM logic and may error to abort the handshake. +`onChanOpenConfirm` will perform custom CONFIRM logic and may error to abort the handshake. ```typescript function onChanOpenConfirm( From f9f00b584bb2a9ef73af19eb1a83f30dcda07eda Mon Sep 17 00:00:00 2001 From: Aditya Sripal Date: Fri, 14 Jan 2022 12:10:21 +0100 Subject: [PATCH 4/4] remove previous check --- .../ics-004-channel-and-packet-semantics/README.md | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/spec/core/ics-004-channel-and-packet-semantics/README.md b/spec/core/ics-004-channel-and-packet-semantics/README.md index 9e9bd150e..696c6e846 100644 --- a/spec/core/ics-004-channel-and-packet-semantics/README.md +++ b/spec/core/ics-004-channel-and-packet-semantics/README.md @@ -343,12 +343,12 @@ function chanOpenTry( counterpartyChannelIdentifier, connectionHops, version} provableStore.set(channelPath(portIdentifier, channelIdentifier), channel) channelCapability = newCapability(channelCapabilityPath(portIdentifier, channelIdentifier)) - // only reset sequences if the previous channel didn't exist, else we might overwrite optimistically-sent packets - if (previous === null) { - provableStore.set(nextSequenceSendPath(portIdentifier, channelIdentifier), 1) - provableStore.set(nextSequenceRecvPath(portIdentifier, channelIdentifier), 1) - provableStore.set(nextSequenceAckPath(portIdentifier, channelIdentifier), 1) - } + + // initialize channel sequences + provableStore.set(nextSequenceSendPath(portIdentifier, channelIdentifier), 1) + provableStore.set(nextSequenceRecvPath(portIdentifier, channelIdentifier), 1) + provableStore.set(nextSequenceAckPath(portIdentifier, channelIdentifier), 1) + return channelCapability } ```