From 5168cf507c746bbb6eb5d4bdccab92be7ac75995 Mon Sep 17 00:00:00 2001 From: Keith Yeung Date: Wed, 24 Nov 2021 18:42:59 -0800 Subject: [PATCH 1/7] Support overweight messages in XCMP queue --- pallets/xcmp-queue/src/lib.rs | 140 +++++++++++++++--- pallets/xcmp-queue/src/mock.rs | 2 + pallets/xcmp-queue/src/tests.rs | 4 +- parachain-template/runtime/src/lib.rs | 1 + .../rococo-parachain/src/lib.rs | 1 + polkadot-parachains/statemine/src/lib.rs | 1 + polkadot-parachains/statemint/src/lib.rs | 1 + polkadot-parachains/westmint/src/lib.rs | 1 + 8 files changed, 131 insertions(+), 20 deletions(-) diff --git a/pallets/xcmp-queue/src/lib.rs b/pallets/xcmp-queue/src/lib.rs index 4ef989dae43..06f07472f13 100644 --- a/pallets/xcmp-queue/src/lib.rs +++ b/pallets/xcmp-queue/src/lib.rs @@ -36,7 +36,7 @@ use cumulus_primitives_core::{ relay_chain::BlockNumber as RelayBlockNumber, ChannelStatus, GetChannelInfo, MessageSendError, ParaId, XcmpMessageFormat, XcmpMessageHandler, XcmpMessageSource, }; -use frame_support::weights::Weight; +use frame_support::weights::{constants::WEIGHT_PER_MILLIS, Weight}; use rand_chacha::{ rand_core::{RngCore, SeedableRng}, ChaChaRng, @@ -48,6 +48,9 @@ use xcm::{latest::prelude::*, VersionedXcm, WrapVersion, MAX_XCM_DECODE_DEPTH}; pub use pallet::*; +/// Index used to identify overweight XCMs. +pub type OverweightIndex = u64; + #[frame_support::pallet] pub mod pallet { use super::*; @@ -70,18 +73,9 @@ pub mod pallet { /// Means of converting an `Xcm` into a `VersionedXcm`. type VersionWrapper: WrapVersion; - } - impl Default for QueueConfigData { - fn default() -> Self { - Self { - suspend_threshold: 2, - drop_threshold: 5, - resume_threshold: 1, - threshold_weight: 100_000, - weight_restrict_decay: 2, - } - } + /// The origin that is allowed to execute overweight messages. + type ExecuteOverweightOrigin: EnsureOrigin; } #[pallet::hooks] @@ -93,7 +87,40 @@ pub mod pallet { } #[pallet::call] - impl Pallet {} + impl Pallet { + /// Services a single overweight XCM. + /// + /// - `origin`: Must pass `ExecuteOverweightOrigin`. + /// - `index`: The index of the overweight XCM to service + /// - `weight_limit`: The amount of weight that XCM execution may take. + /// + /// Errors: + /// - `BadOverweightIndex`: XCM under `index` is not found in the `Overweight` storage map. + /// - `BadXcm`: XCM under `index` cannot be properly decoded into a valid XCM format. + /// - `WeightOverLimit`: XCM execution may use greater `weight_limit`. + /// + /// Events: + /// - `OverweightServiced`: On success. + #[pallet::weight(weight_limit.saturating_add(1_000_000))] + pub fn service_overweight( + origin: OriginFor, + index: OverweightIndex, + weight_limit: Weight, + ) -> DispatchResultWithPostInfo { + T::ExecuteOverweightOrigin::ensure_origin(origin)?; + + let (sender, sent_at, data) = + Overweight::::get(index).ok_or(Error::::BadOverweightIndex)?; + let xcm = + VersionedXcm::::decode_all_with_depth_limit(MAX_XCM_DECODE_DEPTH, &data) + .map_err(|_| Error::::BadXcm)?; + let used = Self::handle_xcm_message(sender, sent_at, xcm, weight_limit) + .map_err(|_| Error::::WeightOverLimit)?; + Overweight::::remove(index); + Self::deposit_event(Event::OverweightServiced(index, used)); + Ok(Some(used.saturating_add(1_000_000)).into()) + } + } #[pallet::event] #[pallet::generate_deposit(pub(super) fn deposit_event)] @@ -110,6 +137,10 @@ pub mod pallet { UpwardMessageSent(Option), /// An HRMP message was sent to a sibling parachain. XcmpMessageSent(Option), + /// An XCM exceeded the individual message weight budget. + OverweightEnqueued(ParaId, RelayBlockNumber, OverweightIndex, Weight), + /// An XCM from the overweight queue was executed with the given actual weight used. + OverweightServiced(OverweightIndex, Weight), } #[pallet::error] @@ -120,6 +151,10 @@ pub mod pallet { BadXcmOrigin, /// Bad XCM data. BadXcm, + /// Bad overweight index. + BadOverweightIndex, + /// Provided weight is possibly not enough to execute the message. + WeightOverLimit, } /// Status of the inbound XCMP channels. @@ -166,6 +201,19 @@ pub mod pallet { /// The configuration which controls the dynamics of the outbound queue. #[pallet::storage] pub(super) type QueueConfig = StorageValue<_, QueueConfigData, ValueQuery>; + + /// The messages that exceeded max individual message weight budget. + /// + /// These message stay in this storage map until they are manually dispatched via + /// `service_overweight`. + #[pallet::storage] + pub(super) type Overweight = + StorageMap<_, Twox64Concat, OverweightIndex, (ParaId, RelayBlockNumber, Vec)>; + + /// The number of overweight messages ever recorded in `Overweight`. Also doubles as the next + /// available free overweight index. + #[pallet::storage] + pub(super) type OverweightCount = StorageValue<_, OverweightIndex, ValueQuery>; } #[derive(Copy, Clone, Eq, PartialEq, Ord, PartialOrd, Encode, Decode, RuntimeDebug, TypeInfo)] @@ -196,6 +244,22 @@ pub struct QueueConfigData { /// The speed to which the available weight approaches the maximum weight. A lower number /// results in a faster progression. A value of 1 makes the entire weight available initially. weight_restrict_decay: Weight, + /// The maximum amount of weight any individual message may consume. Messages above this weight + /// go into the overweight queue and may only be serviced explicitly. + xcmp_max_individual_weight: Weight, +} + +impl Default for QueueConfigData { + fn default() -> Self { + Self { + suspend_threshold: 2, + drop_threshold: 5, + resume_threshold: 1, + threshold_weight: 100_000, + weight_restrict_decay: 2, + xcmp_max_individual_weight: 20 * WEIGHT_PER_MILLIS, + } + } } #[derive(PartialEq, Eq, Copy, Clone, Encode, Decode, TypeInfo)] @@ -339,7 +403,7 @@ impl Pallet { fn handle_xcm_message( sender: ParaId, - _sent_at: RelayBlockNumber, + _sent_at: RelayBlockNumber, // Review Q: why is this arg still here? xcm: VersionedXcm, max_weight: Weight, ) -> Result { @@ -366,6 +430,7 @@ impl Pallet { sender: ParaId, (sent_at, format): (RelayBlockNumber, XcmpMessageFormat), max_weight: Weight, + max_individual_weight: Weight, ) -> (Weight, bool) { let data = >::get(sender, sent_at); let mut last_remaining_fragments; @@ -390,6 +455,20 @@ impl Pallet { remaining_fragments = last_remaining_fragments; break }, + Err(XcmError::WeightLimitReached(required)) + if required > max_individual_weight => + { + // overweight - add to overweight queue and continue with message + // execution consuming the message. + let msg_len = last_remaining_fragments + .len() + .saturating_sub(remaining_fragments.len()); + let overweight_xcm = last_remaining_fragments[..msg_len].to_vec(); + let index = Self::stash_overweight(sender, sent_at, overweight_xcm); + Self::deposit_event(Event::OverweightEnqueued( + sender, sent_at, index, required, + )); + }, Err(_) => { // Message looks invalid; don't attempt to retry }, @@ -440,6 +519,22 @@ impl Pallet { (weight_used, is_empty) } + /// Puts a given XCM into the list of overweight messages, allowing it to be executed later. + fn stash_overweight( + sender: ParaId, + sent_at: RelayBlockNumber, + xcm: Vec, + ) -> OverweightIndex { + let index = ::OverweightCount::mutate(|count| { + let index = *count; + *count += 1; + index + }); + + ::Overweight::insert(index, (sender, sent_at, xcm)); + index + } + /// Service the incoming XCMP message queue attempting to execute up to `max_weight` execution /// weight of messages. /// @@ -473,8 +568,13 @@ impl Pallet { return 0 } - let QueueConfigData { resume_threshold, threshold_weight, weight_restrict_decay, .. } = - >::get(); + let QueueConfigData { + resume_threshold, + threshold_weight, + weight_restrict_decay, + xcmp_max_individual_weight, + .. + } = >::get(); let mut shuffled = Self::create_shuffle(status.len()); let mut weight_used = 0; @@ -516,8 +616,12 @@ impl Pallet { } else { // Process up to one block's worth for now. let weight_remaining = weight_available.saturating_sub(weight_used); - let (weight_processed, is_empty) = - Self::process_xcmp_message(sender, status[index].2[0], weight_remaining); + let (weight_processed, is_empty) = Self::process_xcmp_message( + sender, + status[index].2[0], + weight_remaining, + xcmp_max_individual_weight, + ); if is_empty { status[index].2.remove(0); } diff --git a/pallets/xcmp-queue/src/mock.rs b/pallets/xcmp-queue/src/mock.rs index 3496db5aa32..0519f20f593 100644 --- a/pallets/xcmp-queue/src/mock.rs +++ b/pallets/xcmp-queue/src/mock.rs @@ -16,6 +16,7 @@ use super::*; use crate as xcmp_queue; use frame_support::parameter_types; +use frame_system::EnsureRoot; use sp_core::H256; use sp_runtime::{ testing::Header, @@ -157,6 +158,7 @@ impl Config for Test { type XcmExecutor = xcm_executor::XcmExecutor; type ChannelInfo = ParachainSystem; type VersionWrapper = (); + type ExecuteOverweightOrigin = EnsureRoot; } pub fn new_test_ext() -> sp_io::TestExternalities { diff --git a/pallets/xcmp-queue/src/tests.rs b/pallets/xcmp-queue/src/tests.rs index 117933d8f57..d937e476ecd 100644 --- a/pallets/xcmp-queue/src/tests.rs +++ b/pallets/xcmp-queue/src/tests.rs @@ -43,7 +43,7 @@ fn bad_message_is_handled() { InboundXcmpMessages::::insert(ParaId::from(1000), 1, bad_data); let format = XcmpMessageFormat::ConcatenatedEncodedBlob; // This should exit with an error. - XcmpQueue::process_xcmp_message(1000.into(), (1, format), 10_000_000_000); + XcmpQueue::process_xcmp_message(1000.into(), (1, format), 10_000_000_000, 10_000_000_000); }); } @@ -60,6 +60,6 @@ fn other_bad_message_is_handled() { InboundXcmpMessages::::insert(ParaId::from(1000), 1, bad_data); let format = XcmpMessageFormat::ConcatenatedEncodedBlob; // This should exit with an error. - XcmpQueue::process_xcmp_message(1000.into(), (1, format), 10_000_000_000); + XcmpQueue::process_xcmp_message(1000.into(), (1, format), 10_000_000_000, 10_000_000_000); }); } diff --git a/parachain-template/runtime/src/lib.rs b/parachain-template/runtime/src/lib.rs index d7cb4d70dd6..65bf2f745d4 100644 --- a/parachain-template/runtime/src/lib.rs +++ b/parachain-template/runtime/src/lib.rs @@ -521,6 +521,7 @@ impl cumulus_pallet_xcmp_queue::Config for Runtime { type XcmExecutor = XcmExecutor; type ChannelInfo = ParachainSystem; type VersionWrapper = (); + type ExecuteOverweightOrigin = EnsureRoot; } impl cumulus_pallet_dmp_queue::Config for Runtime { diff --git a/polkadot-parachains/rococo-parachain/src/lib.rs b/polkadot-parachains/rococo-parachain/src/lib.rs index cf6dd18bb13..e90ae0cba13 100644 --- a/polkadot-parachains/rococo-parachain/src/lib.rs +++ b/polkadot-parachains/rococo-parachain/src/lib.rs @@ -447,6 +447,7 @@ impl cumulus_pallet_xcmp_queue::Config for Runtime { type XcmExecutor = XcmExecutor; type ChannelInfo = ParachainSystem; type VersionWrapper = (); + type ExecuteOverweightOrigin = EnsureRoot; } impl cumulus_pallet_dmp_queue::Config for Runtime { diff --git a/polkadot-parachains/statemine/src/lib.rs b/polkadot-parachains/statemine/src/lib.rs index 83934708aab..7cde9006640 100644 --- a/polkadot-parachains/statemine/src/lib.rs +++ b/polkadot-parachains/statemine/src/lib.rs @@ -600,6 +600,7 @@ impl cumulus_pallet_xcmp_queue::Config for Runtime { type XcmExecutor = XcmExecutor; type ChannelInfo = ParachainSystem; type VersionWrapper = PolkadotXcm; + type ExecuteOverweightOrigin = EnsureRoot; } impl cumulus_pallet_dmp_queue::Config for Runtime { diff --git a/polkadot-parachains/statemint/src/lib.rs b/polkadot-parachains/statemint/src/lib.rs index f625880d3bb..a5ba76714d1 100644 --- a/polkadot-parachains/statemint/src/lib.rs +++ b/polkadot-parachains/statemint/src/lib.rs @@ -612,6 +612,7 @@ impl cumulus_pallet_xcmp_queue::Config for Runtime { type XcmExecutor = XcmExecutor; type ChannelInfo = ParachainSystem; type VersionWrapper = PolkadotXcm; + type ExecuteOverweightOrigin = EnsureRoot; } impl cumulus_pallet_dmp_queue::Config for Runtime { diff --git a/polkadot-parachains/westmint/src/lib.rs b/polkadot-parachains/westmint/src/lib.rs index 22a37c5435e..f31898135e4 100644 --- a/polkadot-parachains/westmint/src/lib.rs +++ b/polkadot-parachains/westmint/src/lib.rs @@ -589,6 +589,7 @@ impl cumulus_pallet_xcmp_queue::Config for Runtime { type XcmExecutor = XcmExecutor; type ChannelInfo = ParachainSystem; type VersionWrapper = PolkadotXcm; + type ExecuteOverweightOrigin = EnsureRoot; } impl cumulus_pallet_dmp_queue::Config for Runtime { From b4fd7d7a97f10c471b3eac19cf5bedb74d85fa73 Mon Sep 17 00:00:00 2001 From: Keith Yeung Date: Thu, 25 Nov 2021 23:12:54 -0800 Subject: [PATCH 2/7] Add storage migration logic to XCMP queue pallet --- pallets/xcmp-queue/src/lib.rs | 9 ++ pallets/xcmp-queue/src/migration.rs | 128 ++++++++++++++++++++++++++++ 2 files changed, 137 insertions(+) create mode 100644 pallets/xcmp-queue/src/migration.rs diff --git a/pallets/xcmp-queue/src/lib.rs b/pallets/xcmp-queue/src/lib.rs index 06f07472f13..17e6b7f38c0 100644 --- a/pallets/xcmp-queue/src/lib.rs +++ b/pallets/xcmp-queue/src/lib.rs @@ -25,6 +25,8 @@ #![cfg_attr(not(feature = "std"), no_std)] +pub mod migration; + #[cfg(test)] mod mock; @@ -51,6 +53,8 @@ pub use pallet::*; /// Index used to identify overweight XCMs. pub type OverweightIndex = u64; +const LOG_TARGET: &str = "xcmp_queue"; + #[frame_support::pallet] pub mod pallet { use super::*; @@ -59,6 +63,7 @@ pub mod pallet { #[pallet::pallet] #[pallet::generate_store(pub(super) trait Store)] + #[pallet::storage_version(migration::STORAGE_VERSION)] pub struct Pallet(_); #[pallet::config] @@ -80,6 +85,10 @@ pub mod pallet { #[pallet::hooks] impl Hooks> for Pallet { + fn on_runtime_upgrade() -> Weight { + migration::migrate_to_latest::() + } + fn on_idle(_now: T::BlockNumber, max_weight: Weight) -> Weight { // on_idle processes additional messages with any remaining block weight. Self::service_xcmp_queue(max_weight) diff --git a/pallets/xcmp-queue/src/migration.rs b/pallets/xcmp-queue/src/migration.rs new file mode 100644 index 00000000000..5e1215bd7ae --- /dev/null +++ b/pallets/xcmp-queue/src/migration.rs @@ -0,0 +1,128 @@ +// Copyright 2021 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 . + +//! A module that is responsible for migration of storage. + +use crate::{Config, Pallet, Store}; +use frame_support::{pallet_prelude::*, traits::StorageVersion, weights::Weight}; + +/// The current storage version. +pub const STORAGE_VERSION: StorageVersion = StorageVersion::new(1); + +/// Migrates the pallet storage to the most recent version, checking and setting the +/// `StorageVersion`. +pub fn migrate_to_latest() -> Weight { + let mut weight = 0; + + if StorageVersion::get::>() == 0 { + weight += migrate_to_v1::(); + StorageVersion::new(1).put::>(); + } + + weight +} + +mod v0 { + use codec::{Decode, Encode}; + use super::*; + + #[derive(Encode, Decode, Debug)] + pub struct QueueConfigData { + pub suspend_threshold: u32, + pub drop_threshold: u32, + pub resume_threshold: u32, + pub threshold_weight: Weight, + pub weight_restrict_decay: Weight, + } + + impl Default for QueueConfigData { + fn default() -> Self { + QueueConfigData { + suspend_threshold: 2, + drop_threshold: 5, + resume_threshold: 1, + threshold_weight: 100_000, + weight_restrict_decay: 2, + } + } + } +} + +/// Migrates `QueueConfigData` from v0 (without the `xcmp_max_individual_weight` field) to v1 (with +/// max individual weight). +/// Uses the `Default` implementation of `QueueConfigData` to choose a value for +/// `xcmp_max_individual_weight`. +/// +/// NOTE: Only use this function if you know what you're doing. Default to using +/// `migrate_to_latest`. +pub fn migrate_to_v1() -> 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, + } + }; + + if let Err(_) = as Store>::QueueConfig::translate(|pre| pre.map(translate)) { + log::error!( + target: super::LOG_TARGET, + "unexpected error when performing translation of the QueueConfig type during storage upgrade to v1" + ); + } + + T::DbWeight::get().reads_writes(1, 1) +} + +#[cfg(test)] +mod tests { + use super::*; + use crate::mock::{new_test_ext, Test}; + + #[test] + fn test_migration_to_v1() { + let v0 = v0::QueueConfigData { + suspend_threshold: 5, + drop_threshold: 12, + resume_threshold: 3, + threshold_weight: 333_333, + weight_restrict_decay: 1, + }; + + new_test_ext().execute_with(|| { + // Put the v0 version in the state + frame_support::storage::unhashed::put_raw( + &crate::QueueConfig::::hashed_key(), + &v0.encode(), + ); + + migrate_to_v1::(); + + let v1 = crate::QueueConfig::::get(); + + assert_eq!(v0.suspend_threshold, v1.suspend_threshold); + assert_eq!(v0.drop_threshold, v1.drop_threshold); + assert_eq!(v0.resume_threshold, v1.resume_threshold); + assert_eq!(v0.threshold_weight, v1.threshold_weight); + assert_eq!(v0.weight_restrict_decay, v1.weight_restrict_decay); + assert_eq!(v1.xcmp_max_individual_weight, 20_000_000_000); + }); + } +} From b38016ccb52e4525af05d50c9747ab3c4f417876 Mon Sep 17 00:00:00 2001 From: Keith Yeung Date: Thu, 25 Nov 2021 23:15:02 -0800 Subject: [PATCH 3/7] Check whether required weight is larger than max individual weight first --- pallets/xcmp-queue/src/lib.rs | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/pallets/xcmp-queue/src/lib.rs b/pallets/xcmp-queue/src/lib.rs index 17e6b7f38c0..df1beea1f61 100644 --- a/pallets/xcmp-queue/src/lib.rs +++ b/pallets/xcmp-queue/src/lib.rs @@ -456,14 +456,6 @@ impl Pallet { let weight = max_weight - weight_used; match Self::handle_xcm_message(sender, sent_at, xcm, weight) { Ok(used) => weight_used = weight_used.saturating_add(used), - Err(XcmError::WeightLimitReached(required)) - if required <= max_weight => - { - // That message didn't get processed this time because of being - // too heavy. We leave it around for next time and bail. - remaining_fragments = last_remaining_fragments; - break - }, Err(XcmError::WeightLimitReached(required)) if required > max_individual_weight => { @@ -478,6 +470,14 @@ impl Pallet { sender, sent_at, index, required, )); }, + Err(XcmError::WeightLimitReached(required)) + if required <= max_weight => + { + // That message didn't get processed this time because of being + // too heavy. We leave it around for next time and bail. + remaining_fragments = last_remaining_fragments; + break + }, Err(_) => { // Message looks invalid; don't attempt to retry }, From c723fc307412eff052af3a2feeeb4a99e3c1aea7 Mon Sep 17 00:00:00 2001 From: Keith Yeung Date: Thu, 25 Nov 2021 23:19:23 -0800 Subject: [PATCH 4/7] cargo fmt --- pallets/xcmp-queue/src/migration.rs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/pallets/xcmp-queue/src/migration.rs b/pallets/xcmp-queue/src/migration.rs index 5e1215bd7ae..db5833e2df7 100644 --- a/pallets/xcmp-queue/src/migration.rs +++ b/pallets/xcmp-queue/src/migration.rs @@ -36,8 +36,8 @@ pub fn migrate_to_latest() -> Weight { } mod v0 { - use codec::{Decode, Encode}; use super::*; + use codec::{Decode, Encode}; #[derive(Encode, Decode, Debug)] pub struct QueueConfigData { @@ -76,8 +76,8 @@ pub fn migrate_to_v1() -> Weight { 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, + xcmp_max_individual_weight: super::QueueConfigData::default() + .xcmp_max_individual_weight, } }; From 1538a5bbdd1d0ccf4bc96a7e2d18ee2cc335185c Mon Sep 17 00:00:00 2001 From: Keith Yeung Date: Sun, 28 Nov 2021 17:24:01 -0800 Subject: [PATCH 5/7] Add some unit tests --- pallets/xcmp-queue/src/tests.rs | 23 ++++++++++++++++++++++- 1 file changed, 22 insertions(+), 1 deletion(-) diff --git a/pallets/xcmp-queue/src/tests.rs b/pallets/xcmp-queue/src/tests.rs index d937e476ecd..aa9e489c0af 100644 --- a/pallets/xcmp-queue/src/tests.rs +++ b/pallets/xcmp-queue/src/tests.rs @@ -15,9 +15,10 @@ use super::*; use cumulus_primitives_core::XcmpMessageHandler; +use frame_support::assert_noop; #[cfg(debug_assertions)] use mock::Test; -use mock::{new_test_ext, XcmpQueue}; +use mock::{new_test_ext, Origin, XcmpQueue}; #[test] fn one_message_does_not_panic() { @@ -63,3 +64,23 @@ fn other_bad_message_is_handled() { XcmpQueue::process_xcmp_message(1000.into(), (1, format), 10_000_000_000, 10_000_000_000); }); } + +#[test] +fn service_overweight_unknown() { + new_test_ext().execute_with(|| { + assert_noop!( + XcmpQueue::service_overweight(Origin::root(), 0, 1000), + Error::::BadOverweightIndex, + ); + }); +} + +#[test] +fn service_overweight_bad_xcm_format() { + new_test_ext().execute_with(|| { + let bad_xcm = vec![255]; + Overweight::::insert(0, (ParaId::from(1000), 0, bad_xcm)); + + assert_noop!(XcmpQueue::service_overweight(Origin::root(), 0, 1000), Error::::BadXcm); + }); +} From 40c33b750f15b6d48de2d3668d2676572ff3313c Mon Sep 17 00:00:00 2001 From: Keith Yeung Date: Wed, 1 Dec 2021 16:57:50 -0800 Subject: [PATCH 6/7] Remove review question comment --- pallets/xcmp-queue/src/lib.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pallets/xcmp-queue/src/lib.rs b/pallets/xcmp-queue/src/lib.rs index df765a3a305..39fe3d8c809 100644 --- a/pallets/xcmp-queue/src/lib.rs +++ b/pallets/xcmp-queue/src/lib.rs @@ -460,7 +460,7 @@ impl Pallet { fn handle_xcm_message( sender: ParaId, - _sent_at: RelayBlockNumber, // Review Q: why is this arg still here? + _sent_at: RelayBlockNumber, xcm: VersionedXcm, max_weight: Weight, ) -> Result { From 6d72b3b4fe3c3ebb9d2455f43475b7ddc4506b4e Mon Sep 17 00:00:00 2001 From: Gavin Wood Date: Wed, 8 Dec 2021 22:37:15 +0100 Subject: [PATCH 7/7] Update pallets/xcmp-queue/src/lib.rs --- pallets/xcmp-queue/src/lib.rs | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/pallets/xcmp-queue/src/lib.rs b/pallets/xcmp-queue/src/lib.rs index 39fe3d8c809..cab3a18e424 100644 --- a/pallets/xcmp-queue/src/lib.rs +++ b/pallets/xcmp-queue/src/lib.rs @@ -514,9 +514,8 @@ impl Pallet { .saturating_sub(remaining_fragments.len()); let overweight_xcm = last_remaining_fragments[..msg_len].to_vec(); let index = Self::stash_overweight(sender, sent_at, overweight_xcm); - Self::deposit_event(Event::OverweightEnqueued( - sender, sent_at, index, required, - )); + let e = Event::OverweightEnqueued(sender, sent_at, index, required); + Self::deposit_event(e); }, Err(XcmError::WeightLimitReached(required)) if required <= max_weight =>