From 83ba763ddbf281ce1f94953cbb7ee30b1b776de0 Mon Sep 17 00:00:00 2001 From: Keith Yeung Date: Tue, 21 Feb 2023 11:22:27 -0800 Subject: [PATCH 01/11] Introduce XCM matcher for writing barriers --- xcm/src/lib.rs | 31 ++++++++ xcm/src/v3/matcher.rs | 99 +++++++++++++++++++++++ xcm/src/v3/mod.rs | 6 +- xcm/src/v3/traits.rs | 4 +- xcm/xcm-builder/src/barriers.rs | 137 +++++++++++++++++--------------- 5 files changed, 211 insertions(+), 66 deletions(-) create mode 100644 xcm/src/v3/matcher.rs diff --git a/xcm/src/lib.rs b/xcm/src/lib.rs index 678a825b6608..05878a575ce7 100644 --- a/xcm/src/lib.rs +++ b/xcm/src/lib.rs @@ -47,6 +47,37 @@ pub const MAX_XCM_DECODE_DEPTH: u32 = 8; /// A version of XCM. pub type Version = u32; +/// Creates an instruction matcher from an XCM. Since XCM versions differ, we need to make a trait +/// here to unify the interfaces among them. +pub trait CreateMatcher { + type Matcher; + fn matcher(self) -> Self::Matcher; +} + +/// API that allows to pattern-match against anything that is contained within an XCM. +pub trait MatchXcm { + type Inst; + type Loc; + type Error; + + fn assert_remaining_insts(self, n: usize) -> Result + where + Self: Sized; + fn match_next_inst(self, f: F) -> Result + where + Self: Sized, + F: FnMut(&mut Self::Inst) -> Result<(), Self::Error>; + fn match_next_inst_while(self, cond: C, f: F) -> Result + where + Self: Sized, + C: Fn() -> bool, + F: FnMut(&mut Self::Inst) -> Result<(), Self::Error>; + fn skip_inst_while(self, cond: C) -> Result + where + Self: Sized, + C: Fn(&Self::Inst) -> bool; +} + #[derive(Clone, Eq, PartialEq, Debug)] pub enum Unsupported {} impl Encode for Unsupported {} diff --git a/xcm/src/v3/matcher.rs b/xcm/src/v3/matcher.rs new file mode 100644 index 000000000000..79d4006c4fc7 --- /dev/null +++ b/xcm/src/v3/matcher.rs @@ -0,0 +1,99 @@ +// Copyright 2023 Parity Technologies (UK) Ltd. +// This file is part of Polkadot. + +// Substrate 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. + +// Substrate 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 . + +use super::{Instruction, MultiLocation}; +use crate::{CreateMatcher, MatchXcm}; + +impl<'a, Call> CreateMatcher for &'a mut [Instruction] { + type Matcher = Matcher<'a, Call>; + + fn matcher(self) -> Self::Matcher { + let total_inst = self.len(); + + Matcher { xcm: self, current_idx: 0, total_inst } + } +} + +pub struct Matcher<'a, Call> { + pub(crate) xcm: &'a mut [Instruction], + pub(crate) current_idx: usize, + pub(crate) total_inst: usize, +} + +impl<'a, Call> MatchXcm for Matcher<'a, Call> { + type Error = (); + type Inst = Instruction; + type Loc = MultiLocation; + + fn assert_remaining_insts(self, n: usize) -> Result + where + Self: Sized, + { + if self.total_inst - self.current_idx != n { + return Err(()) + } + + Ok(self) + } + + fn match_next_inst(mut self, mut f: F) -> Result + where + Self: Sized, + F: FnMut(&mut Self::Inst) -> Result<(), Self::Error>, + { + if self.current_idx < self.total_inst { + f(&mut self.xcm[self.current_idx])?; + self.current_idx += 1; + Ok(self) + } else { + Err(()) + } + } + + fn match_next_inst_while(mut self, cond: C, mut f: F) -> Result + where + Self: Sized, + C: Fn() -> bool, + F: FnMut(&mut Self::Inst) -> Result<(), Self::Error>, + { + if self.current_idx >= self.total_inst { + return Err(()) + } + + while cond() && self.current_idx < self.total_inst { + f(&mut self.xcm[self.current_idx])?; + self.current_idx += 1; + } + + Ok(self) + } + + fn skip_inst_while(mut self, cond: C) -> Result + where + Self: Sized, + C: Fn(&Self::Inst) -> bool, + { + if self.current_idx >= self.total_inst { + return Err(()) + } + + while cond(&self.xcm[self.current_idx]) && self.current_idx < self.total_inst { + self.current_idx += 1; + } + + Ok(self) + } +} diff --git a/xcm/src/v3/mod.rs b/xcm/src/v3/mod.rs index 0fed6defb9d4..45a960f28ffe 100644 --- a/xcm/src/v3/mod.rs +++ b/xcm/src/v3/mod.rs @@ -1,5 +1,5 @@ // Copyright 2020 Parity Technologies (UK) Ltd. -// This file is part of Cumulus. +// This file is part of Polkadot. // Substrate is free software: you can redistribute it and/or modify // it under the terms of the GNU General Public License as published by @@ -12,7 +12,7 @@ // GNU General Public License for more details. // You should have received a copy of the GNU General Public License -// along with Cumulus. If not, see . +// along with Polkadot. If not, see . //! Version 3 of the Cross-Consensus Message format data structures. @@ -34,12 +34,14 @@ use scale_info::TypeInfo; mod junction; pub(crate) mod junctions; +mod matcher; mod multiasset; mod multilocation; mod traits; pub use junction::{BodyId, BodyPart, Junction, NetworkId}; pub use junctions::Junctions; +pub use matcher::Matcher; pub use multiasset::{ AssetId, AssetInstance, Fungibility, MultiAsset, MultiAssetFilter, MultiAssets, WildFungibility, WildMultiAsset, diff --git a/xcm/src/v3/traits.rs b/xcm/src/v3/traits.rs index 6ccf01ad06c3..74ca60cd6631 100644 --- a/xcm/src/v3/traits.rs +++ b/xcm/src/v3/traits.rs @@ -1,5 +1,5 @@ // Copyright 2020 Parity Technologies (UK) Ltd. -// This file is part of Cumulus. +// This file is part of Polkadot. // Substrate is free software: you can redistribute it and/or modify // it under the terms of the GNU General Public License as published by @@ -12,7 +12,7 @@ // GNU General Public License for more details. // You should have received a copy of the GNU General Public License -// along with Cumulus. If not, see . +// along with Polkadot. If not, see . //! Cross-Consensus Message format data structures. diff --git a/xcm/xcm-builder/src/barriers.rs b/xcm/xcm-builder/src/barriers.rs index bdc7f6811edd..146a6501c1c6 100644 --- a/xcm/xcm-builder/src/barriers.rs +++ b/xcm/xcm-builder/src/barriers.rs @@ -22,12 +22,15 @@ use frame_support::{ }; use polkadot_parachain::primitives::IsSystem; use sp_std::{marker::PhantomData, result::Result}; -use xcm::latest::{ - Instruction::{self, *}, - InteriorMultiLocation, Junction, Junctions, - Junctions::X1, - MultiLocation, Weight, - WeightLimit::*, +use xcm::{ + latest::{ + Instruction::{self, *}, + InteriorMultiLocation, Junction, Junctions, + Junctions::X1, + MultiLocation, Weight, + WeightLimit::*, + }, + CreateMatcher, MatchXcm, }; use xcm_executor::traits::{OnResponse, ShouldExecute}; @@ -77,32 +80,31 @@ impl> ShouldExecute for AllowTopLevelPaidExecutionFro // 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(())?; - match i { - ReceiveTeleportedAsset(..) | - WithdrawAsset(..) | - ReserveAssetDeposited(..) | - ClaimAsset { .. } => (), - _ => return Err(()), - } - let mut i = iter.next().ok_or(())?; - while let ClearOrigin = i { - i = iter.next().ok_or(())?; - } - match i { - BuyExecution { weight_limit: Limited(ref mut weight), .. } - if weight.all_gte(max_weight) => - { - *weight = weight.max(max_weight); - Ok(()) - }, - BuyExecution { ref mut weight_limit, .. } if weight_limit == &Unlimited => { - *weight_limit = Limited(max_weight); - Ok(()) - }, - _ => Err(()), - } + let end = instructions.len().min(5); + instructions[..end] + .matcher() + .match_next_inst(|inst| match inst { + ReceiveTeleportedAsset(..) | + WithdrawAsset(..) | + ReserveAssetDeposited(..) | + ClaimAsset { .. } => Ok(()), + _ => Err(()), + })? + .skip_inst_while(|inst| matches!(inst, ClearOrigin))? + .match_next_inst(|inst| match inst { + BuyExecution { weight_limit: Limited(ref mut weight), .. } + if weight.all_gte(max_weight) => + { + *weight = weight.max(max_weight); + Ok(()) + }, + BuyExecution { ref mut weight_limit, .. } if weight_limit == &Unlimited => { + *weight_limit = Limited(max_weight); + Ok(()) + }, + _ => Err(()), + })?; + Ok(()) } } @@ -178,20 +180,24 @@ impl< // execution. This technical 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 { - match instructions.get(skipped) { - Some(UniversalOrigin(new_global)) => { - // Note the origin is *relative to local consensus*! So we need to escape local - // consensus with the `parents` before diving in into the `universal_location`. - actual_origin = X1(*new_global).relative_to(&LocalUniversal::get()); - }, - Some(DescendOrigin(j)) => { - actual_origin.append_with(*j).map_err(|_| ())?; - }, - _ => break, - } - skipped += 1; - } + instructions.matcher().match_next_inst_while( + || skipped < MaxPrefixes::get() as usize && skipped < instructions.len(), + |inst| { + let res = match inst { + UniversalOrigin(new_global) => { + // Note the origin is *relative to local consensus*! So we need to escape + // local consensus with the `parents` before diving in into the + // `universal_location`. + actual_origin = X1(*new_global).relative_to(&LocalUniversal::get()); + Ok(()) + }, + DescendOrigin(j) => actual_origin.append_with(*j).map_err(|_| ()), + _ => Ok(()), + }; + skipped += 1; + res + }, + )?; InnerBarrier::should_execute( &actual_origin, &mut instructions[skipped..], @@ -241,12 +247,12 @@ impl> ShouldExecute for AllowExplicitUnpaidExecutionF origin, instructions, max_weight, _weight_credit, ); ensure!(T::contains(origin), ()); - match instructions.first() { - Some(UnpaidExecution { weight_limit: Limited(m), .. }) if m.all_gte(max_weight) => - Ok(()), - Some(UnpaidExecution { weight_limit: Unlimited, .. }) => Ok(()), + instructions.matcher().match_next_inst(|inst| match inst { + UnpaidExecution { weight_limit: Limited(m), .. } if m.all_gte(max_weight) => Ok(()), + UnpaidExecution { weight_limit: Unlimited, .. } => Ok(()), _ => Err(()), - } + })?; + Ok(()) } } @@ -276,13 +282,16 @@ impl ShouldExecute for AllowKnownQueryResponses - Ok(()), - _ => Err(()), - } + instructions + .matcher() + .assert_remaining_insts(1)? + .match_next_inst(|inst| match inst { + QueryResponse { query_id, querier, .. } + if ResponseHandler::expecting_response(origin, *query_id, querier.as_ref()) => + Ok(()), + _ => Err(()), + })?; + Ok(()) } } @@ -302,9 +311,13 @@ impl> ShouldExecute for AllowSubscriptionsFrom { origin, instructions, _max_weight, _weight_credit, ); ensure!(T::contains(origin), ()); - match instructions { - &mut [SubscribeVersion { .. } | UnsubscribeVersion] => Ok(()), - _ => Err(()), - } + instructions + .matcher() + .assert_remaining_insts(1)? + .match_next_inst(|inst| match inst { + SubscribeVersion { .. } | UnsubscribeVersion => Ok(()), + _ => Err(()), + })?; + Ok(()) } } From 3920975f3b6e224c8d04b2eb6ee1d644e3c022d2 Mon Sep 17 00:00:00 2001 From: Keith Yeung Date: Tue, 21 Feb 2023 19:38:10 -0800 Subject: [PATCH 02/11] Fix compilation --- xcm/src/lib.rs | 52 +++++++++++++++++++++++++++++++-- xcm/src/v3/matcher.rs | 25 ++++------------ xcm/xcm-builder/src/barriers.rs | 21 ++++++------- 3 files changed, 66 insertions(+), 32 deletions(-) diff --git a/xcm/src/lib.rs b/xcm/src/lib.rs index 05878a575ce7..5f0b63098dc1 100644 --- a/xcm/src/lib.rs +++ b/xcm/src/lib.rs @@ -23,6 +23,7 @@ #![no_std] extern crate alloc; +use core::ops::ControlFlow; use derivative::Derivative; use parity_scale_codec::{Decode, Encode, Error as CodecError, Input, MaxEncodedLen}; use scale_info::TypeInfo; @@ -55,27 +56,72 @@ pub trait CreateMatcher { } /// API that allows to pattern-match against anything that is contained within an XCM. +/// +/// The intended usage of the matcher API is to enable the ability to chain successive methods of +/// this trait together, along with the ? operator for the purpose of facilitating the writing, +/// maintenance and auditability of XCM barriers. +/// +/// Example: +/// ```rust +/// use xcm::{ +/// v3::{Instruction, Matcher}, +/// CreateMatcher, MatchXcm, +/// }; +/// +/// let mut msg = [Instruction::<()>::ClearOrigin]; +/// let res = msg +/// .matcher() +/// .assert_remaining_insts(1)? +/// .match_next_inst(|inst| match inst { +/// Instruction::<()>::ClearOrigin => Ok(()), +/// _ => Err(()), +/// }); +/// assert!(res.is_ok()); +/// +/// Ok::<(), ()>(()) +/// ``` pub trait MatchXcm { type Inst; type Loc; type Error; + /// Returns success if the number of instructions that still have not been iterated over + /// equals `n`, otherwise returns an error. fn assert_remaining_insts(self, n: usize) -> Result where Self: Sized; + + /// Accepts a closure `f` that contains an argument signifying the next instruction to be + /// iterated over. The closure can then be used to check whether the instruction matches a + /// given condition, and can also be used to mutate the fields of an instruction. + /// + /// The closure `f` returns success when the instruction passes the condition, otherwise it + /// returns an error, which will ultimately be returned by this function. fn match_next_inst(self, f: F) -> Result where Self: Sized, F: FnMut(&mut Self::Inst) -> Result<(), Self::Error>; + + /// Attempts to continuously iterate through the instructions while applying `f` to each of + /// them, until either the last instruction or `cond` returns false. + /// + /// If `f` returns an error, then iteration halts and the function returns that error. + /// Otherwise, `f` returns a `ControlFlow` which signifies whether the iteration breaks or + /// continues. fn match_next_inst_while(self, cond: C, f: F) -> Result where Self: Sized, - C: Fn() -> bool, - F: FnMut(&mut Self::Inst) -> Result<(), Self::Error>; + C: Fn(&Self::Inst) -> bool, + F: FnMut(&mut Self::Inst) -> Result, Self::Error>; + + /// Iterate instructions forward until `cond` returns false. fn skip_inst_while(self, cond: C) -> Result where Self: Sized, - C: Fn(&Self::Inst) -> bool; + C: Fn(&Self::Inst) -> bool + { + Self::match_next_inst_while(self, cond, |_| Ok(ControlFlow::Continue(()))) + } } #[derive(Clone, Eq, PartialEq, Debug)] diff --git a/xcm/src/v3/matcher.rs b/xcm/src/v3/matcher.rs index 79d4006c4fc7..448f33fe99e0 100644 --- a/xcm/src/v3/matcher.rs +++ b/xcm/src/v3/matcher.rs @@ -16,6 +16,7 @@ use super::{Instruction, MultiLocation}; use crate::{CreateMatcher, MatchXcm}; +use core::ops::ControlFlow; impl<'a, Call> CreateMatcher for &'a mut [Instruction] { type Matcher = Matcher<'a, Call>; @@ -66,31 +67,17 @@ impl<'a, Call> MatchXcm for Matcher<'a, Call> { fn match_next_inst_while(mut self, cond: C, mut f: F) -> Result where Self: Sized, - C: Fn() -> bool, - F: FnMut(&mut Self::Inst) -> Result<(), Self::Error>, - { - if self.current_idx >= self.total_inst { - return Err(()) - } - - while cond() && self.current_idx < self.total_inst { - f(&mut self.xcm[self.current_idx])?; - self.current_idx += 1; - } - - Ok(self) - } - - fn skip_inst_while(mut self, cond: C) -> Result - where - Self: Sized, - C: Fn(&Self::Inst) -> bool, + C: Fn(&Self::Inst) -> bool, + F: FnMut(&mut Self::Inst) -> Result, Self::Error>, { if self.current_idx >= self.total_inst { return Err(()) } while cond(&self.xcm[self.current_idx]) && self.current_idx < self.total_inst { + if let ControlFlow::Break(()) = f(&mut self.xcm[self.current_idx])? { + break + } self.current_idx += 1; } diff --git a/xcm/xcm-builder/src/barriers.rs b/xcm/xcm-builder/src/barriers.rs index 146a6501c1c6..349e75580ed7 100644 --- a/xcm/xcm-builder/src/barriers.rs +++ b/xcm/xcm-builder/src/barriers.rs @@ -21,7 +21,7 @@ use frame_support::{ traits::{Contains, Get}, }; use polkadot_parachain::primitives::IsSystem; -use sp_std::{marker::PhantomData, result::Result}; +use sp_std::{cell::Cell, marker::PhantomData, ops::ControlFlow, result::Result}; use xcm::{ latest::{ Instruction::{self, *}, @@ -174,33 +174,34 @@ impl< origin, instructions, max_weight, weight_credit, ); let mut actual_origin = *origin; - let mut skipped = 0; + let skipped = Cell::new(0usize); // 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 // would instantly fail since the first instruction would cause an error with the // invalid UniversalOrigin. instructions.matcher().match_next_inst_while( - || skipped < MaxPrefixes::get() as usize && skipped < instructions.len(), + |_| skipped.get() < MaxPrefixes::get() as usize, |inst| { - let res = match inst { + match inst { UniversalOrigin(new_global) => { // Note the origin is *relative to local consensus*! So we need to escape // local consensus with the `parents` before diving in into the // `universal_location`. actual_origin = X1(*new_global).relative_to(&LocalUniversal::get()); - Ok(()) }, - DescendOrigin(j) => actual_origin.append_with(*j).map_err(|_| ()), - _ => Ok(()), + DescendOrigin(j) => { + let Ok(_) = actual_origin.append_with(*j) else { return Err(()) }; + }, + _ => return Ok(ControlFlow::Break(())), }; - skipped += 1; - res + skipped.set(skipped.get() + 1); + Ok(ControlFlow::Continue(())) }, )?; InnerBarrier::should_execute( &actual_origin, - &mut instructions[skipped..], + &mut instructions[skipped.get()..], max_weight, weight_credit, ) From ba143e86568118d4b004b59983d8b69f3b7cb12f Mon Sep 17 00:00:00 2001 From: Keith Yeung Date: Tue, 21 Feb 2023 20:45:07 -0800 Subject: [PATCH 03/11] cargo fmt --- xcm/src/lib.rs | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/xcm/src/lib.rs b/xcm/src/lib.rs index 5f0b63098dc1..a73aeb136649 100644 --- a/xcm/src/lib.rs +++ b/xcm/src/lib.rs @@ -56,18 +56,18 @@ pub trait CreateMatcher { } /// API that allows to pattern-match against anything that is contained within an XCM. -/// +/// /// The intended usage of the matcher API is to enable the ability to chain successive methods of /// this trait together, along with the ? operator for the purpose of facilitating the writing, /// maintenance and auditability of XCM barriers. -/// +/// /// Example: /// ```rust /// use xcm::{ /// v3::{Instruction, Matcher}, /// CreateMatcher, MatchXcm, /// }; -/// +/// /// let mut msg = [Instruction::<()>::ClearOrigin]; /// let res = msg /// .matcher() @@ -77,7 +77,7 @@ pub trait CreateMatcher { /// _ => Err(()), /// }); /// assert!(res.is_ok()); -/// +/// /// Ok::<(), ()>(()) /// ``` pub trait MatchXcm { @@ -94,7 +94,7 @@ pub trait MatchXcm { /// Accepts a closure `f` that contains an argument signifying the next instruction to be /// iterated over. The closure can then be used to check whether the instruction matches a /// given condition, and can also be used to mutate the fields of an instruction. - /// + /// /// The closure `f` returns success when the instruction passes the condition, otherwise it /// returns an error, which will ultimately be returned by this function. fn match_next_inst(self, f: F) -> Result @@ -104,7 +104,7 @@ pub trait MatchXcm { /// Attempts to continuously iterate through the instructions while applying `f` to each of /// them, until either the last instruction or `cond` returns false. - /// + /// /// If `f` returns an error, then iteration halts and the function returns that error. /// Otherwise, `f` returns a `ControlFlow` which signifies whether the iteration breaks or /// continues. @@ -118,7 +118,7 @@ pub trait MatchXcm { fn skip_inst_while(self, cond: C) -> Result where Self: Sized, - C: Fn(&Self::Inst) -> bool + C: Fn(&Self::Inst) -> bool, { Self::match_next_inst_while(self, cond, |_| Ok(ControlFlow::Continue(()))) } From 0f43e74721b9ee071c63a1a378fb8c39b6c582c8 Mon Sep 17 00:00:00 2001 From: Keith Yeung Date: Tue, 21 Feb 2023 23:21:49 -0800 Subject: [PATCH 04/11] Add more doc comments --- xcm/src/v3/matcher.rs | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/xcm/src/v3/matcher.rs b/xcm/src/v3/matcher.rs index 448f33fe99e0..f6ca276f6755 100644 --- a/xcm/src/v3/matcher.rs +++ b/xcm/src/v3/matcher.rs @@ -28,6 +28,10 @@ impl<'a, Call> CreateMatcher for &'a mut [Instruction] { } } +/// Struct created from calling `fn matcher()` on a mutable slice of `Instruction`s. +/// +/// Implements `MatchXcm` to allow an iterator-like API to match against each `Instruction` +/// contained within the slice, which facilitates the building of XCM barriers. pub struct Matcher<'a, Call> { pub(crate) xcm: &'a mut [Instruction], pub(crate) current_idx: usize, From a3c86eb7edfb1afcd355eb39d5381ecd047d177e Mon Sep 17 00:00:00 2001 From: Keith Yeung Date: Fri, 24 Feb 2023 13:09:11 -0800 Subject: [PATCH 05/11] Add mod doc comment --- xcm/src/v3/matcher.rs | 2 ++ 1 file changed, 2 insertions(+) diff --git a/xcm/src/v3/matcher.rs b/xcm/src/v3/matcher.rs index f6ca276f6755..7a261d779183 100644 --- a/xcm/src/v3/matcher.rs +++ b/xcm/src/v3/matcher.rs @@ -14,6 +14,8 @@ // You should have received a copy of the GNU General Public License // along with Polkadot. If not, see . +//! XCM matcher API, used primarily for writing barrier conditions. + use super::{Instruction, MultiLocation}; use crate::{CreateMatcher, MatchXcm}; use core::ops::ControlFlow; From 23d0edfc57198595ca22e20fea1b90ffcbd66fc4 Mon Sep 17 00:00:00 2001 From: Keith Yeung Date: Thu, 2 Mar 2023 03:47:53 -0800 Subject: [PATCH 06/11] More doc comments --- xcm/src/lib.rs | 3 +++ 1 file changed, 3 insertions(+) diff --git a/xcm/src/lib.rs b/xcm/src/lib.rs index a73aeb136649..0eaab99fb1b0 100644 --- a/xcm/src/lib.rs +++ b/xcm/src/lib.rs @@ -81,8 +81,11 @@ pub trait CreateMatcher { /// Ok::<(), ()>(()) /// ``` pub trait MatchXcm { + /// The concrete instruction type. Necessary to specify as it changes between XCM versions. type Inst; + /// The `MultiLocation` type. Necessary to specify as it changes between XCM versions. type Loc; + /// The error type to throw when errors happen during matching. type Error; /// Returns success if the number of instructions that still have not been iterated over From 5dc0fda42cbb9d5d93f5c6188274c8d5060ccca3 Mon Sep 17 00:00:00 2001 From: Keith Yeung Date: Thu, 2 Mar 2023 15:10:17 -0800 Subject: [PATCH 07/11] Add tests and fix logic --- xcm/src/v3/matcher.rs | 19 ++++++++++++++++++- 1 file changed, 18 insertions(+), 1 deletion(-) diff --git a/xcm/src/v3/matcher.rs b/xcm/src/v3/matcher.rs index 7a261d779183..696f35383dad 100644 --- a/xcm/src/v3/matcher.rs +++ b/xcm/src/v3/matcher.rs @@ -80,7 +80,7 @@ impl<'a, Call> MatchXcm for Matcher<'a, Call> { return Err(()) } - while cond(&self.xcm[self.current_idx]) && self.current_idx < self.total_inst { + while self.current_idx < self.total_inst && cond(&self.xcm[self.current_idx]) { if let ControlFlow::Break(()) = f(&mut self.xcm[self.current_idx])? { break } @@ -90,3 +90,20 @@ impl<'a, Call> MatchXcm for Matcher<'a, Call> { Ok(self) } } + +#[cfg(test)] +mod tests { + use super::*; + use crate::v3::prelude::*; + use alloc::{vec, vec::Vec}; + + #[test] + fn match_next_inst_while_works() { + let mut xcm: Vec> = vec![ClearOrigin]; + + let _ = xcm[..] + .matcher() + .match_next_inst_while(|_| true, |_| Ok(ControlFlow::Continue(()))) + .unwrap(); + } +} From ddbdb8832fdbbe886c7c5de81b1e0ae72ea4ca4a Mon Sep 17 00:00:00 2001 From: Keith Yeung Date: Fri, 3 Mar 2023 15:03:50 -0800 Subject: [PATCH 08/11] Remove redundant syntax --- xcm/src/v3/matcher.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/xcm/src/v3/matcher.rs b/xcm/src/v3/matcher.rs index 696f35383dad..de390338e3f5 100644 --- a/xcm/src/v3/matcher.rs +++ b/xcm/src/v3/matcher.rs @@ -101,7 +101,7 @@ mod tests { fn match_next_inst_while_works() { let mut xcm: Vec> = vec![ClearOrigin]; - let _ = xcm[..] + let _ = xcm .matcher() .match_next_inst_while(|_| true, |_| Ok(ControlFlow::Continue(()))) .unwrap(); From 2a0819b285ace40384f48c4fcd228aecec57ea28 Mon Sep 17 00:00:00 2001 From: Keith Yeung Date: Fri, 3 Mar 2023 15:07:58 -0800 Subject: [PATCH 09/11] Add more doc comments --- xcm/src/lib.rs | 3 +++ 1 file changed, 3 insertions(+) diff --git a/xcm/src/lib.rs b/xcm/src/lib.rs index 0eaab99fb1b0..43a18cfdd3e5 100644 --- a/xcm/src/lib.rs +++ b/xcm/src/lib.rs @@ -51,7 +51,10 @@ pub type Version = u32; /// Creates an instruction matcher from an XCM. Since XCM versions differ, we need to make a trait /// here to unify the interfaces among them. pub trait CreateMatcher { + /// The concrete matcher type. type Matcher; + + /// Method that creates and returns the matcher type from the implementing type. fn matcher(self) -> Self::Matcher; } From c9c8fe3d54fce2e5ca42cbc0662de4b78bae53cf Mon Sep 17 00:00:00 2001 From: Keith Yeung Date: Fri, 3 Mar 2023 15:12:50 -0800 Subject: [PATCH 10/11] Add more doc comments --- xcm/src/lib.rs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/xcm/src/lib.rs b/xcm/src/lib.rs index 43a18cfdd3e5..7c6a423f5f89 100644 --- a/xcm/src/lib.rs +++ b/xcm/src/lib.rs @@ -120,7 +120,8 @@ pub trait MatchXcm { C: Fn(&Self::Inst) -> bool, F: FnMut(&mut Self::Inst) -> Result, Self::Error>; - /// Iterate instructions forward until `cond` returns false. + /// Iterate instructions forward until `cond` returns false. When there are no more instructions + /// to be read, an error is returned. fn skip_inst_while(self, cond: C) -> Result where Self: Sized, From 5e3f85976e087559919561e5aa6029c4275fd121 Mon Sep 17 00:00:00 2001 From: Keith Yeung Date: Fri, 3 Mar 2023 15:13:40 -0800 Subject: [PATCH 11/11] Add more doc comments --- xcm/src/lib.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/xcm/src/lib.rs b/xcm/src/lib.rs index 7c6a423f5f89..a49806e16b77 100644 --- a/xcm/src/lib.rs +++ b/xcm/src/lib.rs @@ -54,7 +54,7 @@ pub trait CreateMatcher { /// The concrete matcher type. type Matcher; - /// Method that creates and returns the matcher type from the implementing type. + /// Method that creates and returns the matcher type from `Self`. fn matcher(self) -> Self::Matcher; }