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

Buffer Downward Messages for Parachains during MBMs #480

Open
ggwpez opened this issue Jul 2, 2023 · 6 comments
Open

Buffer Downward Messages for Parachains during MBMs #480

ggwpez opened this issue Jul 2, 2023 · 6 comments

Comments

@ggwpez
Copy link
Member

ggwpez commented Jul 2, 2023

The relay will soon need to be aware that a parachain may enter into a Multi Block Migration directly after a runtime upgrade.
A parachain that is currently migrating its storage cannot handle any incoming messages since the storage that is needed to handle said messages could get migrated as well.

Possible procedure:

  1. The Relay upgrades the PVF of a parachain.
  2. The Relay now pessimistically assumes that the parachain is migrating and buffers all downward messages locally.
  3. The parachain finishes migrating after a limited number of blocks (maximum enforced by the relay) and sends ResumeDownwardMessages to the relay. Note that the parachain may immediately send that message in case that there are no MBMs scheduled for the upgrade.
  4. The Relay starts to enforce the normal message consumption expectations again and expects the parachain to clear the local buffer within x blocks.

Qs

  • What are the current message consumption expectations that the relay enforces for every parachain? I only know of the HRMP watermark advancement but dont know if other channel types have alike checks.
@ggwpez ggwpez changed the title Pause all downward messages for parachains during MBMs Buffer all downward messages for parachains during MBMs Jul 2, 2023
@ggwpez ggwpez changed the title Buffer all downward messages for parachains during MBMs Buffer Downward Messages for Parachains during MBMs Jul 2, 2023
@liamaharon
Copy link
Contributor

liamaharon commented Jul 2, 2023

I assume there would be a max number / size of messages a Relay chain would potentially buffer? I'm concerned this may introduce a new limit parachains need to be aware of - their migration must complete within N blocks or the Relay chain buffer will fill up. The greater average rate a parachain receives messages, the shorter time window they would have to complete their migration.

Also, pessimistically deciding to buffer/withhold messages for a prolonged period after an upgrade might temporarily break the operations of some parachains that rely on message passing. Imagine a DeFi chain that relies on regular oracle price updates or arbitrage via XCM to function properly.

If these are valid issues it feels to me like it would make more sense for the buffer to be the burden of the parachain not the relay chain. You mentioned in the issue description

A parachain that is currently migrating its storage cannot handle any incoming messages since the storage that is needed to handle said messages could get migrated as well.

but couldn't we leave buffering of inbound messages enabled without performing any handling until the migration is complete?

@ggwpez
Copy link
Member Author

ggwpez commented Jul 4, 2023

I assume there would be a max number / size of messages a Relay chain would potentially buffer? I'm concerned this may introduce a new limit parachains need to be aware of - their migration must complete within N blocks or the Relay chain buffer will fill up. The greater average rate a parachain receives messages, the shorter time window they would have to complete their migration.

Yes. We always need time limits when we build something, simply to guarantee progress. A parachain will in the future need to produce a block every 24h (until the PoV is pruned), so that limit could be re-used here.

Also, pessimistically deciding to buffer/withhold messages for a prolonged period after an upgrade might temporarily break the operations of some parachains that rely on message passing. Imagine a DeFi chain that relies on regular oracle price updates or arbitrage via XCM to function properly.

Yep. But parachains already need to handle the case that another parachain does not respond to its XCMs, so handling this for the MBM case would be no different. An oracle chain could opt to not use MBMs, if that is really time critically important. Or put all things in mandatory hooks and never migrate that storage.

but couldn't we leave buffering of inbound messages enabled without performing any handling until the migration is complete?

No, since the storage layout of the buffer may have changed and cannot be accessed until the MBM is done. I know this is a specific case, and in most cases it will not be the buffer that will be migrated, but this solves it in a more general way.
The parachain will be cut off from all inbound messaging until it is in a consistent storage state again. Putting any data in there while a MBM is ongoing is likely asking for trouble. It ties into all of this, and the transitive bleeding of unmigratable storage... #165. Having it completely cut off just makes this a bit less headache-inducing.

@xlc
Copy link
Contributor

xlc commented Jul 4, 2023

Can we just make sure the message queue pallet never uses MBM and therefore the state is always consistent so we can always safely buffer the queue on dest parachain.

@bkchr
Copy link
Member

bkchr commented Jul 6, 2023

Yes, certain pallets will always need to migrate certain important changes at once to stay operational. However, the issue here is that we can not queue for an unknown time. We will need to process these messages at some point. What you want is that parachain A knows that parachain B will currently not accept any more messages because the queue is full. Then parachain A can prevent any XCM to go out to parachain B.

@xlc
Copy link
Contributor

xlc commented Jul 6, 2023

sounds like we need a way to pause & resume hrmp channel?

@bkchr
Copy link
Member

bkchr commented Jul 6, 2023

We already have this information:

https://github.com/paritytech/cumulus/blob/2114d82fa2de00bfdcb7662d98fbe55a97b5c760/pallets/parachain-system/src/lib.rs#L770-L810

We would just need to take this into account before sending a message from the "upper" layers in a runtime. (Internally this is already being used, but then results just in keeping the messages queued.)

@juangirini juangirini transferred this issue from paritytech/polkadot Aug 24, 2023
jonathanudd pushed a commit to jonathanudd/polkadot-sdk that referenced this issue Apr 10, 2024
* verify messages proofs

* fmt

* clippy

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

No branches or pull requests

5 participants