Get dispatch weight from the target chain (when DispatchFeePayment::AtTargetChain is used)

[svyatonik]

closes #1417

• reintroduce From<SourceChain>InboundLaneApi with inbound_message_details method;
• implement From<SourceChain>InboundLaneApi in runtimes;
• use From<SourceChain>InboundLaneApi from relay;
• tests (where possible :/).

[serban300]

If I understand correctly, this call and extra logic is needed only to access the payload. I was wondering if we could avoid this somehow since TO_CHAIN_MESSAGE_DETAILS_METHOD seems to have access to the payload from the start. For example would it make sense to add the payload as a parameter to the DispatchFeePayment::AtTargetChain option ? I mean redefining the DispatchFeePayment enum as:

pub enum DispatchFeePayment {
	AtSourceChain,
	AtTargetChain(MessagePayload),
}

[svyatonik]

I wouldn't rather touch this enum, since e.g. in encoded-call messaging it is stored as a part of message payload. The alternative would be to add message payload to the message_details method result. But messages may be large and probably can cause a oom in WASM, if we're calling message_details for some range that includes multiple large messages (and we can't know if it'll oom in advance). So probably better to leave it as is (imo)

[serban300]

Sure, it's better to keep it as it is if it's safer. But just one more question:

But messages may be large and probably can cause a oom in WASM, if we're calling message_details for some range that includes multiple large messages (and we can't know if it'll oom in advance)

Than is it possible for WASM to oom when calling the following as well ?

			let refined_messages_encoded = self
				.target_client
				.state_call(
					P::SourceChain::FROM_CHAIN_MESSAGE_DETAILS_METHOD.into(),
					Bytes((self.lane_id, messages_to_refine.values().collect::<Vec<_>>()).encode()),
					None,
				)
				.await?;

Since messages_to_refine contains the payloads.

[svyatonik]

Yeah, that's a valid concern. Then it worth to change FROM_CHAIN_MESSAGE_DETAILS_METHOD runtime API method so that it'll accept single message only. Could you, please, file an issue for that?

[serban300]

Sure. Also I'd be happy to assign it to me if that's ok for you.

[svyatonik]

Sure, go on! :) Thank you!

[serban300]

Nit: Would it make sense to have the logic above (the call to TO_CHAIN_MESSAGE_DETAILS_METHOD) and the decode logic inside make_message_details_map() ? Is there any scenario where we would want to call make_message_details_map without using the output of TO_CHAIN_MESSAGE_DETAILS_METHOD ? Also would it be a lot harder to unit test it this way ?

[svyatonik]

I don't think we need to call make_message_details_map without calling TO_CHAIN_MESSAGE_DETAILS_METHOD before. But yeah - we don't have mocked version of Client right now, so it'll make testing this function much harder.

[serban300]

Sorry, one more thing. If I understand correctly, here we rely on the assumption that the order of the refined_messages returned by the API call is the same as the order of the keys in messages_to_refine, which should be true, but I think it would be safer to test this somehow. Otherwise in case of an unexpected event we may assign a wrong weight to a message. Would it be acceptable to also add the message nonce to the InboundMessageDetails ?

[svyatonik]

I've mentioned this explicitly in the fn message_details documentation. Imo it should be enough, but feel free to add nonce if you think it is better (I'd leave it as is tbh).

[serban300]

Some changes related to the comments were proposed in #1545 . Marking this as reviewed.