ICS26 API requirements: deliver versus dispatch

[livelybug]

Hello,

We are investigating how to integrate ibc-rs in substrate-ibc.
Here's a question about the functions deliver and dispatch in "modules/src/ics26_routing/handler.rs".

As both function deliver and dispatch are public, may I say function deliver is optional?

Why such an assumption, because the function deliver's parameter messages: Vec<Any> is encoded data and later decoded inside the same function deliver.
In a substrate's pallet, the corresponding functions in the same layer of "deliver", already have decoded parameter, you may refer to such a function here and below:

    #[weight = T::DbWeight::get().reads_writes(1, 1).saturating_add(40_000_000)]
    fn create_swap(
      origin,
      target: T::AccountId,
      hashed_proof: HashedProof,
      action: T::SwapAction,
      duration: T::BlockNumber,
    ) { 
      // --snip-- 
    }

We plan to skip ibc-rs' deliver function and call ibc-rs' dispatch function in "substrate-ibc", as below:

#[weight = 0]
fn deliver(origin, data: Vec<Datagram>) -> dispatch::DispatchResult {
  let _sender = ensure_signed(origin)?;
  let mut ctx = informalsystems::Context;
  ibc::ics26_routing::handler::dispatch(&mut ctx, data);
  Ok(())
}

It's appreciated if any advise.
Thank you

---

For Admin Use

• Not duplicate issue
• Appropriate labels applied
• Appropriate contributors tagged
• Contributor assigned/self-assigned

[adizere]

Hi,

As both function deliver and dispatch are public, may I say function deliver is optional?

Good question. The short answer is: We were assuming that deliver should be the main entry point, and perhaps dispatch should even be private -- so then deliver would be mandatory. But this is not set in stone and we need to gain insights into the requirements of users of ibc-rs modules, such as substrate. So your feedback here is greatly appreciated, and please see the questions at the end.

The long answer...

Why such an assumption, because the function deliver's parameter messages: Vec is encoded data and later decoded inside the same function deliver.

Indeed, deliver has two main purposes:

1. decode the messages into domain types (based on type_urls) and
2. apply the side-effects of all messages in a transaction atomically on the context, so that either all messages take effect on the context (in case no error surfaces), or otherwise no message leaves side-effects on the context (in case an error occurs in any of them).

There is one main disadvantage to using dispatch (instead of deliver), which is that this will result in substrate having a larger dependency surface towards ibc-rs. More specifically, with dispatch, in substrate you will need to care about ICS26Envelope type which may evolve faster and prove to be less stable than Vec<Any> (which evolves at the pace of the rest of the ecosystem).

There are two smaller disadvantage of preferring dispatch: (i) substrate would need to ensure point (2) above (i.e., the atomicity of handling all messages in a transaction) and (ii) the return type of dispatch which is HandlerOutput may contain irrelevant data (the relevant return value is probably just a vector of Events, as in the deliver method), but this is not clear to me.

In a substrate's pallet, the corresponding functions in the same layer of "deliver", already have decoded parameter, you may refer to such a function here and below

Thanks for the pointers, they are very helpful.

Looking at your code, then it seems your requirement could be to have an interface to ibc-rs as follows:

pub fn deliver_datagrams<Ctx>(ctx: &mut Ctx, messages: Vec<Datagram>) -> Result<Vec<Event>, Error>

where Datagram is a type similar to our ICS26Envelope, which ensures atomicity of applying the messages to the context, does not need to handle any decoding, and returns the resulting vector of Events. So my questions would be:

• a. between deliver_datagrams and dispatch, is the dispatch method still the most useful for substrate?
• b. it is enough if the return type is a vector of Events? or do you imagine you might need further details about how the messages have been handled (for instance, a Result)?
• c. is the atomicity of handling multiple messages a concern, or would you rather find it useful to have direct access to dispatch and handle each message one by one?

Happy to provide further clarifications if these questions are not clear enough!

ps. Thanks for your patience, it took us some time to respond because this was a slightly busier period.

