Allow callback to IBC app callers

[AdityaSripal]

IBC was designed with callbacks between core IBC and IBC applications. IBC apps would send a packet to core IBC. When the result of the packet lifecycle eventually resolved into either an acknowledgement or a timeout, the core IBC called a callback on the IBC application so that the IBC application could take action on the basis of the result (e.g. unescrow tokens for ICS-20)

We are now seeing the desire for secondary applications to call into IBC apps as part of their state machine logic and then do some actions on the basis of the packet result.

E.g. Send an ICS-20 packet, and if it is successful, then send an ICA-packet to swap tokens on LP and return funds to sender

This requires a second layer of callbacks. The IBC application already gets the result of the packet from core IBC, but currently there is no standardized way to pass this information on to a caller module/smart contract.

Of course this isn't an issue in the IBC protocol since it is a matter of IBC applications getting this information to their own callers (it is one layer above IBC). However standardizing on the best approach will allow callers to implement a standard interface to interact with all IBC apps.

---

Proposal:

Introduce an interface that accounts can optionally implement:

interface IBCCallerAccount {
    onAcknowledgement(portID, channelID string, sequence uint64, ack exported.Acknowledgement)
    onTimeoutPacket(portID, channelID string, sequence uint64)
}

So far, IBC applications have different APIs for their packet sending logic.

Transfer has Transfer, ICA has SendTx. I think this is fine since IBC apps do not want to nakedly pass through packets, and they will need to have different APIs for their different purposes.

However, I think we should standardize on the MsgResponse which should return channelID, portID, sequence of the packet that gets sent by IBC.

MsgResponse {
    packetID: PacketID,
    // app-custom fields
}

IBC applications are also encouraged to accept a sender address field in all of their APIs, and to store this for a particular packet.

The caller application can then call the desired API with their sender address. They can then get back the packetID and store any information they desire keyed on the packetID.

Later, the packet lifecycle resolves. Core IBC calls the IBC app callback. And if the sender is a IBCCallerAccount, then the IBC application can call the callback onAcknowledgmentPacket or onTimeoutPacket with the relevant arguments.

The IBC caller, gets the packet ID and the result of the packet. They can use this along with any information they stored to execute further logic.

This solves many use cases of the form: I want to transfer X tokens, and then do Y after programmatically

---

Open Questions:

1. Should we deal with sender authentication? If so, how?
2. Should we return just packetID on MsgResponse or entire packet
3. Does this minimal API suffice?

---

Note that this isn't a change to core IBC. It's a design pattern IBC applications can adopt to enable complex programs to be written on top of them

cc: @ethanfrey @ValarDragon

[ethanfrey]

As discussed on the call, here is what we pass into contracts that implement IBC: ack and timeout

This is what we include in those messages: IbcPacketAckMsg and IbcPacketTimeoutMsg.
(Both are mainly wrappers around IbcPacket not the id)

[ethanfrey]

Another idea is to switch on success/failure so the caller doesn't need to understand the acknowledgement format. Basically getting a call of: "success", "failure", "timeout". I'm not sure what is exactly in exported.Acknowledgement.

Also, the caller doesn't have access to the sequence usually, as it is assigned after it sends. It should receive the original packet data, which the caller would have

[AdityaSripal]

Also regarding arbitrary gas consumption by callers:

There are cases where the tx might simply never succeed. Perhaps the gas consumption for a tx is higher than the block gas limit. This is especially dangerous in OnTimeoutPacket or OnAcknowledgementPacket because it could lead to a situation where the packet lifecycle can never resolve. (E.g. ICS-20 tokens never refunded because all attempts to relay ack_error or timeout fail on OutOfGas exception).

Thus, we should cache context before sending to caller, recover from OutOfGas panics in the IBC app, discard cache and continue processing the ack/timeout if an outofgas panic occurs in the caller.

[jackzampolin]

We have an issue similar to this in our ICQ impl. Would be great to see thi!

[ethanfrey]

Thus, we should cache context before sending to caller, recover from OutOfGas panics in the IBC app, discard cache and continue processing the ack/timeout if an outofgas panic occurs in the caller.

Oh, this is very dangerous.

Assume I am a malicious relayer and I want to block your very important callback. I know it requires 150,000 gas to run properly in addition to base IBC gas. I then provide 50,000 gas more than base IBC. The OnAcknowledgementCallback will run out of gas and the rest of the packet will be processed (assuming you use some limited sub-tx context to not let it burn all the tx gas, just those 50k or such).

If there is a limit here, it must be from governance consensus, and not set by the relayer or on a per-tx basis

[AdityaSripal]

Agreed, thanks for all the feedback. Will write an ADR with the understanding gathered here

[asalzmann]

We independently ran into this issue with interchain accounts. We implemented a draft PR for ICA callbacks today. A review would be greatly appreciated. I think the design we landed on is pretty close to what's being proposed here, however it's limited to ICA.

Stride-Labs/stride#146

There are cases where the tx might simply never succeed. Perhaps the gas consumption for a tx is higher than the block gas limit. This is especially dangerous in OnTimeoutPacket or OnAcknowledgementPacket because it could lead to a situation where the packet lifecycle can never resolve.

This is an interesting edge case, thoughts on storing the callback id and arguments to state in the ack and then batch processing them in the EndBlocker? This would only work for module developers that weren't behaving adversarially (smart contracts with access to this functionality could of course exploit it by triggering long-running acks). My understanding is that there are already no guarantees on when code in the BeginBlocker and EndBlocker will halt, which any module can trigger, so I don't think this introduces additional risk (although it might be slightly harder to reason about).

@ethanfrey @AdityaSripal

[AdityaSripal]

This is an interesting edge case, thoughts on storing the callback id and arguments to state in the ack and then batch processing them in the EndBlocker? This would only work for module developers that weren't behaving adversarially (smart contracts with access to this functionality could of course exploit it by triggering long-running acks). My understanding is that there are already no guarantees on when code in the BeginBlocker and EndBlocker will halt, which any module can trigger, so I don't think this introduces additional risk (although it might be slightly harder to reason about).

Yes this could only work for trusted modules/smart contracts. There's also a BlockGasLimit that limits endblock/beginblock computation so it still has the same fundamental issue

[asalzmann]

we've more or less finalized our version of this here: Stride-Labs/stride#146
example migration from IBC acks to application level acks: Stride-Labs/stride#169

Of course understand your requirements might be different (especially with cosmwasm)

[fluffydonkey]

Does this issue refer to the problem of being able to callback after an ibc token transfer to then do something with that token?

[crodriguezvega]

Does this issue refer to the problem of being able to callback after an ibc token transfer to then do something with that token?

Correct!