ibc icon indicating copy to clipboard operation
ibc copied to clipboard

Add Packet and Acknowledgement spec to ICS4 along with Commitments

Open AdityaSripal opened this issue 1 year ago • 3 comments

Note to reviewers:

We need feedback on the packet structure and acknowledgement structure to see if it is sufficient for the use cases and platforms people have in mind but we also need feedback on the commitment structure.

The desired properties of the commitments:

  • Canonical (Every compliant implementation should be able to take the same Packet/Acknowledgement and reach the exact same commitment)
  • Secure (Two different packets MUST NOT create the same packet commitment)
  • Implementable (The given commitment structure must be implementable by all desired target platforms: EVM, ZK, Rust, Solana)
  • Cheap (The commitment structures must be feasibly cheap on desired target platforms: EVM, ZK)
  • Upgradable (While this is the hardest to upgrade, by splitting each component into its own byte array and hashing them individually they can also be changed independently)

Context for current decision:

  1. The first option was to hash every field to ensure that each field is clearly separated and cannot be malleated. However, with the multiple payloads (5 fields per payload) this will lead to many hashes that all then have to be hashed together. This could be inefficient and also not particularly elegant. Also by hashing even the payload fields directly, the payload is part of the hashing specification, however if the payload is simply the encoded preimage as bytes passed into the hashing function, then the encoded preimage can be changed without changing the entire hashing specification
  2. Another option considered is to prefix the length of each field in order to ensure bytes from one field do not get confused with another. This seemed error prone and is effectively implementing half an encoding standard so we considered simply encoding the payload with an existing standard.
  3. The third option is to encode the payload. In order to do this, we need a canonical encoding standard that is widely implementable. Unfortunately Protobuf is NOT canonical so different implementations can yield different byte arrays. The two canonical encoding standards i considered were BCS and CBOR. BCS is a blockchain specific encoding standard while CBOR is widely adopted and also implemented in Solidity. Thus i suggest canonical CBOR as an option for now however i haven't completed research on whether this is the best option. The downside is that we are now adding a particular encoding format as a core requirement of IBC v2. The benefit is that we can encode the payload and acknowledgements with this encoding format before hashing. This is simpler to implement and verify for correctness, while also being easier to upgrade. Also this requires a well maintained cbor implementation that is also canonical on the platforms we support.

From these above options, we are considering either option 1 or option 3 (with the specific canonical encoding scheme tbd). Help in determing which option is best to accomplish the desired properties above is greatly appreciated!

Dependent on #1144

AdityaSripal avatar Sep 23 '24 09:09 AdityaSripal

On the topic of encoding, fundamentally I think the problem lies in the leaky abstraction provided by ICS02-Client. The abstract methods such as verifyMembership are designed to be too general for verifying proofs at any Merkle path, with arbitrary bytes. However in reality, the core IBC logic only needs to handle a handful of specific proof verification. With the v2 specs that we are designing, it is likely that the only proof methods we need is to verify packet commitment and packet timeout.

The current design for ICS02 API also place strong assumption on how the commitment path is supposed to be constructed. By the time the path is passed to the client handler, all essential information such as path separators have been erased, making it difficult for the client handler to verify the proof in some other way, such as by using structured data and ZK proofs.

I think we should consider the option to allow for proof verification by passing in the structured Packet object directly, together with other metadata such as commitment prefix as separate arguments. This way, each chain can have their own way of encoding and committing the packet, without there needing to be a standard way of encoding for all possible chains. This way, Cosmos SDK chain implementations can choose to stick with the original method of encoding packets using CBOR or protobuf, while chains like Starknet can choose to use Cairo encoding and ZK storage proofs.

One way of doing this may be to augment the ICS02-Client API, to allow an ICS02 client handler to encode and verify a packet commitment proof. Alternatively, I think it may make sense to do this at a separate level, where different implementation of packet commitment strategy can be chosen during channel initialization. This would allow one chain to also have multiple ways of committing and verifying packets. For example, this would allow a Cosmos SDK chain to commit a packet by encoding with a different format, such as Cairo, and then hash it using Perdersen hash so that it can be verified more easily on Starknet.

It would probably make sense to introduce one (or maybe two) additional layer between ICS02-Client and ICS04-Packet, so that packet commitment and verification would go through this layer, instead of interacting with ICS02-Client directly. On one side, we have a handler that is responsible for encoding incoming packets from a counterparty chain, before using ICS02-Client to verify for membership proofs. On the other side, we have a handler that is responsible for encoding outgoing packets, and commit it using a prefixed ICS24-Host storage. It is worth noting that this decoupling is necessary so that the same ICS02-Client or ICS24-Host can be used to support multiple packet encoding and verification strategies.

