FEATURE: IBC Version 2 Core Specification #1143
Merged
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
* 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>
* rm delayPeriods * add msg and hanlder * fixes * del things * fix order * typos * review comments --------- Co-authored-by: Stefano Angieri <stefano@interchain.io>
* 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>
* 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>
* 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>
* 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 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 to the right folder * linter * linter
DimitrisJim
reviewed
Nov 13, 2024
* 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>
* 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
mpoke
reviewed
Mar 24, 2025
|
|
||
| `Path` is as defined above. `Value` is an arbitrary bytestring encoding of a particular data structure. The specific Path and Values required to be written to the provable store are defined in [ICS-4](../ics-004-channel-and-packet-semantics/PACKET.md). | ||
|
|
||
| These functions MUST be permissioned to the IBC handler module (the implementation of which is described in [ICS-4](../ics-004-channel-and-packet-semantics/PACKET_HANDLER.md)) only, so only the IBC handler module can `set` or `delete` the paths that can be read by `get`. |
There was a problem hiding this comment.
Is the IBC handler module the same as the IBC packet handler?
| type verifyNonMembership = (ClientState, Height, CommitmentProof, Path) => boolean | ||
| ``` | ||
|
|
||
| ProofVerification Inputs: |
There was a problem hiding this comment.
This seems out of scope here in the section "Implementation strategies".
Comment on lines
363
to
367
| CreateClient Inputs: | ||
| `clientType: string`: This is the client-type that references a particular light client implementation on the chain. The `CreateClient` message will create a new instance of the given client-type. | ||
| `ClientState: bytes`: This is the opaque client state as defined for the given client type. It will contain any parameters needed for verifying client updates and proof verification against a `ConsensusState`. The `ClientState` parameterizes the security model as implemented by the client type. | ||
| `ConsensusState: bytes`: This is the opaque consensus state as defined for the given client type. It is the initial consensus state provided and MUST be capable of being used by the `ValidityPredicate` to add new `ConsensusState`s to the client. The initial `ConsensusState` MAY also be used for proof verification but it is not necessary. | ||
| `Height: Number`: This is the height that is associated with the initial consensus state. |
|
|
||
| UpdateClient ErrorConditions: | ||
| - The trusted `ConsensusState` referenced in the `ClientMessage` does not exist in state | ||
| - `ValidityPredicate(clientState, trustedConsensusState, trustedHeight)` returns an error |
There was a problem hiding this comment.
nit: to keep consistency with MisbehaviourPredicate, this should return false
| `ConsensusState: bytes`: This is the opaque consensus state as defined for the given client type. It is the initial consensus state provided and MUST be capable of being used by the `ValidityPredicate` to add new `ConsensusState`s to the client. The initial `ConsensusState` MAY also be used for proof verification but it is not necessary. | ||
| `Height: Number`: This is the height that is associated with the initial consensus state. | ||
|
|
||
| CreateClient Preconditions: |
There was a problem hiding this comment.
Shouldn't the validity conditions for ClientState, ConsensusState, and Height be stated as preconditions?
There was a problem hiding this comment.
I consider preconditions things that are true about the statemachine before the message is submitted for the message to be considered valid
There was a problem hiding this comment.
So the validity of the message inputs should be in error conditions as they currently are. I will write some meta doc explaining the pre/error/post conditions eventually
| Modules must expose the following function signatures to the routing module, which are called upon the receipt of various datagrams: | ||
|
|
||
|
|
||
| ### OnRecvPacket |
There was a problem hiding this comment.
Suggested change
| ### OnRecvPacket | |
| #### OnRecvPacket |
|
|
||
| OnRecvPacket Postconditions: | ||
| - The application has executed all app-specific logic for the given payload and made the appropriate state changes | ||
| - The application returns an app acknowledgment `ack: bytes` to the core IBC handler to be written as an acknowledgement of the payload in this packet. |
| - If the acknowledgement was the `SENTINEL_ERROR_ACKNOWLEDGEMENT`, this will usually involve reverting whatever application state changes were made during `SendPacket` (e.g. unescrowing tokens for transfer) | ||
|
|
||
| OnAcknowledgement Errorconditions: | ||
| - Application specific errors may occur while processing the acknowledgement. The packet lifecycle is already complete. Implementations MAY choose to allow retries or not. |
There was a problem hiding this comment.
This is not clear -- retries of the ack or of the packet?
|
|
||
| OnTimeoutPacket Preconditions: | ||
| - This application had previously sent the provided payload in a packet with the provided `sourceClientId` and `sequence`. | ||
| - All IBC/TAO verification checks have already been authenticated by IBC core handler. Thus, when the application receives an acknowledgement; it can be guaranteed of its authenticity and need only perform the relevant application logic for the given acknowledgement and payload. |
There was a problem hiding this comment.
Suggested change
| - All IBC/TAO verification checks have already been authenticated by IBC core handler. Thus, when the application receives an acknowledgement; it can be guaranteed of its authenticity and need only perform the relevant application logic for the given acknowledgement and payload. | |
| - All IBC/TAO verification checks have already been authenticated by IBC core handler. Thus, when the application receives a timeout, it can be guaranteed of its authenticity and need only perform the relevant application logic for the given payload. |
| - The application has executed all app-specific logic for the given payload and made the appropriate state changes. This will usually involve reverting whatever application state changes were made during `SendPacket` (e.g. unescrowing tokens for transfer) | ||
|
|
||
| OnTimeoutPacket Errorconditions: | ||
| - Application specific errors may occur while processing the acknowledgement. The packet lifecycle is already complete. Implementations MAY choose to allow retries or not. |
mpoke
reviewed
Mar 24, 2025
There was a problem hiding this comment.
Completed the review of IBC v2 core. Great work @AdityaSripal
|
|
||
| ```typescript | ||
| func acknowledgementPath(packet: Packet) { | ||
| return packet.destClientId + byte(0x02) + bigEndian(packet.Sequence) |
There was a problem hiding this comment.
Not consistent with ICS 24, i.e.,
| Value | Path format |
| -------------------------- | -------------------------------------------- |
| Packet Commitment | {sourceClientId}0x1{bigEndianUint64Sequence} |
| Packet Receipt | {destClientId}0x2{bigEndianUint64Sequence} |
| Acknowledgement Commitment | {destClientId}0x3{bigEndianUint64Sequence} ||
|
||
| ## Packet Receipt V2 | ||
|
|
||
| A packet receipt will only tell the sending chain that the counterparty has successfully received the packet. Thus we just need a provable boolean flag uniquely associated with the sent packet. Thus, the receiver chain stores the packet receipt keyed on the destination identifier and the sequence to uniquely identify the packet. |
There was a problem hiding this comment.
Why is the packet receipt needed given that there is an ack? Is it for async acks?
| `proofHeight: Number`: This is the height of the counterparty chain from which the proof was generated. A corresponding consensus state for this height must exist on the source client for the proof to verify correctly. | ||
|
|
||
| AcknowledgePacket Preconditions: | ||
| - A valid client exists on the sending chain with the `sourceClientId` |
There was a problem hiding this comment.
Suggested change
| - A valid client exists on the sending chain with the `sourceClientId` | |
| - A valid client exists on the sending chain with the `packet.sourceClientId` |
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.
The following feature branch will house the new version 2 (Eureka) specification until it is ready to merge into main.
See the eureka document in main for an overview of the changes being made