From b8f55d1b7698b90a88d4514bdd97feaf140479b9 Mon Sep 17 00:00:00 2001 From: Oliver Tale-Yazdi Date: Mon, 29 Jan 2024 17:36:23 +0100 Subject: [PATCH] [Runtime] Bound XCMP queue (#2302) Remove `without_storage_info` from the XCMP queue pallet. Part of https://github.com/paritytech/polkadot-sdk/issues/323 Changes: - Limit the number of channels that can be suspended at the same time. - Limit the number of channels that can have messages or signals pending at the same time. A No-OP migration is put in place to ensure that all `BoundedVec`s still decode and not truncate after upgrade. The storage version is thereby bumped to 4 to have our tooling remind us to deploy that migration. --------- Signed-off-by: Oliver Tale-Yazdi Co-authored-by: Francisco Aguirre --- cumulus/pallets/parachain-system/src/lib.rs | 9 +- cumulus/pallets/xcmp-queue/src/lib.rs | 105 ++++++++++++---- cumulus/pallets/xcmp-queue/src/migration.rs | 26 ++-- .../pallets/xcmp-queue/src/migration/v5.rs | 115 ++++++++++++++++++ cumulus/pallets/xcmp-queue/src/mock.rs | 4 +- cumulus/pallets/xcmp-queue/src/tests.rs | 2 +- cumulus/parachain-template/runtime/src/lib.rs | 4 +- .../assets/asset-hub-rococo/src/lib.rs | 13 +- .../assets/asset-hub-westend/src/lib.rs | 13 +- .../bridge-hubs/bridge-hub-rococo/src/lib.rs | 12 +- .../bridge-hubs/bridge-hub-westend/src/lib.rs | 13 +- .../collectives-westend/src/lib.rs | 13 +- .../contracts/contracts-rococo/src/lib.rs | 3 +- .../contracts-rococo/src/xcm_config.rs | 9 +- .../coretime/coretime-rococo/src/lib.rs | 14 ++- .../coretime/coretime-westend/src/lib.rs | 9 +- .../runtimes/people/people-rococo/src/lib.rs | 9 +- .../runtimes/people/people-westend/src/lib.rs | 9 +- .../runtimes/testing/penpal/src/lib.rs | 4 +- .../testing/rococo-parachain/src/lib.rs | 4 +- cumulus/primitives/core/src/lib.rs | 8 ++ polkadot/parachain/src/primitives.rs | 14 ++- prdoc/pr_2302.prdoc | 20 +++ 23 files changed, 367 insertions(+), 65 deletions(-) create mode 100644 cumulus/pallets/xcmp-queue/src/migration/v5.rs create mode 100644 prdoc/pr_2302.prdoc diff --git a/cumulus/pallets/parachain-system/src/lib.rs b/cumulus/pallets/parachain-system/src/lib.rs index 5a0fa57fb171..05b9dd3c3b63 100644 --- a/cumulus/pallets/parachain-system/src/lib.rs +++ b/cumulus/pallets/parachain-system/src/lib.rs @@ -30,7 +30,7 @@ use codec::{Decode, Encode}; use cumulus_primitives_core::{ relay_chain, AbridgedHostConfiguration, ChannelInfo, ChannelStatus, CollationInfo, - GetChannelInfo, InboundDownwardMessage, InboundHrmpMessage, MessageSendError, + GetChannelInfo, InboundDownwardMessage, InboundHrmpMessage, ListChannelInfos, MessageSendError, OutboundHrmpMessage, ParaId, PersistedValidationData, UpwardMessage, UpwardMessageSender, XcmpMessageHandler, XcmpMessageSource, }; @@ -1023,6 +1023,13 @@ impl FeeTracker for Pallet { } } +impl ListChannelInfos for Pallet { + fn outgoing_channels() -> Vec { + let Some(state) = Self::relevant_messaging_state() else { return Vec::new() }; + state.egress_channels.into_iter().map(|(id, _)| id).collect() + } +} + impl GetChannelInfo for Pallet { fn get_channel_status(id: ParaId) -> ChannelStatus { // Note, that we are using `relevant_messaging_state` which may be from the previous diff --git a/cumulus/pallets/xcmp-queue/src/lib.rs b/cumulus/pallets/xcmp-queue/src/lib.rs index 5b900769622a..0460015da108 100644 --- a/cumulus/pallets/xcmp-queue/src/lib.rs +++ b/cumulus/pallets/xcmp-queue/src/lib.rs @@ -51,7 +51,7 @@ pub mod weights; pub use weights::WeightInfo; use bounded_collections::BoundedBTreeSet; -use codec::{Decode, DecodeLimit, Encode}; +use codec::{Decode, DecodeLimit, Encode, MaxEncodedLen}; use cumulus_primitives_core::{ relay_chain::BlockNumber as RelayBlockNumber, ChannelStatus, GetChannelInfo, MessageSendError, ParaId, XcmpMessageFormat, XcmpMessageHandler, XcmpMessageSource, @@ -105,7 +105,6 @@ pub mod pallet { #[pallet::pallet] #[pallet::storage_version(migration::STORAGE_VERSION)] - #[pallet::without_storage_info] pub struct Pallet(_); #[pallet::config] @@ -132,6 +131,25 @@ pub mod pallet { #[pallet::constant] type MaxInboundSuspended: Get; + /// Maximal number of outbound XCMP channels that can have messages queued at the same time. + /// + /// If this is reached, then no further messages can be sent to channels that do not yet + /// have a message queued. This should be set to the expected maximum of outbound channels + /// which is determined by [`Self::ChannelInfo`]. It is important to set this correctly, + /// since otherwise the congestion control protocol will not work as intended and messages + /// may be dropped. This value increases the PoV and should therefore not be picked too + /// high. + #[pallet::constant] + type MaxActiveOutboundChannels: Get; + + /// The maximal page size for HRMP message pages. + /// + /// A lower limit can be set dynamically, but this is the hard-limit for the PoV worst case + /// benchmarking. The limit for the size of a message is slightly below this, since some + /// overhead is incurred for encoding the format. + #[pallet::constant] + type MaxPageSize: Get; + /// The origin that is allowed to resume or suspend the XCMP queue. type ControllerOrigin: EnsureOrigin; @@ -276,6 +294,10 @@ pub mod pallet { AlreadySuspended, /// The execution is already resumed. AlreadyResumed, + /// There are too many active outbound channels. + TooManyOutboundChannels, + /// The message is too big. + TooBig, } /// The suspended inbound XCMP channels. All others are not suspended. @@ -297,19 +319,28 @@ pub mod pallet { /// case of the need to send a high-priority signal message this block. /// The bool is true if there is a signal message waiting to be sent. #[pallet::storage] - pub(super) type OutboundXcmpStatus = - StorageValue<_, Vec, ValueQuery>; + pub(super) type OutboundXcmpStatus = StorageValue< + _, + BoundedVec, + ValueQuery, + >; - // The new way of doing it: /// The messages outbound in a given XCMP channel. #[pallet::storage] - pub(super) type OutboundXcmpMessages = - StorageDoubleMap<_, Blake2_128Concat, ParaId, Twox64Concat, u16, Vec, ValueQuery>; + pub(super) type OutboundXcmpMessages = StorageDoubleMap< + _, + Blake2_128Concat, + ParaId, + Twox64Concat, + u16, + BoundedVec, + ValueQuery, + >; /// Any signal messages waiting to be sent. #[pallet::storage] pub(super) type SignalMessages = - StorageMap<_, Blake2_128Concat, ParaId, Vec, ValueQuery>; + StorageMap<_, Blake2_128Concat, ParaId, BoundedVec, ValueQuery>; /// The configuration which controls the dynamics of the outbound queue. #[pallet::storage] @@ -331,15 +362,14 @@ pub mod pallet { StorageMap<_, Twox64Concat, ParaId, FixedU128, ValueQuery, InitialFactor>; } -#[derive(Copy, Clone, Eq, PartialEq, Encode, Decode, RuntimeDebug, TypeInfo)] +#[derive(Copy, Clone, Eq, PartialEq, Encode, Decode, RuntimeDebug, TypeInfo, MaxEncodedLen)] pub enum OutboundState { Ok, Suspended, } /// Struct containing detailed information about the outbound channel. -#[derive(Clone, Eq, PartialEq, Encode, Decode, TypeInfo)] -#[cfg_attr(feature = "std", derive(Debug))] +#[derive(Clone, Eq, PartialEq, Encode, Decode, TypeInfo, RuntimeDebug, MaxEncodedLen)] pub struct OutboundChannelDetails { /// The `ParaId` of the parachain that this channel is connected with. recipient: ParaId, @@ -375,7 +405,7 @@ impl OutboundChannelDetails { } } -#[derive(Copy, Clone, Eq, PartialEq, Encode, Decode, RuntimeDebug, TypeInfo)] +#[derive(Copy, Clone, Eq, PartialEq, Encode, Decode, RuntimeDebug, TypeInfo, MaxEncodedLen)] pub struct QueueConfigData { /// The number of pages which must be in the queue for the other side to be told to suspend /// their sending. @@ -478,7 +508,9 @@ impl Pallet { { details } else { - all_channels.push(OutboundChannelDetails::new(recipient)); + all_channels + .try_push(OutboundChannelDetails::new(recipient)) + .map_err(|_| MessageSendError::TooManyChannels)?; all_channels .last_mut() .expect("can't be empty; a new element was just pushed; qed") @@ -503,7 +535,7 @@ impl Pallet { if page.len() + encoded_fragment.len() > max_message_size { return None } - page.extend_from_slice(&encoded_fragment[..]); + page.try_extend(encoded_fragment.iter().cloned()).ok()?; Some(page.len()) }, ) @@ -521,7 +553,9 @@ impl Pallet { new_page.extend_from_slice(&encoded_fragment[..]); let last_page_size = new_page.len(); let number_of_pages = (channel_details.last_index - channel_details.first_index) as u32; - >::insert(recipient, page_index, new_page); + let bounded_page = + BoundedVec::try_from(new_page).map_err(|_| MessageSendError::TooBig)?; + >::insert(recipient, page_index, bounded_page); >::put(all_channels); (number_of_pages, last_page_size) }; @@ -543,17 +577,21 @@ impl Pallet { /// Sends a signal to the `dest` chain over XCMP. This is guaranteed to be dispatched on this /// block. - fn send_signal(dest: ParaId, signal: ChannelSignal) { + fn send_signal(dest: ParaId, signal: ChannelSignal) -> Result<(), Error> { let mut s = >::get(); if let Some(details) = s.iter_mut().find(|item| item.recipient == dest) { details.signals_exist = true; } else { - s.push(OutboundChannelDetails::new(dest).with_signals()); + s.try_push(OutboundChannelDetails::new(dest).with_signals()) + .map_err(|_| Error::::TooManyOutboundChannels)?; } - >::mutate(dest, |page| { - *page = (XcmpMessageFormat::Signals, signal).encode(); - }); + + let page = BoundedVec::try_from((XcmpMessageFormat::Signals, signal).encode()) + .map_err(|_| Error::::TooBig)?; + + >::insert(dest, page); >::put(s); + Ok(()) } fn suspend_channel(target: ParaId) { @@ -563,7 +601,13 @@ impl Pallet { defensive_assert!(ok, "WARNING: Attempt to suspend channel that was not Ok."); details.state = OutboundState::Suspended; } else { - s.push(OutboundChannelDetails::new(target).with_suspended_state()); + if s.try_push(OutboundChannelDetails::new(target).with_suspended_state()).is_err() { + // Nothing that we can do here. The outbound channel does not exist either, so + // there should be no message going out as well. The next time that the channel + // can be created it will again get the suspension from the remote side. It can + // therefore result in a few lost messages, but should eventually self-repair. + defensive!("Cannot pause channel; too many outbound channels"); + } } }); } @@ -664,13 +708,17 @@ impl OnQueueChanged for Pallet { let suspended = suspended_channels.contains(¶); if suspended && fp.pages <= resume_threshold { - Self::send_signal(para, ChannelSignal::Resume); + // If the resuming fails then it is not critical. We will retry in the future. + let _ = Self::send_signal(para, ChannelSignal::Resume); suspended_channels.remove(¶); >::put(suspended_channels); } else if !suspended && fp.pages >= suspend_threshold { log::warn!("XCMP queue for sibling {:?} is full; suspending channel.", para); - Self::send_signal(para, ChannelSignal::Suspend); + if let Err(_) = Self::send_signal(para, ChannelSignal::Suspend) { + // It will retry if `drop_threshold` is not reached, but it could be too late. + defensive!("Could not send suspension signal; future messages may be dropped."); + } if let Err(err) = suspended_channels.try_insert(para) { log::error!("Too many channels suspended; cannot suspend sibling {:?}: {:?}; further messages may be dropped.", para, err); @@ -842,7 +890,7 @@ impl XcmpMessageSource for Pallet { // since it's so unlikely then for now we just drop it. defensive!("WARNING: oversize message in queue - dropping"); } else { - result.push((para_id, page)); + result.push((para_id, page.into_inner())); } let max_total_size = match T::ChannelInfo::get_channel_info(para_id) { @@ -890,7 +938,14 @@ impl XcmpMessageSource for Pallet { let pruned = old_statuses_len - statuses.len(); // removing an item from status implies a message being sent, so the result messages must // be no less than the pruned channels. - statuses.rotate_left(result.len().saturating_sub(pruned)); + + // TODO + { + let mut statuses_inner = statuses.into_inner(); + statuses_inner.rotate_left(result.len().saturating_sub(pruned)); + statuses = BoundedVec::try_from(statuses_inner) + .expect("Rotating does not change the length; it still fits; qed"); + } >::put(statuses); diff --git a/cumulus/pallets/xcmp-queue/src/migration.rs b/cumulus/pallets/xcmp-queue/src/migration.rs index 6c86c3011d23..7d46ded57a84 100644 --- a/cumulus/pallets/xcmp-queue/src/migration.rs +++ b/cumulus/pallets/xcmp-queue/src/migration.rs @@ -16,6 +16,8 @@ //! A module that is responsible for migration of storage. +pub mod v5; + use crate::{Config, OverweightIndex, Pallet, QueueConfig, QueueConfigData, DEFAULT_POV_SIZE}; use cumulus_primitives_core::XcmpMessageFormat; use frame_support::{ @@ -25,7 +27,7 @@ use frame_support::{ }; /// The current storage version. -pub const STORAGE_VERSION: StorageVersion = StorageVersion::new(4); +pub const STORAGE_VERSION: StorageVersion = StorageVersion::new(5); pub const LOG: &str = "runtime::xcmp-queue-migration"; @@ -264,9 +266,9 @@ pub mod v4 { /// Migrates `QueueConfigData` to v4, removing deprecated fields and bumping page /// thresholds to at least the default values. - pub struct UncheckedMigrationToV4(PhantomData); + pub struct UncheckedMigrateV3ToV4(PhantomData); - impl OnRuntimeUpgrade for UncheckedMigrationToV4 { + impl OnRuntimeUpgrade for UncheckedMigrateV3ToV4 { fn on_runtime_upgrade() -> Weight { let translate = |pre: v2::QueueConfigData| -> QueueConfigData { let pre_default = v2::QueueConfigData::default(); @@ -299,13 +301,13 @@ pub mod v4 { } } - /// [`UncheckedMigrationToV4`] wrapped in a + /// [`UncheckedMigrateV3ToV4`] wrapped in a /// [`VersionedMigration`](frame_support::migrations::VersionedMigration), ensuring the /// migration is only performed when on-chain version is 3. - pub type MigrationToV4 = frame_support::migrations::VersionedMigration< + pub type MigrateV3ToV4 = frame_support::migrations::VersionedMigration< 3, 4, - UncheckedMigrationToV4, + UncheckedMigrateV3ToV4, Pallet, ::DbWeight, >; @@ -372,10 +374,10 @@ mod tests { &v2.encode(), ); - let bytes = v4::MigrationToV4::::pre_upgrade(); + let bytes = v4::MigrateV3ToV4::::pre_upgrade(); assert!(bytes.is_ok()); - v4::MigrationToV4::::on_runtime_upgrade(); - assert!(v4::MigrationToV4::::post_upgrade(bytes.unwrap()).is_ok()); + v4::MigrateV3ToV4::::on_runtime_upgrade(); + assert!(v4::MigrateV3ToV4::::post_upgrade(bytes.unwrap()).is_ok()); let v4 = QueueConfig::::get(); @@ -401,10 +403,10 @@ mod tests { &v2.encode(), ); - let bytes = v4::MigrationToV4::::pre_upgrade(); + let bytes = v4::MigrateV3ToV4::::pre_upgrade(); assert!(bytes.is_ok()); - v4::MigrationToV4::::on_runtime_upgrade(); - assert!(v4::MigrationToV4::::post_upgrade(bytes.unwrap()).is_ok()); + v4::MigrateV3ToV4::::on_runtime_upgrade(); + assert!(v4::MigrateV3ToV4::::post_upgrade(bytes.unwrap()).is_ok()); let v4 = QueueConfig::::get(); diff --git a/cumulus/pallets/xcmp-queue/src/migration/v5.rs b/cumulus/pallets/xcmp-queue/src/migration/v5.rs new file mode 100644 index 000000000000..789b82be33f2 --- /dev/null +++ b/cumulus/pallets/xcmp-queue/src/migration/v5.rs @@ -0,0 +1,115 @@ +// Copyright (C) Parity Technologies (UK) Ltd. +// This file is part of Polkadot. + +// Polkadot is free software: you can redistribute it and/or modify +// it under the terms of the GNU General Public License as published by +// the Free Software Foundation, either version 3 of the License, or +// (at your option) any later version. + +// Polkadot is distributed in the hope that it will be useful, +// but WITHOUT ANY WARRANTY; without even the implied warranty of +// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +// GNU General Public License for more details. + +// You should have received a copy of the GNU General Public License +// along with Polkadot. If not, see . + +//! Migrates the storage to version 5. + +use crate::*; +use cumulus_primitives_core::ListChannelInfos; +use frame_support::{pallet_prelude::*, traits::OnRuntimeUpgrade}; + +/// Configs needed to run the V5 migration. +pub trait V5Config: Config { + /// List all outbound channels with their target `ParaId` and maximum message size. + type ChannelList: ListChannelInfos; +} + +/// Ensures that the storage migrates cleanly to V5. +/// +/// The migration itself is a no-op, but it checks that none of the `BoundedVec`s would truncate on +/// the next decode after the upgrade was applied. +pub type MigrateV4ToV5 = frame_support::migrations::VersionedMigration< + 4, + 5, + unversioned::UncheckedMigrateV4ToV5, + Pallet, + ::DbWeight, +>; + +// V4 storage aliases +mod v4 { + use super::*; + + #[frame_support::storage_alias] + pub(super) type OutboundXcmpStatus = + StorageValue, Vec, ValueQuery>; + + #[frame_support::storage_alias] + pub(super) type OutboundXcmpMessages = StorageDoubleMap< + Pallet, + Blake2_128Concat, + ParaId, + Twox64Concat, + u16, + Vec, + ValueQuery, + >; + + #[frame_support::storage_alias] + pub(super) type SignalMessages = + StorageMap, Blake2_128Concat, ParaId, Vec, ValueQuery>; +} + +// Private module to hide the migration. +mod unversioned { + /// Please use [`MigrateV4ToV5`] instead. + pub struct UncheckedMigrateV4ToV5(core::marker::PhantomData); +} + +impl OnRuntimeUpgrade for unversioned::UncheckedMigrateV4ToV5 { + fn on_runtime_upgrade() -> frame_support::weights::Weight { + Default::default() + } + + #[cfg(feature = "try-runtime")] + fn post_upgrade(_: Vec) -> Result<(), sp_runtime::DispatchError> { + // We dont need any front-run protection for this since channels are opened by governance. + ensure!( + v4::OutboundXcmpStatus::::get().len() as u32 <= T::MaxActiveOutboundChannels::get(), + "Too many outbound channels. Close some channels or increase `MaxActiveOutboundChannels`." + ); + + // Check if any channels have a too large message max sizes. + let max_msg_len = T::MaxPageSize::get() - XcmpMessageFormat::max_encoded_len() as u32; + for channel in T::ChannelList::outgoing_channels() { + let info = T::ChannelInfo::get_channel_info(channel) + .expect("All listed channels must provide info"); + + ensure!( + info.max_message_size <= max_msg_len, + "Max message size for channel is too large. This means that the V5 migration can \ + be front-run and an attacker could place a large message just right before the \ + migration to make other messages un-decodable. Please either increase \ + `MaxPageSize` or decrease the `max_message_size` for this channel.", + ); + } + + // Now check that all pages still fit into the new `BoundedVec`s: + for page in v4::OutboundXcmpMessages::::iter_values() { + ensure!( + page.len() < T::MaxPageSize::get() as usize, + "Too long message in storage. Either manually truncate the pages or increase `MaxPageSize`." + ); + } + for page in v4::SignalMessages::::iter_values() { + ensure!( + page.len() < T::MaxPageSize::get() as usize, + "Too long signal in storage. Either manually truncate the pages or increase `MaxPageSize`." + ); + } + + Ok(()) + } +} diff --git a/cumulus/pallets/xcmp-queue/src/mock.rs b/cumulus/pallets/xcmp-queue/src/mock.rs index f8b89258f2f6..b47014a805e0 100644 --- a/cumulus/pallets/xcmp-queue/src/mock.rs +++ b/cumulus/pallets/xcmp-queue/src/mock.rs @@ -275,7 +275,9 @@ impl Config for Test { type ChannelInfo = MockedChannelInfo; type VersionWrapper = (); type XcmpQueue = EnqueueToLocalStorage>; - type MaxInboundSuspended = sp_core::ConstU32<1_000>; + type MaxInboundSuspended = ConstU32<1_000>; + type MaxActiveOutboundChannels = ConstU32<128>; + type MaxPageSize = ConstU32<{ 1 << 16 }>; type ControllerOrigin = EnsureRoot; type ControllerOriginConverter = SystemParachainAsSuperuser; type WeightInfo = (); diff --git a/cumulus/pallets/xcmp-queue/src/tests.rs b/cumulus/pallets/xcmp-queue/src/tests.rs index 0b41095828f2..e1d772456473 100644 --- a/cumulus/pallets/xcmp-queue/src/tests.rs +++ b/cumulus/pallets/xcmp-queue/src/tests.rs @@ -520,7 +520,7 @@ fn hrmp_signals_are_prioritized() { }); // But a signal gets prioritized instead of the messages: - XcmpQueue::send_signal(sibling_para_id.into(), ChannelSignal::Suspend); + assert_ok!(XcmpQueue::send_signal(sibling_para_id.into(), ChannelSignal::Suspend)); let taken = XcmpQueue::take_outbound_messages(130); assert_eq!( diff --git a/cumulus/parachain-template/runtime/src/lib.rs b/cumulus/parachain-template/runtime/src/lib.rs index 0ab36eba315d..980211a577b4 100644 --- a/cumulus/parachain-template/runtime/src/lib.rs +++ b/cumulus/parachain-template/runtime/src/lib.rs @@ -422,7 +422,9 @@ impl cumulus_pallet_xcmp_queue::Config for Runtime { type VersionWrapper = (); // Enqueue XCMP messages from siblings for later processing. type XcmpQueue = TransformOrigin; - type MaxInboundSuspended = sp_core::ConstU32<1_000>; + type MaxInboundSuspended = ConstU32<1_000>; + type MaxActiveOutboundChannels = ConstU32<128>; + type MaxPageSize = ConstU32<{ 1 << 16 }>; type ControllerOrigin = EnsureRoot; type ControllerOriginConverter = XcmOriginToTransactDispatchOrigin; type WeightInfo = (); diff --git a/cumulus/parachains/runtimes/assets/asset-hub-rococo/src/lib.rs b/cumulus/parachains/runtimes/assets/asset-hub-rococo/src/lib.rs index d6f2f41ef312..df84a5f67e76 100644 --- a/cumulus/parachains/runtimes/assets/asset-hub-rococo/src/lib.rs +++ b/cumulus/parachains/runtimes/assets/asset-hub-rococo/src/lib.rs @@ -716,12 +716,19 @@ impl cumulus_pallet_xcmp_queue::Config for Runtime { type ChannelInfo = ParachainSystem; type VersionWrapper = PolkadotXcm; type XcmpQueue = TransformOrigin; - type MaxInboundSuspended = sp_core::ConstU32<1_000>; + type MaxInboundSuspended = ConstU32<1_000>; + type MaxActiveOutboundChannels = ConstU32<128>; + type MaxPageSize = ConstU32<{ 1 << 16 }>; type ControllerOrigin = EnsureRoot; type ControllerOriginConverter = xcm_config::XcmOriginToTransactDispatchOrigin; type PriceForSiblingDelivery = PriceForSiblingParachainDelivery; } +impl cumulus_pallet_xcmp_queue::migration::v5::V5Config for Runtime { + // This must be the same as the `ChannelInfo` from the `Config`: + type ChannelList = ParachainSystem; +} + parameter_types! { pub const RelayOrigin: AggregateMessageOrigin = AggregateMessageOrigin::Parent; } @@ -993,8 +1000,8 @@ pub type UncheckedExtrinsic = pub type Migrations = ( pallet_collator_selection::migration::v1::MigrateToV1, InitStorageVersions, - // unreleased - cumulus_pallet_xcmp_queue::migration::v4::MigrationToV4, + cumulus_pallet_xcmp_queue::migration::v4::MigrateV3ToV4, + cumulus_pallet_xcmp_queue::migration::v5::MigrateV4ToV5, ); /// Migration to initialize storage versions for pallets added after genesis. diff --git a/cumulus/parachains/runtimes/assets/asset-hub-westend/src/lib.rs b/cumulus/parachains/runtimes/assets/asset-hub-westend/src/lib.rs index 66d80fac831f..f87798ff2992 100644 --- a/cumulus/parachains/runtimes/assets/asset-hub-westend/src/lib.rs +++ b/cumulus/parachains/runtimes/assets/asset-hub-westend/src/lib.rs @@ -668,13 +668,20 @@ impl cumulus_pallet_xcmp_queue::Config for Runtime { type VersionWrapper = PolkadotXcm; // Enqueue XCMP messages from siblings for later processing. type XcmpQueue = TransformOrigin; - type MaxInboundSuspended = sp_core::ConstU32<1_000>; + type MaxInboundSuspended = ConstU32<1_000>; + type MaxActiveOutboundChannels = ConstU32<128>; + type MaxPageSize = ConstU32<{ 1 << 16 }>; type ControllerOrigin = EnsureRoot; type ControllerOriginConverter = XcmOriginToTransactDispatchOrigin; type WeightInfo = weights::cumulus_pallet_xcmp_queue::WeightInfo; type PriceForSiblingDelivery = PriceForSiblingParachainDelivery; } +impl cumulus_pallet_xcmp_queue::migration::v5::V5Config for Runtime { + // This must be the same as the `ChannelInfo` from the `Config`: + type ChannelList = ParachainSystem; +} + parameter_types! { pub const RelayOrigin: AggregateMessageOrigin = AggregateMessageOrigin::Parent; } @@ -947,8 +954,8 @@ pub type Migrations = ( InitStorageVersions, // unreleased DeleteUndecodableStorage, - // unreleased - cumulus_pallet_xcmp_queue::migration::v4::MigrationToV4, + cumulus_pallet_xcmp_queue::migration::v4::MigrateV3ToV4, + cumulus_pallet_xcmp_queue::migration::v5::MigrateV4ToV5, ); /// Asset Hub Westend has some undecodable storage, delete it. diff --git a/cumulus/parachains/runtimes/bridge-hubs/bridge-hub-rococo/src/lib.rs b/cumulus/parachains/runtimes/bridge-hubs/bridge-hub-rococo/src/lib.rs index ceba449e6b7b..61b1767cba72 100644 --- a/cumulus/parachains/runtimes/bridge-hubs/bridge-hub-rococo/src/lib.rs +++ b/cumulus/parachains/runtimes/bridge-hubs/bridge-hub-rococo/src/lib.rs @@ -144,8 +144,9 @@ pub type Migrations = ( pallet_collator_selection::migration::v1::MigrateToV1, pallet_multisig::migrations::v1::MigrateToV1, InitStorageVersions, - cumulus_pallet_xcmp_queue::migration::v4::MigrationToV4, // unreleased + cumulus_pallet_xcmp_queue::migration::v4::MigrateV3ToV4, + cumulus_pallet_xcmp_queue::migration::v5::MigrateV4ToV5, snowbridge_pallet_system::migration::v0::InitializeOnUpgrade< Runtime, ConstU32, @@ -405,13 +406,20 @@ impl cumulus_pallet_xcmp_queue::Config for Runtime { type VersionWrapper = PolkadotXcm; // Enqueue XCMP messages from siblings for later processing. type XcmpQueue = TransformOrigin; - type MaxInboundSuspended = sp_core::ConstU32<1_000>; + type MaxInboundSuspended = ConstU32<1_000>; + type MaxActiveOutboundChannels = ConstU32<128>; + type MaxPageSize = ConstU32<{ 1 << 16 }>; type ControllerOrigin = EnsureRoot; type ControllerOriginConverter = XcmOriginToTransactDispatchOrigin; type WeightInfo = weights::cumulus_pallet_xcmp_queue::WeightInfo; type PriceForSiblingDelivery = PriceForSiblingParachainDelivery; } +impl cumulus_pallet_xcmp_queue::migration::v5::V5Config for Runtime { + // This must be the same as the `ChannelInfo` from the `Config`: + type ChannelList = ParachainSystem; +} + parameter_types! { pub const RelayOrigin: AggregateMessageOrigin = AggregateMessageOrigin::Parent; } diff --git a/cumulus/parachains/runtimes/bridge-hubs/bridge-hub-westend/src/lib.rs b/cumulus/parachains/runtimes/bridge-hubs/bridge-hub-westend/src/lib.rs index 10058ef13be3..43c825383bc6 100644 --- a/cumulus/parachains/runtimes/bridge-hubs/bridge-hub-westend/src/lib.rs +++ b/cumulus/parachains/runtimes/bridge-hubs/bridge-hub-westend/src/lib.rs @@ -123,8 +123,8 @@ pub type Migrations = ( pallet_collator_selection::migration::v1::MigrateToV1, pallet_multisig::migrations::v1::MigrateToV1, InitStorageVersions, - // unreleased - cumulus_pallet_xcmp_queue::migration::v4::MigrationToV4, + cumulus_pallet_xcmp_queue::migration::v4::MigrateV3ToV4, + cumulus_pallet_xcmp_queue::migration::v5::MigrateV4ToV5, ); /// Migration to initialize storage versions for pallets added after genesis. @@ -371,13 +371,20 @@ impl cumulus_pallet_xcmp_queue::Config for Runtime { type ChannelInfo = ParachainSystem; type VersionWrapper = PolkadotXcm; type XcmpQueue = TransformOrigin; - type MaxInboundSuspended = sp_core::ConstU32<1_000>; + type MaxInboundSuspended = ConstU32<1_000>; + type MaxActiveOutboundChannels = ConstU32<128>; + type MaxPageSize = ConstU32<{ 1 << 16 }>; type ControllerOrigin = EnsureRoot; type ControllerOriginConverter = XcmOriginToTransactDispatchOrigin; type WeightInfo = weights::cumulus_pallet_xcmp_queue::WeightInfo; type PriceForSiblingDelivery = PriceForSiblingParachainDelivery; } +impl cumulus_pallet_xcmp_queue::migration::v5::V5Config for Runtime { + // This must be the same as the `ChannelInfo` from the `Config`: + type ChannelList = ParachainSystem; +} + parameter_types! { pub const RelayOrigin: AggregateMessageOrigin = AggregateMessageOrigin::Parent; } diff --git a/cumulus/parachains/runtimes/collectives/collectives-westend/src/lib.rs b/cumulus/parachains/runtimes/collectives/collectives-westend/src/lib.rs index be7beafc89af..fb8e9ddf24e2 100644 --- a/cumulus/parachains/runtimes/collectives/collectives-westend/src/lib.rs +++ b/cumulus/parachains/runtimes/collectives/collectives-westend/src/lib.rs @@ -445,13 +445,20 @@ impl cumulus_pallet_xcmp_queue::Config for Runtime { type VersionWrapper = PolkadotXcm; // Enqueue XCMP messages from siblings for later processing. type XcmpQueue = TransformOrigin; - type MaxInboundSuspended = sp_core::ConstU32<1_000>; + type MaxInboundSuspended = ConstU32<1_000>; + type MaxActiveOutboundChannels = ConstU32<128>; + type MaxPageSize = ConstU32<{ 1 << 16 }>; type ControllerOrigin = EitherOfDiverse, Fellows>; type ControllerOriginConverter = XcmOriginToTransactDispatchOrigin; type WeightInfo = weights::cumulus_pallet_xcmp_queue::WeightInfo; type PriceForSiblingDelivery = PriceForSiblingParachainDelivery; } +impl cumulus_pallet_xcmp_queue::migration::v5::V5Config for Runtime { + // This must be the same as the `ChannelInfo` from the `Config`: + type ChannelList = ParachainSystem; +} + parameter_types! { pub const RelayOrigin: AggregateMessageOrigin = AggregateMessageOrigin::Parent; } @@ -720,8 +727,8 @@ pub type UncheckedExtrinsic = type Migrations = ( // unreleased pallet_collator_selection::migration::v1::MigrateToV1, - // unreleased - cumulus_pallet_xcmp_queue::migration::v4::MigrationToV4, + cumulus_pallet_xcmp_queue::migration::v4::MigrateV3ToV4, + cumulus_pallet_xcmp_queue::migration::v5::MigrateV4ToV5, ); /// Executive: handles dispatch to the various modules. diff --git a/cumulus/parachains/runtimes/contracts/contracts-rococo/src/lib.rs b/cumulus/parachains/runtimes/contracts/contracts-rococo/src/lib.rs index f1c5acb4952f..605fa720370b 100644 --- a/cumulus/parachains/runtimes/contracts/contracts-rococo/src/lib.rs +++ b/cumulus/parachains/runtimes/contracts/contracts-rococo/src/lib.rs @@ -104,7 +104,8 @@ pub type Migrations = ( cumulus_pallet_xcmp_queue::migration::v3::MigrationToV3, pallet_contracts::Migration, // unreleased - cumulus_pallet_xcmp_queue::migration::v4::MigrationToV4, + cumulus_pallet_xcmp_queue::migration::v4::MigrateV3ToV4, + cumulus_pallet_xcmp_queue::migration::v5::MigrateV4ToV5, ); type EventRecord = frame_system::EventRecord< diff --git a/cumulus/parachains/runtimes/contracts/contracts-rococo/src/xcm_config.rs b/cumulus/parachains/runtimes/contracts/contracts-rococo/src/xcm_config.rs index 8c0d681361c7..9590f36e6374 100644 --- a/cumulus/parachains/runtimes/contracts/contracts-rococo/src/xcm_config.rs +++ b/cumulus/parachains/runtimes/contracts/contracts-rococo/src/xcm_config.rs @@ -281,7 +281,9 @@ impl cumulus_pallet_xcmp_queue::Config for Runtime { cumulus_primitives_core::ParaId, parachains_common::message_queue::ParaIdToSibling, >; - type MaxInboundSuspended = sp_core::ConstU32<1_000>; + type MaxInboundSuspended = ConstU32<1_000>; + type MaxActiveOutboundChannels = ConstU32<128>; + type MaxPageSize = ConstU32<{ 1 << 16 }>; type ControllerOrigin = EitherOfDiverse< EnsureRoot, EnsureXcm>, @@ -291,6 +293,11 @@ impl cumulus_pallet_xcmp_queue::Config for Runtime { type PriceForSiblingDelivery = PriceForSiblingParachainDelivery; } +impl cumulus_pallet_xcmp_queue::migration::v5::V5Config for Runtime { + // This must be the same as the `ChannelInfo` from the `Config`: + type ChannelList = ParachainSystem; +} + parameter_types! { pub const RelayOrigin: AggregateMessageOrigin = AggregateMessageOrigin::Parent; } diff --git a/cumulus/parachains/runtimes/coretime/coretime-rococo/src/lib.rs b/cumulus/parachains/runtimes/coretime/coretime-rococo/src/lib.rs index 531a0ffe4f86..06475849a5d2 100644 --- a/cumulus/parachains/runtimes/coretime/coretime-rococo/src/lib.rs +++ b/cumulus/parachains/runtimes/coretime/coretime-rococo/src/lib.rs @@ -106,7 +106,10 @@ pub type UncheckedExtrinsic = generic::UncheckedExtrinsic; /// Migrations to apply on runtime upgrade. -pub type Migrations = (cumulus_pallet_xcmp_queue::migration::v4::MigrationToV4,); +pub type Migrations = ( + cumulus_pallet_xcmp_queue::migration::v4::MigrateV3ToV4, + cumulus_pallet_xcmp_queue::migration::v5::MigrateV4ToV5, +); /// Executive: handles dispatch to the various modules. pub type Executive = frame_executive::Executive< @@ -332,13 +335,20 @@ impl cumulus_pallet_xcmp_queue::Config for Runtime { type ChannelInfo = ParachainSystem; type VersionWrapper = PolkadotXcm; type XcmpQueue = TransformOrigin; - type MaxInboundSuspended = sp_core::ConstU32<1_000>; + type MaxInboundSuspended = ConstU32<1_000>; + type MaxActiveOutboundChannels = ConstU32<128>; + type MaxPageSize = ConstU32<{ 1 << 16 }>; type ControllerOrigin = RootOrFellows; type ControllerOriginConverter = XcmOriginToTransactDispatchOrigin; type WeightInfo = weights::cumulus_pallet_xcmp_queue::WeightInfo; type PriceForSiblingDelivery = PriceForSiblingParachainDelivery; } +impl cumulus_pallet_xcmp_queue::migration::v5::V5Config for Runtime { + // This must be the same as the `ChannelInfo` from the `Config`: + type ChannelList = ParachainSystem; +} + pub const PERIOD: u32 = 6 * HOURS; pub const OFFSET: u32 = 0; diff --git a/cumulus/parachains/runtimes/coretime/coretime-westend/src/lib.rs b/cumulus/parachains/runtimes/coretime/coretime-westend/src/lib.rs index 892492724141..96b0d2b476d6 100644 --- a/cumulus/parachains/runtimes/coretime/coretime-westend/src/lib.rs +++ b/cumulus/parachains/runtimes/coretime/coretime-westend/src/lib.rs @@ -323,13 +323,20 @@ impl cumulus_pallet_xcmp_queue::Config for Runtime { type ChannelInfo = ParachainSystem; type VersionWrapper = PolkadotXcm; type XcmpQueue = TransformOrigin; - type MaxInboundSuspended = sp_core::ConstU32<1_000>; + type MaxInboundSuspended = ConstU32<1_000>; + type MaxActiveOutboundChannels = ConstU32<128>; + type MaxPageSize = ConstU32<{ 1 << 16 }>; type ControllerOrigin = RootOrFellows; type ControllerOriginConverter = XcmOriginToTransactDispatchOrigin; type WeightInfo = weights::cumulus_pallet_xcmp_queue::WeightInfo; type PriceForSiblingDelivery = PriceForSiblingParachainDelivery; } +impl cumulus_pallet_xcmp_queue::migration::v5::V5Config for Runtime { + // This must be the same as the `ChannelInfo` from the `Config`: + type ChannelList = ParachainSystem; +} + pub const PERIOD: u32 = 6 * HOURS; pub const OFFSET: u32 = 0; diff --git a/cumulus/parachains/runtimes/people/people-rococo/src/lib.rs b/cumulus/parachains/runtimes/people/people-rococo/src/lib.rs index b21fdc6a68c9..c038f8b4176d 100644 --- a/cumulus/parachains/runtimes/people/people-rococo/src/lib.rs +++ b/cumulus/parachains/runtimes/people/people-rococo/src/lib.rs @@ -301,13 +301,20 @@ impl cumulus_pallet_xcmp_queue::Config for Runtime { type ChannelInfo = ParachainSystem; type VersionWrapper = PolkadotXcm; type XcmpQueue = TransformOrigin; - type MaxInboundSuspended = sp_core::ConstU32<1_000>; + type MaxInboundSuspended = ConstU32<1_000>; + type MaxActiveOutboundChannels = ConstU32<128>; + type MaxPageSize = ConstU32<{ 1 << 16 }>; type ControllerOrigin = RootOrFellows; type ControllerOriginConverter = XcmOriginToTransactDispatchOrigin; type PriceForSiblingDelivery = PriceForSiblingParachainDelivery; type WeightInfo = weights::cumulus_pallet_xcmp_queue::WeightInfo; } +impl cumulus_pallet_xcmp_queue::migration::v5::V5Config for Runtime { + // This must be the same as the `ChannelInfo` from the `Config`: + type ChannelList = ParachainSystem; +} + pub const PERIOD: u32 = 6 * HOURS; pub const OFFSET: u32 = 0; diff --git a/cumulus/parachains/runtimes/people/people-westend/src/lib.rs b/cumulus/parachains/runtimes/people/people-westend/src/lib.rs index 20d8973417a3..4d1f82be97f4 100644 --- a/cumulus/parachains/runtimes/people/people-westend/src/lib.rs +++ b/cumulus/parachains/runtimes/people/people-westend/src/lib.rs @@ -301,13 +301,20 @@ impl cumulus_pallet_xcmp_queue::Config for Runtime { type ChannelInfo = ParachainSystem; type VersionWrapper = PolkadotXcm; type XcmpQueue = TransformOrigin; - type MaxInboundSuspended = sp_core::ConstU32<1_000>; + type MaxInboundSuspended = ConstU32<1_000>; + type MaxActiveOutboundChannels = ConstU32<128>; + type MaxPageSize = ConstU32<{ 1 << 16 }>; type ControllerOrigin = RootOrFellows; type ControllerOriginConverter = XcmOriginToTransactDispatchOrigin; type WeightInfo = weights::cumulus_pallet_xcmp_queue::WeightInfo; type PriceForSiblingDelivery = PriceForSiblingParachainDelivery; } +impl cumulus_pallet_xcmp_queue::migration::v5::V5Config for Runtime { + // This must be the same as the `ChannelInfo` from the `Config`: + type ChannelList = ParachainSystem; +} + pub const PERIOD: u32 = 6 * HOURS; pub const OFFSET: u32 = 0; diff --git a/cumulus/parachains/runtimes/testing/penpal/src/lib.rs b/cumulus/parachains/runtimes/testing/penpal/src/lib.rs index d33f51df5a9a..e38ef33821d0 100644 --- a/cumulus/parachains/runtimes/testing/penpal/src/lib.rs +++ b/cumulus/parachains/runtimes/testing/penpal/src/lib.rs @@ -549,7 +549,9 @@ impl cumulus_pallet_xcmp_queue::Config for Runtime { type VersionWrapper = PolkadotXcm; // Enqueue XCMP messages from siblings for later processing. type XcmpQueue = TransformOrigin; - type MaxInboundSuspended = sp_core::ConstU32<1_000>; + type MaxInboundSuspended = ConstU32<1_000>; + type MaxActiveOutboundChannels = ConstU32<128>; + type MaxPageSize = ConstU32<{ 1 << 16 }>; type ControllerOrigin = EnsureRoot; type ControllerOriginConverter = XcmOriginToTransactDispatchOrigin; type WeightInfo = (); diff --git a/cumulus/parachains/runtimes/testing/rococo-parachain/src/lib.rs b/cumulus/parachains/runtimes/testing/rococo-parachain/src/lib.rs index 253a492871b2..985cccdf5368 100644 --- a/cumulus/parachains/runtimes/testing/rococo-parachain/src/lib.rs +++ b/cumulus/parachains/runtimes/testing/rococo-parachain/src/lib.rs @@ -542,7 +542,9 @@ impl cumulus_pallet_xcmp_queue::Config for Runtime { type VersionWrapper = (); // Enqueue XCMP messages from siblings for later processing. type XcmpQueue = TransformOrigin; - type MaxInboundSuspended = sp_core::ConstU32<1_000>; + type MaxInboundSuspended = ConstU32<1_000>; + type MaxActiveOutboundChannels = ConstU32<128>; + type MaxPageSize = ConstU32<{ 1 << 16 }>; type ControllerOrigin = EnsureRoot; type ControllerOriginConverter = XcmOriginToTransactDispatchOrigin; type WeightInfo = cumulus_pallet_xcmp_queue::weights::SubstrateWeight; diff --git a/cumulus/primitives/core/src/lib.rs b/cumulus/primitives/core/src/lib.rs index 7f7353685657..29216d513465 100644 --- a/cumulus/primitives/core/src/lib.rs +++ b/cumulus/primitives/core/src/lib.rs @@ -64,6 +64,8 @@ pub enum MessageSendError { TooBig, /// Some other error. Other, + /// There are too many channels open at once. + TooManyChannels, } impl From for &'static str { @@ -74,6 +76,7 @@ impl From for &'static str { NoChannel => "NoChannel", TooBig => "TooBig", Other => "Other", + TooManyChannels => "TooManyChannels", } } } @@ -135,6 +138,11 @@ pub trait GetChannelInfo { fn get_channel_info(id: ParaId) -> Option; } +/// List all open outgoing channels. +pub trait ListChannelInfos { + fn outgoing_channels() -> Vec; +} + /// Something that should be called when sending an upward message. pub trait UpwardMessageSender { /// Send the given UMP message; return the expected number of blocks before the message will diff --git a/polkadot/parachain/src/primitives.rs b/polkadot/parachain/src/primitives.rs index 5a1efdf89821..276438436372 100644 --- a/polkadot/parachain/src/primitives.rs +++ b/polkadot/parachain/src/primitives.rs @@ -333,7 +333,19 @@ impl DmpMessageHandler for () { } /// The aggregate XCMP message format. -#[derive(Copy, Clone, Eq, PartialEq, Ord, PartialOrd, Encode, Decode, TypeInfo, RuntimeDebug)] +#[derive( + Copy, + Clone, + Eq, + PartialEq, + Ord, + PartialOrd, + Encode, + Decode, + TypeInfo, + RuntimeDebug, + MaxEncodedLen, +)] pub enum XcmpMessageFormat { /// Encoded `VersionedXcm` messages, all concatenated. ConcatenatedVersionedXcm, diff --git a/prdoc/pr_2302.prdoc b/prdoc/pr_2302.prdoc new file mode 100644 index 000000000000..b11129b9ed06 --- /dev/null +++ b/prdoc/pr_2302.prdoc @@ -0,0 +1,20 @@ +title: Storage bound the XCMP queue pallet + +doc: + - audience: Runtime Dev + description: | + Enforce upper limits for the number of suspended channels and on + the number of channels that have messages pending. This is needed to use + the worst-case PoV benchmarking. + +migrations: + db: [] + + runtime: + - reference: cumulus_pallet_xcmp_queue::migration::v5::MigrateV4ToV5 + description: A No-OP migration is deployed to ensure that all `BoundedVec`s` still decode as expected. + +crates: + - name: cumulus-pallet-xcmp-queue + +host_functions: []