Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Naive fix for sending sized messages #2098

Closed
wants to merge 4 commits into from

Conversation

serban300
Copy link
Collaborator

Fix sized messages by adding an estimated xcm executor overhead.

This can be just a temporary fix, or if it's good enough we can leave it like this. @svyatonik what do you think ? How precise does the payload size need to be ? I see that we already have other estimation here for example.

serban300 added 2 commits May 1, 2023 12:27
It's safer like this since here are some errors that aren't caught when
using native execution
@serban300 serban300 self-assigned this May 1, 2023
@serban300 serban300 changed the title Naive fix for sized messages Naive fix for sending sized messages May 1, 2023
@svyatonik
Copy link
Contributor

This can be just a temporary fix, or if it's good enough we can leave it like this. @svyatonik what do you think ? How precise does the payload size need to be ? I see that we already have other estimation here for example.

In ideal world it needs to be the maximal possible (i.e. compute_maximal_message_size). The general idea is to prove that we are able to deliver messages that our runtime accepts (we now have the only check when we are queening the message - it is the size check). So if we are accepting messages of 1_024 bytes, but we can only deliver 1_023 byte messages, it'll mean that the bridge can break when such message is sent.

There's a lot of assumptions in the code. E.g. the bridge_runtime_common::messages::target::maximal_incoming_message_size method assumes that everything except the message (i.e. storage trie nodes, signed extension data, ...) will fit into 1/3 of maximal extrinsic size. So we leave 2/3 of max extrinsic size to the message itself.

But rhe code you're referencing is about a different situation. This is for cases when messages are sent from the chain where max transaction size is lesser than 2/3 of target chain max transaction size. For our deployments, it is Millau -> Rialto. Millau has 2Mb blocks and Rialto has 5Mb blocks. So we can't send 2/3 * 5Mb messages from Millau to Rialto because the message will be rejected at Millau (because transaction is too large). For the other direction (Rialto -> Millau), IIRC we were using messages of exactly compute_maximal_message_size bytes (but I can't guarantee that :) ).

So I suggest to try to use message generation infrastructure from #2064, but use the ExportMessage instruction instead of ClearOrigin. IMO it'll generate messages that are closer to the compute_maximal_message_size limit than the current one. But feel free to ignore the suggestion - your approach is good enough too (I"m hitting approve and let you decide).

@serban300
Copy link
Collaborator Author

compute_maximal_message_size

Ok, thank you for the details ! Makes sense. I will try to fix this then.

@serban300 serban300 marked this pull request as draft May 1, 2023 11:02
@serban300 serban300 closed this May 2, 2023
@serban300
Copy link
Collaborator Author

Closing this PR for the moment and will open a new one later.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants