From 2f5089b07cc4bb4e2d07403983e9e79acbfd3068 Mon Sep 17 00:00:00 2001 From: Keith Yeung Date: Fri, 4 Mar 2022 21:46:54 -0800 Subject: [PATCH 1/3] Implement a blocking barrier for XCMv3 --- xcm/pallet-xcm/src/lib.rs | 31 +++++- xcm/xcm-builder/src/barriers.rs | 96 ++++++++++++------- xcm/xcm-builder/src/lib.rs | 2 +- xcm/xcm-builder/src/tests/barriers.rs | 51 +++++++--- xcm/xcm-builder/src/tests/mock.rs | 32 +++++-- xcm/xcm-builder/tests/mock/mod.rs | 4 +- xcm/xcm-executor/src/traits/mod.rs | 2 +- xcm/xcm-executor/src/traits/should_execute.rs | 73 ++++++++++++-- 8 files changed, 228 insertions(+), 63 deletions(-) diff --git a/xcm/pallet-xcm/src/lib.rs b/xcm/pallet-xcm/src/lib.rs index 323180d2f90a..c529edbc6607 100644 --- a/xcm/pallet-xcm/src/lib.rs +++ b/xcm/pallet-xcm/src/lib.rs @@ -52,7 +52,8 @@ use frame_system::pallet_prelude::*; pub use pallet::*; use xcm_executor::{ traits::{ - ClaimAssets, DropAssets, MatchesFungible, OnResponse, VersionChangeNotifier, WeightBounds, + CheckSuspension, ClaimAssets, DropAssets, MatchesFungible, OnResponse, + VersionChangeNotifier, WeightBounds, }, Assets, }; @@ -610,6 +611,11 @@ pub mod pallet { OptionQuery, >; + /// Global suspension state of the XCM executor. + #[pallet::storage] + #[pallet::getter(fn suspended)] + pub(super) type XcmExecutionSuspended = StorageValue<_, bool, ValueQuery>; + #[pallet::genesis_config] pub struct GenesisConfig { /// The default version to encode outgoing XCM messages with. @@ -1090,6 +1096,18 @@ pub mod pallet { Some(weight_limit), ) } + + /// Set or unset the global suspension state of the XCM executor. + /// + /// - `origin`: Must be Root. + /// - `suspended`: `true` to suspend, `false` to resume. + #[pallet::call_index(10)] + #[pallet::weight(100_000_000u64)] + pub fn force_suspension(origin: OriginFor, suspended: bool) -> DispatchResult { + ensure_root(origin)?; + XcmExecutionSuspended::::set(suspended); + Ok(()) + } } } @@ -2036,6 +2054,17 @@ impl OnResponse for Pallet { } } +impl CheckSuspension for Pallet { + fn is_suspended( + _origin: &MultiLocation, + _instructions: &mut [Instruction], + _max_weight: Weight, + _weight_credit: &mut Weight, + ) -> bool { + Self::suspended() + } +} + /// Ensure that the origin `o` represents an XCM (`Transact`) origin. /// /// Returns `Ok` with the location of the XCM sender or an `Err` otherwise. diff --git a/xcm/xcm-builder/src/barriers.rs b/xcm/xcm-builder/src/barriers.rs index bdc7f6811edd..851ac4adb8ff 100644 --- a/xcm/xcm-builder/src/barriers.rs +++ b/xcm/xcm-builder/src/barriers.rs @@ -29,7 +29,7 @@ use xcm::latest::{ MultiLocation, Weight, WeightLimit::*, }; -use xcm_executor::traits::{OnResponse, ShouldExecute}; +use xcm_executor::traits::{CheckSuspension, OnResponse, RejectReason, ShouldExecute}; /// Execution barrier that just takes `max_weight` from `weight_credit`. /// @@ -43,13 +43,14 @@ impl ShouldExecute for TakeWeightCredit { _instructions: &mut [Instruction], max_weight: Weight, weight_credit: &mut Weight, - ) -> Result<(), ()> { + ) -> Result<(), RejectReason> { log::trace!( target: "xcm::barriers", "TakeWeightCredit origin: {:?}, instructions: {:?}, max_weight: {:?}, weight_credit: {:?}", _origin, _instructions, max_weight, weight_credit, ); - *weight_credit = weight_credit.checked_sub(&max_weight).ok_or(())?; + *weight_credit = + weight_credit.checked_sub(&max_weight).ok_or(RejectReason::InsufficientCredit)?; Ok(()) } } @@ -66,42 +67,43 @@ impl> ShouldExecute for AllowTopLevelPaidExecutionFro instructions: &mut [Instruction], max_weight: Weight, _weight_credit: &mut Weight, - ) -> Result<(), ()> { + ) -> Result<(), RejectReason> { log::trace!( target: "xcm::barriers", "AllowTopLevelPaidExecutionFrom origin: {:?}, instructions: {:?}, max_weight: {:?}, weight_credit: {:?}", origin, instructions, max_weight, _weight_credit, ); - ensure!(T::contains(origin), ()); + ensure!(T::contains(origin), RejectReason::UntrustedOrigin); // We will read up to 5 instructions. This allows up to 3 `ClearOrigin` instructions. We // allow for more than one since anything beyond the first is a no-op and it's conceivable // that composition of operations might result in more than one being appended. let mut iter = instructions.iter_mut().take(5); - let i = iter.next().ok_or(())?; + let i = iter.next().ok_or(RejectReason::UnexpectedMessageFormat)?; match i { ReceiveTeleportedAsset(..) | WithdrawAsset(..) | ReserveAssetDeposited(..) | ClaimAsset { .. } => (), - _ => return Err(()), + _ => return Err(RejectReason::UnexpectedMessageFormat), } - let mut i = iter.next().ok_or(())?; + let mut i = iter.next().ok_or(RejectReason::UnexpectedMessageFormat)?; while let ClearOrigin = i { - i = iter.next().ok_or(())?; + i = iter.next().ok_or(RejectReason::UnexpectedMessageFormat)?; } match i { - BuyExecution { weight_limit: Limited(ref mut weight), .. } - if weight.all_gte(max_weight) => - { - *weight = weight.max(max_weight); - Ok(()) - }, + BuyExecution { weight_limit: Limited(ref mut weight), .. } => + if weight.all_gte(max_weight) { + *weight = weight.max(max_weight); + Ok(()) + } else { + Err(RejectReason::WeightLimitTooLow) + }, BuyExecution { ref mut weight_limit, .. } if weight_limit == &Unlimited => { *weight_limit = Limited(max_weight); Ok(()) }, - _ => Err(()), + _ => Err(RejectReason::UnexpectedMessageFormat), } } } @@ -165,7 +167,7 @@ impl< instructions: &mut [Instruction], max_weight: Weight, weight_credit: &mut Weight, - ) -> Result<(), ()> { + ) -> Result<(), RejectReason> { log::trace!( target: "xcm::barriers", "WithComputedOrigin origin: {:?}, instructions: {:?}, max_weight: {:?}, weight_credit: {:?}", @@ -175,7 +177,7 @@ impl< let mut skipped = 0; // NOTE: We do not check the validity of `UniversalOrigin` here, meaning that a malicious // origin could place a `UniversalOrigin` in order to spoof some location which gets free - // execution. This technical could get it past the barrier condition, but the execution + // execution. This technically could get it past the barrier condition, but the execution // would instantly fail since the first instruction would cause an error with the // invalid UniversalOrigin. while skipped < MaxPrefixes::get() as usize { @@ -186,7 +188,9 @@ impl< actual_origin = X1(*new_global).relative_to(&LocalUniversal::get()); }, Some(DescendOrigin(j)) => { - actual_origin.append_with(*j).map_err(|_| ())?; + actual_origin + .append_with(*j) + .map_err(|_| RejectReason::OriginMultiLocationTooLong)?; }, _ => break, } @@ -201,6 +205,28 @@ impl< } } +/// Barrier condition that allows for a `SuspensionChecker` that controls whether or not the XCM +/// executor will be suspended from executing the given XCM. +pub struct RespectSuspension(PhantomData<(Inner, SuspensionChecker)>); +impl ShouldExecute for RespectSuspension +where + Inner: ShouldExecute, + SuspensionChecker: CheckSuspension, +{ + fn should_execute( + origin: &MultiLocation, + instructions: &mut [Instruction], + max_weight: Weight, + weight_credit: &mut Weight, + ) -> Result<(), RejectReason> { + if SuspensionChecker::is_suspended(origin, instructions, max_weight, weight_credit) { + Err(RejectReason::Suspended) + } else { + Inner::should_execute(origin, instructions, max_weight, weight_credit) + } + } +} + /// Allows execution from any origin that is contained in `T` (i.e. `T::Contains(origin)`). /// /// Use only for executions from completely trusted origins, from which no unpermissioned messages @@ -212,13 +238,13 @@ impl> ShouldExecute for AllowUnpaidExecutionFrom { instructions: &mut [Instruction], _max_weight: Weight, _weight_credit: &mut Weight, - ) -> Result<(), ()> { + ) -> Result<(), RejectReason> { log::trace!( target: "xcm::barriers", "AllowUnpaidExecutionFrom origin: {:?}, instructions: {:?}, max_weight: {:?}, weight_credit: {:?}", origin, instructions, _max_weight, _weight_credit, ); - ensure!(T::contains(origin), ()); + ensure!(T::contains(origin), RejectReason::UntrustedOrigin); Ok(()) } } @@ -234,18 +260,18 @@ impl> ShouldExecute for AllowExplicitUnpaidExecutionF instructions: &mut [Instruction], max_weight: Weight, _weight_credit: &mut Weight, - ) -> Result<(), ()> { + ) -> Result<(), RejectReason> { log::trace!( target: "xcm::barriers", "AllowExplicitUnpaidExecutionFrom origin: {:?}, instructions: {:?}, max_weight: {:?}, weight_credit: {:?}", origin, instructions, max_weight, _weight_credit, ); - ensure!(T::contains(origin), ()); + ensure!(T::contains(origin), RejectReason::UntrustedOrigin); match instructions.first() { Some(UnpaidExecution { weight_limit: Limited(m), .. }) if m.all_gte(max_weight) => Ok(()), Some(UnpaidExecution { weight_limit: Unlimited, .. }) => Ok(()), - _ => Err(()), + _ => Err(RejectReason::UnexpectedMessageFormat), } } } @@ -270,18 +296,22 @@ impl ShouldExecute for AllowKnownQueryResponses], _max_weight: Weight, _weight_credit: &mut Weight, - ) -> Result<(), ()> { + ) -> Result<(), RejectReason> { log::trace!( target: "xcm::barriers", "AllowKnownQueryResponses origin: {:?}, instructions: {:?}, max_weight: {:?}, weight_credit: {:?}", origin, instructions, _max_weight, _weight_credit, ); - ensure!(instructions.len() == 1, ()); + ensure!(instructions.len() == 1, RejectReason::UnexpectedMessageFormat); match instructions.first() { - Some(QueryResponse { query_id, querier, .. }) - if ResponseHandler::expecting_response(origin, *query_id, querier.as_ref()) => - Ok(()), - _ => Err(()), + Some(QueryResponse { query_id, querier, .. }) => { + if ResponseHandler::expecting_response(origin, *query_id, querier.as_ref()) { + Ok(()) + } else { + Err(RejectReason::UnexpectedResponse) + } + }, + _ => Err(RejectReason::UnexpectedMessageFormat), } } } @@ -295,16 +325,16 @@ impl> ShouldExecute for AllowSubscriptionsFrom { instructions: &mut [Instruction], _max_weight: Weight, _weight_credit: &mut Weight, - ) -> Result<(), ()> { + ) -> Result<(), RejectReason> { log::trace!( target: "xcm::barriers", "AllowSubscriptionsFrom origin: {:?}, instructions: {:?}, max_weight: {:?}, weight_credit: {:?}", origin, instructions, _max_weight, _weight_credit, ); - ensure!(T::contains(origin), ()); + ensure!(T::contains(origin), RejectReason::UntrustedOrigin); match instructions { &mut [SubscribeVersion { .. } | UnsubscribeVersion] => Ok(()), - _ => Err(()), + _ => Err(RejectReason::UnexpectedMessageFormat), } } } diff --git a/xcm/xcm-builder/src/lib.rs b/xcm/xcm-builder/src/lib.rs index eaf6d636795e..d758cc06f388 100644 --- a/xcm/xcm-builder/src/lib.rs +++ b/xcm/xcm-builder/src/lib.rs @@ -49,7 +49,7 @@ mod barriers; pub use barriers::{ AllowExplicitUnpaidExecutionFrom, AllowKnownQueryResponses, AllowSubscriptionsFrom, AllowTopLevelPaidExecutionFrom, AllowUnpaidExecutionFrom, IsChildSystemParachain, - TakeWeightCredit, WithComputedOrigin, + RespectSuspension, TakeWeightCredit, WithComputedOrigin, }; mod currency_adapter; diff --git a/xcm/xcm-builder/src/tests/barriers.rs b/xcm/xcm-builder/src/tests/barriers.rs index a7889c7e212d..0e0ef1d4d92d 100644 --- a/xcm/xcm-builder/src/tests/barriers.rs +++ b/xcm/xcm-builder/src/tests/barriers.rs @@ -36,7 +36,7 @@ fn take_weight_credit_barrier_should_work() { Weight::from_parts(10, 10), &mut weight_credit, ); - assert_eq!(r, Err(())); + assert_eq!(r, Err(RejectReason::InsufficientCredit)); assert_eq!(weight_credit, Weight::zero()); } @@ -69,7 +69,7 @@ fn computed_origin_should_work() { Weight::from_parts(100, 100), &mut Weight::zero(), ); - assert_eq!(r, Err(())); + assert_eq!(r, Err(RejectReason::UntrustedOrigin)); let r = WithComputedOrigin::< AllowTopLevelPaidExecutionFrom>, @@ -81,7 +81,7 @@ fn computed_origin_should_work() { Weight::from_parts(100, 100), &mut Weight::zero(), ); - assert_eq!(r, Err(())); + assert_eq!(r, Err(RejectReason::UntrustedOrigin)); let r = WithComputedOrigin::< AllowTopLevelPaidExecutionFrom>, @@ -109,7 +109,7 @@ fn allow_unpaid_should_work() { Weight::from_parts(10, 10), &mut Weight::zero(), ); - assert_eq!(r, Err(())); + assert_eq!(r, Err(RejectReason::UntrustedOrigin)); let r = AllowUnpaidExecutionFrom::>::should_execute( &Parent.into(), @@ -149,7 +149,7 @@ fn allow_explicit_unpaid_should_work() { Weight::from_parts(20, 20), &mut Weight::zero(), ); - assert_eq!(r, Err(())); + assert_eq!(r, Err(RejectReason::UntrustedOrigin)); let r = AllowExplicitUnpaidExecutionFrom::>::should_execute( &Parent.into(), @@ -157,7 +157,7 @@ fn allow_explicit_unpaid_should_work() { Weight::from_parts(20, 20), &mut Weight::zero(), ); - assert_eq!(r, Err(())); + assert_eq!(r, Err(RejectReason::UnexpectedMessageFormat)); let r = AllowExplicitUnpaidExecutionFrom::>::should_execute( &Parent.into(), @@ -165,7 +165,7 @@ fn allow_explicit_unpaid_should_work() { Weight::from_parts(20, 20), &mut Weight::zero(), ); - assert_eq!(r, Err(())); + assert_eq!(r, Err(RejectReason::UnexpectedMessageFormat)); let r = AllowExplicitUnpaidExecutionFrom::>::should_execute( &Parent.into(), @@ -189,7 +189,7 @@ fn allow_explicit_unpaid_should_work() { Weight::from_parts(20, 20), &mut Weight::zero(), ); - assert_eq!(r, Err(())); + assert_eq!(r, Err(RejectReason::UnexpectedMessageFormat)); let r = AllowExplicitUnpaidExecutionFrom::>::should_execute( &Parent.into(), @@ -213,7 +213,7 @@ fn allow_paid_should_work() { Weight::from_parts(10, 10), &mut Weight::zero(), ); - assert_eq!(r, Err(())); + assert_eq!(r, Err(RejectReason::UntrustedOrigin)); let fees = (Parent, 1).into(); let mut underpaying_message = Xcm::<()>(vec![ @@ -228,7 +228,7 @@ fn allow_paid_should_work() { Weight::from_parts(30, 30), &mut Weight::zero(), ); - assert_eq!(r, Err(())); + assert_eq!(r, Err(RejectReason::WeightLimitTooLow)); let fees = (Parent, 1).into(); let mut paying_message = Xcm::<()>(vec![ @@ -243,7 +243,7 @@ fn allow_paid_should_work() { Weight::from_parts(30, 30), &mut Weight::zero(), ); - assert_eq!(r, Err(())); + assert_eq!(r, Err(RejectReason::UntrustedOrigin)); let r = AllowTopLevelPaidExecutionFrom::>::should_execute( &Parent.into(), @@ -266,7 +266,7 @@ fn allow_paid_should_work() { Weight::from_parts(20, 20), &mut Weight::zero(), ); - assert_eq!(r, Err(())); + assert_eq!(r, Err(RejectReason::WeightLimitTooLow)); let r = AllowTopLevelPaidExecutionFrom::>::should_execute( &Parent.into(), @@ -276,3 +276,30 @@ fn allow_paid_should_work() { ); assert_eq!(r, Ok(())) } + +#[test] +fn suspension_should_work() { + TestSuspender::set_suspended(true); + AllowUnpaidFrom::set(vec![Parent.into()]); + + let mut message = + Xcm::<()>(vec![TransferAsset { assets: (Parent, 100).into(), beneficiary: Here.into() }]); + let r = RespectSuspension::>, TestSuspender>::should_execute( + &Parent.into(), + message.inner_mut(), + Weight::from_parts(10, 10), + &mut Weight::zero(), + ); + assert_eq!(r, Err(RejectReason::Suspended)); + + TestSuspender::set_suspended(false); + let mut message = + Xcm::<()>(vec![TransferAsset { assets: (Parent, 100).into(), beneficiary: Here.into() }]); + let r = RespectSuspension::>, TestSuspender>::should_execute( + &Parent.into(), + message.inner_mut(), + Weight::from_parts(10, 10), + &mut Weight::zero(), + ); + assert_eq!(r, Ok(())); +} diff --git a/xcm/xcm-builder/src/tests/mock.rs b/xcm/xcm-builder/src/tests/mock.rs index fa85d09a443e..2bc9e948d99c 100644 --- a/xcm/xcm-builder/src/tests/mock.rs +++ b/xcm/xcm-builder/src/tests/mock.rs @@ -16,8 +16,9 @@ use crate::{barriers::AllowSubscriptionsFrom, test_utils::*}; pub use crate::{ - AllowExplicitUnpaidExecutionFrom, AllowKnownQueryResponses, AllowTopLevelPaidExecutionFrom, - AllowUnpaidExecutionFrom, FixedRateOfFungible, FixedWeightBounds, TakeWeightCredit, + barriers::RespectSuspension, AllowExplicitUnpaidExecutionFrom, AllowKnownQueryResponses, + AllowTopLevelPaidExecutionFrom, AllowUnpaidExecutionFrom, FixedRateOfFungible, + FixedWeightBounds, TakeWeightCredit, }; use frame_support::traits::{ContainsPair, Everything}; pub use frame_support::{ @@ -32,7 +33,7 @@ pub use frame_support::{ pub use parity_scale_codec::{Decode, Encode}; pub use sp_io::hashing::blake2_256; pub use sp_std::{ - cell::RefCell, + cell::{Cell, RefCell}, collections::{btree_map::BTreeMap, btree_set::BTreeSet}, fmt::Debug, marker::PhantomData, @@ -40,8 +41,8 @@ pub use sp_std::{ pub use xcm::latest::{prelude::*, Weight}; pub use xcm_executor::{ traits::{ - AssetExchange, AssetLock, ConvertOrigin, Enact, ExportXcm, FeeManager, FeeReason, - LockError, OnResponse, TransactAsset, + AssetExchange, AssetLock, CheckSuspension, ConvertOrigin, Enact, ExportXcm, FeeManager, + FeeReason, LockError, OnResponse, RejectReason, TransactAsset, }, Assets, Config, }; @@ -128,6 +129,7 @@ thread_local! { ) -> Result, )>> = RefCell::new(None); pub static SEND_PRICE: RefCell = RefCell::new(MultiAssets::new()); + pub static SUSPENDED: Cell = Cell::new(false); } pub fn sent_xcm() -> Vec<(MultiLocation, opaque::Xcm, XcmHash)> { SENT_XCM.with(|q| (*q.borrow()).clone()) @@ -419,6 +421,24 @@ parameter_types! { pub static MaxInstructions: u32 = 100; } +pub struct TestSuspender; +impl CheckSuspension for TestSuspender { + fn is_suspended( + _origin: &MultiLocation, + _instructions: &mut [Instruction], + _max_weight: Weight, + _weight_credit: &mut Weight, + ) -> bool { + SUSPENDED.with(|s| s.get()) + } +} + +impl TestSuspender { + pub fn set_suspended(suspended: bool) { + SUSPENDED.with(|s| s.set(suspended)); + } +} + pub type TestBarrier = ( TakeWeightCredit, AllowKnownQueryResponses, @@ -629,7 +649,7 @@ impl Config for TestConfig { type IsReserve = TestIsReserve; type IsTeleporter = TestIsTeleporter; type UniversalLocation = ExecutorUniversalLocation; - type Barrier = TestBarrier; + type Barrier = RespectSuspension; type Weigher = FixedWeightBounds; type Trader = FixedRateOfFungible; type ResponseHandler = TestResponseHandler; diff --git a/xcm/xcm-builder/tests/mock/mod.rs b/xcm/xcm-builder/tests/mock/mod.rs index 79cecfbdcb9a..91750703c1c7 100644 --- a/xcm/xcm-builder/tests/mock/mod.rs +++ b/xcm/xcm-builder/tests/mock/mod.rs @@ -33,7 +33,7 @@ use xcm_builder::{ AccountId32Aliases, AllowTopLevelPaidExecutionFrom, AllowUnpaidExecutionFrom, ChildParachainAsNative, ChildParachainConvertsVia, ChildSystemParachainAsSuperuser, CurrencyAdapter as XcmCurrencyAdapter, FixedRateOfFungible, FixedWeightBounds, - IsChildSystemParachain, IsConcrete, MintLocation, SignedAccountId32AsNative, + IsChildSystemParachain, IsConcrete, MintLocation, RespectSuspension, SignedAccountId32AsNative, SignedToAccountId32, SovereignSignedViaLocation, TakeWeightCredit, }; @@ -182,7 +182,7 @@ impl xcm_executor::Config for XcmConfig { type IsReserve = (); type IsTeleporter = TrustedTeleporters; type UniversalLocation = UniversalLocation; - type Barrier = Barrier; + type Barrier = RespectSuspension; type Weigher = FixedWeightBounds; type Trader = FixedRateOfFungible; type ResponseHandler = XcmPallet; diff --git a/xcm/xcm-executor/src/traits/mod.rs b/xcm/xcm-executor/src/traits/mod.rs index 94fe7e98a7db..f35920b47d0d 100644 --- a/xcm/xcm-executor/src/traits/mod.rs +++ b/xcm/xcm-executor/src/traits/mod.rs @@ -40,7 +40,7 @@ pub use token_matching::{ mod on_response; pub use on_response::{OnResponse, VersionChangeNotifier}; mod should_execute; -pub use should_execute::ShouldExecute; +pub use should_execute::{CheckSuspension, RejectReason, ShouldExecute}; mod transact_asset; pub use transact_asset::TransactAsset; mod weight; diff --git a/xcm/xcm-executor/src/traits/should_execute.rs b/xcm/xcm-executor/src/traits/should_execute.rs index 7db8fbe4a09e..3728917f16cd 100644 --- a/xcm/xcm-executor/src/traits/should_execute.rs +++ b/xcm/xcm-executor/src/traits/should_execute.rs @@ -14,9 +14,30 @@ // You should have received a copy of the GNU General Public License // along with Polkadot. If not, see . +use parity_scale_codec::{Decode, Encode}; use sp_std::result::Result; use xcm::latest::{Instruction, MultiLocation, Weight}; +/// The reason why a given XCM is not permitted to execute. +#[derive(Clone, Debug, PartialEq, Eq, Decode, Encode)] +pub enum RejectReason { + /// The supplied weight credit is not enough to pay for the weight required. + InsufficientCredit, + /// The series of `DescendOrigin` instructions would overflow the origin MultiLocation. + OriginMultiLocationTooLong, + /// Does not pass the barrier yet, but might pass in a later block. + Suspended, + /// Barrier does not recognize the incoming XCM format. + UnexpectedMessageFormat, + /// The receiving chain did not expect a response XCM from the origin. + UnexpectedResponse, + /// The origin is not trusted to pass this barrier. + UntrustedOrigin, + /// The incoming XCM contains a weight limit that is lower than the required weight, as weighed + /// by the receiver. + WeightLimitTooLow, +} + /// Trait to determine whether the execution engine should actually execute a given XCM. /// /// Can be amalgamated into a tuple to have multiple trials. If any of the tuple elements returns `Ok()`, the @@ -35,7 +56,7 @@ pub trait ShouldExecute { instructions: &mut [Instruction], max_weight: Weight, weight_credit: &mut Weight, - ) -> Result<(), ()>; + ) -> Result<(), RejectReason>; } #[impl_trait_for_tuples::impl_for_tuples(30)] @@ -45,21 +66,59 @@ impl ShouldExecute for Tuple { instructions: &mut [Instruction], max_weight: Weight, weight_credit: &mut Weight, - ) -> Result<(), ()> { + ) -> Result<(), RejectReason> { + let mut reason = RejectReason::UnexpectedMessageFormat; for_tuples!( #( - match Tuple::should_execute(origin, instructions, max_weight, weight_credit) { + reason = match Tuple::should_execute(origin, instructions, max_weight, weight_credit) { Ok(()) => return Ok(()), - _ => (), - } + Err(e) => e, + }; )* ); log::trace!( target: "xcm::should_execute", - "did not pass barrier: origin: {:?}, instructions: {:?}, max_weight: {:?}, weight_credit: {:?}", + "did not pass barrier: origin: {:?}, instructions: {:?}, max_weight: {:?}, weight_credit: {:?}, reason: {:?}", origin, instructions, max_weight, weight_credit, + reason, ); - Err(()) + Err(reason) + } +} + +/// Trait to determine whether the execution engine is suspended from executing a given XCM. +/// +/// The trait method is given the same parameters as `ShouldExecute::should_execute`, so that the +/// implementer will have all the context necessary to determine whether or not to suspend the +/// XCM executor. +/// +/// Can be chained together in tuples to have multiple rounds of checks. If all of the tuple +/// elements returns false, then execution is not suspended. Otherwise, execution is suspended +/// if any of the tuple elements returns true. +pub trait CheckSuspension { + fn is_suspended( + origin: &MultiLocation, + instructions: &mut [Instruction], + max_weight: Weight, + weight_credit: &mut Weight, + ) -> bool; +} + +#[impl_trait_for_tuples::impl_for_tuples(30)] +impl CheckSuspension for Tuple { + fn is_suspended( + origin: &MultiLocation, + instruction: &mut [Instruction], + max_weight: Weight, + weight_credit: &mut Weight, + ) -> bool { + for_tuples!( #( + if Tuple::is_suspended(origin, instruction, max_weight, weight_credit) { + return true + } + )* ); + + false } } From 1943b29bc4f13f322eed5d885717a5a8304c19ec Mon Sep 17 00:00:00 2001 From: Keith Yeung Date: Thu, 21 Apr 2022 23:48:44 -0700 Subject: [PATCH 2/3] Add RejectReason::NoRulesMatched --- xcm/xcm-executor/src/traits/should_execute.rs | 15 +++++++++------ 1 file changed, 9 insertions(+), 6 deletions(-) diff --git a/xcm/xcm-executor/src/traits/should_execute.rs b/xcm/xcm-executor/src/traits/should_execute.rs index 3728917f16cd..8219ba3e9a21 100644 --- a/xcm/xcm-executor/src/traits/should_execute.rs +++ b/xcm/xcm-executor/src/traits/should_execute.rs @@ -23,6 +23,8 @@ use xcm::latest::{Instruction, MultiLocation, Weight}; pub enum RejectReason { /// The supplied weight credit is not enough to pay for the weight required. InsufficientCredit, + /// None of the barrier rules in a tuple matched successfully. + NoRulesMatched, /// The series of `DescendOrigin` instructions would overflow the origin MultiLocation. OriginMultiLocationTooLong, /// Does not pass the barrier yet, but might pass in a later block. @@ -67,23 +69,24 @@ impl ShouldExecute for Tuple { max_weight: Weight, weight_credit: &mut Weight, ) -> Result<(), RejectReason> { - let mut reason = RejectReason::UnexpectedMessageFormat; for_tuples!( #( - reason = match Tuple::should_execute(origin, instructions, max_weight, weight_credit) { + match Tuple::should_execute(origin, instructions, max_weight, weight_credit) { Ok(()) => return Ok(()), - Err(e) => e, + Err(e) => log::trace!( + target: "xcm::should_execute", + "barrier rule rejected message with reason: {:?}", e + ), }; )* ); log::trace!( target: "xcm::should_execute", - "did not pass barrier: origin: {:?}, instructions: {:?}, max_weight: {:?}, weight_credit: {:?}, reason: {:?}", + "did not pass any barrier rules: origin: {:?}, instructions: {:?}, max_weight: {:?}, weight_credit: {:?}", origin, instructions, max_weight, weight_credit, - reason, ); - Err(reason) + Err(RejectReason::NoRulesMatched) } } From 8bd9da9222c78a65ce102a03da5cc82c8204f9ce Mon Sep 17 00:00:00 2001 From: Keith Yeung Date: Thu, 9 Feb 2023 17:10:08 -0300 Subject: [PATCH 3/3] Add ForbiddenInstructions RejectReason --- xcm/xcm-executor/src/traits/should_execute.rs | 2 ++ 1 file changed, 2 insertions(+) diff --git a/xcm/xcm-executor/src/traits/should_execute.rs b/xcm/xcm-executor/src/traits/should_execute.rs index 8219ba3e9a21..c9196fd0e53a 100644 --- a/xcm/xcm-executor/src/traits/should_execute.rs +++ b/xcm/xcm-executor/src/traits/should_execute.rs @@ -21,6 +21,8 @@ use xcm::latest::{Instruction, MultiLocation, Weight}; /// The reason why a given XCM is not permitted to execute. #[derive(Clone, Debug, PartialEq, Eq, Decode, Encode)] pub enum RejectReason { + /// Some instructions in the XCM are not permitted to be sent or executed. + ForbiddenInstructions, /// The supplied weight credit is not enough to pay for the weight required. InsufficientCredit, /// None of the barrier rules in a tuple matched successfully.