Skip to content
This repository has been archived by the owner on Nov 15, 2023. It is now read-only.

Support overweight messages in XCMP queue #799

Merged
merged 8 commits into from
Dec 9, 2021
Merged

Conversation

KiChjang
Copy link
Contributor

Implementation follows closely to the UMP overweight message handling, as seen here in this PR.

Fixes paritytech/srlabs_findings#145.

Copy link
Contributor

@apopiak apopiak left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would like to see a test that tries to process an overweight XCM and then calls the service_overweight functionality.

pallets/xcmp-queue/src/lib.rs Show resolved Hide resolved
pallets/xcmp-queue/src/lib.rs Show resolved Hide resolved
Comment on lines +71 to +82
pub fn migrate_to_v1<T: Config>() -> Weight {
let translate = |pre: v0::QueueConfigData| -> super::QueueConfigData {
super::QueueConfigData {
suspend_threshold: pre.suspend_threshold,
drop_threshold: pre.drop_threshold,
resume_threshold: pre.resume_threshold,
threshold_weight: pre.threshold_weight,
weight_restrict_decay: pre.weight_restrict_decay,
xcmp_max_individual_weight: super::QueueConfigData::default()
.xcmp_max_individual_weight,
}
};
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would it make sense to parameterize the migration function to take a xcmp_max_individual_weight so that people can migrate to whatever config value they want?
Alternatively we'd want to expose some knob somewhere to update the queue config, no?
Maybe a candidate for a follow-up issue?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

added an issue: #801

Copy link
Contributor

@apopiak apopiak left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, I'll let Gav address the question in code

@gavofyork
Copy link
Member

Looks reasonable.

@KiChjang KiChjang added A0-pleasereview B0-silent Changes should not be mentioned in any release notes labels Dec 9, 2021
@KiChjang KiChjang merged commit b5a7ab4 into master Dec 9, 2021
@KiChjang KiChjang deleted the kckyeung/xcmp-overweight branch December 9, 2021 02:07
HCastano added a commit to paritytech/canvas that referenced this pull request Jan 8, 2022
cmichi pushed a commit to paritytech/canvas that referenced this pull request Jan 10, 2022
* Bump Cumulus, Polkadot, and Substrate

Also bumps some other depenencies

* Remove duplicate `polkadot` dependency

* Update `service.rs`

Changes related to: paritytech/cumulus#835

* Update `command.rs`

* Add `AddressGenerator` config type

From: paritytech/substrate#10521

* Allow Root to execute overweight XCMP messages

From: paritytech/cumulus#799

* Add `header` argument to `collect_collation_info`

From: paritytech/cumulus#882

* Update Cumulus and friends again

* Add Fork ID to genesis config

paritytech/cumulus#870

* Add `state_version` field

paritytech/substrate#9732

* Add `MaxConsumers` config parameter

paritytech/substrate#10382

* Update Substrate compatibility note in README
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
B0-silent Changes should not be mentioned in any release notes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants