From 7ea7832597ec0afb8acc9302f65eba9797b96d83 Mon Sep 17 00:00:00 2001 From: Aditya Sripal Date: Thu, 20 Apr 2023 17:35:57 +0200 Subject: [PATCH 01/19] make changes to INIT and TRY --- .../UPGRADES.md | 403 +++++++++--------- 1 file changed, 196 insertions(+), 207 deletions(-) diff --git a/spec/core/ics-004-channel-and-packet-semantics/UPGRADES.md b/spec/core/ics-004-channel-and-packet-semantics/UPGRADES.md index dc2e5659f..90adc73a4 100644 --- a/spec/core/ics-004-channel-and-packet-semantics/UPGRADES.md +++ b/spec/core/ics-004-channel-and-packet-semantics/UPGRADES.md @@ -15,8 +15,9 @@ As new features get added to IBC, chains may wish the take advantage of new chan - The channel upgrade protocol is atomic, i.e., - either it is unsuccessful and then the channel MUST fall-back to the original channel parameters; - or it is successful and then both channel ends MUST adopt the new channel parameters and the applications must process packet data appropriately. +- Packets sent under the previously negotiated parameters must be processed under the previously negotiated parameters, packets sent under the newly negotiated parameters must be processed under the newly negotiated parameters. - The channel upgrade protocol should have the ability to change all channel-related parameters; however the channel upgrade protocol MUST NOT be able to change the underlying `ConnectionEnd`. -The channel upgrade protocol MUST NOT modify the channel identifiers. +- The channel upgrade protocol MUST NOT modify the channel identifiers. ## Technical Specification @@ -32,12 +33,12 @@ enum ChannelState { TRYOPEN, OPEN, INITUPGRADE, - TRYUPGRADE, + BLOCKUPGRADE, } ``` - The chain that is proposing the upgrade should set the channel state from `OPEN` to `INITUPGRADE` -- The counterparty chain that accepts the upgrade should set the channel state from `OPEN` to `TRYUPGRADE` +- The counterparty chain that accepts the upgrade should set the channel state from `OPEN` to `BLOCKUPGRADE` #### `ChannelEnd` @@ -49,16 +50,27 @@ interface ChannelEnd { counterpartyChannelIdentifier: Identifier connectionHops: [Identifier] version: string + upgradeSequence: uint64 } ``` The desired property that the channel upgrade protocol MUST NOT modify the underlying clients or channel identifiers, means that only some fields of `ChannelEnd` are upgradable by the upgrade protocol. -- `state`: The state is specified by the handshake steps of the upgrade protocol. +- `state`: The state is specified by the handshake steps of the upgrade protocol and will be mutated in place during the handshake. + +#### `UpgradeFields` + +```typescript +interface UpgradeFields { + version: string + ordering: ChannelOrder + connectionHops: [Identifier] +} +``` MAY BE MODIFIED: - `version`: The version MAY be modified by the upgrade protocol. The same version negotiation that happens in the initial channel handshake can be employed for the upgrade handshake. -- `ordering`: The ordering MAY be modified by the upgrade protocol. However, it MUST be the case that the previous ordering is a valid subset of the new ordering. Thus, the only supported change is from stricter ordering rules to less strict ordering. For example, switching from ORDERED to UNORDERED is supported, switching from UNORDERED to ORDERED is **unsupported**. +- `ordering`: The ordering MAY be modified by the upgrade protocol. - `connectionHops`: The connectionHops MAY be modified by the upgrade protocol. MUST NOT BE MODIFIED: @@ -81,6 +93,20 @@ interface UpgradeTimeout { At least one of the `timeoutHeight` or `timeoutTimestamp` MUST be non-zero. +#### `Upgrade` + +The upgrade type will represent a particular upgrade attempt on a channel end. + +```typescript +interface Upgrade { + fields: UpgradeFields + timeout: UpgradeTimeout + lastPacketSent: uint64 +} +``` + +The upgrade contains the proposed upgrade for the channel end on the executing chain, the timeout for the upgrade attempt, and the last packet send sequence for the channel. The `lastPacketSent` allows the counterparty to know which packets need to be flushed before the channel can reopen with the newly negotiated parameters. + #### `ErrorReceipt` ```typescript @@ -95,42 +121,42 @@ interface ErrorReceipt { ### Store Paths -#### Upgrade Sequence Path +#### Upgrade Channel Path -The upgrade sequence path is a public path that stores the current sequence of the upgrade attempt. The sequence will increment with each attempted upgrade on the given channel. The sequence will be used to ensure that different error receipts referring to different upgrade attempts do not interfere with each other. +The chain must store the proposed upgrade until the upgrade handshake successfully. This must be stored in the publice store. It may be deleted once the upgrade is successful. ```typescript -function channelUpgradeSequencePath(portIdentifier: Identifier, channelIdentifier: Identifier) Path { - return "channelUpgrades/upgradeSequence/ports/{portIdentifier}/channels/{channelIdentifier}" -} +function channelUpgradePath(portIdentifier: Identifier, channelIdentifier: Identifier): Path { + return "channelUpgrades/upgrades/ports/{portIdentifier}/channels/{channelIdentifier}" + } ``` -The upgrade sequence MUST also have a verification method so that chains can prove the upgrade sequence on the counterparty for the given channel upgrade. +The upgrade path has an associated verification membership added to the connection interface so that a counterparty may verify that chain has stored the provided counterparty upgrade. ```typescript -// Connection VerifyChannelUpgradeSequence method -function verifyChannelUpgradeSequence( - connection: ConnectionEnd, - height: Height, - proof: CommitmentProof, - counterpartyPortIdentifier: Identifier, - counterpartyChannelIdentifier: Identifier, - sequence: uint64 +// Connection VerifyChannelUpgrade method +function verifyChannelUpgrade( + connection: ConnectionEnd, + height: Height, + proof: CommitmentProof, + counterpartyPortIdentifier: Identifier, + counterpartyChannelIdentifier: Identifier, + upgrade: Upgrade ) { clientState = queryClientState(connection.clientIdentifier) - path = applyPrefix(connection.counterpartyPrefix, channelUpgradeSequencePath(counterpartyPortIdentifier, counterpartyChannelIdentifier)) - return verifyMembership(clientState, height, 0, 0, proof, path, sequence) + path = applyPrefix(connection.counterpartyPrefix, channelUpgradePath(counterpartyPortIdentifier, counterpartyChannelIdentifier)) + return verifyMembership(clientState, height, 0, 0, proof, path, upgrade) } ``` -#### Restore Channel Path +#### CounterpartyLastPacketSequence Path -The chain must store the previous channel end so that it may restore it if the upgrade handshake fails. This may be stored in the private store. +The chain must store the counterparty's last packet sequence on `blockUpgradeHandshake`. This will be stored in the `counterpartyLastPacketSequence` path on the private store/ ```typescript -function channelRestorePath(portIdentifier: Identifier, channelIdentifier: Identifier): Path { - return "channelUpgrades/restore/ports/{portIdentifier}/channels/{channelIdentifier}" - } +function channelCounterpartyLastPacketSequencePath(portIdentifier: Identifier, channelIdentifier: ChannelIdentifier): Path { + return "channelUpgrades/counterpartyLastPacketSequence/ports/{portIdentifier}/channels/{channelIdentifier}" +} ``` #### Upgrade Error Path @@ -176,68 +202,109 @@ function verifyChannelUpgradeErrorAbsence( } ``` -#### Upgrade Timeout Path +## Sub-Protocols + +The channel upgrade process consists of the following sub-protocols: `InitUpgradeHandshake`, `BlockUpgradeHandshake`, `OpenUpgradeHandshake`, `CancelChannelUpgrade`, and `TimeoutChannelUpgrade`. In the case where both chains approve of the proposed upgrade, the upgrade handshake protocol should complete successfully and the `ChannelEnd` should upgrade to the new parameters in OPEN state. + +### Utility Functions -The upgrade 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. The TRY step will prove the timeout values set by the initiating chain and ensure the 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 channel. +`initUpgradeChannelHandshake` is a subprotocol that will initialize the channel end for the upgrade handshake. It will verify the upgrade parameters and set the channel state to INITUPGRADE and block sendpackets. During this time; `receivePacket`, `acknowledgePacket` and `timeoutPacket` will still be allowed and processed according to the original channel parameters. The new proposed upgrade will be stored in the public store for counterparty verification. ```typescript -function channelUpgradeTimeoutPath(portIdentifier: Identifier, channelIdentifier: Identifier) Path { - return "channelUpgrades/upgradeTimeout/ports/{portIdentifier}/channels/{channelIdentifier}" +function initUpgradeChannelHandshake( + portIdentifier: Identifier, + channelIdentifier: Identifier, + proposedFields: UpgradeFields, + timeout: UpgradeTimeout +): uint64 { + // current channel must be OPEN + currentChannel = provableStore.get(channelPath(portIdentifier, channelIdentifier)) + abortTransactionUnless(channel.state == OPEN) + + // new channel version must be nonempty + abortTransactionUnless(proposedFields.Version != "") + + // proposedConnection must exist and be in OPEN state for + // channel upgrade to be accepted + proposedConnection = provableStore.Get(connectionPath(proposedFields.connectionHops[0]) + abortTransactionUnless(proposedConnection != null && proposedConnection.state == OPEN) + + // either timeout height or timestamp must be non-zero + abortTransactionUnless(timeout.timeoutHeight != 0 || timeout.timeoutTimestamp != 0) + + // get last packet sent on channel and set it in the upgrade struct + // last packet sent is the nextSequenceSend on the channel minus 1 + lastPacketSendSequence = publicStore.get(nextSequenceSendPath(portIdentifier, channelIdentifier)) - 1 + upgrade = Upgrade{ + fields: proposedFields, + timeout: timeout, + lastPacketSent: lastPacketSendSequence, + } + + // store upgrade in public store for counterparty proof verification + publicStore.set(channelUpgradePath(portIdentifier, channelIdentifier), upgrade) + + currentChannel.sequence = currentChannel.sequence + 1 + currentChannel.state = INITUPGRADE + publicStore.set(channelPath(portIdentifier, channelIdentifier), channel) + return currentChannel.sequence } ``` -The upgrade timeout path MUST have an associated verification membership method on the connection interface in order for a counterparty to prove that a chain stored a particular timeout in the upgrade timeout path. +`blockUpgradeHandshake` will set the counterparty last packet send and continue blocking the upgrade from continuing until all in-flight packets have been flushed. When the channel is in blocked mode, any packet receive above the counterparty last packet send will be rejected. ```typescript -// Connection VerifyChannelUpgradeTimeout method -function verifyChannelUpgradeTimeout( - connection: ConnectionEnd, - height: Height, - proof: CommitmentProof, - counterpartyPortIdentifier: Identifier, - counterpartyChannelIdentifier: Identifier, - upgradeTimeout: UpgradeTimeout, +function blockUpgradeHandshake( + portIdentifier: Identifier, + channelIdentifier: Identifier, + proposedUpgradeFields: UpgradeFields, + counterpartyChannel: ChannelEnd, + counterpartyUpgrade: Upgrade, + proofChannel: CommitmentProof, + proofUpgrade: CommitmentProof, + proofHeight: Height ) { - clientState = queryClientState(connection.clientIdentifier) - path = applyPrefix(connection.counterpartyPrefix, channelUpgradeTimeoutPath(counterpartyPortIdentifier, counterpartyChannelIdentifier)) - return verifyMembership(clientState, height, 0, 0, proof, path, upgradeTimeout) -} -``` + // current channel must be INITUPGRADE + currentChannel = provableStore.get(channelPath(portIdentifier, channelIdentifier)) + abortTransactionUnless(channel.state == INITUPGRADE) -## Sub-Protocols + // connectionHops can change in a channelUpgrade, however both sides must still be each other's counterparty. + proposedConnection = provableStore.get(connectionPath(proposedUpgradeFields.connectionHops[0]) + abortTransactionUnless(proposedConnection != null && proposedConnection.state == OPEN) + abortTransactionUnless(counterpartyUpgrade.fields.connectionHops[0] == proposedConnection.counterpartyConnectionIdentifier) -The channel upgrade process consists of three sub-protocols: `UpgradeChannelHandshake`, `CancelChannelUpgrade`, and `TimeoutChannelUpgrade`. In the case where both chains approve of the proposed upgrade, the upgrade handshake protocol should complete successfully and the `ChannelEnd` should upgrade successfully. + // get underlying connection for proof verification + connection = getConnection(currentChannel.connectionIdentifier) + counterpartyHops = getCounterpartyHops(connection) -### Utility Functions + // verify proofs of counterparty state + abortTransactionUnless(verifyChannelState(connection, proofHeight, proofChannel, currentChannel.counterpartyPortIdentifier, currentChannel.counterpartyChannelIdentifier, counterpartyChannel)) + abortTransactionUnless(verifyChannelUpgrade(connection, proofHeight, proofUpgradeTimeout, currentChannel.counterpartyPortIdentifier, currentChannel.counterpartyChannelIdentifier, counterpartyUpgrade)) + + currentChannel.state = BLOCKUPGRADE + publicStore.set(channelPath(portIdentifier, channelIdentifier), channel) -`restoreChannel()` is a utility function that allows a chain to abort an upgrade handshake in progress, and return the `ChannelEnd` to its original pre-upgrade state while also setting the upgrade `errorReceipt`. A relayer can then send a `ChannelUpgradeCancelMsg` to the counterparty so that it can restore its `ChannelEnd` to its pre-upgrade state as well. Once both channel ends are back to the pre-upgrade state, packet processing will resume with the original channel and application parameters. + privateStore.set(channelCounterpartyLastPacketSequencePath(portIdentifier, channelIdentifier), counterpartyUpgrade.lastPacketSent) +} +``` + +`restoreChannel` will write an error receipt, set the original channel and delete upgrade information when the executing channel needs to abort the upgrade handshake and return to the original parameters. ```typescript -function restoreChannel(portIdentifier: Identifier, channelIdentifier: Identifier) { - // cancel upgrade - // write an error receipt with the current sequence into the error path - // and restore original channel - sequence = provableStore.get(channelUpgradeSequencePath(portIdentifier, channelIdentifier)) +// restoreChannel signature may be modified to take a custom error +function restoreChannel( + portIdentifier: Identifier, + channelIdentifier: Identifier, +) { + channel = provableStore.get(channelPath(portIdentifier, channelIdentifier)) errorReceipt = ErrorReceipt{ - sequence: sequence, - errorMsg: "" + channel.sequence, + "upgrade handshake is aborted", // constant string changable by implementation } provableStore.set(channelUpgradeErrorPath(portIdentifier, channelIdentifier), errorReceipt) - originalChannel = privateStore.get(channelRestorePath(portIdentifier, channelIdentifier)) - provableStore.set(channelPath(portIdentifier, channelIdentifier), originalChannel) - provableStore.delete(channelUpgradeTimeoutPath(portIdentifier, channelIdentifier)) - privateStore.delete(channelRestorePath(portIdentifier, channelIdentifier)) - - // call modules onChanUpgradeRestore callback - module = lookupModule(portIdentifier) - // restore callback must not return error and it must successfully restore - // application to its pre-upgrade state - module.onChanUpgradeRestore( - portIdentifier, - channelIdentifier - ) - - // caller should return as well + channel.state = OPEN + provableStore.set(channelPath(portIdentifier, channelIdentifier), channel) + provableStore.delete(channelUpgradePath(portIdentifier, channelIdentifier)) } ``` @@ -271,75 +338,23 @@ If an upgrade message arrives after the specified timeout, then the message MUST function chanUpgradeInit( portIdentifier: Identifier, channelIdentifier: Identifier, - proposedUpgradeChannel: ChannelEnd, - counterpartyTimeoutHeight: Height, - counterpartyTimeoutTimestamp: uint64, + proposedUpgradeFields: Upgrade, + timeout: UpgradeTimeout, ) { - // current channel must be OPEN - currentChannel = provableStore.get(channelPath(portIdentifier, channelIdentifier)) - abortTransactionUnless(channel.state == OPEN) - - // abort transaction if an unmodifiable field is modified - // upgraded channel state must be in `INITUPGRADE` - // NOTE: Any added fields are by default modifiable. - abortTransactionUnless( - proposedUpgradeChannel.state == INITUPGRADE && - proposedUpgradeChannel.counterpartyPortIdentier == currentChannel.counterpartyPortIdentifier && - proposedUpgradeChannel.counterpartyChannelIdentifier == currentChannel.counterpartyChannelIdentifier - ) - - // new channel version must be nonempty - abortTransactionUnless(proposedUpgradeChannel.Version != "") - - // current ordering must be a valid ordering of packets - // in the proposed ordering - // e.g. ORDERED -> UNORDERED, ORDERED -> DAG - abortTransactionUnless( - currentChannel.ordering.subsetOf(proposedUpgradeChannel.ordering) - ) - - // proposedConnection must exist and be in OPEN state for - // channel upgrade to be accepted - proposedConnection = provableStore.Get(connectionPath(proposedUpgradeChannel.ConnectionHops[0]) - abortTransactionUnless(proposedConnection != null && proposedConnection.state == OPEN) - - // either timeout height or timestamp must be non-zero - abortTransactionUnless(counterpartyTimeoutHeight != 0 || counterpartyTimeoutTimestamp != 0) - - upgradeTimeout = UpgradeTimeout{ - timeoutHeight: counterpartyTimeoutHeight, - timeoutTimestamp: counterpartyTimeoutTimestamp, - } - - sequence := provableStore.get(channelUpgradeSequencePath(portIdentifier, channelIdentifier)) - if sequence == nil { - sequence = 1 - } else { - sequence++ - } + upgradeSequence = initUpgradeChannel(portIdentifier, channelIdentifier, proposedUpgradeFields, timeout) // call modules onChanUpgradeInit callback module = lookupModule(portIdentifier) version, err = module.onChanUpgradeInit( - proposedUpgradeChannel.ordering, - proposedUpgradeChannel.connectionHops, + proposedUpgrade.fields.ordering, + proposedUpgrade.fields.connectionHops, portIdentifier, - channelIdentifer, - sequence, - proposedUpgradeChannel.counterpartyPortIdentifer, - proposedUpgradeChannel.counterpartyChannelIdentifier, - proposedUpgradeChannel.version + channelIdentifier, + upgradeSequence, + proposedUpgrade.fields.version ) // abort transaction if callback returned error abortTransactionUnless(err != nil) - - // replace channel version with the version returned by application - // in case it was modified - proposedUpgradeChannel.version = version - - provableStore.set(channelUpgradeTimeoutPath(portIdentifier, channelIdentifier), upgradeTimeout) - provableStore.set(channelPath(portIdentifier, channelIdentifier), proposedUpgradeChannel) - privateStore.set(channelRestorePath(portIdentifier, channelIdentifier), currentChannel) } ``` @@ -350,94 +365,57 @@ Access control on counterparty should inform choice of timeout values, i.e. time function chanUpgradeTry( portIdentifier: Identifier, channelIdentifier: Identifier, - counterpartyChannel: ChannelEnd, - counterpartySequence: uint64, - proposedUpgradeChannel: ChannelEnd, - timeoutHeight: Height, - timeoutTimestamp: uint64, + counterpartyUpgrade: Upgrade, + counterpartyUpgradeSequence: uint64, + proposedConnectionHops: [Identifier], proofChannel: CommitmentProof, - proofUpgradeTimeout: CommitmentProof, - proofUpgradeSequence: CommitmentProof, + proofUpgrade: CommitmentProof, proofHeight: Height ) { // current channel must be OPEN or INITUPGRADE (crossing hellos) currentChannel = provableStore.get(channelPath(portIdentifier, channelIdentifier)) abortTransactionUnless(currentChannel.state == OPEN || currentChannel.state == INITUPGRADE) - // abort transaction if an unmodifiable field is modified - // upgraded channel state must be in `TRYUPGRADE` - // NOTE: Any added fields are by default modifiable. - abortTransactionUnless( - proposedUpgradeChannel.state == TRYUPGRADE && - proposedUpgradeChannel.counterpartyPortIdentifier == currentChannel.counterpartyPortIdentifier && - proposedUpgradeChannel.counterpartyChannelIdentifier == currentChannel.counterpartyChannelIdentifier - ) - - // proposedConnection must exist and be in OPEN state - // connectionHops can change in a channelUpgrade, however both sides must still be each other's counterparty. - proposedConnection = provableStore.Get(connectionPath(proposedUpgradeChannel.ConnectionHops[0]) - abortTransactionUnless(proposedConnection != null && proposedConnection.state == OPEN) - abortTransactionUnless(counterpartyChannel.connectionHops[0] == proposedConnection.counterpartyConnectionIdentifier) + // create upgrade fields for this chain from counterparty upgrade and relayer-provided information + // version may be mutated by application callback + upgradeFields = Upgrade{ + ordering: counterpartyUpgrade.fields.ordering, + connectionHops: proposedConnectionHops, + version: counterpartyUpgrade.fields.version, + } // 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 channel on the executing chain and allow counterparty to use the ChannelUpgradeCancelMsg to restore their channel. - abortTransactionUnless(timeoutHeight != 0 || timeoutTimestamp != 0) - - // current ordering must be a valid ordering of packets - // in the proposed ordering - // e.g. ORDERED -> UNORDERED, ORDERED -> DAG + timeout = counterpartyUpgrade.timeout + abortTransactionUnless(timeout.timeoutHeight != 0 || timeout.timeoutTimestamp != 0) + // counterparty-specified timeout must not have exceeded abortTransactionUnless( - currentChannel.ordering.subsetOf(proposedUpgradeChannel.ordering) + (currentHeight() > timeout.timeoutHeight && timeout.timeoutHeight != 0) || + (currentTimestamp() > timeout.timeoutTimestamp && timeout.timeoutTimestamp != 0) ) - // new channel version must be nonempty - abortTransactionUnless(proposedUpgradeChannel.Version != "") - - // both channel ends must be mutually compatible. - // this means that the ordering must be the same and - // any future introduced fields that must be compatible - // should also be checked - abortTransactionUnless(counterpartyChannel.ordering != proposedUpgradeChannel.ordering) - - // construct upgradeTimeout so it can be verified against counterparty state - upgradeTimeout = UpgradeTimeout{ - timeoutHeight: timeoutHeight, - timeoutTimestamp: timeoutTimestamp, - } - - // get underlying connection for proof verification - connection = getConnection(currentChannel.connectionIdentifier) - - // verify proofs of counterparty state - abortTransactionUnless(verifyChannelState(connection, proofHeight, proofChannel, currentChannel.counterpartyPortIdentifier, currentChannel.counterpartyChannelIdentifier, counterpartyChannel)) - abortTransactionUnless(verifyChannelUpgradeTimeout(connection, proofHeight, proofUpgradeTimeout, currentChannel.counterpartyPortIdentifier, currentChannel.counterpartyChannelIdentifier, upgradeTimeout)) - abortTransactionUnless(verifyUpgradeSequence(connection, proofHeight, proofUpgradeSequence, currentChannel.counterpartyPortIdentifier, - currentChannel.counterpartyChannelIdentifier, counterpartySequence)) - - // get current sequence on this channel - currentSequence = provableStore.get(channelUpgradeSequencePath(portIdentifier, channelIdentifier)) - - // move upgrade sequence to fresh sequence for new upgrade attempt - if channel.state === types.OPEN { - channel.upgradeSequence = channel.upgradeSequence + 1 - + // if OPEN, then initialize handshake with upgradeFields + // otherwise, assert that the upgrade fields are the same for crossing-hellos case + if currentChannel.state == OPEN { // if the counterparty sequence is greater than the current sequence, we fast forward to the counterparty sequence // so that both channel ends are using the same sequence for the current upgrade + // initUpgradeChannelHandshake will increment the sequence so after that call + // both sides will have the same upgradeSequence if counterpartyUpgradeSequence > channel.upgradeSequence { - channel.upgradeSequence = counterpartyUpgradeSequence + channel.upgradeSequence = counterpartyUpgradeSequence - 1 } - // this is first message in upgrade handshake on this chain so we must store original channel in restore channel path - // in case we need to restore channel later. - privateStore.set(channelRestorePath(portIdentifier, channelIdentifier), currentChannel) - provableStore.set(channelPath(portIdentifier, channelIdentifier), channel) + initUpgradeChannelHandshake(portIdentifier, channelIdentifier, upgradeFields, counterpartyUpgrade.timeout) + } else if currentChannel.state == INITUPGRADE { + existingUpgrade = publicStore.get(channelUpgradePath) + abortTransactionUnless(existingUpgrade.fields == upgradeFields) } // if the counterparty sequence is not equal to the current sequence, then either the counterparty chain is out-of-sync or // the message is out-of-sync and we write an error receipt with our own sequence so that the counterparty can update // their sequence as well. We must then increment our sequence so both sides start the next upgrade with a fresh sequence. - if counterpartySequence != channel.upgradeSequence { + if counterpartyUpgradeSequence != channel.upgradeSequence { // error on the higher sequence so that both chains move to a fresh sequence maxSequence = max(counterpartySequence, channel.upgradeSequence) errorReceipt = ErrorReceipt{ @@ -449,13 +427,28 @@ function chanUpgradeTry( return } - // counterparty-specified timeout must not have exceeded - if (currentHeight() > timeoutHeight && timeoutHeight != 0) || - (currentTimestamp() > timeoutTimestamp && timeoutTimestamp != 0) { - restoreChannel(portIdentifier, channelIdentifier) - return + // get counterpartyHops for given connection + connection = getConnection(currentChannel.connectionIdentifier) + counterpartyHops = getCounterpartyHops(connection) + + // construct counterpartyChannel from existing information and provided + // counterpartyUpgradeSequence + counterpartyChannel = ChannelEnd{ + state: INITUPGRADE, + ordering: currentChannel.ordering, + counterpartyPortIdentifier: portIdentifier, + counterpartyChannelIdentifier: channelIdentifier, + connectionHops: counterpartyHops, + version: currentChannel.version, + counterpartyUpgradeSequence, } + // call blockUpgrade handshake to move channel from INITUPGRADE to BLOCKUPGRADE + blockUpgradeHandshake(portIdentifier, channelIdentifier, upgradeFields, counterpartyChannel, counterpartyUpgrade, proofChannel, proofUpgrade, proofHeight) + + // refresh currentChannel to get latest state + currentChannel = provableStore.get(channelPath(portIdentifier, channelIdentifier)) + // call modules onChanUpgradeTry callback module = lookupModule(portIdentifier) version, err = module.onChanUpgradeTry( @@ -463,7 +456,7 @@ function chanUpgradeTry( proposedUpgradeChannel.connectionHops, portIdentifier, channelIdentifer, - currentSequence, + currentChannel.sequence, proposedUpgradeChannel.counterpartyPortIdentifer, proposedUpgradeChannel.counterpartyChannelIdentifier, proposedUpgradeChannel.version @@ -476,9 +469,9 @@ function chanUpgradeTry( // replace channel version with the version returned by application // in case it was modified - proposedUpgradeChannel.version = version - - provableStore.set(channelPath(portIdentifier, channelIdentifier), proposedUpgradeChannel) + upgrade = publicStore.get(channelUpgradePath(portIdentifier, channelIdentifier)) + upgrade.fields.version = version + provableStore.set(channelUpgradePath(portIdentifier, channelIdentifier), upgrade) } ``` @@ -497,10 +490,6 @@ function chanUpgradeAck( currentChannel = provableStore.get(channelPath(portIdentifier, channelIdentifier)) abortTransactionUnless(currentChannel.state == INITUPGRADE || currentChannel.state == TRYUPGRADE) - // connectionHops can change in a channelUpgrade, however both sides must still be each other's counterparty. - proposedConnection = provableStore.get(connectionPath(proposedUpgradeChannel.connectionHops[0]) - abortTransactionUnless(counterpartyChannel.connectionHops[0] == proposedConnection.counterpartyConnectionIdentifier) - // get underlying connection from the original channel for proof verification originalChannel = provableStore.get(channelRestorePath(portIdentifier, channelIdentifier)) connection = getConnection(originalChannel.connectionIdentifier) From c004220c9df6bc5519d3a1369627321155450d60 Mon Sep 17 00:00:00 2001 From: Aditya Sripal Date: Thu, 20 Apr 2023 17:56:53 +0200 Subject: [PATCH 02/19] ACK complete for in-flight packets option --- .../UPGRADES.md | 83 ++++++++++--------- 1 file changed, 42 insertions(+), 41 deletions(-) diff --git a/spec/core/ics-004-channel-and-packet-semantics/UPGRADES.md b/spec/core/ics-004-channel-and-packet-semantics/UPGRADES.md index 90adc73a4..45907c349 100644 --- a/spec/core/ics-004-channel-and-packet-semantics/UPGRADES.md +++ b/spec/core/ics-004-channel-and-packet-semantics/UPGRADES.md @@ -268,11 +268,6 @@ function blockUpgradeHandshake( currentChannel = provableStore.get(channelPath(portIdentifier, channelIdentifier)) abortTransactionUnless(channel.state == INITUPGRADE) - // connectionHops can change in a channelUpgrade, however both sides must still be each other's counterparty. - proposedConnection = provableStore.get(connectionPath(proposedUpgradeFields.connectionHops[0]) - abortTransactionUnless(proposedConnection != null && proposedConnection.state == OPEN) - abortTransactionUnless(counterpartyUpgrade.fields.connectionHops[0] == proposedConnection.counterpartyConnectionIdentifier) - // get underlying connection for proof verification connection = getConnection(currentChannel.connectionIdentifier) counterpartyHops = getCounterpartyHops(connection) @@ -281,6 +276,20 @@ function blockUpgradeHandshake( abortTransactionUnless(verifyChannelState(connection, proofHeight, proofChannel, currentChannel.counterpartyPortIdentifier, currentChannel.counterpartyChannelIdentifier, counterpartyChannel)) abortTransactionUnless(verifyChannelUpgrade(connection, proofHeight, proofUpgradeTimeout, currentChannel.counterpartyPortIdentifier, currentChannel.counterpartyChannelIdentifier, counterpartyUpgrade)) + // new ordering must be the same as the counterparty proposed ordering + if proposedUpgradeFields.ordering != counterpartyUpgradeFields.ordering { + restoreChannel(portIdentifier, channelIdentifier) + } + + // connectionHops can change in a channelUpgrade, however both sides must still be each other's counterparty. + proposedConnection = provableStore.get(connectionPath(proposedUpgradeFields.connectionHops[0]) + if (proposedConnection == null || proposedConnection.state != OPEN) { + restoreChannel(portIdentifier, channelIdentifier) + } + if (counterpartyUpgrade.fields.connectionHops[0] != proposedConnection.counterpartyConnectionIdentifier) { + restoreChannel(portIdentifier, channelIdentifier) + } + currentChannel.state = BLOCKUPGRADE publicStore.set(channelPath(portIdentifier, channelIdentifier), channel) @@ -440,7 +449,7 @@ function chanUpgradeTry( counterpartyChannelIdentifier: channelIdentifier, connectionHops: counterpartyHops, version: currentChannel.version, - counterpartyUpgradeSequence, + sequence: counterpartyUpgradeSequence, } // call blockUpgrade handshake to move channel from INITUPGRADE to BLOCKUPGRADE @@ -481,52 +490,39 @@ NOTE: It is up to individual implementations how they will provide access-contro function chanUpgradeAck( portIdentifier: Identifier, channelIdentifier: Identifier, - counterpartyChannel: ChannelEnd, + counterpartyUpgrade: Upgrade, proofChannel: CommitmentProof, - proofUpgradeSequence: CommitmentProof, + proofUpgrade: CommitmentProof, proofHeight: Height ) { // current channel is in INITUPGRADE or TRYUPGRADE (crossing hellos) currentChannel = provableStore.get(channelPath(portIdentifier, channelIdentifier)) abortTransactionUnless(currentChannel.state == INITUPGRADE || currentChannel.state == TRYUPGRADE) - // get underlying connection from the original channel for proof verification - originalChannel = provableStore.get(channelRestorePath(portIdentifier, channelIdentifier)) - connection = getConnection(originalChannel.connectionIdentifier) - - // verify proofs of counterparty state - abortTransactionUnless(verifyChannelState(connection, proofHeight, proofChannel, currentChannel.counterpartyPortIdentifier, currentChannel.counterpartyChannelIdentifier, counterpartyChannel)) - - // verify that the counterparty sequence is the same as the current sequence to ensure that the proofs were - // retrieved from the current upgrade attempt - // since all proofs are retrieved from same proof height, and there can not be multiple upgrade states in the store for a given - // channel at the same time - sequence = provableStore.get(channelUpgradeSequencePath(portIdentifier, channelIdentifier)) - abortTransactionUnless(verifyUpgradeSequence(connection, proofHeight, proofUpgradeSequence, currentChannel.counterpartyPortIdentifier, - currentChannel.counterpartyChannelIdentifier, sequence)) + connection = getConnection(currentChannel.connectionIdentifier) + counterpartyHops = getCounterpartyHops(connection) - // counterparty must be in TRY state - if counterpartyChannel.State != TRYUPGRADE { - restoreChannel(portIdentifier, channelIdentifier) - return + // construct counterpartyChannel from existing information and provided + // counterpartyUpgradeSequence + counterpartyChannel = ChannelEnd{ + state: TRYUPGRADE, + ordering: currentChannel.ordering, + counterpartyPortIdentifier: portIdentifier, + counterpartyChannelIdentifier: channelIdentifier, + connectionHops: counterpartyHops, + version: currentChannel.version, + sequence: counterpartyUpgradeSequence, } - // both channel ends must be mutually compatible. - // this means that the ordering must be the same and - // any future introduced fields that must be compatible - // should also be checked - if counterpartyChannel.ordering != proposedUpgradeChannel.ordering { - restoreChannel(portIdentifier, channelIdentifier) - return - } + upgrade = provableStore.get(channelUpgradePath(portIdentifier, channelIdentifier)) + blockUpgradeHandshake(portIdentifier, channelIdentifier, upgrade.fields, counterpartyChannel, counterpartyUpgrade, proofChannel, proofUpgrade, proofHeight) // call modules onChanUpgradeAck callback module = lookupModule(portIdentifier) err = module.onChanUpgradeAck( portIdentifier, channelIdentifier, - counterpartyChannel.channelIdentifier, - counterpartyChannel.version + counterpartyUpgrade.version ) // restore channel if callback returned error if err != nil { @@ -534,12 +530,17 @@ function chanUpgradeAck( return } - // upgrade is complete - // set channel to OPEN and remove unnecessary state - currentChannel.state = OPEN + // if no error, agree on final version + upgrade.version = counterpartyUpgrade.version + provableStore.set(channelUpgradePath(portIdentifier, channelIdentifier), upgrade) + + + + // set channel to BLOCKUPGRADE + currentChannel.state = BLOCKUPGRADE provableStore.set(channelPath(portIdentifier, channelIdentifier), currentChannel) - provableStore.delete(channelUpgradeTimeoutPath(portIdentifier, channelIdentifier)) - privateStore.delete(channelRestorePath(portIdentifier, channelIdentifier)) + + // TODO: if packet commitments are empty then open the upgrade handshake } ``` From 06db3a9b6f4b1d6b14bf1f47a551eafd0348ddca Mon Sep 17 00:00:00 2001 From: Aditya Sripal Date: Mon, 24 Apr 2023 20:11:34 +0200 Subject: [PATCH 03/19] complete ACK --- .../UPGRADES.md | 75 +++++++++++++++---- 1 file changed, 59 insertions(+), 16 deletions(-) diff --git a/spec/core/ics-004-channel-and-packet-semantics/UPGRADES.md b/spec/core/ics-004-channel-and-packet-semantics/UPGRADES.md index 45907c349..e0b6ea482 100644 --- a/spec/core/ics-004-channel-and-packet-semantics/UPGRADES.md +++ b/spec/core/ics-004-channel-and-packet-semantics/UPGRADES.md @@ -260,14 +260,11 @@ function blockUpgradeHandshake( proposedUpgradeFields: UpgradeFields, counterpartyChannel: ChannelEnd, counterpartyUpgrade: Upgrade, + blockingState: ChannelState, proofChannel: CommitmentProof, proofUpgrade: CommitmentProof, proofHeight: Height ) { - // current channel must be INITUPGRADE - currentChannel = provableStore.get(channelPath(portIdentifier, channelIdentifier)) - abortTransactionUnless(channel.state == INITUPGRADE) - // get underlying connection for proof verification connection = getConnection(currentChannel.connectionIdentifier) counterpartyHops = getCounterpartyHops(connection) @@ -276,7 +273,7 @@ function blockUpgradeHandshake( abortTransactionUnless(verifyChannelState(connection, proofHeight, proofChannel, currentChannel.counterpartyPortIdentifier, currentChannel.counterpartyChannelIdentifier, counterpartyChannel)) abortTransactionUnless(verifyChannelUpgrade(connection, proofHeight, proofUpgradeTimeout, currentChannel.counterpartyPortIdentifier, currentChannel.counterpartyChannelIdentifier, counterpartyUpgrade)) - // new ordering must be the same as the counterparty proposed ordering + // proposed ordering must be the same as the counterparty proposed ordering if proposedUpgradeFields.ordering != counterpartyUpgradeFields.ordering { restoreChannel(portIdentifier, channelIdentifier) } @@ -297,6 +294,28 @@ function blockUpgradeHandshake( } ``` +`openUpgradeHandshake` will open the channel and switch the existing channel parameters to the newly agreed-upon uprade channel fields. + +```typescript +function openUpgradeHandshake( + portIdentifier: Identifier, + channelIdentifier: Identifier, +) { + // current channel must be BLOCKUPGRADE + currentChannel = provableStore.get(channelPath(portIdentifier, channelIdentifier)) + abortTransactionUnless(channel.state == BLOCKUPGRADE) + + upgrade = provableStore.get(channelUpgradePath(portIdentifier, channelIdentifier)) + + // switch channel fields to upgrade fields + // and set channel state to OPEN + currentChannel.ordering = upgrade.fields.ordering + currentChannel.version = upgrade.fields.version + currentChannel.connectionHops = upgrade.fields.connectionHops + currentchannel.state = OPEN +} +``` + `restoreChannel` will write an error receipt, set the original channel and delete upgrade information when the executing channel needs to abort the upgrade handshake and return to the original parameters. ```typescript @@ -317,6 +336,16 @@ function restoreChannel( } ``` +`pendingInflightPackets` will return the list of in-flight packet sequences sent from this `channelEnd`. This can be monitored since the packet commitments are deleted when the packet lifecycle is complete. Thus if the packet commitment exists on the sender chain, the packet lifecycle is incomplete. The pseudocode is not provided in this spec since it will be dependent on the state machine in-question. The ibc-go implementation will use the store iterator to implement this functionality. The function signature is provided below: + +```typescript +// pendingInflightPacketSequences returns the packet sequences sent on this end that have not had their lifecycle completed +function pendingInflightPacketSequences( + portIdentifier: Identifier, + channelIdentifier: Identifier, +) [uint64] +``` + ### Upgrade Handshake The upgrade handshake defines four datagrams: *ChanUpgradeInit*, *ChanUpgradeTry*, *ChanUpgradeAck*, and *ChanUpgradeConfirm* @@ -452,8 +481,8 @@ function chanUpgradeTry( sequence: counterpartyUpgradeSequence, } - // call blockUpgrade handshake to move channel from INITUPGRADE to BLOCKUPGRADE - blockUpgradeHandshake(portIdentifier, channelIdentifier, upgradeFields, counterpartyChannel, counterpartyUpgrade, proofChannel, proofUpgrade, proofHeight) + // call blockUpgrade handshake to move channel from INITUPGRADE to TRYUPGRADE + blockUpgradeHandshake(portIdentifier, channelIdentifier, upgradeFields, counterpartyChannel, counterpartyUpgrade, TRYUPGRADE, proofChannel, proofUpgrade, proofHeight) // refresh currentChannel to get latest state currentChannel = provableStore.get(channelPath(portIdentifier, channelIdentifier)) @@ -515,9 +544,22 @@ function chanUpgradeAck( } upgrade = provableStore.get(channelUpgradePath(portIdentifier, channelIdentifier)) - blockUpgradeHandshake(portIdentifier, channelIdentifier, upgrade.fields, counterpartyChannel, counterpartyUpgrade, proofChannel, proofUpgrade, proofHeight) + + // in the crossing hellos case, the versions returned by both on TRY must be the same + if currentChannel.state == TRYUPGRADE { + if upgrade.fields.version != counterpartyUpgrade.fields.version { + restoreChannel(portIdentifier, channelIdentifier) + } + } + + // prove counterparty and move our own state to BLOCKUPGRADE + // we can open unilaterally once packets have been flushed + blockUpgradeHandshake(portIdentifier, channelIdentifier, upgrade.fields, counterpartyChannel, counterpartyUpgrade, BLOCKUPGRADE, proofChannel, proofUpgrade, proofHeight) // call modules onChanUpgradeAck callback + // module can error on counterparty version + // ACK should not change state to the new parameters yet + // as that will happen on the onChanUpgradeOpen callback module = lookupModule(portIdentifier) err = module.onChanUpgradeAck( portIdentifier, @@ -534,13 +576,11 @@ function chanUpgradeAck( upgrade.version = counterpartyUpgrade.version provableStore.set(channelUpgradePath(portIdentifier, channelIdentifier), upgrade) - - - // set channel to BLOCKUPGRADE - currentChannel.state = BLOCKUPGRADE - provableStore.set(channelPath(portIdentifier, channelIdentifier), currentChannel) - - // TODO: if packet commitments are empty then open the upgrade handshake + // if packet commitments are empty then open the upgrade handshake immediately + if pendingInflightPackets(portIdentifier, channelIdentifier) == nil { + openChannelHandshake(portIdentifier, channelIdentifier) + module.onChanUpgradeOpen(portIdentifier, channelIdentifier) + } } ``` @@ -554,9 +594,12 @@ function chanUpgradeConfirm( proofUpgradeSequence: CommitmentProof, proofHeight: Height, ) { + // if packet commitments are not empty then abort the transaction + abortTransactionUnless(pendingInflightPackets(portIdentifier, channelIdentifier)) + // current channel is in TRYUPGRADE currentChannel = provableStore.get(channelPath(portIdentifier, channelIdentifier)) - abortTransactionUnless(channel.state == TRYUPGRADE) + abortTransactionUnless(channel.state == BLOCKUPGRADE) // counterparty must be in OPEN state abortTransactionUnless(counterpartyChannel.State == OPEN) From 6c23a1f053ee97d8a2e4f29dcb79a3c59ed9b427 Mon Sep 17 00:00:00 2001 From: Aditya Sripal Date: Mon, 24 Apr 2023 20:27:15 +0200 Subject: [PATCH 04/19] fix happy path case --- .../UPGRADES.md | 91 +++++++++++++------ 1 file changed, 62 insertions(+), 29 deletions(-) diff --git a/spec/core/ics-004-channel-and-packet-semantics/UPGRADES.md b/spec/core/ics-004-channel-and-packet-semantics/UPGRADES.md index e0b6ea482..8a319da5f 100644 --- a/spec/core/ics-004-channel-and-packet-semantics/UPGRADES.md +++ b/spec/core/ics-004-channel-and-packet-semantics/UPGRADES.md @@ -588,56 +588,89 @@ function chanUpgradeAck( function chanUpgradeConfirm( portIdentifier: Identifier, channelIdentifier: Identifier, - counterpartyChannel: ChannelEnd, + counterpartyChannelState: ChannelState, proofChannel: CommitmentProof, proofUpgradeError: CommitmentProof, proofUpgradeSequence: CommitmentProof, proofHeight: Height, ) { - // if packet commitments are not empty then abort the transaction + // if packet commitments are not empty then abort the transaction abortTransactionUnless(pendingInflightPackets(portIdentifier, channelIdentifier)) // current channel is in TRYUPGRADE currentChannel = provableStore.get(channelPath(portIdentifier, channelIdentifier)) - abortTransactionUnless(channel.state == BLOCKUPGRADE) + abortTransactionUnless(channel.state == TRYUPGRADE) - // counterparty must be in OPEN state - abortTransactionUnless(counterpartyChannel.State == OPEN) - - // get current sequence - sequence = provableStore.get(channelUpgradeSequencePath(portIdentifier, channelIdentifier)) + connection = getConnection(currentChannel.connectionIdentifier) + counterpartyHops = getCounterpartyHops(connection) - // get underlying connection from the original channel for proof verification - originalChannel = provableStore.get(channelRestorePath(portIdentifier, channelIdentifier)) - connection = getConnection(originalChannel.connectionIdentifier) + // counterparty must be in OPEN or BLOCKUPGRADE state + if counterpartyChannelState == OPEN { + // get upgrade since counterparty should have upgraded to these parameters + upgrade = provableStore.get(channelUpgradePath(portIdentifier, channelIdentifier)) + + counterpartyChannel = ChannelEnd{ + state: OPEN, + ordering: upgrade.fields.ordering, + counterpartyPortIdentifier: portIdentifier, + counterpartyChannelIdentifier: channelIdentifier, + connectionHops: upgrade.fields.connectionHops, + version: upgrade.fields.version, + sequence: currentChannel.sequence, + } + } else if counterpartyChannelState == BLOCKUPGRADE { + counterpartyChannel = ChannelEnd{ + state: BLOCKUPGRADE, + ordering: currentChannel.ordering, + counterpartyPortIdentifier: portIdentifier, + counterpartyChannelIdentifier: channelIdentifier, + connectionHops: counterpartyHops, + version: currentChannel.version, + sequence: currentChannel.sequence, + } + } else { + abortTransactionUnless(false) + } - // verify proofs of counterparty state abortTransactionUnless(verifyChannelState(connection, proofHeight, proofChannel, currentChannel.counterpartyPortIdentifier, currentChannel.counterpartyChannelIdentifier, counterpartyChannel)) - // verify counterparty did not abort upgrade handshake by writing upgrade error - // must have absent value at upgradeError path at the current sequence - abortTransactionUnless(verifyUpgradeChannelErrorAbsence(connection, proofHeight, proofUpgradeError, currentChannel.counterpartyPortIdentifier, currentChannel.counterpartyChannelIdentifier)) - // verify that the counterparty sequence is the same as the current sequence to ensure that the proofs were - // retrieved from the current upgrade attempt - // since all proofs are retrieved from same proof height, and there can not be multiple upgrade states in the store for a given - // channel at the same time - abortTransactionUnless(verifyUpgradeSequence(connection, proofHeight, proofUpgradeSequence, currentChannel.counterpartyPortIdentifier, - currentChannel.counterpartyChannelIdentifier, sequence)) + // move channel to OPEN and adopt upgrade parameters + openChannelHandshake(portIdentifier, channelIdentifier) // call modules onChanUpgradeConfirm callback module = lookupModule(portIdentifier) // confirm callback must not return error since counterparty successfully upgraded - module.onChanUpgradeConfirm( + module.onChanUpgradeOpen( + portIdentifer, + channelIdentifier + ) +} +``` + +```typescript +// chanUpgradeOpen can be called unilaterally on the channelEnd that is at `BLOCKUPGRADE` +// once the packets are flushed +function chanUpgradeOpen( + portIdentifier: Identifier, + channelIdentifier: Identifier, +) { + // current channel is in BLOCKUPGRADE + currentChannel = provableStore.get(channelPath(portIdentifier, channelIdentifier)) + abortTransactionUnless(channel.state == BLOCKUPGRADE) + + // if packet commitments are not empty then abort the transaction + abortTransactionUnless(pendingInflightPackets(portIdentifier, channelIdentifier)) + + // move channel to OPEN and adopt upgrade parameters + openChannelHandshake(portIdentifier, channelIdentifier) + + // call modules onChanUpgradeConfirm callback + module = lookupModule(portIdentifier) + // confirm callback must not return error since counterparty successfully upgraded + module.onChanUpgradeOpen( portIdentifer, channelIdentifier ) - - // upgrade is complete - // set channel to OPEN and remove unnecessary state - currentChannel.state = OPEN - provableStore.set(channelPath(portIdentifier, channelIdentifier), currentChannel) - provableStore.delete(channelUpgradeTimeoutPath(portIdentifier, channelIdentifier)) - privateStore.delete(channelRestorePath(portIdentifier, channelIdentifier)) } ``` From 89fa81e5aa6cc83e0a47960999d7ae23e8a907c9 Mon Sep 17 00:00:00 2001 From: Aditya Sripal Date: Mon, 24 Apr 2023 20:28:50 +0200 Subject: [PATCH 05/19] fix arg list --- spec/core/ics-004-channel-and-packet-semantics/UPGRADES.md | 2 -- 1 file changed, 2 deletions(-) diff --git a/spec/core/ics-004-channel-and-packet-semantics/UPGRADES.md b/spec/core/ics-004-channel-and-packet-semantics/UPGRADES.md index 8a319da5f..501cf9489 100644 --- a/spec/core/ics-004-channel-and-packet-semantics/UPGRADES.md +++ b/spec/core/ics-004-channel-and-packet-semantics/UPGRADES.md @@ -590,8 +590,6 @@ function chanUpgradeConfirm( channelIdentifier: Identifier, counterpartyChannelState: ChannelState, proofChannel: CommitmentProof, - proofUpgradeError: CommitmentProof, - proofUpgradeSequence: CommitmentProof, proofHeight: Height, ) { // if packet commitments are not empty then abort the transaction From 7f317aeac6036413b604a4726c4611dc64019358 Mon Sep 17 00:00:00 2001 From: Aditya Sripal Date: Tue, 25 Apr 2023 17:16:41 +0200 Subject: [PATCH 06/19] update non-pseudocode parts of happy path spec --- .../UPGRADES.md | 45 ++++++++++++------- 1 file changed, 29 insertions(+), 16 deletions(-) diff --git a/spec/core/ics-004-channel-and-packet-semantics/UPGRADES.md b/spec/core/ics-004-channel-and-packet-semantics/UPGRADES.md index 501cf9489..8b3136c3d 100644 --- a/spec/core/ics-004-channel-and-packet-semantics/UPGRADES.md +++ b/spec/core/ics-004-channel-and-packet-semantics/UPGRADES.md @@ -16,14 +16,13 @@ As new features get added to IBC, chains may wish the take advantage of new chan - either it is unsuccessful and then the channel MUST fall-back to the original channel parameters; - or it is successful and then both channel ends MUST adopt the new channel parameters and the applications must process packet data appropriately. - Packets sent under the previously negotiated parameters must be processed under the previously negotiated parameters, packets sent under the newly negotiated parameters must be processed under the newly negotiated parameters. -- The channel upgrade protocol should have the ability to change all channel-related parameters; however the channel upgrade protocol MUST NOT be able to change the underlying `ConnectionEnd`. - The channel upgrade protocol MUST NOT modify the channel identifiers. ## Technical Specification ### Data Structures -The `ChannelState` and `ChannelEnd` are defined in [ICS-4](./README.md), they are reproduced here for the reader's convenience. `INITUPGRADE`, `TRYUPGRADE` are additional states added to enable the upgrade feature. +The `ChannelState` and `ChannelEnd` are defined in [ICS-4](./README.md), they are reproduced here for the reader's convenience. `INITUPGRADE`, `TRYUPGRADE`, `BLOCKUPGRADE` are additional states added to enable the upgrade feature. #### `ChannelState` @@ -33,12 +32,18 @@ enum ChannelState { TRYOPEN, OPEN, INITUPGRADE, + TRYUPGRADE, BLOCKUPGRADE, } ``` - The chain that is proposing the upgrade should set the channel state from `OPEN` to `INITUPGRADE` -- The counterparty chain that accepts the upgrade should set the channel state from `OPEN` to `BLOCKUPGRADE` +- The counterparty chain that accepts the upgrade should set the channel state from `OPEN` to `TRYUPGRADE` +- Once the initiating chain verifies the counterparty is in `TRYUPGRADE`, it must move to `BLOCKUPGRADE` in the case where there still exist in-flight packets on its end or complete the upgrade and move to `OPEN` +- The `TRYUPGRADE` chain must prove the counterparty is in `BLOCKUPGRADE` or completed the upgrade in `OPEN` AND have no in-flight packets before it can complete the upgrade and move to `OPEN`. +- The `BLOCKUPGRADE` chain may OPEN as soon as the in-flight packets on its end have been flushed. + +Both `TRYUPGRADE` and `BLOCKUPGRADE` are "blocking" states in that they will prevent the upgrade handshake from proceeding until the in-flight packets are flushed. The `TRYUPGRADE` state must additionally prove the counterparty state before proceeding to open, while the `BLOCKUPGRADE` state may move to `OPEN` unilaterally once packets are flushed on its end. #### `ChannelEnd` @@ -54,9 +59,10 @@ interface ChannelEnd { } ``` -The desired property that the channel upgrade protocol MUST NOT modify the underlying clients or channel identifiers, means that only some fields of `ChannelEnd` are upgradable by the upgrade protocol. - - `state`: The state is specified by the handshake steps of the upgrade protocol and will be mutated in place during the handshake. +- `upgradeSequence`: The upgrade sequence will be incremented and agreed upon during the upgrade handshake and will be mutated in place. + +All other parameters will remain the same during the upgrade handshake until the upgrade handshake completes. When the channel is reset to `OPEN` on a successful upgrade handshake, the fields on the channel end will be switched over to the `UpgradeFields` specified in the upgrade. #### `UpgradeFields` @@ -251,7 +257,7 @@ function initUpgradeChannelHandshake( } ``` -`blockUpgradeHandshake` will set the counterparty last packet send and continue blocking the upgrade from continuing until all in-flight packets have been flushed. When the channel is in blocked mode, any packet receive above the counterparty last packet send will be rejected. +`blockUpgradeHandshake` will set the counterparty last packet send and continue blocking the upgrade from continuing until all in-flight packets have been flushed. When the channel is in blocked mode, any packet receive above the counterparty last packet send will be rejected. It will verify the upgrade parameters and set the channel state to one of the blocking states (`TRYUPGRADE` or `BLOCKUPGRADE`) passed in by caller and block sendpackets. During this time; `receivePacket`, `acknowledgePacket` and `timeoutPacket` will still be allowed and processed according to the original channel parameters. The new proposed upgrade will be stored in the public store for counterparty verification. ```typescript function blockUpgradeHandshake( @@ -265,6 +271,8 @@ function blockUpgradeHandshake( proofUpgrade: CommitmentProof, proofHeight: Height ) { + abortTransactionUnless(blockingState == TRYUPGRADE || blockingState == BLOCKUPGRADE) + // get underlying connection for proof verification connection = getConnection(currentChannel.connectionIdentifier) counterpartyHops = getCounterpartyHops(connection) @@ -287,7 +295,7 @@ function blockUpgradeHandshake( restoreChannel(portIdentifier, channelIdentifier) } - currentChannel.state = BLOCKUPGRADE + currentChannel.state = blockingState publicStore.set(channelPath(portIdentifier, channelIdentifier), channel) privateStore.set(channelCounterpartyLastPacketSequencePath(portIdentifier, channelIdentifier), counterpartyUpgrade.lastPacketSent) @@ -301,9 +309,9 @@ function openUpgradeHandshake( portIdentifier: Identifier, channelIdentifier: Identifier, ) { - // current channel must be BLOCKUPGRADE + // current channel must be TRYUPGRADE or BLOCKUPGRADE currentChannel = provableStore.get(channelPath(portIdentifier, channelIdentifier)) - abortTransactionUnless(channel.state == BLOCKUPGRADE) + abortTransactionUnless(channel.state == TRYUPGRADE || channel.state == BLOCKUPGRADE) upgrade = provableStore.get(channelUpgradePath(portIdentifier, channelIdentifier)) @@ -352,17 +360,22 @@ The upgrade handshake defines four datagrams: *ChanUpgradeInit*, *ChanUpgradeTry A successful protocol execution flows as follows (note that all calls are made through modules per [ICS 25](../ics-025-handler-interface)): -| Initiator | Datagram | Chain acted upon | Prior state (A, B) | Posterior state (A, B) | -| --------- | -------------------- | ---------------- | --------------------------- | --------------------------- | -| Actor | `ChanUpgradeInit` | A | (OPEN, OPEN) | (INITUPGRADE, OPEN) | -| Actor | `ChanUpgradeTry` | B | (INITUPGRADE, OPEN) | (INITUPGRADE, TRYUPGRADE) | -| Relayer | `ChanUpgradeAck` | A | (INITUPGRADE, TRYUPGRADE) | (OPEN, TRYUPGRADE) | -| Relayer | `ChanUpgradeConfirm` | B | (OPEN, TRYUPGRADE) | (OPEN, OPEN) | +| Initiator | Datagram | Chain acted upon | Prior state (A, B) | Posterior state (A, B) | +| --------- | -------------------- | ---------------- | ------------------------------- | ------------------------------- | +| Actor | `ChanUpgradeInit` | A | (OPEN, OPEN) | (INITUPGRADE, OPEN) | +| Actor | `ChanUpgradeTry` | B | (INITUPGRADE, OPEN) | (INITUPGRADE, TRYUPGRADE) | +| Relayer | `ChanUpgradeAck` | A | (INITUPGRADE, TRYUPGRADE) | (BLOCKUPGRADE/OPEN, TRYUPGRADE) | +| Relayer | `ChanUpgradeConfirm` | B | (BLOCKUPGRADE/OPEN, TRYUPGRADE) | (BLOCKUPGRADE/OPEN, OPEN) | +| Relayer | `ChanUpgradeOpen` | A | (BLOCKUPGRADE, OPEN/TRYUPGRADE) | (OPEN, OPEN/TRYUPGRADE) | + +`ChanUpgradeConfirm` and `ChanUpgradeOpen` may happen in any order. `ChanUpgradeOpen` is only necessary to call on chain A if the chain was not moved to `OPEN` on `ChanUpgradeAck`. -At the end of an upgrade handshake between two chains implementing the sub-protocol, the following properties hold: +At the end of a successful upgrade handshake between two chains implementing the sub-protocol, the following properties hold: - Each chain is running their new upgraded channel 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 channel parameters. +- All packets sent before the handshake have been completely flushed (acked or timed out) with the old parameters. +- All packets sent after a channel end moves to OPEN will either timeout using new parameters on sending channelEnd or will be received by the counterparty using new parameters. If a chain does not agree to the proposed counterparty upgraded `ChannelEnd`, it may abort the upgrade handshake by writing an `ErrorReceipt` into the `channelUpgradeErrorPath` and restoring the original channel. The `ErrorReceipt` must contain the current upgrade sequence on the erroring chain's channel end. From c12d6edb761a425f765f96628d6a90650ce861e9 Mon Sep 17 00:00:00 2001 From: Aditya Sripal Date: Thu, 27 Apr 2023 16:07:29 +0200 Subject: [PATCH 07/19] move FLUSH to its own state, and prove both ends have flushed before we OPEN --- .../UPGRADES.md | 143 ++++++++++-------- 1 file changed, 80 insertions(+), 63 deletions(-) diff --git a/spec/core/ics-004-channel-and-packet-semantics/UPGRADES.md b/spec/core/ics-004-channel-and-packet-semantics/UPGRADES.md index 8b3136c3d..72efdd9ac 100644 --- a/spec/core/ics-004-channel-and-packet-semantics/UPGRADES.md +++ b/spec/core/ics-004-channel-and-packet-semantics/UPGRADES.md @@ -22,7 +22,7 @@ As new features get added to IBC, chains may wish the take advantage of new chan ### Data Structures -The `ChannelState` and `ChannelEnd` are defined in [ICS-4](./README.md), they are reproduced here for the reader's convenience. `INITUPGRADE`, `TRYUPGRADE`, `BLOCKUPGRADE` are additional states added to enable the upgrade feature. +The `ChannelState` and `ChannelEnd` are defined in [ICS-4](./README.md), they are reproduced here for the reader's convenience. `INITUPGRADE`, `TRYUPGRADE`, `ACKUPGRADE` are additional states added to enable the upgrade feature. #### `ChannelState` @@ -33,17 +33,17 @@ enum ChannelState { OPEN, INITUPGRADE, TRYUPGRADE, - BLOCKUPGRADE, + ACKUPGRADE, } ``` - The chain that is proposing the upgrade should set the channel state from `OPEN` to `INITUPGRADE` - The counterparty chain that accepts the upgrade should set the channel state from `OPEN` to `TRYUPGRADE` -- Once the initiating chain verifies the counterparty is in `TRYUPGRADE`, it must move to `BLOCKUPGRADE` in the case where there still exist in-flight packets on its end or complete the upgrade and move to `OPEN` -- The `TRYUPGRADE` chain must prove the counterparty is in `BLOCKUPGRADE` or completed the upgrade in `OPEN` AND have no in-flight packets before it can complete the upgrade and move to `OPEN`. -- The `BLOCKUPGRADE` chain may OPEN as soon as the in-flight packets on its end have been flushed. +- Once the initiating chain verifies the counterparty is in `TRYUPGRADE`, it must move to `ACKUPGRADE` in the case where there still exist in-flight packets on its end or complete the upgrade and move to `OPEN` +- The `TRYUPGRADE` chain must prove the counterparty is in `ACKUPGRADE` or completed the upgrade in `OPEN` AND have no in-flight packets before it can complete the upgrade and move to `OPEN`. +- The `ACKUPGRADE` chain may OPEN as soon as the in-flight packets on its end have been flushed. -Both `TRYUPGRADE` and `BLOCKUPGRADE` are "blocking" states in that they will prevent the upgrade handshake from proceeding until the in-flight packets are flushed. The `TRYUPGRADE` state must additionally prove the counterparty state before proceeding to open, while the `BLOCKUPGRADE` state may move to `OPEN` unilaterally once packets are flushed on its end. +Both `TRYUPGRADE` and `ACKUPGRADE` are "blocking" states in that they will prevent the upgrade handshake from proceeding until the in-flight packets are flushed. The `TRYUPGRADE` state must additionally prove the counterparty state before proceeding to open, while the `ACKUPGRADE` state may move to `OPEN` unilaterally once packets are flushed on its end. #### `ChannelEnd` @@ -56,12 +56,23 @@ interface ChannelEnd { connectionHops: [Identifier] version: string upgradeSequence: uint64 + flushStatus: FlushStatus } ``` - `state`: The state is specified by the handshake steps of the upgrade protocol and will be mutated in place during the handshake. - `upgradeSequence`: The upgrade sequence will be incremented and agreed upon during the upgrade handshake and will be mutated in place. +```typescript +enum FlushStatus { + NOTINFLUSH + FLUSHING + FLUSHCOMPLETE +} +``` + +FlushStatus will be in `NOTINFLUSH` state when the channel is not in an upgrade handshake. It will be in `FLUSHING` mode when the channel end is flushing in-flight packets. The FlushStatus will change to `FLUSHCOMPLETE` once there are no in-flight packets left and the channelEnd is ready to move to OPEN. + All other parameters will remain the same during the upgrade handshake until the upgrade handshake completes. When the channel is reset to `OPEN` on a successful upgrade handshake, the fields on the channel end will be switched over to the `UpgradeFields` specified in the upgrade. #### `UpgradeFields` @@ -157,7 +168,7 @@ function verifyChannelUpgrade( #### CounterpartyLastPacketSequence Path -The chain must store the counterparty's last packet sequence on `blockUpgradeHandshake`. This will be stored in the `counterpartyLastPacketSequence` path on the private store/ +The chain must store the counterparty's last packet sequence on `startFlushUpgradeHandshake`. This will be stored in the `counterpartyLastPacketSequence` path on the private store/ ```typescript function channelCounterpartyLastPacketSequencePath(portIdentifier: Identifier, channelIdentifier: ChannelIdentifier): Path { @@ -257,21 +268,21 @@ function initUpgradeChannelHandshake( } ``` -`blockUpgradeHandshake` will set the counterparty last packet send and continue blocking the upgrade from continuing until all in-flight packets have been flushed. When the channel is in blocked mode, any packet receive above the counterparty last packet send will be rejected. It will verify the upgrade parameters and set the channel state to one of the blocking states (`TRYUPGRADE` or `BLOCKUPGRADE`) passed in by caller and block sendpackets. During this time; `receivePacket`, `acknowledgePacket` and `timeoutPacket` will still be allowed and processed according to the original channel parameters. The new proposed upgrade will be stored in the public store for counterparty verification. +`startFlushUpgradeHandshake` will set the counterparty last packet send and continue blocking the upgrade from continuing until all in-flight packets have been flushed. When the channel is in blocked mode, any packet receive above the counterparty last packet send will be rejected. It will verify the upgrade parameters and set the channel state to one of the flushing states (`TRYUPGRADE` or `ACKUPGRADE`) passed in by caller, set the `FlushStatus` to `FLUSHING` and block sendpackets. During this time; `receivePacket`, `acknowledgePacket` and `timeoutPacket` will still be allowed and processed according to the original channel parameters. The new proposed upgrade will be stored in the public store for counterparty verification. ```typescript -function blockUpgradeHandshake( +function startFlushUpgradeHandshake( portIdentifier: Identifier, channelIdentifier: Identifier, proposedUpgradeFields: UpgradeFields, counterpartyChannel: ChannelEnd, counterpartyUpgrade: Upgrade, - blockingState: ChannelState, + channelState: ChannelState, proofChannel: CommitmentProof, proofUpgrade: CommitmentProof, proofHeight: Height ) { - abortTransactionUnless(blockingState == TRYUPGRADE || blockingState == BLOCKUPGRADE) + abortTransactionUnless(channelState == TRYUPGRADE || channelState == ACKUPGRADE) // get underlying connection for proof verification connection = getConnection(currentChannel.connectionIdentifier) @@ -295,7 +306,14 @@ function blockUpgradeHandshake( restoreChannel(portIdentifier, channelIdentifier) } - currentChannel.state = blockingState + currentChannel.state = channelState + currentChannel.flushState = FLUSHING + + // if there are no in-flight packets on our end, we can automatically go to FLUSHCOMPLETE + if pendingInflightPackets(portIdentifier, channelIdentifier) == nil { + currentChannel.flushState = FLUSHCOMPLETE + } + publicStore.set(channelPath(portIdentifier, channelIdentifier), channel) privateStore.set(channelCounterpartyLastPacketSequencePath(portIdentifier, channelIdentifier), counterpartyUpgrade.lastPacketSent) @@ -305,14 +323,12 @@ function blockUpgradeHandshake( `openUpgradeHandshake` will open the channel and switch the existing channel parameters to the newly agreed-upon uprade channel fields. ```typescript +// caller must do all relevant checks before calling this function function openUpgradeHandshake( portIdentifier: Identifier, channelIdentifier: Identifier, ) { - // current channel must be TRYUPGRADE or BLOCKUPGRADE currentChannel = provableStore.get(channelPath(portIdentifier, channelIdentifier)) - abortTransactionUnless(channel.state == TRYUPGRADE || channel.state == BLOCKUPGRADE) - upgrade = provableStore.get(channelUpgradePath(portIdentifier, channelIdentifier)) // switch channel fields to upgrade fields @@ -321,6 +337,8 @@ function openUpgradeHandshake( currentChannel.version = upgrade.fields.version currentChannel.connectionHops = upgrade.fields.connectionHops currentchannel.state = OPEN + currentChannel.flushStatus = NOTINFLUSH + provableStore.set(channelPath(portIdentifier, channelIdentifier), currentChannel) } ``` @@ -360,15 +378,15 @@ The upgrade handshake defines four datagrams: *ChanUpgradeInit*, *ChanUpgradeTry A successful protocol execution flows as follows (note that all calls are made through modules per [ICS 25](../ics-025-handler-interface)): -| Initiator | Datagram | Chain acted upon | Prior state (A, B) | Posterior state (A, B) | -| --------- | -------------------- | ---------------- | ------------------------------- | ------------------------------- | -| Actor | `ChanUpgradeInit` | A | (OPEN, OPEN) | (INITUPGRADE, OPEN) | -| Actor | `ChanUpgradeTry` | B | (INITUPGRADE, OPEN) | (INITUPGRADE, TRYUPGRADE) | -| Relayer | `ChanUpgradeAck` | A | (INITUPGRADE, TRYUPGRADE) | (BLOCKUPGRADE/OPEN, TRYUPGRADE) | -| Relayer | `ChanUpgradeConfirm` | B | (BLOCKUPGRADE/OPEN, TRYUPGRADE) | (BLOCKUPGRADE/OPEN, OPEN) | -| Relayer | `ChanUpgradeOpen` | A | (BLOCKUPGRADE, OPEN/TRYUPGRADE) | (OPEN, OPEN/TRYUPGRADE) | +| Initiator | Datagram | Chain acted upon | Prior state (A, B) | Posterior state (A, B) | +| --------- | -------------------- | ---------------- | ----------------------------- | ------------------------- | +| Actor | `ChanUpgradeInit` | A | (OPEN, OPEN) | (INITUPGRADE, OPEN) | +| Actor | `ChanUpgradeTry` | B | (INITUPGRADE, OPEN) | (INITUPGRADE, TRYUPGRADE) | +| Relayer | `ChanUpgradeAck` | A | (INITUPGRADE, TRYUPGRADE) | (ACKUPGRADE, TRYUPGRADE) | + +Once both states are in `ACKUPGRADE` and `TRYUPGRADE` respectively, both sides must move to `FLUSHINGCOMPLETE` respectively by clearing their in-flight packets. Once both sides have complete flushing, a relayer may submit a `ChanUpgradeOpen` message to both ends proving that the counterparty has also completed flushing in order to move the channelEnd to `OPEN`. -`ChanUpgradeConfirm` and `ChanUpgradeOpen` may happen in any order. `ChanUpgradeOpen` is only necessary to call on chain A if the chain was not moved to `OPEN` on `ChanUpgradeAck`. +`ChanUpgradeOpen` is only necessary to call on chain A if the chain was not moved to `OPEN` on `ChanUpgradeAck` which may happen if all packets on both ends are already flushed. At the end of a successful upgrade handshake between two chains implementing the sub-protocol, the following properties hold: @@ -494,8 +512,9 @@ function chanUpgradeTry( sequence: counterpartyUpgradeSequence, } - // call blockUpgrade handshake to move channel from INITUPGRADE to TRYUPGRADE - blockUpgradeHandshake(portIdentifier, channelIdentifier, upgradeFields, counterpartyChannel, counterpartyUpgrade, TRYUPGRADE, proofChannel, proofUpgrade, proofHeight) + // call startFlushUpgrade handshake to move channel from INITUPGRADE to TRYUPGRADE and start flushing + // upgrade is blocked on this channelEnd from progressing until flush completes on both ends + startFlushUpgradeHandshake(portIdentifier, channelIdentifier, upgradeFields, counterpartyChannel, counterpartyUpgrade, TRYUPGRADE, proofChannel, proofUpgrade, proofHeight) // refresh currentChannel to get latest state currentChannel = provableStore.get(channelPath(portIdentifier, channelIdentifier)) @@ -532,6 +551,7 @@ NOTE: It is up to individual implementations how they will provide access-contro function chanUpgradeAck( portIdentifier: Identifier, channelIdentifier: Identifier, + counterpartyFlushState: FlushStatus, counterpartyUpgrade: Upgrade, proofChannel: CommitmentProof, proofUpgrade: CommitmentProof, @@ -541,6 +561,9 @@ function chanUpgradeAck( currentChannel = provableStore.get(channelPath(portIdentifier, channelIdentifier)) abortTransactionUnless(currentChannel.state == INITUPGRADE || currentChannel.state == TRYUPGRADE) + // counterparty flush status must be FLUSHING or FLUSHINGCOMPLETE + abortTransactionUnless(counterpartyFlushStatus == FLUSHING || counterpartyFlushStatus == FLUSHCOMPLETE) + connection = getConnection(currentChannel.connectionIdentifier) counterpartyHops = getCounterpartyHops(connection) @@ -554,6 +577,7 @@ function chanUpgradeAck( connectionHops: counterpartyHops, version: currentChannel.version, sequence: counterpartyUpgradeSequence, + flushState: counterpartyFlushStatus, } upgrade = provableStore.get(channelUpgradePath(portIdentifier, channelIdentifier)) @@ -565,9 +589,9 @@ function chanUpgradeAck( } } - // prove counterparty and move our own state to BLOCKUPGRADE - // we can open unilaterally once packets have been flushed - blockUpgradeHandshake(portIdentifier, channelIdentifier, upgrade.fields, counterpartyChannel, counterpartyUpgrade, BLOCKUPGRADE, proofChannel, proofUpgrade, proofHeight) + // prove counterparty and move our own state to ACKUPGRADE and start flushing + // upgrade is blocked on this channelEnd from progressing until flush completes on both ends + startFlushUpgradeHandshake(portIdentifier, channelIdentifier, upgrade.fields, counterpartyChannel, counterpartyUpgrade, ACKUPGRADE, proofChannel, proofUpgrade, proofHeight) // call modules onChanUpgradeAck callback // module can error on counterparty version @@ -589,8 +613,11 @@ function chanUpgradeAck( upgrade.version = counterpartyUpgrade.version provableStore.set(channelUpgradePath(portIdentifier, channelIdentifier), upgrade) - // if packet commitments are empty then open the upgrade handshake immediately - if pendingInflightPackets(portIdentifier, channelIdentifier) == nil { + // refresh channel + currentChannel = provableStore.get(channelPath(portIdentifier, channelIdentifier)) + + // if both sides have already flushed then open the upgrade handshake immediately + if currentChannel.state == FLUSHCOMPLETE && counterpartyFlushStatus == FLUSHCOMPLETE { openChannelHandshake(portIdentifier, channelIdentifier) module.onChanUpgradeOpen(portIdentifier, channelIdentifier) } @@ -598,7 +625,7 @@ function chanUpgradeAck( ``` ```typescript -function chanUpgradeConfirm( +function chanUpgradeOpen( portIdentifier: Identifier, channelIdentifier: Identifier, counterpartyChannelState: ChannelState, @@ -608,14 +635,15 @@ function chanUpgradeConfirm( // if packet commitments are not empty then abort the transaction abortTransactionUnless(pendingInflightPackets(portIdentifier, channelIdentifier)) - // current channel is in TRYUPGRADE + // currentChannel must be in TRYUPGRADE or ACKUPGRADE and have completed flushing currentChannel = provableStore.get(channelPath(portIdentifier, channelIdentifier)) - abortTransactionUnless(channel.state == TRYUPGRADE) + abortTransactionUnless(currentChannel.state == TRYUPGRADE || currentChannel.state == ACKUPGRADE) + abortTransactionUnless(currentChannel.flushStatus == FLUSHCOMPLETE) connection = getConnection(currentChannel.connectionIdentifier) counterpartyHops = getCounterpartyHops(connection) - // counterparty must be in OPEN or BLOCKUPGRADE state + // counterparty must be in OPEN, TRYUPGRADE, ACKUPGRADE state if counterpartyChannelState == OPEN { // get upgrade since counterparty should have upgraded to these parameters upgrade = provableStore.get(channelUpgradePath(portIdentifier, channelIdentifier)) @@ -628,16 +656,32 @@ function chanUpgradeConfirm( connectionHops: upgrade.fields.connectionHops, version: upgrade.fields.version, sequence: currentChannel.sequence, + flushStatus: NOTINFLUSH + } + } else if counterpartyChannelState == TRYUPGRADE { + // MsgUpgradeAck must already have been executed before we can OPEN + // so abort if currentState is not ACKUPGRADE + abortTransactionUnless(currentChannel.state == ACKUPGRADE) + counterpartyChannel = ChannelEnd{ + state: TRYUPGRADE, + ordering: currentChannel.ordering, + counterpartyPortIdentifier: portIdentifier, + counterpartyChannelIdentifier: channelIdentifier, + connectionHops: counterpartyHops, + version: currentChannel.version, + sequence: currentChannel.sequence, + flushStatus: FLUSHCOMPLETE } - } else if counterpartyChannelState == BLOCKUPGRADE { + } else if counterpartyChannelState == ACKUPGRADE { counterpartyChannel = ChannelEnd{ - state: BLOCKUPGRADE, + state: ACKUPGRADE, ordering: currentChannel.ordering, counterpartyPortIdentifier: portIdentifier, counterpartyChannelIdentifier: channelIdentifier, connectionHops: counterpartyHops, version: currentChannel.version, sequence: currentChannel.sequence, + flushStatus: FLUSHCOMPLETE } } else { abortTransactionUnless(false) @@ -658,33 +702,6 @@ function chanUpgradeConfirm( } ``` -```typescript -// chanUpgradeOpen can be called unilaterally on the channelEnd that is at `BLOCKUPGRADE` -// once the packets are flushed -function chanUpgradeOpen( - portIdentifier: Identifier, - channelIdentifier: Identifier, -) { - // current channel is in BLOCKUPGRADE - currentChannel = provableStore.get(channelPath(portIdentifier, channelIdentifier)) - abortTransactionUnless(channel.state == BLOCKUPGRADE) - - // if packet commitments are not empty then abort the transaction - abortTransactionUnless(pendingInflightPackets(portIdentifier, channelIdentifier)) - - // move channel to OPEN and adopt upgrade parameters - openChannelHandshake(portIdentifier, channelIdentifier) - - // call modules onChanUpgradeConfirm callback - module = lookupModule(portIdentifier) - // confirm callback must not return error since counterparty successfully upgraded - module.onChanUpgradeOpen( - portIdentifer, - channelIdentifier - ) -} -``` - ### Cancel Upgrade Process During the upgrade handshake a chain may cancel the upgrade by writing an error receipt into the upgrade error path and restoring the original channel to `OPEN`. The counterparty must then restore its channel to `OPEN` as well. A relayer can facilitate this by sending `ChannelUpgradeCancelMsg` to the handler: From a9e2662e8a939ff56c02ad01ab18f3d2673bb76c Mon Sep 17 00:00:00 2001 From: Aditya Sripal Date: Thu, 27 Apr 2023 16:18:48 +0200 Subject: [PATCH 08/19] fix cancel and timeout --- .../UPGRADES.md | 29 ++++++++++++------- 1 file changed, 19 insertions(+), 10 deletions(-) diff --git a/spec/core/ics-004-channel-and-packet-semantics/UPGRADES.md b/spec/core/ics-004-channel-and-packet-semantics/UPGRADES.md index 72efdd9ac..453badcb5 100644 --- a/spec/core/ics-004-channel-and-packet-semantics/UPGRADES.md +++ b/spec/core/ics-004-channel-and-packet-semantics/UPGRADES.md @@ -339,6 +339,10 @@ function openUpgradeHandshake( currentchannel.state = OPEN currentChannel.flushStatus = NOTINFLUSH provableStore.set(channelPath(portIdentifier, channelIdentifier), currentChannel) + + // delete auxilliary state + provableStore.delete(channelUpgradePath(portIdentifier, channelIdentifier)) + privateStore.delete(channelCounterpartyLastPacketSequencePath(portIdentifier, channelIdentifier)) } ``` @@ -357,8 +361,12 @@ function restoreChannel( } provableStore.set(channelUpgradeErrorPath(portIdentifier, channelIdentifier), errorReceipt) channel.state = OPEN + channel.flushStatus = NOTINFLUSH provableStore.set(channelPath(portIdentifier, channelIdentifier), channel) + + // delete auxilliary state provableStore.delete(channelUpgradePath(portIdentifier, channelIdentifier)) + privateStore.delete(channelCounterpartyLastPacketSequencePath(portIdentifier, channelIdentifier)) } ``` @@ -733,14 +741,15 @@ function cancelChannelUpgrade( abortTransactionUnless(verifyChannelUpgradeError(connection, proofHeight, proofUpgradeError, currentChannel.counterpartyPortIdentifier, currentChannel.counterpartyChannelIdentifier, errorReceipt)) // cancel upgrade - // and restore original conneciton + // and restore original channel // delete unnecessary state - originalChannel = privateStore.get(channelRestorePath(portIdentifier, channelIdentifier)) + currentChannel.state = OPEN + currentChannel.flushStatus = NOTINFLUSH provableStore.set(channelPath(portIdentifier, channelIdentifier), originalChannel) - // delete auxilliary upgrade state - provableStore.delete(channelUpgradeTimeoutPath(portIdentifier, channelIdentifier)) - privateStore.delete(channelRestorePath(portIdentifier, channelIdentifier)) + // delete auxilliary state + provableStore.delete(channelUpgradePath(portIdentifier, channelIdentifier)) + privateStore.delete(channelCounterpartyLastPacketSequencePath(portIdentifier, channelIdentifier)) // call modules onChanUpgradeRestore callback module = lookupModule(portIdentifier) @@ -803,12 +812,12 @@ function timeoutChannelUpgrade( } // we must restore the channel since the timeout verification has passed - originalChannel = privateStore.get(channelRestorePath(portIdentifier, channelIdentifier)) - provableStore.set(channelPath(portIdentifier, channelIdentifier), originalChannel) + currentChannel.state = OPEN + provableStore.set(channelPath(portIdentifier, channelIdentifier), currentChannel) - // delete auxilliary upgrade state - provableStore.delete(channelUpgradeTimeoutPath(portIdentifier, channelIdentifier)) - privateStore.delete(channelRestorePath(portIdentifier, channelIdentifier)) + // delete auxilliary state + provableStore.delete(channelUpgradePath(portIdentifier, channelIdentifier)) + privateStore.delete(channelCounterpartyLastPacketSequencePath(portIdentifier, channelIdentifier)) // call modules onChanUpgradeRestore callback module = lookupModule(portIdentifier) From e90b2280420cbafe7aeb203a72810a1589c9a58a Mon Sep 17 00:00:00 2001 From: Aditya Sripal Date: Thu, 27 Apr 2023 16:20:32 +0200 Subject: [PATCH 09/19] fix documentation --- spec/core/ics-004-channel-and-packet-semantics/UPGRADES.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/spec/core/ics-004-channel-and-packet-semantics/UPGRADES.md b/spec/core/ics-004-channel-and-packet-semantics/UPGRADES.md index 453badcb5..183d5d80c 100644 --- a/spec/core/ics-004-channel-and-packet-semantics/UPGRADES.md +++ b/spec/core/ics-004-channel-and-packet-semantics/UPGRADES.md @@ -346,7 +346,7 @@ function openUpgradeHandshake( } ``` -`restoreChannel` will write an error receipt, set the original channel and delete upgrade information when the executing channel needs to abort the upgrade handshake and return to the original parameters. +`restoreChannel` will write an error receipt, set the channel back to its original state and delete upgrade information when the executing channel needs to abort the upgrade handshake and return to the original parameters. ```typescript // restoreChannel signature may be modified to take a custom error @@ -767,7 +767,7 @@ It is possible for the channel upgrade process to stall indefinitely on TRYUPGRA In this case, we do not want the initializing chain to be stuck indefinitely in the `INITUPGRADE` step. Thus, the `ChannelUpgradeInitMsg` message will contain a `TimeoutHeight` and `TimeoutTimestamp`. The counterparty chain is expected to reject `ChannelUpgradeTryMsg` message if the specified timeout has already elapsed. -A relayer must then submit an `ChannelUpgradeTimeoutMsg` 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 channel and cancel the upgrade. +A relayer must then submit an `ChannelUpgradeTimeoutMsg` 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 channel to `OPEN` and cancel the upgrade. ```typescript function timeoutChannelUpgrade( From 2dfc09d7a495e3f07a90a1ed942381050e6c39c5 Mon Sep 17 00:00:00 2001 From: Aditya Date: Thu, 27 Apr 2023 16:36:58 +0200 Subject: [PATCH 10/19] Apply suggestions from code review MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: colin axnér <25233464+colin-axner@users.noreply.github.com> --- spec/core/ics-004-channel-and-packet-semantics/UPGRADES.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/spec/core/ics-004-channel-and-packet-semantics/UPGRADES.md b/spec/core/ics-004-channel-and-packet-semantics/UPGRADES.md index 183d5d80c..bd2da8078 100644 --- a/spec/core/ics-004-channel-and-packet-semantics/UPGRADES.md +++ b/spec/core/ics-004-channel-and-packet-semantics/UPGRADES.md @@ -140,7 +140,7 @@ interface ErrorReceipt { #### Upgrade Channel Path -The chain must store the proposed upgrade until the upgrade handshake successfully. This must be stored in the publice store. It may be deleted once the upgrade is successful. +The chain must store the proposed upgrade upon initiating an upgrade. The proposed upgrade must be stored in the public store. It may be deleted once the upgrade is successful or has been aborted. ```typescript function channelUpgradePath(portIdentifier: Identifier, channelIdentifier: Identifier): Path { @@ -148,7 +148,7 @@ function channelUpgradePath(portIdentifier: Identifier, channelIdentifier: Ident } ``` -The upgrade path has an associated verification membership added to the connection interface so that a counterparty may verify that chain has stored the provided counterparty upgrade. +The upgrade path has an associated verification of membership method added to the connection interface so that a counterparty may verify that chain has stored the provided counterparty upgrade. ```typescript // Connection VerifyChannelUpgrade method From a8a419c3fdde8df5e1ab6cb3c27bbd1c8aa09493 Mon Sep 17 00:00:00 2001 From: Aditya Sripal Date: Thu, 27 Apr 2023 16:38:01 +0200 Subject: [PATCH 11/19] rename subprotocol --- spec/core/ics-004-channel-and-packet-semantics/UPGRADES.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/spec/core/ics-004-channel-and-packet-semantics/UPGRADES.md b/spec/core/ics-004-channel-and-packet-semantics/UPGRADES.md index bd2da8078..603989b40 100644 --- a/spec/core/ics-004-channel-and-packet-semantics/UPGRADES.md +++ b/spec/core/ics-004-channel-and-packet-semantics/UPGRADES.md @@ -221,7 +221,7 @@ function verifyChannelUpgradeErrorAbsence( ## Sub-Protocols -The channel upgrade process consists of the following sub-protocols: `InitUpgradeHandshake`, `BlockUpgradeHandshake`, `OpenUpgradeHandshake`, `CancelChannelUpgrade`, and `TimeoutChannelUpgrade`. In the case where both chains approve of the proposed upgrade, the upgrade handshake protocol should complete successfully and the `ChannelEnd` should upgrade to the new parameters in OPEN state. +The channel upgrade process consists of the following sub-protocols: `InitUpgradeHandshake`, `StartFlushUpgradeHandshake`, `OpenUpgradeHandshake`, `CancelChannelUpgrade`, and `TimeoutChannelUpgrade`. In the case where both chains approve of the proposed upgrade, the upgrade handshake protocol should complete successfully and the `ChannelEnd` should upgrade to the new parameters in OPEN state. ### Utility Functions From 5173aaaac36dc9301efeaa74ecab4e765f8b779e Mon Sep 17 00:00:00 2001 From: Aditya Date: Wed, 3 May 2023 17:32:28 +0200 Subject: [PATCH 12/19] Apply suggestions from code review Co-authored-by: Damian Nolan --- .../ics-004-channel-and-packet-semantics/UPGRADES.md | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/spec/core/ics-004-channel-and-packet-semantics/UPGRADES.md b/spec/core/ics-004-channel-and-packet-semantics/UPGRADES.md index 603989b40..e9b04a238 100644 --- a/spec/core/ics-004-channel-and-packet-semantics/UPGRADES.md +++ b/spec/core/ics-004-channel-and-packet-semantics/UPGRADES.md @@ -140,7 +140,7 @@ interface ErrorReceipt { #### Upgrade Channel Path -The chain must store the proposed upgrade upon initiating an upgrade. The proposed upgrade must be stored in the public store. It may be deleted once the upgrade is successful or has been aborted. +The chain must store the proposed upgrade upon initiating an upgrade. The proposed upgrade must be stored in the provable store. It may be deleted once the upgrade is successful or has been aborted. ```typescript function channelUpgradePath(portIdentifier: Identifier, channelIdentifier: Identifier): Path { @@ -148,7 +148,7 @@ function channelUpgradePath(portIdentifier: Identifier, channelIdentifier: Ident } ``` -The upgrade path has an associated verification of membership method added to the connection interface so that a counterparty may verify that chain has stored the provided counterparty upgrade. +The upgrade path has an associated membership verification method added to the connection interface so that a counterparty may verify that chain has stored and committed to a particular set of upgrade parameters. ```typescript // Connection VerifyChannelUpgrade method @@ -225,7 +225,7 @@ The channel upgrade process consists of the following sub-protocols: `InitUpgrad ### Utility Functions -`initUpgradeChannelHandshake` is a subprotocol that will initialize the channel end for the upgrade handshake. It will verify the upgrade parameters and set the channel state to INITUPGRADE and block sendpackets. During this time; `receivePacket`, `acknowledgePacket` and `timeoutPacket` will still be allowed and processed according to the original channel parameters. The new proposed upgrade will be stored in the public store for counterparty verification. +`initUpgradeChannelHandshake` is a sub-protocol that will initialize the channel end for the upgrade handshake. It will validate the upgrade parameters and set the channel state to INITUPGRADE, blocking `sendPacket` from processing outbound packets on the channel end. During this time; `receivePacket`, `acknowledgePacket` and `timeoutPacket` will still be allowed and processed according to the original channel parameters. The new proposed upgrade will be stored in the provable store for counterparty verification. ```typescript function initUpgradeChannelHandshake( @@ -251,7 +251,7 @@ function initUpgradeChannelHandshake( // get last packet sent on channel and set it in the upgrade struct // last packet sent is the nextSequenceSend on the channel minus 1 - lastPacketSendSequence = publicStore.get(nextSequenceSendPath(portIdentifier, channelIdentifier)) - 1 + lastPacketSendSequence = provableStore.get(nextSequenceSendPath(portIdentifier, channelIdentifier)) - 1 upgrade = Upgrade{ fields: proposedFields, timeout: timeout, @@ -259,11 +259,11 @@ function initUpgradeChannelHandshake( } // store upgrade in public store for counterparty proof verification - publicStore.set(channelUpgradePath(portIdentifier, channelIdentifier), upgrade) + provableStore.set(channelUpgradePath(portIdentifier, channelIdentifier), upgrade) currentChannel.sequence = currentChannel.sequence + 1 currentChannel.state = INITUPGRADE - publicStore.set(channelPath(portIdentifier, channelIdentifier), channel) + provableStore.set(channelPath(portIdentifier, channelIdentifier), channel) return currentChannel.sequence } ``` From b6f7549a6c3ae257d2ed9793465ee3ea14b18119 Mon Sep 17 00:00:00 2001 From: Aditya Sripal Date: Tue, 23 May 2023 13:36:48 -0400 Subject: [PATCH 13/19] rewrite state to clarify flushing has to happen on both ends --- .../core/ics-004-channel-and-packet-semantics/UPGRADES.md | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/spec/core/ics-004-channel-and-packet-semantics/UPGRADES.md b/spec/core/ics-004-channel-and-packet-semantics/UPGRADES.md index 603989b40..b0f970133 100644 --- a/spec/core/ics-004-channel-and-packet-semantics/UPGRADES.md +++ b/spec/core/ics-004-channel-and-packet-semantics/UPGRADES.md @@ -39,11 +39,11 @@ enum ChannelState { - The chain that is proposing the upgrade should set the channel state from `OPEN` to `INITUPGRADE` - The counterparty chain that accepts the upgrade should set the channel state from `OPEN` to `TRYUPGRADE` -- Once the initiating chain verifies the counterparty is in `TRYUPGRADE`, it must move to `ACKUPGRADE` in the case where there still exist in-flight packets on its end or complete the upgrade and move to `OPEN` -- The `TRYUPGRADE` chain must prove the counterparty is in `ACKUPGRADE` or completed the upgrade in `OPEN` AND have no in-flight packets before it can complete the upgrade and move to `OPEN`. -- The `ACKUPGRADE` chain may OPEN as soon as the in-flight packets on its end have been flushed. +- Once the initiating chain verifies the counterparty is in `TRYUPGRADE`, it must move to `ACKUPGRADE` in the case where there still exist in-flight packets on **both ends** or complete the upgrade and move to `OPEN` +- The `TRYUPGRADE` chain must prove the counterparty is in `ACKUPGRADE` or completed the upgrade in `OPEN` AND have no in-flight packets on **both ends** before it can complete the upgrade and move to `OPEN`. +- The `ACKUPGRADE` chain may OPEN once in-flight packets on **both ends** have been flushed. -Both `TRYUPGRADE` and `ACKUPGRADE` are "blocking" states in that they will prevent the upgrade handshake from proceeding until the in-flight packets are flushed. The `TRYUPGRADE` state must additionally prove the counterparty state before proceeding to open, while the `ACKUPGRADE` state may move to `OPEN` unilaterally once packets are flushed on its end. +Both `TRYUPGRADE` and `ACKUPGRADE` are "blocking" states in that they will prevent the upgrade handshake from proceeding until the in-flight packets on both channel ends are flushed. The `TRYUPGRADE` state must additionally prove the counterparty state before proceeding to open, while the `ACKUPGRADE` state may move to `OPEN` unilaterally once packets are flushed on both ends. #### `ChannelEnd` From c77d4d2497f2bcd2221a75fc0ce1a382ca1c9e32 Mon Sep 17 00:00:00 2001 From: Aditya Sripal Date: Tue, 23 May 2023 16:58:20 -0400 Subject: [PATCH 14/19] move error receipt down after proof verification and typos --- .../UPGRADES.md | 47 ++++++++++--------- 1 file changed, 26 insertions(+), 21 deletions(-) diff --git a/spec/core/ics-004-channel-and-packet-semantics/UPGRADES.md b/spec/core/ics-004-channel-and-packet-semantics/UPGRADES.md index b0f970133..0bdfcff67 100644 --- a/spec/core/ics-004-channel-and-packet-semantics/UPGRADES.md +++ b/spec/core/ics-004-channel-and-packet-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 channel features without abandoning the accumulated state and network effect(s) of an already existing channel. The upgrade protocol proposed would allow chains to renegotiate an existing channel to take advantage of new features without having to create a new channel, thus preserving all existing packet state processed on the channel. +As new features get added to IBC, chains may wish to take advantage of new channel features without abandoning the accumulated state and network effect(s) of an already existing channel. The upgrade protocol proposed would allow chains to renegotiate an existing channel to take advantage of new features without having to create a new channel, thus preserving all existing packet state processed on the channel. ### Desired Properties @@ -168,7 +168,7 @@ function verifyChannelUpgrade( #### CounterpartyLastPacketSequence Path -The chain must store the counterparty's last packet sequence on `startFlushUpgradeHandshake`. This will be stored in the `counterpartyLastPacketSequence` path on the private store/ +The chain must store the counterparty's last packet sequence on `startFlushUpgradeHandshake`. This will be stored in the `counterpartyLastPacketSequence` path on the private store. ```typescript function channelCounterpartyLastPacketSequencePath(portIdentifier: Identifier, channelIdentifier: ChannelIdentifier): Path { @@ -284,13 +284,17 @@ function startFlushUpgradeHandshake( ) { abortTransactionUnless(channelState == TRYUPGRADE || channelState == ACKUPGRADE) + // current channel must be OPEN + currentChannel = provableStore.get(channelPath(portIdentifier, channelIdentifier)) + abortTransactionUnless(channel.state == OPEN) + // get underlying connection for proof verification connection = getConnection(currentChannel.connectionIdentifier) counterpartyHops = getCounterpartyHops(connection) // verify proofs of counterparty state abortTransactionUnless(verifyChannelState(connection, proofHeight, proofChannel, currentChannel.counterpartyPortIdentifier, currentChannel.counterpartyChannelIdentifier, counterpartyChannel)) - abortTransactionUnless(verifyChannelUpgrade(connection, proofHeight, proofUpgradeTimeout, currentChannel.counterpartyPortIdentifier, currentChannel.counterpartyChannelIdentifier, counterpartyUpgrade)) + abortTransactionUnless(verifyChannelUpgrade(connection, proofHeight, proofUpgrade, currentChannel.counterpartyPortIdentifier, currentChannel.counterpartyChannelIdentifier, counterpartyUpgrade)) // proposed ordering must be the same as the counterparty proposed ordering if proposedUpgradeFields.ordering != counterpartyUpgradeFields.ordering { @@ -382,7 +386,7 @@ function pendingInflightPacketSequences( ### Upgrade Handshake -The upgrade handshake defines four datagrams: *ChanUpgradeInit*, *ChanUpgradeTry*, *ChanUpgradeAck*, and *ChanUpgradeConfirm* +The upgrade handshake defines four datagrams: *ChanUpgradeInit*, *ChanUpgradeTry*, *ChanUpgradeAck*, and *ChanUpgradeOpen* A successful protocol execution flows as follows (note that all calls are made through modules per [ICS 25](../ics-025-handler-interface)): @@ -489,21 +493,6 @@ function chanUpgradeTry( abortTransactionUnless(existingUpgrade.fields == upgradeFields) } - // if the counterparty sequence is not equal to the current sequence, then either the counterparty chain is out-of-sync or - // the message is out-of-sync and we write an error receipt with our own sequence so that the counterparty can update - // their sequence as well. We must then increment our sequence so both sides start the next upgrade with a fresh sequence. - if counterpartyUpgradeSequence != channel.upgradeSequence { - // error on the higher sequence so that both chains move to a fresh sequence - maxSequence = max(counterpartySequence, channel.upgradeSequence) - errorReceipt = ErrorReceipt{ - sequence: maxSequence, - errorMsg: "" - } - provableStore.set(channelUpgradeErrorPath(portIdentifier, channelIdentifier), errorReceipt) - provableStore.set(channelUpgradeSequencePath(portIdentifier, channelIdentifier), maxSequence) - return - } - // get counterpartyHops for given connection connection = getConnection(currentChannel.connectionIdentifier) counterpartyHops = getCounterpartyHops(connection) @@ -524,6 +513,21 @@ function chanUpgradeTry( // upgrade is blocked on this channelEnd from progressing until flush completes on both ends startFlushUpgradeHandshake(portIdentifier, channelIdentifier, upgradeFields, counterpartyChannel, counterpartyUpgrade, TRYUPGRADE, proofChannel, proofUpgrade, proofHeight) + // if the counterparty sequence is not equal to the current sequence, then either the counterparty chain is out-of-sync or + // the message is out-of-sync and we write an error receipt with our own sequence so that the counterparty can update + // their sequence as well. We must then increment our sequence so both sides start the next upgrade with a fresh sequence. + if counterpartyUpgradeSequence != channel.upgradeSequence { + // error on the higher sequence so that both chains move to a fresh sequence + maxSequence = max(counterpartyUpgradeSequence, channel.upgradeSequence) + errorReceipt = ErrorReceipt{ + sequence: maxSequence, + errorMsg: "" + } + provableStore.set(channelUpgradeErrorPath(portIdentifier, channelIdentifier), errorReceipt) + provableStore.set(channelUpgradeSequencePath(portIdentifier, channelIdentifier), maxSequence) + return + } + // refresh currentChannel to get latest state currentChannel = provableStore.get(channelPath(portIdentifier, channelIdentifier)) @@ -559,7 +563,8 @@ NOTE: It is up to individual implementations how they will provide access-contro function chanUpgradeAck( portIdentifier: Identifier, channelIdentifier: Identifier, - counterpartyFlushState: FlushStatus, + counterpartyUpgradeSequence: uint64, + counterpartyFlushStatus: FlushStatus, counterpartyUpgrade: Upgrade, proofChannel: CommitmentProof, proofUpgrade: CommitmentProof, @@ -724,7 +729,7 @@ function cancelChannelUpgrade( ) { // current channel is in INITUPGRADE or TRYUPGRADE currentChannel = provableStore.get(channelPath(portIdentifier, channelIdentifier)) - abortTransactionUnless(channel.state == INITUPGRADE || channel.state == TRYUPGRADE) + abortTransactionUnless(currentChannel.state == INITUPGRADE || currentChannel.state == TRYUPGRADE) abortTransactionUnless(!isEmpty(errorReceipt)) From 9fa17558631b04a86b9ba689e379769a6052932f Mon Sep 17 00:00:00 2001 From: Aditya Sripal Date: Thu, 25 May 2023 18:20:14 -0400 Subject: [PATCH 15/19] address comments --- .../UPGRADES.md | 94 +++++++++++-------- 1 file changed, 57 insertions(+), 37 deletions(-) diff --git a/spec/core/ics-004-channel-and-packet-semantics/UPGRADES.md b/spec/core/ics-004-channel-and-packet-semantics/UPGRADES.md index 07fa91919..8042374ab 100644 --- a/spec/core/ics-004-channel-and-packet-semantics/UPGRADES.md +++ b/spec/core/ics-004-channel-and-packet-semantics/UPGRADES.md @@ -15,7 +15,7 @@ As new features get added to IBC, chains may wish to take advantage of new chann - The channel upgrade protocol is atomic, i.e., - either it is unsuccessful and then the channel MUST fall-back to the original channel parameters; - or it is successful and then both channel ends MUST adopt the new channel parameters and the applications must process packet data appropriately. -- Packets sent under the previously negotiated parameters must be processed under the previously negotiated parameters, packets sent under the newly negotiated parameters must be processed under the newly negotiated parameters. +- Packets sent under the previously negotiated parameters must be processed under the previously negotiated parameters, packets sent under the newly negotiated parameters must be processed under the newly negotiated parameters. Thus, in-flight packets sent before upgrade handshake is complete will be processed according to the original parameters. - The channel upgrade protocol MUST NOT modify the channel identifiers. ## Technical Specification @@ -87,7 +87,7 @@ interface UpgradeFields { MAY BE MODIFIED: - `version`: The version MAY be modified by the upgrade protocol. The same version negotiation that happens in the initial channel handshake can be employed for the upgrade handshake. -- `ordering`: The ordering MAY be modified by the upgrade protocol. +- `ordering`: The ordering MAY be modified by the upgrade protocol so long as the new ordering is supported by underlying connection. - `connectionHops`: The connectionHops MAY be modified by the upgrade protocol. MUST NOT BE MODIFIED: @@ -122,7 +122,7 @@ interface Upgrade { } ``` -The upgrade contains the proposed upgrade for the channel end on the executing chain, the timeout for the upgrade attempt, and the last packet send sequence for the channel. The `lastPacketSent` allows the counterparty to know which packets need to be flushed before the channel can reopen with the newly negotiated parameters. +The upgrade contains the proposed upgrade for the channel end on the executing chain, the timeout for the upgrade attempt, and the last packet send sequence for the channel. The `lastPacketSent` allows the counterparty to know which packets need to be flushed before the channel can reopen with the newly negotiated parameters. Any packet sent to the channel end with a packet sequence above the `lastPacketSent` will be rejected until the upgrade is complete. #### `ErrorReceipt` @@ -138,7 +138,7 @@ interface ErrorReceipt { ### Store Paths -#### Upgrade Channel Path +#### Channel Upgrade Path The chain must store the proposed upgrade upon initiating an upgrade. The proposed upgrade must be stored in the provable store. It may be deleted once the upgrade is successful or has been aborted. @@ -171,7 +171,7 @@ function verifyChannelUpgrade( The chain must store the counterparty's last packet sequence on `startFlushUpgradeHandshake`. This will be stored in the `counterpartyLastPacketSequence` path on the private store. ```typescript -function channelCounterpartyLastPacketSequencePath(portIdentifier: Identifier, channelIdentifier: ChannelIdentifier): Path { +function channelCounterpartyLastPacketSequencePath(portIdentifier: Identifier, channelIdentifier: Identifier): Path { return "channelUpgrades/counterpartyLastPacketSequence/ports/{portIdentifier}/channels/{channelIdentifier}" } ``` @@ -221,17 +221,21 @@ function verifyChannelUpgradeErrorAbsence( ## Sub-Protocols -The channel upgrade process consists of the following sub-protocols: `InitUpgradeHandshake`, `StartFlushUpgradeHandshake`, `OpenUpgradeHandshake`, `CancelChannelUpgrade`, and `TimeoutChannelUpgrade`. In the case where both chains approve of the proposed upgrade, the upgrade handshake protocol should complete successfully and the `ChannelEnd` should upgrade to the new parameters in OPEN state. +The channel upgrade process consists of the following sub-protocols: `initUpgradeHandshake`, `startFlushUpgradeHandshake`, `openUpgradeHandshake`, `cancelChannelUpgrade`, and `timeoutChannelUpgrade`. In the case where both chains approve of the proposed upgrade, the upgrade handshake protocol should complete successfully and the `ChannelEnd` should upgrade to the new parameters in OPEN state. ### Utility Functions -`initUpgradeChannelHandshake` is a sub-protocol that will initialize the channel end for the upgrade handshake. It will validate the upgrade parameters and set the channel state to INITUPGRADE, blocking `sendPacket` from processing outbound packets on the channel end. During this time; `receivePacket`, `acknowledgePacket` and `timeoutPacket` will still be allowed and processed according to the original channel parameters. The new proposed upgrade will be stored in the provable store for counterparty verification. +`initUpgradeHandshake` is a sub-protocol that will initialize the channel end for the upgrade handshake. It will validate the upgrade parameters and set the channel state to INITUPGRADE, blocking `sendPacket` from processing outbound packets on the channel end. During this time; `receivePacket`, `acknowledgePacket` and `timeoutPacket` will still be allowed and processed according to the original channel parameters. The new proposed upgrade will be stored in the provable store for counterparty verification. ```typescript -function initUpgradeChannelHandshake( +// initUpgradeHandshake will verify that the channel is in the correct precondition to call the initUpgradeHandshake protocol +// it will verify the new upgrade field parameters, and make the relevant state changes for initializing a new upgrade: +// - moving channel state to INITUPGRADE +// - incrementing upgrade sequence +function initUpgradeHandshake( portIdentifier: Identifier, channelIdentifier: Identifier, - proposedFields: UpgradeFields, + proposedUpgradeFields: UpgradeFields, timeout: UpgradeTimeout ): uint64 { // current channel must be OPEN @@ -239,13 +243,16 @@ function initUpgradeChannelHandshake( abortTransactionUnless(channel.state == OPEN) // new channel version must be nonempty - abortTransactionUnless(proposedFields.Version != "") + abortTransactionUnless(proposedUpgradeFields.Version != "") // proposedConnection must exist and be in OPEN state for // channel upgrade to be accepted - proposedConnection = provableStore.Get(connectionPath(proposedFields.connectionHops[0]) + proposedConnection = provableStore.Get(connectionPath(proposedUpgradeFields.connectionHops[0]) abortTransactionUnless(proposedConnection != null && proposedConnection.state == OPEN) + // new order must be supported by the new connection + abortTransactionUnless(isSupported(proposedConnection, proposedUpgradeFields.ordering)) + // either timeout height or timestamp must be non-zero abortTransactionUnless(timeout.timeoutHeight != 0 || timeout.timeoutTimestamp != 0) @@ -253,7 +260,7 @@ function initUpgradeChannelHandshake( // last packet sent is the nextSequenceSend on the channel minus 1 lastPacketSendSequence = provableStore.get(nextSequenceSendPath(portIdentifier, channelIdentifier)) - 1 upgrade = Upgrade{ - fields: proposedFields, + fields: proposedUpgradeFields, timeout: timeout, lastPacketSent: lastPacketSendSequence, } @@ -271,22 +278,26 @@ function initUpgradeChannelHandshake( `startFlushUpgradeHandshake` will set the counterparty last packet send and continue blocking the upgrade from continuing until all in-flight packets have been flushed. When the channel is in blocked mode, any packet receive above the counterparty last packet send will be rejected. It will verify the upgrade parameters and set the channel state to one of the flushing states (`TRYUPGRADE` or `ACKUPGRADE`) passed in by caller, set the `FlushStatus` to `FLUSHING` and block sendpackets. During this time; `receivePacket`, `acknowledgePacket` and `timeoutPacket` will still be allowed and processed according to the original channel parameters. The new proposed upgrade will be stored in the public store for counterparty verification. ```typescript +// startFlushUpgradeSequence will verify that the channel is in a valid precondition for calling the startFlushUpgradeHandshake +// and that the desiredChannelState is valid +// it will verify the proofs of the counterparty channel and upgrade +// it will verify that the upgrades on both ends are mutually compatible +// it will set the channel to desiredChannel state and move to flushing mode +// if flush is already complete, it will automatically set flushStatus to FLUSHCOMPLETE function startFlushUpgradeHandshake( portIdentifier: Identifier, channelIdentifier: Identifier, proposedUpgradeFields: UpgradeFields, counterpartyChannel: ChannelEnd, counterpartyUpgrade: Upgrade, - channelState: ChannelState, + desiredChannelState: ChannelState, proofChannel: CommitmentProof, proofUpgrade: CommitmentProof, proofHeight: Height ) { - abortTransactionUnless(channelState == TRYUPGRADE || channelState == ACKUPGRADE) + abortTransactionUnless(desiredChannelState == TRYUPGRADE || desiredChannelState == ACKUPGRADE) - // current channel must be OPEN currentChannel = provableStore.get(channelPath(portIdentifier, channelIdentifier)) - abortTransactionUnless(channel.state == OPEN) // get underlying connection for proof verification connection = getConnection(currentChannel.connectionIdentifier) @@ -296,6 +307,26 @@ function startFlushUpgradeHandshake( abortTransactionUnless(verifyChannelState(connection, proofHeight, proofChannel, currentChannel.counterpartyPortIdentifier, currentChannel.counterpartyChannelIdentifier, counterpartyChannel)) abortTransactionUnless(verifyChannelUpgrade(connection, proofHeight, proofUpgrade, currentChannel.counterpartyPortIdentifier, currentChannel.counterpartyChannelIdentifier, counterpartyUpgrade)) + // if the counterparty sequence is not equal to the current sequence, then either the counterparty chain is out-of-sync or + // the message is out-of-sync and we write an error receipt with our own sequence so that the counterparty can update + // their sequence as well. We must then increment our sequence so both sides start the next upgrade with a fresh sequence. + if counterpartyUpgradeSequence != channel.upgradeSequence { + // error on the higher sequence so that both chains move to a fresh sequence + maxSequence = max(counterpartyUpgradeSequence, channel.upgradeSequence) + errorReceipt = ErrorReceipt{ + sequence: maxSequence, + errorMsg: "" + } + provableStore.set(channelUpgradeErrorPath(portIdentifier, channelIdentifier), errorReceipt) + provableStore.set(channelUpgradeSequencePath(portIdentifier, channelIdentifier), maxSequence) + return + } + + // timeouts on upgrade should be the same + if upgrade.timeout != counterpartyUpgrade.timeout { + restoreChannel(portIdentifier, channelIdentifier) + } + // proposed ordering must be the same as the counterparty proposed ordering if proposedUpgradeFields.ordering != counterpartyUpgradeFields.ordering { restoreChannel(portIdentifier, channelIdentifier) @@ -310,7 +341,7 @@ function startFlushUpgradeHandshake( restoreChannel(portIdentifier, channelIdentifier) } - currentChannel.state = channelState + currentChannel.state = desiredChannelState currentChannel.flushState = FLUSHING // if there are no in-flight packets on our end, we can automatically go to FLUSHCOMPLETE @@ -327,6 +358,9 @@ function startFlushUpgradeHandshake( `openUpgradeHandshake` will open the channel and switch the existing channel parameters to the newly agreed-upon uprade channel fields. ```typescript +// openUpgradeHandshake will switch the channel fields over to the agreed upon upgrade fields +// it will reset the channel state and flushStatus to their pre-upgrade state. +// it will delete auxilliary upgrade state // caller must do all relevant checks before calling this function function openUpgradeHandshake( portIdentifier: Identifier, @@ -353,7 +387,9 @@ function openUpgradeHandshake( `restoreChannel` will write an error receipt, set the channel back to its original state and delete upgrade information when the executing channel needs to abort the upgrade handshake and return to the original parameters. ```typescript -// restoreChannel signature may be modified to take a custom error +// restoreChannel will restore the channel state and flush status to their pre-upgrade state so that upgrade is aborted +// it write an error receipt to state so counterparty can restore as well. +// NOTE: this function signature may be modified by implementors to take a custom error function restoreChannel( portIdentifier: Identifier, channelIdentifier: Identifier, @@ -513,21 +549,6 @@ function chanUpgradeTry( // upgrade is blocked on this channelEnd from progressing until flush completes on both ends startFlushUpgradeHandshake(portIdentifier, channelIdentifier, upgradeFields, counterpartyChannel, counterpartyUpgrade, TRYUPGRADE, proofChannel, proofUpgrade, proofHeight) - // if the counterparty sequence is not equal to the current sequence, then either the counterparty chain is out-of-sync or - // the message is out-of-sync and we write an error receipt with our own sequence so that the counterparty can update - // their sequence as well. We must then increment our sequence so both sides start the next upgrade with a fresh sequence. - if counterpartyUpgradeSequence != channel.upgradeSequence { - // error on the higher sequence so that both chains move to a fresh sequence - maxSequence = max(counterpartyUpgradeSequence, channel.upgradeSequence) - errorReceipt = ErrorReceipt{ - sequence: maxSequence, - errorMsg: "" - } - provableStore.set(channelUpgradeErrorPath(portIdentifier, channelIdentifier), errorReceipt) - provableStore.set(channelUpgradeSequencePath(portIdentifier, channelIdentifier), maxSequence) - return - } - // refresh currentChannel to get latest state currentChannel = provableStore.get(channelPath(portIdentifier, channelIdentifier)) @@ -563,7 +584,6 @@ NOTE: It is up to individual implementations how they will provide access-contro function chanUpgradeAck( portIdentifier: Identifier, channelIdentifier: Identifier, - counterpartyUpgradeSequence: uint64, counterpartyFlushStatus: FlushStatus, counterpartyUpgrade: Upgrade, proofChannel: CommitmentProof, @@ -581,7 +601,7 @@ function chanUpgradeAck( counterpartyHops = getCounterpartyHops(connection) // construct counterpartyChannel from existing information and provided - // counterpartyUpgradeSequence + // flushStatus counterpartyChannel = ChannelEnd{ state: TRYUPGRADE, ordering: currentChannel.ordering, @@ -589,8 +609,8 @@ function chanUpgradeAck( counterpartyChannelIdentifier: channelIdentifier, connectionHops: counterpartyHops, version: currentChannel.version, - sequence: counterpartyUpgradeSequence, - flushState: counterpartyFlushStatus, + sequence: channel.sequence, + flushStatus: counterpartyFlushStatus, } upgrade = provableStore.get(channelUpgradePath(portIdentifier, channelIdentifier)) From 43d2c63ee2c157778637a83b2e8aaf3db25c5953 Mon Sep 17 00:00:00 2001 From: Aditya Sripal Date: Thu, 25 May 2023 18:26:53 -0400 Subject: [PATCH 16/19] update docs to be clearer --- spec/core/ics-004-channel-and-packet-semantics/UPGRADES.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/spec/core/ics-004-channel-and-packet-semantics/UPGRADES.md b/spec/core/ics-004-channel-and-packet-semantics/UPGRADES.md index 8042374ab..c9228f087 100644 --- a/spec/core/ics-004-channel-and-packet-semantics/UPGRADES.md +++ b/spec/core/ics-004-channel-and-packet-semantics/UPGRADES.md @@ -39,7 +39,7 @@ enum ChannelState { - The chain that is proposing the upgrade should set the channel state from `OPEN` to `INITUPGRADE` - The counterparty chain that accepts the upgrade should set the channel state from `OPEN` to `TRYUPGRADE` -- Once the initiating chain verifies the counterparty is in `TRYUPGRADE`, it must move to `ACKUPGRADE` in the case where there still exist in-flight packets on **both ends** or complete the upgrade and move to `OPEN` +- Once the initiating chain verifies the counterparty is in `TRYUPGRADE`, it must move to `ACKUPGRADE` unless all in-flight packets are already flushed on both ends, in which case it must move directly to `OPEN`. - The `TRYUPGRADE` chain must prove the counterparty is in `ACKUPGRADE` or completed the upgrade in `OPEN` AND have no in-flight packets on **both ends** before it can complete the upgrade and move to `OPEN`. - The `ACKUPGRADE` chain may OPEN once in-flight packets on **both ends** have been flushed. From 2458b48a3c212044aae52f385d0752c371d84ef5 Mon Sep 17 00:00:00 2001 From: Aditya Date: Tue, 30 May 2023 17:28:59 -0400 Subject: [PATCH 17/19] Apply suggestions from code review MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: colin axnér <25233464+colin-axner@users.noreply.github.com> --- spec/core/ics-004-channel-and-packet-semantics/UPGRADES.md | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/spec/core/ics-004-channel-and-packet-semantics/UPGRADES.md b/spec/core/ics-004-channel-and-packet-semantics/UPGRADES.md index c9228f087..b6b244649 100644 --- a/spec/core/ics-004-channel-and-packet-semantics/UPGRADES.md +++ b/spec/core/ics-004-channel-and-packet-semantics/UPGRADES.md @@ -15,7 +15,7 @@ As new features get added to IBC, chains may wish to take advantage of new chann - The channel upgrade protocol is atomic, i.e., - either it is unsuccessful and then the channel MUST fall-back to the original channel parameters; - or it is successful and then both channel ends MUST adopt the new channel parameters and the applications must process packet data appropriately. -- Packets sent under the previously negotiated parameters must be processed under the previously negotiated parameters, packets sent under the newly negotiated parameters must be processed under the newly negotiated parameters. Thus, in-flight packets sent before upgrade handshake is complete will be processed according to the original parameters. +- Packets sent under the previously negotiated parameters must be processed under the previously negotiated parameters, packets sent under the newly negotiated parameters must be processed under the newly negotiated parameters. Thus, in-flight packets sent before the upgrade handshake is complete will be processed according to the original parameters. - The channel upgrade protocol MUST NOT modify the channel identifiers. ## Technical Specification @@ -240,7 +240,7 @@ function initUpgradeHandshake( ): uint64 { // current channel must be OPEN currentChannel = provableStore.get(channelPath(portIdentifier, channelIdentifier)) - abortTransactionUnless(channel.state == OPEN) + abortTransactionUnless(currentChannel.state == OPEN) // new channel version must be nonempty abortTransactionUnless(proposedUpgradeFields.Version != "") @@ -301,7 +301,6 @@ function startFlushUpgradeHandshake( // get underlying connection for proof verification connection = getConnection(currentChannel.connectionIdentifier) - counterpartyHops = getCounterpartyHops(connection) // verify proofs of counterparty state abortTransactionUnless(verifyChannelState(connection, proofHeight, proofChannel, currentChannel.counterpartyPortIdentifier, currentChannel.counterpartyChannelIdentifier, counterpartyChannel)) @@ -349,7 +348,7 @@ function startFlushUpgradeHandshake( currentChannel.flushState = FLUSHCOMPLETE } - publicStore.set(channelPath(portIdentifier, channelIdentifier), channel) + publicStore.set(channelPath(portIdentifier, channelIdentifier), currentChannel) privateStore.set(channelCounterpartyLastPacketSequencePath(portIdentifier, channelIdentifier), counterpartyUpgrade.lastPacketSent) } From 189a5b3fc96d849f599f38d29c6f6c8d2aaf011e Mon Sep 17 00:00:00 2001 From: Aditya Sripal Date: Wed, 31 May 2023 15:40:13 -0400 Subject: [PATCH 18/19] wrap up final requests --- .../UPGRADES.md | 32 +++++++++---------- 1 file changed, 16 insertions(+), 16 deletions(-) diff --git a/spec/core/ics-004-channel-and-packet-semantics/UPGRADES.md b/spec/core/ics-004-channel-and-packet-semantics/UPGRADES.md index b6b244649..0ade076b5 100644 --- a/spec/core/ics-004-channel-and-packet-semantics/UPGRADES.md +++ b/spec/core/ics-004-channel-and-packet-semantics/UPGRADES.md @@ -312,18 +312,11 @@ function startFlushUpgradeHandshake( if counterpartyUpgradeSequence != channel.upgradeSequence { // error on the higher sequence so that both chains move to a fresh sequence maxSequence = max(counterpartyUpgradeSequence, channel.upgradeSequence) - errorReceipt = ErrorReceipt{ - sequence: maxSequence, - errorMsg: "" - } - provableStore.set(channelUpgradeErrorPath(portIdentifier, channelIdentifier), errorReceipt) - provableStore.set(channelUpgradeSequencePath(portIdentifier, channelIdentifier), maxSequence) - return - } - - // timeouts on upgrade should be the same - if upgrade.timeout != counterpartyUpgrade.timeout { + currentChannel.UpgradeSequence = maxSequence + provableStore.set(channelPath(portIdentifier, channelIdentifier), currentChannel) + restoreChannel(portIdentifier, channelIdentifier) + return } // proposed ordering must be the same as the counterparty proposed ordering @@ -462,15 +455,21 @@ function chanUpgradeInit( // call modules onChanUpgradeInit callback module = lookupModule(portIdentifier) version, err = module.onChanUpgradeInit( - proposedUpgrade.fields.ordering, - proposedUpgrade.fields.connectionHops, portIdentifier, channelIdentifier, + proposedUpgrade.fields.ordering, + proposedUpgrade.fields.connectionHops, upgradeSequence, proposedUpgrade.fields.version ) // abort transaction if callback returned error abortTransactionUnless(err != nil) + + // replace channel version with the version returned by application + // in case it was modified + currentChannel = provableStore.get(channelPath(portIdentifier, channelIdentifier)) + currentChannel.version = version + provableStore.set(channelPath(portIdentifier, channelIdentifier), currentChannel) } ``` @@ -518,8 +517,8 @@ function chanUpgradeTry( // so that both channel ends are using the same sequence for the current upgrade // initUpgradeChannelHandshake will increment the sequence so after that call // both sides will have the same upgradeSequence - if counterpartyUpgradeSequence > channel.upgradeSequence { - channel.upgradeSequence = counterpartyUpgradeSequence - 1 + if counterpartyUpgradeSequence > currentChannel.upgradeSequence { + currentChannel.upgradeSequence = counterpartyUpgradeSequence - 1 } initUpgradeChannelHandshake(portIdentifier, channelIdentifier, upgradeFields, counterpartyUpgrade.timeout) @@ -542,6 +541,7 @@ function chanUpgradeTry( connectionHops: counterpartyHops, version: currentChannel.version, sequence: counterpartyUpgradeSequence, + flushStatus: NOTINFLUSH, } // call startFlushUpgrade handshake to move channel from INITUPGRADE to TRYUPGRADE and start flushing @@ -650,7 +650,7 @@ function chanUpgradeAck( // if both sides have already flushed then open the upgrade handshake immediately if currentChannel.state == FLUSHCOMPLETE && counterpartyFlushStatus == FLUSHCOMPLETE { - openChannelHandshake(portIdentifier, channelIdentifier) + openUpgradelHandshake(portIdentifier, channelIdentifier) module.onChanUpgradeOpen(portIdentifier, channelIdentifier) } } From ac18bb13c1563814799ae44ec6ac8960d545822c Mon Sep 17 00:00:00 2001 From: Aditya Sripal Date: Wed, 31 May 2023 15:42:03 -0400 Subject: [PATCH 19/19] add some OPEN documentation --- spec/core/ics-004-channel-and-packet-semantics/UPGRADES.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/spec/core/ics-004-channel-and-packet-semantics/UPGRADES.md b/spec/core/ics-004-channel-and-packet-semantics/UPGRADES.md index 0ade076b5..3112498d5 100644 --- a/spec/core/ics-004-channel-and-packet-semantics/UPGRADES.md +++ b/spec/core/ics-004-channel-and-packet-semantics/UPGRADES.md @@ -656,6 +656,8 @@ function chanUpgradeAck( } ``` +`chanUpgradeOpen` may only be called once both sides have moved to FLUSHCOMPLETE. If there exists unprocessed packets in the queue when the handshake goes into `FLUSHING` mode, then the packet handlers must move the channelEnd to `FLUSHCOMPLETE` once the last packet on the channelEnd has been processed. + ```typescript function chanUpgradeOpen( portIdentifier: Identifier,