diff --git a/xcm/pallet-xcm/src/lib.rs b/xcm/pallet-xcm/src/lib.rs index e08bf7c1cbb1..9021dc73cbc9 100644 --- a/xcm/pallet-xcm/src/lib.rs +++ b/xcm/pallet-xcm/src/lib.rs @@ -106,6 +106,8 @@ pub mod pallet { CannotReanchor, /// Too many assets have been attempted for transfer. TooManyAssets, + /// Origin is invalid for sending. + InvalidOrigin, } #[pallet::hooks] @@ -120,12 +122,12 @@ pub mod pallet { message: Box>, ) -> DispatchResult { let origin_location = T::SendXcmOrigin::ensure_origin(origin)?; - Self::send_xcm(origin_location.clone(), *dest.clone(), *message.clone()).map_err( - |e| match e { - XcmError::CannotReachDestination(..) => Error::::Unreachable, - _ => Error::::SendFailure, - }, - )?; + let interior = + origin_location.clone().try_into().map_err(|_| Error::::InvalidOrigin)?; + Self::send_xcm(interior, *dest.clone(), *message.clone()).map_err(|e| match e { + XcmError::CannotReachDestination(..) => Error::::Unreachable, + _ => Error::::SendFailure, + })?; Self::deposit_event(Event::Sent(origin_location, *dest, *message)); Ok(()) } @@ -302,11 +304,11 @@ pub mod pallet { /// Relay an XCM `message` from a given `interior` location in this context to a given `dest` /// location. A null `dest` is not handled. pub fn send_xcm( - interior: MultiLocation, + interior: Junctions, dest: MultiLocation, message: Xcm<()>, ) -> Result<(), XcmError> { - let message = if interior.is_here() { + let message = if let Junctions::Here = interior { message } else { Xcm::<()>::RelayedFrom { who: interior, message: Box::new(message) } diff --git a/xcm/pallet-xcm/src/tests.rs b/xcm/pallet-xcm/src/tests.rs index 1e9d9acb42dc..62d5f87807bb 100644 --- a/xcm/pallet-xcm/src/tests.rs +++ b/xcm/pallet-xcm/src/tests.rs @@ -17,10 +17,8 @@ use crate::mock::*; use frame_support::{assert_noop, assert_ok, traits::Currency}; use polkadot_parachain::primitives::{AccountIdConversion, Id as ParaId}; -use xcm::{ - opaque::v1::prelude::*, - v1::{Junction, Xcm}, -}; +use std::convert::TryInto; +use xcm::v1::prelude::*; const ALICE: AccountId = AccountId::new([0u8; 32]); const BOB: AccountId = AccountId::new([1u8; 32]); @@ -55,7 +53,10 @@ fn send_works() { sent_xcm(), vec![( Here.into(), - RelayedFrom { who: sender.clone(), message: Box::new(message.clone()) } + RelayedFrom { + who: sender.clone().try_into().unwrap(), + message: Box::new(message.clone()), + } )] ); assert_eq!( diff --git a/xcm/src/v0/mod.rs b/xcm/src/v0/mod.rs index 8b41eaec85f1..3616a9258b57 100644 --- a/xcm/src/v0/mod.rs +++ b/xcm/src/v0/mod.rs @@ -30,7 +30,7 @@ mod multi_asset; mod multi_location; mod order; mod traits; -use super::v1::{Response as Response1, Xcm as Xcm1}; +use super::v1::{MultiLocation as MultiLocation1, Response as Response1, Xcm as Xcm1}; pub use junction::{BodyId, BodyPart, Junction, NetworkId}; pub use multi_asset::{AssetInstance, MultiAsset}; pub use multi_location::MultiLocation::{self, *}; @@ -376,7 +376,7 @@ impl TryFrom> for Xcm { Xcm1::Transact { origin_type, require_weight_at_most, call } => Transact { origin_type, require_weight_at_most, call: call.into() }, Xcm1::RelayedFrom { who, message } => RelayedFrom { - who: who.try_into()?, + who: MultiLocation1 { interior: who, parents: 0 }.try_into()?, message: alloc::boxed::Box::new((*message).try_into()?), }, }) diff --git a/xcm/src/v1/mod.rs b/xcm/src/v1/mod.rs index acf60252a1f2..5eb9340881d5 100644 --- a/xcm/src/v1/mod.rs +++ b/xcm/src/v1/mod.rs @@ -267,14 +267,11 @@ pub enum Xcm { /// A message to indicate that the embedded XCM is actually arriving on behalf of some consensus /// location within the origin. /// - /// Safety: `who` must be an interior location of the context. This basically means that no `Parent` - /// junctions are allowed in it. This should be verified at the time of XCM execution. - /// /// Kind: *Instruction* /// /// Errors: #[codec(index = 10)] - RelayedFrom { who: MultiLocation, message: alloc::boxed::Box> }, + RelayedFrom { who: Junctions, message: alloc::boxed::Box> }, } impl Xcm { @@ -375,7 +372,7 @@ impl TryFrom> for Xcm { Xcm0::Transact { origin_type, require_weight_at_most, call } => Transact { origin_type, require_weight_at_most, call: call.into() }, Xcm0::RelayedFrom { who, message } => RelayedFrom { - who: who.try_into()?, + who: MultiLocation::try_from(who)?.try_into()?, message: alloc::boxed::Box::new((*message).try_into()?), }, }) diff --git a/xcm/src/v1/multilocation.rs b/xcm/src/v1/multilocation.rs index 165d1941956c..f1546b509924 100644 --- a/xcm/src/v1/multilocation.rs +++ b/xcm/src/v1/multilocation.rs @@ -149,53 +149,36 @@ impl MultiLocation { (multilocation, last) } - /// Mutates `self`, suffixing its interior junctions with `new`. Returns `Err` in case of overflow. - pub fn push_interior(&mut self, new: Junction) -> result::Result<(), ()> { - let mut n = Junctions::Here; - mem::swap(&mut self.interior, &mut n); - match n.pushed_with(new) { - Ok(result) => { - self.interior = result; - Ok(()) - }, - Err(old) => { - self.interior = old; - Err(()) - }, - } + /// Mutates `self`, suffixing its interior junctions with `new`. Returns `Err` with `new` in + /// case of overflow. + pub fn push_interior(&mut self, new: Junction) -> result::Result<(), Junction> { + self.interior.push(new) } - /// Mutates `self`, prefixing its interior junctions with `new`. Returns `Err` in case of overflow. - pub fn push_front_interior(&mut self, new: Junction) -> result::Result<(), ()> { - let mut n = Junctions::Here; - mem::swap(&mut self.interior, &mut n); - match n.pushed_front_with(new) { - Ok(result) => { - self.interior = result; - Ok(()) - }, - Err(old) => { - self.interior = old; - Err(()) - }, - } + /// Mutates `self`, prefixing its interior junctions with `new`. Returns `Err` with `new` in + /// case of overflow. + pub fn push_front_interior(&mut self, new: Junction) -> result::Result<(), Junction> { + self.interior.push_front(new) } - /// Consumes `self` and returns a `MultiLocation` suffixed with `new`, or an `Err` with the original value of + /// Consumes `self` and returns a `MultiLocation` suffixed with `new`, or an `Err` with theoriginal value of /// `self` in case of overflow. - pub fn pushed_with_interior(self, new: Junction) -> result::Result { + pub fn pushed_with_interior(self, new: Junction) -> result::Result { match self.interior.pushed_with(new) { Ok(i) => Ok(MultiLocation { interior: i, parents: self.parents }), - Err(i) => Err(MultiLocation { interior: i, parents: self.parents }), + Err((i, j)) => Err((MultiLocation { interior: i, parents: self.parents }, j)), } } /// 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_interior(self, new: Junction) -> result::Result { + pub fn pushed_front_with_interior( + self, + new: Junction, + ) -> result::Result { match self.interior.pushed_front_with(new) { Ok(i) => Ok(MultiLocation { interior: i, parents: self.parents }), - Err(i) => Err(MultiLocation { interior: i, parents: self.parents }), + Err((i, j)) => Err((MultiLocation { interior: i, parents: self.parents }, j)), } } @@ -259,8 +242,7 @@ impl MultiLocation { self.interior.match_and_split(&prefix.interior) } - /// Mutate `self` so that it is suffixed with `suffix`. The correct normalized form is returned, - /// removing any internal [Non-Parent, `Parent`] combinations. + /// Mutate `self` so that it is suffixed with `suffix`. /// /// Does not modify `self` and returns `Err` with `suffix` in case of overflow. /// @@ -268,22 +250,19 @@ impl MultiLocation { /// ```rust /// # use xcm::v1::{Junctions::*, Junction::*, MultiLocation}; /// # fn main() { - /// let mut m = MultiLocation::new(1, X2(Parachain(21), OnlyChild)); - /// assert_eq!(m.append_with(MultiLocation::new(1, X1(PalletInstance(3)))), Ok(())); + /// let mut m = MultiLocation::new(1, X1(Parachain(21))); + /// assert_eq!(m.append_with(X1(PalletInstance(3))), Ok(())); /// assert_eq!(m, MultiLocation::new(1, X2(Parachain(21), PalletInstance(3)))); /// # } /// ``` - pub fn append_with(&mut self, suffix: MultiLocation) -> Result<(), MultiLocation> { - let mut prefix = suffix; - core::mem::swap(self, &mut prefix); - match self.prepend_with(prefix) { - Ok(()) => Ok(()), - Err(prefix) => { - let mut suffix = prefix; - core::mem::swap(self, &mut suffix); - Err(suffix) - }, + pub fn append_with(&mut self, suffix: Junctions) -> Result<(), Junctions> { + if self.interior.len().saturating_add(suffix.len()) > MAX_JUNCTIONS { + return Err(suffix) } + for j in suffix.into_iter() { + self.interior.push(j).expect("Already checked the sum of the len()s; qed") + } + Ok(()) } /// Mutate `self` so that it is prefixed with `prefix`. @@ -767,9 +746,41 @@ impl Junctions { tail } - /// Consumes `self` and returns a `Junctions` 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 { + /// Mutates `self` to be appended with `new` or returns an `Err` with `new` if would overflow. + pub fn push(&mut self, new: Junction) -> result::Result<(), Junction> { + let mut dummy = Junctions::Here; + mem::swap(self, &mut dummy); + match dummy.pushed_with(new) { + Ok(s) => { + *self = s; + Ok(()) + }, + Err((s, j)) => { + *self = s; + Err(j) + }, + } + } + + /// Mutates `self` to be prepended with `new` or returns an `Err` with `new` if would overflow. + pub fn push_front(&mut self, new: Junction) -> result::Result<(), Junction> { + let mut dummy = Junctions::Here; + mem::swap(self, &mut dummy); + match dummy.pushed_front_with(new) { + Ok(s) => { + *self = s; + Ok(()) + }, + Err((s, j)) => { + *self = s; + Err(j) + }, + } + } + + /// Consumes `self` and returns a `Junctions` suffixed with `new`, or an `Err` with the + /// original value of `self` and `new` in case of overflow. + pub fn pushed_with(self, new: Junction) -> result::Result { Ok(match self { Junctions::Here => Junctions::X1(new), Junctions::X1(a) => Junctions::X2(a, new), @@ -779,13 +790,13 @@ impl Junctions { Junctions::X5(a, b, c, d, e) => Junctions::X6(a, b, c, d, e, new), Junctions::X6(a, b, c, d, e, f) => Junctions::X7(a, b, c, d, e, f, new), Junctions::X7(a, b, c, d, e, f, g) => Junctions::X8(a, b, c, d, e, f, g, new), - s => Err(s)?, + s => Err((s, new))?, }) } - /// Consumes `self` and returns a `Junctions` 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 { + /// Consumes `self` and returns a `Junctions` prefixed with `new`, or an `Err` with the + /// original value of `self` and `new` in case of overflow. + pub fn pushed_front_with(self, new: Junction) -> result::Result { Ok(match self { Junctions::Here => Junctions::X1(new), Junctions::X1(a) => Junctions::X2(new, a), @@ -795,7 +806,7 @@ impl Junctions { Junctions::X5(a, b, c, d, e) => Junctions::X6(new, a, b, c, d, e), Junctions::X6(a, b, c, d, e, f) => Junctions::X7(new, a, b, c, d, e, f), Junctions::X7(a, b, c, d, e, f, g) => Junctions::X8(new, a, b, c, d, e, f, g), - s => Err(s)?, + s => Err((s, new))?, }) } @@ -1245,7 +1256,7 @@ mod tests { fn append_with_works() { let acc = AccountIndex64 { network: Any, index: 23 }; let mut m = MultiLocation { parents: 1, interior: X1(Parachain(42)) }; - assert_eq!(m.append_with(MultiLocation::from(X2(PalletInstance(3), acc.clone()))), Ok(())); + assert_eq!(m.append_with(X2(PalletInstance(3), acc.clone())), Ok(())); assert_eq!( m, MultiLocation { @@ -1256,12 +1267,12 @@ mod tests { // cannot append to create overly long multilocation let acc = AccountIndex64 { network: Any, index: 23 }; - let mut m = MultiLocation { + let m = MultiLocation { parents: 254, interior: X5(Parachain(42), OnlyChild, OnlyChild, OnlyChild, OnlyChild), }; - let suffix = MultiLocation::from(X4(PalletInstance(3), acc.clone(), OnlyChild, OnlyChild)); - assert_eq!(m.append_with(suffix.clone()), Err(suffix)); + let suffix = X4(PalletInstance(3), acc.clone(), OnlyChild, OnlyChild); + assert_eq!(m.clone().append_with(suffix.clone()), Err(suffix)); } #[test] diff --git a/xcm/xcm-builder/src/fungibles_adapter.rs b/xcm/xcm-builder/src/fungibles_adapter.rs index 2107acdb4a36..d63703368a8c 100644 --- a/xcm/xcm-builder/src/fungibles_adapter.rs +++ b/xcm/xcm-builder/src/fungibles_adapter.rs @@ -55,7 +55,7 @@ impl, AssetId: Clone, ConvertAssetId: Convert) -> result::Result { let mut location = Prefix::get(); let id = ConvertAssetId::reverse_ref(what)?; - location.push_interior(Junction::GeneralIndex(id))?; + location.push_interior(Junction::GeneralIndex(id)).map_err(|_| ())?; Ok(location) } } diff --git a/xcm/xcm-executor/src/lib.rs b/xcm/xcm-executor/src/lib.rs index 68bca5518afa..3306c303e261 100644 --- a/xcm/xcm-executor/src/lib.rs +++ b/xcm/xcm-executor/src/lib.rs @@ -244,7 +244,6 @@ impl XcmExecutor { None }, (origin, Xcm::RelayedFrom { who, message }) => { - ensure!(who.parent_count() == 0, XcmError::EscalationOfPrivilege); let mut origin = origin; origin.append_with(who).map_err(|_| XcmError::MultiLocationFull)?; let surplus = Self::do_execute_xcm( diff --git a/xcm/xcm-simulator/example/src/lib.rs b/xcm/xcm-simulator/example/src/lib.rs index 547c6f1858be..2649d46991ce 100644 --- a/xcm/xcm-simulator/example/src/lib.rs +++ b/xcm/xcm-simulator/example/src/lib.rs @@ -111,7 +111,7 @@ mod tests { ); Relay::execute_with(|| { assert_ok!(RelayChainPalletXcm::send_xcm( - Here.into(), + Here, Parachain(1).into(), Transact { origin_type: OriginKind::SovereignAccount, @@ -138,7 +138,7 @@ mod tests { ); ParaA::execute_with(|| { assert_ok!(ParachainPalletXcm::send_xcm( - Here.into(), + Here, Parent.into(), Transact { origin_type: OriginKind::SovereignAccount, @@ -165,7 +165,7 @@ mod tests { ); ParaA::execute_with(|| { assert_ok!(ParachainPalletXcm::send_xcm( - Here.into(), + Here, MultiLocation::new(1, X1(Parachain(2))), Transact { origin_type: OriginKind::SovereignAccount,