04-Packet-Flow-v2 #1148
Merged
04-Packet-Flow-v2 #1148
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
AdityaSripal
suggested changes
Sep 27, 2024
There was a problem hiding this comment.
Great start!! I really like the explanations you gave for the Counterparty
A couple major tweaks as you continue working on this:
- We need to decide on a new name for Counterparty and make Channel a clear use again in the specification
- We need to not tie client id to channel id (aliasing approach)
- The ICS24 paths and commitments need to follow their new v2 specification as defined in ICS24 README and PACKET.md
| - The `nextSequenceSend`, stored separately, tracks the sequence number for the next packet to be sent. | ||
|
|
||
| ```typescript | ||
| function nextSequenceSendPath(sourceID: bytes, destID: bytes): Path { |
There was a problem hiding this comment.
This shouldn't be keyed on the string clients.
And there's no need for there to be both source and destId. Since we only need our sides' identifier to uniquely key on the channel.
It can just be
nextSequenceSend/{sourceId}
| @@ -190,10 +167,23 @@ The architecture of clients, connections, channels and packets: | |||
|
|
|||
| #### Store paths | |||
|
|
|||
| NOTE do we stil need this, or we can retrieve the sequence from the commitment path associated with the clientID? | |||
There was a problem hiding this comment.
We still need this. We want to keep a track of the nonce
| Channel structures are stored under a store path prefix unique to a combination of a port identifier and channel identifier: | ||
|
|
||
| ```typescript | ||
| function channelPath(portIdentifier: Identifier, channelIdentifier: Identifier): Path { | ||
| function channelPath(portIdentifier: bytes, channelIdentifier: bytes): Path { |
|
|
||
| Constant-size commitments to packet data fields are stored under the packet sequence number: | ||
|
|
||
| ```typescript | ||
| function packetCommitmentPath(portIdentifier: Identifier, channelIdentifier: Identifier, sequence: uint64): Path { | ||
| return "commitments/ports/{portIdentifier}/channels/{channelIdentifier}/sequences/{sequence}" | ||
| function packetCommitmentPath(sourceId: bytes, sequence: uint64): Path { |
There was a problem hiding this comment.
the sequence needs to be in bigEndian format. See ICS24 for details
| } | ||
| ``` | ||
|
|
||
| Packet acknowledgement data are stored under the `packetAcknowledgementPath`: | ||
|
|
||
| ```typescript | ||
| function packetAcknowledgementPath(portIdentifier: Identifier, channelIdentifier: Identifier, sequence: uint64): Path { | ||
| return "acks/ports/{portIdentifier}/channels/{channelIdentifier}/sequences/{sequence}" | ||
| function packetAcknowledgementPath(sourceId: bytes, sequence: uint64): Path { |
Comment on lines
343
to
344
| sourceClientId: bytes, | ||
| destClientId: bytes, |
| provableStore.set( | ||
| packetCommitmentPath(sourcePort, sourceChannel, sequence), | ||
| hash(hash(data), timeoutHeight, timeoutTimestamp) | ||
| // Note do we need to keep the channelStore? Should this be instead the counterParty store or something similar? Do we keep it for backward compatibility? |
There was a problem hiding this comment.
It should just be the provableStore
| // Note do we need to keep the channelStore? Should this be instead the counterParty store or something similar? Do we keep it for backward compatibility? | ||
| channelStore.set( | ||
| packetCommitmentPath(sourceClientId sequence), | ||
| hash(hash(data), timeoutTimestamp) |
There was a problem hiding this comment.
This is wrong, we need to use the commitment function defined in PACKET.md
| break; | ||
| } | ||
| merklePath, | ||
| hash(packet.data, packet.timeoutTimestamp) |
There was a problem hiding this comment.
need to use Commitment function defined in PACKET.md
|
|
||
| channelStore.set( | ||
| packetReceiptPath(packet.sourceChannelId, packet.sequence), | ||
| SUCCESSFUL_RECEIPT |
There was a problem hiding this comment.
Nice start @sangier !
We should also check the counterparty channel id is as expected in the packet handlers
Comment on lines
84
to
88
| An `OpaquePacket` is a packet, but cloaked in an obscuring data type by the host state machine, such that a module cannot act upon it other than to pass it to the IBC handler. The IBC handler can cast a `Packet` to an `OpaquePacket` and vice versa. | ||
|
|
||
| ```typescript | ||
| type OpaquePacket = object | ||
| ``` |
There was a problem hiding this comment.
Don't think this is necessary
There was a problem hiding this comment.
Let's remove all references to OpaquePacket and just use Packet
|
|
||
| ```typescript | ||
| interface Acknowledgement { | ||
| appAcknowledgement: [bytes] // array of bytes. Each element of the array contains an acknowledgement from a specific application |
There was a problem hiding this comment.
This is not an array of bytes, but an array of an array of bytes
byte[][] not byte[]
|
|
||
| Given a scenario where we are sending a packet from a sender chain `A` to a receiver chain `B` the protocol follows the following rules: | ||
|
|
||
| - Sender `A` can call either {`sendPacket`,`acknowledgePacket`,`timeoutPacket`} |
There was a problem hiding this comment.
Hmm don't think this is so clear since it sounds like you can call either in any order at any time
There was a problem hiding this comment.
At the beginning, Sender A can only call sendPacket
| function sendPacket( | ||
| sourceChannelId: bytes, | ||
| timeoutTimestamp: uint64, | ||
| payloads: []byte |
There was a problem hiding this comment.
This should be a list of Payload concrete type
Co-authored-by: Aditya <14364734+AdityaSripal@users.noreply.github.com>
sangier
commented
Oct 23, 2024
| // Executes Application logic ∀ Payload | ||
| payload=packet.data[0] | ||
| cbs = router.callbacks[payload.destPort] | ||
| ack,success = cbs.onReceivePacket(packet.channelDestId,payload,relayer,packet.sequence) // Note that payload includes the version. The application is required to inspect the version to route the data to the proper callback |
AdityaSripal
approved these changes
Nov 6, 2024
AdityaSripal
added a commit
that referenced
this pull request
Mar 26, 2025
* add v2 folder * create structure and paste v1 specs for now * TAO V2 support in 07 client (#1137) * deprecate delay period params * add params into clientState * fix comment * include PR link in history * fixes * fix comment * fix modified field * Copied README.md to v2/ while preserving history * Reverted README.md to commit 825d47d * fix deadlinks * rm v1 deprecation references * del v2 folder and readme * cp v1 spec into TAO_V1_README * apply v2 changes * fix dead links * fix deadlinks 2 * fix image links * fixes * fixes * fix link to v2 version --------- Co-authored-by: Stefano Angieri <stefano@interchain.io> * 02-client-TAOv2 (#1147) * rm delayPeriods * add msg and hanlder * fixes * del things * fix order * typos * review comments --------- Co-authored-by: Stefano Angieri <stefano@interchain.io> * 24-host: Reduce provable surface area to only packet flow keys (#1144) * start 24-host by removing unnecessary requirements and moving some provable keys to private keys * fix layout rendering * fix links * address Stefano's comments * high level path space and commitment specification --------- Co-authored-by: Carlos Rodriguez <carlos@interchain.io> * 04-Packet-Flow-v2 (#1148) * start removing channel refs * add counterparty refs * introducing multi-packet data structure * add multi-data packet * introduce packet handlers * send packet handl * change packet commitment paths * fix paths * mod sendPacket * mod receive * allign with new packet structure * mod ack and paths * change counterparty def * discussions-related-fixes * add sketch packet flow * rework + ack logic * fixes + timeout logic * start addressing review * addressing review comments * fixes * client-creation-registration * ante-error-post conditions sendPacket * fixes * handlers pre-error-post conditions * fixes * add mermaid diagrams, fixes * router fixes * improvements and fixes * minor fix * improvements * adding considerations * add condition table * change condition table format * multi-payload,paths,router,acks fixes * improve createClient conditions * table fix * registerChannel table * fixes * improvements * fixes * setup fixes * diagram fixes * fix diagram * test fix * two and three step setup * send-receive conditions * router fixes * createChannl conditions * register table fixes * conditions tables * table fixes * self review * self review * add soundness and correctness * self review * self review * fixes * minor fixes * rename folder * polish * change registerChannel to registerCounterparty * Apply suggestions from code review Co-authored-by: Aditya <14364734+AdityaSripal@users.noreply.github.com> * address review comments * rm counterparty channel id check in send packet * mod writeAck function * delete packet once async ack has been written * fix callbacks * fixes * fix mermaid * rm relayer from SendPack * Apply suggestions from code review --------- Co-authored-by: Aditya <14364734+AdityaSripal@users.noreply.github.com> * ics20-tao-v2-support (#1157) * start v2 changes * mod onRecv callback * mod revert,refund * wip refactor * wip-packet-reasoning * callbacks interface update * fixes * v2Tao support in v1 * Apply suggestions from code review Co-authored-by: Aditya <14364734+AdityaSripal@users.noreply.github.com> * fix sequence type * unrmarshal utility function * fix write ack * fix revertInFlightsChanges * fixes * rm relayer from onSend * Apply suggestions from code review --------- Co-authored-by: Aditya <14364734+AdityaSripal@users.noreply.github.com> * ICS24: Restructure provable packet keys (#1155) * imp: note that commitments must be lexographically ordered to maintain soundness (#1153) * restructure ibc keys * add value instead of redundant provable store column * chore: remove myself as codeowner (#1156) * Fix variable name (#1162) * fix link and put new provable keys into 04-channel spec --------- Co-authored-by: colin axnér <25233464+colin-axner@users.noreply.github.com> Co-authored-by: Christoph Otter <chipshort@tutanota.com> * add timeoutTimestamp in forwarding (#1163) * Packet Spec: Hash App data rather than standardizing encoding (#1152) * add packet and acknowledgement structs and commitment details * add CBOR as suggested encoding and SENTINEL_ACKNOWLEDGEMENT value * explain each field in the code * change id to channel * switch cbor encoding to hashing in packet commitment * switch cbor encoding to hashing in acknowledgement * Apply suggestions from code review Co-authored-by: sangier <45793271+sangier@users.noreply.github.com> * add recursive hashing suggestion from amulet * prepend protocol byte --------- Co-authored-by: sangier <45793271+sangier@users.noreply.github.com> * Move PACKET.md (#1166) * move PACKET.md to the right folder * linter * linter * ICS4: Create 32 byte commitment (#1168) * move protocol version inside final hash * Update spec/core/v2/ics-004-packet-semantics/PACKET.md Co-authored-by: DimitrisJim <d.f.hilliard@gmail.com> --------- Co-authored-by: DimitrisJim <d.f.hilliard@gmail.com> * ICS4: Event Emission Specification (NO PSEUDOCODE) (#1172) * event specification * specify events for channel creation * add client id to register counterparty events * rename emitLogEntry to emitEvents * address reviews * fix typo * remove explicit event implementation and include event writeup on what should be included * continue changes * add packet handler to v2 spec * move to v2 folder in top-level spec * simplify the spec * rename folder for more visibility * specify client handler specification * cleanup ICS-2 and start with ICS24 * ics5 port specification * halfway through ICS26 * specify packet callbacks * move folders to core * fix table * write overview doc * add link to payload docs * header corrections * address marius reviews and lint lists * revert non-core changes * revert client changes * satisfy linter * fix links * fix more linting issues * lint --------- Co-authored-by: sangier <45793271+sangier@users.noreply.github.com> Co-authored-by: Stefano Angieri <stefano@interchain.io> Co-authored-by: Carlos Rodriguez <carlos@interchain.io> Co-authored-by: colin axnér <25233464+colin-axner@users.noreply.github.com> Co-authored-by: Christoph Otter <chipshort@tutanota.com> Co-authored-by: DimitrisJim <d.f.hilliard@gmail.com>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR introduces the draft specification for the IBC version 2 ICS-04: Channel and Packet Semantics.
It formalizes the semantics for channel creation, counterparty registration, and packet flow management in IBC version 2 taking into account all the v2 spec changes.
Closes #1140