From 5ee6dc0d9c96c5f152af131abb651bbd88eefce7 Mon Sep 17 00:00:00 2001 From: alindima Date: Tue, 1 Oct 2024 13:49:03 +0300 Subject: [PATCH 1/9] parachain-system: send core selector ump signal --- .../consensus/aura/src/collators/lookahead.rs | 7 ++-- cumulus/pallets/parachain-system/Cargo.toml | 2 ++ cumulus/pallets/parachain-system/src/lib.rs | 32 +++++++++++++++++-- 3 files changed, 35 insertions(+), 6 deletions(-) diff --git a/cumulus/client/consensus/aura/src/collators/lookahead.rs b/cumulus/client/consensus/aura/src/collators/lookahead.rs index 322baaa01498..8ac43fbd116e 100644 --- a/cumulus/client/consensus/aura/src/collators/lookahead.rs +++ b/cumulus/client/consensus/aura/src/collators/lookahead.rs @@ -36,7 +36,9 @@ use cumulus_client_collator::service::ServiceInterface as CollatorServiceInterfa use cumulus_client_consensus_common::{self as consensus_common, ParachainBlockImportMarker}; use cumulus_client_consensus_proposer::ProposerInterface; use cumulus_primitives_aura::AuraUnincludedSegmentApi; -use cumulus_primitives_core::{ClaimQueueOffset, CollectCollationInfo, PersistedValidationData}; +use cumulus_primitives_core::{ + ClaimQueueOffset, CollectCollationInfo, PersistedValidationData, DEFAULT_CLAIM_QUEUE_OFFSET, +}; use cumulus_relay_chain_interface::RelayChainInterface; use polkadot_node_primitives::{PoV, SubmitCollationParams}; @@ -260,8 +262,7 @@ where relay_parent, params.para_id, &mut params.relay_client, - // Use depth 0, to preserve behaviour. - ClaimQueueOffset(0), + ClaimQueueOffset(DEFAULT_CLAIM_QUEUE_OFFSET), ) .await .get(0) diff --git a/cumulus/pallets/parachain-system/Cargo.toml b/cumulus/pallets/parachain-system/Cargo.toml index 66429625d5bf..3cb0394c4b95 100644 --- a/cumulus/pallets/parachain-system/Cargo.toml +++ b/cumulus/pallets/parachain-system/Cargo.toml @@ -121,3 +121,5 @@ try-runtime = [ "polkadot-runtime-parachains/try-runtime", "sp-runtime/try-runtime", ] + +experimental-ump-signals = [] diff --git a/cumulus/pallets/parachain-system/src/lib.rs b/cumulus/pallets/parachain-system/src/lib.rs index 21af35fe3de3..d16819ce483e 100644 --- a/cumulus/pallets/parachain-system/src/lib.rs +++ b/cumulus/pallets/parachain-system/src/lib.rs @@ -193,6 +193,7 @@ pub mod ump_constants { /// Trait for selecting the next core to build the candidate for. pub trait SelectCore { + fn select_core() -> (CoreSelector, ClaimQueueOffset); fn select_core_for_child() -> (CoreSelector, ClaimQueueOffset); } @@ -200,6 +201,12 @@ pub trait SelectCore { pub struct DefaultCoreSelector(PhantomData); impl SelectCore for DefaultCoreSelector { + fn select_core() -> (CoreSelector, ClaimQueueOffset) { + let core_selector: U256 = frame_system::Pallet::::block_number().into(); + + (CoreSelector(core_selector.byte(0)), ClaimQueueOffset(DEFAULT_CLAIM_QUEUE_OFFSET)) + } + fn select_core_for_child() -> (CoreSelector, ClaimQueueOffset) { let core_selector: U256 = (frame_system::Pallet::::block_number() + One::one()).into(); @@ -365,6 +372,11 @@ pub mod pallet { UpwardMessages::::put(&up[..num as usize]); *up = up.split_off(num as usize); + // Send the core selector UMP signal. This is experimental until relay chain + // validators are upgraded to handle ump signals. + #[cfg(feature = "experimental-ump-signals")] + Self::send_ump_signals(); + // If the total size of the pending messages is less than the threshold, // we decrease the fee factor, since the queue is less congested. // This makes delivery of new messages cheaper. @@ -390,8 +402,7 @@ pub mod pallet { let maximum_channels = host_config .hrmp_max_message_num_per_candidate - .min(>::take()) - as usize; + .min(>::take()) as usize; // Note: this internally calls the `GetChannelInfo` implementation for this // pallet, which draws on the `RelevantMessagingState`. That in turn has @@ -1397,7 +1408,7 @@ impl Pallet { } } - /// Returns the core selector. + /// Returns the core selector for the next block. pub fn core_selector() -> (CoreSelector, ClaimQueueOffset) { T::SelectCore::select_core_for_child() } @@ -1418,6 +1429,21 @@ impl Pallet { CustomValidationHeadData::::put(head_data); } + /// Send the ump signals + #[cfg(feature = "experimental-ump-signals")] + fn send_ump_signals() { + use cumulus_primitives_core::relay_chain::vstaging::{UMPSignal, UMP_SEPARATOR}; + + UpwardMessages::::mutate(|up| { + up.push(UMP_SEPARATOR); + + // Send the core selector signal. + let core_selector = T::SelectCore::select_core(); + up.push(UMPSignal::SelectCore(core_selector.0, core_selector.1).encode()); + s + }); + } + /// Open HRMP channel for using it in benchmarks or tests. /// /// The caller assumes that the pallet will accept regular outbound message to the sibling From e15138d9d466275c16bcc2d603faed8e75558264 Mon Sep 17 00:00:00 2001 From: GitHub Action Date: Tue, 1 Oct 2024 12:19:45 +0000 Subject: [PATCH 2/9] Update from alindima running command 'prdoc --pr 5888' --- prdoc/pr_5888.prdoc | 12 ++++++++++++ 1 file changed, 12 insertions(+) create mode 100644 prdoc/pr_5888.prdoc diff --git a/prdoc/pr_5888.prdoc b/prdoc/pr_5888.prdoc new file mode 100644 index 000000000000..519704a1fa29 --- /dev/null +++ b/prdoc/pr_5888.prdoc @@ -0,0 +1,12 @@ +title: 'parachain-system: send core selector ump signal' +doc: +- audience: TODO + description: |- + Runtime side of https://github.com/paritytech/polkadot-sdk/issues/5048 + + Send the core selector ump signal in cumulus. Guarded by a compile time feature until nodes are upgraded to a version that includes https://github.com/paritytech/polkadot-sdk/pull/5423 for gracefully handling ump signals. +crates: +- name: cumulus-client-consensus-aura + bump: TODO +- name: cumulus-pallet-parachain-system + bump: TODO From 7a5f2f7a76dab9a902cfb1af7d054f2007915507 Mon Sep 17 00:00:00 2001 From: alindima Date: Tue, 1 Oct 2024 17:46:01 +0300 Subject: [PATCH 3/9] fill in prdoc --- prdoc/pr_5888.prdoc | 18 ++++++++++-------- 1 file changed, 10 insertions(+), 8 deletions(-) diff --git a/prdoc/pr_5888.prdoc b/prdoc/pr_5888.prdoc index 519704a1fa29..38c1328cd2a2 100644 --- a/prdoc/pr_5888.prdoc +++ b/prdoc/pr_5888.prdoc @@ -1,12 +1,14 @@ title: 'parachain-system: send core selector ump signal' + doc: -- audience: TODO - description: |- - Runtime side of https://github.com/paritytech/polkadot-sdk/issues/5048 + - audience: Runtime Dev + description: | + Send the core selector ump signal in cumulus. Guarded by a compile time feature called `experimental-ump-signals` + until nodes are upgraded to a version that includes https://github.com/paritytech/polkadot-sdk/pull/5423 for + gracefully handling ump signals. - Send the core selector ump signal in cumulus. Guarded by a compile time feature until nodes are upgraded to a version that includes https://github.com/paritytech/polkadot-sdk/pull/5423 for gracefully handling ump signals. crates: -- name: cumulus-client-consensus-aura - bump: TODO -- name: cumulus-pallet-parachain-system - bump: TODO + - name: cumulus-client-consensus-aura + bump: minor + - name: cumulus-pallet-parachain-system + bump: major From 4715230df45ad85068b56b91c9be81d7262fde6a Mon Sep 17 00:00:00 2001 From: alindima Date: Tue, 1 Oct 2024 17:58:15 +0300 Subject: [PATCH 4/9] signals -> signal --- cumulus/pallets/parachain-system/src/lib.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/cumulus/pallets/parachain-system/src/lib.rs b/cumulus/pallets/parachain-system/src/lib.rs index d16819ce483e..3ebf7ee3c52b 100644 --- a/cumulus/pallets/parachain-system/src/lib.rs +++ b/cumulus/pallets/parachain-system/src/lib.rs @@ -375,7 +375,7 @@ pub mod pallet { // Send the core selector UMP signal. This is experimental until relay chain // validators are upgraded to handle ump signals. #[cfg(feature = "experimental-ump-signals")] - Self::send_ump_signals(); + Self::send_ump_signal(); // If the total size of the pending messages is less than the threshold, // we decrease the fee factor, since the queue is less congested. @@ -1431,7 +1431,7 @@ impl Pallet { /// Send the ump signals #[cfg(feature = "experimental-ump-signals")] - fn send_ump_signals() { + fn send_ump_signal() { use cumulus_primitives_core::relay_chain::vstaging::{UMPSignal, UMP_SEPARATOR}; UpwardMessages::::mutate(|up| { From ebf75625550095c21b7b474b6be5f4183731253c Mon Sep 17 00:00:00 2001 From: alindima Date: Tue, 1 Oct 2024 18:24:11 +0300 Subject: [PATCH 5/9] fix typo --- cumulus/pallets/parachain-system/src/lib.rs | 1 - 1 file changed, 1 deletion(-) diff --git a/cumulus/pallets/parachain-system/src/lib.rs b/cumulus/pallets/parachain-system/src/lib.rs index 3ebf7ee3c52b..a8e6305dccdc 100644 --- a/cumulus/pallets/parachain-system/src/lib.rs +++ b/cumulus/pallets/parachain-system/src/lib.rs @@ -1440,7 +1440,6 @@ impl Pallet { // Send the core selector signal. let core_selector = T::SelectCore::select_core(); up.push(UMPSignal::SelectCore(core_selector.0, core_selector.1).encode()); - s }); } From e4a962b0b571e195841c209709343cbde4a366d5 Mon Sep 17 00:00:00 2001 From: alindima Date: Thu, 3 Oct 2024 11:55:26 +0300 Subject: [PATCH 6/9] review comments --- cumulus/pallets/parachain-system/src/lib.rs | 17 ++++++++++------- 1 file changed, 10 insertions(+), 7 deletions(-) diff --git a/cumulus/pallets/parachain-system/src/lib.rs b/cumulus/pallets/parachain-system/src/lib.rs index a8e6305dccdc..029a7b826f74 100644 --- a/cumulus/pallets/parachain-system/src/lib.rs +++ b/cumulus/pallets/parachain-system/src/lib.rs @@ -193,21 +193,23 @@ pub mod ump_constants { /// Trait for selecting the next core to build the candidate for. pub trait SelectCore { - fn select_core() -> (CoreSelector, ClaimQueueOffset); - fn select_core_for_child() -> (CoreSelector, ClaimQueueOffset); + /// Core selector information for the current block. + fn selected_core() -> (CoreSelector, ClaimQueueOffset); + /// Core selector information for the next block. + fn select_next_core() -> (CoreSelector, ClaimQueueOffset); } /// The default core selection policy. pub struct DefaultCoreSelector(PhantomData); impl SelectCore for DefaultCoreSelector { - fn select_core() -> (CoreSelector, ClaimQueueOffset) { + fn selected_core() -> (CoreSelector, ClaimQueueOffset) { let core_selector: U256 = frame_system::Pallet::::block_number().into(); (CoreSelector(core_selector.byte(0)), ClaimQueueOffset(DEFAULT_CLAIM_QUEUE_OFFSET)) } - fn select_core_for_child() -> (CoreSelector, ClaimQueueOffset) { + fn select_next_core() -> (CoreSelector, ClaimQueueOffset) { let core_selector: U256 = (frame_system::Pallet::::block_number() + One::one()).into(); (CoreSelector(core_selector.byte(0)), ClaimQueueOffset(DEFAULT_CLAIM_QUEUE_OFFSET)) @@ -402,7 +404,8 @@ pub mod pallet { let maximum_channels = host_config .hrmp_max_message_num_per_candidate - .min(>::take()) as usize; + .min(>::take()) + as usize; // Note: this internally calls the `GetChannelInfo` implementation for this // pallet, which draws on the `RelevantMessagingState`. That in turn has @@ -1410,7 +1413,7 @@ impl Pallet { /// Returns the core selector for the next block. pub fn core_selector() -> (CoreSelector, ClaimQueueOffset) { - T::SelectCore::select_core_for_child() + T::SelectCore::select_next_core() } /// Set a custom head data that should be returned as result of `validate_block`. @@ -1438,7 +1441,7 @@ impl Pallet { up.push(UMP_SEPARATOR); // Send the core selector signal. - let core_selector = T::SelectCore::select_core(); + let core_selector = T::SelectCore::selected_core(); up.push(UMPSignal::SelectCore(core_selector.0, core_selector.1).encode()); }); } From f2e75ce32d27b9026192edfa4c076c3cd3b19179 Mon Sep 17 00:00:00 2001 From: alindima Date: Thu, 3 Oct 2024 12:07:54 +0300 Subject: [PATCH 7/9] DEFAULT_CLAIM_QUEUE_OFFSET of 0 --- cumulus/primitives/core/src/lib.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cumulus/primitives/core/src/lib.rs b/cumulus/primitives/core/src/lib.rs index 6bad65b3ff2c..dfb574ef3301 100644 --- a/cumulus/primitives/core/src/lib.rs +++ b/cumulus/primitives/core/src/lib.rs @@ -335,7 +335,7 @@ pub mod rpsr_digest { /// The default claim queue offset to be used if it's not configured/accessible in the parachain /// runtime -pub const DEFAULT_CLAIM_QUEUE_OFFSET: u8 = 1; +pub const DEFAULT_CLAIM_QUEUE_OFFSET: u8 = 0; /// Information about a collation. /// From ecb5eab18410480e0c3268026c5ba5547e209fc7 Mon Sep 17 00:00:00 2001 From: alindima Date: Thu, 3 Oct 2024 12:11:26 +0300 Subject: [PATCH 8/9] add lookahead core selector --- cumulus/pallets/parachain-system/src/lib.rs | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) diff --git a/cumulus/pallets/parachain-system/src/lib.rs b/cumulus/pallets/parachain-system/src/lib.rs index 029a7b826f74..a4b505b98e54 100644 --- a/cumulus/pallets/parachain-system/src/lib.rs +++ b/cumulus/pallets/parachain-system/src/lib.rs @@ -216,6 +216,23 @@ impl SelectCore for DefaultCoreSelector { } } +/// Core selection policy that builds on claim queue offset 1. +pub struct LookaheadCoreSelector(PhantomData); + +impl SelectCore for LookaheadCoreSelector { + fn selected_core() -> (CoreSelector, ClaimQueueOffset) { + let core_selector: U256 = frame_system::Pallet::::block_number().into(); + + (CoreSelector(core_selector.byte(0)), ClaimQueueOffset(1)) + } + + fn select_next_core() -> (CoreSelector, ClaimQueueOffset) { + let core_selector: U256 = (frame_system::Pallet::::block_number() + One::one()).into(); + + (CoreSelector(core_selector.byte(0)), ClaimQueueOffset(1)) + } +} + #[frame_support::pallet] pub mod pallet { use super::*; From 55133bd72cc8b43d62f4416304bb766949959e15 Mon Sep 17 00:00:00 2001 From: alindima Date: Thu, 3 Oct 2024 13:42:25 +0300 Subject: [PATCH 9/9] amend prdoc --- prdoc/pr_5888.prdoc | 2 ++ 1 file changed, 2 insertions(+) diff --git a/prdoc/pr_5888.prdoc b/prdoc/pr_5888.prdoc index 38c1328cd2a2..9552eada6915 100644 --- a/prdoc/pr_5888.prdoc +++ b/prdoc/pr_5888.prdoc @@ -12,3 +12,5 @@ crates: bump: minor - name: cumulus-pallet-parachain-system bump: major + - name: cumulus-primitives-core + bump: minor