soareschen avatar Sep 26 '24 10:09 soareschen

In addition to the consideration of allowing custom packet commitment verification, for a concrete packet encoding implementation, I think we should also consider the IETF approach of specifying the encoding as packet diagrams.

Here are some resources I find that can help understand how to design packet diagrams:

  • https://www.rfc-editor.org/rfc/rfc2360.html#section-3.1
  • https://www.ietf.org/archive/id/draft-mcquistin-augmented-ascii-diagrams-13.html
  • https://www.ietf.org/archive/id/draft-mcquistin-quic-augmented-diagrams-05.html

Some of the things we can consider with this encoding:

  • Have a magic number prefix at the beginning to help distinguish IBC packets from arbitrary bytes.
  • Leave some reserved bits for future extensions, and require them to be set to zero for now.
  • Compact length fields to use a few bits with a reasonable maximum size limit
    • Note that for fields that require non-zero length, we can save one bit by using the field value + 1 as the length.

Here is an example rough attempt at defining the IBC packet header encoding using a packet diagram:

  0                   1                   2                   3
  0 1 2 3 4 5 6 7 8 9 0 1 2 3 4 5 6 7 8 9 0 1 2 3 4 5 6 7 8 9 0 1
 +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
 |     Magic     | R |     NL    |    SrcIdLen   |    DstIdLen   |
 +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
 |   TimeoutLen  |    P Count    |
 +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
 |                Total Payload Size             |
 +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
 |                          Nonce                              ...
 +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
 |                          SrcId                              ...
 +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
 |                          DstId                              ...
 +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
 |                         Timeout                             ...
 +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+

Field descriptions:

  • Magic - An 8-bit magic number such as 0xBC to identify the datagram as an IBC packet.
  • Reserve (R) - A 2-bit reserve for future extensions. MUST always be set to zero in the current version.
  • Nonce Length (NL) - A 6-bit field for the length of the nonce in value+1 bytes, allowing up to 128 bytes (1024-bits) nonce.
  • Source Identifier Length (SrcIdLen) - An 8-bit field for the length of the source identifier in value+1 bytes, allowing up to 512 bytes as identifier.
  • Destination Identifier Length (DstIdLen) - An 8-bit field for the length of the source identifier in value+1 bytes, allowing up to 512 bytes as identifier.
  • Timeout Length (TimeoutLen) - An 8-bit field for the length of the timeout field in value+1 bytes, allowing up to 512 bytes to encode timeout information.
  • Payload Count (P Count) - An 8-bit field inndicating value+1 number of payloads in the payload body, allowing up to 512 payloads.
  • Total Payload size - A 16-bit field for the total size of the payload body in value+1 bytes, allowing up to 32 MiB of payload body. This includes both the payload header and the payload data of all payloads.

Note that we leave out the packet payload body in the packet diagram, as we separate the definition of the IBC packet header and the body. We can then define separate mechanism to produce one hash for the packet header, one hash for the packet body, and then concatenate the two hashes together.

soareschen avatar Sep 27 '24 10:09 soareschen

We want to eventually allow either partial failure or full atomic packets.

We just uncovered some potential issues in supporting atomic IBC packets with multiple payloads. The problem is that inside an atomic transaction, if we want to fail and revert the state transition, then it would also not be possible to write any failed acknowledgement to the commitment store. But if we allow the transaction to return successfully, then we may not be able to revert any partial failure when handling the payload.

I guess in Cosmos SDK, it probably provides some kind of revertible context that can be used by ibc-go when calling applications. However, this would require privileged access from ibc-go to allow that to happen. This would not scale to other blockchains, in particular for smart contract platforms such as CosmWasm or Starknet. Each IBC application may be deployed as separate smart contracts. So we cannot revert only one of the success calls to a smart contract, unless we also revert the entire transaction.

On the other hand, in case if atomic payloads cannot be supported, I find it to be unnecessary complex to support multiple payloads in one IBC packet. Doing so would introduce unnecessary coupling between independent payloads. Any failure from one of the payloads can easily force the entire transaction to fail. It may be simpler to keep with the single payload model, and instead rely on multi-message transactions to handle any necessary atomicity.

soareschen avatar Oct 01 '24 10:10 soareschen

Superceded by #1152

We will go with hashing the packet and acknowledgement. Modulo further security and auditor feedback

AdityaSripal avatar Nov 12 '24 14:11 AdityaSripal