-
Notifications
You must be signed in to change notification settings - Fork 1.6k
Implementer's Guide: Flesh out more details for upward messages #1556
Conversation
I pushed a commit that gives up a strict enforcement that dispatchables should be decodable and that they are not exceeding some critical weight per dispatchable. Instead, I introduced a downward message that is sent in response for each dispatchable upward message that reports the status of execution, basically That allows not only to perform handling of failure on the parachain side, e.g. by retrying or releasing the assets that could be potentially locked, but it also mitigates a more broad problem. It's not only the runtime code can invalidate a dispatchable but some dispatchable fails just because of some racy but nonetheless legit condition. |
> NOTE: It is conceivable that upward messages will be divided to more fine-grained kinds with a dispatchable upward message | ||
being only one kind of multiple. That would make upward messages inspectable and therefore would allow imposing additional | ||
validity criteria for the candidates that contain these messages. | ||
One kind of messages is `Dispatchable`. They could be thought of similar to extrinsics sent to a relay chain: they also |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Since this is the only kind right now, phrasing as "One kind, and the only current kind, of message..."
}, | ||
// Examples: | ||
// HrmpOpenChannel { .. }, | ||
// HrmpCloseChannel { .. }, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What are these examples of?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Of other potential kinds of messages, I can remove that if you think that's unnecessary given that those will become real messages
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would be in favour of removing them, I find it unclear what usage they show.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM except for grumbles!
Co-authored-by: Robert Habermeier <rphmeier@gmail.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A couple of nitpicks, but LGTM 👍
bot merge |
Trying merge. |
* master: Companion PR for #6862 (#1564) implement collation generation subsystem (#1557) Add spawn_blocking to SubsystemContext (#1570) Companion PR for #6846 (#1568) overseer: add a test to ensure all subsystem receive msgs (#1590) Implementer's Guide: Flesh out more details for upward messages (#1556)
…n-race-condition * master: Companion PR for #6862 (#1564) implement collation generation subsystem (#1557) Add spawn_blocking to SubsystemContext (#1570) Companion PR for #6846 (#1568) overseer: add a test to ensure all subsystem receive msgs (#1590) Implementer's Guide: Flesh out more details for upward messages (#1556) Make parachain validation wasm executor functional (#1574) Use async test helper to simplify node testing (#1578) guide: validation data refactoring (#1576) Remove v0 parachains runtime (#1501) [CI] Add github token to generate-release-text (#1581) Allow using any polkadot client instead of enum Client (#1575) service/src/lib: Update authority discovery construction (#1563)
* master: overseer: fix build (#1596) Companion PR for #6862 (#1564) implement collation generation subsystem (#1557) Add spawn_blocking to SubsystemContext (#1570) Companion PR for #6846 (#1568) overseer: add a test to ensure all subsystem receive msgs (#1590) Implementer's Guide: Flesh out more details for upward messages (#1556) Make parachain validation wasm executor functional (#1574) Use async test helper to simplify node testing (#1578) guide: validation data refactoring (#1576) Remove v0 parachains runtime (#1501) [CI] Add github token to generate-release-text (#1581) Allow using any polkadot client instead of enum Client (#1575) service/src/lib: Update authority discovery construction (#1563) Update .editorconfig to what we have in practice (#1545) Companion PR for substrate #6672 (#1560) pre-redenomination tockenSymbol change (#1561)
Closes #1534
This PR changes the way how we process the upward messages.
Upward Message Kind
Upward messages are split to different kinds, with
Dispatchable
being only one of them. The upside is that the unlike plain opaque dispatchables, explicit message kinds allows us to impose some validity criteria upon the upward messages. E.g. we can reject candidates that we know will fail upfront.However, we only introduce the scaffolding for different upward message kinds without actually introducing any new.
Weight
Then, in the current construction the weight consumed by messages is not limited. In the proposed changes, each upward message of dispatchable kind consumes some predefined weight and the total weight consumed during dispatching messages is limited, leaving a guaranteed amount of weight to other extrinsics.
This proposal doesn't yet address charging fees for the weight.
Round-robin dispatching
Since it can happen that some messages won't fit in the weight budget devoted for a block, we need to fairly distribute the weight among the pending dispatchables.
To address this, we go over all queues in a round robin fashion dequeuing a message each time. When we reach the weight budget for executing dispatchables or there are none left we finish processing. There can be multiple iterations. If any unused weight left it can be leveraged by signed extrinsics.
Dispatch order
The current construction dispatches messages during finalization. The proposed construction moves this processing to be just right at the end of inclusion inherent giving the dispatchables an advantage over the regular signed extrinsics. That alleviates the front-running the parachain upward messages by the validators to some degree.
Optionally, perhaps we can add another round of dispatching during the finalization stage to leverage unused weight, but that would worsen the property against front-running