From 03d1470a0e68ce08894f5631b5c7405a46725f5d Mon Sep 17 00:00:00 2001 From: kianenigma Date: Wed, 28 Apr 2021 11:16:51 +0200 Subject: [PATCH 01/35] WIP --- runtime/rococo/src/lib.rs | 2 +- xcm/src/v0/multi_asset.rs | 66 +++++++++++------ xcm/src/v0/traits.rs | 70 +++++++++++++++++-- xcm/xcm-builder/src/lib.rs | 2 + xcm/xcm-executor/src/lib.rs | 11 ++- xcm/xcm-executor/src/traits/conversion.rs | 50 ++++++++++++- xcm/xcm-executor/src/traits/should_execute.rs | 13 ++-- 7 files changed, 180 insertions(+), 34 deletions(-) diff --git a/runtime/rococo/src/lib.rs b/runtime/rococo/src/lib.rs index 2ef95f3820b8..6167e5a9afaa 100644 --- a/runtime/rococo/src/lib.rs +++ b/runtime/rococo/src/lib.rs @@ -619,7 +619,7 @@ pub type TrustedTeleporters = ( ); parameter_types! { - pub AllowUnpaidFrom: Vec = + pub AllowUnpaidFrom: Vec = vec![ X1(Parachain{id: 100}), X1(Parachain{id: 110}), diff --git a/xcm/src/v0/multi_asset.rs b/xcm/src/v0/multi_asset.rs index 216745217588..8fa326b2a292 100644 --- a/xcm/src/v0/multi_asset.rs +++ b/xcm/src/v0/multi_asset.rs @@ -61,35 +61,34 @@ pub enum AssetInstance { /// ### Abstract identifiers /// /// Abstract identifiers are absolute identifiers that represent a notional asset which can exist within multiple -/// consensus systems. These tend to be simpler to deal with since their broad meaning is unchanged regardless stay -/// of the consensus system in which it is interpreted. +/// consensus systems. These tend to be simpler to deal with since their broad meaning is unchanged regardless stay of +/// the consensus system in which it is interpreted. /// /// However, in the attempt to provide uniformity across consensus systems, they may conflate different instantiations /// of some notional asset (e.g. the reserve asset and a local reserve-backed derivative of it) under the same name, -/// leading to confusion. It also implies that one notional asset is accounted for locally in only one way. This may -/// not be the case, e.g. where there are multiple bridge instances each providing a bridged "BTC" token yet none -/// being fungible between the others. +/// leading to confusion. It also implies that one notional asset is accounted for locally in only one way. This may not +/// be the case, e.g. where there are multiple bridge instances each providing a bridged "BTC" token yet none being +/// fungible between the others. /// -/// Since they are meant to be absolute and universal, a global registry is needed to ensure that name collisions -/// do not occur. +/// Since they are meant to be absolute and universal, a global registry is needed to ensure that name collisions do not +/// occur. /// /// An abstract identifier is represented as a simple variable-size byte string. As of writing, no global registry /// exists and no proposals have been put forth for asset labeling. /// /// ### Concrete identifiers /// -/// Concrete identifiers are *relative identifiers* that specifically identify a single asset through its location in -/// a consensus system relative to the context interpreting. Use of a `MultiLocation` ensures that similar but non -/// fungible variants of the same underlying asset can be properly distinguished, and obviates the need for any kind -/// of central registry. +/// Concrete identifiers are *relative identifiers* that specifically identify a single asset through its location in a +/// consensus system relative to the context interpreting. Use of a `MultiLocation` ensures that similar but non +/// fungible variants of the same underlying asset can be properly distinguished, and obviates the need for any kind of +/// central registry. /// -/// The limitation is that the asset identifier cannot be trivially copied between consensus -/// systems and must instead be "re-anchored" whenever being moved to a new consensus system, using the two systems' -/// relative paths. +/// The limitation is that the asset identifier cannot be trivially copied between consensus systems and must instead be +/// "re-anchored" whenever being moved to a new consensus system, using the two systems' relative paths. /// -/// Throughout XCM, messages are authored such that *when interpreted from the receiver's point of view* they will -/// have the desired meaning/effect. This means that relative paths should always by constructed to be read from the -/// point of view of the receiving system, *which may be have a completely different meaning in the authoring system*. +/// Throughout XCM, messages are authored such that *when interpreted from the receiver's point of view* they will have +/// the desired meaning/effect. This means that relative paths should always by constructed to be read from the point of +/// view of the receiving system, *which may be have a completely different meaning in the authoring system*. /// /// Concrete identifiers are the preferred way of identifying an asset since they are entirely unambiguous. /// @@ -99,8 +98,8 @@ pub enum AssetInstance { /// /// - `/PalletInstance()` for a Frame chain with a single-asset pallet instance (such as an instance of the /// Balances pallet). -/// - `/PalletInstance()/GeneralIndex()` for a Frame chain with an indexed multi-asset pallet -/// instance (such as an instance of the Assets pallet). +/// - `/PalletInstance()/GeneralIndex()` for a Frame chain with an indexed multi-asset pallet instance +/// (such as an instance of the Assets pallet). /// - `/AccountId32` for an ERC-20-style single-asset smart-contract on a Frame-based contracts chain. /// - `/AccountKey20` for an ERC-20-style single-asset smart-contract on an Ethereum-like chain. /// @@ -147,6 +146,9 @@ pub enum MultiAsset { } impl MultiAsset { + /// Returns `true` if the `MultiAsset` is a wildcard and can refer to classes of assets, instead of just one. + /// + /// Typically can also be inferred by the name staring with `All`. pub fn is_wildcard(&self) -> bool { match self { MultiAsset::None @@ -178,6 +180,7 @@ impl MultiAsset { } } + // TODO: `All` is both fungible and non-fungible, and I think it should be none. fn is_fungible(&self) -> bool { match self { MultiAsset::All @@ -209,7 +212,6 @@ impl MultiAsset { fn is_concrete_fungible(&self, id: &MultiLocation) -> bool { match self { MultiAsset::AllFungible => true, - MultiAsset::AllConcreteFungible { id: i } | MultiAsset::ConcreteFungible { id: i, .. } => i == id, @@ -250,8 +252,32 @@ impl MultiAsset { fn is_all(&self) -> bool { matches!(self, MultiAsset::All) } + /// Returns true if `self` is a super-set of the given `inner`. + /// + /// # Examples + /// + /// ```rust + /// # use xcm::v0::MultiAsset; + # fn main() { + // trivial case: all contains anything + assert!(MultiAsset::All.contains(MultiAsset::None)); + assert!(MultiAsset::All.contains(MultiAsset::AllFungible)); + + // trivial case: none contains nothing + assert!(!MultiAsset::None.contains(MultiAsset::None)); + assert!(!MultiAsset::None.contains(MultiAsset::AllFungible)); + + // A bit more sneaky: Nothing can contains All, even All. + // TODO: I think this is inconsistent, there should be an exception for this case. + assert!(!MultiAsset::All.contains(MultiAsset::All)); + // but AllFungible and AllNonFungible contain themselves. + assert!(MultiAsset::AllFungible.contains(MultiAsset::AllFungible)); + assert!(MultiAsset::AllNonFungible.contains(MultiAsset::AllNonFungible)); + # } + /// ``` pub fn contains(&self, inner: &MultiAsset) -> bool { use MultiAsset::*; + // Inner cannot be wild if inner.is_wildcard() { return false } // Everything contains nothing. diff --git a/xcm/src/v0/traits.rs b/xcm/src/v0/traits.rs index 938f5ddd1904..0aca681d9cc9 100644 --- a/xcm/src/v0/traits.rs +++ b/xcm/src/v0/traits.rs @@ -85,7 +85,7 @@ pub type Result = result::Result<(), Error>; /// Local weight type; execution time in picoseconds. pub type Weight = u64; -/// Outcome of an XCM excution. +/// Outcome of an XCM execution. #[derive(Clone, Encode, Decode, Eq, PartialEq, Debug)] pub enum Outcome { /// Execution completed successfully; given weight was used. @@ -121,6 +121,8 @@ impl Outcome { } } +/// Something that can execute an xcm message. +// TODO: why is this both generic and associated? pub trait ExecuteXcm { type Call; fn execute_xcm(origin: MultiLocation, message: Xcm, weight_limit: Weight) -> Outcome; @@ -135,13 +137,72 @@ impl ExecuteXcm for () { /// Utility for sending an XCM message. /// -/// These can be amalgamted in tuples to form sophisticated routing systems. +/// These can be amalgamated in tuples to form sophisticated routing systems. In tuple format, each router might return +/// `CannotReachDestination` to pass the execution to the next sender item. Note that each `CannotReachDestination` +/// might alter the destination and the xcm message for to the next router. +/// +/// +/// # Example +/// ```rust +/// # use xcm::v0::{MultiLocation, Xcm, Junction, Error, OriginKind, SendXcm, Result}; +/// # use parity_scale_codec::Encode; +/// +/// /// A sender that only passes the message through and does nothing. +/// struct Sender1; +/// impl SendXcm for Sender1 { +/// fn send_xcm(destination: MultiLocation, message: Xcm<()>) -> Result { +/// return Err(Error::CannotReachDestination(destination, message)) +/// } +/// } +/// +/// /// A sender that accepts a message that has an X2 junction, otherwise stops the routing. +/// struct Sender2; +/// impl SendXcm for Sender2 { +/// fn send_xcm(destination: MultiLocation, message: Xcm<()>) -> Result { +/// if let MultiLocation::X2(j1, j2) = destination { +/// Ok(()) +/// } else { +/// Err(Error::Undefined) +/// } +/// } +/// } +/// +/// /// A sender that accepts a message from a X1 parent junction, passing through otherwise. +/// struct Sender3; +/// impl SendXcm for Sender3 { +/// fn send_xcm(destination: MultiLocation, message: Xcm<()>) -> Result { +/// match destination { +/// MultiLocation::X1(j) if j == Junction::Parent => Ok(()), +/// _ => Err(Error::CannotReachDestination(destination, message)), +/// } +/// } +/// } +/// +/// // an xcm message to send. We don't really care about this. +/// # fn main() { +/// let call: Vec = ().encode(); +/// let message = Xcm::Transact { origin_type: OriginKind::Superuser, require_weight_at_most: 0, call: call.into() }; +/// let destination = MultiLocation::X1(Junction::Parent); +/// +/// assert!( +/// // Sender2 will block this. +/// <(Sender1, Sender2, Sender3) as SendXcm>::send_xcm(destination.clone(), message.clone()) +/// .is_err() +/// ); +/// +/// assert!( +/// // Sender3 will catch this. +/// <(Sender1, Sender3) as SendXcm>::send_xcm(destination.clone(), message.clone()) +/// .is_ok() +/// ); +/// # } +/// ``` pub trait SendXcm { /// Send an XCM `message` to a given `destination`. /// /// If it is not a destination which can be reached with this type but possibly could by others, - /// then it *MUST* return `CannotReachDestination`. Any other error will cause the tuple implementation to - /// exit early without trying other type fields. + /// then it *MUST* return `CannotReachDestination`. Any other error will cause the tuple + /// implementation to exit early without trying other type fields. fn send_xcm(destination: MultiLocation, message: Xcm<()>) -> Result; } @@ -149,6 +210,7 @@ pub trait SendXcm { impl SendXcm for Tuple { fn send_xcm(destination: MultiLocation, message: Xcm<()>) -> Result { for_tuples!( #( + // we shadow `destination` and `message` in each expansion for the next one. let (destination, message) = match Tuple::send_xcm(destination, message) { Err(Error::CannotReachDestination(d, m)) => (d, m), o @ _ => return o, diff --git a/xcm/xcm-builder/src/lib.rs b/xcm/xcm-builder/src/lib.rs index 4dc63aff797e..5cb67d8fa306 100644 --- a/xcm/xcm-builder/src/lib.rs +++ b/xcm/xcm-builder/src/lib.rs @@ -14,6 +14,8 @@ // You should have received a copy of the GNU General Public License // along with Polkadot. If not, see . +//! Types and helpers for creating XCM configuration. + #![cfg_attr(not(feature = "std"), no_std)] #[cfg(test)] diff --git a/xcm/xcm-executor/src/lib.rs b/xcm/xcm-executor/src/lib.rs index 5c51341543b1..d5371f517b69 100644 --- a/xcm/xcm-executor/src/lib.rs +++ b/xcm/xcm-executor/src/lib.rs @@ -28,8 +28,8 @@ use xcm::v0::{ pub mod traits; use traits::{ - TransactAsset, ConvertOrigin, FilterAssetLocation, InvertLocation, WeightBounds, WeightTrader, ShouldExecute, - OnResponse + TransactAsset, ConvertOrigin, FilterAssetLocation, InvertLocation, WeightBounds, WeightTrader, + ShouldExecute, OnResponse }; mod assets; @@ -37,10 +37,12 @@ pub use assets::{Assets, AssetId}; mod config; pub use config::Config; +/// The XCM executor. pub struct XcmExecutor(PhantomData); impl ExecuteXcm for XcmExecutor { type Call = Config::Call; + /// execute an XCM call with the given weight limit. fn execute_xcm(origin: MultiLocation, message: Xcm, weight_limit: Weight) -> Outcome { // TODO: #2841 #HARDENXCM We should identify recursive bombs here and bail. let mut message = Xcm::::from(message); @@ -52,13 +54,16 @@ impl ExecuteXcm for XcmExecutor { Ok(x) => x, Err(()) => return Outcome::Error(XcmError::WeightNotComputable), }; + let maximum_weight = match shallow_weight.checked_add(deep_weight) { Some(x) => x, - None => return Outcome::Error(XcmError::WeightLimitReached), + None => return Outcome::Error(XcmError::WeightLimitReached), // TODO: this is overflow... but anyhow. }; + if maximum_weight > weight_limit { return Outcome::Error(XcmError::WeightLimitReached); } + let mut trader = Config::Trader::new(); match Self::do_execute_xcm(origin, true, message, &mut 0, Some(shallow_weight), &mut trader) { Ok(surplus) => Outcome::Complete(maximum_weight.saturating_sub(surplus)), diff --git a/xcm/xcm-executor/src/traits/conversion.rs b/xcm/xcm-executor/src/traits/conversion.rs index 6237571335bc..44cb27136692 100644 --- a/xcm/xcm-executor/src/traits/conversion.rs +++ b/xcm/xcm-executor/src/traits/conversion.rs @@ -24,6 +24,9 @@ use xcm::v0::{MultiLocation, OriginKind}; /// One of `convert`/`convert_ref` and `reverse`/`reverse_ref` MUST be implemented. If possible, implement /// `convert_ref`, since this will never result in a clone. Use `convert` when you definitely need to consume /// the source value. +/// +/// Can be amalgamated into tuples. If any of the tuple elements converts into `Ok(_)` short circuits, otherwise returns +/// the `Err(_)` of the last failing conversion (or `Err(())` for ref conversions). pub trait Convert { /// Convert from `value` (of type `A`) into an equivalent value of type `B`, `Err` if not possible. fn convert(value: A) -> Result { Self::convert_ref(&value).map_err(|_| value) } @@ -115,7 +118,49 @@ impl Convert, T> for Decoded { fn reverse_ref(value: impl Borrow) -> Result, ()> { Ok(value.borrow().encode()) } } +/// A convertor trait for origin types. +/// +/// Can be amalgamated into tuples. If any of the tuple elements returns `Ok(_)`, it short circuits. Else, the `Err(_)` +/// of the last tuple item is returned. Each intermediate `Err(_)` might return a different `origin` of type `Origin` +/// which is passed to the next convert item. +/// +/// ```rust +/// # use xcm::v0::{MultiLocation, Junction, OriginKind}; +/// # use xcm_executor::traits::ConvertOrigin; +/// // A convertor that will bump the para id and pass it to the next one. +/// struct BumpParaId; +/// impl ConvertOrigin for BumpParaId { +/// fn convert_origin(origin: MultiLocation, _: OriginKind) -> Result { +/// match origin { +/// MultiLocation::X1(Junction::Parachain { id }) => { +/// Err(MultiLocation::X1(Junction::Parachain { id: id + 1 })) +/// } +/// _ => unreachable!() +/// } +/// } +/// } +/// +/// struct AcceptPara7; +/// impl ConvertOrigin for AcceptPara7 { +/// fn convert_origin(origin: MultiLocation, _: OriginKind) -> Result { +/// match origin { +/// MultiLocation::X1(Junction::Parachain { id }) if id == 7 => { +/// Ok(7) +/// } +/// _ => Err(origin) +/// } +/// } +/// } +/// # fn main() { +/// let origin = MultiLocation::X1(Junction::Parachain { id: 6 }); +/// assert!( +/// <(BumpParaId, AcceptPara7) as ConvertOrigin>::convert_origin(origin, OriginKind::Native) +/// .is_ok() +/// ); +/// # } +/// ``` pub trait ConvertOrigin { + /// Attempt to convert `origin` to the generic `Origin` whilst consuming it. fn convert_origin(origin: MultiLocation, kind: OriginKind) -> Result; } @@ -123,7 +168,10 @@ pub trait ConvertOrigin { impl ConvertOrigin for Tuple { fn convert_origin(origin: MultiLocation, kind: OriginKind) -> Result { for_tuples!( #( - let origin = match Tuple::convert_origin(origin, kind) { Err(o) => o, r => return r }; + let origin = match Tuple::convert_origin(origin, kind) { + Err(o) => o, + r => return r + }; )* ); Err(origin) } diff --git a/xcm/xcm-executor/src/traits/should_execute.rs b/xcm/xcm-executor/src/traits/should_execute.rs index e373616d5ee2..3c35ec404397 100644 --- a/xcm/xcm-executor/src/traits/should_execute.rs +++ b/xcm/xcm-executor/src/traits/should_execute.rs @@ -19,18 +19,21 @@ use xcm::v0::{Xcm, MultiLocation}; use frame_support::weights::Weight; /// 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 the tuple elements returns `Ok()`, the +/// execution stops. Else, `Err(_)` is returned if all elements reject the message. pub trait ShouldExecute { /// Returns `true` if the given `message` may be executed. /// /// - `origin`: The origin (sender) of the message. - /// - `top_level`: `true`` indicates the initial XCM coming from the `origin`, `false` indicates an embedded - /// XCM executed internally as part of another message or an `Order`. + /// - `top_level`: `true`` indicates the initial XCM coming from the `origin`, `false` indicates an embedded XCM + /// executed internally as part of another message or an `Order`. /// - `message`: The message itself. /// - `shallow_weight`: The weight of the non-negotiable execution of the message. This does not include any /// embedded XCMs sat behind mechanisms like `BuyExecution` which would need to answer for their own weight. - /// - `weight_credit`: The pre-established amount of weight that the system has determined this message - /// may utilise in its execution. Typically non-zero only because of prior fee payment, but could - /// in principle be due to other factors. + /// - `weight_credit`: The pre-established amount of weight that the system has determined this message may utilise + /// in its execution. Typically non-zero only because of prior fee payment, but could in principle be due to other + /// factors. fn should_execute( origin: &MultiLocation, top_level: bool, From 59de11dc05cd2b17a16c82bd9dfcedd8217a736e Mon Sep 17 00:00:00 2001 From: Alexander Popiak Date: Tue, 27 Apr 2021 17:56:23 +0100 Subject: [PATCH 02/35] add tests and docs for DoubleEncoded --- xcm/src/double_encoded.rs | 44 +++++++++++++++++++++++++++++++++++++-- 1 file changed, 42 insertions(+), 2 deletions(-) diff --git a/xcm/src/double_encoded.rs b/xcm/src/double_encoded.rs index fd6d10054f05..d9ba93d7d6eb 100644 --- a/xcm/src/double_encoded.rs +++ b/xcm/src/double_encoded.rs @@ -17,6 +17,8 @@ use alloc::vec::Vec; use parity_scale_codec::{Encode, Decode}; +/// Wrapper around the encoded and decoded versions of a value. +/// Caches the decoded value once computed. #[derive(Encode, Decode)] #[codec(encode_bound())] #[codec(decode_bound())] @@ -29,11 +31,12 @@ pub struct DoubleEncoded { impl Clone for DoubleEncoded { fn clone(&self) -> Self { Self { encoded: self.encoded.clone(), decoded: None } } } -impl Eq for DoubleEncoded { -} + impl PartialEq for DoubleEncoded { fn eq(&self, other: &Self) -> bool { self.encoded.eq(&other.encoded) } } +impl Eq for DoubleEncoded {} + impl core::fmt::Debug for DoubleEncoded { fn fmt(&self, f: &mut core::fmt::Formatter<'_>) -> core::fmt::Result { self.encoded.fmt(f) } } @@ -46,29 +49,66 @@ impl From> for DoubleEncoded { impl DoubleEncoded { pub fn into(self) -> DoubleEncoded { DoubleEncoded::from(self) } + pub fn from(e: DoubleEncoded) -> Self { Self { encoded: e.encoded, decoded: None, } } + + /// Provides an API similar to `AsRef` that provides access to the inner value. + /// `AsRef` implementation would expect an `&Option` return type. pub fn as_ref(&self) -> Option<&T> { self.decoded.as_ref() } } impl DoubleEncoded { + /// Decode the inner encoded value and store it. + /// Returns a reference to the value in case of success and `Err(())` in case the decoding fails. pub fn ensure_decoded(&mut self) -> Result<&T, ()> { if self.decoded.is_none() { self.decoded = T::decode(&mut &self.encoded[..]).ok(); } self.decoded.as_ref().ok_or(()) } + + /// Move the decoded value out or (if not present) decode `encoded`. pub fn take_decoded(&mut self) -> Result { self.decoded.take().or_else(|| T::decode(&mut &self.encoded[..]).ok()).ok_or(()) } + + /// Provides an API similar to `TryInto` that allows fallible conversion to the inner value type. + /// `TryInto` implementation would collide with std blanket implementation based on `TryFrom`. pub fn try_into(mut self) -> Result { self.ensure_decoded()?; self.decoded.ok_or(()) } } + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn ensure_decoded_works() { + let val: u64 = 42; + let mut encoded: DoubleEncoded<_> = Encode::encode(&val).into(); + assert_eq!(encoded.ensure_decoded(), Ok(&val)); + } + + #[test] + fn take_decoded_works() { + let val: u64 = 42; + let mut encoded: DoubleEncoded<_> = Encode::encode(&val).into(); + assert_eq!(encoded.take_decoded(), Ok(val)); + } + + #[test] + fn try_into_works() { + let val: u64 = 42; + let encoded: DoubleEncoded<_> = Encode::encode(&val).into(); + assert_eq!(encoded.try_into(), Ok(val)); + } +} From 71c9b5ba458399e568a96798c60558fd655440fb Mon Sep 17 00:00:00 2001 From: Alexander Popiak Date: Tue, 27 Apr 2021 18:23:08 +0100 Subject: [PATCH 03/35] reformat parent_count --- xcm/src/v0/multi_location.rs | 113 ++++++++++++++--------------------- 1 file changed, 44 insertions(+), 69 deletions(-) diff --git a/xcm/src/v0/multi_location.rs b/xcm/src/v0/multi_location.rs index 5226c67cc7e7..6c79a0dd010e 100644 --- a/xcm/src/v0/multi_location.rs +++ b/xcm/src/v0/multi_location.rs @@ -479,76 +479,51 @@ impl MultiLocation { /// Returns the number of `Parent` junctions at the beginning of `self`. pub fn parent_count(&self) -> usize { + use Junction::Parent; match self { - MultiLocation::X8( - Junction::Parent, Junction::Parent, Junction::Parent, Junction::Parent, Junction::Parent, - Junction::Parent, Junction::Parent, Junction::Parent - ) => 8, - - MultiLocation::X8( - Junction::Parent, Junction::Parent, Junction::Parent, Junction::Parent, Junction::Parent, - Junction::Parent, Junction::Parent, .. - ) => 7, - MultiLocation::X7( - Junction::Parent, Junction::Parent, Junction::Parent, Junction::Parent, Junction::Parent, - Junction::Parent, Junction::Parent - ) => 7, - - MultiLocation::X8( - Junction::Parent, Junction::Parent, Junction::Parent, Junction::Parent, Junction::Parent, - Junction::Parent, .. - ) => 6, - MultiLocation::X7( - Junction::Parent, Junction::Parent, Junction::Parent, Junction::Parent, Junction::Parent, - Junction::Parent, .. - ) => 6, - MultiLocation::X6( - Junction::Parent, Junction::Parent, Junction::Parent, Junction::Parent, Junction::Parent, - Junction::Parent - ) => 6, - - MultiLocation::X8( - Junction::Parent, Junction::Parent, Junction::Parent, Junction::Parent, Junction::Parent, .. - ) => 5, - MultiLocation::X7( - Junction::Parent, Junction::Parent, Junction::Parent, Junction::Parent, Junction::Parent, .. - ) => 5, - MultiLocation::X6( - Junction::Parent, Junction::Parent, Junction::Parent, Junction::Parent, Junction::Parent, .. - ) => 5, - MultiLocation::X5( - Junction::Parent, Junction::Parent, Junction::Parent, Junction::Parent, Junction::Parent - ) => 5, - - MultiLocation::X8(Junction::Parent, Junction::Parent, Junction::Parent, Junction::Parent, ..) => 4, - MultiLocation::X7(Junction::Parent, Junction::Parent, Junction::Parent, Junction::Parent, ..) => 4, - MultiLocation::X6(Junction::Parent, Junction::Parent, Junction::Parent, Junction::Parent, ..) => 4, - MultiLocation::X5(Junction::Parent, Junction::Parent, Junction::Parent, Junction::Parent, ..) => 4, - MultiLocation::X4(Junction::Parent, Junction::Parent, Junction::Parent, Junction::Parent) => 4, - - MultiLocation::X8(Junction::Parent, Junction::Parent, Junction::Parent, ..) => 3, - MultiLocation::X7(Junction::Parent, Junction::Parent, Junction::Parent, ..) => 3, - MultiLocation::X6(Junction::Parent, Junction::Parent, Junction::Parent, ..) => 3, - MultiLocation::X5(Junction::Parent, Junction::Parent, Junction::Parent, ..) => 3, - MultiLocation::X4(Junction::Parent, Junction::Parent, Junction::Parent, ..) => 3, - MultiLocation::X3(Junction::Parent, Junction::Parent, Junction::Parent) => 3, - - MultiLocation::X8(Junction::Parent, Junction::Parent, ..) => 2, - MultiLocation::X7(Junction::Parent, Junction::Parent, ..) => 2, - MultiLocation::X6(Junction::Parent, Junction::Parent, ..) => 2, - MultiLocation::X5(Junction::Parent, Junction::Parent, ..) => 2, - MultiLocation::X4(Junction::Parent, Junction::Parent, ..) => 2, - MultiLocation::X3(Junction::Parent, Junction::Parent, ..) => 2, - MultiLocation::X2(Junction::Parent, Junction::Parent) => 2, - - MultiLocation::X8(Junction::Parent, ..) => 1, - MultiLocation::X7(Junction::Parent, ..) => 1, - MultiLocation::X6(Junction::Parent, ..) => 1, - MultiLocation::X5(Junction::Parent, ..) => 1, - MultiLocation::X4(Junction::Parent, ..) => 1, - MultiLocation::X3(Junction::Parent, ..) => 1, - MultiLocation::X2(Junction::Parent, ..) => 1, - MultiLocation::X1(Junction::Parent) => 1, + MultiLocation::X8(Parent, Parent, Parent, Parent, Parent, Parent, Parent, Parent) => 8, + + MultiLocation::X8(Parent, Parent, Parent, Parent, Parent, Parent, Parent, ..) => 7, + MultiLocation::X7(Parent, Parent, Parent, Parent, Parent, Parent, Parent) => 7, + + MultiLocation::X8(Parent, Parent, Parent, Parent, Parent, Parent, ..) => 6, + MultiLocation::X7(Parent, Parent, Parent, Parent, Parent, Parent, ..) => 6, + MultiLocation::X6(Parent, Parent, Parent, Parent, Parent, Parent) => 6, + + MultiLocation::X8(Parent, Parent, Parent, Parent, Parent, ..) => 5, + MultiLocation::X7(Parent, Parent, Parent, Parent, Parent, ..) => 5, + MultiLocation::X6(Parent, Parent, Parent, Parent, Parent, ..) => 5, + MultiLocation::X5(Parent, Parent, Parent, Parent, Parent) => 5, + + MultiLocation::X8(Parent, Parent, Parent, Parent, ..) => 4, + MultiLocation::X7(Parent, Parent, Parent, Parent, ..) => 4, + MultiLocation::X6(Parent, Parent, Parent, Parent, ..) => 4, + MultiLocation::X5(Parent, Parent, Parent, Parent, ..) => 4, + MultiLocation::X4(Parent, Parent, Parent, Parent) => 4, + + MultiLocation::X8(Parent, Parent, Parent, ..) => 3, + MultiLocation::X7(Parent, Parent, Parent, ..) => 3, + MultiLocation::X6(Parent, Parent, Parent, ..) => 3, + MultiLocation::X5(Parent, Parent, Parent, ..) => 3, + MultiLocation::X4(Parent, Parent, Parent, ..) => 3, + MultiLocation::X3(Parent, Parent, Parent) => 3, + + MultiLocation::X8(Parent, Parent, ..) => 2, + MultiLocation::X7(Parent, Parent, ..) => 2, + MultiLocation::X6(Parent, Parent, ..) => 2, + MultiLocation::X5(Parent, Parent, ..) => 2, + MultiLocation::X4(Parent, Parent, ..) => 2, + MultiLocation::X3(Parent, Parent, ..) => 2, + MultiLocation::X2(Parent, Parent) => 2, + + MultiLocation::X8(Parent, ..) => 1, + MultiLocation::X7(Parent, ..) => 1, + MultiLocation::X6(Parent, ..) => 1, + MultiLocation::X5(Parent, ..) => 1, + MultiLocation::X4(Parent, ..) => 1, + MultiLocation::X3(Parent, ..) => 1, + MultiLocation::X2(Parent, ..) => 1, + MultiLocation::X1(Parent) => 1, _ => 0, } } From bbec29d6db01d16de0e1664e877334ff9e6923ed Mon Sep 17 00:00:00 2001 From: Alexander Popiak Date: Tue, 27 Apr 2021 18:23:24 +0100 Subject: [PATCH 04/35] add test for match_and_split --- xcm/src/v0/multi_location.rs | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) diff --git a/xcm/src/v0/multi_location.rs b/xcm/src/v0/multi_location.rs index 6c79a0dd010e..e9e0154536c2 100644 --- a/xcm/src/v0/multi_location.rs +++ b/xcm/src/v0/multi_location.rs @@ -594,3 +594,21 @@ impl TryFrom for MultiLocation { } } } + +#[cfg(test)] +mod tests { + use super::MultiLocation::*; + + use crate::opaque::v0::{Junction::*, NetworkId::Any}; + + #[test] + fn match_and_split_works() { + let m = X3(Parent, Parachain { id: 42 }, AccountIndex64 { network: Any, index: 23 }); + assert_eq!(m.match_and_split(&X1(Parent)), None); + assert_eq!( + m.match_and_split(&X2(Parent, Parachain { id: 42 })), + Some(&AccountIndex64 { network: Any, index: 23 }) + ); + assert_eq!(m.match_and_split(&m), None); + } +} From b96f8981f0b4d879762dfbd190e48bf27eacbb9f Mon Sep 17 00:00:00 2001 From: Alexander Popiak Date: Wed, 28 Apr 2021 10:20:32 +0100 Subject: [PATCH 05/35] fix append_with docs and add tests --- xcm/src/v0/multi_location.rs | 19 +++++++++++++++++-- 1 file changed, 17 insertions(+), 2 deletions(-) diff --git a/xcm/src/v0/multi_location.rs b/xcm/src/v0/multi_location.rs index e9e0154536c2..fb353570052b 100644 --- a/xcm/src/v0/multi_location.rs +++ b/xcm/src/v0/multi_location.rs @@ -528,10 +528,10 @@ impl MultiLocation { } } - /// Mutate `self` so that it is suffixed with `prefix`. The correct normalised form is returned, removing any + /// Mutate `self` so that it is suffixed with `suffix`. The correct normalised form is returned, removing any /// internal `Parent`s. /// - /// Does not modify `self` and returns `Err` with `prefix` in case of overflow. + /// Does not modify `self` and returns `Err` with `suffix` in case of overflow. pub fn append_with(&mut self, suffix: MultiLocation) -> Result<(), MultiLocation> { let mut prefix = suffix; core::mem::swap(self, &mut prefix); @@ -611,4 +611,19 @@ mod tests { ); assert_eq!(m.match_and_split(&m), None); } + + #[test] + fn append_with_works() { + let mut m = X2(Parent, Parachain { id: 42 }); + assert_eq!(m.append_with(X1(AccountIndex64 { network: Any, index: 23 })), Ok(())); + assert_eq!(m, X3(Parent, Parachain { id: 42 }, AccountIndex64 { network: Any, index: 23 })); + } + + #[test] + fn prepend_with_works() { + let mut m = X3(Parent, Parachain { id: 42 }, AccountIndex64 { network: Any, index: 23 }); + // prepend drops any redundant parents + assert_eq!(m.prepend_with(X2(Parent, OnlyChild)), Ok(())); + assert_eq!(m, X3(Parent, Parachain { id: 42 }, AccountIndex64 { network: Any, index: 23 })); + } } From 1b3bcc5ec3cc426e41303fe81269ea1bf3496a85 Mon Sep 17 00:00:00 2001 From: Alexander Popiak Date: Wed, 28 Apr 2021 10:27:50 +0100 Subject: [PATCH 06/35] move Parachain enum variant to tuple --- xcm/src/v0/multi_location.rs | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/xcm/src/v0/multi_location.rs b/xcm/src/v0/multi_location.rs index fb353570052b..6fa3560e0565 100644 --- a/xcm/src/v0/multi_location.rs +++ b/xcm/src/v0/multi_location.rs @@ -603,10 +603,10 @@ mod tests { #[test] fn match_and_split_works() { - let m = X3(Parent, Parachain { id: 42 }, AccountIndex64 { network: Any, index: 23 }); + let m = X3(Parent, Parachain(42), AccountIndex64 { network: Any, index: 23 }); assert_eq!(m.match_and_split(&X1(Parent)), None); assert_eq!( - m.match_and_split(&X2(Parent, Parachain { id: 42 })), + m.match_and_split(&X2(Parent, Parachain(42))), Some(&AccountIndex64 { network: Any, index: 23 }) ); assert_eq!(m.match_and_split(&m), None); @@ -614,16 +614,16 @@ mod tests { #[test] fn append_with_works() { - let mut m = X2(Parent, Parachain { id: 42 }); + let mut m = X2(Parent, Parachain(42)); assert_eq!(m.append_with(X1(AccountIndex64 { network: Any, index: 23 })), Ok(())); - assert_eq!(m, X3(Parent, Parachain { id: 42 }, AccountIndex64 { network: Any, index: 23 })); + assert_eq!(m, X3(Parent, Parachain(42), AccountIndex64 { network: Any, index: 23 })); } #[test] fn prepend_with_works() { - let mut m = X3(Parent, Parachain { id: 42 }, AccountIndex64 { network: Any, index: 23 }); + let mut m = X3(Parent, Parachain(42), AccountIndex64 { network: Any, index: 23 }); // prepend drops any redundant parents assert_eq!(m.prepend_with(X2(Parent, OnlyChild)), Ok(())); - assert_eq!(m, X3(Parent, Parachain { id: 42 }, AccountIndex64 { network: Any, index: 23 })); + assert_eq!(m, X3(Parent, Parachain(42), AccountIndex64 { network: Any, index: 23 })); } } From 5b59ee6a02525db4050f3c96173dfd0276c14991 Mon Sep 17 00:00:00 2001 From: kianenigma Date: Wed, 28 Apr 2021 11:51:18 +0200 Subject: [PATCH 07/35] Fix stuff --- xcm/src/v0/multi_asset.rs | 89 +++++++++++++++++------ xcm/xcm-executor/src/traits/conversion.rs | 8 +- 2 files changed, 69 insertions(+), 28 deletions(-) diff --git a/xcm/src/v0/multi_asset.rs b/xcm/src/v0/multi_asset.rs index 8fa326b2a292..be6f203af2ec 100644 --- a/xcm/src/v0/multi_asset.rs +++ b/xcm/src/v0/multi_asset.rs @@ -254,27 +254,8 @@ impl MultiAsset { /// Returns true if `self` is a super-set of the given `inner`. /// - /// # Examples - /// - /// ```rust - /// # use xcm::v0::MultiAsset; - # fn main() { - // trivial case: all contains anything - assert!(MultiAsset::All.contains(MultiAsset::None)); - assert!(MultiAsset::All.contains(MultiAsset::AllFungible)); - - // trivial case: none contains nothing - assert!(!MultiAsset::None.contains(MultiAsset::None)); - assert!(!MultiAsset::None.contains(MultiAsset::AllFungible)); - - // A bit more sneaky: Nothing can contains All, even All. - // TODO: I think this is inconsistent, there should be an exception for this case. - assert!(!MultiAsset::All.contains(MultiAsset::All)); - // but AllFungible and AllNonFungible contain themselves. - assert!(MultiAsset::AllFungible.contains(MultiAsset::AllFungible)); - assert!(MultiAsset::AllNonFungible.contains(MultiAsset::AllNonFungible)); - # } - /// ``` + /// Typically, any wildcard is never contained in anything else, and a wildcard can contain any other non-wildcard. + /// For more details, see the implementation and tests. pub fn contains(&self, inner: &MultiAsset) -> bool { use MultiAsset::*; @@ -299,15 +280,18 @@ impl MultiAsset { AllConcreteNonFungible { class } => inner.is_concrete_non_fungible(class), AllAbstractNonFungible { class } => inner.is_abstract_non_fungible(class), + // TODO: should it not be `if i == id && amount >= a`? for self to contain `inner`, self should contain + // larger quantities than inner. ConcreteFungible { id, amount } => matches!(inner, ConcreteFungible { id: i , amount: a } if i == id && a >= amount), AbstractFungible { id, amount } => matches!(inner, AbstractFungible { id: i , amount: a } if i == id && a >= amount), ConcreteNonFungible { class, instance } - => matches!(inner, ConcreteNonFungible { class: i , instance: a } if i == class && a == instance), + => matches!(inner, ConcreteNonFungible { class: c , instance: i } if c == class && i == instance), AbstractNonFungible { class, instance } - => matches!(inner, AbstractNonFungible { class: i , instance: a } if i == class && a == instance), - + => matches!(inner, AbstractNonFungible { class: c , instance: i } if c == class && i == instance), + // ConcreteNonFungible { .. } => self == inner, + // AbstractNonFungible { .. } => self == inner, _ => false, } } @@ -339,3 +323,60 @@ impl TryFrom for MultiAsset { } } } + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn contains_works() { + use alloc::vec; + use MultiAsset::*; + // trivial case: all contains any non-wildcard. + assert!(All.contains(&None)); + assert!(All.contains(&AbstractFungible { id: alloc::vec![99u8], amount: 1 })); + + // trivial case: none contains nothing, except itself. + assert!(None.contains(&None)); + assert!(!None.contains(&AllFungible)); + assert!(!None.contains(&All)); + + // A bit more sneaky: Nothing can contain wildcard, even All ir the thing itself. + assert!(!All.contains(&All)); + assert!(!All.contains(&AllFungible)); + assert!(!AllFungible.contains(&AllFungible)); + assert!(!AllNonFungible.contains(&AllNonFungible)); + + // For fungibles, containing is basically equality, or equal with higher amount. + assert!( + !AbstractFungible { id: vec![99u8], amount: 9 } + .contains(&AbstractFungible { id: vec![98u8], amount: 99 }) + ); + assert!( + AbstractFungible { id: vec![99u8], amount: 9 } + .contains(&AbstractFungible { id: vec![99u8], amount: 99 }) + ); + assert!( + AbstractFungible { id: vec![99u8], amount: 9 } + .contains(&AbstractFungible { id: vec![99u8], amount: 9 }) + ); + assert!( + !AbstractFungible { id: vec![99u8], amount: 9 } + .contains(&AbstractFungible { id: vec![99u8], amount: 1 }) + ); + + // For non-fungibles, containing is equality. + assert!( + !AbstractNonFungible {class: vec![99u8], instance: AssetInstance::Index { id: 9 } } + .contains(&AbstractNonFungible { class: vec![98u8], instance: AssetInstance::Index { id: 9 } }) + ); + assert!( + !AbstractNonFungible { class: vec![99u8], instance: AssetInstance::Index { id: 8 } } + .contains(&AbstractNonFungible { class: vec![99u8], instance: AssetInstance::Index { id: 9 } }) + ); + assert!( + AbstractNonFungible { class: vec![99u8], instance: AssetInstance::Index { id: 9 } } + .contains(&AbstractNonFungible { class: vec![99u8], instance: AssetInstance::Index { id: 9 } }) + ); + } +} diff --git a/xcm/xcm-executor/src/traits/conversion.rs b/xcm/xcm-executor/src/traits/conversion.rs index 44cb27136692..4dceea7ae379 100644 --- a/xcm/xcm-executor/src/traits/conversion.rs +++ b/xcm/xcm-executor/src/traits/conversion.rs @@ -132,8 +132,8 @@ impl Convert, T> for Decoded { /// impl ConvertOrigin for BumpParaId { /// fn convert_origin(origin: MultiLocation, _: OriginKind) -> Result { /// match origin { -/// MultiLocation::X1(Junction::Parachain { id }) => { -/// Err(MultiLocation::X1(Junction::Parachain { id: id + 1 })) +/// MultiLocation::X1(Junction::Parachain(id)) => { +/// Err(MultiLocation::X1(Junction::Parachain(id + 1))) /// } /// _ => unreachable!() /// } @@ -144,7 +144,7 @@ impl Convert, T> for Decoded { /// impl ConvertOrigin for AcceptPara7 { /// fn convert_origin(origin: MultiLocation, _: OriginKind) -> Result { /// match origin { -/// MultiLocation::X1(Junction::Parachain { id }) if id == 7 => { +/// MultiLocation::X1(Junction::Parachain(id)) if id == 7 => { /// Ok(7) /// } /// _ => Err(origin) @@ -152,7 +152,7 @@ impl Convert, T> for Decoded { /// } /// } /// # fn main() { -/// let origin = MultiLocation::X1(Junction::Parachain { id: 6 }); +/// let origin = MultiLocation::X1(Junction::Parachain(6)); /// assert!( /// <(BumpParaId, AcceptPara7) as ConvertOrigin>::convert_origin(origin, OriginKind::Native) /// .is_ok() From 928c20dfb1dd9c7046bc54ecdbcccb7fc446b271 Mon Sep 17 00:00:00 2001 From: Alexander Popiak Date: Wed, 28 Apr 2021 10:59:05 +0100 Subject: [PATCH 08/35] add to append test --- xcm/src/v0/multi_location.rs | 16 +++++++++++----- 1 file changed, 11 insertions(+), 5 deletions(-) diff --git a/xcm/src/v0/multi_location.rs b/xcm/src/v0/multi_location.rs index 6fa3560e0565..47e52d28fd19 100644 --- a/xcm/src/v0/multi_location.rs +++ b/xcm/src/v0/multi_location.rs @@ -545,8 +545,8 @@ impl MultiLocation { } } - /// Mutate `self` so that it is prefixed with `prefix`. The correct normalised form is returned, removing any - /// internal `Parent`s. + /// Mutate `self` so that it is prefixed with `prefix`. The correct normalised form is returned, + /// removing any internal `Parent`s. /// /// Does not modify `self` and returns `Err` with `prefix` in case of overflow. pub fn prepend_with(&mut self, prefix: MultiLocation) -> Result<(), MultiLocation> { @@ -614,15 +614,21 @@ mod tests { #[test] fn append_with_works() { + let acc = AccountIndex64 { network: Any, index: 23 }; let mut m = X2(Parent, Parachain(42)); - assert_eq!(m.append_with(X1(AccountIndex64 { network: Any, index: 23 })), Ok(())); - assert_eq!(m, X3(Parent, Parachain(42), AccountIndex64 { network: Any, index: 23 })); + assert_eq!(m.append_with(X2(PalletInstance(3), acc.clone())), Ok(())); + assert_eq!(m, X4(Parent, Parachain(42), PalletInstance(3), acc.clone())); + + // cannot append to create overly long multilocation + let acc = AccountIndex64 { network: Any, index: 23 }; + let mut m = X7(Parent, Parent, Parent, Parent, Parent, Parent, Parachain(42)); + let suffix = X2(PalletInstance(3), acc.clone()); + assert_eq!(m.append_with(suffix.clone()), Err(suffix)); } #[test] fn prepend_with_works() { let mut m = X3(Parent, Parachain(42), AccountIndex64 { network: Any, index: 23 }); - // prepend drops any redundant parents assert_eq!(m.prepend_with(X2(Parent, OnlyChild)), Ok(())); assert_eq!(m, X3(Parent, Parachain(42), AccountIndex64 { network: Any, index: 23 })); } From 1c3b6441d1254ec1252091d44a411892a1da35e2 Mon Sep 17 00:00:00 2001 From: kianenigma Date: Wed, 28 Apr 2021 12:59:36 +0200 Subject: [PATCH 09/35] simplify match --- xcm/src/v0/multi_asset.rs | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-) diff --git a/xcm/src/v0/multi_asset.rs b/xcm/src/v0/multi_asset.rs index be6f203af2ec..ba76b647be2f 100644 --- a/xcm/src/v0/multi_asset.rs +++ b/xcm/src/v0/multi_asset.rs @@ -286,12 +286,8 @@ impl MultiAsset { => matches!(inner, ConcreteFungible { id: i , amount: a } if i == id && a >= amount), AbstractFungible { id, amount } => matches!(inner, AbstractFungible { id: i , amount: a } if i == id && a >= amount), - ConcreteNonFungible { class, instance } - => matches!(inner, ConcreteNonFungible { class: c , instance: i } if c == class && i == instance), - AbstractNonFungible { class, instance } - => matches!(inner, AbstractNonFungible { class: c , instance: i } if c == class && i == instance), - // ConcreteNonFungible { .. } => self == inner, - // AbstractNonFungible { .. } => self == inner, + ConcreteNonFungible { .. } => self == inner, + AbstractNonFungible { .. } => self == inner, _ => false, } } From f74a298ff46f6f7c2615ccbbab4a683201ee0aff Mon Sep 17 00:00:00 2001 From: Alexander Popiak Date: Wed, 28 Apr 2021 12:05:33 +0100 Subject: [PATCH 10/35] formatting --- xcm/src/v0/junction.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/xcm/src/v0/junction.rs b/xcm/src/v0/junction.rs index 2f3af556b462..e4f4dd77ef26 100644 --- a/xcm/src/v0/junction.rs +++ b/xcm/src/v0/junction.rs @@ -138,8 +138,8 @@ pub enum Junction { } impl Junction { - /// Returns true if this junction can be considered an interior part of its context. This is generally `true`, - /// except for the `Parent` item. + /// Returns true if this junction can be considered an interior part of its context. + /// This is generally `true`, except for the `Parent` item. pub fn is_interior(&self) -> bool { match self { Junction::Parent => false, From c951986e9f6f8cfdc47b4b73fa4ecde89b3e5df1 Mon Sep 17 00:00:00 2001 From: Alexander Popiak Date: Wed, 28 Apr 2021 12:06:15 +0100 Subject: [PATCH 11/35] format and extend doc comments (including examples) --- xcm/src/v0/multi_location.rs | 79 ++++++++++++++++++++++++++++-------- 1 file changed, 63 insertions(+), 16 deletions(-) diff --git a/xcm/src/v0/multi_location.rs b/xcm/src/v0/multi_location.rs index 47e52d28fd19..52e4205e5c03 100644 --- a/xcm/src/v0/multi_location.rs +++ b/xcm/src/v0/multi_location.rs @@ -238,8 +238,8 @@ impl MultiLocation { } } - /// Splits off the first junction, returning the remaining suffix (first item in tuple) and the first element - /// (second item in tuple) or `None` if it was empty. + /// Splits off the first junction, returning the remaining suffix (first item in tuple) and the + /// first element (second item in tuple) or `None` if it was empty. pub fn split_first(self) -> (MultiLocation, Option) { match self { MultiLocation::Null => (MultiLocation::Null, None), @@ -254,8 +254,8 @@ impl MultiLocation { } } - /// Splits off the last junction, returning the remaining prefix (first item in tuple) and the last element - /// (second item in tuple) or `None` if it was empty. + /// Splits off the last junction, returning the remaining prefix (first item in tuple) and the + /// last element (second item in tuple) or `None` if it was empty. pub fn split_last(self) -> (MultiLocation, Option) { match self { MultiLocation::Null => (MultiLocation::Null, None), @@ -288,8 +288,8 @@ impl MultiLocation { tail } - /// Consumes `self` and returns a `MultiLocation` suffixed with `new`, or an `Err` with the original value of - /// `self` in case of overflow. + /// Consumes `self` and returns a `MultiLocation` suffixed with `new`, or an `Err` with the + /// original value of `self` in case of overflow. pub fn pushed_with(self, new: Junction) -> result::Result { Ok(match self { MultiLocation::Null => MultiLocation::X1(new), @@ -304,8 +304,8 @@ impl MultiLocation { }) } - /// Consumes `self` and returns a `MultiLocation` prefixed with `new`, or an `Err` with the original value of - /// `self` in case of overflow. + /// Consumes `self` and returns a `MultiLocation` prefixed with `new`, or an `Err` with the + /// original value of `self` in case of overflow. pub fn pushed_front_with(self, new: Junction) -> result::Result { Ok(match self { MultiLocation::Null => MultiLocation::X1(new), @@ -442,8 +442,18 @@ impl MultiLocation { MultiLocationReverseIterator(self) } - /// Ensures that self begins with `prefix` and that it has a single `Junction` item following. If - /// so, returns a reference to this `Junction` item. + /// Ensures that self begins with `prefix` and that it has a single `Junction` item following. + /// If so, returns a reference to this `Junction` item. + /// + /// # Example + /// ```rust + /// # use xcm::v0::{MultiLocation::*, Junction::*}; + /// # fn main() { + /// let mut m = X3(Parent, PalletInstance(3), OnlyChild); + /// assert_eq!(m.match_and_split(&X2(Parent, PalletInstance(3))), Some(&OnlyChild)); + /// assert_eq!(m.match_and_split(&X1(Parent)), None); + /// # } + /// ``` pub fn match_and_split(&self, prefix: &MultiLocation) -> Option<&Junction> { if prefix.len() + 1 != self.len() { return None @@ -528,10 +538,20 @@ impl MultiLocation { } } - /// Mutate `self` so that it is suffixed with `suffix`. The correct normalised form is returned, removing any - /// internal `Parent`s. + /// Mutate `self` so that it is suffixed with `suffix`. The correct normalised form is returned, + /// removing any internal [Non-Parent, `Parent`] combinations. /// /// Does not modify `self` and returns `Err` with `suffix` in case of overflow. + /// + /// # Example + /// ```rust + /// # use xcm::v0::{MultiLocation::*, Junction::*}; + /// # fn main() { + /// let mut m = X3(Parent, Parachain(21), OnlyChild); + /// assert_eq!(m.append_with(X2(Parent, PalletInstance(3))), Ok(())); + /// assert_eq!(m, X3(Parent, Parachain(21), PalletInstance(3))); + /// # } + /// ``` pub fn append_with(&mut self, suffix: MultiLocation) -> Result<(), MultiLocation> { let mut prefix = suffix; core::mem::swap(self, &mut prefix); @@ -546,9 +566,19 @@ impl MultiLocation { } /// Mutate `self` so that it is prefixed with `prefix`. The correct normalised form is returned, - /// removing any internal `Parent`s. + /// removing any internal [Non-Parent, `Parent`] combinations. /// /// Does not modify `self` and returns `Err` with `prefix` in case of overflow. + /// + /// # Example + /// ```rust + /// # use xcm::v0::{MultiLocation::*, Junction::*, NetworkId::Any}; + /// # fn main() { + /// let mut m = X3(Parent, Parent, PalletInstance(3)); + /// assert_eq!(m.prepend_with(X3(Parent, Parachain(21), OnlyChild)), Ok(())); + /// assert_eq!(m, X2(Parent, PalletInstance(3))); + /// # } + /// ``` pub fn prepend_with(&mut self, prefix: MultiLocation) -> Result<(), MultiLocation> { let self_parents = self.parent_count(); let prefix_rest = prefix.len() - prefix.parent_count(); @@ -559,7 +589,7 @@ impl MultiLocation { let mut prefix = prefix; while match (prefix.last(), self.first()) { - (Some(x), Some(Junction::Parent)) if x != &Junction::Parent => { + (Some(x), Some(Junction::Parent)) if x.is_interior() => { prefix.take_last(); self.take_first(); true @@ -573,8 +603,20 @@ impl MultiLocation { Ok(()) } - /// Returns true iff `self` is an interior location. For this it may not contain any `Junction`s for which - /// `Junction::is_interior` returns `false`. This + /// Returns true iff `self` is an interior location. For this it may not contain any `Junction`s + /// for which `Junction::is_interior` returns `false`. This is generally true, except for the + /// `Parent` item. + /// + /// # Example + /// ```rust + /// # use xcm::v0::{MultiLocation::*, Junction::*, NetworkId::Any}; + /// # fn main() { + /// let parent = X1(Parent); + /// assert_eq!(parent.is_interior(), false); + /// let m = X2(PalletInstance(12), AccountIndex64 { network: Any, index: 23 }); + /// assert_eq!(m.is_interior(), true); + /// # } + /// ``` pub fn is_interior(&self) -> bool { self.iter().all(Junction::is_interior) } @@ -631,5 +673,10 @@ mod tests { let mut m = X3(Parent, Parachain(42), AccountIndex64 { network: Any, index: 23 }); assert_eq!(m.prepend_with(X2(Parent, OnlyChild)), Ok(())); assert_eq!(m, X3(Parent, Parachain(42), AccountIndex64 { network: Any, index: 23 })); + + // cannot prepend to create overly long multilocation + let mut m = X7(Parent, Parent, Parent, Parent, Parent, Parent, Parachain(42)); + let prefix = X2(Parent, Parent); + assert_eq!(m.prepend_with(prefix.clone()), Err(prefix)); } } From b128b22423b0edb613a4d081ae37bca9cff3c983 Mon Sep 17 00:00:00 2001 From: Alexander Popiak Date: Wed, 28 Apr 2021 13:15:39 +0200 Subject: [PATCH 12/35] fix typo --- xcm/src/v0/multi_asset.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/xcm/src/v0/multi_asset.rs b/xcm/src/v0/multi_asset.rs index ba76b647be2f..b8a4a1187abb 100644 --- a/xcm/src/v0/multi_asset.rs +++ b/xcm/src/v0/multi_asset.rs @@ -148,7 +148,7 @@ pub enum MultiAsset { impl MultiAsset { /// Returns `true` if the `MultiAsset` is a wildcard and can refer to classes of assets, instead of just one. /// - /// Typically can also be inferred by the name staring with `All`. + /// Typically can also be inferred by the name starting with `All`. pub fn is_wildcard(&self) -> bool { match self { MultiAsset::None From 270095214e4c2e9e04d485c009cc48766a5eb0a3 Mon Sep 17 00:00:00 2001 From: Alexander Popiak Date: Wed, 28 Apr 2021 12:33:16 +0100 Subject: [PATCH 13/35] add some doc comments --- xcm/xcm-builder/src/location_conversion.rs | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/xcm/xcm-builder/src/location_conversion.rs b/xcm/xcm-builder/src/location_conversion.rs index 44b25e1358bc..4ef268158174 100644 --- a/xcm/xcm-builder/src/location_conversion.rs +++ b/xcm/xcm-builder/src/location_conversion.rs @@ -23,7 +23,6 @@ use xcm::v0::{MultiLocation, NetworkId, Junction}; use xcm_executor::traits::{InvertLocation, Convert}; pub struct Account32Hash(PhantomData<(Network, AccountId)>); - impl< Network: Get, AccountId: From<[u8; 32]> + Into<[u8; 32]> + Clone, @@ -37,6 +36,8 @@ impl< } } +/// A [`MultiLocation`] consisting of a single `Parent` [`Junction`] will be converted to the +/// default value of `AccountId` (e.g. all zeros for `AccountId32`). pub struct ParentIsDefault(PhantomData); impl< AccountId: Default + Eq + Clone, @@ -81,7 +82,6 @@ impl< } pub struct SiblingParachainConvertsVia(PhantomData<(ParaId, AccountId)>); - impl< ParaId: From + Into + AccountIdConversion, AccountId: Clone, @@ -103,6 +103,7 @@ impl< } } +/// Extracts the `AccountId32` from the passed `location` if the network matches. pub struct AccountId32Aliases(PhantomData<(Network, AccountId)>); impl< Network: Get, From a0c2c99d1ff1b8c88f7dceb5fc71f535ff401b2b Mon Sep 17 00:00:00 2001 From: Alexander Popiak Date: Wed, 28 Apr 2021 16:05:50 +0100 Subject: [PATCH 14/35] add test for location inverter --- xcm/xcm-builder/src/location_conversion.rs | 73 ++++++++++++++++++++++ xcm/xcm-executor/src/traits/conversion.rs | 4 +- 2 files changed, 75 insertions(+), 2 deletions(-) diff --git a/xcm/xcm-builder/src/location_conversion.rs b/xcm/xcm-builder/src/location_conversion.rs index 4ef268158174..a15e4636fc7f 100644 --- a/xcm/xcm-builder/src/location_conversion.rs +++ b/xcm/xcm-builder/src/location_conversion.rs @@ -144,6 +144,38 @@ impl< } /// Simple location inverter; give it this location's ancestry and it'll figure out the inverted location. +/// +/// # Example +/// ## Network Topology +/// ```txt +/// v Source +/// Relay -> Para 1 -> Account20 +/// -> Para 2 -> Account32 +/// ^ Target +/// ``` +/// ```rust +/// # use frame_support::parameter_types; +/// # use xcm::v0::{MultiLocation::{self, *}, Junction::*, NetworkId::Any}; +/// # use xcm_builder::LocationInverter; +/// # use xcm_executor::traits::InvertLocation; +/// # fn main() { +/// parameter_types!{ +/// pub Ancestry: MultiLocation = X2( +/// Parachain(1), +/// AccountKey20 { network: Any, key: Default::default()}, +/// ); +/// } +/// +/// let input = X4(Parent, Parent, Parachain(2), AccountId32 { network: Any, id: Default::default() }); +/// let inverted = LocationInverter::::invert_location(&input); +/// assert_eq!(inverted, X4( +/// Parent, +/// Parent, +/// Parachain(1), +/// AccountKey20 { network: Any, key: Default::default() }, +/// )); +/// # } +/// ``` pub struct LocationInverter(PhantomData); impl> InvertLocation for LocationInverter { fn invert_location(location: &MultiLocation) -> MultiLocation { @@ -161,3 +193,44 @@ impl> InvertLocation for LocationInverter result } } + +#[cfg(test)] +mod tests { + use super::*; + + use frame_support::parameter_types; + use xcm::v0::{MultiLocation::*, Junction::*, NetworkId::Any}; + + // Network Topology + // v Source + // Relay -> Para 1 -> SmartContract -> Account + // -> Para 2 -> Account + // ^ Target + // + // Inputs and outputs written as file paths: + // + // input location (source to target): ../../../para_2/account32_default + // ancestry (root to source): para_1/account20_default/account20_default + // => + // output (target to source): ../../para_1/account20_default/account20_default + #[test] + fn location_inverter() { + parameter_types!{ + pub Ancestry: MultiLocation = X3( + Parachain(1), + AccountKey20 { network: Any, key: Default::default() }, + AccountKey20 { network: Any, key: Default::default() }, + ); + } + + let input = X5(Parent, Parent, Parent, Parachain(2), AccountId32 { network: Any, id: Default::default() }); + let inverted = LocationInverter::::invert_location(&input); + assert_eq!(inverted, X5( + Parent, + Parent, + Parachain(1), + AccountKey20 { network: Any, key: Default::default() }, + AccountKey20 { network: Any, key: Default::default() }, + )); + } +} diff --git a/xcm/xcm-executor/src/traits/conversion.rs b/xcm/xcm-executor/src/traits/conversion.rs index 4dceea7ae379..2c549be235c3 100644 --- a/xcm/xcm-executor/src/traits/conversion.rs +++ b/xcm/xcm-executor/src/traits/conversion.rs @@ -177,8 +177,8 @@ impl ConvertOrigin for Tuple { } } -/// Means of inverting a location: given a location which describes a `target` interpreted from the `source`, this -/// will provide the corresponding location which describes the `source` +/// Means of inverting a location: given a location which describes a `target` interpreted from the +/// `source`, this will provide the corresponding location which describes the `source`. pub trait InvertLocation { fn invert_location(l: &MultiLocation) -> MultiLocation; } From 169fdbab7b57d8fed9423c6d9ad2f7d386bfefb1 Mon Sep 17 00:00:00 2001 From: kianenigma Date: Thu, 29 Apr 2021 09:03:48 +0200 Subject: [PATCH 15/35] Add more tests/docs --- xcm/xcm-builder/src/barriers.rs | 8 +++++++ xcm/xcm-builder/src/filter_asset_location.rs | 4 ++++ xcm/xcm-builder/src/location_conversion.rs | 2 -- xcm/xcm-builder/src/mock.rs | 21 ++++++++++++------- xcm/xcm-builder/src/origin_conversion.rs | 5 +++-- .../src/traits/filter_asset_location.rs | 3 +++ xcm/xcm-executor/src/traits/on_response.rs | 3 +++ xcm/xcm-executor/src/traits/should_execute.rs | 4 ++-- xcm/xcm-executor/src/traits/transact_asset.rs | 8 ++++--- 9 files changed, 41 insertions(+), 17 deletions(-) diff --git a/xcm/xcm-builder/src/barriers.rs b/xcm/xcm-builder/src/barriers.rs index 770104ec6092..d9bb857a3cf4 100644 --- a/xcm/xcm-builder/src/barriers.rs +++ b/xcm/xcm-builder/src/barriers.rs @@ -14,11 +14,14 @@ // You should have received a copy of the GNU General Public License // along with Polkadot. If not, see . +//! Various implementations for `ShouldExecute`. + use sp_std::{result::Result, marker::PhantomData}; use xcm::v0::{Xcm, Order, MultiLocation}; use frame_support::{ensure, traits::Contains, weights::Weight}; use xcm_executor::traits::{OnResponse, ShouldExecute}; +/// Execution barrier that just takes `shallow_weight` from `weight_credit`. pub struct TakeWeightCredit; impl ShouldExecute for TakeWeightCredit { fn should_execute( @@ -33,6 +36,8 @@ impl ShouldExecute for TakeWeightCredit { } } +/// Allows execution from `origin` is origin is contained in `T` (i.e. `T::Contains(origin)`) taking payments into +/// account. pub struct AllowTopLevelPaidExecutionFrom(PhantomData); impl> ShouldExecute for AllowTopLevelPaidExecutionFrom { fn should_execute( @@ -58,6 +63,8 @@ impl> ShouldExecute for AllowTopLevelPaidExecutionFro } } +/// Allows execution from any origin that is contained in `T` (i.e. `T::Contains(origin)`) without any payments. use +/// only for executions from trusted origin groups. pub struct AllowUnpaidExecutionFrom(PhantomData); impl> ShouldExecute for AllowUnpaidExecutionFrom { fn should_execute( @@ -72,6 +79,7 @@ impl> ShouldExecute for AllowUnpaidExecutionFrom { } } +/// Allows only messages if the generic `ResponseHandler` expects them via `expecting_response`. pub struct AllowKnownQueryResponses(PhantomData); impl ShouldExecute for AllowKnownQueryResponses { fn should_execute( diff --git a/xcm/xcm-builder/src/filter_asset_location.rs b/xcm/xcm-builder/src/filter_asset_location.rs index edf67b0296dd..4ee9eccbe2b7 100644 --- a/xcm/xcm-builder/src/filter_asset_location.rs +++ b/xcm/xcm-builder/src/filter_asset_location.rs @@ -14,11 +14,14 @@ // You should have received a copy of the GNU General Public License // along with Polkadot. If not, see . +//! various implementations of `FilterAssetLocation`. + use sp_std::marker::PhantomData; use xcm::v0::{MultiAsset, MultiLocation}; use frame_support::traits::Get; use xcm_executor::traits::FilterAssetLocation; +/// Accepts an asset IFF it is a native asset. pub struct NativeAsset; impl FilterAssetLocation for NativeAsset { fn filter_asset_location(asset: &MultiAsset, origin: &MultiLocation) -> bool { @@ -26,6 +29,7 @@ impl FilterAssetLocation for NativeAsset { } } +/// Accepts an asset if it is contained in the given `T`'s `Get` impl. pub struct Case(PhantomData); impl> FilterAssetLocation for Case { fn filter_asset_location(asset: &MultiAsset, origin: &MultiLocation) -> bool { diff --git a/xcm/xcm-builder/src/location_conversion.rs b/xcm/xcm-builder/src/location_conversion.rs index 44b25e1358bc..1d31c8a90c24 100644 --- a/xcm/xcm-builder/src/location_conversion.rs +++ b/xcm/xcm-builder/src/location_conversion.rs @@ -23,7 +23,6 @@ use xcm::v0::{MultiLocation, NetworkId, Junction}; use xcm_executor::traits::{InvertLocation, Convert}; pub struct Account32Hash(PhantomData<(Network, AccountId)>); - impl< Network: Get, AccountId: From<[u8; 32]> + Into<[u8; 32]> + Clone, @@ -81,7 +80,6 @@ impl< } pub struct SiblingParachainConvertsVia(PhantomData<(ParaId, AccountId)>); - impl< ParaId: From + Into + AccountIdConversion, AccountId: Clone, diff --git a/xcm/xcm-builder/src/mock.rs b/xcm/xcm-builder/src/mock.rs index bc815cf14ef6..56d7d753e49e 100644 --- a/xcm/xcm-builder/src/mock.rs +++ b/xcm/xcm-builder/src/mock.rs @@ -36,8 +36,17 @@ pub use crate::{ FixedRateOfConcreteFungible, AllowKnownQueryResponses, LocationInverter, }; -pub enum TestOrigin { Root, Relay, Signed(u64), Parachain(u32) } +pub enum TestOrigin { + Root, + Relay, + Signed(u64), + Parachain(u32), +} +/// A dummy call. +/// +/// Each item contains the amount of weight that it *wants* to consume as the first item, and the actual amount (if +/// different from the former) in the second option. #[derive(Debug, Encode, Decode, Eq, PartialEq, Clone, Copy)] pub enum TestCall { OnlyRoot(Weight, Option), @@ -60,17 +69,13 @@ impl Dispatchable for TestCall { => maybe_actual, }; if match (&origin, &self) { - (TestOrigin::Parachain(i), TestCall::OnlyParachain(_, _, Some(j))) - => i == j, - (TestOrigin::Signed(i), TestCall::OnlySigned(_, _, Some(j))) - => i == j, - + (TestOrigin::Parachain(i), TestCall::OnlyParachain(_, _, Some(j))) => i == j, + (TestOrigin::Signed(i), TestCall::OnlySigned(_, _, Some(j))) => i == j, (TestOrigin::Root, TestCall::OnlyRoot(..)) | (TestOrigin::Parachain(_), TestCall::OnlyParachain(_, _, None)) | (TestOrigin::Signed(_), TestCall::OnlySigned(_, _, None)) | (_, TestCall::Any(..)) => true, - _ => false, } { Ok(post_info) @@ -151,7 +156,7 @@ pub fn to_account(l: MultiLocation) -> Result { X1(Parachain(id)) => 1000 + id as u64, // Self at 3000 Null => 3000, - // Parent at 3000 + // Parent at 3001 X1(Parent) => 3001, l => return Err(l), }) diff --git a/xcm/xcm-builder/src/origin_conversion.rs b/xcm/xcm-builder/src/origin_conversion.rs index 134dfc30821f..daa51f3ee8e8 100644 --- a/xcm/xcm-builder/src/origin_conversion.rs +++ b/xcm/xcm-builder/src/origin_conversion.rs @@ -14,6 +14,8 @@ // You should have received a copy of the GNU General Public License // along with Polkadot. If not, see . +//! Various implementations for `ConvertOrigin`. + use sp_std::{marker::PhantomData, convert::TryInto}; use xcm::v0::{MultiLocation, OriginKind, NetworkId, Junction, BodyId, BodyPart}; use xcm_executor::traits::{Convert, ConvertOrigin}; @@ -21,8 +23,7 @@ use frame_support::traits::{EnsureOrigin, Get, OriginTrait, GetBacking}; use frame_system::RawOrigin as SystemRawOrigin; use polkadot_parachain::primitives::IsSystem; -/// Sovereign accounts use the system's `Signed` origin with an account ID derived from the -/// `LocationConverter`. +/// Sovereign accounts use the system's `Signed` origin with an account ID derived from the `LocationConverter`. pub struct SovereignSignedViaLocation( PhantomData<(LocationConverter, Origin)> ); diff --git a/xcm/xcm-executor/src/traits/filter_asset_location.rs b/xcm/xcm-executor/src/traits/filter_asset_location.rs index 084c5c1a0331..c68fcd6ff79a 100644 --- a/xcm/xcm-executor/src/traits/filter_asset_location.rs +++ b/xcm/xcm-executor/src/traits/filter_asset_location.rs @@ -16,6 +16,9 @@ use xcm::v0::{MultiAsset, MultiLocation}; +/// Filters assets/location pairs. +/// +/// Can be amalgamated into tuples. If any item returns `true`, it short-circuits, else `false` is returned. pub trait FilterAssetLocation { /// A filter to distinguish between asset/location pairs. fn filter_asset_location(asset: &MultiAsset, origin: &MultiLocation) -> bool; diff --git a/xcm/xcm-executor/src/traits/on_response.rs b/xcm/xcm-executor/src/traits/on_response.rs index f74c8bdd8c75..a90586d27c77 100644 --- a/xcm/xcm-executor/src/traits/on_response.rs +++ b/xcm/xcm-executor/src/traits/on_response.rs @@ -17,8 +17,11 @@ use xcm::v0::{Response, MultiLocation}; use frame_support::weights::Weight; +/// Define what needs to be done upon receiving a query response. pub trait OnResponse { + /// Returns `true` if we are expecting a response from `origin` for query `query_id`. fn expecting_response(origin: &MultiLocation, query_id: u64) -> bool; + /// Handler for receiving a `response` from `origin` relating to `query_id`. fn on_response(origin: MultiLocation, query_id: u64, response: Response) -> Weight; } impl OnResponse for () { diff --git a/xcm/xcm-executor/src/traits/should_execute.rs b/xcm/xcm-executor/src/traits/should_execute.rs index 3c35ec404397..872ffc2924d6 100644 --- a/xcm/xcm-executor/src/traits/should_execute.rs +++ b/xcm/xcm-executor/src/traits/should_execute.rs @@ -26,7 +26,7 @@ pub trait ShouldExecute { /// Returns `true` if the given `message` may be executed. /// /// - `origin`: The origin (sender) of the message. - /// - `top_level`: `true`` indicates the initial XCM coming from the `origin`, `false` indicates an embedded XCM + /// - `top_level`: `true` indicates the initial XCM coming from the `origin`, `false` indicates an embedded XCM /// executed internally as part of another message or an `Order`. /// - `message`: The message itself. /// - `shallow_weight`: The weight of the non-negotiable execution of the message. This does not include any @@ -54,7 +54,7 @@ impl ShouldExecute for Tuple { ) -> Result<(), ()> { for_tuples!( #( match Tuple::should_execute(origin, top_level, message, shallow_weight, weight_credit) { - o @ Ok(()) => return o, + Ok(()) => return Ok(()) _ => (), } )* ); diff --git a/xcm/xcm-executor/src/traits/transact_asset.rs b/xcm/xcm-executor/src/traits/transact_asset.rs index 7e699425e3ee..9e52f461b470 100644 --- a/xcm/xcm-executor/src/traits/transact_asset.rs +++ b/xcm/xcm-executor/src/traits/transact_asset.rs @@ -20,9 +20,11 @@ use crate::Assets; /// Facility for asset transacting. /// -/// This should work with as many asset/location combinations as possible. Locations to support may include non- -/// account locations such as a `MultiLocation::X1(Junction::Parachain)`. Different chains may handle them in -/// different ways. +/// This should work with as many asset/location combinations as possible. Locations to support may include non- account +/// locations such as a `MultiLocation::X1(Junction::Parachain)`. Different chains may handle them in different ways. +/// +/// can be amalgamated as a tuple of items that implement this trait. In such executions, if any of the transactors +/// returns `Ok(())`, then it will short circuit. Else, execution is passed to the next transactor. pub trait TransactAsset { /// Deposit the `what` asset into the account of `who`. /// From 058648bfdd8d7c2045799e5358b3a72481625964 Mon Sep 17 00:00:00 2001 From: kianenigma Date: Thu, 29 Apr 2021 09:41:50 +0200 Subject: [PATCH 16/35] Fix build --- xcm/xcm-builder/src/currency_adapter.rs | 12 ++++------ xcm/xcm-builder/src/fungibles_adapter.rs | 21 +++++++--------- xcm/xcm-builder/src/matches_fungible.rs | 24 +++++++++++++++++++ xcm/xcm-executor/src/traits/should_execute.rs | 2 +- 4 files changed, 39 insertions(+), 20 deletions(-) diff --git a/xcm/xcm-builder/src/currency_adapter.rs b/xcm/xcm-builder/src/currency_adapter.rs index 4b08398a2dfb..b50d1cdeccad 100644 --- a/xcm/xcm-builder/src/currency_adapter.rs +++ b/xcm/xcm-builder/src/currency_adapter.rs @@ -33,12 +33,11 @@ enum Error { impl From for XcmError { fn from(e: Error) -> Self { + use XcmError::FailedToTransactAsset; match e { - Error::AssetNotFound => XcmError::FailedToTransactAsset("AssetNotFound"), - Error::AccountIdConversionFailed => - XcmError::FailedToTransactAsset("AccountIdConversionFailed"), - Error::AmountToBalanceConversionFailed => - XcmError::FailedToTransactAsset("AmountToBalanceConversionFailed"), + Error::AssetNotFound => FailedToTransactAsset("AssetNotFound"), + Error::AccountIdConversionFailed => FailedToTransactAsset("AccountIdConversionFailed"), + Error::AmountToBalanceConversionFailed => FailedToTransactAsset("AmountToBalanceConversionFailed"), } } } @@ -51,9 +50,8 @@ impl< Matcher: MatchesFungible, AccountIdConverter: Convert, Currency: frame_support::traits::Currency, - AccountId: Clone, // can't get away without it since Currency is generic over it. + AccountId: Clone, // can't get away without it since Currency is generic over it. > TransactAsset for CurrencyAdapter { - fn deposit_asset(what: &MultiAsset, who: &MultiLocation) -> Result { // Check we handle this asset. let amount: u128 = Matcher::matches_fungible(&what) diff --git a/xcm/xcm-builder/src/fungibles_adapter.rs b/xcm/xcm-builder/src/fungibles_adapter.rs index adb865ea7086..e8a912846898 100644 --- a/xcm/xcm-builder/src/fungibles_adapter.rs +++ b/xcm/xcm-builder/src/fungibles_adapter.rs @@ -33,21 +33,19 @@ pub enum Error { impl From for XcmError { fn from(e: Error) -> Self { + use XcmError::FailedToTransactAsset; match e { - Error::AssetNotFound => XcmError::FailedToTransactAsset("AssetNotFound"), - Error::AccountIdConversionFailed => - XcmError::FailedToTransactAsset("AccountIdConversionFailed"), - Error::AmountToBalanceConversionFailed => - XcmError::FailedToTransactAsset("AmountToBalanceConversionFailed"), - Error::AssetIdConversionFailed => - XcmError::FailedToTransactAsset("AssetIdConversionFailed"), + Error::AssetNotFound => FailedToTransactAsset("AssetNotFound"), + Error::AccountIdConversionFailed => FailedToTransactAsset("AccountIdConversionFailed"), + Error::AmountToBalanceConversionFailed => FailedToTransactAsset("AmountToBalanceConversionFailed"), + Error::AssetIdConversionFailed => FailedToTransactAsset("AssetIdConversionFailed"), } } } -/// Converter struct implementing `AssetIdConversion` converting a numeric asset ID (must be TryFrom/TryInto) -/// into a `GeneralIndex` junction, prefixed by some `MultiLocation` value. The `MultiLocation` value will -/// typically be a `PalletInstance` junction. +/// Converter struct implementing `AssetIdConversion` converting a numeric asset ID (must be TryFrom/TryInto) into +/// a `GeneralIndex` junction, prefixed by some `MultiLocation` value. The `MultiLocation` value will typically be a +/// `PalletInstance` junction. pub struct AsPrefixedGeneralIndex(PhantomData<(Prefix, AssetId, ConvertAssetId)>); impl< Prefix: Get, @@ -167,7 +165,7 @@ impl< Assets: fungibles::Mutate, Matcher: MatchesFungibles, AccountIdConverter: Convert, - AccountId: Clone, // can't get away without it since Currency is generic over it. + AccountId: Clone, // can't get away without it since Currency is generic over it. > TransactAsset for FungiblesMutateAdapter { fn deposit_asset(what: &MultiAsset, who: &MultiLocation) -> Result { @@ -202,7 +200,6 @@ impl< AccountIdConverter: Convert, AccountId: Clone, // can't get away without it since Currency is generic over it. > TransactAsset for FungiblesAdapter { - fn deposit_asset(what: &MultiAsset, who: &MultiLocation) -> Result { FungiblesMutateAdapter::::deposit_asset(what, who) } diff --git a/xcm/xcm-builder/src/matches_fungible.rs b/xcm/xcm-builder/src/matches_fungible.rs index 3053bbcdb9e5..1c6ccad38e82 100644 --- a/xcm/xcm-builder/src/matches_fungible.rs +++ b/xcm/xcm-builder/src/matches_fungible.rs @@ -14,12 +14,35 @@ // You should have received a copy of the GNU General Public License // along with Polkadot. If not, see . +//! Various implementations for the `MatchesFungible` trait. + use sp_std::{marker::PhantomData, convert::TryFrom}; use sp_runtime::traits::CheckedConversion; use xcm::v0::{MultiAsset, MultiLocation}; use frame_support::traits::Get; use xcm_executor::traits::MatchesFungible; +/// Converts a `MultiAsset` into balance `B` if it is a concrete fungible with an id equal to that +/// of given by `T`'s `Get`. +/// +/// # Example +/// +/// ``` +/// use xcm::v0::{MultiAsset, MultiLocation, Junction}; +/// use xcm_builder::IsConcrete; +/// use xcm_executor::traits::MatchesFungible; +/// +/// frame_support::parameter_types! { +/// pub TargetLocation: MultiLocation = MultiLocation::X1(Junction::Parent); +/// } +/// +/// # fn main() { +/// let id = MultiLocation::X1(Junction::Parent); +/// let asset = MultiAsset::ConcreteFungible { id, amount: 999u128 }; +/// // match `asset` if it is a concrete asset in `TargetLocation`. +/// assert_eq!( as MatchesFungible>::matches_fungible(&asset), Some(999)); +/// # } +/// ``` pub struct IsConcrete(PhantomData); impl, B: TryFrom> MatchesFungible for IsConcrete { fn matches_fungible(a: &MultiAsset) -> Option { @@ -30,6 +53,7 @@ impl, B: TryFrom> MatchesFungible for IsConcrete< } } } + pub struct IsAbstract(PhantomData); impl, B: TryFrom> MatchesFungible for IsAbstract { fn matches_fungible(a: &MultiAsset) -> Option { diff --git a/xcm/xcm-executor/src/traits/should_execute.rs b/xcm/xcm-executor/src/traits/should_execute.rs index 872ffc2924d6..c42e5156d93a 100644 --- a/xcm/xcm-executor/src/traits/should_execute.rs +++ b/xcm/xcm-executor/src/traits/should_execute.rs @@ -54,7 +54,7 @@ impl ShouldExecute for Tuple { ) -> Result<(), ()> { for_tuples!( #( match Tuple::should_execute(origin, top_level, message, shallow_weight, weight_credit) { - Ok(()) => return Ok(()) + Ok(()) => return Ok(()), _ => (), } )* ); From d1918b69134b4e9c2e934a0ecf8c45c88d573252 Mon Sep 17 00:00:00 2001 From: kianenigma Date: Thu, 29 Apr 2021 09:49:36 +0200 Subject: [PATCH 17/35] matches fungibles --- xcm/xcm-builder/src/matches_fungible.rs | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+) diff --git a/xcm/xcm-builder/src/matches_fungible.rs b/xcm/xcm-builder/src/matches_fungible.rs index 1c6ccad38e82..fd6820655a6a 100644 --- a/xcm/xcm-builder/src/matches_fungible.rs +++ b/xcm/xcm-builder/src/matches_fungible.rs @@ -54,6 +54,25 @@ impl, B: TryFrom> MatchesFungible for IsConcrete< } } +/// Same as [`IsConcrete`] but for a fungible with abstract location. +/// +/// # Example +/// +/// ``` +/// use xcm::v0::{MultiAsset}; +/// use xcm_builder::IsAbstract; +/// use xcm_executor::traits::MatchesFungible; +/// +/// frame_support::parameter_types! { +/// pub TargetLocation: &'static [u8] = &[7u8]; +/// } +/// +/// # fn main() { +/// let asset = MultiAsset::AbstractFungible { id: vec![7u8], amount: 999u128 }; +/// // match `asset` if it is a concrete asset in `TargetLocation`. +/// assert_eq!( as MatchesFungible>::matches_fungible(&asset), Some(999)); +/// # } +/// ``` pub struct IsAbstract(PhantomData); impl, B: TryFrom> MatchesFungible for IsAbstract { fn matches_fungible(a: &MultiAsset) -> Option { From df78d3887beabef979b1fab4963274cbe937d4f0 Mon Sep 17 00:00:00 2001 From: kianenigma Date: Thu, 29 Apr 2021 10:45:21 +0200 Subject: [PATCH 18/35] currency adapter. --- xcm/xcm-builder/src/currency_adapter.rs | 33 +++++++++++++++++++++++++ 1 file changed, 33 insertions(+) diff --git a/xcm/xcm-builder/src/currency_adapter.rs b/xcm/xcm-builder/src/currency_adapter.rs index b50d1cdeccad..d954ceeb162a 100644 --- a/xcm/xcm-builder/src/currency_adapter.rs +++ b/xcm/xcm-builder/src/currency_adapter.rs @@ -42,6 +42,39 @@ impl From for XcmError { } } +/// Simple adapter to use a currency as asset transactor. This type can be used as `type AssetTransactor` in +/// `xcm::Config`. +/// +/// # Example +/// ``` +/// use frame_support::parameter_types; +/// use xcm::v0::{MultiLocation, Junction}; +/// use xcm_builder::{ParentIsDefault, CurrencyAdapter, IsConcrete}; +/// +/// /// Our chain's account id. We need to name it explicitly: +/// type AccountId = sp_runtime::AccountId32; +/// +/// /// Our relay chain's location. +/// parameter_types! { +/// RelayChain: MultiLocation = MultiLocation::X1(Junction::Parent); +/// } +/// +/// /// Some items that implement `Convert`. Can be more, but for now we just assume we accept +/// /// messages from the parent (relay chain). +/// pub type LocationConvertor = (ParentIsDefault); +/// +/// /// Final current adapter. This can be used in `xcm::Config` to specify how asset related transactions happen. +/// pub type AssetTransactor = CurrencyAdapter< +/// // Use this currency: +/// u128, +/// // This is your matcher: use the currency when the asset is a concrete asset in our relay chain. +/// IsConcrete, +/// // Do a simple punn to convert an AccountId32 MultiLocation into a native chain account ID: +/// LocationConvertor, +/// // Our chain's account ID type (we can't get away without mentioning it explicitly): +/// AccountId, +/// >; +/// ``` pub struct CurrencyAdapter( PhantomData<(Currency, Matcher, AccountIdConverter, AccountId)> ); From c5198c93d700baf25b11578ea6579d548cbfee01 Mon Sep 17 00:00:00 2001 From: Alexander Popiak Date: Thu, 29 Apr 2021 10:17:25 +0100 Subject: [PATCH 19/35] add more tests for location inverter --- xcm/xcm-builder/src/location_conversion.rs | 75 +++++++++++++++------- 1 file changed, 52 insertions(+), 23 deletions(-) diff --git a/xcm/xcm-builder/src/location_conversion.rs b/xcm/xcm-builder/src/location_conversion.rs index a15e4636fc7f..cdf0a2bf5171 100644 --- a/xcm/xcm-builder/src/location_conversion.rs +++ b/xcm/xcm-builder/src/location_conversion.rs @@ -143,7 +143,8 @@ impl< } } -/// Simple location inverter; give it this location's ancestry and it'll figure out the inverted location. +/// Simple location inverter; give it this location's ancestry and it'll figure out the inverted +/// location. /// /// # Example /// ## Network Topology @@ -160,19 +161,19 @@ impl< /// # use xcm_executor::traits::InvertLocation; /// # fn main() { /// parameter_types!{ -/// pub Ancestry: MultiLocation = X2( -/// Parachain(1), -/// AccountKey20 { network: Any, key: Default::default()}, -/// ); +/// pub Ancestry: MultiLocation = X2( +/// Parachain(1), +/// AccountKey20 { network: Any, key: Default::default() }, +/// ); /// } /// /// let input = X4(Parent, Parent, Parachain(2), AccountId32 { network: Any, id: Default::default() }); /// let inverted = LocationInverter::::invert_location(&input); /// assert_eq!(inverted, X4( -/// Parent, -/// Parent, -/// Parachain(1), -/// AccountKey20 { network: Any, key: Default::default() }, +/// Parent, +/// Parent, +/// Parachain(1), +/// AccountKey20 { network: Any, key: Default::default() }, /// )); /// # } /// ``` @@ -201,6 +202,14 @@ mod tests { use frame_support::parameter_types; use xcm::v0::{MultiLocation::*, Junction::*, NetworkId::Any}; + fn account20() -> Junction { + AccountKey20 { network: Any, key: Default::default() } + } + + fn account32() -> Junction { + AccountId32 { network: Any, id: Default::default() } + } + // Network Topology // v Source // Relay -> Para 1 -> SmartContract -> Account @@ -214,23 +223,43 @@ mod tests { // => // output (target to source): ../../para_1/account20_default/account20_default #[test] - fn location_inverter() { + fn inverter_works_in_tree() { + parameter_types!{ + pub Ancestry: MultiLocation = X3(Parachain(1), account20(), account20()); + } + + let input = X5(Parent, Parent, Parent, Parachain(2), account32()); + let inverted = LocationInverter::::invert_location(&input); + assert_eq!(inverted, X5(Parent, Parent, Parachain(1), account20(), account20())); + } + + // Network Topology + // v Source + // Relay -> Para 1 -> SmartContract -> Account + // ^ Target + #[test] + fn inverter_uses_ancestry_as_inverted_location() { + parameter_types!{ + pub Ancestry: MultiLocation = X2(account20(), account20()); + } + + let input = X2(Parent, Parent); + let inverted = LocationInverter::::invert_location(&input); + assert_eq!(inverted, X2(account20(), account20())); + } + + // Network Topology + // v Source + // Relay -> Para 1 -> CollectivePallet -> Plurality + // ^ Target + #[test] + fn inverter_uses_only_child_on_missing_ancestry() { parameter_types!{ - pub Ancestry: MultiLocation = X3( - Parachain(1), - AccountKey20 { network: Any, key: Default::default() }, - AccountKey20 { network: Any, key: Default::default() }, - ); + pub Ancestry: MultiLocation = X1(PalletInstance(5)); } - let input = X5(Parent, Parent, Parent, Parachain(2), AccountId32 { network: Any, id: Default::default() }); + let input = X2(Parent, Parent); let inverted = LocationInverter::::invert_location(&input); - assert_eq!(inverted, X5( - Parent, - Parent, - Parachain(1), - AccountKey20 { network: Any, key: Default::default() }, - AccountKey20 { network: Any, key: Default::default() }, - )); + assert_eq!(inverted, X2(PalletInstance(5), OnlyChild)); } } From 4645f31a9cbe0ef54a5dbc5a74f9f432b336d6aa Mon Sep 17 00:00:00 2001 From: Alexander Popiak Date: Thu, 29 Apr 2021 11:38:58 +0100 Subject: [PATCH 20/35] extract max length magic number into constant --- xcm/src/v0/multi_location.rs | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/xcm/src/v0/multi_location.rs b/xcm/src/v0/multi_location.rs index 52e4205e5c03..1e28feac13fb 100644 --- a/xcm/src/v0/multi_location.rs +++ b/xcm/src/v0/multi_location.rs @@ -66,6 +66,8 @@ pub enum MultiLocation { X8(Junction, Junction, Junction, Junction, Junction, Junction, Junction, Junction), } +pub const MAX_MULTILOCATION_LENGTH: usize = 8; + impl From for MultiLocation { fn from(x: Junction) -> Self { MultiLocation::X1(x) @@ -583,7 +585,7 @@ impl MultiLocation { let self_parents = self.parent_count(); let prefix_rest = prefix.len() - prefix.parent_count(); let skipped = self_parents.min(prefix_rest); - if self.len() + prefix.len() - 2 * skipped > 4 { + if self.len() + prefix.len() - 2 * skipped > MAX_MULTILOCATION_LENGTH { return Err(prefix); } @@ -598,7 +600,7 @@ impl MultiLocation { } {} for j in prefix.into_iter_rev() { - self.push_front(j).expect("len + prefix minus 2*skipped is less than 4; qed"); + self.push_front(j).expect("len + prefix minus 2*skipped is less than max length; qed"); } Ok(()) } From 7790e5d4ea94d824a46c2315d3233262596136a0 Mon Sep 17 00:00:00 2001 From: kianenigma Date: Thu, 29 Apr 2021 13:02:10 +0200 Subject: [PATCH 21/35] adapters. --- xcm/xcm-builder/src/currency_adapter.rs | 2 ++ xcm/xcm-builder/src/fungibles_adapter.rs | 5 ++++- 2 files changed, 6 insertions(+), 1 deletion(-) diff --git a/xcm/xcm-builder/src/currency_adapter.rs b/xcm/xcm-builder/src/currency_adapter.rs index d954ceeb162a..8e873245a279 100644 --- a/xcm/xcm-builder/src/currency_adapter.rs +++ b/xcm/xcm-builder/src/currency_adapter.rs @@ -14,6 +14,8 @@ // You should have received a copy of the GNU General Public License // along with Polkadot. If not, see . +//! Adapters to work with `frame_support::traits::Currency` through XCM. + use sp_std::{result, convert::TryInto, marker::PhantomData}; use xcm::v0::{Error as XcmError, Result, MultiAsset, MultiLocation}; use sp_runtime::traits::SaturatedConversion; diff --git a/xcm/xcm-builder/src/fungibles_adapter.rs b/xcm/xcm-builder/src/fungibles_adapter.rs index e8a912846898..cd7db2391d30 100644 --- a/xcm/xcm-builder/src/fungibles_adapter.rs +++ b/xcm/xcm-builder/src/fungibles_adapter.rs @@ -14,6 +14,8 @@ // You should have received a copy of the GNU General Public License // along with Polkadot. If not, see . +//! Adapters to work with `frame_support::traits::tokens::fungibles` through XCM. + use sp_std::{prelude::*, result, marker::PhantomData, borrow::Borrow}; use xcm::v0::{Error as XcmError, Result, MultiAsset, MultiLocation, Junction}; use frame_support::traits::{Get, tokens::fungibles}; @@ -71,6 +73,7 @@ impl< } } +// TODO: Comparing with `MatchesFungible`, this seems quite misplaced. pub trait MatchesFungibles { fn matches_fungibles(a: &MultiAsset) -> result::Result<(AssetId, Balance), Error>; } @@ -198,7 +201,7 @@ impl< Assets: fungibles::Mutate + fungibles::Transfer, Matcher: MatchesFungibles, AccountIdConverter: Convert, - AccountId: Clone, // can't get away without it since Currency is generic over it. + AccountId: Clone, // can't get away without it since Currency is generic over it. > TransactAsset for FungiblesAdapter { fn deposit_asset(what: &MultiAsset, who: &MultiLocation) -> Result { FungiblesMutateAdapter::::deposit_asset(what, who) From b98a29cfd6a3a21f3aa4a849cb13c82ac3a42170 Mon Sep 17 00:00:00 2001 From: Kian Paimani <5588131+kianenigma@users.noreply.github.com> Date: Thu, 29 Apr 2021 13:14:26 +0200 Subject: [PATCH 22/35] Apply suggestions from code review --- xcm/src/v0/multi_asset.rs | 1 - xcm/src/v0/multi_location.rs | 1 - xcm/xcm-executor/src/lib.rs | 3 --- 3 files changed, 5 deletions(-) diff --git a/xcm/src/v0/multi_asset.rs b/xcm/src/v0/multi_asset.rs index b8a4a1187abb..7bf7a18dcea3 100644 --- a/xcm/src/v0/multi_asset.rs +++ b/xcm/src/v0/multi_asset.rs @@ -180,7 +180,6 @@ impl MultiAsset { } } - // TODO: `All` is both fungible and non-fungible, and I think it should be none. fn is_fungible(&self) -> bool { match self { MultiAsset::All diff --git a/xcm/src/v0/multi_location.rs b/xcm/src/v0/multi_location.rs index 1e28feac13fb..e7a6aac7e7a2 100644 --- a/xcm/src/v0/multi_location.rs +++ b/xcm/src/v0/multi_location.rs @@ -642,7 +642,6 @@ impl TryFrom for MultiLocation { #[cfg(test)] mod tests { use super::MultiLocation::*; - use crate::opaque::v0::{Junction::*, NetworkId::Any}; #[test] diff --git a/xcm/xcm-executor/src/lib.rs b/xcm/xcm-executor/src/lib.rs index f6721826e141..4691970697a4 100644 --- a/xcm/xcm-executor/src/lib.rs +++ b/xcm/xcm-executor/src/lib.rs @@ -54,16 +54,13 @@ impl ExecuteXcm for XcmExecutor { Ok(x) => x, Err(()) => return Outcome::Error(XcmError::WeightNotComputable), }; - let maximum_weight = match shallow_weight.checked_add(deep_weight) { Some(x) => x, None => return Outcome::Error(XcmError::Overflow), }; - if maximum_weight > weight_limit { return Outcome::Error(XcmError::WeightLimitReached(maximum_weight)); } - let mut trader = Config::Trader::new(); let result = Self::do_execute_xcm(origin, true, message, &mut 0, Some(shallow_weight), &mut trader); drop(trader); From 192c2f52db92fb3ea1932825d7386d065257ccba Mon Sep 17 00:00:00 2001 From: kianenigma Date: Fri, 7 May 2021 09:56:47 +0200 Subject: [PATCH 23/35] Final touches. --- parachain/src/primitives.rs | 23 +++++++++++++---------- xcm/src/v0/junction.rs | 4 ++-- xcm/src/v0/multi_location.rs | 16 ++++++++-------- xcm/xcm-builder/src/currency_adapter.rs | 20 ++++++++++---------- xcm/xcm-builder/src/fungibles_adapter.rs | 3 ++- xcm/xcm-builder/src/lib.rs | 4 +++- 6 files changed, 38 insertions(+), 32 deletions(-) diff --git a/parachain/src/primitives.rs b/parachain/src/primitives.rs index 7e3b7e51de59..55d0c12e932c 100644 --- a/parachain/src/primitives.rs +++ b/parachain/src/primitives.rs @@ -100,18 +100,21 @@ impl From for Id { } } -// When we added a second From impl for Id, type inference could no longer determine which impl -// should apply for things like `5.into()`. It therefore raised a bunch of errors in our test code, -// scattered throughout the various modules' tests, that there is no impl of `From` (`i32` -// being the default numeric type). +// When we added a second From impl for Id, type inference could no longer +// determine which impl should apply for things like `5.into()`. It therefore +// raised a bunch of errors in our test code, scattered throughout the +// various modules' tests, that there is no impl of `From` (`i32` being +// the default numeric type). // -// We can't use `cfg(test)` here, because that configuration directive does not propagate between -// crates, which would fail to fix tests in crates other than this one. +// We can't use `cfg(test)` here, because that configuration directive does not +// propagate between crates, which would fail to fix tests in crates other than +// this one. // -// Instead, let's take advantage of the observation that what really matters for a ParaId within a -// test context is that it is unique and constant. I believe that there is no case where someone -// does `(-1).into()` anyway, but if they do, it never matters whether the actual contained ID is -// `-1` or `4294967295`. Nobody does arithmetic on a `ParaId`; doing so would be a bug. +// Instead, let's take advantage of the observation that what really matters for a +// ParaId within a test context is that it is unique and constant. I believe that +// there is no case where someone does `(-1).into()` anyway, but if they do, it +// never matters whether the actual contained ID is `-1` or `4294967295`. Nobody +// does arithmetic on a `ParaId`; doing so would be a bug. impl From for Id { fn from(x: i32) -> Self { Id(x as u32) diff --git a/xcm/src/v0/junction.rs b/xcm/src/v0/junction.rs index e4f4dd77ef26..2f3af556b462 100644 --- a/xcm/src/v0/junction.rs +++ b/xcm/src/v0/junction.rs @@ -138,8 +138,8 @@ pub enum Junction { } impl Junction { - /// Returns true if this junction can be considered an interior part of its context. - /// This is generally `true`, except for the `Parent` item. + /// Returns true if this junction can be considered an interior part of its context. This is generally `true`, + /// except for the `Parent` item. pub fn is_interior(&self) -> bool { match self { Junction::Parent => false, diff --git a/xcm/src/v0/multi_location.rs b/xcm/src/v0/multi_location.rs index e7a6aac7e7a2..991f8d6a4921 100644 --- a/xcm/src/v0/multi_location.rs +++ b/xcm/src/v0/multi_location.rs @@ -240,8 +240,8 @@ impl MultiLocation { } } - /// Splits off the first junction, returning the remaining suffix (first item in tuple) and the - /// first element (second item in tuple) or `None` if it was empty. + /// Splits off the first junction, returning the remaining suffix (first item in tuple) and the first element + /// (second item in tuple) or `None` if it was empty. pub fn split_first(self) -> (MultiLocation, Option) { match self { MultiLocation::Null => (MultiLocation::Null, None), @@ -256,8 +256,8 @@ impl MultiLocation { } } - /// Splits off the last junction, returning the remaining prefix (first item in tuple) and the - /// last element (second item in tuple) or `None` if it was empty. + /// Splits off the last junction, returning the remaining prefix (first item in tuple) and the last element + /// (second item in tuple) or `None` if it was empty. pub fn split_last(self) -> (MultiLocation, Option) { match self { MultiLocation::Null => (MultiLocation::Null, None), @@ -290,8 +290,8 @@ impl MultiLocation { tail } - /// Consumes `self` and returns a `MultiLocation` suffixed with `new`, or an `Err` with the - /// original value of `self` in case of overflow. + /// Consumes `self` and returns a `MultiLocation` suffixed with `new`, or an `Err` with the original value of + /// `self` in case of overflow. pub fn pushed_with(self, new: Junction) -> result::Result { Ok(match self { MultiLocation::Null => MultiLocation::X1(new), @@ -306,8 +306,8 @@ impl MultiLocation { }) } - /// Consumes `self` and returns a `MultiLocation` prefixed with `new`, or an `Err` with the - /// original value of `self` in case of overflow. + /// Consumes `self` and returns a `MultiLocation` prefixed with `new`, or an `Err` with the original value of + /// `self` in case of overflow. pub fn pushed_front_with(self, new: Junction) -> result::Result { Ok(match self { MultiLocation::Null => MultiLocation::X1(new), diff --git a/xcm/xcm-builder/src/currency_adapter.rs b/xcm/xcm-builder/src/currency_adapter.rs index 8e873245a279..29ae18c1ec4e 100644 --- a/xcm/xcm-builder/src/currency_adapter.rs +++ b/xcm/xcm-builder/src/currency_adapter.rs @@ -53,12 +53,12 @@ impl From for XcmError { /// use xcm::v0::{MultiLocation, Junction}; /// use xcm_builder::{ParentIsDefault, CurrencyAdapter, IsConcrete}; /// -/// /// Our chain's account id. We need to name it explicitly: +/// /// Our chain's account id. /// type AccountId = sp_runtime::AccountId32; /// /// /// Our relay chain's location. /// parameter_types! { -/// RelayChain: MultiLocation = MultiLocation::X1(Junction::Parent); +/// RelayChain: MultiLocation = MultiLocation::X1(Junction::Parent); /// } /// /// /// Some items that implement `Convert`. Can be more, but for now we just assume we accept @@ -67,14 +67,14 @@ impl From for XcmError { /// /// /// Final current adapter. This can be used in `xcm::Config` to specify how asset related transactions happen. /// pub type AssetTransactor = CurrencyAdapter< -/// // Use this currency: -/// u128, -/// // This is your matcher: use the currency when the asset is a concrete asset in our relay chain. -/// IsConcrete, -/// // Do a simple punn to convert an AccountId32 MultiLocation into a native chain account ID: -/// LocationConvertor, -/// // Our chain's account ID type (we can't get away without mentioning it explicitly): -/// AccountId, +/// // Use this currency: +/// u128, +/// // The matcher: use the currency when the asset is a concrete asset in our relay chain. +/// IsConcrete, +/// // The local convertor: default account of the parent relay chain. +/// LocationConvertor, +/// // Our chain's account ID type. +/// AccountId, /// >; /// ``` pub struct CurrencyAdapter( diff --git a/xcm/xcm-builder/src/fungibles_adapter.rs b/xcm/xcm-builder/src/fungibles_adapter.rs index cd7db2391d30..0ff5c47f4c86 100644 --- a/xcm/xcm-builder/src/fungibles_adapter.rs +++ b/xcm/xcm-builder/src/fungibles_adapter.rs @@ -73,7 +73,8 @@ impl< } } -// TODO: Comparing with `MatchesFungible`, this seems quite misplaced. +// TODO: Comparing with `MatchesFungible`, this seems quite misplaced. It should be in xcm-executor +// traits and implemented here. pub trait MatchesFungibles { fn matches_fungibles(a: &MultiAsset) -> result::Result<(AssetId, Balance), Error>; } diff --git a/xcm/xcm-builder/src/lib.rs b/xcm/xcm-builder/src/lib.rs index f5f2b5ff6a2c..0bc908c5930b 100644 --- a/xcm/xcm-builder/src/lib.rs +++ b/xcm/xcm-builder/src/lib.rs @@ -14,7 +14,9 @@ // You should have received a copy of the GNU General Public License // along with Polkadot. If not, see . -//! Types and helpers for creating XCM configuration. +//! # XCM-Builder +//! +//! Types and helpers for *building* XCM configuration. #![cfg_attr(not(feature = "std"), no_std)] From ac47999853c558947ff7b3b77eef19a394b196ee Mon Sep 17 00:00:00 2001 From: kianenigma Date: Sun, 9 May 2021 16:28:50 +0200 Subject: [PATCH 24/35] Repot and fixes --- xcm/src/v0/multi_asset.rs | 28 ++++---- xcm/xcm-builder/src/fungibles_adapter.rs | 69 ++++--------------- .../src/traits/matches_fungibles.rs | 59 ++++++++++++++++ xcm/xcm-executor/src/traits/mod.rs | 2 + xcm/xcm-executor/src/traits/transact_asset.rs | 2 +- 5 files changed, 90 insertions(+), 70 deletions(-) create mode 100644 xcm/xcm-executor/src/traits/matches_fungibles.rs diff --git a/xcm/src/v0/multi_asset.rs b/xcm/src/v0/multi_asset.rs index 7bf7a18dcea3..20032e7169a4 100644 --- a/xcm/src/v0/multi_asset.rs +++ b/xcm/src/v0/multi_asset.rs @@ -279,12 +279,14 @@ impl MultiAsset { AllConcreteNonFungible { class } => inner.is_concrete_non_fungible(class), AllAbstractNonFungible { class } => inner.is_abstract_non_fungible(class), - // TODO: should it not be `if i == id && amount >= a`? for self to contain `inner`, self should contain - // larger quantities than inner. - ConcreteFungible { id, amount } - => matches!(inner, ConcreteFungible { id: i , amount: a } if i == id && a >= amount), - AbstractFungible { id, amount } - => matches!(inner, AbstractFungible { id: i , amount: a } if i == id && a >= amount), + ConcreteFungible { id, amount } => matches!( + inner, + ConcreteFungible { id: inner_id , amount: inner_amount } if inner_id == id && amount >= inner_amount + ), + AbstractFungible { id, amount } => matches!( + inner, + AbstractFungible { id: inner_id , amount: inner_amount } if inner_id == id && amount >= inner_amount + ), ConcreteNonFungible { .. } => self == inner, AbstractNonFungible { .. } => self == inner, _ => false, @@ -342,22 +344,22 @@ mod tests { assert!(!AllFungible.contains(&AllFungible)); assert!(!AllNonFungible.contains(&AllNonFungible)); - // For fungibles, containing is basically equality, or equal with higher amount. + // For fungibles, containing is basically equality, or equal id with higher amount. assert!( - !AbstractFungible { id: vec![99u8], amount: 9 } - .contains(&AbstractFungible { id: vec![98u8], amount: 99 }) + !AbstractFungible { id: vec![99u8], amount: 99 } + .contains(&AbstractFungible { id: vec![1u8], amount: 99 }) ); assert!( - AbstractFungible { id: vec![99u8], amount: 9 } + AbstractFungible { id: vec![99u8], amount: 99 } .contains(&AbstractFungible { id: vec![99u8], amount: 99 }) ); assert!( - AbstractFungible { id: vec![99u8], amount: 9 } + AbstractFungible { id: vec![99u8], amount: 99 } .contains(&AbstractFungible { id: vec![99u8], amount: 9 }) ); assert!( - !AbstractFungible { id: vec![99u8], amount: 9 } - .contains(&AbstractFungible { id: vec![99u8], amount: 1 }) + !AbstractFungible { id: vec![99u8], amount: 99 } + .contains(&AbstractFungible { id: vec![99u8], amount: 100 }) ); // For non-fungibles, containing is equality. diff --git a/xcm/xcm-builder/src/fungibles_adapter.rs b/xcm/xcm-builder/src/fungibles_adapter.rs index 0ff5c47f4c86..39c09cf9c43e 100644 --- a/xcm/xcm-builder/src/fungibles_adapter.rs +++ b/xcm/xcm-builder/src/fungibles_adapter.rs @@ -19,31 +19,7 @@ use sp_std::{prelude::*, result, marker::PhantomData, borrow::Borrow}; use xcm::v0::{Error as XcmError, Result, MultiAsset, MultiLocation, Junction}; use frame_support::traits::{Get, tokens::fungibles}; -use xcm_executor::traits::{TransactAsset, Convert}; - -/// Asset transaction errors. -pub enum Error { - /// Asset not found. - AssetNotFound, - /// `MultiLocation` to `AccountId` conversion failed. - AccountIdConversionFailed, - /// `u128` amount to currency `Balance` conversion failed. - AmountToBalanceConversionFailed, - /// `MultiLocation` to `AssetId` conversion failed. - AssetIdConversionFailed, -} - -impl From for XcmError { - fn from(e: Error) -> Self { - use XcmError::FailedToTransactAsset; - match e { - Error::AssetNotFound => FailedToTransactAsset("AssetNotFound"), - Error::AccountIdConversionFailed => FailedToTransactAsset("AccountIdConversionFailed"), - Error::AmountToBalanceConversionFailed => FailedToTransactAsset("AmountToBalanceConversionFailed"), - Error::AssetIdConversionFailed => FailedToTransactAsset("AssetIdConversionFailed"), - } - } -} +use xcm_executor::traits::{TransactAsset, Convert, MatchesFungibles, Error as MatchError}; /// Converter struct implementing `AssetIdConversion` converting a numeric asset ID (must be TryFrom/TryInto) into /// a `GeneralIndex` junction, prefixed by some `MultiLocation` value. The `MultiLocation` value will typically be a @@ -73,25 +49,6 @@ impl< } } -// TODO: Comparing with `MatchesFungible`, this seems quite misplaced. It should be in xcm-executor -// traits and implemented here. -pub trait MatchesFungibles { - fn matches_fungibles(a: &MultiAsset) -> result::Result<(AssetId, Balance), Error>; -} - -#[impl_trait_for_tuples::impl_for_tuples(30)] -impl< - AssetId: Clone, - Balance: Clone, -> MatchesFungibles for Tuple { - fn matches_fungibles(a: &MultiAsset) -> result::Result<(AssetId, Balance), Error> { - for_tuples!( #( - match Tuple::matches_fungibles(a) { o @ Ok(_) => return o, _ => () } - )* ); - Err(Error::AssetNotFound) - } -} - pub struct ConvertedConcreteAssetId( PhantomData<(AssetId, Balance, ConvertAssetId, ConvertBalance)> ); @@ -103,13 +60,13 @@ impl< > MatchesFungibles for ConvertedConcreteAssetId { - fn matches_fungibles(a: &MultiAsset) -> result::Result<(AssetId, Balance), Error> { + fn matches_fungibles(a: &MultiAsset) -> result::Result<(AssetId, Balance), MatchError> { let (id, amount) = match a { MultiAsset::ConcreteFungible { id, amount } => (id, amount), - _ => return Err(Error::AssetNotFound), + _ => return Err(MatchError::AssetNotFound), }; - let what = ConvertAssetId::convert_ref(id).map_err(|_| Error::AssetIdConversionFailed)?; - let amount = ConvertBalance::convert_ref(amount).map_err(|_| Error::AmountToBalanceConversionFailed)?; + let what = ConvertAssetId::convert_ref(id).map_err(|_| MatchError::AssetIdConversionFailed)?; + let amount = ConvertBalance::convert_ref(amount).map_err(|_| MatchError::AmountToBalanceConversionFailed)?; Ok((what, amount)) } } @@ -125,13 +82,13 @@ impl< > MatchesFungibles for ConvertedAbstractAssetId { - fn matches_fungibles(a: &MultiAsset) -> result::Result<(AssetId, Balance), Error> { + fn matches_fungibles(a: &MultiAsset) -> result::Result<(AssetId, Balance), MatchError> { let (id, amount) = match a { MultiAsset::AbstractFungible { id, amount } => (id, amount), - _ => return Err(Error::AssetNotFound), + _ => return Err(MatchError::AssetNotFound), }; - let what = ConvertAssetId::convert_ref(id).map_err(|_| Error::AssetIdConversionFailed)?; - let amount = ConvertBalance::convert_ref(amount).map_err(|_| Error::AmountToBalanceConversionFailed)?; + let what = ConvertAssetId::convert_ref(id).map_err(|_| MatchError::AssetIdConversionFailed)?; + let amount = ConvertBalance::convert_ref(amount).map_err(|_| MatchError::AmountToBalanceConversionFailed)?; Ok((what, amount)) } } @@ -153,9 +110,9 @@ impl< // Check we handle this asset. let (asset_id, amount) = Matcher::matches_fungibles(what)?; let source = AccountIdConverter::convert_ref(from) - .map_err(|()| Error::AccountIdConversionFailed)?; + .map_err(|()| MatchError::AccountIdConversionFailed)?; let dest = AccountIdConverter::convert_ref(to) - .map_err(|()| Error::AccountIdConversionFailed)?; + .map_err(|()| MatchError::AccountIdConversionFailed)?; Assets::transfer(asset_id, &source, &dest, amount, true) .map_err(|e| XcmError::FailedToTransactAsset(e.into()))?; Ok(what.clone().into()) @@ -176,7 +133,7 @@ impl< // Check we handle this asset. let (asset_id, amount) = Matcher::matches_fungibles(what)?; let who = AccountIdConverter::convert_ref(who) - .map_err(|()| Error::AccountIdConversionFailed)?; + .map_err(|()| MatchError::AccountIdConversionFailed)?; Assets::mint_into(asset_id, &who, amount) .map_err(|e| XcmError::FailedToTransactAsset(e.into())) } @@ -188,7 +145,7 @@ impl< // Check we handle this asset. let (asset_id, amount) = Matcher::matches_fungibles(what)?; let who = AccountIdConverter::convert_ref(who) - .map_err(|()| Error::AccountIdConversionFailed)?; + .map_err(|()| MatchError::AccountIdConversionFailed)?; Assets::burn_from(asset_id, &who, amount) .map_err(|e| XcmError::FailedToTransactAsset(e.into()))?; Ok(what.clone().into()) diff --git a/xcm/xcm-executor/src/traits/matches_fungibles.rs b/xcm/xcm-executor/src/traits/matches_fungibles.rs new file mode 100644 index 000000000000..75de0ae8be44 --- /dev/null +++ b/xcm/xcm-executor/src/traits/matches_fungibles.rs @@ -0,0 +1,59 @@ +// Copyright 2020 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 . + +use xcm::v0::{MultiAsset, Error as XcmError}; +use sp_std::result; + +/// Errors associated with [`MatchesFungibles`] operation. +pub enum Error { + /// Asset not found. + AssetNotFound, + /// `MultiLocation` to `AccountId` conversion failed. + AccountIdConversionFailed, + /// `u128` amount to currency `Balance` conversion failed. + AmountToBalanceConversionFailed, + /// `MultiLocation` to `AssetId` conversion failed. + AssetIdConversionFailed, +} + +impl From for XcmError { + fn from(e: Error) -> Self { + use XcmError::FailedToTransactAsset; + match e { + Error::AssetNotFound => FailedToTransactAsset("AssetNotFound"), + Error::AccountIdConversionFailed => FailedToTransactAsset("AccountIdConversionFailed"), + Error::AmountToBalanceConversionFailed => FailedToTransactAsset("AmountToBalanceConversionFailed"), + Error::AssetIdConversionFailed => FailedToTransactAsset("AssetIdConversionFailed"), + } + } +} + +pub trait MatchesFungibles { + fn matches_fungibles(a: &MultiAsset) -> result::Result<(AssetId, Balance), Error>; +} + +#[impl_trait_for_tuples::impl_for_tuples(30)] +impl< + AssetId: Clone, + Balance: Clone, +> MatchesFungibles for Tuple { + fn matches_fungibles(a: &MultiAsset) -> result::Result<(AssetId, Balance), Error> { + for_tuples!( #( + match Tuple::matches_fungibles(a) { o @ Ok(_) => return o, _ => () } + )* ); + Err(Error::AssetNotFound) + } +} diff --git a/xcm/xcm-executor/src/traits/mod.rs b/xcm/xcm-executor/src/traits/mod.rs index 163e4f0f79d1..483b8746d722 100644 --- a/xcm/xcm-executor/src/traits/mod.rs +++ b/xcm/xcm-executor/src/traits/mod.rs @@ -22,6 +22,8 @@ mod filter_asset_location; pub use filter_asset_location::{FilterAssetLocation}; mod matches_fungible; pub use matches_fungible::{MatchesFungible}; +mod matches_fungibles; +pub use matches_fungibles::{MatchesFungibles, Error}; mod on_response; pub use on_response::OnResponse; mod should_execute; diff --git a/xcm/xcm-executor/src/traits/transact_asset.rs b/xcm/xcm-executor/src/traits/transact_asset.rs index 9e52f461b470..b03f58ce9046 100644 --- a/xcm/xcm-executor/src/traits/transact_asset.rs +++ b/xcm/xcm-executor/src/traits/transact_asset.rs @@ -23,7 +23,7 @@ use crate::Assets; /// This should work with as many asset/location combinations as possible. Locations to support may include non- account /// locations such as a `MultiLocation::X1(Junction::Parachain)`. Different chains may handle them in different ways. /// -/// can be amalgamated as a tuple of items that implement this trait. In such executions, if any of the transactors +/// Can be amalgamated as a tuple of items that implement this trait. In such executions, if any of the transactors /// returns `Ok(())`, then it will short circuit. Else, execution is passed to the next transactor. pub trait TransactAsset { /// Deposit the `what` asset into the account of `who`. From 09f9118832a8a976e5e57036a301cb4a1ab5edfe Mon Sep 17 00:00:00 2001 From: kianenigma Date: Sun, 9 May 2021 16:29:53 +0200 Subject: [PATCH 25/35] Remove last todo --- xcm/src/v0/traits.rs | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/xcm/src/v0/traits.rs b/xcm/src/v0/traits.rs index 76487bf77f30..880efa2694e3 100644 --- a/xcm/src/v0/traits.rs +++ b/xcm/src/v0/traits.rs @@ -126,8 +126,6 @@ impl Outcome { } } -/// Something that can execute an xcm message. -// TODO: why is this both generic and associated? pub trait ExecuteXcm { type Call; fn execute_xcm(origin: MultiLocation, message: Xcm, weight_limit: Weight) -> Outcome; @@ -205,9 +203,9 @@ impl ExecuteXcm for () { pub trait SendXcm { /// Send an XCM `message` to a given `destination`. /// - /// If it is not a destination which can be reached with this type but possibly could by others, - /// then it *MUST* return `CannotReachDestination`. Any other error will cause the tuple - /// implementation to exit early without trying other type fields. + /// If it is not a destination which can be reached with this type but possibly could by others, then it *MUST* + /// return `CannotReachDestination`. Any other error will cause the tuple implementation to exit early without + /// trying other type fields. fn send_xcm(destination: MultiLocation, message: Xcm<()>) -> Result; } From 37921ed6ee4d87ece96e13d94591aa07f8292bea Mon Sep 17 00:00:00 2001 From: Kian Paimani <5588131+kianenigma@users.noreply.github.com> Date: Mon, 10 May 2021 11:09:24 +0200 Subject: [PATCH 26/35] Apply suggestions from code review Co-authored-by: Alexander Popiak --- parachain/src/primitives.rs | 2 +- xcm/src/v0/multi_location.rs | 1 + xcm/src/v0/traits.rs | 4 ++-- 3 files changed, 4 insertions(+), 3 deletions(-) diff --git a/parachain/src/primitives.rs b/parachain/src/primitives.rs index 3e40759cb38b..939103abf879 100644 --- a/parachain/src/primitives.rs +++ b/parachain/src/primitives.rs @@ -139,7 +139,7 @@ impl Id { /// Determine if a parachain is a system parachain or not. pub trait IsSystem { - /// Returns true if a parachain is a system parachain or not. + /// Returns `true` if a parachain is a system parachain, `false` otherwise. fn is_system(&self) -> bool; } diff --git a/xcm/src/v0/multi_location.rs b/xcm/src/v0/multi_location.rs index 991f8d6a4921..2cfb1fe1c541 100644 --- a/xcm/src/v0/multi_location.rs +++ b/xcm/src/v0/multi_location.rs @@ -66,6 +66,7 @@ pub enum MultiLocation { X8(Junction, Junction, Junction, Junction, Junction, Junction, Junction, Junction), } +/// Maximum number of junctions a multilocation can contain. pub const MAX_MULTILOCATION_LENGTH: usize = 8; impl From for MultiLocation { diff --git a/xcm/src/v0/traits.rs b/xcm/src/v0/traits.rs index 880efa2694e3..13226145b40f 100644 --- a/xcm/src/v0/traits.rs +++ b/xcm/src/v0/traits.rs @@ -170,7 +170,7 @@ impl ExecuteXcm for () { /// } /// } /// -/// /// A sender that accepts a message from a X1 parent junction, passing through otherwise. +/// /// A sender that accepts a message from an X1 parent junction, passing through otherwise. /// struct Sender3; /// impl SendXcm for Sender3 { /// fn send_xcm(destination: MultiLocation, message: Xcm<()>) -> Result { @@ -181,7 +181,7 @@ impl ExecuteXcm for () { /// } /// } /// -/// // an xcm message to send. We don't really care about this. +/// // A call to send via XCM. We don't really care about this. /// # fn main() { /// let call: Vec = ().encode(); /// let message = Xcm::Transact { origin_type: OriginKind::Superuser, require_weight_at_most: 0, call: call.into() }; From d12b36faa1811f4f8ed9086c65fd39c567f564b9 Mon Sep 17 00:00:00 2001 From: Kian Paimani <5588131+kianenigma@users.noreply.github.com> Date: Wed, 12 May 2021 11:34:30 +0200 Subject: [PATCH 27/35] Update xcm/xcm-builder/src/barriers.rs Co-authored-by: Alexander Popiak --- xcm/xcm-builder/src/barriers.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/xcm/xcm-builder/src/barriers.rs b/xcm/xcm-builder/src/barriers.rs index f9f1b3cf82be..281861d968b5 100644 --- a/xcm/xcm-builder/src/barriers.rs +++ b/xcm/xcm-builder/src/barriers.rs @@ -37,7 +37,7 @@ impl ShouldExecute for TakeWeightCredit { } } -/// Allows execution from `origin` is origin is contained in `T` (i.e. `T::Contains(origin)`) taking payments into +/// Allows execution from `origin` if it is contained in `T` (i.e. `T::Contains(origin)`) taking payments into /// account. pub struct AllowTopLevelPaidExecutionFrom(PhantomData); impl> ShouldExecute for AllowTopLevelPaidExecutionFrom { From db134b8a1ef64c8854bb4147c86015c8f53db491 Mon Sep 17 00:00:00 2001 From: Kian Paimani <5588131+kianenigma@users.noreply.github.com> Date: Wed, 12 May 2021 11:34:38 +0200 Subject: [PATCH 28/35] Update xcm/xcm-builder/src/barriers.rs Co-authored-by: Alexander Popiak --- xcm/xcm-builder/src/barriers.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/xcm/xcm-builder/src/barriers.rs b/xcm/xcm-builder/src/barriers.rs index 281861d968b5..ea3d80660940 100644 --- a/xcm/xcm-builder/src/barriers.rs +++ b/xcm/xcm-builder/src/barriers.rs @@ -64,8 +64,8 @@ impl> ShouldExecute for AllowTopLevelPaidExecutionFro } } -/// Allows execution from any origin that is contained in `T` (i.e. `T::Contains(origin)`) without any payments. use -/// only for executions from trusted origin groups. +/// Allows execution from any origin that is contained in `T` (i.e. `T::Contains(origin)`) without any payments. +/// Use only for executions from trusted origin groups. pub struct AllowUnpaidExecutionFrom(PhantomData); impl> ShouldExecute for AllowUnpaidExecutionFrom { fn should_execute( From e5a1b67306426314eaa1fe8a1000b93373f3205c Mon Sep 17 00:00:00 2001 From: Kian Paimani <5588131+kianenigma@users.noreply.github.com> Date: Wed, 12 May 2021 11:34:46 +0200 Subject: [PATCH 29/35] Update xcm/xcm-builder/src/currency_adapter.rs Co-authored-by: Alexander Popiak --- xcm/xcm-builder/src/currency_adapter.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/xcm/xcm-builder/src/currency_adapter.rs b/xcm/xcm-builder/src/currency_adapter.rs index 29ae18c1ec4e..53055ecc8c4c 100644 --- a/xcm/xcm-builder/src/currency_adapter.rs +++ b/xcm/xcm-builder/src/currency_adapter.rs @@ -65,9 +65,9 @@ impl From for XcmError { /// /// messages from the parent (relay chain). /// pub type LocationConvertor = (ParentIsDefault); /// -/// /// Final current adapter. This can be used in `xcm::Config` to specify how asset related transactions happen. +/// /// Final currency adapter. This can be used in `xcm::Config` to specify how asset related transactions happen. /// pub type AssetTransactor = CurrencyAdapter< -/// // Use this currency: +/// // Use this balance type: /// u128, /// // The matcher: use the currency when the asset is a concrete asset in our relay chain. /// IsConcrete, From f4bf1211ed0c9db36e50fa5cefee9c615f14e3d3 Mon Sep 17 00:00:00 2001 From: Kian Paimani <5588131+kianenigma@users.noreply.github.com> Date: Wed, 12 May 2021 11:34:56 +0200 Subject: [PATCH 30/35] Update xcm/xcm-builder/src/filter_asset_location.rs Co-authored-by: Alexander Popiak --- xcm/xcm-builder/src/filter_asset_location.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/xcm/xcm-builder/src/filter_asset_location.rs b/xcm/xcm-builder/src/filter_asset_location.rs index 4ee9eccbe2b7..31db271e1830 100644 --- a/xcm/xcm-builder/src/filter_asset_location.rs +++ b/xcm/xcm-builder/src/filter_asset_location.rs @@ -14,7 +14,7 @@ // You should have received a copy of the GNU General Public License // along with Polkadot. If not, see . -//! various implementations of `FilterAssetLocation`. +//! Various implementations of `FilterAssetLocation`. use sp_std::marker::PhantomData; use xcm::v0::{MultiAsset, MultiLocation}; From a7af09bdd45a2ef1067804f61e24fc10f27956ed Mon Sep 17 00:00:00 2001 From: Kian Paimani <5588131+kianenigma@users.noreply.github.com> Date: Wed, 12 May 2021 11:35:03 +0200 Subject: [PATCH 31/35] Update xcm/xcm-builder/src/matches_fungible.rs Co-authored-by: Alexander Popiak --- xcm/xcm-builder/src/matches_fungible.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/xcm/xcm-builder/src/matches_fungible.rs b/xcm/xcm-builder/src/matches_fungible.rs index fd6820655a6a..4d76a49b6bd8 100644 --- a/xcm/xcm-builder/src/matches_fungible.rs +++ b/xcm/xcm-builder/src/matches_fungible.rs @@ -23,7 +23,7 @@ use frame_support::traits::Get; use xcm_executor::traits::MatchesFungible; /// Converts a `MultiAsset` into balance `B` if it is a concrete fungible with an id equal to that -/// of given by `T`'s `Get`. +/// given by `T`'s `Get`. /// /// # Example /// From 91ecc09fada139f0ae06dabc8ff3481df198c180 Mon Sep 17 00:00:00 2001 From: Kian Paimani <5588131+kianenigma@users.noreply.github.com> Date: Wed, 12 May 2021 11:35:13 +0200 Subject: [PATCH 32/35] Update xcm/xcm-executor/src/traits/conversion.rs Co-authored-by: Alexander Popiak --- xcm/xcm-executor/src/traits/conversion.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/xcm/xcm-executor/src/traits/conversion.rs b/xcm/xcm-executor/src/traits/conversion.rs index 2c549be235c3..20d2f9e8f1a1 100644 --- a/xcm/xcm-executor/src/traits/conversion.rs +++ b/xcm/xcm-executor/src/traits/conversion.rs @@ -152,7 +152,7 @@ impl Convert, T> for Decoded { /// } /// } /// # fn main() { -/// let origin = MultiLocation::X1(Junction::Parachain(6)); +/// let origin = MultiLocation::X1(Junction::Parachain(6)); /// assert!( /// <(BumpParaId, AcceptPara7) as ConvertOrigin>::convert_origin(origin, OriginKind::Native) /// .is_ok() From 291ab3b4a058ecbf6b0fce48daccc23692e61ef1 Mon Sep 17 00:00:00 2001 From: Kian Paimani <5588131+kianenigma@users.noreply.github.com> Date: Wed, 12 May 2021 11:35:20 +0200 Subject: [PATCH 33/35] Update xcm/xcm-executor/src/traits/conversion.rs Co-authored-by: Alexander Popiak --- xcm/xcm-executor/src/traits/conversion.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/xcm/xcm-executor/src/traits/conversion.rs b/xcm/xcm-executor/src/traits/conversion.rs index 20d2f9e8f1a1..da39c3cecd39 100644 --- a/xcm/xcm-executor/src/traits/conversion.rs +++ b/xcm/xcm-executor/src/traits/conversion.rs @@ -25,7 +25,7 @@ use xcm::v0::{MultiLocation, OriginKind}; /// `convert_ref`, since this will never result in a clone. Use `convert` when you definitely need to consume /// the source value. /// -/// Can be amalgamated into tuples. If any of the tuple elements converts into `Ok(_)` short circuits, otherwise returns +/// Can be amalgamated into tuples. If any of the tuple elements converts into `Ok(_)` it short circuits. Otherwise returns /// the `Err(_)` of the last failing conversion (or `Err(())` for ref conversions). pub trait Convert { /// Convert from `value` (of type `A`) into an equivalent value of type `B`, `Err` if not possible. From 73cba92dcf1b83535b7a6f3d07a695bb09466d79 Mon Sep 17 00:00:00 2001 From: Kian Paimani <5588131+kianenigma@users.noreply.github.com> Date: Wed, 12 May 2021 11:35:27 +0200 Subject: [PATCH 34/35] Update xcm/xcm-executor/src/traits/transact_asset.rs Co-authored-by: Alexander Popiak --- xcm/xcm-executor/src/traits/transact_asset.rs | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/xcm/xcm-executor/src/traits/transact_asset.rs b/xcm/xcm-executor/src/traits/transact_asset.rs index b03f58ce9046..338a341068fd 100644 --- a/xcm/xcm-executor/src/traits/transact_asset.rs +++ b/xcm/xcm-executor/src/traits/transact_asset.rs @@ -20,7 +20,7 @@ use crate::Assets; /// Facility for asset transacting. /// -/// This should work with as many asset/location combinations as possible. Locations to support may include non- account +/// This should work with as many asset/location combinations as possible. Locations to support may include non-account /// locations such as a `MultiLocation::X1(Junction::Parachain)`. Different chains may handle them in different ways. /// /// Can be amalgamated as a tuple of items that implement this trait. In such executions, if any of the transactors @@ -85,4 +85,3 @@ impl TransactAsset for Tuple { Err(XcmError::Unimplemented) } } - From c57e637da4c0a2f64116db5d3141effd7da5397d Mon Sep 17 00:00:00 2001 From: Kian Paimani <5588131+kianenigma@users.noreply.github.com> Date: Wed, 12 May 2021 11:35:34 +0200 Subject: [PATCH 35/35] Update xcm/xcm-executor/src/traits/should_execute.rs Co-authored-by: Alexander Popiak --- xcm/xcm-executor/src/traits/should_execute.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/xcm/xcm-executor/src/traits/should_execute.rs b/xcm/xcm-executor/src/traits/should_execute.rs index c42e5156d93a..15f66d5105ee 100644 --- a/xcm/xcm-executor/src/traits/should_execute.rs +++ b/xcm/xcm-executor/src/traits/should_execute.rs @@ -20,7 +20,7 @@ use frame_support::weights::Weight; /// 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 the tuple elements returns `Ok()`, the +/// Can be amalgamated into a tuple to have multiple trials. If any of the tuple elements returns `Ok()`, the /// execution stops. Else, `Err(_)` is returned if all elements reject the message. pub trait ShouldExecute { /// Returns `true` if the given `message` may be executed.