From a3b968e45eb3ddd87502dbceba83e3254b907f68 Mon Sep 17 00:00:00 2001
From: Gav Wood <gavin@parity.io>
Date: Mon, 6 Dec 2021 09:21:25 +0000
Subject: [PATCH 01/12] Reanchor should return canonical

---
 xcm/pallet-xcm/src/lib.rs                  |  10 +-
 xcm/src/v1/multiasset.rs                   |  47 +++++---
 xcm/src/v1/multilocation.rs                | 121 ++++++++++++++++++++-
 xcm/xcm-builder/src/location_conversion.rs |   3 +
 xcm/xcm-executor/src/assets.rs             |  31 +++++-
 xcm/xcm-executor/src/lib.rs                |   9 +-
 xcm/xcm-executor/src/traits/conversion.rs  |   1 +
 7 files changed, 194 insertions(+), 28 deletions(-)

diff --git a/xcm/pallet-xcm/src/lib.rs b/xcm/pallet-xcm/src/lib.rs
index 3cb75153b1f6..c6985057c1c9 100644
--- a/xcm/pallet-xcm/src/lib.rs
+++ b/xcm/pallet-xcm/src/lib.rs
@@ -777,13 +777,12 @@ pub mod pallet {
 			let value = (origin_location, assets.drain());
 			ensure!(T::XcmReserveTransferFilter::contains(&value), Error::<T>::Filtered);
 			let (origin_location, assets) = value;
-			let inv_dest = T::LocationInverter::invert_location(&dest)
-				.map_err(|()| Error::<T>::DestinationNotInvertible)?;
+			let ancestry = T::LocationInverter::ancestry();
 			let fees = assets
 				.get(fee_asset_item as usize)
 				.ok_or(Error::<T>::Empty)?
 				.clone()
-				.reanchored(&inv_dest)
+				.reanchored(&dest, &ancestry)
 				.map_err(|_| Error::<T>::CannotReanchor)?;
 			let max_assets = assets.len() as u32;
 			let assets: MultiAssets = assets.into();
@@ -835,13 +834,12 @@ pub mod pallet {
 			let value = (origin_location, assets.drain());
 			ensure!(T::XcmTeleportFilter::contains(&value), Error::<T>::Filtered);
 			let (origin_location, assets) = value;
-			let inv_dest = T::LocationInverter::invert_location(&dest)
-				.map_err(|()| Error::<T>::DestinationNotInvertible)?;
+			let ancestry = T::LocationInverter::ancestry();
 			let fees = assets
 				.get(fee_asset_item as usize)
 				.ok_or(Error::<T>::Empty)?
 				.clone()
-				.reanchored(&inv_dest)
+				.reanchored(&dest, &ancestry)
 				.map_err(|_| Error::<T>::CannotReanchor)?;
 			let max_assets = assets.len() as u32;
 			let assets: MultiAssets = assets.into();
diff --git a/xcm/src/v1/multiasset.rs b/xcm/src/v1/multiasset.rs
index 129c5731542f..6c3d88a22d93 100644
--- a/xcm/src/v1/multiasset.rs
+++ b/xcm/src/v1/multiasset.rs
@@ -116,13 +116,22 @@ impl From<Vec<u8>> for AssetId {
 
 impl AssetId {
 	/// Prepend a `MultiLocation` to a concrete asset, giving it a new root location.
-	pub fn reanchor(&mut self, prepend: &MultiLocation) -> Result<(), ()> {
+	pub fn prepend_with(&mut self, prepend: &MultiLocation) -> Result<(), ()> {
 		if let AssetId::Concrete(ref mut l) = self {
 			l.prepend_with(prepend.clone()).map_err(|_| ())?;
 		}
 		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 `ancestry`.
+	pub fn reanchor(&mut self, target: &MultiLocation, ancestry: &MultiLocation) -> Result<(), ()> {
+		if let AssetId::Concrete(ref mut l) = self {
+			l.reanchor(target, ancestry)?;
+		}
+		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 }
@@ -203,13 +212,20 @@ impl MultiAsset {
 	}
 
 	/// Prepend a `MultiLocation` to a concrete asset, giving it a new root location.
-	pub fn reanchor(&mut self, prepend: &MultiLocation) -> Result<(), ()> {
-		self.id.reanchor(prepend)
+	pub fn prepend_with(&mut self, prepend: &MultiLocation) -> Result<(), ()> {
+		self.id.prepend_with(prepend)
 	}
 
-	/// Prepend a `MultiLocation` to a concrete asset, giving it a new root location.
-	pub fn reanchored(mut self, prepend: &MultiLocation) -> Result<Self, ()> {
-		self.reanchor(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 `ancestry`.
+	pub fn reanchor(&mut self, target: &MultiLocation, ancestry: &MultiLocation) -> Result<(), ()> {
+		self.id.reanchor(target, ancestry)
+	}
+
+	/// 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 `ancestry`.
+	pub fn reanchored(mut self, target: &MultiLocation, ancestry: &MultiLocation) -> Result<Self, ()> {
+		self.id.reanchor(target, ancestry)?;
 		Ok(self)
 	}
 
@@ -413,8 +429,13 @@ impl MultiAssets {
 	}
 
 	/// Prepend a `MultiLocation` to any concrete asset items, giving it a new root location.
-	pub fn reanchor(&mut self, prepend: &MultiLocation) -> Result<(), ()> {
-		self.0.iter_mut().try_for_each(|i| i.reanchor(prepend))
+	pub fn prepend_with(&mut self, prefix: &MultiLocation) -> Result<(), ()> {
+		self.0.iter_mut().try_for_each(|i| i.prepend_with(prefix))
+	}
+
+	/// Prepend a `MultiLocation` to any concrete asset items, giving it a new root location.
+	pub fn reanchor(&mut self, target: &MultiLocation, ancestry: &MultiLocation) -> Result<(), ()> {
+		self.0.iter_mut().try_for_each(|i| i.reanchor(target, ancestry))
 	}
 
 	/// Return a reference to an item at a specific index or `None` if it doesn't exist.
@@ -483,10 +504,10 @@ impl WildMultiAsset {
 	}
 
 	/// Prepend a `MultiLocation` to any concrete asset components, giving it a new root location.
-	pub fn reanchor(&mut self, prepend: &MultiLocation) -> Result<(), ()> {
+	pub fn reanchor(&mut self, target: &MultiLocation, ancestry: &MultiLocation) -> Result<(), ()> {
 		use WildMultiAsset::*;
 		match self {
-			AllOf { ref mut id, .. } => id.reanchor(prepend).map_err(|_| ()),
+			AllOf { ref mut id, .. } => id.reanchor(target, ancestry).map_err(|_| ()),
 			All => Ok(()),
 		}
 	}
@@ -545,10 +566,10 @@ impl MultiAssetFilter {
 	}
 
 	/// Prepend a `MultiLocation` to any concrete asset components, giving it a new root location.
-	pub fn reanchor(&mut self, prepend: &MultiLocation) -> Result<(), ()> {
+	pub fn reanchor(&mut self, target: &MultiLocation, ancestry: &MultiLocation) -> Result<(), ()> {
 		match self {
-			MultiAssetFilter::Definite(ref mut assets) => assets.reanchor(prepend),
-			MultiAssetFilter::Wild(ref mut wild) => wild.reanchor(prepend),
+			MultiAssetFilter::Definite(ref mut assets) => assets.reanchor(target, ancestry),
+			MultiAssetFilter::Wild(ref mut wild) => wild.reanchor(target, ancestry),
 		}
 	}
 }
diff --git a/xcm/src/v1/multilocation.rs b/xcm/src/v1/multilocation.rs
index 9931ba4a6137..a3dee195a123 100644
--- a/xcm/src/v1/multilocation.rs
+++ b/xcm/src/v1/multilocation.rs
@@ -323,6 +323,64 @@ impl MultiLocation {
 		}
 		Ok(())
 	}
+
+	/// Mutate `self` so that it represents the same location from the point of view of `target`.
+	/// The context of `self` is provided as `ancestry`.
+	///
+	/// Does not modify `self` in case of overflow.
+	pub fn reanchor(
+		&mut self,
+		target: &MultiLocation,
+		ancestry: &MultiLocation,
+	) -> Result<(), ()> {
+		// TODO: Optimize this.
+
+		// 1. Use our `ancestry` to figure out how the `target` would address us.
+		let inverted_target = ancestry.inverted(target)?;
+
+		// 2. Prepend `inverted_target` to `self` to get self's location from the perspective of
+		// `target`.
+		self.prepend_with(inverted_target).map_err(|_| ())?;
+
+		// 3. Given that we know some of `target` ancestry, ensure that any parents in `self` are
+		// strictly needed.
+		self.simplify(target.interior());
+
+		Ok(())
+	}
+
+	/// Treating `self` as a context, determine how it would be referenced by a `target` location.
+	pub fn inverted(&self, target: &MultiLocation) -> Result<MultiLocation, ()> {
+		use Junction::OnlyChild;
+		let mut ancestry = self.clone();
+		let mut junctions = Junctions::Here;
+		for _ in 0..target.parent_count() {
+			junctions = junctions
+				.pushed_with(ancestry.interior.take_last().unwrap_or(OnlyChild))
+				.map_err(|_| ())?;
+		}
+		let parents = target.interior().len() as u8;
+		Ok(MultiLocation::new(parents, junctions))
+	}
+
+	/// 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,
+			}
+		}
+	}
 }
 
 /// A unit struct which can be converted into a `MultiLocation` of `parents` value 1.
@@ -773,9 +831,70 @@ impl TryFrom<MultiLocation> for Junctions {
 #[cfg(test)]
 mod tests {
 	use super::{Ancestor, AncestorThen, Junctions::*, MultiLocation, Parent, ParentThen};
-	use crate::opaque::v1::{Junction::*, NetworkId::Any};
+	use crate::opaque::v1::{Junction::*, NetworkId::*};
 	use parity_scale_codec::{Decode, Encode};
 
+	#[test]
+	fn inverted_works() {
+		let ancestry: MultiLocation = (Parachain(1000), PalletInstance(42)).into();
+		let target = (Parent, PalletInstance(69)).into();
+		let expected = (Parent, PalletInstance(42)).into();
+		let inverted = ancestry.inverted(&target).unwrap();
+		assert_eq!(inverted, expected);
+	}
+
+	#[test]
+	fn simplify_basic_works() {
+		let mut location: MultiLocation = (Parent, Parent, Parachain(1000), PalletInstance(42), GeneralIndex(69)).into();
+		let context = X2(Parachain(1000), PalletInstance(42));
+		let expected = GeneralIndex(69).into();
+		location.simplify(&context);
+		assert_eq!(location, expected);
+
+		let mut location: MultiLocation = (Parent, PalletInstance(42), GeneralIndex(69)).into();
+		let context = X1(PalletInstance(42));
+		let expected = GeneralIndex(69).into();
+		location.simplify(&context);
+		assert_eq!(location, expected);
+
+		let mut location: MultiLocation = (Parent, PalletInstance(42), GeneralIndex(69)).into();
+		let context = X2(Parachain(1000), PalletInstance(42));
+		let expected = GeneralIndex(69).into();
+		location.simplify(&context);
+		assert_eq!(location, expected);
+
+		let mut location: MultiLocation = (Parent, Parent, Parachain(1000), PalletInstance(42), GeneralIndex(69)).into();
+		let context = X3(OnlyChild, Parachain(1000), PalletInstance(42));
+		let expected = GeneralIndex(69).into();
+		location.simplify(&context);
+		assert_eq!(location, expected);
+	}
+
+	#[test]
+	fn simplify_incompatible_location_fails() {
+		let mut location: MultiLocation = (Parent, Parent, Parachain(1000), PalletInstance(42), GeneralIndex(69)).into();
+		let context = X3(Parachain(1000), PalletInstance(42), GeneralIndex(42));
+		let expected = (Parent, Parent, Parachain(1000), PalletInstance(42), GeneralIndex(69)).into();
+		location.simplify(&context);
+		assert_eq!(location, expected);
+
+		let mut location: MultiLocation = (Parent, Parent, Parachain(1000), PalletInstance(42), GeneralIndex(69)).into();
+		let context = X1(Parachain(1000));
+		let expected = (Parent, Parent, Parachain(1000), PalletInstance(42), GeneralIndex(69)).into();
+		location.simplify(&context);
+		assert_eq!(location, expected);
+	}
+
+	#[test]
+	fn reanchor_works() {
+		let mut id: MultiLocation = (Parent, Parachain(1000), GeneralIndex(42)).into();
+		let ancestry = Parachain(2000).into();
+		let target = (Parent, Parachain(1000)).into();
+		let expected = GeneralIndex(42).into();
+		id.reanchor(&target, &ancestry).unwrap();
+		assert_eq!(id, expected);
+	}
+
 	#[test]
 	fn encode_and_decode_works() {
 		let m = MultiLocation {
diff --git a/xcm/xcm-builder/src/location_conversion.rs b/xcm/xcm-builder/src/location_conversion.rs
index be6e5aa629c0..a6933b52668f 100644
--- a/xcm/xcm-builder/src/location_conversion.rs
+++ b/xcm/xcm-builder/src/location_conversion.rs
@@ -182,6 +182,9 @@ impl<Network: Get<NetworkId>, AccountId: From<[u8; 20]> + Into<[u8; 20]> + Clone
 /// ```
 pub struct LocationInverter<Ancestry>(PhantomData<Ancestry>);
 impl<Ancestry: Get<MultiLocation>> InvertLocation for LocationInverter<Ancestry> {
+	fn ancestry() -> MultiLocation {
+		Ancestry::get()
+	}
 	fn invert_location(location: &MultiLocation) -> Result<MultiLocation, ()> {
 		let mut ancestry = Ancestry::get();
 		let mut junctions = Here;
diff --git a/xcm/xcm-executor/src/assets.rs b/xcm/xcm-executor/src/assets.rs
index 93a29d7af880..9da8875ad411 100644
--- a/xcm/xcm-executor/src/assets.rs
+++ b/xcm/xcm-executor/src/assets.rs
@@ -194,7 +194,7 @@ impl Assets {
 		self.fungible = fungible
 			.into_iter()
 			.map(|(mut id, amount)| {
-				let _ = id.reanchor(prepend);
+				let _ = id.prepend_with(prepend);
 				(id, amount)
 			})
 			.collect();
@@ -203,7 +203,34 @@ impl Assets {
 		self.non_fungible = non_fungible
 			.into_iter()
 			.map(|(mut class, inst)| {
-				let _ = class.reanchor(prepend);
+				let _ = class.prepend_with(prepend);
+				(class, inst)
+			})
+			.collect();
+	}
+
+	/// Mutate the assets to be interpreted as the same assets from the perspective of a `target`
+	/// chain. The local chain's `ancestry` is provided.
+	///
+	/// WARNING: For now we consider this infallible and swallow any errors. It is thus the caller's
+	/// responsibility to ensure that any internal asset IDs are able to be prepended without
+	/// overflow.
+	pub fn reanchor(&mut self, target: &MultiLocation, ancestry: &MultiLocation) {
+		let mut fungible = Default::default();
+		mem::swap(&mut self.fungible, &mut fungible);
+		self.fungible = fungible
+			.into_iter()
+			.map(|(mut id, amount)| {
+				let _ = id.reanchor(target, ancestry);
+				(id, amount)
+			})
+			.collect();
+		let mut non_fungible = Default::default();
+		mem::swap(&mut self.non_fungible, &mut non_fungible);
+		self.non_fungible = non_fungible
+			.into_iter()
+			.map(|(mut class, inst)| {
+				let _ = class.reanchor(target, ancestry);
 				(class, inst)
 			})
 			.collect();
diff --git a/xcm/xcm-executor/src/lib.rs b/xcm/xcm-executor/src/lib.rs
index cd6060aebb79..ff672fba1cc0 100644
--- a/xcm/xcm-executor/src/lib.rs
+++ b/xcm/xcm-executor/src/lib.rs
@@ -304,12 +304,11 @@ impl<Config: config::Config> XcmExecutor<Config> {
 			TransferReserveAsset { mut assets, dest, xcm } => {
 				let origin = self.origin.as_ref().ok_or(XcmError::BadOrigin)?;
 				// Take `assets` from the origin account (on-chain) and place into dest account.
-				let inv_dest = Config::LocationInverter::invert_location(&dest)
-					.map_err(|()| XcmError::MultiLocationNotInvertible)?;
 				for asset in assets.inner() {
 					Config::AssetTransactor::beam_asset(asset, origin, &dest)?;
 				}
-				assets.reanchor(&inv_dest).map_err(|()| XcmError::MultiLocationFull)?;
+				let ancestry = Config::LocationInverter::ancestry();
+				assets.reanchor(&dest, &ancestry).map_err(|()| XcmError::MultiLocationFull)?;
 				let mut message = vec![ReserveAssetDeposited(assets), ClearOrigin];
 				message.extend(xcm.0.into_iter());
 				Config::XcmSender::send_xcm(dest, Xcm(message)).map_err(Into::into)
@@ -498,9 +497,7 @@ impl<Config: config::Config> XcmExecutor<Config> {
 	}
 
 	fn reanchored(mut assets: Assets, dest: &MultiLocation) -> Result<MultiAssets, XcmError> {
-		let inv_dest = Config::LocationInverter::invert_location(&dest)
-			.map_err(|()| XcmError::MultiLocationNotInvertible)?;
-		assets.prepend_location(&inv_dest);
+		assets.reanchor(dest, &Config::LocationInverter::ancestry());
 		Ok(assets.into_assets_iter().collect::<Vec<_>>().into())
 	}
 }
diff --git a/xcm/xcm-executor/src/traits/conversion.rs b/xcm/xcm-executor/src/traits/conversion.rs
index dbfb2d6a5fad..d8c50037662d 100644
--- a/xcm/xcm-executor/src/traits/conversion.rs
+++ b/xcm/xcm-executor/src/traits/conversion.rs
@@ -207,5 +207,6 @@ impl<O> ConvertOrigin<O> 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`.
 pub trait InvertLocation {
+	fn ancestry() -> MultiLocation;
 	fn invert_location(l: &MultiLocation) -> Result<MultiLocation, ()>;
 }

From db7cd5fce3834d69e28d76d435dddaf02b058470 Mon Sep 17 00:00:00 2001
From: Gav Wood <gavin@parity.io>
Date: Mon, 6 Dec 2021 12:38:28 +0000
Subject: [PATCH 02/12] Formatting

---
 xcm/src/v1/multiasset.rs    |  6 +++++-
 xcm/src/v1/multilocation.rs | 24 +++++++++++++-----------
 2 files changed, 18 insertions(+), 12 deletions(-)

diff --git a/xcm/src/v1/multiasset.rs b/xcm/src/v1/multiasset.rs
index 6c3d88a22d93..2014ed9ef875 100644
--- a/xcm/src/v1/multiasset.rs
+++ b/xcm/src/v1/multiasset.rs
@@ -224,7 +224,11 @@ impl MultiAsset {
 
 	/// 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 `ancestry`.
-	pub fn reanchored(mut self, target: &MultiLocation, ancestry: &MultiLocation) -> Result<Self, ()> {
+	pub fn reanchored(
+		mut self,
+		target: &MultiLocation,
+		ancestry: &MultiLocation,
+	) -> Result<Self, ()> {
 		self.id.reanchor(target, ancestry)?;
 		Ok(self)
 	}
diff --git a/xcm/src/v1/multilocation.rs b/xcm/src/v1/multilocation.rs
index a3dee195a123..71478645f6f2 100644
--- a/xcm/src/v1/multilocation.rs
+++ b/xcm/src/v1/multilocation.rs
@@ -328,11 +328,7 @@ impl MultiLocation {
 	/// The context of `self` is provided as `ancestry`.
 	///
 	/// Does not modify `self` in case of overflow.
-	pub fn reanchor(
-		&mut self,
-		target: &MultiLocation,
-		ancestry: &MultiLocation,
-	) -> Result<(), ()> {
+	pub fn reanchor(&mut self, target: &MultiLocation, ancestry: &MultiLocation) -> Result<(), ()> {
 		// TODO: Optimize this.
 
 		// 1. Use our `ancestry` to figure out how the `target` would address us.
@@ -845,7 +841,8 @@ mod tests {
 
 	#[test]
 	fn simplify_basic_works() {
-		let mut location: MultiLocation = (Parent, Parent, Parachain(1000), PalletInstance(42), GeneralIndex(69)).into();
+		let mut location: MultiLocation =
+			(Parent, Parent, Parachain(1000), PalletInstance(42), GeneralIndex(69)).into();
 		let context = X2(Parachain(1000), PalletInstance(42));
 		let expected = GeneralIndex(69).into();
 		location.simplify(&context);
@@ -863,7 +860,8 @@ mod tests {
 		location.simplify(&context);
 		assert_eq!(location, expected);
 
-		let mut location: MultiLocation = (Parent, Parent, Parachain(1000), PalletInstance(42), GeneralIndex(69)).into();
+		let mut location: MultiLocation =
+			(Parent, Parent, Parachain(1000), PalletInstance(42), GeneralIndex(69)).into();
 		let context = X3(OnlyChild, Parachain(1000), PalletInstance(42));
 		let expected = GeneralIndex(69).into();
 		location.simplify(&context);
@@ -872,15 +870,19 @@ mod tests {
 
 	#[test]
 	fn simplify_incompatible_location_fails() {
-		let mut location: MultiLocation = (Parent, Parent, Parachain(1000), PalletInstance(42), GeneralIndex(69)).into();
+		let mut location: MultiLocation =
+			(Parent, Parent, Parachain(1000), PalletInstance(42), GeneralIndex(69)).into();
 		let context = X3(Parachain(1000), PalletInstance(42), GeneralIndex(42));
-		let expected = (Parent, Parent, Parachain(1000), PalletInstance(42), GeneralIndex(69)).into();
+		let expected =
+			(Parent, Parent, Parachain(1000), PalletInstance(42), GeneralIndex(69)).into();
 		location.simplify(&context);
 		assert_eq!(location, expected);
 
-		let mut location: MultiLocation = (Parent, Parent, Parachain(1000), PalletInstance(42), GeneralIndex(69)).into();
+		let mut location: MultiLocation =
+			(Parent, Parent, Parachain(1000), PalletInstance(42), GeneralIndex(69)).into();
 		let context = X1(Parachain(1000));
-		let expected = (Parent, Parent, Parachain(1000), PalletInstance(42), GeneralIndex(69)).into();
+		let expected =
+			(Parent, Parent, Parachain(1000), PalletInstance(42), GeneralIndex(69)).into();
 		location.simplify(&context);
 		assert_eq!(location, expected);
 	}

From 55da4eaef16c44967dfe3624fa74900a7c7df68e Mon Sep 17 00:00:00 2001
From: Gav Wood <gavin@parity.io>
Date: Mon, 6 Dec 2021 12:40:27 +0000
Subject: [PATCH 03/12] Formatting

---
 runtime/test-runtime/src/xcm_config.rs | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/runtime/test-runtime/src/xcm_config.rs b/runtime/test-runtime/src/xcm_config.rs
index 3a3c762c6b03..2faf3ffd0606 100644
--- a/runtime/test-runtime/src/xcm_config.rs
+++ b/runtime/test-runtime/src/xcm_config.rs
@@ -71,6 +71,9 @@ impl InvertLocation for InvertNothing {
 	fn invert_location(_: &MultiLocation) -> sp_std::result::Result<MultiLocation, ()> {
 		Ok(Here.into())
 	}
+	fn ancestry() -> MultiLocation {
+		Here.into()
+	}
 }
 
 pub struct XcmConfig;

From b441587af0ba77af6a1982c365a6bf68052c285c Mon Sep 17 00:00:00 2001
From: Gavin Wood <gavin@parity.io>
Date: Tue, 7 Dec 2021 17:19:31 +0100
Subject: [PATCH 04/12] Update xcm/src/v1/multilocation.rs

---
 xcm/src/v1/multilocation.rs | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/xcm/src/v1/multilocation.rs b/xcm/src/v1/multilocation.rs
index 71478645f6f2..43a62462fab9 100644
--- a/xcm/src/v1/multilocation.rs
+++ b/xcm/src/v1/multilocation.rs
@@ -837,6 +837,12 @@ mod tests {
 		let expected = (Parent, PalletInstance(42)).into();
 		let inverted = ancestry.inverted(&target).unwrap();
 		assert_eq!(inverted, expected);
+		
+		let ancestry: MultiLocation = (Parachain(1000), PalletInstance(42), GeneralIndex(1)).into();
+		let target = (Parent, Parent, PalletInstance(69), GeneralIndex(2)).into();
+		let expected = (Parent, Parent, PalletInstance(42), GeneralIndex(1)).into();
+		let inverted = ancestry.inverted(&target).unwrap();
+		assert_eq!(inverted, expected);
 	}
 
 	#[test]

From 95e6babdd7994507f179657f3727d5e3e1c16078 Mon Sep 17 00:00:00 2001
From: Gav Wood <gavin@parity.io>
Date: Tue, 7 Dec 2021 17:24:31 +0100
Subject: [PATCH 05/12] Formatting

---
 xcm/src/v1/multilocation.rs | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/xcm/src/v1/multilocation.rs b/xcm/src/v1/multilocation.rs
index 43a62462fab9..d32baf623f61 100644
--- a/xcm/src/v1/multilocation.rs
+++ b/xcm/src/v1/multilocation.rs
@@ -837,7 +837,7 @@ mod tests {
 		let expected = (Parent, PalletInstance(42)).into();
 		let inverted = ancestry.inverted(&target).unwrap();
 		assert_eq!(inverted, expected);
-		
+
 		let ancestry: MultiLocation = (Parachain(1000), PalletInstance(42), GeneralIndex(1)).into();
 		let target = (Parent, Parent, PalletInstance(69), GeneralIndex(2)).into();
 		let expected = (Parent, Parent, PalletInstance(42), GeneralIndex(1)).into();

From 81e2396b4afaea53c29e3f03c22eb5b42bf691c1 Mon Sep 17 00:00:00 2001
From: Gav Wood <gavin@parity.io>
Date: Wed, 8 Dec 2021 12:06:21 +0100
Subject: [PATCH 06/12] Fixes

---
 xcm/src/v1/multilocation.rs | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/xcm/src/v1/multilocation.rs b/xcm/src/v1/multilocation.rs
index d32baf623f61..ee0238e16852 100644
--- a/xcm/src/v1/multilocation.rs
+++ b/xcm/src/v1/multilocation.rs
@@ -352,7 +352,7 @@ impl MultiLocation {
 		let mut junctions = Junctions::Here;
 		for _ in 0..target.parent_count() {
 			junctions = junctions
-				.pushed_with(ancestry.interior.take_last().unwrap_or(OnlyChild))
+				.pushed_front_with(ancestry.interior.take_last().unwrap_or(OnlyChild))
 				.map_err(|_| ())?;
 		}
 		let parents = target.interior().len() as u8;

From 2a4681629a8125bb0d5fb2d3b2cc4d7fb4b1327a Mon Sep 17 00:00:00 2001
From: Gav Wood <gavin@parity.io>
Date: Wed, 8 Dec 2021 12:39:52 +0100
Subject: [PATCH 07/12] Don't discard unreanchorable assets

---
 xcm/xcm-executor/src/assets.rs | 29 ++++++++++++++++++++++-------
 xcm/xcm-executor/src/lib.rs    | 18 +++++++++++-------
 2 files changed, 33 insertions(+), 14 deletions(-)

diff --git a/xcm/xcm-executor/src/assets.rs b/xcm/xcm-executor/src/assets.rs
index 9da8875ad411..0e24df738a17 100644
--- a/xcm/xcm-executor/src/assets.rs
+++ b/xcm/xcm-executor/src/assets.rs
@@ -215,23 +215,38 @@ impl Assets {
 	/// WARNING: For now we consider this infallible and swallow any errors. It is thus the caller's
 	/// responsibility to ensure that any internal asset IDs are able to be prepended without
 	/// overflow.
-	pub fn reanchor(&mut self, target: &MultiLocation, ancestry: &MultiLocation) {
+	pub fn reanchor(
+		&mut self,
+		target: &MultiLocation,
+		ancestry: &MultiLocation,
+		failed_bin: &mut Self,
+	) {
 		let mut fungible = Default::default();
 		mem::swap(&mut self.fungible, &mut fungible);
 		self.fungible = fungible
 			.into_iter()
-			.map(|(mut id, amount)| {
-				let _ = id.reanchor(target, ancestry);
-				(id, amount)
+			.filter_map(|(mut id, amount)| {
+				match id.reanchor(target, ancestry) {
+					Ok(()) => Some((id, amount)),
+					Err(()) => {
+						failed_bin.fungible.insert(id, amount);
+						None
+					}
+				}
 			})
 			.collect();
 		let mut non_fungible = Default::default();
 		mem::swap(&mut self.non_fungible, &mut non_fungible);
 		self.non_fungible = non_fungible
 			.into_iter()
-			.map(|(mut class, inst)| {
-				let _ = class.reanchor(target, ancestry);
-				(class, inst)
+			.filter_map(|(mut class, inst)| {
+				match class.reanchor(target, ancestry) {
+					Ok(()) => Some((class, inst)),
+					Err(()) => {
+						failed_bin.non_fungible.insert((class, inst));
+						None
+					}
+				}
 			})
 			.collect();
 	}
diff --git a/xcm/xcm-executor/src/lib.rs b/xcm/xcm-executor/src/lib.rs
index ff672fba1cc0..bc514e0f416f 100644
--- a/xcm/xcm-executor/src/lib.rs
+++ b/xcm/xcm-executor/src/lib.rs
@@ -400,13 +400,13 @@ impl<Config: config::Config> XcmExecutor<Config> {
 				for asset in deposited.assets_iter() {
 					Config::AssetTransactor::deposit_asset(&asset, &dest)?;
 				}
-				let assets = Self::reanchored(deposited, &dest)?;
+				let assets = Self::reanchored(deposited, &dest, &mut self.holding);
 				let mut message = vec![ReserveAssetDeposited(assets), ClearOrigin];
 				message.extend(xcm.0.into_iter());
 				Config::XcmSender::send_xcm(dest, Xcm(message)).map_err(Into::into)
 			},
 			InitiateReserveWithdraw { assets, reserve, xcm } => {
-				let assets = Self::reanchored(self.holding.saturating_take(assets), &reserve)?;
+				let assets = Self::reanchored(self.holding.saturating_take(assets), &reserve, &mut self.holding);
 				let mut message = vec![WithdrawAsset(assets), ClearOrigin];
 				message.extend(xcm.0.into_iter());
 				Config::XcmSender::send_xcm(reserve, Xcm(message)).map_err(Into::into)
@@ -417,13 +417,13 @@ impl<Config: config::Config> XcmExecutor<Config> {
 				for asset in assets.assets_iter() {
 					Config::AssetTransactor::check_out(&dest, &asset);
 				}
-				let assets = Self::reanchored(assets, &dest)?;
+				let assets = Self::reanchored(assets, &dest, &mut self.holding);
 				let mut message = vec![ReceiveTeleportedAsset(assets), ClearOrigin];
 				message.extend(xcm.0.into_iter());
 				Config::XcmSender::send_xcm(dest, Xcm(message)).map_err(Into::into)
 			},
 			QueryHolding { query_id, dest, assets, max_response_weight } => {
-				let assets = Self::reanchored(self.holding.min(&assets), &dest)?;
+				let assets = Self::reanchored(self.holding.min(&assets), &dest, &mut self.holding);
 				let max_weight = max_response_weight;
 				let response = Response::Assets(assets);
 				let instruction = QueryResponse { query_id, response, max_weight };
@@ -496,8 +496,12 @@ impl<Config: config::Config> XcmExecutor<Config> {
 		}
 	}
 
-	fn reanchored(mut assets: Assets, dest: &MultiLocation) -> Result<MultiAssets, XcmError> {
-		assets.reanchor(dest, &Config::LocationInverter::ancestry());
-		Ok(assets.into_assets_iter().collect::<Vec<_>>().into())
+	fn reanchored(
+		mut assets: Assets,
+		dest: &MultiLocation,
+		failed_bin: &mut Assets,
+	) -> MultiAssets {
+		assets.reanchor(dest, &Config::LocationInverter::ancestry(), failed_bin);
+		assets.into_assets_iter().collect::<Vec<_>>().into()
 	}
 }

From cf270f250c09bf3226f9702e4d0d8214746b221f Mon Sep 17 00:00:00 2001
From: Gav Wood <gavin@parity.io>
Date: Wed, 8 Dec 2021 12:39:59 +0100
Subject: [PATCH 08/12] Formatting

---
 xcm/xcm-executor/src/assets.rs | 28 ++++++++++++----------------
 xcm/xcm-executor/src/lib.rs    |  6 +++++-
 2 files changed, 17 insertions(+), 17 deletions(-)

diff --git a/xcm/xcm-executor/src/assets.rs b/xcm/xcm-executor/src/assets.rs
index 0e24df738a17..8ef07b5ee9bc 100644
--- a/xcm/xcm-executor/src/assets.rs
+++ b/xcm/xcm-executor/src/assets.rs
@@ -225,28 +225,24 @@ impl Assets {
 		mem::swap(&mut self.fungible, &mut fungible);
 		self.fungible = fungible
 			.into_iter()
-			.filter_map(|(mut id, amount)| {
-				match id.reanchor(target, ancestry) {
-					Ok(()) => Some((id, amount)),
-					Err(()) => {
-						failed_bin.fungible.insert(id, amount);
-						None
-					}
-				}
+			.filter_map(|(mut id, amount)| match id.reanchor(target, ancestry) {
+				Ok(()) => Some((id, amount)),
+				Err(()) => {
+					failed_bin.fungible.insert(id, amount);
+					None
+				},
 			})
 			.collect();
 		let mut non_fungible = Default::default();
 		mem::swap(&mut self.non_fungible, &mut non_fungible);
 		self.non_fungible = non_fungible
 			.into_iter()
-			.filter_map(|(mut class, inst)| {
-				match class.reanchor(target, ancestry) {
-					Ok(()) => Some((class, inst)),
-					Err(()) => {
-						failed_bin.non_fungible.insert((class, inst));
-						None
-					}
-				}
+			.filter_map(|(mut class, inst)| match class.reanchor(target, ancestry) {
+				Ok(()) => Some((class, inst)),
+				Err(()) => {
+					failed_bin.non_fungible.insert((class, inst));
+					None
+				},
 			})
 			.collect();
 	}
diff --git a/xcm/xcm-executor/src/lib.rs b/xcm/xcm-executor/src/lib.rs
index bc514e0f416f..64aa7912e76b 100644
--- a/xcm/xcm-executor/src/lib.rs
+++ b/xcm/xcm-executor/src/lib.rs
@@ -406,7 +406,11 @@ impl<Config: config::Config> XcmExecutor<Config> {
 				Config::XcmSender::send_xcm(dest, Xcm(message)).map_err(Into::into)
 			},
 			InitiateReserveWithdraw { assets, reserve, xcm } => {
-				let assets = Self::reanchored(self.holding.saturating_take(assets), &reserve, &mut self.holding);
+				let assets = Self::reanchored(
+					self.holding.saturating_take(assets),
+					&reserve,
+					&mut self.holding,
+				);
 				let mut message = vec![WithdrawAsset(assets), ClearOrigin];
 				message.extend(xcm.0.into_iter());
 				Config::XcmSender::send_xcm(reserve, Xcm(message)).map_err(Into::into)

From 6306c849fd84ba45df1655fc1f2da3ebb80b6aa4 Mon Sep 17 00:00:00 2001
From: Gav Wood <gavin@parity.io>
Date: Wed, 8 Dec 2021 12:44:17 +0100
Subject: [PATCH 09/12] Docs

---
 xcm/src/v1/multilocation.rs | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/xcm/src/v1/multilocation.rs b/xcm/src/v1/multilocation.rs
index ee0238e16852..12b507329215 100644
--- a/xcm/src/v1/multilocation.rs
+++ b/xcm/src/v1/multilocation.rs
@@ -329,7 +329,7 @@ impl MultiLocation {
 	///
 	/// Does not modify `self` in case of overflow.
 	pub fn reanchor(&mut self, target: &MultiLocation, ancestry: &MultiLocation) -> Result<(), ()> {
-		// TODO: Optimize this.
+		// TODO: https://github.com/paritytech/polkadot/issues/4489 Optimize this.
 
 		// 1. Use our `ancestry` to figure out how the `target` would address us.
 		let inverted_target = ancestry.inverted(target)?;

From 915b5d759a4ac70a3906b34d476a95dcc1c6c29b Mon Sep 17 00:00:00 2001
From: Gav Wood <gavin@parity.io>
Date: Wed, 8 Dec 2021 13:26:14 +0100
Subject: [PATCH 10/12] Fixes

---
 xcm/xcm-executor/src/assets.rs | 17 +++++++++++------
 xcm/xcm-executor/src/lib.rs    | 21 +++++++++++++++------
 2 files changed, 26 insertions(+), 12 deletions(-)

diff --git a/xcm/xcm-executor/src/assets.rs b/xcm/xcm-executor/src/assets.rs
index 8ef07b5ee9bc..b00c5579d9b2 100644
--- a/xcm/xcm-executor/src/assets.rs
+++ b/xcm/xcm-executor/src/assets.rs
@@ -212,23 +212,25 @@ impl Assets {
 	/// Mutate the assets to be interpreted as the same assets from the perspective of a `target`
 	/// chain. The local chain's `ancestry` is provided.
 	///
-	/// WARNING: For now we consider this infallible and swallow any errors. It is thus the caller's
-	/// responsibility to ensure that any internal asset IDs are able to be prepended without
-	/// overflow.
+	/// Any assets which were unable to be reanchored are introduced into `failed_bin`.
 	pub fn reanchor(
 		&mut self,
 		target: &MultiLocation,
 		ancestry: &MultiLocation,
-		failed_bin: &mut Self,
+		mut maybe_failed_bin: Option<&mut Self>,
 	) {
 		let mut fungible = Default::default();
 		mem::swap(&mut self.fungible, &mut fungible);
+		let maybe_failed_bin_ref = &mut maybe_failed_bin;
 		self.fungible = fungible
 			.into_iter()
 			.filter_map(|(mut id, amount)| match id.reanchor(target, ancestry) {
 				Ok(()) => Some((id, amount)),
 				Err(()) => {
-					failed_bin.fungible.insert(id, amount);
+					*maybe_failed_bin_ref = maybe_failed_bin_ref.take().map(|failed| {
+						failed.fungible.insert(id, amount);
+						failed
+					});
 					None
 				},
 			})
@@ -240,7 +242,10 @@ impl Assets {
 			.filter_map(|(mut class, inst)| match class.reanchor(target, ancestry) {
 				Ok(()) => Some((class, inst)),
 				Err(()) => {
-					failed_bin.non_fungible.insert((class, inst));
+					*maybe_failed_bin_ref = maybe_failed_bin_ref.take().map(|failed| {
+						failed.non_fungible.insert((class, inst));
+						failed
+					});
 					None
 				},
 			})
diff --git a/xcm/xcm-executor/src/lib.rs b/xcm/xcm-executor/src/lib.rs
index 64aa7912e76b..f57e72f2ac7d 100644
--- a/xcm/xcm-executor/src/lib.rs
+++ b/xcm/xcm-executor/src/lib.rs
@@ -400,16 +400,20 @@ impl<Config: config::Config> XcmExecutor<Config> {
 				for asset in deposited.assets_iter() {
 					Config::AssetTransactor::deposit_asset(&asset, &dest)?;
 				}
-				let assets = Self::reanchored(deposited, &dest, &mut self.holding);
+				// Note that we pass `None` as `maybe_failed_bin` and drop any assets which cannot
+				// be reanchored  because we have already called `deposit_asset` on all assets.
+				let assets = Self::reanchored(deposited, &dest, None);
 				let mut message = vec![ReserveAssetDeposited(assets), ClearOrigin];
 				message.extend(xcm.0.into_iter());
 				Config::XcmSender::send_xcm(dest, Xcm(message)).map_err(Into::into)
 			},
 			InitiateReserveWithdraw { assets, reserve, xcm } => {
+				// Note that here we are able to place any assets which could not be reanchored
+				// back into Holding.
 				let assets = Self::reanchored(
 					self.holding.saturating_take(assets),
 					&reserve,
-					&mut self.holding,
+					Some(&mut self.holding),
 				);
 				let mut message = vec![WithdrawAsset(assets), ClearOrigin];
 				message.extend(xcm.0.into_iter());
@@ -421,13 +425,17 @@ impl<Config: config::Config> XcmExecutor<Config> {
 				for asset in assets.assets_iter() {
 					Config::AssetTransactor::check_out(&dest, &asset);
 				}
-				let assets = Self::reanchored(assets, &dest, &mut self.holding);
+				// Note that we pass `None` as `maybe_failed_bin` and drop any assets which cannot
+				// be reanchored  because we have already checked all assets out.
+				let assets = Self::reanchored(assets, &dest, None);
 				let mut message = vec![ReceiveTeleportedAsset(assets), ClearOrigin];
 				message.extend(xcm.0.into_iter());
 				Config::XcmSender::send_xcm(dest, Xcm(message)).map_err(Into::into)
 			},
 			QueryHolding { query_id, dest, assets, max_response_weight } => {
-				let assets = Self::reanchored(self.holding.min(&assets), &dest, &mut self.holding);
+				// Note that we pass `None` as `maybe_failed_bin` since no assets were ever removed
+				// from Holding.
+				let assets = Self::reanchored(self.holding.min(&assets), &dest, None);
 				let max_weight = max_response_weight;
 				let response = Response::Assets(assets);
 				let instruction = QueryResponse { query_id, response, max_weight };
@@ -500,12 +508,13 @@ impl<Config: config::Config> XcmExecutor<Config> {
 		}
 	}
 
+	/// NOTE: Any assets which were unable to be reanchored are introduced into `failed_bin`.
 	fn reanchored(
 		mut assets: Assets,
 		dest: &MultiLocation,
-		failed_bin: &mut Assets,
+		maybe_failed_bin: Option<&mut Assets>,
 	) -> MultiAssets {
-		assets.reanchor(dest, &Config::LocationInverter::ancestry(), failed_bin);
+		assets.reanchor(dest, &Config::LocationInverter::ancestry(), maybe_failed_bin);
 		assets.into_assets_iter().collect::<Vec<_>>().into()
 	}
 }

From b4b1cbb0355056d82049b7a9dc17bffa61672481 Mon Sep 17 00:00:00 2001
From: Gav Wood <gavin@parity.io>
Date: Wed, 8 Dec 2021 13:45:39 +0100
Subject: [PATCH 11/12] Fixes

---
 xcm/xcm-executor/src/assets.rs | 14 ++++++--------
 1 file changed, 6 insertions(+), 8 deletions(-)

diff --git a/xcm/xcm-executor/src/assets.rs b/xcm/xcm-executor/src/assets.rs
index b00c5579d9b2..64e144576580 100644
--- a/xcm/xcm-executor/src/assets.rs
+++ b/xcm/xcm-executor/src/assets.rs
@@ -227,10 +227,9 @@ impl Assets {
 			.filter_map(|(mut id, amount)| match id.reanchor(target, ancestry) {
 				Ok(()) => Some((id, amount)),
 				Err(()) => {
-					*maybe_failed_bin_ref = maybe_failed_bin_ref.take().map(|failed| {
-						failed.fungible.insert(id, amount);
-						failed
-					});
+					if let Some(ref mut failed) = maybe_failed_bin_ref {
+						(*failed).fungible.insert(id, amount);
+					}
 					None
 				},
 			})
@@ -242,10 +241,9 @@ impl Assets {
 			.filter_map(|(mut class, inst)| match class.reanchor(target, ancestry) {
 				Ok(()) => Some((class, inst)),
 				Err(()) => {
-					*maybe_failed_bin_ref = maybe_failed_bin_ref.take().map(|failed| {
-						failed.non_fungible.insert((class, inst));
-						failed
-					});
+					if let Some(ref mut failed) = maybe_failed_bin_ref {
+						(*failed).non_fungible.insert((class, inst));
+					}
 					None
 				},
 			})

From 0dc5a288dcf42ba2c2d6a5862e26a54b3b8f5b2d Mon Sep 17 00:00:00 2001
From: Gav Wood <gavin@parity.io>
Date: Wed, 8 Dec 2021 14:35:50 +0100
Subject: [PATCH 12/12] tidy

---
 xcm/xcm-executor/src/assets.rs | 9 ++-------
 1 file changed, 2 insertions(+), 7 deletions(-)

diff --git a/xcm/xcm-executor/src/assets.rs b/xcm/xcm-executor/src/assets.rs
index 64e144576580..324e92dce9ff 100644
--- a/xcm/xcm-executor/src/assets.rs
+++ b/xcm/xcm-executor/src/assets.rs
@@ -221,15 +221,12 @@ impl Assets {
 	) {
 		let mut fungible = Default::default();
 		mem::swap(&mut self.fungible, &mut fungible);
-		let maybe_failed_bin_ref = &mut maybe_failed_bin;
 		self.fungible = fungible
 			.into_iter()
 			.filter_map(|(mut id, amount)| match id.reanchor(target, ancestry) {
 				Ok(()) => Some((id, amount)),
 				Err(()) => {
-					if let Some(ref mut failed) = maybe_failed_bin_ref {
-						(*failed).fungible.insert(id, amount);
-					}
+					maybe_failed_bin.as_mut().map(|f| f.fungible.insert(id, amount));
 					None
 				},
 			})
@@ -241,9 +238,7 @@ impl Assets {
 			.filter_map(|(mut class, inst)| match class.reanchor(target, ancestry) {
 				Ok(()) => Some((class, inst)),
 				Err(()) => {
-					if let Some(ref mut failed) = maybe_failed_bin_ref {
-						(*failed).non_fungible.insert((class, inst));
-					}
+					maybe_failed_bin.as_mut().map(|f| f.non_fungible.insert((class, inst)));
 					None
 				},
 			})