From 952c2539af5b7217c946c627796c7fc1236cd7c8 Mon Sep 17 00:00:00 2001 From: Francisco Aguirre Date: Tue, 23 May 2023 16:51:51 -0300 Subject: [PATCH 1/4] XCM: Fix issue with RequestUnlock --- xcm/src/v3/mod.rs | 8 +- xcm/src/v3/multiasset.rs | 88 +++++++++++++--------- xcm/src/v3/multilocation.rs | 56 +++++++------- xcm/src/v3/traits.rs | 26 +++++++ xcm/xcm-executor/src/assets.rs | 2 +- xcm/xcm-executor/src/lib.rs | 19 +++-- xcm/xcm-simulator/example/src/lib.rs | 26 ++++++- xcm/xcm-simulator/example/src/parachain.rs | 25 +++++- 8 files changed, 167 insertions(+), 83 deletions(-) diff --git a/xcm/src/v3/mod.rs b/xcm/src/v3/mod.rs index 435998255a9c..729aa31edca8 100644 --- a/xcm/src/v3/mod.rs +++ b/xcm/src/v3/mod.rs @@ -48,8 +48,8 @@ pub use multilocation::{ Ancestor, AncestorThen, InteriorMultiLocation, MultiLocation, Parent, ParentThen, }; pub use traits::{ - send_xcm, validate_send, Error, ExecuteXcm, Outcome, PreparedMessage, Result, SendError, - SendResult, SendXcm, Unwrappable, Weight, XcmHash, + send_xcm, validate_send, Error, ExecuteXcm, Outcome, PreparedMessage, Reanchorable, Result, + SendError, SendResult, SendXcm, Unwrappable, Weight, XcmHash, }; // These parts of XCM v2 are unchanged in XCM v3, and are re-imported here. pub use super::v2::OriginKind; @@ -183,8 +183,8 @@ pub mod prelude { MultiAssets, MultiLocation, NetworkId::{self, *}, OriginKind, Outcome, PalletInfo, Parent, ParentThen, PreparedMessage, QueryId, - QueryResponseInfo, Response, Result as XcmResult, SendError, SendResult, SendXcm, - Unwrappable, + QueryResponseInfo, Reanchorable, Response, Result as XcmResult, SendError, SendResult, + SendXcm, Unwrappable, WeightLimit::{self, *}, WildFungibility::{self, Fungible as WildFungible, NonFungible as WildNonFungible}, WildMultiAsset::{self, *}, diff --git a/xcm/src/v3/multiasset.rs b/xcm/src/v3/multiasset.rs index 06cd2c8b5b82..685a01ef7091 100644 --- a/xcm/src/v3/multiasset.rs +++ b/xcm/src/v3/multiasset.rs @@ -24,11 +24,14 @@ //! account. use super::{InteriorMultiLocation, MultiLocation}; -use crate::v2::{ - AssetId as OldAssetId, AssetInstance as OldAssetInstance, Fungibility as OldFungibility, - MultiAsset as OldMultiAsset, MultiAssetFilter as OldMultiAssetFilter, - MultiAssets as OldMultiAssets, WildFungibility as OldWildFungibility, - WildMultiAsset as OldWildMultiAsset, +use crate::{ + latest::Reanchorable, + v2::{ + AssetId as OldAssetId, AssetInstance as OldAssetInstance, Fungibility as OldFungibility, + MultiAsset as OldMultiAsset, MultiAssetFilter as OldMultiAssetFilter, + MultiAssets as OldMultiAssets, WildFungibility as OldWildFungibility, + WildMultiAsset as OldWildMultiAsset, + }, }; use alloc::{vec, vec::Vec}; use core::{ @@ -374,9 +377,22 @@ impl AssetId { Ok(()) } - /// Mutate the asset to represent the same value from the perspective of a new `target` - /// location. The local chain's location is provided in `context`. - pub fn reanchor( + /// Use the value of `self` along with a `fun` fungibility specifier to create the corresponding `MultiAsset` value. + pub fn into_multiasset(self, fun: Fungibility) -> MultiAsset { + MultiAsset { fun, id: self } + } + + /// Use the value of `self` along with a `fun` fungibility specifier to create the corresponding `WildMultiAsset` + /// wildcard (`AllOf`) value. + pub fn into_wild(self, fun: WildFungibility) -> WildMultiAsset { + WildMultiAsset::AllOf { fun, id: self } + } +} + +impl Reanchorable for AssetId { + type Error = (); + + fn reanchor( &mut self, target: &MultiLocation, context: InteriorMultiLocation, @@ -387,15 +403,15 @@ impl AssetId { Ok(()) } - /// Use the value of `self` along with a `fun` fungibility specifier to create the corresponding `MultiAsset` value. - pub fn into_multiasset(self, fun: Fungibility) -> MultiAsset { - MultiAsset { fun, id: self } - } - - /// Use the value of `self` along with a `fun` fungibility specifier to create the corresponding `WildMultiAsset` - /// wildcard (`AllOf`) value. - pub fn into_wild(self, fun: WildFungibility) -> WildMultiAsset { - WildMultiAsset::AllOf { fun, id: self } + fn reanchored( + mut self, + target: &MultiLocation, + context: InteriorMultiLocation, + ) -> Result { + match self.reanchor(target, context) { + Ok(()) => Ok(self), + Err(()) => Err(()), + } } } @@ -448,9 +464,24 @@ impl MultiAsset { self.id.prepend_with(prepend) } - /// Mutate the location of the asset identifier if concrete, giving it the same location - /// relative to a `target` context. The local context is provided as `context`. - pub fn reanchor( + /// Returns true if `self` is a super-set of the given `inner` asset. + pub fn contains(&self, inner: &MultiAsset) -> bool { + use Fungibility::*; + if self.id == inner.id { + match (&self.fun, &inner.fun) { + (Fungible(a), Fungible(i)) if a >= i => return true, + (NonFungible(a), NonFungible(i)) if a == i => return true, + _ => (), + } + } + false + } +} + +impl Reanchorable for MultiAsset { + type Error = (); + + fn reanchor( &mut self, target: &MultiLocation, context: InteriorMultiLocation, @@ -458,9 +489,7 @@ impl MultiAsset { self.id.reanchor(target, context) } - /// Mutate the location of the asset identifier if concrete, giving it the same location - /// relative to a `target` context. The local context is provided as `context`. - pub fn reanchored( + fn reanchored( mut self, target: &MultiLocation, context: InteriorMultiLocation, @@ -468,19 +497,6 @@ impl MultiAsset { self.id.reanchor(target, context)?; Ok(self) } - - /// Returns true if `self` is a super-set of the given `inner` asset. - pub fn contains(&self, inner: &MultiAsset) -> bool { - use Fungibility::*; - if self.id == inner.id { - match (&self.fun, &inner.fun) { - (Fungible(a), Fungible(i)) if a >= i => return true, - (NonFungible(a), NonFungible(i)) if a == i => return true, - _ => (), - } - } - false - } } impl TryFrom for MultiAsset { diff --git a/xcm/src/v3/multilocation.rs b/xcm/src/v3/multilocation.rs index 7a2f3eb0ca33..270cc2cc3d0d 100644 --- a/xcm/src/v3/multilocation.rs +++ b/xcm/src/v3/multilocation.rs @@ -16,7 +16,7 @@ //! XCM `MultiLocation` datatype. -use super::{Junction, Junctions}; +use super::{traits::Reanchorable, Junction, Junctions}; use crate::{v2::MultiLocation as OldMultiLocation, VersionedMultiLocation}; use core::{ convert::{TryFrom, TryInto}, @@ -385,11 +385,30 @@ impl MultiLocation { } } - /// Mutate `self` so that it represents the same location from the point of view of `target`. - /// The context of `self` is provided as `context`. - /// - /// Does not modify `self` in case of overflow. - pub fn reanchor( + /// Remove any unneeded parents/junctions in `self` based on the given context it will be + /// interpreted in. + pub fn simplify(&mut self, context: &Junctions) { + if context.len() < self.parents as usize { + // Not enough context + return + } + while self.parents > 0 { + let maybe = context.at(context.len() - (self.parents as usize)); + match (self.interior.first(), maybe) { + (Some(i), Some(j)) if i == j => { + self.interior.take_first(); + self.parents -= 1; + }, + _ => break, + } + } + } +} + +impl Reanchorable for MultiLocation { + type Error = Self; + + fn reanchor( &mut self, target: &MultiLocation, context: InteriorMultiLocation, @@ -410,11 +429,7 @@ impl MultiLocation { Ok(()) } - /// Consume `self` and return a new value representing the same location from the point of view - /// of `target`. The context of `self` is provided as `context`. - /// - /// Returns the original `self` in case of overflow. - pub fn reanchored( + fn reanchored( mut self, target: &MultiLocation, context: InteriorMultiLocation, @@ -424,25 +439,6 @@ impl MultiLocation { Err(()) => Err(self), } } - - /// Remove any unneeded parents/junctions in `self` based on the given context it will be - /// interpreted in. - pub fn simplify(&mut self, context: &Junctions) { - if context.len() < self.parents as usize { - // Not enough context - return - } - while self.parents > 0 { - let maybe = context.at(context.len() - (self.parents as usize)); - match (self.interior.first(), maybe) { - (Some(i), Some(j)) if i == j => { - self.interior.take_first(); - self.parents -= 1; - }, - _ => break, - } - } - } } impl TryFrom for MultiLocation { diff --git a/xcm/src/v3/traits.rs b/xcm/src/v3/traits.rs index b752647b0819..248909868c56 100644 --- a/xcm/src/v3/traits.rs +++ b/xcm/src/v3/traits.rs @@ -382,6 +382,32 @@ impl Unwrappable for Option { } } +/// TODO: Add documentation +pub trait Reanchorable: Sized { + /// Type to return in case of an error. + type Error; + + /// Mutate `self` so that it represents the same location from the point of view of `target`. + /// The context of `self` is provided as `context`. + /// + /// Does not modify `self` in case of overflow. + fn reanchor( + &mut self, + target: &MultiLocation, + context: InteriorMultiLocation, + ) -> core::result::Result<(), ()>; + + /// Consume `self` and return a new value representing the same location from the point of view + /// of `target`. The context of `self` is provided as `context`. + /// + /// Returns the original `self` in case of overflow. + fn reanchored( + self, + target: &MultiLocation, + context: InteriorMultiLocation, + ) -> core::result::Result; +} + /// Utility for sending an XCM message to a given location. /// /// These can be amalgamated in tuples to form sophisticated routing systems. In tuple format, each diff --git a/xcm/xcm-executor/src/assets.rs b/xcm/xcm-executor/src/assets.rs index f5e0659931eb..5a5cd0deb8bc 100644 --- a/xcm/xcm-executor/src/assets.rs +++ b/xcm/xcm-executor/src/assets.rs @@ -23,7 +23,7 @@ use sp_std::{ use xcm::latest::{ AssetId, AssetInstance, Fungibility::{Fungible, NonFungible}, - InteriorMultiLocation, MultiAsset, MultiAssetFilter, MultiAssets, MultiLocation, + InteriorMultiLocation, MultiAsset, MultiAssetFilter, MultiAssets, MultiLocation, Reanchorable, WildFungibility::{Fungible as WildFungible, NonFungible as WildNonFungible}, WildMultiAsset::{All, AllCounted, AllOf, AllOfCounted}, }; diff --git a/xcm/xcm-executor/src/lib.rs b/xcm/xcm-executor/src/lib.rs index 749a63114d95..94ab95aafda9 100644 --- a/xcm/xcm-executor/src/lib.rs +++ b/xcm/xcm-executor/src/lib.rs @@ -873,9 +873,11 @@ impl XcmExecutor { RequestUnlock { asset, locker } => { let origin = *self.origin_ref().ok_or(XcmError::BadOrigin)?; let remote_asset = Self::try_reanchor(asset.clone(), &locker)?.0; + let remote_target = Self::try_reanchor(origin.clone(), &locker)?.0; let reduce_ticket = Config::AssetLocker::prepare_reduce_unlockable(locker, asset, origin)?; - let msg = Xcm::<()>(vec![UnlockAsset { asset: remote_asset, target: origin }]); + let msg = + Xcm::<()>(vec![UnlockAsset { asset: remote_asset, target: remote_target }]); let (ticket, price) = validate_send::(locker, msg)?; self.take_fee(price, FeeReason::RequestUnlock)?; reduce_ticket.enact()?; @@ -976,15 +978,18 @@ impl XcmExecutor { Config::XcmSender::deliver(ticket).map_err(Into::into) } - fn try_reanchor( - asset: MultiAsset, + fn try_reanchor( + reanchorable: T, destination: &MultiLocation, - ) -> Result<(MultiAsset, InteriorMultiLocation), XcmError> { + ) -> Result<(T, InteriorMultiLocation), XcmError> + where + T: Reanchorable, + { let reanchor_context = Config::UniversalLocation::get(); - let asset = asset + let reanchored = reanchorable .reanchored(&destination, reanchor_context) - .map_err(|()| XcmError::ReanchorFailed)?; - Ok((asset, reanchor_context)) + .map_err(|_| XcmError::ReanchorFailed)?; + Ok((reanchored, reanchor_context)) } /// NOTE: Any assets which were unable to be reanchored are introduced into `failed_bin`. diff --git a/xcm/xcm-simulator/example/src/lib.rs b/xcm/xcm-simulator/example/src/lib.rs index 33a5b2c70a9f..800eee51b0aa 100644 --- a/xcm/xcm-simulator/example/src/lib.rs +++ b/xcm/xcm-simulator/example/src/lib.rs @@ -272,7 +272,7 @@ mod tests { } #[test] - fn remote_locking() { + fn remote_locking_and_unlocking() { MockNet::reset(); let locked_amount = 100; @@ -306,6 +306,30 @@ mod tests { }])] ); }); + + ParaB::execute_with(|| { + // Request unlocking part of the funds on the relay chain + let message = Xcm(vec![RequestUnlock { + asset: (Parent, locked_amount - 50).into(), + locker: Parent.into(), + }]); + assert_ok!(ParachainPalletXcm::send_xcm(Here, (Parent, Parachain(1)), message)); + }); + + Relay::execute_with(|| { + use pallet_balances::{BalanceLock, Reasons}; + // Lock is reduced + assert_eq!( + relay_chain::Balances::locks(&child_account_id(2)), + vec![BalanceLock { + id: *b"py/xcmlk", + amount: locked_amount - 50, + reasons: Reasons::All + }] + ); + }); + + // TODO: Test case where full lock is unlocked } /// Scenario: diff --git a/xcm/xcm-simulator/example/src/parachain.rs b/xcm/xcm-simulator/example/src/parachain.rs index 1985c721765e..d10e73dbf6e3 100644 --- a/xcm/xcm-simulator/example/src/parachain.rs +++ b/xcm/xcm-simulator/example/src/parachain.rs @@ -17,9 +17,10 @@ //! Parachain runtime mock. use codec::{Decode, Encode}; +use core::marker::PhantomData; use frame_support::{ construct_runtime, parameter_types, - traits::{EnsureOrigin, EnsureOriginWithArg, Everything, EverythingBut, Nothing}, + traits::{ContainsPair, EnsureOrigin, EnsureOriginWithArg, Everything, EverythingBut, Nothing}, weights::{constants::WEIGHT_REF_TIME_PER_SECOND, Weight}, }; @@ -27,7 +28,7 @@ use frame_system::EnsureRoot; use sp_core::{ConstU32, H256}; use sp_runtime::{ testing::Header, - traits::{Hash, IdentityLookup}, + traits::{Get, Hash, IdentityLookup}, AccountId32, }; use sp_std::prelude::*; @@ -238,7 +239,7 @@ impl Config for XcmConfig { type Trader = FixedRateOfFungible; type ResponseHandler = (); type AssetTrap = (); - type AssetLocker = (); + type AssetLocker = PolkadotXcm; type AssetExchanger = (); type AssetClaims = (); type SubscriptionService = (); @@ -403,6 +404,22 @@ parameter_types! { pub ReachableDest: Option = Some(Parent.into()); } +pub struct TrustedLockerCase(PhantomData); +impl> ContainsPair + for TrustedLockerCase +{ + fn contains(origin: &MultiLocation, asset: &MultiAsset) -> bool { + let (o, a) = T::get(); + a.matches(asset) && &o == origin + } +} + +parameter_types! { + pub RelayTokenForRelay: (MultiLocation, MultiAssetFilter) = (Parent.into(), Wild(AllOf { id: Concrete(Parent.into()), fun: WildFungible })); +} + +pub type TrustedLockers = TrustedLockerCase; + impl pallet_xcm::Config for Runtime { type RuntimeEvent = RuntimeEvent; type SendXcmOrigin = EnsureXcmOrigin; @@ -420,7 +437,7 @@ impl pallet_xcm::Config for Runtime { type AdvertisedXcmVersion = pallet_xcm::CurrentXcmVersion; type Currency = Balances; type CurrencyMatcher = (); - type TrustedLockers = (); + type TrustedLockers = TrustedLockers; type SovereignAccountOf = LocationToAccountId; type MaxLockers = ConstU32<8>; type MaxRemoteLockConsumers = ConstU32<0>; From 8af34f8630a74b7bcc79ba2c2155c77d01495193 Mon Sep 17 00:00:00 2001 From: Francisco Aguirre Date: Thu, 25 May 2023 17:49:28 -0300 Subject: [PATCH 2/4] Leave API changes for v4 --- xcm/src/v3/mod.rs | 8 +-- xcm/src/v3/multiasset.rs | 88 ++++++++++++---------------- xcm/src/v3/multilocation.rs | 56 ++++++++++-------- xcm/src/v3/traits.rs | 26 -------- xcm/xcm-executor/src/assets.rs | 2 +- xcm/xcm-executor/src/lib.rs | 26 +++++--- xcm/xcm-simulator/example/src/lib.rs | 2 - 7 files changed, 88 insertions(+), 120 deletions(-) diff --git a/xcm/src/v3/mod.rs b/xcm/src/v3/mod.rs index 729aa31edca8..435998255a9c 100644 --- a/xcm/src/v3/mod.rs +++ b/xcm/src/v3/mod.rs @@ -48,8 +48,8 @@ pub use multilocation::{ Ancestor, AncestorThen, InteriorMultiLocation, MultiLocation, Parent, ParentThen, }; pub use traits::{ - send_xcm, validate_send, Error, ExecuteXcm, Outcome, PreparedMessage, Reanchorable, Result, - SendError, SendResult, SendXcm, Unwrappable, Weight, XcmHash, + send_xcm, validate_send, Error, ExecuteXcm, Outcome, PreparedMessage, Result, SendError, + SendResult, SendXcm, Unwrappable, Weight, XcmHash, }; // These parts of XCM v2 are unchanged in XCM v3, and are re-imported here. pub use super::v2::OriginKind; @@ -183,8 +183,8 @@ pub mod prelude { MultiAssets, MultiLocation, NetworkId::{self, *}, OriginKind, Outcome, PalletInfo, Parent, ParentThen, PreparedMessage, QueryId, - QueryResponseInfo, Reanchorable, Response, Result as XcmResult, SendError, SendResult, - SendXcm, Unwrappable, + QueryResponseInfo, Response, Result as XcmResult, SendError, SendResult, SendXcm, + Unwrappable, WeightLimit::{self, *}, WildFungibility::{self, Fungible as WildFungible, NonFungible as WildNonFungible}, WildMultiAsset::{self, *}, diff --git a/xcm/src/v3/multiasset.rs b/xcm/src/v3/multiasset.rs index 685a01ef7091..06cd2c8b5b82 100644 --- a/xcm/src/v3/multiasset.rs +++ b/xcm/src/v3/multiasset.rs @@ -24,14 +24,11 @@ //! account. use super::{InteriorMultiLocation, MultiLocation}; -use crate::{ - latest::Reanchorable, - v2::{ - AssetId as OldAssetId, AssetInstance as OldAssetInstance, Fungibility as OldFungibility, - MultiAsset as OldMultiAsset, MultiAssetFilter as OldMultiAssetFilter, - MultiAssets as OldMultiAssets, WildFungibility as OldWildFungibility, - WildMultiAsset as OldWildMultiAsset, - }, +use crate::v2::{ + AssetId as OldAssetId, AssetInstance as OldAssetInstance, Fungibility as OldFungibility, + MultiAsset as OldMultiAsset, MultiAssetFilter as OldMultiAssetFilter, + MultiAssets as OldMultiAssets, WildFungibility as OldWildFungibility, + WildMultiAsset as OldWildMultiAsset, }; use alloc::{vec, vec::Vec}; use core::{ @@ -377,22 +374,9 @@ impl AssetId { Ok(()) } - /// Use the value of `self` along with a `fun` fungibility specifier to create the corresponding `MultiAsset` value. - pub fn into_multiasset(self, fun: Fungibility) -> MultiAsset { - MultiAsset { fun, id: self } - } - - /// Use the value of `self` along with a `fun` fungibility specifier to create the corresponding `WildMultiAsset` - /// wildcard (`AllOf`) value. - pub fn into_wild(self, fun: WildFungibility) -> WildMultiAsset { - WildMultiAsset::AllOf { fun, id: self } - } -} - -impl Reanchorable for AssetId { - type Error = (); - - fn reanchor( + /// Mutate the asset to represent the same value from the perspective of a new `target` + /// location. The local chain's location is provided in `context`. + pub fn reanchor( &mut self, target: &MultiLocation, context: InteriorMultiLocation, @@ -403,15 +387,15 @@ impl Reanchorable for AssetId { Ok(()) } - fn reanchored( - mut self, - target: &MultiLocation, - context: InteriorMultiLocation, - ) -> Result { - match self.reanchor(target, context) { - Ok(()) => Ok(self), - Err(()) => Err(()), - } + /// Use the value of `self` along with a `fun` fungibility specifier to create the corresponding `MultiAsset` value. + pub fn into_multiasset(self, fun: Fungibility) -> MultiAsset { + MultiAsset { fun, id: self } + } + + /// Use the value of `self` along with a `fun` fungibility specifier to create the corresponding `WildMultiAsset` + /// wildcard (`AllOf`) value. + pub fn into_wild(self, fun: WildFungibility) -> WildMultiAsset { + WildMultiAsset::AllOf { fun, id: self } } } @@ -464,24 +448,9 @@ impl MultiAsset { self.id.prepend_with(prepend) } - /// Returns true if `self` is a super-set of the given `inner` asset. - pub fn contains(&self, inner: &MultiAsset) -> bool { - use Fungibility::*; - if self.id == inner.id { - match (&self.fun, &inner.fun) { - (Fungible(a), Fungible(i)) if a >= i => return true, - (NonFungible(a), NonFungible(i)) if a == i => return true, - _ => (), - } - } - false - } -} - -impl Reanchorable for MultiAsset { - type Error = (); - - fn reanchor( + /// Mutate the location of the asset identifier if concrete, giving it the same location + /// relative to a `target` context. The local context is provided as `context`. + pub fn reanchor( &mut self, target: &MultiLocation, context: InteriorMultiLocation, @@ -489,7 +458,9 @@ impl Reanchorable for MultiAsset { self.id.reanchor(target, context) } - fn reanchored( + /// Mutate the location of the asset identifier if concrete, giving it the same location + /// relative to a `target` context. The local context is provided as `context`. + pub fn reanchored( mut self, target: &MultiLocation, context: InteriorMultiLocation, @@ -497,6 +468,19 @@ impl Reanchorable for MultiAsset { self.id.reanchor(target, context)?; Ok(self) } + + /// Returns true if `self` is a super-set of the given `inner` asset. + pub fn contains(&self, inner: &MultiAsset) -> bool { + use Fungibility::*; + if self.id == inner.id { + match (&self.fun, &inner.fun) { + (Fungible(a), Fungible(i)) if a >= i => return true, + (NonFungible(a), NonFungible(i)) if a == i => return true, + _ => (), + } + } + false + } } impl TryFrom for MultiAsset { diff --git a/xcm/src/v3/multilocation.rs b/xcm/src/v3/multilocation.rs index 270cc2cc3d0d..7a2f3eb0ca33 100644 --- a/xcm/src/v3/multilocation.rs +++ b/xcm/src/v3/multilocation.rs @@ -16,7 +16,7 @@ //! XCM `MultiLocation` datatype. -use super::{traits::Reanchorable, Junction, Junctions}; +use super::{Junction, Junctions}; use crate::{v2::MultiLocation as OldMultiLocation, VersionedMultiLocation}; use core::{ convert::{TryFrom, TryInto}, @@ -385,30 +385,11 @@ impl MultiLocation { } } - /// Remove any unneeded parents/junctions in `self` based on the given context it will be - /// interpreted in. - pub fn simplify(&mut self, context: &Junctions) { - if context.len() < self.parents as usize { - // Not enough context - return - } - while self.parents > 0 { - let maybe = context.at(context.len() - (self.parents as usize)); - match (self.interior.first(), maybe) { - (Some(i), Some(j)) if i == j => { - self.interior.take_first(); - self.parents -= 1; - }, - _ => break, - } - } - } -} - -impl Reanchorable for MultiLocation { - type Error = Self; - - fn reanchor( + /// Mutate `self` so that it represents the same location from the point of view of `target`. + /// The context of `self` is provided as `context`. + /// + /// Does not modify `self` in case of overflow. + pub fn reanchor( &mut self, target: &MultiLocation, context: InteriorMultiLocation, @@ -429,7 +410,11 @@ impl Reanchorable for MultiLocation { Ok(()) } - fn reanchored( + /// Consume `self` and return a new value representing the same location from the point of view + /// of `target`. The context of `self` is provided as `context`. + /// + /// Returns the original `self` in case of overflow. + pub fn reanchored( mut self, target: &MultiLocation, context: InteriorMultiLocation, @@ -439,6 +424,25 @@ impl Reanchorable for MultiLocation { Err(()) => Err(self), } } + + /// Remove any unneeded parents/junctions in `self` based on the given context it will be + /// interpreted in. + pub fn simplify(&mut self, context: &Junctions) { + if context.len() < self.parents as usize { + // Not enough context + return + } + while self.parents > 0 { + let maybe = context.at(context.len() - (self.parents as usize)); + match (self.interior.first(), maybe) { + (Some(i), Some(j)) if i == j => { + self.interior.take_first(); + self.parents -= 1; + }, + _ => break, + } + } + } } impl TryFrom for MultiLocation { diff --git a/xcm/src/v3/traits.rs b/xcm/src/v3/traits.rs index 248909868c56..b752647b0819 100644 --- a/xcm/src/v3/traits.rs +++ b/xcm/src/v3/traits.rs @@ -382,32 +382,6 @@ impl Unwrappable for Option { } } -/// TODO: Add documentation -pub trait Reanchorable: Sized { - /// Type to return in case of an error. - type Error; - - /// Mutate `self` so that it represents the same location from the point of view of `target`. - /// The context of `self` is provided as `context`. - /// - /// Does not modify `self` in case of overflow. - fn reanchor( - &mut self, - target: &MultiLocation, - context: InteriorMultiLocation, - ) -> core::result::Result<(), ()>; - - /// Consume `self` and return a new value representing the same location from the point of view - /// of `target`. The context of `self` is provided as `context`. - /// - /// Returns the original `self` in case of overflow. - fn reanchored( - self, - target: &MultiLocation, - context: InteriorMultiLocation, - ) -> core::result::Result; -} - /// Utility for sending an XCM message to a given location. /// /// These can be amalgamated in tuples to form sophisticated routing systems. In tuple format, each diff --git a/xcm/xcm-executor/src/assets.rs b/xcm/xcm-executor/src/assets.rs index 5a5cd0deb8bc..f5e0659931eb 100644 --- a/xcm/xcm-executor/src/assets.rs +++ b/xcm/xcm-executor/src/assets.rs @@ -23,7 +23,7 @@ use sp_std::{ use xcm::latest::{ AssetId, AssetInstance, Fungibility::{Fungible, NonFungible}, - InteriorMultiLocation, MultiAsset, MultiAssetFilter, MultiAssets, MultiLocation, Reanchorable, + InteriorMultiLocation, MultiAsset, MultiAssetFilter, MultiAssets, MultiLocation, WildFungibility::{Fungible as WildFungible, NonFungible as WildNonFungible}, WildMultiAsset::{All, AllCounted, AllOf, AllOfCounted}, }; diff --git a/xcm/xcm-executor/src/lib.rs b/xcm/xcm-executor/src/lib.rs index 94ab95aafda9..937a91d4a900 100644 --- a/xcm/xcm-executor/src/lib.rs +++ b/xcm/xcm-executor/src/lib.rs @@ -873,7 +873,7 @@ impl XcmExecutor { RequestUnlock { asset, locker } => { let origin = *self.origin_ref().ok_or(XcmError::BadOrigin)?; let remote_asset = Self::try_reanchor(asset.clone(), &locker)?.0; - let remote_target = Self::try_reanchor(origin.clone(), &locker)?.0; + let remote_target = Self::try_reanchor_multilocation(origin.clone(), &locker)?.0; let reduce_ticket = Config::AssetLocker::prepare_reduce_unlockable(locker, asset, origin)?; let msg = @@ -978,18 +978,26 @@ impl XcmExecutor { Config::XcmSender::deliver(ticket).map_err(Into::into) } - fn try_reanchor( - reanchorable: T, + fn try_reanchor( + asset: MultiAsset, destination: &MultiLocation, - ) -> Result<(T, InteriorMultiLocation), XcmError> - where - T: Reanchorable, - { + ) -> Result<(MultiAsset, InteriorMultiLocation), XcmError> { let reanchor_context = Config::UniversalLocation::get(); - let reanchored = reanchorable + let asset = asset + .reanchored(&destination, reanchor_context) + .map_err(|()| XcmError::ReanchorFailed)?; + Ok((asset, reanchor_context)) + } + + fn try_reanchor_multilocation( + location: MultiLocation, + destination: &MultiLocation, + ) -> Result<(MultiLocation, InteriorMultiLocation), XcmError> { + let reanchor_context = Config::UniversalLocation::get(); + let location = location .reanchored(&destination, reanchor_context) .map_err(|_| XcmError::ReanchorFailed)?; - Ok((reanchored, reanchor_context)) + Ok((location, reanchor_context)) } /// NOTE: Any assets which were unable to be reanchored are introduced into `failed_bin`. diff --git a/xcm/xcm-simulator/example/src/lib.rs b/xcm/xcm-simulator/example/src/lib.rs index 800eee51b0aa..ee4d82e3cbbd 100644 --- a/xcm/xcm-simulator/example/src/lib.rs +++ b/xcm/xcm-simulator/example/src/lib.rs @@ -328,8 +328,6 @@ mod tests { }] ); }); - - // TODO: Test case where full lock is unlocked } /// Scenario: From e88b1e1fff06bf276fd6b63406f4dbc91c2244ee Mon Sep 17 00:00:00 2001 From: Francisco Aguirre Date: Fri, 26 May 2023 12:18:09 -0300 Subject: [PATCH 3/4] Fix clippy errors --- xcm/xcm-executor/src/lib.rs | 2 +- xcm/xcm-simulator/example/src/parachain.rs | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/xcm/xcm-executor/src/lib.rs b/xcm/xcm-executor/src/lib.rs index 937a91d4a900..897eb405203a 100644 --- a/xcm/xcm-executor/src/lib.rs +++ b/xcm/xcm-executor/src/lib.rs @@ -873,7 +873,7 @@ impl XcmExecutor { RequestUnlock { asset, locker } => { let origin = *self.origin_ref().ok_or(XcmError::BadOrigin)?; let remote_asset = Self::try_reanchor(asset.clone(), &locker)?.0; - let remote_target = Self::try_reanchor_multilocation(origin.clone(), &locker)?.0; + let remote_target = Self::try_reanchor_multilocation(origin, &locker)?.0; let reduce_ticket = Config::AssetLocker::prepare_reduce_unlockable(locker, asset, origin)?; let msg = diff --git a/xcm/xcm-simulator/example/src/parachain.rs b/xcm/xcm-simulator/example/src/parachain.rs index d10e73dbf6e3..ae24c6307f05 100644 --- a/xcm/xcm-simulator/example/src/parachain.rs +++ b/xcm/xcm-simulator/example/src/parachain.rs @@ -326,7 +326,7 @@ pub mod mock_msg_queue { Ok(xcm) => { let location = (Parent, Parachain(sender.into())); match T::XcmExecutor::execute_xcm(location, xcm, message_hash, max_weight) { - Outcome::Error(e) => (Err(e.clone()), Event::Fail(Some(hash), e)), + Outcome::Error(e) => (Err(e), Event::Fail(Some(hash), e)), Outcome::Complete(w) => (Ok(w), Event::Success(Some(hash))), // As far as the caller is concerned, this was dispatched without error, so // we just report the weight used. @@ -350,7 +350,7 @@ pub mod mock_msg_queue { let _ = XcmpMessageFormat::decode(&mut data_ref) .expect("Simulator encodes with versioned xcm format; qed"); - let mut remaining_fragments = &data_ref[..]; + let mut remaining_fragments = data_ref; while !remaining_fragments.is_empty() { if let Ok(xcm) = VersionedXcm::::decode(&mut remaining_fragments) From 664ab511e28b325164a3ccce80fcf756d4fd3ad7 Mon Sep 17 00:00:00 2001 From: Francisco Aguirre Date: Fri, 26 May 2023 16:03:55 -0300 Subject: [PATCH 4/4] Fix tests --- xcm/xcm-builder/src/tests/locking.rs | 8 ++++++-- xcm/xcm-simulator/example/src/lib.rs | 2 +- 2 files changed, 7 insertions(+), 3 deletions(-) diff --git a/xcm/xcm-builder/src/tests/locking.rs b/xcm/xcm-builder/src/tests/locking.rs index 7c408c999b56..f4ef618ac0e7 100644 --- a/xcm/xcm-builder/src/tests/locking.rs +++ b/xcm/xcm-builder/src/tests/locking.rs @@ -136,6 +136,8 @@ fn remote_unlock_roundtrip_should_work() { set_send_price((Parent, 10u128)); // We have been told by Parachain #1 that Account #3 has locked funds which we can unlock. + // Previously, we must have sent a LockAsset instruction to Parachain #1. + // This caused Parachain #1 to send us the NoteUnlockable instruction. let message = Xcm(vec![NoteUnlockable { asset: (Parent, 100u128).into(), owner: (3u64,).into() }]); let hash = fake_message_hash(&message); @@ -169,8 +171,10 @@ fn remote_unlock_roundtrip_should_work() { assert_eq!(r, Outcome::Complete(Weight::from_parts(40, 40))); assert_eq!(asset_list((3u64,)), vec![(Parent, 990u128).into()]); - let expected_msg = - Xcm::<()>(vec![UnlockAsset { target: (3u64,).into(), asset: (Parent, 100u128).into() }]); + let expected_msg = Xcm::<()>(vec![UnlockAsset { + target: (Parent, Parachain(42), 3u64).into(), + asset: (Parent, 100u128).into(), + }]); let expected_hash = fake_message_hash(&expected_msg); assert_eq!(sent_xcm(), vec![((Parent, Parachain(1)).into(), expected_msg, expected_hash)]); assert_eq!( diff --git a/xcm/xcm-simulator/example/src/lib.rs b/xcm/xcm-simulator/example/src/lib.rs index ee4d82e3cbbd..bd5ebb0b472f 100644 --- a/xcm/xcm-simulator/example/src/lib.rs +++ b/xcm/xcm-simulator/example/src/lib.rs @@ -280,7 +280,7 @@ mod tests { ParaB::execute_with(|| { let message = Xcm(vec![LockAsset { asset: (Here, locked_amount).into(), - unlocker: (Parachain(1),).into(), + unlocker: Parachain(1).into(), }]); assert_ok!(ParachainPalletXcm::send_xcm(Here, Parent, message.clone())); });