Optimize multilocation reanchoring logic in XCM

xcm/src/v1/multilocation.rs

@@ -329,36 +329,21 @@ impl MultiLocation {
 	///
 	/// Does not modify `self` in case of overflow.
 	pub fn reanchor(&mut self, target: &MultiLocation, ancestry: &MultiLocation) -> Result<(), ()> {
-		// TODO: https://github.com/paritytech/polkadot/issues/4489 Optimize this.
+		let mut target = target.clone();
 
-		// 1. Use our `ancestry` to figure out how the `target` would address us.
-		let inverted_target = ancestry.inverted(target)?;
+		self.prepend_with(ancestry.clone()).map_err(|_| ())?;
+		target.prepend_with(ancestry.clone()).map_err(|_| ())?;
 
-		// 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());
+		// Remove common parents between now normalized self and target
+		while target.interior().first() == self.interior().first() {
+			target.take_first_interior();
+			self.take_first_interior();
+		}
 
+		self.parents = self.parent_count() + target.parent_count() + target.interior().len() as u8;
 		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_front_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) {
@@ -830,21 +815,6 @@ mod tests {
 	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);
-
-		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]
 	fn simplify_basic_works() {
 		let mut location: MultiLocation =
@@ -903,6 +873,119 @@ mod tests {
 		assert_eq!(id, expected);
 	}
 
+	#[test]
+	fn reanchor_where_target_deeper_than_id_but_expected_is_same_branch() {
+		let mut id: MultiLocation = (Parent, Parachain(1000)).into();
+		let ancestry = Parachain(2000).into();
+		let target = (Parent, Parachain(1000), GeneralIndex(42)).into();
+		let expected = (Parent, Here).into();
+		id.reanchor(&target, &ancestry).unwrap();
+		assert_eq!(id, expected);
+	}
+
+	#[test]
+	fn reanchor_where_target_on_same_branch_as_id_with_equal_path_length() {
+		let mut id: MultiLocation = (Parent, Parachain(1000), GeneralIndex(42)).into();
+		let ancestry = Parachain(2000).into();
+		let target = (Parent, Parachain(1000), Parachain(3000)).into();
+		let expected = (Parent, GeneralIndex(42)).into();
+		id.reanchor(&target, &ancestry).unwrap();
+		assert_eq!(id, expected);
+	}
+
+	#[test]
+	fn reanchor_where_target_deeper_than_id_but_on_same_branch() {
+		// Test where the target is on the same branch as the current id (target deeper), longer example.
+		let mut id: MultiLocation = (Parent, Parachain(1000), GeneralIndex(42)).into();
+		let ancestry = Parachain(2000).into();
+		let target = (Parent, Parachain(1000), Parachain(3000), Parachain(4000)).into();
+		let expected = (Parent, Parent, GeneralIndex(42)).into();
+		id.reanchor(&target, &ancestry).unwrap();
+		assert_eq!(id, expected);
+	}
+
+	#[test]
+	fn reanchor_where_target_shallower_than_id_but_on_same_branch() {
+		let mut id: MultiLocation =
+			(Parent, Parachain(1000), Parachain(4000), GeneralIndex(42)).into();
+		let ancestry = Parachain(2000).into();
+		let target = (Parent, Parachain(1000), Parachain(3000)).into();
+		let expected = (Parent, Parachain(4000), GeneralIndex(42)).into();
+		id.reanchor(&target, &ancestry).unwrap();
+		assert_eq!(id, expected);
+	}
+
+	#[test]
+	fn reanchor_where_target_and_current_id_on_different_branches() {
+		let mut id: MultiLocation = (Parent, Parachain(4000), GeneralIndex(42)).into();
+		let ancestry = Parachain(2000).into();
+		let target = (Parent, Parachain(1000), Parachain(3000)).into();
+		let expected = (Parent, Parent, Parachain(4000), GeneralIndex(42)).into();
+		id.reanchor(&target, &ancestry).unwrap();
+		assert_eq!(id, expected);
+	}
+
+	#[test]
+	fn reanchor_with_longer_expected_result() {
+		let mut id: MultiLocation = (Parachain(4000), GeneralIndex(42)).into();
+		let ancestry =
+			(Parachain(2000), Parachain(2022), PalletInstance(50), GeneralIndex(32)).into();
+		let target = (Parent, Parent, Parachain(1000), Parachain(3000)).into();
+		let expected = (
+			Parent,
+			Parent,
+			PalletInstance(50),
+			GeneralIndex(32),
+			Parachain(4000),
+			GeneralIndex(42),
+		)
+			.into();
+		id.reanchor(&target, &ancestry).unwrap();
+		assert_eq!(id, expected);
+	}
+
+	#[test]
+	fn reanchor_where_id_has_no_parent() {
+		let mut id: MultiLocation = (Parachain(4000), GeneralIndex(42)).into();
+		let ancestry = Parachain(2000).into();
+		let target = (Parent, Parachain(1000), Parachain(3000)).into();
+		let expected = (Parent, Parent, Parachain(2000), Parachain(4000), GeneralIndex(42)).into();
+		id.reanchor(&target, &ancestry).unwrap();
+		assert_eq!(id, expected);
+	}
+
+	#[test]
+	fn reanchor_where_target_has_no_parent() {
+		let mut id: MultiLocation = (Parent, Parachain(4000), GeneralIndex(42)).into();
+		let ancestry = Parachain(2000).into();
+		let target = (Parachain(1000), Parachain(3000)).into();
+		let expected = (Parent, Parent, Parent, Parachain(4000), GeneralIndex(42)).into();
+		id.reanchor(&target, &ancestry).unwrap();
+		assert_eq!(id, expected);
+	}
+
+	#[test]
+	fn reanchor_where_id_has_similar_indexed_parachain_as_ancestry() {
+		let mut id: MultiLocation =
+			(Parent, Parachain(2000), Parachain(4000), GeneralIndex(42)).into();
+		let ancestry = Parachain(2000).into();
+		let target = (Parent, Parachain(1000), Parachain(3000)).into();
+		let expected = (Parent, Parent, Parachain(2000), Parachain(4000), GeneralIndex(42)).into();
+		id.reanchor(&target, &ancestry).unwrap();
+		assert_eq!(id, expected);
+	}
+
+	#[test]
+	fn reanchor_where_target_and_id_on_different_branches_and_multiple_ancestry_junctions() {
+		let mut id: MultiLocation = (Parent, Parachain(4000), GeneralIndex(42)).into();
+		let ancestry =
+			(Parachain(2000), Parachain(2022), PalletInstance(50), GeneralIndex(32)).into();
+		let target = (Parent, Parachain(1000), Parachain(3000)).into();
+		let expected = (Parent, Parent, Parachain(4000), GeneralIndex(42)).into();
+		id.reanchor(&target, &ancestry).unwrap();
+		assert_eq!(id, expected);
+	}
+
 	#[test]
 	fn encode_and_decode_works() {
 		let m = MultiLocation {