From e385e8db2dbee06941351191f2a5dc6f3129426e Mon Sep 17 00:00:00 2001 From: Derek Date: Fri, 28 Jul 2023 16:55:43 -0700 Subject: [PATCH] wip address latest PR feedback --- .../README.md | 259 +++++++++++------- spec/core/ics-033-multi-hop/README.md | 19 +- 2 files changed, 177 insertions(+), 101 deletions(-) diff --git a/spec/core/ics-004-channel-and-packet-semantics/README.md b/spec/core/ics-004-channel-and-packet-semantics/README.md index fbe40feff..67dd8ffe0 100644 --- a/spec/core/ics-004-channel-and-packet-semantics/README.md +++ b/spec/core/ics-004-channel-and-packet-semantics/README.md @@ -289,6 +289,7 @@ If not provided, the default `validateChannelIdentifier` function will always re | --------- | ---------------- | ---------------- | ------------------ | ---------------------- | | Actor | ChanCloseInit | A | (OPEN, OPEN) | (CLOSED, OPEN) | | Relayer | ChanCloseConfirm | B | (CLOSED, OPEN) | (CLOSED, CLOSED) | +| Actor | ChanCloseFrozen | A or B | (OPEN, OPEN) | (CLOSED, CLOSED) | ##### Opening handshake @@ -373,13 +374,8 @@ function chanOpenTry( abortTransactionUnless(connection !== null) abortTransactionUnless(connection.state === OPEN) - let counterpartyHops: []string = [] - if (connectionHops.length > 1) { - counterpartyHops = getCounterPartyHops(proofInit, connection) - } else { - counterpartyHops = [connection.counterpartyConnectionIdentifier] - } - + // return hops from counterparty's view + counterpartyHops = getCounterPartyHops(proofInit, connection) expected = ChannelEnd{ INIT, order, portIdentifier, @@ -388,7 +384,7 @@ function chanOpenTry( } if (connectionHops.length > 1) { - key = host.ChannelPath(counterparty.PortId, counterparty.ChannelId) + key = channelPath(counterparty.PortId, counterparty.ChannelId) abortTransactionUnless(connection.verifyMultihopMembership( connection, proofHeight, @@ -436,18 +432,14 @@ function chanOpenAck( abortTransactionUnless(connection !== null) abortTransactionUnless(connection.state === OPEN) - let counterpartyHops: []string = [] - if (channel.connectionHops.length > 1) { - counterpartyHops = getCounterPartyHops(proofTry, connection) - } else { - counterpartyHops = [connection.counterpartyConnectionIdentifier] - } + // return hops from counterparty's view + counterpartyHops = getCounterPartyHops(proofTry, connection) expected = ChannelEnd{TRYOPEN, channel.order, portIdentifier, channelIdentifier, counterpartyHops, counterpartyVersion} if (channel.connectionHops.length > 1) { - key = host.ChannelPath(counterparty.PortId, counterparty.ChannelId) + key = channelPath(counterparty.PortId, counterparty.ChannelId) abortTransactionUnless(connection.verifyMultihopMembership( connection, proofHeight, @@ -490,18 +482,14 @@ function chanOpenConfirm( abortTransactionUnless(connection !== null) abortTransactionUnless(connection.state === OPEN) - let counterpartyHops: []string = [] - if (connectionHops.length > 1) { - counterpartyHops = getCounterPartyHops(proofAck, connection) - } else { - counterpartyHops = [connection.counterpartyConnectionIdentifier] - } + // return hops from counterparty's view + counterpartyHops = getCounterPartyHops(proofAck, connection) expected = ChannelEnd{OPEN, channel.order, portIdentifier, channelIdentifier, counterpartyHops, channel.version} if (connectionHops.length > 1) { - key = host.ChannelPath(counterparty.PortId, counterparty.ChannelId) + key = channelPath(counterparty.PortId, counterparty.ChannelId) abortTransactionUnless(connection.verifyMultihopMembership( connection, proofHeight, @@ -565,7 +553,7 @@ maximum height/time were mandated & tracked, and future specification versions m function chanCloseConfirm( portIdentifier: Identifier, channelIdentifier: Identifier, - proofInit: CommitmentProof, + proofInit: CommitmentProof | MultihopProof, proofHeight: Height) { abortTransactionUnless(authenticateCapability(channelCapabilityPath(portIdentifier, channelIdentifier), capability)) channel = provableStore.get(channelPath(portIdentifier, channelIdentifier)) @@ -575,19 +563,14 @@ function chanCloseConfirm( abortTransactionUnless(connection !== null) abortTransactionUnless(connection.state === OPEN) - let counterpartyHops: []string = [] - if (connectionHops.length > 1) { - counterpartyHops = getCounterPartyHops(proofInit, connection) - } else { - counterpartyHops = [connection.counterpartyConnectionIdentifier] - } + // return hops from counterparty's view + counterpartyHops = getCounterPartyHops(proofInit, connection) expected = ChannelEnd{CLOSED, channel.order, portIdentifier, channelIdentifier, counterpartyHops, channel.version} if (connectionHops.length > 1) { - key = host.ChannelPath(counterparty.PortId, counterparty.ChannelId) - + key = channelPath(counterparty.PortId, counterparty.ChannelId) abortTransactionUnless(connection.verifyMultihopMembership( connection, proofHeight, @@ -645,7 +628,7 @@ function chanCloseFrozen( let connectionIdx = proofConnection.ConsensusProofs.length + 1 abortTransactionUnless(connectionIdx < hopsLength) let connectionID = channel.ConnectionHops[connectionIdx] - let connectionProofKey = host.ConnectionPath(connectionID) + let connectionProofKey = connectionPath(connectionID) let connectionProofValue = mProof.KeyProof.Value let frozenConnectionEnd = abortTransactionUnless(Unmarshal(connectionProofValue)) @@ -666,7 +649,7 @@ function chanCloseFrozen( // key and value for the frozen client state - let clientStateKey = host.FullClientStatePath(clientID) + let clientStateKey = clientStatePath(clientID) let clientStateValue = proofClientState.KeyProof.Value let frozenClientState = abortTransactionUnless(Unmarshal(clientStateValue)) @@ -691,18 +674,18 @@ function chanCloseFrozen( ```typescript // Return the counterparty connectionHops -function getCounterPartyHops(proof: MultihopProof, lastConnection: ConnectionEnd) string[] { - let counterpartyHops: string[] = [] - for connData in proofs.ConnectionProofs { - connectionEnd = abortTransactionUnless(Unmarshal(connData.Value)) - counterpartyHops.push(connectionEnd.GetCounterparty().GetConnectionID()) - } +function getCounterPartyHops(proof: CommitmentProof | MultihopProof, lastConnection: ConnectionEnd) string[] { - // reverse the hops so they are ordered from sender --> receiver - counterpartyHops = counterpartyHops.reverse() + let counterpartyHops: string[] = [lastConnection.counterpartyConnectionIdentifier] + if typeof(proof) === 'MultihopProof' { + for connData in proofs.ConnectionProofs { + connectionEnd = abortTransactionUnless(Unmarshal(connData.Value)) + counterpartyHops.push(connectionEnd.GetCounterparty().GetConnectionID()) + } - // add the counterparty of the connection on the receiving chain - counterpartyHops.push(lastConnection.GetCounterparty().GetConnectionID()) + // reverse the hops so they are ordered from sender --> receiver + counterpartyHops = counterpartyHops.reverse() + } return counterpartyHops } @@ -841,7 +824,7 @@ function recvPacket( abortTransactionUnless(connection.state === OPEN) if (len(channel.connectionHops) > 1) { - key = host.PacketCommitmentPath(packet.GetSourcePort(), packet.GetSourceChannel(), packet.GetSequence()) + key = packetCommitmentPath(packet.GetSourcePort(), packet.GetSourceChannel(), packet.GetSequence()) abortTransactionUnless(connection.verifyMultihopMembership( connection, proofHeight, @@ -1048,7 +1031,7 @@ function acknowledgePacket( // abort transaction unless correct acknowledgement on counterparty chain if (len(channel.connectionHops) > 1) { - key = host.PacketAcknowledgementPath(packet.GetDestPort(), packet.GetDestChannel(), packet.GetSequence()) + key = packetAcknowledgementPath(packet.GetDestPort(), packet.GetDestChannel(), packet.GetSequence()) abortTransactionUnless(connection.verifyMultihopMembership( connection, proofHeight, @@ -1153,7 +1136,7 @@ We pass the `relayer` address just as in [Receiving packets](#receiving-packets) ```typescript function timeoutPacket( packet: OpaquePacket, - proof: CommitmentProof | MultihopProof, // TODO: update with multihop proof logic + proof: CommitmentProof | MultihopProof, proofHeight: Height, nextSequenceRecv: Maybe, relayer: string): Packet { @@ -1165,13 +1148,24 @@ function timeoutPacket( abortTransactionUnless(packet.destChannel === channel.counterpartyChannelIdentifier) connection = provableStore.get(connectionPath(channel.connectionHops[0])) + abortTransactionUnless(connection !== null) + // note: the connection may have been closed abortTransactionUnless(packet.destPort === channel.counterpartyPortIdentifier) + // get the timestamp from the final consensus state in the channel path + var proofTimestamp + if (channel.connectionHops.length > 1) { + consensusState = abortTransactionUnless(Unmarshal(proof.ConsensusProofs[proof.ConsensusProofs.length-1].Value)) + proofTimestamp = consensusState.GetTimestamp() + } else { + proofTimestamp, err = connection.getTimestampAtHeight(connection, proofHeight) + } + // check that timeout height or timeout timestamp has passed on the other end abortTransactionUnless( (packet.timeoutHeight > 0 && proofHeight >= packet.timeoutHeight) || - (packet.timeoutTimestamp > 0 && getTimestampAtHeight(connection, proofHeight) >= packet.timeoutTimestamp)) + (packet.timeoutTimestamp > 0 && proofTimestamp >= packet.timeoutTimestamp)) // verify we actually sent this packet, check the store abortTransactionUnless(provableStore.get(packetCommitmentPath(packet.sourcePort, packet.sourceChannel, packet.sequence)) @@ -1183,25 +1177,49 @@ function timeoutPacket( // only allow timeout on next sequence so all sequences before the timed out packet are processed (received/timed out) // before this packet times out abortTransactionUnless(nextSequenceRecv == packet.sequence) + // ordered channel: check that the recv sequence is as claimed - abortTransactionUnless(connection.verifyNextSequenceRecv( - proofHeight, - proof, - packet.destPort, - packet.destChannel, - nextSequenceRecv - )) + if (channel.connectionHops.length > 1) { + key = nextSequenceRecvPath(packet.srcPort, packet.srcChannel) + abortTransactionUnless(connection.verifyMultihopMembership( + connection, + proofHeight, + proof, + channel.ConnectionHops, + key, + nextSequenceRecv + )) + } else { + abortTransactionUnless(connection.verifyNextSequenceRecv( + proofHeight, + proof, + packet.destPort, + packet.destChannel, + nextSequenceRecv + )) + } break; case UNORDERED: - // unordered channel: verify absence of receipt at packet index - abortTransactionUnless(connection.verifyPacketReceiptAbsence( - proofHeight, - proof, - packet.destPort, - packet.destChannel, - packet.sequence - )) + if (channel.connectionHops.length > 1) { + key = packetReceiptPath(packet.srcPort, packet.srcChannel, packet.sequence) + abortTransactionUnless(connection.verifyMultihopNonMembership( + connection, + proofHeight, + proof, + channel.ConnectionHops, + key + )) + } else { + // unordered channel: verify absence of receipt at packet index + abortTransactionUnless(connection.verifyPacketReceiptAbsence( + proofHeight, + proof, + packet.destPort, + packet.destChannel, + packet.sequence + )) + } break; // NOTE: For ORDERED_ALLOW_TIMEOUT, the relayer must first attempt the receive on the destination chain @@ -1211,14 +1229,26 @@ function timeoutPacket( // only allow timeout on next sequence so all sequences before the timed out packet are processed (received/timed out) // before this packet times out abortTransactionUnless(nextSequenceRecv == packet.sequence) - abortTransactionUnless(connection.verifyPacketReceipt( - proofHeight, - proof, - packet.destPort, - packet.destChannel, - packet.sequence - TIMEOUT_RECEIPT, - )) + + if (channel.connectionHops.length > 1) { + abortTransactionUnless(connection.verifyMultihopMembership( + connection, + proofHeight, + proof, + channel.ConnectionHops, + packetReceiptPath(packet.destPort, packet.destChannel, packet.sequence), + TIMEOUT_RECEIPT + )) + } else { + abortTransactionUnless(connection.verifyPacketReceipt( + proofHeight, + proof, + packet.destPort, + packet.destChannel, + packet.sequence + TIMEOUT_RECEIPT, + )) + } break; default: @@ -1277,8 +1307,8 @@ We pass the `relayer` address just as in [Receiving packets](#receiving-packets) ```typescript function timeoutOnClose( packet: Packet, - proof: CommitmentProof | MultihopProof, // TODO: update with multihop proof logic - proofClosed: CommitmentProof | MultihopProof, // TODO: update with multihop proof logic + proof: CommitmentProof | MultihopProof, + proofClosed: CommitmentProof | MultihopProof, proofHeight: Height, nextSequenceRecv: Maybe, relayer: string): Packet { @@ -1296,37 +1326,78 @@ function timeoutOnClose( abortTransactionUnless(provableStore.get(packetCommitmentPath(packet.sourcePort, packet.sourceChannel, packet.sequence)) === hash(packet.data, packet.timeoutHeight, packet.timeoutTimestamp)) + // return hops from counterparty's view + counterpartyHops = getCounterpartyHops(proof, connection) + // check that the opposing channel end has closed expected = ChannelEnd{CLOSED, channel.order, channel.portIdentifier, - channel.channelIdentifier, channel.connectionHops.reverse(), channel.version} - abortTransactionUnless(connection.verifyChannelState( - proofHeight, - proofClosed, - channel.counterpartyPortIdentifier, - channel.counterpartyChannelIdentifier, - expected - )) + channel.channelIdentifier, counterpartyHops, channel.version} - if channel.order === ORDERED || channel.order == ORDERED_ALLOW_TIMEOUT { - // ordered channel: check that the recv sequence is as claimed - abortTransactionUnless(connection.verifyNextSequenceRecv( + // verify channel is closed + if (channel.connectionHops.length > 1) { + key = channelPath(counterparty.PortId, counterparty.ChannelId) + abortTransactionUnless(connection.VerifyMultihopMembership( + connection, proofHeight, - proof, - packet.destPort, - packet.destChannel, - nextSequenceRecv + proofClosed, + channel.ConnectionHops, + key, + expected + )) + } else { + abortTransactionUnless(connection.verifyChannelState( + proofHeight, + proofClosed, + channel.counterpartyPortIdentifier, + channel.counterpartyChannelIdentifier, + expected )) + } + + if channel.order === ORDERED || channel.order == ORDERED_ALLOW_TIMEOUT { + // ordered channel: check that packet has not been received abortTransactionUnless(nextSequenceRecv <= packet.sequence) + + // ordered channel: check that the recv sequence is as claimed + if (channel.connectionHops.length > 1) { + key = nextSequenceRecvPath(packet.destPort, packet.destChannel) + abortTransactionUnless(connection.verifyMultihopMembership( + connection, + proofHeight, + proof, + channel.ConnectionHops, + key, + nextSequenceRecv + )) + } else { + abortTransactionUnless(connection.verifyNextSequenceRecv( + proofHeight, + proof, + packet.destPort, + packet.destChannel, + nextSequenceRecv + )) + } } else // unordered channel: verify absence of receipt at packet index - abortTransactionUnless(connection.verifyPacketReceiptAbsence( - proofHeight, - proof, - packet.destPort, - packet.destChannel, - packet.sequence - )) + if (channel.connectionHops.length > 1) { + abortTransactionUnless(connection.verifyMultihopNonMembership( + connection, + proofHeight, + proof, + channel.ConnectionHops, + key + )) + } else { + abortTransactionUnless(connection.verifyPacketReceiptAbsence( + proofHeight, + proof, + packet.destPort, + packet.destChannel, + packet.sequence + )) + } // all assertions passed, we can alter state diff --git a/spec/core/ics-033-multi-hop/README.md b/spec/core/ics-033-multi-hop/README.md index 006c2c5aa..160e1b763 100644 --- a/spec/core/ics-033-multi-hop/README.md +++ b/spec/core/ics-033-multi-hop/README.md @@ -12,7 +12,7 @@ modified: 2022-12-16 ## Synopsis -This document describes a standard for multi-hop IBC channels. Multi-hop channels specifies a way to route messages across a path of IBC enabled blockchains utilizing multiple pre-existing IBC connections. +This document describes a standard for multi-hop IBC channels. Multi-hop channels specify a way to route messages across a path of IBC enabled blockchains utilizing multiple pre-existing IBC connections. ### Motivation @@ -33,22 +33,27 @@ Associated definitions are as defined in referenced prior standards (where the f ### Desired Properties - IBC channel handshake and message packets should be able to be utilize pre-existing connections to form a logical proof chain to relay messages between unconnected chains. -- Relaying for a multi connection IBC channel should NOT require additional writes to intermediate hops. +- Relaying for a multi-hop IBC channel should NOT require writing additional channel, packet, or timeout state to intermediate hops. +- The design should strive to minimize the number of required client updates to generate and query multi-hop proofs. - Minimal additional required state and changes to core and app IBC specs. - Retain desired properties of connection, channel and packet definitions. - Retain backwards compatibility for messaging over a single connection hop. ## Technical Specification -The bulk of the spec will be around proof generation and verification. IBC connections remain unchanged. Additionally, channel handshake and packet message types as well as general round trip messaging semantics and flow will remain the same. There is additional work on the verifier side on the receiving chain as well as the relayers who need to query for proofs. +The bulk of the spec describes multi-hop proof generation and verification. IBC connections remain unchanged. Additionally, channel handshake and packet message types as well as general round trip messaging semantics and flow will remain the same. There is additional work on the verifier side on the receiving chain as well as the relayers who need to query for proofs. -Messages passed over multiple hops require proof of the connection path from source chain to receiving chain as well as the packet commitment on the source chain. The connection path is proven by verifying the connection state and consensus state of each connection in the path to the receiving chain. On a high level, this can be thought of as a channel path proof chain where the receiving chain can prove a key/value on the source chain by iteratively proving each connection and consensus state in the channel path starting with the consensus state associated with the final client on the receiving chain. Each subsequent consensus state and connection is proven until the source chain's consensus state is proven which can then be used to prove the desired key/value on the source chain. +Messages passed over multiple hops require proof of the connection path from source chain to receiving chain as well as the packet commitment on the source chain. The connection path is proven by verifying the connection state and consensus state of each connection in the path to the receiving chain. On a high level, this can be thought of as a chained proof over a channel path which enables the receiving chain to prove a key/value on the source chain by iteratively proving each connection and consensus state in the channel path starting with the consensus state associated with the final client on the receiving chain. Each subsequent consensus state and connection is proven until the source chain's consensus state is proven which can then be used to prove the desired key/value on the source chain. + +## Assumptions + +This multi-hop spec assumes that all proof specs for membership verification for each chain are equal. ### Channel Handshake and Packet Messages -For both channel handshake and packet messages, additional connection hops are defined in the pre-existing `connectionHops` field. The connection IDs along the channel path must be pre-existing and in the `OPEN` state to guarantee delivery to the correct recipient. See `Path Forgery Protection` for more info. +For both channel handshake and packet messages, additional connection hops are defined in the pre-existing `connectionHops` field. The connections along the channel path must exist in the `OPEN` state to guarantee delivery to the correct recipient. See `Path Forgery Protection` for more information. -The spec for channel handshakes and packets remains the same. See [ICS 4](https://github.com/cosmos/ibc/tree/main/spec/core/ics-004-channel-and-packet-semantics). +See [ICS 4](https://github.com/cosmos/ibc/tree/main/spec/core/ics-004-channel-and-packet-semantics) for multi-hop related spec changes. Multi-hop does not change existing spec behavior for channel handshakes, packet delivery, and timeout handling. However, multi-hop channels require special handling for frozen clients (see `chanCloseFrozen`). In terms of connection topology, a user would be able to determine a viable channel path from sender -> receiver using information from the [chain registry](https://github.com/cosmos/chain-registry). They can also independently verify this information via network queries. @@ -62,7 +67,7 @@ For each multi-hop channel (detailed proof logic below): 2. Read the connectionHops field in from the scanned message to determine the channel path. 3. Lookup connection endpoints via chain registry configuration and update the clients associated with the connections in the channel path to reflect the latest consensus state on the sending chain including the key/value to be proven. 4. Query for proof of connection, and consensus state for each intermediate connection in the channel path. -5. Query proof of packet commitment or handshake message commitment on source chain. +5. Query proof of packet or handshake message commitments on source chain. 6. Submit proofs and data to RPC endpoint on receiving chain. Relayers are connection topology aware with configurations sourced from the [chain registry](https://github.com/cosmos/chain-registry).