From e0fa2e1621a55480899ef1fa9726860c0c6749dd Mon Sep 17 00:00:00 2001 From: Aditya Sripal Date: Tue, 30 Nov 2021 19:24:02 +0100 Subject: [PATCH 01/27] connection upgrade progress --- .../ics-003-connection-semantics/UPGRADES.md | 163 ++++++++++++++++++ 1 file changed, 163 insertions(+) create mode 100644 spec/core/ics-003-connection-semantics/UPGRADES.md diff --git a/spec/core/ics-003-connection-semantics/UPGRADES.md b/spec/core/ics-003-connection-semantics/UPGRADES.md new file mode 100644 index 000000000..48b27ae1e --- /dev/null +++ b/spec/core/ics-003-connection-semantics/UPGRADES.md @@ -0,0 +1,163 @@ +# Upgrading Connections + +### Synopsis + +This standard document specifies the interfaces and state machine logic that IBC implementations must implement in order to enable existing connections to upgrade after initial connection handshake. + +### Motivation + +As new features get added to IBC, chains may wish the take advantage of new connection features without abandoning the accumulated state and network effect(s) of an already existing connection. The upgrade protocol proposed would allow chains to renegotiate an existing connection to take advantage of new features without having to create a new connection. + +### Desired Properties + +- Both chains must agree to the renegotiated connection parameters +- Connection state and logic on both chains should either be using the old parameters or the new parameters, but should not be in an in-between state. i.e. It should not be possible for a chain to write state to an old proof path, while the counterparty expects a new proof path. +- If the upgrade handshake is unsuccessful, the connection must fall-back to the original connection parameters +- If the upgrade handshake is successful, then both connection ends have adopted the new connection parameters and process IBC data appropriately. +- The connection upgrade protocol should have the ability to change all connection-related parameters; however the connection upgrade protocol MUST NOT be able to change the underlying `ClientState`. +- The connection upgrade protocol may not modify the connection identifiers. + +## Technical Specification + +### Data Structures + +```typescript +enum ConnectionState { + INIT, + TRYOPEN, + OPEN, + UPGRADE_INIT, + UPGRADE_TRY, + UPGRADE_ERR, +} +``` + +- The chain that is proposing the upgrade should set the connection state from `OPEN` to `UPGRADE_INIT` +- The counterparty chain that accepts the upgrade should set the connection state from `OPEN` to `UPGRADE_TRY` + +```typescript +interface ConnectionEnd { + state: ConnectionState + counterpartyConnectionIdentifier: Identifier + counterpartyPrefix: CommitmentPrefix + clientIdentifier: Identifier + counterpartyClientIdentifier: Identifier + version: string | []string + delayPeriodTime: uint64 + delayPeriodBlocks: uint64 +} +``` + +```typescript +interface UpgradeStatus { + state: ConnectionState + timeoutHeight: Height + timeoutTimestamp: uint64 +} +``` + +The desired property that the connection upgrade protocol may not modify the underlying clients or connection identifiers, means that only some fields are upgradable by the upgrade protocol. + +- `state`: The state is specified by the handshake steps of the upgrade protocol. +- `counterpartyConnectionIdentifier`: The counterparty connection identifier CAN NOT be modified by the upgrade protocol. +- `counterpartyPrefix`: The prefix MAY be modified in the upgrade protocol. The counterparty must accept the new proposed prefix value, or it must return an error during the upgrade handshake. +- `clientIdentifier`: The client identifier CAN NOT be modified by the upgrade protocol +- `counterpartyClientIdentifier`: The counterparty client identifier CAN NOT be modified by the upgrade protocol +- `version`: The version MAY be modified by the upgrade protocol. The same version negotiation that happens in the initial connection handshake can be employed for the upgrade handshake. +- `delayPeriodTime`: The delay period MAY be modified by the upgrade protocol. The counterparty MUST accept the new proposed value or return an error during the upgrade handshake. +- `delayPeriodBlocks`: The delay period MAY be modified by the upgrade protocol. The counterparty MUST accept the new proposed value or return an error during the upgrade handshake. + +```typescript +interface UpgradeConnectionState { + connection: ConnectionEnd + timeoutHeight: Height + timeoutTimestamp: uint64 +} +``` + +- `proposedConnectionState`: Proposed `ConnectionState` to replace the current `ConnectionState`, it MUST ONLY modify the fields that are permissable to +- `timeoutHeight`: Timeout height indicates the height at which the counterparty must no longer proceed with the upgrade handshake. The chains will then preserve their original connection and the upgrade handshake is aborted. +- `timeoutTimestamp`: Timeout timestamp indicates the time on the counterparty at which the counterparty must no longer proceed with the upgrade handshake. The chains will then preserve their original connection and the upgrade handshake is aborted. + +NOTE: One of the timeout height or timeout timestamp must be non-zero. + +### Store Paths + +##### Restore Connection Path + +The chain must store the previous connection end so that it may restore it if the upgrade handshake fails. This may be stored in the private store. + +```typescript +function restorePath(id: Identifier): Path { + return "connections/{id}/restore" +} +``` + +##### UpgradeStatus Path + +The upgrade status path is a public path that can signal the status of the upgrade to the counterparty. It does not store anything in the successful case, but it will store a sentinel abort value in the case that a chain does not accept the proposed upgrade. + +```typescript +function upgradePath(id: Identifier): Path { + return "connections/{id}/upgradeStatus" + +} +``` + +### Upgrade Handshake + +The upgrade handshake defines four datagrams: *ConnUpgradeInit*, *ConnUpgradeTry*, *ConnUpgradeAck*, and *ConnUpgradeConfirm* + +A successful protocol execution flows as follows (note that all calls are made through modules per ICS 25): + +| Initiator | Datagram | Chain acted upon | Prior state (A, B) | Posterior state (A, B) | +| --------- | -------------------- | ---------------- | --------------------------- | --------------------------- | +| Actor | `ConnUpgradeInit` | A | (OPEN, OPEN) | (UPGRADE_INIT, OPEN) | +| Relayer | `ConnUpgradeTry` | B | (UPGRADE_INIT, OPEN) | (UPGRADE_INIT, UPGRADE_TRY) | +| Relayer | `ConnUpgradeAck` | A | (UPGRADE_INIT, UPGRADE_TRY) | (OPEN, UPGRADE_TRY) | +| Relayer | `ConnUpgradeConfirm` | B | (OPEN, UPGRADE_TRY) | (OPEN, OPEN) | + +At the end of an opening handshake between two chains implementing the sub-protocol, the following properties hold: + +- Each chain is running their new upgraded connection end and is processing upgraded logic and state according to the upgraded parameters. +- Each chain has knowledge of and has agreed to the counterparty's upgraded connection parameters. + +If a chain does not agree to the proposed counterparty `UpgradeConnectionState`, it may abort the upgrade handshake by writing a sentinel abort value into the upgrade-failed connection path. + +`connection/{identifier}/proposedUpgradeFailed` => `[]byte(0x1)`. + +The counterparty receiving proof of this must cancel the upgrade and resume the + +If an upgrade message arrives after the specified timeout, then the message MUST NOT execute successfully. + +```typescript +function connUpgradeInit( + identifier: Identifier, + proposedUpgrade: UpgradeConnectionState +) { + // current connection must be OPEN + currentConnection = provableStore.get(connectionPath(identifier)) + abortTransactionUnless(connection.state == OPEN) + + // abort transaction if an unmodifiable field is modified + // upgraded connection state must be in `UPGRADE_INIT` + abortTransactionUnless( + proposedUpgrade.connection.state == UPGRADE_INIT && + proposedUpgrade.connection.counterpartyConnectionIdentifier == currentConnection.counterpartyConnectionIdentifier && + proposedUpgrade.connection.clientIdentifier == currentConnection.clientIdentifier && + proposedUpgrade.connection.counterpartyClientIdentifier == currentConnection.counterpartyClientIdentifier && + ) + + // either timeout height or timestamp must be non-zero + abortTransactionUnless(proposedUpgrade.TimeoutHeight != 0 || proposedUpgrade.TimeoutTimestamp != 0) + + provableStore.set(connectionPath(identifier), proposedUpgrade) + privateStore.set(restorePath(identifier), currentConnection) +} +``` + +```typescript +function connUpgradeTry( + identifier: Identifier, + proposedUpgrade +) From cb0b5c57a035197c724ef2624a0a1860af6be46f Mon Sep 17 00:00:00 2001 From: Aditya Sripal Date: Wed, 1 Dec 2021 16:47:28 +0100 Subject: [PATCH 02/27] initial writeup --- .../ics-003-connection-semantics/UPGRADES.md | 217 ++++++++++++++++-- 1 file changed, 204 insertions(+), 13 deletions(-) diff --git a/spec/core/ics-003-connection-semantics/UPGRADES.md b/spec/core/ics-003-connection-semantics/UPGRADES.md index 48b27ae1e..5964ec6b4 100644 --- a/spec/core/ics-003-connection-semantics/UPGRADES.md +++ b/spec/core/ics-003-connection-semantics/UPGRADES.md @@ -48,14 +48,6 @@ interface ConnectionEnd { } ``` -```typescript -interface UpgradeStatus { - state: ConnectionState - timeoutHeight: Height - timeoutTimestamp: uint64 -} -``` - The desired property that the connection upgrade protocol may not modify the underlying clients or connection identifiers, means that only some fields are upgradable by the upgrade protocol. - `state`: The state is specified by the handshake steps of the upgrade protocol. @@ -67,6 +59,18 @@ The desired property that the connection upgrade protocol may not modify the und - `delayPeriodTime`: The delay period MAY be modified by the upgrade protocol. The counterparty MUST accept the new proposed value or return an error during the upgrade handshake. - `delayPeriodBlocks`: The delay period MAY be modified by the upgrade protocol. The counterparty MUST accept the new proposed value or return an error during the upgrade handshake. +```typescript +interface UpgradeStatus { + state: ConnectionState + timeoutHeight: Height + timeoutTimestamp: uint64 +} +``` + +- UpgradeStatus contains the state of the upgrade. If upgrade is successful it will contain the `ConnectionState` of the upgrading connection on the executing chain. If the executing chain wants to abort the upgrade, it will restore its previous connection under its connection path with state `OPEN` and write an `UpgradeStatus` with state `UPGRADE_ERR`. +- `timeoutHeight`: Timeout height indicates the height at which the counterparty must no longer proceed with the upgrade handshake. The chains will then preserve their original connection and the upgrade handshake is aborted. +- `timeoutTimestamp`: Timeout timestamp indicates the time on the counterparty at which the counterparty must no longer proceed with the upgrade handshake. The chains will then preserve their original connection and the upgrade handshake is aborted. + ```typescript interface UpgradeConnectionState { connection: ConnectionEnd @@ -98,7 +102,7 @@ function restorePath(id: Identifier): Path { The upgrade status path is a public path that can signal the status of the upgrade to the counterparty. It does not store anything in the successful case, but it will store a sentinel abort value in the case that a chain does not accept the proposed upgrade. ```typescript -function upgradePath(id: Identifier): Path { +function statusPath(id: Identifier): Path { return "connections/{id}/upgradeStatus" } @@ -113,7 +117,7 @@ A successful protocol execution flows as follows (note that all calls are made t | Initiator | Datagram | Chain acted upon | Prior state (A, B) | Posterior state (A, B) | | --------- | -------------------- | ---------------- | --------------------------- | --------------------------- | | Actor | `ConnUpgradeInit` | A | (OPEN, OPEN) | (UPGRADE_INIT, OPEN) | -| Relayer | `ConnUpgradeTry` | B | (UPGRADE_INIT, OPEN) | (UPGRADE_INIT, UPGRADE_TRY) | +| Actor | `ConnUpgradeTry` | B | (UPGRADE_INIT, OPEN) | (UPGRADE_INIT, UPGRADE_TRY) | | Relayer | `ConnUpgradeAck` | A | (UPGRADE_INIT, UPGRADE_TRY) | (OPEN, UPGRADE_TRY) | | Relayer | `ConnUpgradeConfirm` | B | (OPEN, UPGRADE_TRY) | (OPEN, OPEN) | @@ -151,7 +155,14 @@ function connUpgradeInit( // either timeout height or timestamp must be non-zero abortTransactionUnless(proposedUpgrade.TimeoutHeight != 0 || proposedUpgrade.TimeoutTimestamp != 0) - provableStore.set(connectionPath(identifier), proposedUpgrade) + upgradeStatus = UpgradeStatus{ + state: UPGRADE_INIT, + timeoutHeight: proposedUpgrade.timeoutHeight, + timeoutTimestamp: proposedUpgrade.timeoutTimestamp, + } + + provableStore.set(statusPath(identifier), upgradeStatus) + provableStore.set(connectionPath(identifier), proposedUpgrade.connection) privateStore.set(restorePath(identifier), currentConnection) } ``` @@ -159,5 +170,185 @@ function connUpgradeInit( ```typescript function connUpgradeTry( identifier: Identifier, - proposedUpgrade -) + proposedUpgrade: UpgradeConnectionState, + counterpartyConnection: ConnectionEnd, + counterpartyUpgradeStatus: UpgradeStatus, + proofConnection: CommitmentProof, + proofUpgradeStatus: CommitmentProof, + proofHeight: Height +) { + // current connection must be OPEN or UPGRADE_INIT (crossing hellos) + currentConnection = provableStore.get(connectionPath(identifier)) + abortTransactionUnless(connection.state == (OPEN || UPGRADE_INIT)) + + // abort transaction if an unmodifiable field is modified + // upgraded connection state must be in `UPGRADE_TRY` + abortTransactionUnless( + proposedUpgrade.connection.state == UPGRADE_TRY && + proposedUpgrade.connection.counterpartyConnectionIdentifier == currentConnection.counterpartyConnectionIdentifier && + proposedUpgrade.connection.clientIdentifier == currentConnection.clientIdentifier && + proposedUpgrade.connection.counterpartyClientIdentifier == currentConnection.counterpartyClientIdentifier && + ) + + // either timeout height or timestamp must be non-zero + abortTransactionUnless(proposedUpgrade.TimeoutHeight != 0 || proposedUpgrade.TimeoutTimestamp != 0) + + + // verify proofs of counterparty state + abortTransactionUnless(currentConnection.client.verifyConnectionState(proofHeight, proofConnection, counterpartyConnection)) + abortTransactionUnless(currentConnection.client.verifyUpgradeStatus(proofHeight, proofUpgradeStatus, counterpartyUpgradeStatus)) + + // verify that counterparty connection unmodifiable fields have not changed and counterparty state + // is UPGRADE_INIT + restoreConnectionUnless( + counterpartyConnection.state == UPGRADE_INIT && + counterpartyConnection.counterpartyConnectionIdentifier == identifier && + counterpartyConnection.clientIdentifier == currentConnection.counterpartyClientIdentifier && + counterpartyConnection.counterpartyClientIdentifier == currentConnection.clientIdentifier + ) + restoreConnectionUnless(counterpartyUpgradeStatus.state == UPGRADE_INIT) + + // counterparty-specified timeout must not have exceeded + restoreConnectionUnless( + counterparty.UpgradeStatus.TimeoutHeight < currentHeight() && + counterparty.UpgradeStatus.TimeoutTimestamp < currentTimestamp() + ) + + // verify chosen versions are compatible + versionsIntersection = intersection(counterpartyConnection.version, proposedUpgrade.Connection.version) + version = pickVersion(versionsIntersection) // throws if there is no intersection + + // both connection ends must be mutually compatible. + restoreConnectionUnless(IsCompatible(counterpartyConnection, proposedUpgrade.Connection)) + + upgradeStatus = UpgradeStatus{ + state: UPGRADE_TRY, + timeoutHeight: proposedUpgrade.timeoutHeight, + timeoutTimestamp: proposedUpgrade.timeoutTimestamp, + } + + provableStore.set(statusPath(identifier), upgradeStatus) + provableStore.set(connectionPath(identifier), proposedUpgrade.connection) + privateStore.set(restorePath(identifier), currentConnection) +} +``` + +```typescript +function onChanUpgradeAck( + identifier: Identifier, + counterpartyConnection: ConnectionEnd, + counterpartyStatus: UpgradeStatus, + proofConnection: CommitmentProof, + proofUpgradeStatus: CommitmentProof, + proofHeight: Height +) { + // current connection is in UPGRADE_INIT + currentConnection = provableStore.get(connectionPath(identifier)) + abortTransactionUnless(connection.state == UPGRADE_INIT) + + + // verify proofs of counterparty state + abortTransactionUnless(currentConnection.client.verifyConnectionState(proofHeight, proofConnection, counterpartyConnection)) + abortTransactionUnless(currentConnection.client.verifyUpgradeStatus(proofHeight, proofUpgradeStatus, counterpartyUpgradeStatus)) + + // counterparty must be in TRY state + restoreConnectionUnless(counterpartyStatus.state == UPGRADE_TRY) + restoreConnectionUnless(counterpartyConnection.State == UPGRADE_TRY) + + // verify connections are mutually compatible + // this will also check counterparty chosen version is valid + restoreConnectionUnless(IsCompatible(counterpartyConnection, connection)) + + // counterparty-specified timeout must not have exceeded + restoreConnectionUnless( + counterparty.UpgradeStatus.TimeoutHeight < currentHeight() && + counterparty.UpgradeStatus.TimeoutTimestamp < currentTimestamp() + ) + + // upgrade is complete + // set connection to OPEN and remove unnecessary state + currentConnection.state = OPEN + provableStore.set(connectionPath(identifier), currentConnection) + provableStore.delete(statusPath(identifier)) + privateStore.delete(restorePath(identifier)) +} +``` + +```typescript +function onChanUpgradeConfirm( + identifier: Identifier, + counterpartyConnection: ConnectionEnd, + proofConnection: CommitmentProof, + proofHeight: Height, +) { + // current connection is in UPGRADE_TRY + currentConnection = provableStore.get(connectionPath(identifier)) + abortTransactionUnless(connection.state == UPGRADE_TRY) + + // counterparty must be in OPEN state + abortTransactionUnless(counterpartyConnection.State == OPEN) + + // verify proofs of counterparty state + abortTransactionUnless(currentConnection.client.verifyConnectionState(proofHeight, proofConnection, counterpartyConnection)) + + // upgrade is complete + // set connection to OPEN and remove unnecessary state + currentConnection.state = OPEN + provableStore.set(connectionPath(identifier), currentConnection) + provableStore.delete(statusPath(identifier)) + privateStore.delete(restorePath(identifier)) +} +``` + +Note: In the `TRY` and `ACK` steps, the chain has the ability to abort the upgrade process if the upgrade parameters are not suitable or the timeout has exceeded. In this case, the chain is expected to write `UPGRADE_ERR` as the state in the `UPGRADE_STATUS`, and restore the original connection with state `OPEN`. The counterparty can then verify that the upgrade status is `UPGRADE_ERR` and restore its original connection to `OPEN` as well, thus cancelling the upgrade. + +```typescript +function restoreConnectionUnless(condition: bool) { + if !condition { + // cancel upgrade + // set UpgradeStatus to `UPGRADE_ERR` + // and restore original conneciton + errStatus = UpgradeStatus{UPGRADE_ERR} + provableStore.set(statusPath(identifier), errStatus) + originalConnection = privateStore.get(restorePath(identifier)) + provableStore.set(connectionPath(identifier), originalConnection) + privateStore.delete(restorePath(identifier)) + // caller should return as well + } else { + // caller should continue execution + } +} +``` + +### Cancel Upgrade Process + +As discussed, during the upgrade handshake a chain may cancel the upgrade by writing `UPGRADE_ERR` into the `UpgradeStatus` and restoring the original connection to `OPEN`. The counterparty must then restore its connection to `OPEN` as well. A relayer can facilitate this by calling `CancelConnectionUpgrade` + +```typescript +function cancelConnectionUpgrade( + identifier: Identifer, + counterpartyUpgradeStatus: UpgradeStatus, + proofUpgradeStatus: CommitmentProof, + proofHeight: Height, +) { + // current connection is in UPGRADE_INIT or UPGRADE_TRY + currentConnection = provableStore.get(connectionPath(identifier)) + abortTransactionUnless(connection.state == (UPGRADE_INIT || UPGRADE_TRY)) + + abortTransactionUnless(counterpartyUpgradeStatus.state == UPGRADE_ERR) + + abortTransactionUnless(currentConnection.client.verifyUpgradeStatus(proofHeight, proofUpgradeStatus, counterpartyUpgradeStatus)) + + // cancel upgrade + // and restore original conneciton + // delete unnecessary state + originalConnection = privateStore.get(restorePath(identifier)) + provableStore.set(connectionPath(identifier), originalConnection) + provableStore.delete(statusPath(identifier)) + privateStore.delete(restorePath(identifier)) +} +``` + +It is possible that there is an extraordinary delay in the upgrade handshake occurs because of liveness issues. In this case, we do not want to indefinitely stay in the upgrade process and we will revert to the original connection by calling `CancelConnectionUpgradeTimeout`. + +// TODO \ No newline at end of file From 067956e16b0fcaa8901a6abd76665099e0984f19 Mon Sep 17 00:00:00 2001 From: Aditya Sripal Date: Wed, 1 Dec 2021 16:50:43 +0100 Subject: [PATCH 03/27] add notes on init and try access control --- spec/core/ics-003-connection-semantics/UPGRADES.md | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/spec/core/ics-003-connection-semantics/UPGRADES.md b/spec/core/ics-003-connection-semantics/UPGRADES.md index 5964ec6b4..7795b3ccf 100644 --- a/spec/core/ics-003-connection-semantics/UPGRADES.md +++ b/spec/core/ics-003-connection-semantics/UPGRADES.md @@ -167,6 +167,9 @@ function connUpgradeInit( } ``` +NOTE: It is up to individual implementations how they will provide access-control to the `ConnUpgradeInit` function. E.g. chain governance, permissioned actor, DAO, etc. +Access control on counterparty should inform choice of timeout values, i.e. timeout value should be large if counterparty's `UpgradeTry` is gated by chain governance. + ```typescript function connUpgradeTry( identifier: Identifier, @@ -233,6 +236,9 @@ function connUpgradeTry( } ``` +NOTE: It is up to individual implementations how they will provide access-control to the `ConnUpgradeTry` function. E.g. chain governance, permissioned actor, DAO, etc. + + ```typescript function onChanUpgradeAck( identifier: Identifier, From dc00b01152e27f157761037e82004d23792c3018 Mon Sep 17 00:00:00 2001 From: Aditya Sripal Date: Fri, 3 Dec 2021 13:16:16 +0100 Subject: [PATCH 04/27] address colin suggestions --- .../ics-003-connection-semantics/UPGRADES.md | 19 ++++++++++++------- 1 file changed, 12 insertions(+), 7 deletions(-) diff --git a/spec/core/ics-003-connection-semantics/UPGRADES.md b/spec/core/ics-003-connection-semantics/UPGRADES.md index 7795b3ccf..8035501a2 100644 --- a/spec/core/ics-003-connection-semantics/UPGRADES.md +++ b/spec/core/ics-003-connection-semantics/UPGRADES.md @@ -51,14 +51,18 @@ interface ConnectionEnd { The desired property that the connection upgrade protocol may not modify the underlying clients or connection identifiers, means that only some fields are upgradable by the upgrade protocol. - `state`: The state is specified by the handshake steps of the upgrade protocol. -- `counterpartyConnectionIdentifier`: The counterparty connection identifier CAN NOT be modified by the upgrade protocol. + +CAN BE MODIFIED: - `counterpartyPrefix`: The prefix MAY be modified in the upgrade protocol. The counterparty must accept the new proposed prefix value, or it must return an error during the upgrade handshake. -- `clientIdentifier`: The client identifier CAN NOT be modified by the upgrade protocol -- `counterpartyClientIdentifier`: The counterparty client identifier CAN NOT be modified by the upgrade protocol - `version`: The version MAY be modified by the upgrade protocol. The same version negotiation that happens in the initial connection handshake can be employed for the upgrade handshake. - `delayPeriodTime`: The delay period MAY be modified by the upgrade protocol. The counterparty MUST accept the new proposed value or return an error during the upgrade handshake. - `delayPeriodBlocks`: The delay period MAY be modified by the upgrade protocol. The counterparty MUST accept the new proposed value or return an error during the upgrade handshake. +CAN NOT BE MODIFIED: +- `counterpartyConnectionIdentifier`: The counterparty connection identifier CAN NOT be modified by the upgrade protocol. +- `clientIdentifier`: The client identifier CAN NOT be modified by the upgrade protocol +- `counterpartyClientIdentifier`: The counterparty client identifier CAN NOT be modified by the upgrade protocol + ```typescript interface UpgradeStatus { state: ConnectionState @@ -126,13 +130,14 @@ At the end of an opening handshake between two chains implementing the sub-proto - Each chain is running their new upgraded connection end and is processing upgraded logic and state according to the upgraded parameters. - Each chain has knowledge of and has agreed to the counterparty's upgraded connection parameters. -If a chain does not agree to the proposed counterparty `UpgradeConnectionState`, it may abort the upgrade handshake by writing a sentinel abort value into the upgrade-failed connection path. +If a chain does not agree to the proposed counterparty `UpgradeConnectionState`, it may abort the upgrade handshake by writing `UPGRADE_ERR` as the state in the `UpgradeStatus` and storing +it under the status path and restore the original connection. -`connection/{identifier}/proposedUpgradeFailed` => `[]byte(0x1)`. +`statusPath(id) => UpgradeStatus{UPGRADE_ERR}` -The counterparty receiving proof of this must cancel the upgrade and resume the +A relayer may then submit a `CancelConnectionUpgradeMsg` to the counterparty. Upon receiving this message a chain must verify that the counterparty wrote `UPGRADE_ERR` into its `UpgradeStatus` and if successful, it will restore its original connection as well thus cancelling the upgrade. -If an upgrade message arrives after the specified timeout, then the message MUST NOT execute successfully. +If an upgrade message arrives after the specified timeout, then the message MUST NOT execute successfully. Again a relayer may submit a proof of this in a `CancelConnectionUpgradeTimeoutMsg` so that counterparty cancels the upgrade and restores it original connection as well. ```typescript function connUpgradeInit( From 82f72860ed9b12c7459b14dce730117d30e9c5d2 Mon Sep 17 00:00:00 2001 From: Aditya Date: Mon, 6 Dec 2021 10:57:24 +0100 Subject: [PATCH 05/27] Apply suggestions from code review Co-authored-by: Christopher Goes --- spec/core/ics-003-connection-semantics/UPGRADES.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/spec/core/ics-003-connection-semantics/UPGRADES.md b/spec/core/ics-003-connection-semantics/UPGRADES.md index 8035501a2..973d26d25 100644 --- a/spec/core/ics-003-connection-semantics/UPGRADES.md +++ b/spec/core/ics-003-connection-semantics/UPGRADES.md @@ -58,7 +58,7 @@ CAN BE MODIFIED: - `delayPeriodTime`: The delay period MAY be modified by the upgrade protocol. The counterparty MUST accept the new proposed value or return an error during the upgrade handshake. - `delayPeriodBlocks`: The delay period MAY be modified by the upgrade protocol. The counterparty MUST accept the new proposed value or return an error during the upgrade handshake. -CAN NOT BE MODIFIED: +CANNOT BE MODIFIED: - `counterpartyConnectionIdentifier`: The counterparty connection identifier CAN NOT be modified by the upgrade protocol. - `clientIdentifier`: The client identifier CAN NOT be modified by the upgrade protocol - `counterpartyClientIdentifier`: The counterparty client identifier CAN NOT be modified by the upgrade protocol @@ -83,7 +83,7 @@ interface UpgradeConnectionState { } ``` -- `proposedConnectionState`: Proposed `ConnectionState` to replace the current `ConnectionState`, it MUST ONLY modify the fields that are permissable to +- `proposedConnectionState`: Proposed `ConnectionState` to replace the current `ConnectionState`, it MUST ONLY modify the fields that are permissible to - `timeoutHeight`: Timeout height indicates the height at which the counterparty must no longer proceed with the upgrade handshake. The chains will then preserve their original connection and the upgrade handshake is aborted. - `timeoutTimestamp`: Timeout timestamp indicates the time on the counterparty at which the counterparty must no longer proceed with the upgrade handshake. The chains will then preserve their original connection and the upgrade handshake is aborted. From 26169bdd1c57dc70788acc8cfc14c1f5d0acf3a4 Mon Sep 17 00:00:00 2001 From: Aditya Sripal Date: Mon, 6 Dec 2021 11:06:10 +0100 Subject: [PATCH 06/27] address chris initial review --- spec/core/ics-003-connection-semantics/UPGRADES.md | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/spec/core/ics-003-connection-semantics/UPGRADES.md b/spec/core/ics-003-connection-semantics/UPGRADES.md index 8035501a2..0efbcdb65 100644 --- a/spec/core/ics-003-connection-semantics/UPGRADES.md +++ b/spec/core/ics-003-connection-semantics/UPGRADES.md @@ -6,7 +6,7 @@ This standard document specifies the interfaces and state machine logic that IBC ### Motivation -As new features get added to IBC, chains may wish the take advantage of new connection features without abandoning the accumulated state and network effect(s) of an already existing connection. The upgrade protocol proposed would allow chains to renegotiate an existing connection to take advantage of new features without having to create a new connection. +As new features get added to IBC, chains may wish the take advantage of new connection features without abandoning the accumulated state and network effect(s) of an already existing connection. The upgrade protocol proposed would allow chains to renegotiate an existing connection to take advantage of new features without having to create a new connection, thus preserving all existing channels that built on top of the connection. ### Desired Properties @@ -63,6 +63,8 @@ CAN NOT BE MODIFIED: - `clientIdentifier`: The client identifier CAN NOT be modified by the upgrade protocol - `counterpartyClientIdentifier`: The counterparty client identifier CAN NOT be modified by the upgrade protocol +NOTE: If the upgrade adds any fields to the `ConnectionEnd` these are by default modifiable, and can be arbitrarily chosen by an Actor (e.g. chain governance) which has permission to initiate the upgrade. + ```typescript interface UpgradeStatus { state: ConnectionState @@ -362,4 +364,4 @@ function cancelConnectionUpgrade( It is possible that there is an extraordinary delay in the upgrade handshake occurs because of liveness issues. In this case, we do not want to indefinitely stay in the upgrade process and we will revert to the original connection by calling `CancelConnectionUpgradeTimeout`. -// TODO \ No newline at end of file +// TODO Timeout Specification \ No newline at end of file From d71f150f01af99190f629b5664651e6435b603d8 Mon Sep 17 00:00:00 2001 From: Aditya Sripal Date: Thu, 10 Feb 2022 12:43:35 +0100 Subject: [PATCH 07/27] add timeout logic --- .../ics-003-connection-semantics/UPGRADES.md | 46 +++++++++++++++---- 1 file changed, 37 insertions(+), 9 deletions(-) diff --git a/spec/core/ics-003-connection-semantics/UPGRADES.md b/spec/core/ics-003-connection-semantics/UPGRADES.md index 1925836d3..ee33888ee 100644 --- a/spec/core/ics-003-connection-semantics/UPGRADES.md +++ b/spec/core/ics-003-connection-semantics/UPGRADES.md @@ -152,6 +152,7 @@ function connUpgradeInit( // abort transaction if an unmodifiable field is modified // upgraded connection state must be in `UPGRADE_INIT` + // NOTE: Any added fields are by default modifiable. abortTransactionUnless( proposedUpgrade.connection.state == UPGRADE_INIT && proposedUpgrade.connection.counterpartyConnectionIdentifier == currentConnection.counterpartyConnectionIdentifier && @@ -193,6 +194,7 @@ function connUpgradeTry( // abort transaction if an unmodifiable field is modified // upgraded connection state must be in `UPGRADE_TRY` + // NOTE: Any added fields are by default modifiable. abortTransactionUnless( proposedUpgrade.connection.state == UPGRADE_TRY && proposedUpgrade.connection.counterpartyConnectionIdentifier == currentConnection.counterpartyConnectionIdentifier && @@ -203,7 +205,6 @@ function connUpgradeTry( // either timeout height or timestamp must be non-zero abortTransactionUnless(proposedUpgrade.TimeoutHeight != 0 || proposedUpgrade.TimeoutTimestamp != 0) - // verify proofs of counterparty state abortTransactionUnless(currentConnection.client.verifyConnectionState(proofHeight, proofConnection, counterpartyConnection)) abortTransactionUnless(currentConnection.client.verifyUpgradeStatus(proofHeight, proofUpgradeStatus, counterpartyUpgradeStatus)) @@ -272,12 +273,6 @@ function onChanUpgradeAck( // this will also check counterparty chosen version is valid restoreConnectionUnless(IsCompatible(counterpartyConnection, connection)) - // counterparty-specified timeout must not have exceeded - restoreConnectionUnless( - counterparty.UpgradeStatus.TimeoutHeight < currentHeight() && - counterparty.UpgradeStatus.TimeoutTimestamp < currentTimestamp() - ) - // upgrade is complete // set connection to OPEN and remove unnecessary state currentConnection.state = OPEN @@ -362,6 +357,39 @@ function cancelConnectionUpgrade( } ``` -It is possible that there is an extraordinary delay in the upgrade handshake occurs because of liveness issues. In this case, we do not want to indefinitely stay in the upgrade process and we will revert to the original connection by calling `CancelConnectionUpgradeTimeout`. +### Timeout Upgrade Process + +It is possible for the connection upgrade process to timeout on TRY. This may be because of a liveness issue, or because the UPGRADE_TRY transaction simply cannot pass on the counterparty; for example, the upgrade feature may not be enabled on the counterparty chain. + +In this case, we do not want the initializing chain to be stuck indefinitely in the `UPGRADE_INIT` step. Thus, the `UpgradeInit` message will contain a `TimeoutHeight` and `TimeoutTimestamp`. The counterparty chain is expected to reject `UpgradeTry` message if the specified timeout has already elapsed. + +A relayer must then submit an `UpgradeTimeout` message to the initializing chain which proves that the counterparty is still in its original state. If the proof succeeds, then the initializing chain shall also restore its original connection and cancel the upgrade. + +```typescript +function timeoutConnectionUpgrade( + identifier: Identifier, + counterpartyConnection: ConnectionEnd, + proofConnection: CommitmentProof, + proofHeight: Height, +) { + upgradeStatus = provableStore.get(statusPath(identifier)) + + // proof must be from a height after timeout has elapsed. Either timeoutHeight or timeoutTimestamp must be defined. + // if timeoutHeight is defined and proof is from before timeout height + // then abort transaction + abortTransactionUnless(upgradeStatus.TimeoutHeight.IsZero() || proofHeight >= upgradeStatus.TimeoutHeight) + // if timeoutTimestamp is defined then the consensus time from proof height must be greater than timeout timestamp + connection = getConnection(identifer) + consensusState = getConsensusState(connection.clientIdentifer, proofHeight) + abortTransactionUnless(upgradeStatus.TimeoutTimestamp.IsZero() || consensusState.Timestamp >= upgradeStatus.Timestamp) + + // counterparty connection must be proved to still be in OPEN state + abortTransactionUnless(counterpartyConnection.State === OPEN) + abortTransactionUnless(connection.client.verifyConnectionState(proofHeight, proofConnection, counterpartyConnection)) + + // we must restore the connection since the timeout verification has passed + restoreConnectionUnless(false) +} +``` -// TODO Timeout Specification \ No newline at end of file +Note that the timeout logic only applies to the INIT step. This is to protect an upgrading chain from being stuck in a non-OPEN state if the counterparty cannot execute the TRY successfully. Once the TRY step succeeds, then both sides are guaranteed to have the upgrade feature enabled. Liveness is no longer an issue, because we can wait until liveness is restored to execute the ACK step which will move the connection definitely into an OPEN state (either a successful upgrade or a rollback). \ No newline at end of file From e398dbb5edd74c934521d44439dc381aa3065d83 Mon Sep 17 00:00:00 2001 From: Aditya Sripal Date: Fri, 11 Feb 2022 15:27:17 +0100 Subject: [PATCH 08/27] crossing hellos logic --- spec/core/ics-003-connection-semantics/UPGRADES.md | 14 ++++++++++---- 1 file changed, 10 insertions(+), 4 deletions(-) diff --git a/spec/core/ics-003-connection-semantics/UPGRADES.md b/spec/core/ics-003-connection-semantics/UPGRADES.md index ee33888ee..89bc3d9eb 100644 --- a/spec/core/ics-003-connection-semantics/UPGRADES.md +++ b/spec/core/ics-003-connection-semantics/UPGRADES.md @@ -190,7 +190,7 @@ function connUpgradeTry( ) { // current connection must be OPEN or UPGRADE_INIT (crossing hellos) currentConnection = provableStore.get(connectionPath(identifier)) - abortTransactionUnless(connection.state == (OPEN || UPGRADE_INIT)) + abortTransactionUnless(currentConnection.state == OPEN || currentConnection.state == UPGRADE_INIT) // abort transaction if an unmodifiable field is modified // upgraded connection state must be in `UPGRADE_TRY` @@ -202,6 +202,13 @@ function connUpgradeTry( proposedUpgrade.connection.counterpartyClientIdentifier == currentConnection.counterpartyClientIdentifier && ) + // if there is a crossing hello, ie an UpgradeInit has been called on both connectionEnds, + // then we must ensure that the proposedUpgrade by the counterparty is the same as the currentConnection + // except for the connection state (upgrade connection will be in UPGRADE_TRY and current connection will be in UPGRADE_INIT) + // if the proposed upgrades on either side are incompatible, then we will restore the connection and cancel the upgrade. + currentConnection.state = UPGRADE_TRY + restoreConnectionUnless(currentConnection.IsEqual(proposedUpgrade.connection)) + // either timeout height or timestamp must be non-zero abortTransactionUnless(proposedUpgrade.TimeoutHeight != 0 || proposedUpgrade.TimeoutTimestamp != 0) @@ -256,10 +263,9 @@ function onChanUpgradeAck( proofUpgradeStatus: CommitmentProof, proofHeight: Height ) { - // current connection is in UPGRADE_INIT + // current connection is in UPGRADE_INIT or UPGRADE_TRY (crossing hellos) currentConnection = provableStore.get(connectionPath(identifier)) - abortTransactionUnless(connection.state == UPGRADE_INIT) - + abortTransactionUnless(currentConnection.state == UPGRADE_INIT || currentConnection.state == UPGRADE_TRY) // verify proofs of counterparty state abortTransactionUnless(currentConnection.client.verifyConnectionState(proofHeight, proofConnection, counterpartyConnection)) From b92cced283461a70591e93d1c7b68a354ad3af36 Mon Sep 17 00:00:00 2001 From: Aditya Sripal Date: Fri, 11 Feb 2022 15:53:50 +0100 Subject: [PATCH 09/27] migrations and crossing hello cleanup --- spec/core/ics-003-connection-semantics/UPGRADES.md | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-) diff --git a/spec/core/ics-003-connection-semantics/UPGRADES.md b/spec/core/ics-003-connection-semantics/UPGRADES.md index 89bc3d9eb..820f05407 100644 --- a/spec/core/ics-003-connection-semantics/UPGRADES.md +++ b/spec/core/ics-003-connection-semantics/UPGRADES.md @@ -210,7 +210,9 @@ function connUpgradeTry( restoreConnectionUnless(currentConnection.IsEqual(proposedUpgrade.connection)) // either timeout height or timestamp must be non-zero - abortTransactionUnless(proposedUpgrade.TimeoutHeight != 0 || proposedUpgrade.TimeoutTimestamp != 0) + // if the upgrade feature is implemented on the TRY chain, then a relayer may submit a TRY transaction after the timeout. + // this will restore the connection on the executing chain and allow counterparty to use the CancelUpgradeMsg to restore their connection. + restoreTransactionUnless(proposedUpgrade.TimeoutHeight != 0 || proposedUpgrade.TimeoutTimestamp != 0) // verify proofs of counterparty state abortTransactionUnless(currentConnection.client.verifyConnectionState(proofHeight, proofConnection, counterpartyConnection)) @@ -365,7 +367,7 @@ function cancelConnectionUpgrade( ### Timeout Upgrade Process -It is possible for the connection upgrade process to timeout on TRY. This may be because of a liveness issue, or because the UPGRADE_TRY transaction simply cannot pass on the counterparty; for example, the upgrade feature may not be enabled on the counterparty chain. +It is possible for the connection upgrade process to stall indefinitely on UPGRADE_TRY if the UPGRADE_TRY transaction simply cannot pass on the counterparty; for example, the upgrade feature may not be enabled on the counterparty chain. In this case, we do not want the initializing chain to be stuck indefinitely in the `UPGRADE_INIT` step. Thus, the `UpgradeInit` message will contain a `TimeoutHeight` and `TimeoutTimestamp`. The counterparty chain is expected to reject `UpgradeTry` message if the specified timeout has already elapsed. @@ -398,4 +400,8 @@ function timeoutConnectionUpgrade( } ``` -Note that the timeout logic only applies to the INIT step. This is to protect an upgrading chain from being stuck in a non-OPEN state if the counterparty cannot execute the TRY successfully. Once the TRY step succeeds, then both sides are guaranteed to have the upgrade feature enabled. Liveness is no longer an issue, because we can wait until liveness is restored to execute the ACK step which will move the connection definitely into an OPEN state (either a successful upgrade or a rollback). \ No newline at end of file +Note that the timeout logic only applies to the INIT step. This is to protect an upgrading chain from being stuck in a non-OPEN state if the counterparty cannot execute the TRY successfully. Once the TRY step succeeds, then both sides are guaranteed to have the upgrade feature enabled. Liveness is no longer an issue, because we can wait until liveness is restored to execute the ACK step which will move the connection definitely into an OPEN state (either a successful upgrade or a rollback). + +### Migrations + +A chain may have to update its internal state to be consistent with the new upgraded connection. In this case, a migration handler should be a part of the chain binary before the upgrade process so that the chain can properly migrate its state once the upgrade is successful. If a migration handler is necessary for a given upgrade but is not available, then th executing chain must reject the upgrade so as not to enter into an invalid state. This state migration will not be verified by the counterparty since it will just assume that if the connection is upgraded to a particular connection version, then the auxilliary state on the counterparty will also be updated to match the specification for the given connection version. The migration must only run once the upgrade has successfully completed and the new connection is `OPEN` (ie. on `ACK` and `CONFIRM`). \ No newline at end of file From e7ff7bf9b1cf6843b79503a9f6d9b2acfb367091 Mon Sep 17 00:00:00 2001 From: Aditya Date: Tue, 22 Feb 2022 18:22:35 +0100 Subject: [PATCH 10/27] Apply suggestions from code review Co-authored-by: Marius Poke --- .../ics-003-connection-semantics/UPGRADES.md | 27 ++++++++++--------- 1 file changed, 14 insertions(+), 13 deletions(-) diff --git a/spec/core/ics-003-connection-semantics/UPGRADES.md b/spec/core/ics-003-connection-semantics/UPGRADES.md index 820f05407..b33528094 100644 --- a/spec/core/ics-003-connection-semantics/UPGRADES.md +++ b/spec/core/ics-003-connection-semantics/UPGRADES.md @@ -2,7 +2,7 @@ ### Synopsis -This standard document specifies the interfaces and state machine logic that IBC implementations must implement in order to enable existing connections to upgrade after initial connection handshake. +This standard document specifies the interfaces and state machine logic that IBC implementations must implement in order to enable existing connections to upgrade after the initial connection handshake. ### Motivation @@ -10,12 +10,13 @@ As new features get added to IBC, chains may wish the take advantage of new conn ### Desired Properties -- Both chains must agree to the renegotiated connection parameters -- Connection state and logic on both chains should either be using the old parameters or the new parameters, but should not be in an in-between state. i.e. It should not be possible for a chain to write state to an old proof path, while the counterparty expects a new proof path. -- If the upgrade handshake is unsuccessful, the connection must fall-back to the original connection parameters -- If the upgrade handshake is successful, then both connection ends have adopted the new connection parameters and process IBC data appropriately. +- Both chains MUST agree to the renegotiated connection parameters. +- Connection state and logic on both chains SHOULD either be using the old parameters or the new parameters, but MUST NOT be in an in-between state, e.g., it MUST NOT be possible for a chain to write state to an old proof path, while the counterparty expects a new proof path. +- The connection upgrade protocol is atomic, i.e., + - either it is unsuccessful and then the connection MUST fall-back to the original connection parameters; + - or it is successful and then both connection ends MUST adopt the new connection parameters and process IBC data appropriately. - The connection upgrade protocol should have the ability to change all connection-related parameters; however the connection upgrade protocol MUST NOT be able to change the underlying `ClientState`. -- The connection upgrade protocol may not modify the connection identifiers. +The connection upgrade protocol MUST NOT modify the connection identifiers. ## Technical Specification @@ -48,17 +49,17 @@ interface ConnectionEnd { } ``` -The desired property that the connection upgrade protocol may not modify the underlying clients or connection identifiers, means that only some fields are upgradable by the upgrade protocol. +The desired property that the connection upgrade protocol MUST NOT modify the underlying clients or connection identifiers, means that only some fields of `ConnectionEnd` are upgradable by the upgrade protocol. - `state`: The state is specified by the handshake steps of the upgrade protocol. -CAN BE MODIFIED: +MAY BE MODIFIED: - `counterpartyPrefix`: The prefix MAY be modified in the upgrade protocol. The counterparty must accept the new proposed prefix value, or it must return an error during the upgrade handshake. - `version`: The version MAY be modified by the upgrade protocol. The same version negotiation that happens in the initial connection handshake can be employed for the upgrade handshake. - `delayPeriodTime`: The delay period MAY be modified by the upgrade protocol. The counterparty MUST accept the new proposed value or return an error during the upgrade handshake. - `delayPeriodBlocks`: The delay period MAY be modified by the upgrade protocol. The counterparty MUST accept the new proposed value or return an error during the upgrade handshake. -CANNOT BE MODIFIED: +MUST NOT BE MODIFIED: - `counterpartyConnectionIdentifier`: The counterparty connection identifier CAN NOT be modified by the upgrade protocol. - `clientIdentifier`: The client identifier CAN NOT be modified by the upgrade protocol - `counterpartyClientIdentifier`: The counterparty client identifier CAN NOT be modified by the upgrade protocol @@ -93,7 +94,7 @@ NOTE: One of the timeout height or timeout timestamp must be non-zero. ### Store Paths -##### Restore Connection Path +#### Restore Connection Path The chain must store the previous connection end so that it may restore it if the upgrade handshake fails. This may be stored in the private store. @@ -103,7 +104,7 @@ function restorePath(id: Identifier): Path { } ``` -##### UpgradeStatus Path +#### UpgradeStatus Path The upgrade status path is a public path that can signal the status of the upgrade to the counterparty. It does not store anything in the successful case, but it will store a sentinel abort value in the case that a chain does not accept the proposed upgrade. @@ -157,7 +158,7 @@ function connUpgradeInit( proposedUpgrade.connection.state == UPGRADE_INIT && proposedUpgrade.connection.counterpartyConnectionIdentifier == currentConnection.counterpartyConnectionIdentifier && proposedUpgrade.connection.clientIdentifier == currentConnection.clientIdentifier && - proposedUpgrade.connection.counterpartyClientIdentifier == currentConnection.counterpartyClientIdentifier && + proposedUpgrade.connection.counterpartyClientIdentifier == currentConnection.counterpartyClientIdentifier ) // either timeout height or timestamp must be non-zero @@ -199,7 +200,7 @@ function connUpgradeTry( proposedUpgrade.connection.state == UPGRADE_TRY && proposedUpgrade.connection.counterpartyConnectionIdentifier == currentConnection.counterpartyConnectionIdentifier && proposedUpgrade.connection.clientIdentifier == currentConnection.clientIdentifier && - proposedUpgrade.connection.counterpartyClientIdentifier == currentConnection.counterpartyClientIdentifier && + proposedUpgrade.connection.counterpartyClientIdentifier == currentConnection.counterpartyClientIdentifier ) // if there is a crossing hello, ie an UpgradeInit has been called on both connectionEnds, From bd795ae83db08f1fbc26517bf887084dd3ad6ed1 Mon Sep 17 00:00:00 2001 From: Aditya Sripal Date: Mon, 28 Feb 2022 18:49:46 +0100 Subject: [PATCH 11/27] continue addressing @mpoke suggestions --- .../ics-003-connection-semantics/UPGRADES.md | 28 +++++++++++-------- 1 file changed, 17 insertions(+), 11 deletions(-) diff --git a/spec/core/ics-003-connection-semantics/UPGRADES.md b/spec/core/ics-003-connection-semantics/UPGRADES.md index b33528094..a36dba094 100644 --- a/spec/core/ics-003-connection-semantics/UPGRADES.md +++ b/spec/core/ics-003-connection-semantics/UPGRADES.md @@ -22,6 +22,8 @@ The connection upgrade protocol MUST NOT modify the connection identifiers. ### Data Structures +The `ConnectionState` and `ConnectionEnd` are defined in [ICS-3](./README.md), they are reproduced here for the reader's convenience. `UPGRADE_INIT`, `UPGRADE_TRY`, `UPGRADE_ERR` are additional states added to enable the upgrade feature. + ```typescript enum ConnectionState { INIT, @@ -74,23 +76,23 @@ interface UpgradeStatus { } ``` -- UpgradeStatus contains the state of the upgrade. If upgrade is successful it will contain the `ConnectionState` of the upgrading connection on the executing chain. If the executing chain wants to abort the upgrade, it will restore its previous connection under its connection path with state `OPEN` and write an `UpgradeStatus` with state `UPGRADE_ERR`. +- `state`: If the upgrade step was successful, then the `state` of the `UpgradeStatus` will be the same as the `state` of the `ConnectionEnd`. If the upgrade step failed, then the `ConnectionEnd` will be reverted to its previous `OPEN` state; and the `UpgradeStatus` will have state `UPGRADE_ERR` to signal to counterparty to revert its connection end as well. - `timeoutHeight`: Timeout height indicates the height at which the counterparty must no longer proceed with the upgrade handshake. The chains will then preserve their original connection and the upgrade handshake is aborted. - `timeoutTimestamp`: Timeout timestamp indicates the time on the counterparty at which the counterparty must no longer proceed with the upgrade handshake. The chains will then preserve their original connection and the upgrade handshake is aborted. ```typescript interface UpgradeConnectionState { - connection: ConnectionEnd + proposedConnection: ConnectionEnd timeoutHeight: Height timeoutTimestamp: uint64 } ``` -- `proposedConnectionState`: Proposed `ConnectionState` to replace the current `ConnectionState`, it MUST ONLY modify the fields that are permissible to +- `proposedConnection`: Proposed `ConnectionEnd` to replace the current `ConnectionEnd`, it MUST ONLY modify the fields that are permissable to be modified by upgrade connection process. - `timeoutHeight`: Timeout height indicates the height at which the counterparty must no longer proceed with the upgrade handshake. The chains will then preserve their original connection and the upgrade handshake is aborted. - `timeoutTimestamp`: Timeout timestamp indicates the time on the counterparty at which the counterparty must no longer proceed with the upgrade handshake. The chains will then preserve their original connection and the upgrade handshake is aborted. -NOTE: One of the timeout height or timeout timestamp must be non-zero. +At least one of the timeoutHeight or timeoutTimestamp MUST be non-zero. ### Store Paths @@ -115,6 +117,10 @@ function statusPath(id: Identifier): Path { } ``` +## Sub-Protocols + +The Connection Upgrade process consists of three sub-protocols: `UpgradeHandshake`, `CancelConnectionUpgrade`, and `TimeoutConnectionUpgrade`. In the case where both chains approve of the proposed upgrade, the upgrade handshake protocol should complete successfully and the ConnectionEnd should upgrade successf + ### Upgrade Handshake The upgrade handshake defines four datagrams: *ConnUpgradeInit*, *ConnUpgradeTry*, *ConnUpgradeAck*, and *ConnUpgradeConfirm* @@ -216,8 +222,8 @@ function connUpgradeTry( restoreTransactionUnless(proposedUpgrade.TimeoutHeight != 0 || proposedUpgrade.TimeoutTimestamp != 0) // verify proofs of counterparty state - abortTransactionUnless(currentConnection.client.verifyConnectionState(proofHeight, proofConnection, counterpartyConnection)) - abortTransactionUnless(currentConnection.client.verifyUpgradeStatus(proofHeight, proofUpgradeStatus, counterpartyUpgradeStatus)) + abortTransactionUnless(verifyConnectionState(currentConnection, proofHeight, proofConnection, currentConnection.counterpartyConnectionIdentifier, counterpartyConnection)) + abortTransactionUnless(verifyUpgradeStatus(currentConnection, proofHeight, proofUpgradeStatus, currentConnection.counterpartyConnectionIdentifier, counterpartyUpgradeStatus)) // verify that counterparty connection unmodifiable fields have not changed and counterparty state // is UPGRADE_INIT @@ -271,8 +277,8 @@ function onChanUpgradeAck( abortTransactionUnless(currentConnection.state == UPGRADE_INIT || currentConnection.state == UPGRADE_TRY) // verify proofs of counterparty state - abortTransactionUnless(currentConnection.client.verifyConnectionState(proofHeight, proofConnection, counterpartyConnection)) - abortTransactionUnless(currentConnection.client.verifyUpgradeStatus(proofHeight, proofUpgradeStatus, counterpartyUpgradeStatus)) + abortTransactionUnless(verifyConnectionState(currentConnection, proofHeight, proofConnection, currentConnection.counterpartyConnectionIdentifier, counterpartyConnection)) + abortTransactionUnless(verifyUpgradeStatus(currentConnection, proofHeight, proofUpgradeStatus, currentConnection.counterpartyConnectionIdentifier, counterpartyUpgradeStatus)) // counterparty must be in TRY state restoreConnectionUnless(counterpartyStatus.state == UPGRADE_TRY) @@ -306,7 +312,7 @@ function onChanUpgradeConfirm( abortTransactionUnless(counterpartyConnection.State == OPEN) // verify proofs of counterparty state - abortTransactionUnless(currentConnection.client.verifyConnectionState(proofHeight, proofConnection, counterpartyConnection)) + abortTransactionUnless(verifyConnectionState(currentConnection, proofHeight, proofConnection, currentConnection.counterpartyConnectionIdentifier, counterpartyConnection)) // upgrade is complete // set connection to OPEN and remove unnecessary state @@ -324,7 +330,7 @@ function restoreConnectionUnless(condition: bool) { if !condition { // cancel upgrade // set UpgradeStatus to `UPGRADE_ERR` - // and restore original conneciton + // and restore original connection errStatus = UpgradeStatus{UPGRADE_ERR} provableStore.set(statusPath(identifier), errStatus) originalConnection = privateStore.get(restorePath(identifier)) @@ -354,7 +360,7 @@ function cancelConnectionUpgrade( abortTransactionUnless(counterpartyUpgradeStatus.state == UPGRADE_ERR) - abortTransactionUnless(currentConnection.client.verifyUpgradeStatus(proofHeight, proofUpgradeStatus, counterpartyUpgradeStatus)) + abortTransactionUnless(verifyUpgradeStatus(currentConnection, proofHeight, proofUpgradeStatus, currentConnection.counterpartyConnectionIdentifier, counterpartyUpgradeStatus)) // cancel upgrade // and restore original conneciton From da255adb398e8d2550b5c21d2ddcab13a597d07f Mon Sep 17 00:00:00 2001 From: Aditya Sripal Date: Tue, 1 Mar 2022 18:17:07 +0100 Subject: [PATCH 12/27] clarify handshake pseudocode --- .../ics-003-connection-semantics/UPGRADES.md | 30 +++++++++++++------ 1 file changed, 21 insertions(+), 9 deletions(-) diff --git a/spec/core/ics-003-connection-semantics/UPGRADES.md b/spec/core/ics-003-connection-semantics/UPGRADES.md index a36dba094..abd81b387 100644 --- a/spec/core/ics-003-connection-semantics/UPGRADES.md +++ b/spec/core/ics-003-connection-semantics/UPGRADES.md @@ -199,6 +199,19 @@ function connUpgradeTry( currentConnection = provableStore.get(connectionPath(identifier)) abortTransactionUnless(currentConnection.state == OPEN || currentConnection.state == UPGRADE_INIT) + if currentConnection.state == UPGRADE_INIT { + // if there is a crossing hello, ie an UpgradeInit has been called on both connectionEnds, + // then we must ensure that the proposedUpgrade by the counterparty is the same as the currentConnection + // except for the connection state (upgrade connection will be in UPGRADE_TRY and current connection will be in UPGRADE_INIT) + // if the proposed upgrades on either side are incompatible, then we will restore the connection and cancel the upgrade. + currentConnection.state = UPGRADE_TRY + restoreConnectionUnless(currentConnection.IsEqual(proposedUpgrade.connection)) + } else { + // this is first message in upgrade handshake on this chain so we must store original connection in restore path + // in case we need to restore connection later. + privateStore.set(restorePath(identifier), currentConnection) + } + // abort transaction if an unmodifiable field is modified // upgraded connection state must be in `UPGRADE_TRY` // NOTE: Any added fields are by default modifiable. @@ -209,17 +222,11 @@ function connUpgradeTry( proposedUpgrade.connection.counterpartyClientIdentifier == currentConnection.counterpartyClientIdentifier ) - // if there is a crossing hello, ie an UpgradeInit has been called on both connectionEnds, - // then we must ensure that the proposedUpgrade by the counterparty is the same as the currentConnection - // except for the connection state (upgrade connection will be in UPGRADE_TRY and current connection will be in UPGRADE_INIT) - // if the proposed upgrades on either side are incompatible, then we will restore the connection and cancel the upgrade. - currentConnection.state = UPGRADE_TRY - restoreConnectionUnless(currentConnection.IsEqual(proposedUpgrade.connection)) - + // either timeout height or timestamp must be non-zero // if the upgrade feature is implemented on the TRY chain, then a relayer may submit a TRY transaction after the timeout. // this will restore the connection on the executing chain and allow counterparty to use the CancelUpgradeMsg to restore their connection. - restoreTransactionUnless(proposedUpgrade.TimeoutHeight != 0 || proposedUpgrade.TimeoutTimestamp != 0) + restoreConnectionUnless(proposedUpgrade.TimeoutHeight != 0 || proposedUpgrade.TimeoutTimestamp != 0) // verify proofs of counterparty state abortTransactionUnless(verifyConnectionState(currentConnection, proofHeight, proofConnection, currentConnection.counterpartyConnectionIdentifier, counterpartyConnection)) @@ -246,6 +253,9 @@ function connUpgradeTry( version = pickVersion(versionsIntersection) // throws if there is no intersection // both connection ends must be mutually compatible. + // this function has been left unspecified since it will depend on the specific structure of the new connection. + // It is the responsibility of implementations to make sure that verification that the proposed new connections + // on either side are correctly constructed according to the new version selected. restoreConnectionUnless(IsCompatible(counterpartyConnection, proposedUpgrade.Connection)) upgradeStatus = UpgradeStatus{ @@ -256,7 +266,6 @@ function connUpgradeTry( provableStore.set(statusPath(identifier), upgradeStatus) provableStore.set(connectionPath(identifier), proposedUpgrade.connection) - privateStore.set(restorePath(identifier), currentConnection) } ``` @@ -286,6 +295,9 @@ function onChanUpgradeAck( // verify connections are mutually compatible // this will also check counterparty chosen version is valid + // this function has been left unspecified since it will depend on the specific structure of the new connection. + // It is the responsibility of implementations to make sure that verification that the proposed new connections + // on either side are correctly constructed according to the new version selected. restoreConnectionUnless(IsCompatible(counterpartyConnection, connection)) // upgrade is complete From d5588c8f892dbb5b2ea78a3300dd522ca2263dc5 Mon Sep 17 00:00:00 2001 From: Aditya Sripal Date: Wed, 2 Mar 2022 17:31:52 +0100 Subject: [PATCH 13/27] remove unnecessary structs and simplify protocol --- .../ics-003-connection-semantics/UPGRADES.md | 199 ++++++++++++------ 1 file changed, 131 insertions(+), 68 deletions(-) diff --git a/spec/core/ics-003-connection-semantics/UPGRADES.md b/spec/core/ics-003-connection-semantics/UPGRADES.md index abd81b387..69a63806f 100644 --- a/spec/core/ics-003-connection-semantics/UPGRADES.md +++ b/spec/core/ics-003-connection-semantics/UPGRADES.md @@ -69,26 +69,12 @@ MUST NOT BE MODIFIED: NOTE: If the upgrade adds any fields to the `ConnectionEnd` these are by default modifiable, and can be arbitrarily chosen by an Actor (e.g. chain governance) which has permission to initiate the upgrade. ```typescript -interface UpgradeStatus { - state: ConnectionState +interface UpgradeTimeout { timeoutHeight: Height timeoutTimestamp: uint64 } ``` -- `state`: If the upgrade step was successful, then the `state` of the `UpgradeStatus` will be the same as the `state` of the `ConnectionEnd`. If the upgrade step failed, then the `ConnectionEnd` will be reverted to its previous `OPEN` state; and the `UpgradeStatus` will have state `UPGRADE_ERR` to signal to counterparty to revert its connection end as well. -- `timeoutHeight`: Timeout height indicates the height at which the counterparty must no longer proceed with the upgrade handshake. The chains will then preserve their original connection and the upgrade handshake is aborted. -- `timeoutTimestamp`: Timeout timestamp indicates the time on the counterparty at which the counterparty must no longer proceed with the upgrade handshake. The chains will then preserve their original connection and the upgrade handshake is aborted. - -```typescript -interface UpgradeConnectionState { - proposedConnection: ConnectionEnd - timeoutHeight: Height - timeoutTimestamp: uint64 -} -``` - -- `proposedConnection`: Proposed `ConnectionEnd` to replace the current `ConnectionEnd`, it MUST ONLY modify the fields that are permissable to be modified by upgrade connection process. - `timeoutHeight`: Timeout height indicates the height at which the counterparty must no longer proceed with the upgrade handshake. The chains will then preserve their original connection and the upgrade handshake is aborted. - `timeoutTimestamp`: Timeout timestamp indicates the time on the counterparty at which the counterparty must no longer proceed with the upgrade handshake. The chains will then preserve their original connection and the upgrade handshake is aborted. @@ -106,14 +92,87 @@ function restorePath(id: Identifier): Path { } ``` -#### UpgradeStatus Path +#### UpgradeError Path + +The upgrade error path is a public path that can signal an error of the upgrade to the counterparty. It does not store anything in the successful case, but it will store a sentinel abort value in the case that a chain does not accept the proposed upgrade. + +```typescript +function errorPath(id: Identifier): Path { + return "connections/{id}/upgradeError" + +} +``` + +The UpgradeError MUST have an associated verification function added to the connection and client interfaces so that a counterparty may verify that chain has stored an error in the UpgradeError path. + +```typescript +// Connection VerifyUpgradeError method +function verifyUpgradeError( + connection: ConnectionEnd, + height: Height, + proof: CommitmentProof, + upgradeErrorReceipt: []byte, +) { + client = queryClient(connection.clientIdentifier) + client.verifyUpgradeError(height, connection.counterpartyPrefix, proof, connection.counterpartyConnectionIdentifier, upgradeErrorReceipt) +} +``` + +```typescript +// Client VerifyUpgradeError +function verifyUpgradeError( + clientState: ClientState, + height: Height, + prefix: CommitmentPrefix, + proof: CommitmentProof, + counterpartyConnectionIdentifier: Identifier, + upgradeErrorReceipt []byte, +) { + path = applyPrefix(prefix, errorPath(counterpartyConnectionIdentifier)) + abortTransactionUnless(!clientState.frozen) + return clientState.verifiedRoots[height].verifyMembership(path, upgradeErrorReceipt, proof) +} +``` + +#### TimeoutPath -The upgrade status path is a public path that can signal the status of the upgrade to the counterparty. It does not store anything in the successful case, but it will store a sentinel abort value in the case that a chain does not accept the proposed upgrade. +The timeout path is a public path set by the upgrade initiator to determine when the TRY step should timeout. It stores the `timeoutHeight` and `timeoutTimestamp` by which point the counterparty must have progressed to the TRY step. This path will be proven on the counterparty chain in case of a successful TRY, to ensure timeout has not passed. Or in the case of a timeout, in which case counterparty proves that the timeout has passed on its chain and restores the connection. ```typescript -function statusPath(id: Identifier): Path { - return "connections/{id}/upgradeStatus" +function timeoutPath(id: Identifier) Path { + return "connections/{id}/upgradeTimeout" +} +``` +The timeout path MUST have associated verification methods on the connection and client interfaces in order for a counterparty to prove that a chain stored a particular `UpgradeTimeout`. + +```typescript +// Connection VerifyUpgradeTimeout method +function verifyUpgradeTimeout( + connection: ConnectionEnd, + height: Height, + proof: CommitmentProof, + upgradeTimeout: UpgradeTimeout, +) { + client = queryClient(connection.clientIdentifier) + client.verifyUpgradeTimeout(height, connection.counterpartyPrefix, proof, connection.counterpartyConnectionIdentifier, upgradeTimeout) +} +``` + +```typescript +// Client VerifyUpgradeTimeout +function verifyUpgradeTimeout( + clientState: ClientState, + height: Height, + prefix: CommitmentPrefix, + proof: CommitmentProof, + counterpartyConnectionIdentifier: Identifier, + upgradeTimeout: UpgradeTimeout, +) { + path = applyPrefix(prefix, timeoutPath(counterpartyConnectionIdentifier)) + abortTransactionUnless(!clientState.frozen) + timeoutBytes = protobuf.marshal(upgradeTimeout) + return clientState.verifiedRoots[height].verifyMembership(path, timeoutBytes, proof) } ``` @@ -139,19 +198,20 @@ At the end of an opening handshake between two chains implementing the sub-proto - Each chain is running their new upgraded connection end and is processing upgraded logic and state according to the upgraded parameters. - Each chain has knowledge of and has agreed to the counterparty's upgraded connection parameters. -If a chain does not agree to the proposed counterparty `UpgradeConnectionState`, it may abort the upgrade handshake by writing `UPGRADE_ERR` as the state in the `UpgradeStatus` and storing -it under the status path and restore the original connection. +If a chain does not agree to the proposed counterparty `UpgradedConnection`, it may abort the upgrade handshake by writing an error receipt into the `errorPath` and restoring the original connection. The error receipt MAY be arbitrary bytes and MUST be non-empty. -`statusPath(id) => UpgradeStatus{UPGRADE_ERR}` +`errorPath(id) => error_receipt` -A relayer may then submit a `CancelConnectionUpgradeMsg` to the counterparty. Upon receiving this message a chain must verify that the counterparty wrote `UPGRADE_ERR` into its `UpgradeStatus` and if successful, it will restore its original connection as well thus cancelling the upgrade. +A relayer may then submit a `CancelConnectionUpgradeMsg` to the counterparty. Upon receiving this message a chain must verify that the counterparty wrote a non-empty error receipt into its `UpgradeError` and if successful, it will restore its original connection as well thus cancelling the upgrade. If an upgrade message arrives after the specified timeout, then the message MUST NOT execute successfully. Again a relayer may submit a proof of this in a `CancelConnectionUpgradeTimeoutMsg` so that counterparty cancels the upgrade and restores it original connection as well. ```typescript function connUpgradeInit( identifier: Identifier, - proposedUpgrade: UpgradeConnectionState + proposedUpgradeConnection: ConnectionEnd, + counterpartyTimeoutHeight: Height, + counterpartyTimeoutTimestamp: uint64, ) { // current connection must be OPEN currentConnection = provableStore.get(connectionPath(identifier)) @@ -161,22 +221,21 @@ function connUpgradeInit( // upgraded connection state must be in `UPGRADE_INIT` // NOTE: Any added fields are by default modifiable. abortTransactionUnless( - proposedUpgrade.connection.state == UPGRADE_INIT && - proposedUpgrade.connection.counterpartyConnectionIdentifier == currentConnection.counterpartyConnectionIdentifier && - proposedUpgrade.connection.clientIdentifier == currentConnection.clientIdentifier && - proposedUpgrade.connection.counterpartyClientIdentifier == currentConnection.counterpartyClientIdentifier + proposedUpgradeConnection.state == UPGRADE_INIT && + proposedUpgradeConnection.counterpartyConnectionIdentifier == currentConnection.counterpartyConnectionIdentifier && + proposedUpgradeConnection.clientIdentifier == currentConnection.clientIdentifier && + proposedUpgradeConnection.counterpartyClientIdentifier == currentConnection.counterpartyClientIdentifier ) // either timeout height or timestamp must be non-zero - abortTransactionUnless(proposedUpgrade.TimeoutHeight != 0 || proposedUpgrade.TimeoutTimestamp != 0) + abortTransactionUnless(counterpartyTimeoutHeight != 0 || counterpartyTimeoutTimestamp != 0) - upgradeStatus = UpgradeStatus{ - state: UPGRADE_INIT, - timeoutHeight: proposedUpgrade.timeoutHeight, - timeoutTimestamp: proposedUpgrade.timeoutTimestamp, + upgradeTimeout = UpgradeTimeout{ + timeoutHeight: counterpartyTimeoutHeight, + timeoutTimestamp: counterpartyTimeoutTimestamp, } - provableStore.set(statusPath(identifier), upgradeStatus) + provableStore.set(timeoutPath(identifier), upgradeTimeout) provableStore.set(connectionPath(identifier), proposedUpgrade.connection) privateStore.set(restorePath(identifier), currentConnection) } @@ -190,9 +249,11 @@ function connUpgradeTry( identifier: Identifier, proposedUpgrade: UpgradeConnectionState, counterpartyConnection: ConnectionEnd, - counterpartyUpgradeStatus: UpgradeStatus, + timeoutHeight: Height, + timeoutTimestamp: uint64, + UpgradeTimeout: UpgradeTimeout, proofConnection: CommitmentProof, - proofUpgradeStatus: CommitmentProof, + proofUpgradeTimeout: CommitmentProof, proofHeight: Height ) { // current connection must be OPEN or UPGRADE_INIT (crossing hellos) @@ -226,11 +287,15 @@ function connUpgradeTry( // either timeout height or timestamp must be non-zero // if the upgrade feature is implemented on the TRY chain, then a relayer may submit a TRY transaction after the timeout. // this will restore the connection on the executing chain and allow counterparty to use the CancelUpgradeMsg to restore their connection. - restoreConnectionUnless(proposedUpgrade.TimeoutHeight != 0 || proposedUpgrade.TimeoutTimestamp != 0) + restoreConnectionUnless(timeoutHeight != 0 || timeoutTimestamp != 0) + upgradeTimeout = UpgradeTimeout{ + timeoutHeight: timeoutHeight, + timeoutTimestamp: timeoutTimestamp, + } // verify proofs of counterparty state abortTransactionUnless(verifyConnectionState(currentConnection, proofHeight, proofConnection, currentConnection.counterpartyConnectionIdentifier, counterpartyConnection)) - abortTransactionUnless(verifyUpgradeStatus(currentConnection, proofHeight, proofUpgradeStatus, currentConnection.counterpartyConnectionIdentifier, counterpartyUpgradeStatus)) + abortTransactionUnless(verifyUpgradeTimeout(currentConnection, proofHeight, proofUpgradeTimeout, currentConnection.counterpartyConnectionIdentifier, upgradeTimeout)) // verify that counterparty connection unmodifiable fields have not changed and counterparty state // is UPGRADE_INIT @@ -240,12 +305,11 @@ function connUpgradeTry( counterpartyConnection.clientIdentifier == currentConnection.counterpartyClientIdentifier && counterpartyConnection.counterpartyClientIdentifier == currentConnection.clientIdentifier ) - restoreConnectionUnless(counterpartyUpgradeStatus.state == UPGRADE_INIT) // counterparty-specified timeout must not have exceeded restoreConnectionUnless( - counterparty.UpgradeStatus.TimeoutHeight < currentHeight() && - counterparty.UpgradeStatus.TimeoutTimestamp < currentTimestamp() + timeoutHeight < currentHeight() && + timeoutTimestamp < currentTimestamp() ) // verify chosen versions are compatible @@ -258,13 +322,6 @@ function connUpgradeTry( // on either side are correctly constructed according to the new version selected. restoreConnectionUnless(IsCompatible(counterpartyConnection, proposedUpgrade.Connection)) - upgradeStatus = UpgradeStatus{ - state: UPGRADE_TRY, - timeoutHeight: proposedUpgrade.timeoutHeight, - timeoutTimestamp: proposedUpgrade.timeoutTimestamp, - } - - provableStore.set(statusPath(identifier), upgradeStatus) provableStore.set(connectionPath(identifier), proposedUpgrade.connection) } ``` @@ -276,9 +333,9 @@ NOTE: It is up to individual implementations how they will provide access-contro function onChanUpgradeAck( identifier: Identifier, counterpartyConnection: ConnectionEnd, - counterpartyStatus: UpgradeStatus, + counterpartyStatus: UpgradeError, proofConnection: CommitmentProof, - proofUpgradeStatus: CommitmentProof, + proofUpgradeError: CommitmentProof, proofHeight: Height ) { // current connection is in UPGRADE_INIT or UPGRADE_TRY (crossing hellos) @@ -287,10 +344,8 @@ function onChanUpgradeAck( // verify proofs of counterparty state abortTransactionUnless(verifyConnectionState(currentConnection, proofHeight, proofConnection, currentConnection.counterpartyConnectionIdentifier, counterpartyConnection)) - abortTransactionUnless(verifyUpgradeStatus(currentConnection, proofHeight, proofUpgradeStatus, currentConnection.counterpartyConnectionIdentifier, counterpartyUpgradeStatus)) // counterparty must be in TRY state - restoreConnectionUnless(counterpartyStatus.state == UPGRADE_TRY) restoreConnectionUnless(counterpartyConnection.State == UPGRADE_TRY) // verify connections are mutually compatible @@ -304,7 +359,7 @@ function onChanUpgradeAck( // set connection to OPEN and remove unnecessary state currentConnection.state = OPEN provableStore.set(connectionPath(identifier), currentConnection) - provableStore.delete(statusPath(identifier)) + provableStore.delete(timeoutPath(identifier)) privateStore.delete(restorePath(identifier)) } ``` @@ -330,23 +385,24 @@ function onChanUpgradeConfirm( // set connection to OPEN and remove unnecessary state currentConnection.state = OPEN provableStore.set(connectionPath(identifier), currentConnection) - provableStore.delete(statusPath(identifier)) + provableStore.delete(timeoutPath(identifier)) privateStore.delete(restorePath(identifier)) } ``` -Note: In the `TRY` and `ACK` steps, the chain has the ability to abort the upgrade process if the upgrade parameters are not suitable or the timeout has exceeded. In this case, the chain is expected to write `UPGRADE_ERR` as the state in the `UPGRADE_STATUS`, and restore the original connection with state `OPEN`. The counterparty can then verify that the upgrade status is `UPGRADE_ERR` and restore its original connection to `OPEN` as well, thus cancelling the upgrade. +Note: In the `TRY` and `ACK` steps, the chain has the ability to abort the upgrade process if the upgrade parameters are not suitable or the timeout has exceeded. In this case, the chain is expected to write `UPGRADE_ERR` as the state in the `UPGRADE_STATUS`, and restore the original connection with state `OPEN`. The counterparty can then verify that the upgrade error is `UPGRADE_ERR` and restore its original connection to `OPEN` as well, thus cancelling the upgrade. ```typescript function restoreConnectionUnless(condition: bool) { if !condition { // cancel upgrade - // set UpgradeStatus to `UPGRADE_ERR` + // write an error receipt into the error path // and restore original connection - errStatus = UpgradeStatus{UPGRADE_ERR} - provableStore.set(statusPath(identifier), errStatus) + errorReceipt = []byte{1} + provableStore.set(errorPath(identifier), errorReceipt) originalConnection = privateStore.get(restorePath(identifier)) provableStore.set(connectionPath(identifier), originalConnection) + provableStore.delete(timeoutPath(identifier)) privateStore.delete(restorePath(identifier)) // caller should return as well } else { @@ -357,29 +413,32 @@ function restoreConnectionUnless(condition: bool) { ### Cancel Upgrade Process -As discussed, during the upgrade handshake a chain may cancel the upgrade by writing `UPGRADE_ERR` into the `UpgradeStatus` and restoring the original connection to `OPEN`. The counterparty must then restore its connection to `OPEN` as well. A relayer can facilitate this by calling `CancelConnectionUpgrade` +As discussed, during the upgrade handshake a chain may cancel the upgrade by writing an error receipt into the `UpgradeError` and restoring the original connection to `OPEN`. The counterparty must then restore its connection to `OPEN` as well. A relayer can facilitate this by calling `CancelConnectionUpgrade`: ```typescript function cancelConnectionUpgrade( identifier: Identifer, - counterpartyUpgradeStatus: UpgradeStatus, - proofUpgradeStatus: CommitmentProof, + errorReceipt: []byte, + counterpartyUpgradeError: UpgradeError, + proofUpgradeError: CommitmentProof, proofHeight: Height, ) { // current connection is in UPGRADE_INIT or UPGRADE_TRY currentConnection = provableStore.get(connectionPath(identifier)) - abortTransactionUnless(connection.state == (UPGRADE_INIT || UPGRADE_TRY)) + abortTransactionUnless(connection.state == UPGRADE_INIT || connection.state == UPGRADE_TRY) - abortTransactionUnless(counterpartyUpgradeStatus.state == UPGRADE_ERR) + abortTransactionUnless(!isEmpty(errorReceipt)) - abortTransactionUnless(verifyUpgradeStatus(currentConnection, proofHeight, proofUpgradeStatus, currentConnection.counterpartyConnectionIdentifier, counterpartyUpgradeStatus)) + abortTransactionUnless(verifyUpgradeError(currentConnection, proofHeight, proofUpgradeError, currentConnection.counterpartyConnectionIdentifier, counterpartyUpgradeError)) // cancel upgrade // and restore original conneciton // delete unnecessary state originalConnection = privateStore.get(restorePath(identifier)) provableStore.set(connectionPath(identifier), originalConnection) - provableStore.delete(statusPath(identifier)) + + // delete auxilliary upgrade state + provableStore.delete(timeoutPath(identifier)) privateStore.delete(restorePath(identifier)) } ``` @@ -399,16 +458,20 @@ function timeoutConnectionUpgrade( proofConnection: CommitmentProof, proofHeight: Height, ) { - upgradeStatus = provableStore.get(statusPath(identifier)) + // current connection must be in UPGRADE_INIT + currentConnection = provableStore.get(connectionPath(identifier)) + abortTransactionUnles(currentConnection.state == UPGRADE_INIT) + + upgradeTimeout = provableStore.get(timeoutPath(identifier)) // proof must be from a height after timeout has elapsed. Either timeoutHeight or timeoutTimestamp must be defined. // if timeoutHeight is defined and proof is from before timeout height // then abort transaction - abortTransactionUnless(upgradeStatus.TimeoutHeight.IsZero() || proofHeight >= upgradeStatus.TimeoutHeight) + abortTransactionUnless(upgradeTimeout.timeoutHeight.IsZero() || proofHeight >= upgradeTimeout.timeoutHeight) // if timeoutTimestamp is defined then the consensus time from proof height must be greater than timeout timestamp connection = getConnection(identifer) consensusState = getConsensusState(connection.clientIdentifer, proofHeight) - abortTransactionUnless(upgradeStatus.TimeoutTimestamp.IsZero() || consensusState.Timestamp >= upgradeStatus.Timestamp) + abortTransactionUnless(upgradeTimeout.timeoutTimestamp.IsZero() || consensusState.getTimestamp() >= upgradeTimeout.timestamp) // counterparty connection must be proved to still be in OPEN state abortTransactionUnless(counterpartyConnection.State === OPEN) From 3d30b283f7e2955f6defe136d85d32f3cfd478df Mon Sep 17 00:00:00 2001 From: Aditya Sripal Date: Wed, 2 Mar 2022 17:42:39 +0100 Subject: [PATCH 14/27] clarify TRY actor reasoning --- spec/core/ics-003-connection-semantics/UPGRADES.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/spec/core/ics-003-connection-semantics/UPGRADES.md b/spec/core/ics-003-connection-semantics/UPGRADES.md index 69a63806f..23036618f 100644 --- a/spec/core/ics-003-connection-semantics/UPGRADES.md +++ b/spec/core/ics-003-connection-semantics/UPGRADES.md @@ -326,7 +326,7 @@ function connUpgradeTry( } ``` -NOTE: It is up to individual implementations how they will provide access-control to the `ConnUpgradeTry` function. E.g. chain governance, permissioned actor, DAO, etc. +NOTE: It is up to individual implementations how they will provide access-control to the `ConnUpgradeTry` function. E.g. chain governance, permissioned actor, DAO, etc. A chain may decide to have permissioned **or** permissionless `UpgradeTry`. In the permissioned case, both chains must explicitly consent to the upgrade, in the permissionless case; one chain initiates the upgrade and the other chain agrees to the upgrade by default. In the permissionless case, a relayer may submit the `ConnUpgradeTry` datagram. ```typescript From 37916cdcf85b62dab2f5ae98052d61330d7db99c Mon Sep 17 00:00:00 2001 From: Aditya Sripal Date: Wed, 2 Mar 2022 17:47:21 +0100 Subject: [PATCH 15/27] TRY timeout clarification --- spec/core/ics-003-connection-semantics/UPGRADES.md | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/spec/core/ics-003-connection-semantics/UPGRADES.md b/spec/core/ics-003-connection-semantics/UPGRADES.md index 23036618f..0ff481d4a 100644 --- a/spec/core/ics-003-connection-semantics/UPGRADES.md +++ b/spec/core/ics-003-connection-semantics/UPGRADES.md @@ -390,8 +390,6 @@ function onChanUpgradeConfirm( } ``` -Note: In the `TRY` and `ACK` steps, the chain has the ability to abort the upgrade process if the upgrade parameters are not suitable or the timeout has exceeded. In this case, the chain is expected to write `UPGRADE_ERR` as the state in the `UPGRADE_STATUS`, and restore the original connection with state `OPEN`. The counterparty can then verify that the upgrade error is `UPGRADE_ERR` and restore its original connection to `OPEN` as well, thus cancelling the upgrade. - ```typescript function restoreConnectionUnless(condition: bool) { if !condition { @@ -413,7 +411,7 @@ function restoreConnectionUnless(condition: bool) { ### Cancel Upgrade Process -As discussed, during the upgrade handshake a chain may cancel the upgrade by writing an error receipt into the `UpgradeError` and restoring the original connection to `OPEN`. The counterparty must then restore its connection to `OPEN` as well. A relayer can facilitate this by calling `CancelConnectionUpgrade`: +During the upgrade handshake a chain may cancel the upgrade by writing an error receipt into the error path and restoring the original connection to `OPEN`. The counterparty must then restore its connection to `OPEN` as well. A relayer can facilitate this by calling `CancelConnectionUpgrade`: ```typescript function cancelConnectionUpgrade( @@ -484,6 +482,8 @@ function timeoutConnectionUpgrade( Note that the timeout logic only applies to the INIT step. This is to protect an upgrading chain from being stuck in a non-OPEN state if the counterparty cannot execute the TRY successfully. Once the TRY step succeeds, then both sides are guaranteed to have the upgrade feature enabled. Liveness is no longer an issue, because we can wait until liveness is restored to execute the ACK step which will move the connection definitely into an OPEN state (either a successful upgrade or a rollback). +The TRY chain will receive the timeout parameters chosen by the counterparty on INIT, so that it can reject any TRY message that is received after the specified timeout. This prevents the handshake from entering into an invalid state, in which the INIT chain processes a timeout successfully and restores its connection to `OPEN` while the TRY chain at a later point successfully writes a `TRY` state. + ### Migrations A chain may have to update its internal state to be consistent with the new upgraded connection. In this case, a migration handler should be a part of the chain binary before the upgrade process so that the chain can properly migrate its state once the upgrade is successful. If a migration handler is necessary for a given upgrade but is not available, then th executing chain must reject the upgrade so as not to enter into an invalid state. This state migration will not be verified by the counterparty since it will just assume that if the connection is upgraded to a particular connection version, then the auxilliary state on the counterparty will also be updated to match the specification for the given connection version. The migration must only run once the upgrade has successfully completed and the new connection is `OPEN` (ie. on `ACK` and `CONFIRM`). \ No newline at end of file From f483d1f5ab6076980ece018998ba8e3bc7bafea5 Mon Sep 17 00:00:00 2001 From: Aditya Sripal Date: Wed, 2 Mar 2022 17:53:12 +0100 Subject: [PATCH 16/27] cleanup pseudocode --- .../ics-003-connection-semantics/UPGRADES.md | 19 +++++++++---------- 1 file changed, 9 insertions(+), 10 deletions(-) diff --git a/spec/core/ics-003-connection-semantics/UPGRADES.md b/spec/core/ics-003-connection-semantics/UPGRADES.md index 0ff481d4a..d2d9c596c 100644 --- a/spec/core/ics-003-connection-semantics/UPGRADES.md +++ b/spec/core/ics-003-connection-semantics/UPGRADES.md @@ -293,10 +293,6 @@ function connUpgradeTry( timeoutTimestamp: timeoutTimestamp, } - // verify proofs of counterparty state - abortTransactionUnless(verifyConnectionState(currentConnection, proofHeight, proofConnection, currentConnection.counterpartyConnectionIdentifier, counterpartyConnection)) - abortTransactionUnless(verifyUpgradeTimeout(currentConnection, proofHeight, proofUpgradeTimeout, currentConnection.counterpartyConnectionIdentifier, upgradeTimeout)) - // verify that counterparty connection unmodifiable fields have not changed and counterparty state // is UPGRADE_INIT restoreConnectionUnless( @@ -314,7 +310,7 @@ function connUpgradeTry( // verify chosen versions are compatible versionsIntersection = intersection(counterpartyConnection.version, proposedUpgrade.Connection.version) - version = pickVersion(versionsIntersection) // throws if there is no intersection + version = pickVersion(versionsIntersection) // aborts transaction if there is no intersection // both connection ends must be mutually compatible. // this function has been left unspecified since it will depend on the specific structure of the new connection. @@ -322,6 +318,10 @@ function connUpgradeTry( // on either side are correctly constructed according to the new version selected. restoreConnectionUnless(IsCompatible(counterpartyConnection, proposedUpgrade.Connection)) + // verify proofs of counterparty state + abortTransactionUnless(verifyConnectionState(currentConnection, proofHeight, proofConnection, currentConnection.counterpartyConnectionIdentifier, counterpartyConnection)) + abortTransactionUnless(verifyUpgradeTimeout(currentConnection, proofHeight, proofUpgradeTimeout, currentConnection.counterpartyConnectionIdentifier, upgradeTimeout)) + provableStore.set(connectionPath(identifier), proposedUpgrade.connection) } ``` @@ -342,9 +342,6 @@ function onChanUpgradeAck( currentConnection = provableStore.get(connectionPath(identifier)) abortTransactionUnless(currentConnection.state == UPGRADE_INIT || currentConnection.state == UPGRADE_TRY) - // verify proofs of counterparty state - abortTransactionUnless(verifyConnectionState(currentConnection, proofHeight, proofConnection, currentConnection.counterpartyConnectionIdentifier, counterpartyConnection)) - // counterparty must be in TRY state restoreConnectionUnless(counterpartyConnection.State == UPGRADE_TRY) @@ -355,6 +352,9 @@ function onChanUpgradeAck( // on either side are correctly constructed according to the new version selected. restoreConnectionUnless(IsCompatible(counterpartyConnection, connection)) + // verify proofs of counterparty state + abortTransactionUnless(verifyConnectionState(currentConnection, proofHeight, proofConnection, currentConnection.counterpartyConnectionIdentifier, counterpartyConnection)) + // upgrade is complete // set connection to OPEN and remove unnecessary state currentConnection.state = OPEN @@ -467,8 +467,7 @@ function timeoutConnectionUpgrade( // then abort transaction abortTransactionUnless(upgradeTimeout.timeoutHeight.IsZero() || proofHeight >= upgradeTimeout.timeoutHeight) // if timeoutTimestamp is defined then the consensus time from proof height must be greater than timeout timestamp - connection = getConnection(identifer) - consensusState = getConsensusState(connection.clientIdentifer, proofHeight) + consensusState = queryConsensusState(currentConnection.clientIdentifer, proofHeight) abortTransactionUnless(upgradeTimeout.timeoutTimestamp.IsZero() || consensusState.getTimestamp() >= upgradeTimeout.timestamp) // counterparty connection must be proved to still be in OPEN state From 7239327cfb232ef17ce4ede9fd6ae3ab28d7bc1a Mon Sep 17 00:00:00 2001 From: Aditya Sripal Date: Mon, 7 Mar 2022 18:32:08 +0100 Subject: [PATCH 17/27] fix mistakes after readthrough --- .../ics-003-connection-semantics/UPGRADES.md | 33 ++++++++----------- 1 file changed, 14 insertions(+), 19 deletions(-) diff --git a/spec/core/ics-003-connection-semantics/UPGRADES.md b/spec/core/ics-003-connection-semantics/UPGRADES.md index d2d9c596c..793915dbe 100644 --- a/spec/core/ics-003-connection-semantics/UPGRADES.md +++ b/spec/core/ics-003-connection-semantics/UPGRADES.md @@ -31,7 +31,6 @@ enum ConnectionState { OPEN, UPGRADE_INIT, UPGRADE_TRY, - UPGRADE_ERR, } ``` @@ -106,8 +105,8 @@ function errorPath(id: Identifier): Path { The UpgradeError MUST have an associated verification function added to the connection and client interfaces so that a counterparty may verify that chain has stored an error in the UpgradeError path. ```typescript -// Connection VerifyUpgradeError method -function verifyUpgradeError( +// Connection VerifyConnectionUpgradeError method +function verifyConnectionUpgradeError( connection: ConnectionEnd, height: Height, proof: CommitmentProof, @@ -120,7 +119,7 @@ function verifyUpgradeError( ```typescript // Client VerifyUpgradeError -function verifyUpgradeError( +function verifyConnectionUpgradeError( clientState: ClientState, height: Height, prefix: CommitmentPrefix, @@ -147,8 +146,8 @@ function timeoutPath(id: Identifier) Path { The timeout path MUST have associated verification methods on the connection and client interfaces in order for a counterparty to prove that a chain stored a particular `UpgradeTimeout`. ```typescript -// Connection VerifyUpgradeTimeout method -function verifyUpgradeTimeout( +// Connection VerifyConnectionUpgradeTimeout method +function verifyConnectionUpgradeTimeout( connection: ConnectionEnd, height: Height, proof: CommitmentProof, @@ -161,7 +160,7 @@ function verifyUpgradeTimeout( ```typescript // Client VerifyUpgradeTimeout -function verifyUpgradeTimeout( +function verifyConnectionUpgradeTimeout( clientState: ClientState, height: Height, prefix: CommitmentPrefix, @@ -178,7 +177,7 @@ function verifyUpgradeTimeout( ## Sub-Protocols -The Connection Upgrade process consists of three sub-protocols: `UpgradeHandshake`, `CancelConnectionUpgrade`, and `TimeoutConnectionUpgrade`. In the case where both chains approve of the proposed upgrade, the upgrade handshake protocol should complete successfully and the ConnectionEnd should upgrade successf +The Connection Upgrade process consists of three sub-protocols: `UpgradeConnectionHandshake`, `CancelConnectionUpgrade`, and `TimeoutConnectionUpgrade`. In the case where both chains approve of the proposed upgrade, the upgrade handshake protocol should complete successfully and the ConnectionEnd should upgrade successfully. ### Upgrade Handshake @@ -247,8 +246,7 @@ Access control on counterparty should inform choice of timeout values, i.e. time ```typescript function connUpgradeTry( identifier: Identifier, - proposedUpgrade: UpgradeConnectionState, - counterpartyConnection: ConnectionEnd, + proposedUpgradeConnection: ConnectionEnd, timeoutHeight: Height, timeoutTimestamp: uint64, UpgradeTimeout: UpgradeTimeout, @@ -277,10 +275,10 @@ function connUpgradeTry( // upgraded connection state must be in `UPGRADE_TRY` // NOTE: Any added fields are by default modifiable. abortTransactionUnless( - proposedUpgrade.connection.state == UPGRADE_TRY && - proposedUpgrade.connection.counterpartyConnectionIdentifier == currentConnection.counterpartyConnectionIdentifier && - proposedUpgrade.connection.clientIdentifier == currentConnection.clientIdentifier && - proposedUpgrade.connection.counterpartyClientIdentifier == currentConnection.counterpartyClientIdentifier + proposedUpgradeConnection.state == UPGRADE_TRY && + proposedUpgradeConnection.counterpartyConnectionIdentifier == currentConnection.counterpartyConnectionIdentifier && + proposedUpgradeConnection.clientIdentifier == currentConnection.clientIdentifier && + proposedUpgradeConnection.counterpartyClientIdentifier == currentConnection.counterpartyClientIdentifier ) @@ -319,7 +317,7 @@ function connUpgradeTry( restoreConnectionUnless(IsCompatible(counterpartyConnection, proposedUpgrade.Connection)) // verify proofs of counterparty state - abortTransactionUnless(verifyConnectionState(currentConnection, proofHeight, proofConnection, currentConnection.counterpartyConnectionIdentifier, counterpartyConnection)) + abortTransactionUnless(verifyConnectionState(currentConnection, proofHeight, proofConnection, currentConnection.counterpartyConnectionIdentifier, proposedUpgradeConnection)) abortTransactionUnless(verifyUpgradeTimeout(currentConnection, proofHeight, proofUpgradeTimeout, currentConnection.counterpartyConnectionIdentifier, upgradeTimeout)) provableStore.set(connectionPath(identifier), proposedUpgrade.connection) @@ -333,9 +331,7 @@ NOTE: It is up to individual implementations how they will provide access-contro function onChanUpgradeAck( identifier: Identifier, counterpartyConnection: ConnectionEnd, - counterpartyStatus: UpgradeError, proofConnection: CommitmentProof, - proofUpgradeError: CommitmentProof, proofHeight: Height ) { // current connection is in UPGRADE_INIT or UPGRADE_TRY (crossing hellos) @@ -417,7 +413,6 @@ During the upgrade handshake a chain may cancel the upgrade by writing an error function cancelConnectionUpgrade( identifier: Identifer, errorReceipt: []byte, - counterpartyUpgradeError: UpgradeError, proofUpgradeError: CommitmentProof, proofHeight: Height, ) { @@ -427,7 +422,7 @@ function cancelConnectionUpgrade( abortTransactionUnless(!isEmpty(errorReceipt)) - abortTransactionUnless(verifyUpgradeError(currentConnection, proofHeight, proofUpgradeError, currentConnection.counterpartyConnectionIdentifier, counterpartyUpgradeError)) + abortTransactionUnless(verifyUpgradeError(currentConnection, proofHeight, proofUpgradeError, currentConnection.counterpartyConnectionIdentifier, errorReceipt)) // cancel upgrade // and restore original conneciton From 6f7f5721232b89542aa70a7bc14557df393195a8 Mon Sep 17 00:00:00 2001 From: Aditya Sripal Date: Mon, 7 Mar 2022 18:35:42 +0100 Subject: [PATCH 18/27] chan->conn --- spec/core/ics-003-connection-semantics/UPGRADES.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/spec/core/ics-003-connection-semantics/UPGRADES.md b/spec/core/ics-003-connection-semantics/UPGRADES.md index 793915dbe..c4bf7169e 100644 --- a/spec/core/ics-003-connection-semantics/UPGRADES.md +++ b/spec/core/ics-003-connection-semantics/UPGRADES.md @@ -328,7 +328,7 @@ NOTE: It is up to individual implementations how they will provide access-contro ```typescript -function onChanUpgradeAck( +function connUpgradeAck( identifier: Identifier, counterpartyConnection: ConnectionEnd, proofConnection: CommitmentProof, @@ -361,7 +361,7 @@ function onChanUpgradeAck( ``` ```typescript -function onChanUpgradeConfirm( +function connUpgradeConfirm( identifier: Identifier, counterpartyConnection: ConnectionEnd, proofConnection: CommitmentProof, From 758e5ab101e4e9c06ad178963d9cf4e4e8176b5f Mon Sep 17 00:00:00 2001 From: Aditya Date: Wed, 9 Mar 2022 15:45:55 +0100 Subject: [PATCH 19/27] Apply suggestions from code review Co-authored-by: Marius Poke --- spec/core/ics-003-connection-semantics/UPGRADES.md | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/spec/core/ics-003-connection-semantics/UPGRADES.md b/spec/core/ics-003-connection-semantics/UPGRADES.md index c4bf7169e..54532adcd 100644 --- a/spec/core/ics-003-connection-semantics/UPGRADES.md +++ b/spec/core/ics-003-connection-semantics/UPGRADES.md @@ -235,7 +235,7 @@ function connUpgradeInit( } provableStore.set(timeoutPath(identifier), upgradeTimeout) - provableStore.set(connectionPath(identifier), proposedUpgrade.connection) + provableStore.set(connectionPath(identifier), proposedUpgradeConnection) privateStore.set(restorePath(identifier), currentConnection) } ``` @@ -302,7 +302,7 @@ function connUpgradeTry( // counterparty-specified timeout must not have exceeded restoreConnectionUnless( - timeoutHeight < currentHeight() && + timeoutHeight < getCurrentHeight() && timeoutTimestamp < currentTimestamp() ) @@ -318,7 +318,7 @@ function connUpgradeTry( // verify proofs of counterparty state abortTransactionUnless(verifyConnectionState(currentConnection, proofHeight, proofConnection, currentConnection.counterpartyConnectionIdentifier, proposedUpgradeConnection)) - abortTransactionUnless(verifyUpgradeTimeout(currentConnection, proofHeight, proofUpgradeTimeout, currentConnection.counterpartyConnectionIdentifier, upgradeTimeout)) + abortTransactionUnless(verifyConnectionUpgradeTimeout(currentConnection, proofHeight, proofUpgradeTimeout, upgradeTimeout)) provableStore.set(connectionPath(identifier), proposedUpgrade.connection) } @@ -369,7 +369,7 @@ function connUpgradeConfirm( ) { // current connection is in UPGRADE_TRY currentConnection = provableStore.get(connectionPath(identifier)) - abortTransactionUnless(connection.state == UPGRADE_TRY) + abortTransactionUnless(currentConnection.state == UPGRADE_TRY) // counterparty must be in OPEN state abortTransactionUnless(counterpartyConnection.State == OPEN) @@ -422,7 +422,7 @@ function cancelConnectionUpgrade( abortTransactionUnless(!isEmpty(errorReceipt)) - abortTransactionUnless(verifyUpgradeError(currentConnection, proofHeight, proofUpgradeError, currentConnection.counterpartyConnectionIdentifier, errorReceipt)) + abortTransactionUnless(verifyConnectionUpgradeError(currentConnection, proofHeight, proofUpgradeError, errorReceipt)) // cancel upgrade // and restore original conneciton From 67f2a03b9da13f7d7f2739c8d889f066854d3b03 Mon Sep 17 00:00:00 2001 From: Aditya Sripal Date: Wed, 9 Mar 2022 16:02:07 +0100 Subject: [PATCH 20/27] complete fixing minor issues --- .../ics-003-connection-semantics/UPGRADES.md | 20 +++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/spec/core/ics-003-connection-semantics/UPGRADES.md b/spec/core/ics-003-connection-semantics/UPGRADES.md index c4bf7169e..2275cd121 100644 --- a/spec/core/ics-003-connection-semantics/UPGRADES.md +++ b/spec/core/ics-003-connection-semantics/UPGRADES.md @@ -214,7 +214,7 @@ function connUpgradeInit( ) { // current connection must be OPEN currentConnection = provableStore.get(connectionPath(identifier)) - abortTransactionUnless(connection.state == OPEN) + abortTransactionUnless(currentConnection.state == OPEN) // abort transaction if an unmodifiable field is modified // upgraded connection state must be in `UPGRADE_INIT` @@ -235,7 +235,7 @@ function connUpgradeInit( } provableStore.set(timeoutPath(identifier), upgradeTimeout) - provableStore.set(connectionPath(identifier), proposedUpgrade.connection) + provableStore.set(connectionPath(identifier), proposedUpgradeConnection) privateStore.set(restorePath(identifier), currentConnection) } ``` @@ -247,9 +247,9 @@ Access control on counterparty should inform choice of timeout values, i.e. time function connUpgradeTry( identifier: Identifier, proposedUpgradeConnection: ConnectionEnd, + counterpartyConnection: ConnectionEnd, timeoutHeight: Height, timeoutTimestamp: uint64, - UpgradeTimeout: UpgradeTimeout, proofConnection: CommitmentProof, proofUpgradeTimeout: CommitmentProof, proofHeight: Height @@ -264,7 +264,7 @@ function connUpgradeTry( // except for the connection state (upgrade connection will be in UPGRADE_TRY and current connection will be in UPGRADE_INIT) // if the proposed upgrades on either side are incompatible, then we will restore the connection and cancel the upgrade. currentConnection.state = UPGRADE_TRY - restoreConnectionUnless(currentConnection.IsEqual(proposedUpgrade.connection)) + restoreConnectionUnless(currentConnection.IsEqual(proposedUpgradeConnection)) } else { // this is first message in upgrade handshake on this chain so we must store original connection in restore path // in case we need to restore connection later. @@ -307,20 +307,20 @@ function connUpgradeTry( ) // verify chosen versions are compatible - versionsIntersection = intersection(counterpartyConnection.version, proposedUpgrade.Connection.version) + versionsIntersection = intersection(counterpartyConnection.version, proposedUpgradeConnection.version) version = pickVersion(versionsIntersection) // aborts transaction if there is no intersection // both connection ends must be mutually compatible. // this function has been left unspecified since it will depend on the specific structure of the new connection. // It is the responsibility of implementations to make sure that verification that the proposed new connections // on either side are correctly constructed according to the new version selected. - restoreConnectionUnless(IsCompatible(counterpartyConnection, proposedUpgrade.Connection)) + restoreConnectionUnless(IsCompatible(counterpartyConnection, proposedUpgradeConnection)) // verify proofs of counterparty state abortTransactionUnless(verifyConnectionState(currentConnection, proofHeight, proofConnection, currentConnection.counterpartyConnectionIdentifier, proposedUpgradeConnection)) abortTransactionUnless(verifyUpgradeTimeout(currentConnection, proofHeight, proofUpgradeTimeout, currentConnection.counterpartyConnectionIdentifier, upgradeTimeout)) - provableStore.set(connectionPath(identifier), proposedUpgrade.connection) + provableStore.set(connectionPath(identifier), proposedUpgradeConnection) } ``` @@ -369,7 +369,7 @@ function connUpgradeConfirm( ) { // current connection is in UPGRADE_TRY currentConnection = provableStore.get(connectionPath(identifier)) - abortTransactionUnless(connection.state == UPGRADE_TRY) + abortTransactionUnless(currentConnection.state == UPGRADE_TRY) // counterparty must be in OPEN state abortTransactionUnless(counterpartyConnection.State == OPEN) @@ -418,7 +418,7 @@ function cancelConnectionUpgrade( ) { // current connection is in UPGRADE_INIT or UPGRADE_TRY currentConnection = provableStore.get(connectionPath(identifier)) - abortTransactionUnless(connection.state == UPGRADE_INIT || connection.state == UPGRADE_TRY) + abortTransactionUnless(currentConnection.state == UPGRADE_INIT || currentConnection.state == UPGRADE_TRY) abortTransactionUnless(!isEmpty(errorReceipt)) @@ -467,7 +467,7 @@ function timeoutConnectionUpgrade( // counterparty connection must be proved to still be in OPEN state abortTransactionUnless(counterpartyConnection.State === OPEN) - abortTransactionUnless(connection.client.verifyConnectionState(proofHeight, proofConnection, counterpartyConnection)) + abortTransactionUnless(currentConnection.verifyConnectionState(proofHeight, proofConnection, currentConnection.counterpartyConnectionIdentifier, counterpartyConnection)) // we must restore the connection since the timeout verification has passed restoreConnectionUnless(false) From a1526276963dfa44e021973c68b746b7dd375240 Mon Sep 17 00:00:00 2001 From: Aditya Sripal Date: Wed, 9 Mar 2022 16:08:21 +0100 Subject: [PATCH 21/27] reorder utility function and fix timeout conditional bug --- .../ics-003-connection-semantics/UPGRADES.md | 46 ++++++++++--------- 1 file changed, 25 insertions(+), 21 deletions(-) diff --git a/spec/core/ics-003-connection-semantics/UPGRADES.md b/spec/core/ics-003-connection-semantics/UPGRADES.md index 6e67ce63a..cd8ad47f7 100644 --- a/spec/core/ics-003-connection-semantics/UPGRADES.md +++ b/spec/core/ics-003-connection-semantics/UPGRADES.md @@ -175,6 +175,29 @@ function verifyConnectionUpgradeTimeout( } ``` +## Utility Functions + +`restoreConnectionUnless(condition: bool)` is a utility function that allows a chain to abort an upgrade handshake in progress, and return the connection to its original pre-upgrade state if the condition passed in is `true`. + +```typescript +function restoreConnectionUnless(condition: bool) { + if !condition { + // cancel upgrade + // write an error receipt into the error path + // and restore original connection + errorReceipt = []byte{1} + provableStore.set(errorPath(identifier), errorReceipt) + originalConnection = privateStore.get(restorePath(identifier)) + provableStore.set(connectionPath(identifier), originalConnection) + provableStore.delete(timeoutPath(identifier)) + privateStore.delete(restorePath(identifier)) + // caller should return as well + } else { + // caller should continue execution + } +} +``` + ## Sub-Protocols The Connection Upgrade process consists of three sub-protocols: `UpgradeConnectionHandshake`, `CancelConnectionUpgrade`, and `TimeoutConnectionUpgrade`. In the case where both chains approve of the proposed upgrade, the upgrade handshake protocol should complete successfully and the ConnectionEnd should upgrade successfully. @@ -302,8 +325,8 @@ function connUpgradeTry( // counterparty-specified timeout must not have exceeded restoreConnectionUnless( - timeoutHeight < getCurrentHeight() && - timeoutTimestamp < currentTimestamp() + timeoutHeight > getCurrentHeight() || + timeoutTimestamp > currentTimestamp() ) // verify chosen versions are compatible @@ -386,25 +409,6 @@ function connUpgradeConfirm( } ``` -```typescript -function restoreConnectionUnless(condition: bool) { - if !condition { - // cancel upgrade - // write an error receipt into the error path - // and restore original connection - errorReceipt = []byte{1} - provableStore.set(errorPath(identifier), errorReceipt) - originalConnection = privateStore.get(restorePath(identifier)) - provableStore.set(connectionPath(identifier), originalConnection) - provableStore.delete(timeoutPath(identifier)) - privateStore.delete(restorePath(identifier)) - // caller should return as well - } else { - // caller should continue execution - } -} -``` - ### Cancel Upgrade Process During the upgrade handshake a chain may cancel the upgrade by writing an error receipt into the error path and restoring the original connection to `OPEN`. The counterparty must then restore its connection to `OPEN` as well. A relayer can facilitate this by calling `CancelConnectionUpgrade`: From 6a59321430b8dfb0b990ae94d1393ec771194fad Mon Sep 17 00:00:00 2001 From: Aditya Sripal Date: Mon, 14 Mar 2022 19:01:22 +0100 Subject: [PATCH 22/27] fix restore connection logic --- .../ics-003-connection-semantics/UPGRADES.md | 127 ++++++++++-------- 1 file changed, 73 insertions(+), 54 deletions(-) diff --git a/spec/core/ics-003-connection-semantics/UPGRADES.md b/spec/core/ics-003-connection-semantics/UPGRADES.md index cd8ad47f7..c89de738d 100644 --- a/spec/core/ics-003-connection-semantics/UPGRADES.md +++ b/spec/core/ics-003-connection-semantics/UPGRADES.md @@ -177,24 +177,20 @@ function verifyConnectionUpgradeTimeout( ## Utility Functions -`restoreConnectionUnless(condition: bool)` is a utility function that allows a chain to abort an upgrade handshake in progress, and return the connection to its original pre-upgrade state if the condition passed in is `true`. +`restoreConnection()` is a utility function that allows a chain to abort an upgrade handshake in progress, and return the `connectionEnd` to its original pre-upgrade state while also setting the `errorReceipt`. A relayer can then send a `cancelUpgradeMsg` to the counterparty so that it can restore its `connectionEnd` to its pre-upgrade state as well. Once both connection ends are back to the pre-upgrade state, the connection will resume processing with its original connection parameters ```typescript -function restoreConnectionUnless(condition: bool) { - if !condition { - // cancel upgrade - // write an error receipt into the error path - // and restore original connection - errorReceipt = []byte{1} - provableStore.set(errorPath(identifier), errorReceipt) - originalConnection = privateStore.get(restorePath(identifier)) - provableStore.set(connectionPath(identifier), originalConnection) - provableStore.delete(timeoutPath(identifier)) - privateStore.delete(restorePath(identifier)) - // caller should return as well - } else { - // caller should continue execution - } +function restoreConnection() { + // cancel upgrade + // write an error receipt into the error path + // and restore original connection + errorReceipt = []byte{1} + provableStore.set(errorPath(identifier), errorReceipt) + originalConnection = privateStore.get(restorePath(identifier)) + provableStore.set(connectionPath(identifier), originalConnection) + provableStore.delete(timeoutPath(identifier)) + privateStore.delete(restorePath(identifier)) + // caller should return as well } ``` @@ -281,19 +277,6 @@ function connUpgradeTry( currentConnection = provableStore.get(connectionPath(identifier)) abortTransactionUnless(currentConnection.state == OPEN || currentConnection.state == UPGRADE_INIT) - if currentConnection.state == UPGRADE_INIT { - // if there is a crossing hello, ie an UpgradeInit has been called on both connectionEnds, - // then we must ensure that the proposedUpgrade by the counterparty is the same as the currentConnection - // except for the connection state (upgrade connection will be in UPGRADE_TRY and current connection will be in UPGRADE_INIT) - // if the proposed upgrades on either side are incompatible, then we will restore the connection and cancel the upgrade. - currentConnection.state = UPGRADE_TRY - restoreConnectionUnless(currentConnection.IsEqual(proposedUpgradeConnection)) - } else { - // this is first message in upgrade handshake on this chain so we must store original connection in restore path - // in case we need to restore connection later. - privateStore.set(restorePath(identifier), currentConnection) - } - // abort transaction if an unmodifiable field is modified // upgraded connection state must be in `UPGRADE_TRY` // NOTE: Any added fields are by default modifiable. @@ -304,30 +287,56 @@ function connUpgradeTry( proposedUpgradeConnection.counterpartyClientIdentifier == currentConnection.counterpartyClientIdentifier ) - - // either timeout height or timestamp must be non-zero - // if the upgrade feature is implemented on the TRY chain, then a relayer may submit a TRY transaction after the timeout. - // this will restore the connection on the executing chain and allow counterparty to use the CancelUpgradeMsg to restore their connection. - restoreConnectionUnless(timeoutHeight != 0 || timeoutTimestamp != 0) - upgradeTimeout = UpgradeTimeout{ - timeoutHeight: timeoutHeight, - timeoutTimestamp: timeoutTimestamp, - } + // verify proofs of counterparty state + abortTransactionUnless(verifyConnectionState(currentConnection, proofHeight, proofConnection, currentConnection.counterpartyConnectionIdentifier, proposedUpgradeConnection)) + abortTransactionUnless(verifyConnectionUpgradeTimeout(currentConnection, proofHeight, proofUpgradeTimeout, upgradeTimeout)) // verify that counterparty connection unmodifiable fields have not changed and counterparty state // is UPGRADE_INIT - restoreConnectionUnless( + abortTransactionUnless( counterpartyConnection.state == UPGRADE_INIT && counterpartyConnection.counterpartyConnectionIdentifier == identifier && counterpartyConnection.clientIdentifier == currentConnection.counterpartyClientIdentifier && counterpartyConnection.counterpartyClientIdentifier == currentConnection.clientIdentifier ) + if currentConnection.state == UPGRADE_INIT { + // if there is a crossing hello, ie an UpgradeInit has been called on both connectionEnds, + // then we must ensure that the proposedUpgrade by the counterparty is the same as the currentConnection + // except for the connection state (upgrade connection will be in UPGRADE_TRY and current connection will be in UPGRADE_INIT) + // if the proposed upgrades on either side are incompatible, then we will restore the connection and cancel the upgrade. + currentConnection.state = UPGRADE_TRY + if !currentConnection.IsEqual(proposedUpgradeConnection) { + restoreConnection() + return + } + } else if currentConnection.state == OPEN { + // this is first message in upgrade handshake on this chain so we must store original connection in restore path + // in case we need to restore connection later. + privateStore.set(restorePath(identifier), currentConnection) + } else { + // abort transaction if current connection is not in INIT or OPEN + abortTransactionUnless(false) + } + + // either timeout height or timestamp must be non-zero + // if the upgrade feature is implemented on the TRY chain, then a relayer may submit a TRY transaction after the timeout. + // this will restore the connection on the executing chain and allow counterparty to use the CancelUpgradeMsg to restore their connection. + if timeoutHeight == 0 && timeoutTimestamp == 0 { + restoreConnection() + return + } + upgradeTimeout = UpgradeTimeout{ + timeoutHeight: timeoutHeight, + timeoutTimestamp: timeoutTimestamp, + } + // counterparty-specified timeout must not have exceeded - restoreConnectionUnless( - timeoutHeight > getCurrentHeight() || - timeoutTimestamp > currentTimestamp() - ) + if (currentHeight() > timeoutHeight && timeoutHeight != 0) || + (currentTimestamp() > timeoutTimestamp && timeoutTimestamp != 0) { + restoreConnection() + return + } // verify chosen versions are compatible versionsIntersection = intersection(counterpartyConnection.version, proposedUpgradeConnection.version) @@ -337,12 +346,11 @@ function connUpgradeTry( // this function has been left unspecified since it will depend on the specific structure of the new connection. // It is the responsibility of implementations to make sure that verification that the proposed new connections // on either side are correctly constructed according to the new version selected. - restoreConnectionUnless(IsCompatible(counterpartyConnection, proposedUpgradeConnection)) + if !IsCompatible(counterpartyConnection, proposedUpgradeConnection) { + restoreConnection() + return + } - // verify proofs of counterparty state - abortTransactionUnless(verifyConnectionState(currentConnection, proofHeight, proofConnection, currentConnection.counterpartyConnectionIdentifier, proposedUpgradeConnection)) - abortTransactionUnless(verifyConnectionUpgradeTimeout(currentConnection, proofHeight, proofUpgradeTimeout, upgradeTimeout)) - provableStore.set(connectionPath(identifier), proposedUpgradeConnection) } ``` @@ -362,17 +370,23 @@ function connUpgradeAck( abortTransactionUnless(currentConnection.state == UPGRADE_INIT || currentConnection.state == UPGRADE_TRY) // counterparty must be in TRY state - restoreConnectionUnless(counterpartyConnection.State == UPGRADE_TRY) + if counterpartyConnection.State != UPGRADE_TRY { + restoreConnection() + return + } + + // verify proofs of counterparty state + abortTransactionUnless(verifyConnectionState(currentConnection, proofHeight, proofConnection, currentConnection.counterpartyConnectionIdentifier, counterpartyConnection)) // verify connections are mutually compatible // this will also check counterparty chosen version is valid // this function has been left unspecified since it will depend on the specific structure of the new connection. // It is the responsibility of implementations to make sure that verification that the proposed new connections // on either side are correctly constructed according to the new version selected. - restoreConnectionUnless(IsCompatible(counterpartyConnection, connection)) - - // verify proofs of counterparty state - abortTransactionUnless(verifyConnectionState(currentConnection, proofHeight, proofConnection, currentConnection.counterpartyConnectionIdentifier, counterpartyConnection)) + if !IsCompatible(counterpartyConnection, connection) { + restoreConnection() + return + } // upgrade is complete // set connection to OPEN and remove unnecessary state @@ -474,7 +488,12 @@ function timeoutConnectionUpgrade( abortTransactionUnless(currentConnection.verifyConnectionState(proofHeight, proofConnection, currentConnection.counterpartyConnectionIdentifier, counterpartyConnection)) // we must restore the connection since the timeout verification has passed - restoreConnectionUnless(false) + originalConnection = privateStore.get(restorePath(identifier)) + provableStore.set(connectionPath(identifier), originalConnection) + + // delete auxilliary upgrade state + provableStore.delete(timeoutPath(identifier)) + privateStore.delete(restorePath(identifier)) } ``` From a4c9fdc65e4e125b058f4100cffe2d5ffbded595 Mon Sep 17 00:00:00 2001 From: Aditya Sripal Date: Tue, 15 Mar 2022 17:22:41 +0100 Subject: [PATCH 23/27] use 02-client generic verify methods --- .../ics-003-connection-semantics/UPGRADES.md | 51 +++++-------------- 1 file changed, 13 insertions(+), 38 deletions(-) diff --git a/spec/core/ics-003-connection-semantics/UPGRADES.md b/spec/core/ics-003-connection-semantics/UPGRADES.md index c89de738d..b6300fba9 100644 --- a/spec/core/ics-003-connection-semantics/UPGRADES.md +++ b/spec/core/ics-003-connection-semantics/UPGRADES.md @@ -86,7 +86,7 @@ At least one of the timeoutHeight or timeoutTimestamp MUST be non-zero. The chain must store the previous connection end so that it may restore it if the upgrade handshake fails. This may be stored in the private store. ```typescript -function restorePath(id: Identifier): Path { +function connectionRestorePath(id: Identifier): Path { return "connections/{id}/restore" } ``` @@ -96,7 +96,7 @@ function restorePath(id: Identifier): Path { The upgrade error path is a public path that can signal an error of the upgrade to the counterparty. It does not store anything in the successful case, but it will store a sentinel abort value in the case that a chain does not accept the proposed upgrade. ```typescript -function errorPath(id: Identifier): Path { +function connectionErrorPath(id: Identifier): Path { return "connections/{id}/upgradeError" } @@ -105,7 +105,7 @@ function errorPath(id: Identifier): Path { The UpgradeError MUST have an associated verification function added to the connection and client interfaces so that a counterparty may verify that chain has stored an error in the UpgradeError path. ```typescript -// Connection VerifyConnectionUpgradeError method +// ConnectionEnd VerifyConnectionUpgradeError method function verifyConnectionUpgradeError( connection: ConnectionEnd, height: Height, @@ -113,23 +113,11 @@ function verifyConnectionUpgradeError( upgradeErrorReceipt: []byte, ) { client = queryClient(connection.clientIdentifier) - client.verifyUpgradeError(height, connection.counterpartyPrefix, proof, connection.counterpartyConnectionIdentifier, upgradeErrorReceipt) -} -``` - -```typescript -// Client VerifyUpgradeError -function verifyConnectionUpgradeError( - clientState: ClientState, - height: Height, - prefix: CommitmentPrefix, - proof: CommitmentProof, - counterpartyConnectionIdentifier: Identifier, - upgradeErrorReceipt []byte, -) { - path = applyPrefix(prefix, errorPath(counterpartyConnectionIdentifier)) - abortTransactionUnless(!clientState.frozen) - return clientState.verifiedRoots[height].verifyMembership(path, upgradeErrorReceipt, proof) + // construct CommitmentPath + path = applyPrefix(connection.counterpartyPrefix, connectionErrorPath(connection.counterpartyConnectionIdentifier)) + // verify upgradeErrorReceipt is stored under the constructed path + // delay period is unnecessary for non-packet verification so pass in 0 for delay period fields + return client.verifyMembership(height, 0, 0, proof, path, upgradeErrorReceipt) } ``` @@ -146,7 +134,7 @@ function timeoutPath(id: Identifier) Path { The timeout path MUST have associated verification methods on the connection and client interfaces in order for a counterparty to prove that a chain stored a particular `UpgradeTimeout`. ```typescript -// Connection VerifyConnectionUpgradeTimeout method +// ConnectionEnd VerifyConnectionUpgradeTimeout method function verifyConnectionUpgradeTimeout( connection: ConnectionEnd, height: Height, @@ -154,24 +142,11 @@ function verifyConnectionUpgradeTimeout( upgradeTimeout: UpgradeTimeout, ) { client = queryClient(connection.clientIdentifier) - client.verifyUpgradeTimeout(height, connection.counterpartyPrefix, proof, connection.counterpartyConnectionIdentifier, upgradeTimeout) -} -``` - -```typescript -// Client VerifyUpgradeTimeout -function verifyConnectionUpgradeTimeout( - clientState: ClientState, - height: Height, - prefix: CommitmentPrefix, - proof: CommitmentProof, - counterpartyConnectionIdentifier: Identifier, - upgradeTimeout: UpgradeTimeout, -) { - path = applyPrefix(prefix, timeoutPath(counterpartyConnectionIdentifier)) - abortTransactionUnless(!clientState.frozen) + // construct CommitmentPath + path = applyPrefix(connection.counterpartyPrefix, connectionTimeoutPath(connection.counterpartyConnectionIdentifier)) + // marshal upgradeTimeout into bytes with standardized protobuf codec timeoutBytes = protobuf.marshal(upgradeTimeout) - return clientState.verifiedRoots[height].verifyMembership(path, timeoutBytes, proof) + client.verifyMembership(height, proof, path, timeoutBytes) } ``` From 7359894103d0beee26e6e9fa1f7fa23ee417f125 Mon Sep 17 00:00:00 2001 From: Aditya Sripal Date: Tue, 15 Mar 2022 18:02:40 +0100 Subject: [PATCH 24/27] address colin comments --- .../ics-003-connection-semantics/UPGRADES.md | 22 +++++++++++++++---- 1 file changed, 18 insertions(+), 4 deletions(-) diff --git a/spec/core/ics-003-connection-semantics/UPGRADES.md b/spec/core/ics-003-connection-semantics/UPGRADES.md index b6300fba9..6b2e93e03 100644 --- a/spec/core/ics-003-connection-semantics/UPGRADES.md +++ b/spec/core/ics-003-connection-semantics/UPGRADES.md @@ -22,7 +22,9 @@ The connection upgrade protocol MUST NOT modify the connection identifiers. ### Data Structures -The `ConnectionState` and `ConnectionEnd` are defined in [ICS-3](./README.md), they are reproduced here for the reader's convenience. `UPGRADE_INIT`, `UPGRADE_TRY`, `UPGRADE_ERR` are additional states added to enable the upgrade feature. +The `ConnectionState` and `ConnectionEnd` are defined in [ICS-3](./README.md), they are reproduced here for the reader's convenience. `UPGRADE_INIT`, `UPGRADE_TRY` are additional states added to enable the upgrade feature. + +#### **ConnectionState (reproduced from [ICS-3](README.md)** ```typescript enum ConnectionState { @@ -37,6 +39,8 @@ enum ConnectionState { - The chain that is proposing the upgrade should set the connection state from `OPEN` to `UPGRADE_INIT` - The counterparty chain that accepts the upgrade should set the connection state from `OPEN` to `UPGRADE_TRY` +#### **ConnectionEnd (reproduced from [ICS-3](README.md)** + ```typescript interface ConnectionEnd { state: ConnectionState @@ -67,6 +71,10 @@ MUST NOT BE MODIFIED: NOTE: If the upgrade adds any fields to the `ConnectionEnd` these are by default modifiable, and can be arbitrarily chosen by an Actor (e.g. chain governance) which has permission to initiate the upgrade. +Modifiable fields may also be removed completely. + +#### **UpgradeTimeout** + ```typescript interface UpgradeTimeout { timeoutHeight: Height @@ -398,9 +406,15 @@ function connUpgradeConfirm( } ``` +NOTE: Since the counterparty has already successfully upgraded and moved to `OPEN` in `ACK` step, we cannot restore the connection here. We simply verify that the counterparty has successfully upgraded and then upgrade ourselves. + ### Cancel Upgrade Process -During the upgrade handshake a chain may cancel the upgrade by writing an error receipt into the error path and restoring the original connection to `OPEN`. The counterparty must then restore its connection to `OPEN` as well. A relayer can facilitate this by calling `CancelConnectionUpgrade`: +During the upgrade handshake a chain may cancel the upgrade by writing an error receipt into the error path and restoring the original connection to `OPEN`. The counterparty must then restore its connection to `OPEN` as well. + +A connectionEnd may only cancel the upgrade during the upgrade negotiation process (TRY, ACK). An upgrade cannot be cancelled on one end once the other chain has already completed its upgrade and moved to `OPEN` since that will lead the connection to being in an invalid state. + +A relayer can facilitate this by calling `CancelConnectionUpgrade`: ```typescript function cancelConnectionUpgrade( @@ -458,8 +472,8 @@ function timeoutConnectionUpgrade( consensusState = queryConsensusState(currentConnection.clientIdentifer, proofHeight) abortTransactionUnless(upgradeTimeout.timeoutTimestamp.IsZero() || consensusState.getTimestamp() >= upgradeTimeout.timestamp) - // counterparty connection must be proved to still be in OPEN state - abortTransactionUnless(counterpartyConnection.State === OPEN) + // counterparty connection must be proved to still be in OPEN state or INIT state (crossing hellos) + abortTransactionUnless(counterpartyConnection.State === OPEN || counterpartyConnection.State == INIT) abortTransactionUnless(currentConnection.verifyConnectionState(proofHeight, proofConnection, currentConnection.counterpartyConnectionIdentifier, counterpartyConnection)) // we must restore the connection since the timeout verification has passed From 849059ae2bb321b8b425837307600678a5d1cf01 Mon Sep 17 00:00:00 2001 From: Aditya Date: Tue, 22 Mar 2022 11:58:51 +0100 Subject: [PATCH 25/27] Apply suggestions from code review Co-authored-by: Marius Poke --- spec/core/ics-003-connection-semantics/UPGRADES.md | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/spec/core/ics-003-connection-semantics/UPGRADES.md b/spec/core/ics-003-connection-semantics/UPGRADES.md index 6b2e93e03..e4750b6e1 100644 --- a/spec/core/ics-003-connection-semantics/UPGRADES.md +++ b/spec/core/ics-003-connection-semantics/UPGRADES.md @@ -24,7 +24,7 @@ The connection upgrade protocol MUST NOT modify the connection identifiers. The `ConnectionState` and `ConnectionEnd` are defined in [ICS-3](./README.md), they are reproduced here for the reader's convenience. `UPGRADE_INIT`, `UPGRADE_TRY` are additional states added to enable the upgrade feature. -#### **ConnectionState (reproduced from [ICS-3](README.md)** +#### **ConnectionState (reproduced from [ICS-3](README.md))** ```typescript enum ConnectionState { @@ -39,7 +39,7 @@ enum ConnectionState { - The chain that is proposing the upgrade should set the connection state from `OPEN` to `UPGRADE_INIT` - The counterparty chain that accepts the upgrade should set the connection state from `OPEN` to `UPGRADE_TRY` -#### **ConnectionEnd (reproduced from [ICS-3](README.md)** +#### **ConnectionEnd (reproduced from [ICS-3](README.md))** ```typescript interface ConnectionEnd { @@ -179,7 +179,7 @@ function restoreConnection() { ## Sub-Protocols -The Connection Upgrade process consists of three sub-protocols: `UpgradeConnectionHandshake`, `CancelConnectionUpgrade`, and `TimeoutConnectionUpgrade`. In the case where both chains approve of the proposed upgrade, the upgrade handshake protocol should complete successfully and the ConnectionEnd should upgrade successfully. +The Connection Upgrade process consists of three sub-protocols: `UpgradeConnectionHandshake`, `CancelConnectionUpgrade`, and `TimeoutConnectionUpgrade`. In the case where both chains approve of the proposed upgrade, the upgrade handshake protocol should complete successfully and the `ConnectionEnd` should upgrade successfully. ### Upgrade Handshake @@ -194,7 +194,7 @@ A successful protocol execution flows as follows (note that all calls are made t | Relayer | `ConnUpgradeAck` | A | (UPGRADE_INIT, UPGRADE_TRY) | (OPEN, UPGRADE_TRY) | | Relayer | `ConnUpgradeConfirm` | B | (OPEN, UPGRADE_TRY) | (OPEN, OPEN) | -At the end of an opening handshake between two chains implementing the sub-protocol, the following properties hold: +At the end of an upgrade handshake between two chains implementing the sub-protocol, the following properties hold: - Each chain is running their new upgraded connection end and is processing upgraded logic and state according to the upgraded parameters. - Each chain has knowledge of and has agreed to the counterparty's upgraded connection parameters. From c261dd3bddb4f45ae3fad6e831e2bd6a7c721db5 Mon Sep 17 00:00:00 2001 From: Aditya Sripal Date: Tue, 22 Mar 2022 12:43:16 +0100 Subject: [PATCH 26/27] address rest of reviews --- .../ics-003-connection-semantics/UPGRADES.md | 52 ++++++++++++++----- 1 file changed, 38 insertions(+), 14 deletions(-) diff --git a/spec/core/ics-003-connection-semantics/UPGRADES.md b/spec/core/ics-003-connection-semantics/UPGRADES.md index e4750b6e1..a0e38f512 100644 --- a/spec/core/ics-003-connection-semantics/UPGRADES.md +++ b/spec/core/ics-003-connection-semantics/UPGRADES.md @@ -110,7 +110,7 @@ function connectionErrorPath(id: Identifier): Path { } ``` -The UpgradeError MUST have an associated verification function added to the connection and client interfaces so that a counterparty may verify that chain has stored an error in the UpgradeError path. +The UpgradeError MUST have an associated verification membership and non-membership functions added to the connection interface so that a counterparty may verify that chain has stored an error in the UpgradeError path. ```typescript // ConnectionEnd VerifyConnectionUpgradeError method @@ -129,6 +129,22 @@ function verifyConnectionUpgradeError( } ``` +```typescript +// ConnectionEnd VerifyConnectionUpgradeErrorAbsence method +function verifyConnectionUpgradeErrorAbsence( + connection: ConnectionEnd, + height: Height, + proof: CommitmentProof, +) { + client = queryClient(connection.clientIdentifier) + // construct CommitmentPath + path = applyPrefix(connection.counterpartyPrefix, connectionErrorPath(connection.counterpartyConnectionIdentifier)) + // verify upgradeError path is empty + // delay period is unnecessary for non-packet verification so pass in 0 for delay period fields + return client.verifyNonMembership(height, 0, 0, proof, path) +} +``` + #### TimeoutPath The timeout path is a public path set by the upgrade initiator to determine when the TRY step should timeout. It stores the `timeoutHeight` and `timeoutTimestamp` by which point the counterparty must have progressed to the TRY step. This path will be proven on the counterparty chain in case of a successful TRY, to ensure timeout has not passed. Or in the case of a timeout, in which case counterparty proves that the timeout has passed on its chain and restores the connection. @@ -139,7 +155,7 @@ function timeoutPath(id: Identifier) Path { } ``` -The timeout path MUST have associated verification methods on the connection and client interfaces in order for a counterparty to prove that a chain stored a particular `UpgradeTimeout`. +The timeout path MUST have associated verification methods on the connection interface in order for a counterparty to prove that a chain stored a particular `UpgradeTimeout`. ```typescript // ConnectionEnd VerifyConnectionUpgradeTimeout method @@ -154,7 +170,7 @@ function verifyConnectionUpgradeTimeout( path = applyPrefix(connection.counterpartyPrefix, connectionTimeoutPath(connection.counterpartyConnectionIdentifier)) // marshal upgradeTimeout into bytes with standardized protobuf codec timeoutBytes = protobuf.marshal(upgradeTimeout) - client.verifyMembership(height, proof, path, timeoutBytes) + client.verifyMembership(height, 0, 0, proof, path, timeoutBytes) } ``` @@ -270,6 +286,13 @@ function connUpgradeTry( proposedUpgradeConnection.counterpartyClientIdentifier == currentConnection.counterpartyClientIdentifier ) + // construct upgrade timeout from timeoutHeight and timeoutTimestamp + // so that we can prove they were set by counterparty + upgradeTimeout = UpgradeTimeout{ + timeoutHeight: timeoutHeight, + timeoutTimestamp: timeoutTimestamp, + } + // verify proofs of counterparty state abortTransactionUnless(verifyConnectionState(currentConnection, proofHeight, proofConnection, currentConnection.counterpartyConnectionIdentifier, proposedUpgradeConnection)) abortTransactionUnless(verifyConnectionUpgradeTimeout(currentConnection, proofHeight, proofUpgradeTimeout, upgradeTimeout)) @@ -309,11 +332,7 @@ function connUpgradeTry( restoreConnection() return } - upgradeTimeout = UpgradeTimeout{ - timeoutHeight: timeoutHeight, - timeoutTimestamp: timeoutTimestamp, - } - + // counterparty-specified timeout must not have exceeded if (currentHeight() > timeoutHeight && timeoutHeight != 0) || (currentTimestamp() > timeoutTimestamp && timeoutTimestamp != 0) { @@ -352,15 +371,15 @@ function connUpgradeAck( currentConnection = provableStore.get(connectionPath(identifier)) abortTransactionUnless(currentConnection.state == UPGRADE_INIT || currentConnection.state == UPGRADE_TRY) + // verify proofs of counterparty state + abortTransactionUnless(verifyConnectionState(currentConnection, proofHeight, proofConnection, currentConnection.counterpartyConnectionIdentifier, counterpartyConnection)) + // counterparty must be in TRY state if counterpartyConnection.State != UPGRADE_TRY { restoreConnection() return } - // verify proofs of counterparty state - abortTransactionUnless(verifyConnectionState(currentConnection, proofHeight, proofConnection, currentConnection.counterpartyConnectionIdentifier, counterpartyConnection)) - // verify connections are mutually compatible // this will also check counterparty chosen version is valid // this function has been left unspecified since it will depend on the specific structure of the new connection. @@ -385,6 +404,7 @@ function connUpgradeConfirm( identifier: Identifier, counterpartyConnection: ConnectionEnd, proofConnection: CommitmentProof, + proofUpgradeError: CommitmentProof, proofHeight: Height, ) { // current connection is in UPGRADE_TRY @@ -396,6 +416,10 @@ function connUpgradeConfirm( // verify proofs of counterparty state abortTransactionUnless(verifyConnectionState(currentConnection, proofHeight, proofConnection, currentConnection.counterpartyConnectionIdentifier, counterpartyConnection)) + + // verify that counterparty did not restore the connection and store an upgrade error + // in the connection upgradeError path + abortTransactionUnless(verifyConnectionUpgradeErrorAbsence(currentConnection, proofHeight, proofUpgradeError)) // upgrade is complete // set connection to OPEN and remove unnecessary state @@ -472,9 +496,9 @@ function timeoutConnectionUpgrade( consensusState = queryConsensusState(currentConnection.clientIdentifer, proofHeight) abortTransactionUnless(upgradeTimeout.timeoutTimestamp.IsZero() || consensusState.getTimestamp() >= upgradeTimeout.timestamp) - // counterparty connection must be proved to still be in OPEN state or INIT state (crossing hellos) - abortTransactionUnless(counterpartyConnection.State === OPEN || counterpartyConnection.State == INIT) - abortTransactionUnless(currentConnection.verifyConnectionState(proofHeight, proofConnection, currentConnection.counterpartyConnectionIdentifier, counterpartyConnection)) + // counterparty connection must be proved to still be in OPEN state or UPGRADE_INIT state (crossing hellos) + abortTransactionUnless(counterpartyConnection.State === OPEN || counterpartyConnection.State == UPGRADE_INIT) + abortTransactionUnless(verifyConnectionState(currentConnection, proofHeight, proofConnection, currentConnection.counterpartyConnectionIdentifier, counterpartyConnection)) // we must restore the connection since the timeout verification has passed originalConnection = privateStore.get(restorePath(identifier)) From 1948a82add4ea71e07a839ce7dfaa1c2f6ff84eb Mon Sep 17 00:00:00 2001 From: Aditya Sripal Date: Tue, 22 Mar 2022 14:44:24 +0100 Subject: [PATCH 27/27] use connection method --- spec/core/ics-003-connection-semantics/UPGRADES.md | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/spec/core/ics-003-connection-semantics/UPGRADES.md b/spec/core/ics-003-connection-semantics/UPGRADES.md index a0e38f512..98336e3a2 100644 --- a/spec/core/ics-003-connection-semantics/UPGRADES.md +++ b/spec/core/ics-003-connection-semantics/UPGRADES.md @@ -493,8 +493,7 @@ function timeoutConnectionUpgrade( // then abort transaction abortTransactionUnless(upgradeTimeout.timeoutHeight.IsZero() || proofHeight >= upgradeTimeout.timeoutHeight) // if timeoutTimestamp is defined then the consensus time from proof height must be greater than timeout timestamp - consensusState = queryConsensusState(currentConnection.clientIdentifer, proofHeight) - abortTransactionUnless(upgradeTimeout.timeoutTimestamp.IsZero() || consensusState.getTimestamp() >= upgradeTimeout.timestamp) + abortTransactionUnless(upgradeTimeout.timeoutTimestamp.IsZero() || getTimestampAtHeight(currentConnection, proofHeight) >= upgradeTimeout.timestamp) // counterparty connection must be proved to still be in OPEN state or UPGRADE_INIT state (crossing hellos) abortTransactionUnless(counterpartyConnection.State === OPEN || counterpartyConnection.State == UPGRADE_INIT)