ibc icon indicating copy to clipboard operation
ibc copied to clipboard

Extend acknowledgement type

Open ethanfrey opened this issue 4 years ago • 8 comments

Note: on hold pending #581 discussion. If that is merged, this will likely be closed. If that is rejected, I would integrate all feedback and revive this proposal.

This is discussing changing the acknowledgement type from application-dependent bytes to actually care more protocol-level metadata, like the Packet type.

So far, I work on defining the type and update some functions to get general feedback.

How a chain / channel / app should handle different types in a backwards compatible way is the huge open question:

  • Can we require all applications / modules on a v2 chain to use the new acknowledgement format? Or do we do this on a channel/connection-level basis?
  • We must handle the case where a v2 implementation talks with a v1 implementation. How do we handle backwards-compatibility?

ethanfrey avatar Jun 03 '21 13:06 ethanfrey

One idea for backwards compatibility is to do this on a chain-by-chain basis.

  • v1 - v1 uses the current acknowledgement bytes both in the application interface and on the wire
  • v2 - v2 uses the new acknowledgement object both in the application interface and on the wire
  • v1 - v2 must be supported and a bit tricky. This must be detected somewhere (eg. establishing a connection), and we need to use v1 acknowledgements on the wire.

In this last case, the v2 application can return the entire Acknowledgement object on writeAcknowledgement and the IBC handler (which detects this is sent on a v1 connection) can drop the extra fields and just return the acknowledgement.data on the wire.

When they receive an acknowledgement from v1, they can wrap it in an Acknowledgement object before passing to the applications. In this case, all fields but data will be unset, but the applications can be shielded from this mainly.

Note: the implications is that limited fees would theoretically be possible on a v2-v1 connection. That is packets sent from the v2 chain to the v1 chain could incentivize ack and timeout, but not receive. Maybe they pay extra on ack and hope that gets picked up by the forward relayer (at least to some decent approximation over many runs). The packets sent from v1-v2 would still be unincentivized

ethanfrey avatar Jun 03 '21 13:06 ethanfrey

Nice work, I think from a backwards compatibility standpoint. It is easier for the applications to still only create and process opaque bytes as the acknowledgement.

Regardless of what version the chain is on, the modules just return and receive the application-specific acknowledgement bytes.

If the connection version is on V2, then the ICS-4 handlers just wrap/unwrap the acknowledgement bytes in the structure you have and send it over the wire but the application is not aware of this at all.

On write acknowledgement, app returns ack bytes and ICS-4 places it in ack struct along with forward relayer address. During acknowledgePacket, ICS-4 unwraps ack struct and pays forward relayer, it then passes the raw ack bytes to the application callback.

If the connection version is on V1, then the ICS-4 handlers just commit the acknowledgement bytes directly as it does already.

The only reason I see to bring the structured Acknowledgement all the way to the application is to enable future field extensions that might be used directly by apps but it seems unnecessary for this case.

AdityaSripal avatar Jun 03 '21 22:06 AdityaSripal

In order to keep things even easier for relayers to integrate this, we could keep the proto definiton of Acknowledgement still have bytes in all the relayer messages. It is just required that the bytes unmarshal to the structured Acknowledgement if the connection is v2.

In this case, the relayer doesn't even need to send a different type of acknowledgement message. Though this may be too loose of an implementation, and relayers will have to adapt anyway to include forward address.

AdityaSripal avatar Jun 03 '21 22:06 AdityaSripal

On write acknowledgement, app returns ack bytes and ICS-4 places it in ack struct along with forward relayer address.

Note that this requires some storage in core IBC in order to support asynchronous acks

AdityaSripal avatar Jun 03 '21 22:06 AdityaSripal

Nice work, I think from a backwards compatibility standpoint. It is easier for the applications to still only create and process opaque bytes as the acknowledgement.

Regardless of what version the chain is on, the modules just return and receive the application-specific acknowledgement bytes.

If the connection version is on V2, then the ICS-4 handlers just wrap/unwrap the acknowledgement bytes in the structure you have and send it over the wire but the application is not aware of this at all.

On write acknowledgement, app returns ack bytes and ICS-4 places it in ack struct along with forward relayer address. During acknowledgePacket, ICS-4 unwraps ack struct and pays forward relayer, it then passes the raw ack bytes to the application callback.

If the connection version is on V1, then the ICS-4 handlers just commit the acknowledgement bytes directly as it does already.

The only reason I see to bring the structured Acknowledgement all the way to the application is to enable future field extensions that might be used directly by apps but it seems unnecessary for this case.

This seems like a good design.

My biggest issue in this pseudocode is that it seems to be defining the ICS-4 handlers, but doesn't explicit show what it passes into the application-level.

You propose:

  • OnReceivePacket - ICS-4 receives all data, stores payOnSource (and forwardRelayer?)based on "packet id". It then calls the application without thepayOnSource`
  • WriteAcknowledgement - ICS-4 takes bytes from the application. It checks if it is v2 and if so, loads the data stored above and constructs a proper Acknowledgement struct, which is will proto-encode and then commit to that. If the connection is v1, then it just commits the raw acknowledgement.
    • For better relayer compatibility, we can just return the pre-hashed commitment as an event, regardless if it is raw app data or wrapper. Does ibc-go emit acknowledgement_hex event as well (like with packet) for better binary compatibility?
  • OnAcknowledge - ICS-4 will check the connection version. If it is v1, then it will pass those raw ack bytes to the application. If it is v2, it will parse the acknowledgement bytes into Acknowledgement message, call the fee handler with ack relayer and payOnSource from ack packet, and then pass the acknowledgement.data field to the applicaton.

To make this all more clear, it would be nice to define somewhere in this psuedo-code the flow and call from ICS-4 handlers into the application modules. Or is that only done in ibc-go?

ethanfrey avatar Jun 04 '21 13:06 ethanfrey

I would deprecate this for the new ics-30 middleware solution being developed in #581

If that middleware solution is not followed, I would be happy to update this spec with all comments. But the 2 are mutually exclusive in the current design.

ethanfrey avatar Jun 21 '21 20:06 ethanfrey

@ethanfrey Is this PR still relevant?

mpoke avatar Apr 11 '22 13:04 mpoke

Maybe.

I think a discussion of actually providing structure to the acknowledgement would be a good thing.

There is no concrete action item here

ethanfrey avatar Apr 11 '22 17:04 ethanfrey