-
Notifications
You must be signed in to change notification settings - Fork 1.1k
Introduce MaxParachainBlockWeight and related functionality
#10315
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
base: master
Are you sure you want to change the base?
Conversation
This pull request introduces `MaxParachainBlockWeight` to calculate the max weight per parachain block. This is a preparation for [Block Bundling](#6495) which requires that the maximum block weight is dynamic. Block bundling requires a dynamic maximum block weight because it bundles multiple blocks into one `PoV`. Each `PoV` gets `2s` of execution time and `10MiB` of proof size. These resources need to be split up between all the blocks of one `PoV`. This doesn't require the weight to be dynamic. However, it gets complicated when a transaction should be applied that requires more resources than what one of these blocks can provide, e.g. for doing a runtime upgrade. In this case `MaxParachainBlockWeight` supports to increase the block weight of one block to take up the weight of the full `PoV`. The feature will not only be useful for things like runtime upgrade, but also could enable users to pay for running some huge contracts or whatever. For more information, please refer to the docs provided in the code of this pull request. For `MaxParachainBlockWeight` to work correctly, it provides a pre-inherent hook and a transaction extension. Both are required to track the weight correctly.
cumulus/pallets/parachain-system/src/block_weight/transaction_extension.rs
Outdated
Show resolved
Hide resolved
cumulus/pallets/parachain-system/src/block_weight/transaction_extension.rs
Outdated
Show resolved
Hide resolved
cumulus/pallets/parachain-system/src/block_weight/transaction_extension.rs
Outdated
Show resolved
Hide resolved
…extension.rs Co-authored-by: Guillaume Thiolliere <[email protected]>
…extension.rs Co-authored-by: Guillaume Thiolliere <[email protected]>
ggwpez
left a comment
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.
Seems quite complex to me, maybe we should do a tentative deployment to Westend from this branch with the extension included before merging.
| //! The [`MaxParachainBlockWeight`] provides a [`Get`] implementation that will return the max block | ||
| //! weight as determined by the [`DynamicMaxBlockWeight`] transaction extension. | ||
| //! | ||
| //! [`DynamicMaxBlockWeightHooks`] needs to be registered as a pre-inherent hook. It is used to |
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.
Maybe we can somehow test this in the runtime integrity check, like the migrations pallet does for the MultiBlockMigrator hook.
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.
Yeah I thought about this as well, but did not came up with any proper implementation I'm happy with.
| /// Transaction extension that dynamically changes the max block weight. | ||
| /// | ||
| /// With block bundling, parachains are running with block weights that may not allow certain | ||
| /// transactions to be applied, e.g. a runtime upgrade. To ensure that these transactions can still |
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.
If I want to send a big transaction, how do I tell the node to put into the first block of a batch? Or will it do that automatically?
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.
The node is ordering based on priority. The runtime lays out priority based on the fees that you are going to pay. Fees are based on weight. There is generally no guarantee for the ordering. If a transaction does not fit, it will be tried at the next block.
| /// The current block weight mode. | ||
| /// | ||
| /// This is used to determine what is the maximum allowed block weight, for more information see | ||
| /// [`block_weight`]. |
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.
Can you please explain here when it is set and when it is removed?
cumulus/pallets/parachain-system/src/validate_block/implementation.rs
Outdated
Show resolved
Hide resolved
| .saturating_add(T::DbWeight::get().writes((1_u64).saturating_mul(n.into()))) | ||
| } | ||
| fn block_weight_tx_extension_max_weight() -> Weight { | ||
| Weight::zero() |
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.
Need to rebench
| pub number_of_cores: Compact<u16>, | ||
| } | ||
|
|
||
| impl core::hash::Hash for CoreInfo { |
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.
This is for std only?
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 exactly?
I don't want to directly deploy this to production. :) Not sure what you want to test there on Westend. I have also more tests in another branch where zombienet tests are running based on this stuff. |
…tion.rs Co-authored-by: Oliver Tale-Yazdi <[email protected]>
…ck-weight' into bkchr-parachain-block-weight
|
/cmd fmt |
|
All GitHub workflows were cancelled due to failure one of the required jobs. |
This pull request introduces
MaxParachainBlockWeightto calculate the max weight per parachain block. This is a preparation for Block Bundling which requires that the maximum block weight is dynamic. Block bundling requires a dynamic maximum block weight because it bundles multiple blocks into onePoV. EachPoVgets2sof execution time and10MiBof proof size. These resources need to be split up between all the blocks of onePoV. This doesn't require the weight to be dynamic. However, it gets complicated when a transaction should be applied that requires more resources than what one of these blocks can provide, e.g. for doing a runtime upgrade. In this caseMaxParachainBlockWeightsupports to increase the block weight of one block to take up the weight of the fullPoV. The feature will not only be useful for things like runtime upgrade, but also could enable users to pay for running some huge contracts or whatever. For more information, please refer to the docs provided in the code of this pull request.For
MaxParachainBlockWeightto work correctly, it provides a pre-inherent hook and a transaction extension. Both are required to track the weight correctly.