[en]

Thanks, Adi

The interface is not a big problem.
A painful issue is how to submit a prost::Any to the chain through a substrate transaction.
In substrate, all transaction parameters must be serializable by SCALE.
So we have two options:

1. Implement SCALE for prost::Any.
2. Encode prost::Any to Vec, and use Vec as transaction parameter. When substrate receives Vec, we decode it back to Prost::Any, then call ibc-rs::deliver()

For option 1, because neither prost::Any nor SCALE is defined by us, it will not work.
For option 2, some redundant encode/decode will be done.

At present, a workaround is wrapping a self-defined deliver(see below), looking forward to a better solution.

Sending transaction:
https://github.com/cdot-network/ibc-demo/blob/ffa8e06b540d734a5e90522c4dc487a5a5d65d8f/cli/src/main.rs#L478-L497

Receiving transaction:
https://github.com/cdot-network/substrate-ibc/blob/832e55e290100f2601b5661cfe47e581fc5f140d/src/lib.rs#L366-L381

Also, for substrate, the signer in MsgCreateAnyClient will not be used, because substrate handles signing and verifying signatures.

[livelybug]

Thanks, Adi

The interface is not a big problem.
A painful issue is how to submit a prost::Any to the chain through a substrate transaction.
In substrate, all transaction parameters must be serializable by SCALE.
So we have two options:

1. Implement SCALE for prost::Any.
2. Encode prost::Any to Vec, and use Vec as transaction parameter. When substrate receives Vec, we decode it back to Prost::Any, then call ibc-rs::deliver()

For option 1, because neither prost::Any nor SCALE is defined by us, it will not work.
For option 2, some redundant encode/decode will be done.

At present, a workaround is wrapping a self-defined deliver(see below), looking forward to a better solution.

Sending transaction:
https://github.com/cdot-network/ibc-demo/blob/ffa8e06b540d734a5e90522c4dc487a5a5d65d8f/cli/src/main.rs#L478-L497

Receiving transaction:
https://github.com/cdot-network/substrate-ibc/blob/832e55e290100f2601b5661cfe47e581fc5f140d/src/lib.rs#L366-L381

Also, for substrate, the signer in MsgCreateAnyClient will not be used, because substrate handles signing and verifying signatures.

The wrapped deliver function:
octopus-network/substrate-ibc@832e55e#diff-b1a35a68f14e696205874893c07fd24fdb88882b47c23cc0e0c80a30c7d53759R355-R387

[livelybug]

Thank you @adizere for the clarification.
It clears our doubts for current development.

Here are some supplements.

a. between deliver_datagrams and dispatch, is the dispatch method still the most useful for substrate?
b. it is enough if the return type is a vector of Events? or do you imagine you might need further details about how the messages have been handled (for instance, a Result)?

As our current wrapped deliver, the dispatch is dissembled before applied. We extract the essentials from deliver and dispatch into our code for the time being. Calling deliver instead of dispatch is more logical, but need a solution the serialization problem.

c. is the atomicity of handling multiple messages a concern, or would you rather find it useful to have direct access to dispatch and handle each message one by one?

Atomicity of multiple messages is not a concern in our current development stage. We are still exploring solutions for the integration, will follow ibc-rs' update, and adapt our code accordingly.

[livelybug]

PS: could you add an issue label named "substrate-ibc"? it would be convenient to track the relevant issues, thank you

[en]

hmm ... I realized that I can wrap a local Any in the same way, and then call deliver() directly.
So there is no problem with the interface. Let's continue to test to see if there is any problem with processing messages.

related code:
https://github.com/cdot-network/ibc-demo/blob/84b196c9a170dc63a14a90b544f6d3876f7e641e/cli/src/main.rs#L385-L423
https://github.com/cdot-network/substrate-ibc/blob/c542519baa4b0de6073d187c62a8ed90cc0a0926/src/lib.rs#L361-L366

[andynog]

Hi @livelybug, I'm going to close this issue as per email response confirming this is not an issue anymore.