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

Check that the bridge satisfies XCM design assumptions #2433

Closed
Tracked by #2430
serban300 opened this issue Apr 13, 2023 · 7 comments
Closed
Tracked by #2430

Check that the bridge satisfies XCM design assumptions #2433

serban300 opened this issue Apr 13, 2023 · 7 comments
Assignees

Comments

@serban300
Copy link
Collaborator

Related to #2443 . Specifically: XCM is designed to not fail at low level (e.g queues are essentially unlimited, overweight messages stick around). And the bridge should be designed in the same way

I would like to:

  1. Take a look on the XCMP/UMP/DMP queues and get a better idea on how they are designed and the resilience mechanisms built-in
  2. Check the bridge message queues and see if there's anything that we can improve
  3. Check the messages pallet and make sure that it can't fail unpredictably
@serban300 serban300 self-assigned this Apr 13, 2023
@serban300
Copy link
Collaborator Author

serban300 commented Apr 13, 2023

For point 1:

More useful info:

Taking a look on them

@serban300
Copy link
Collaborator Author

serban300 commented Apr 24, 2023

Looked into multiple aspects related to point 1:

  1. The backpressure mechanism in case of an overweight message. This is important because it will be used by the XCMP queues that our logic interacts with

In the case of an overweight message, the substrate MessageQueue keeps it in a stale page that's removed from the book => it doesn't impact the PoV size since that page won't be touched anymore and it has no impact on performance. The message will need to be manually executed by calling the execute_overweight extrinsic or manually removed by executing the reap_page extrinsic. This looks good.

  1. Is the substrate MessageQueue more resilient than our queue ?

Personally I didn't notice anything related to this yet.

  1. Could we use the substrate MessageQueue instead of our queue ?

The MessageQueue has a couple of advantages:

But I don't think we can use it, because we would need to be able to prove each message individually, and I don't know if that can be done.

  • In the bridges queue we store messages in a double-map and prove that the messages are in storage by their key in the map.
  • The substrate MessageQueue uses pages, and each page stores some messages in an encoded heap and I'm not sure how we could prove that a message is in storage.

@serban300
Copy link
Collaborator Author

Looked on this in detail and pushed just some small improvements. From what I understand the bridge messages queue satisfies the XCM design assumptions. There is just 1 thing that I would mention:

As far as I understand we don't handle overweight messages. We assume that any message will be just pushed to a queue (XCMP/DMP/UMP) on the bridged chain, so the weight of this operation is constant and fits the block. This is true for all the current bridge configurations. But I'm wondering if it would make sense to have a failsafe mechanism anyway, just in case a message ends up being overweight. Just want to spend a bit more time thinking about this.

@svyatonik
Copy link
Contributor

#2219 has changed the message size limit down to 64Kb. We need to have some document before delivering #2451 - on what lane users can do and what they can't (including this 64Kb limit).

@svyatonik
Copy link
Contributor

svyatonik commented Jun 27, 2023

A couple of things that I've also found/noticed:

  1. XcmpQueue assumes that there's never more than 65_535 pages in the queue (see https:/paritytech/cumulus/blob/7bcbff3605e38fb59d6882e30f7a8a10060234d0/pallets/xcmp-queue/src/lib.rs#L332-L346 - docs and usage of u16 nearby);
  2. overweight messages are processed our of order (see this loop - https:/paritytech/cumulus/blob/7bcbff3605e38fb59d6882e30f7a8a10060234d0/pallets/xcmp-queue/src/lib.rs#L678-L698). Shall we allow overweight messages at all - maybe cut off them earlier (at the sending side?);
  3. maximal message sizes are set to different values in different directions (iiuc). E,g. max_downward_message_size is set to 1Mb, max_upward_message_size is set to 50Kb and we set it to 64Kb.
  4. if there are more than drop_threshold (5) unprocessed message pages, pages are "kinda" silently dropped.

svyatonik referenced this issue Jul 17, 2023
* [benchmarks] pr with weights (#2026)

Co-authored-by: paritytech-ci <[email protected]>

* [benchmarks] pr with weights Collectives (#2025)

* [benchmarks] pr with weights

* provide veto method for trait

Co-authored-by: paritytech-ci <[email protected]>
Co-authored-by: muharem <[email protected]>

* [benchmarks] pr with weights (#2027)

Co-authored-by: paritytech-ci <[email protected]>

Co-authored-by: paritytech-ci <[email protected]>
Co-authored-by: muharem <[email protected]>
@EmmanuellNorbertTulbure EmmanuellNorbertTulbure transferred this issue from paritytech/parity-bridges-common Aug 25, 2023
@the-right-joyce the-right-joyce transferred this issue from paritytech/polkadot-sdk Aug 25, 2023
@EmmanuellNorbertTulbure
Copy link

@serban300 to leave conclusion for derived new issue

@serban300
Copy link
Collaborator Author

Created #2522 for using the ``pallet-message-queue`. Resolving.

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

No branches or pull requests

3 participants