Fix Hub token reserve location for post-AHM XCM transfers

asset-registry/src/mock/para.rs

@@ -10,10 +10,8 @@ use frame_support::{
 	PalletId,
 };
 use frame_system::{EnsureRoot, EnsureSignedBy};
-use orml_traits::{
-	location::{AbsoluteReserveProvider, RelativeReserveProvider},
-	parameter_type_with_key, FixedConversionRateProvider, MultiCurrency,
-};
+use orml_traits::{parameter_type_with_key, FixedConversionRateProvider, MultiCurrency};
+use orml_xtokens::{AbsoluteReserveProvider, RelativeReserveProvider};
 use orml_xcm_support::{IsNativeConcrete, MultiCurrencyAdapter, MultiNativeAsset};
 use pallet_xcm::XcmPassthrough;
 use parity_scale_codec::{Decode, DecodeWithMemTracking, Encode, MaxEncodedLen};

traits/src/location.rs

@@ -1,75 +1,13 @@
 use sp_core::{bounded::BoundedVec, ConstU32};
 use xcm::v5::prelude::*;
 
-pub trait Parse {
-	/// Returns the "chain" location part. It could be parent, sibling
-	/// parachain, or child parachain.
-	fn chain_part(&self) -> Option<Location>;
-	/// Returns "non-chain" location part.
-	fn non_chain_part(&self) -> Option<Location>;
-}
-
-fn is_chain_junction(junction: Option<&Junction>) -> bool {
-	matches!(junction, Some(Parachain(_)))
-}
-
-impl Parse for Location {
-	fn chain_part(&self) -> Option<Location> {
-		match (self.parents, self.first_interior()) {
-			// sibling parachain
-			(1, Some(Parachain(id))) => Some(Location::new(1, [Parachain(*id)])),
-			// parent
-			(1, _) => Some(Location::parent()),
-			// children parachain
-			(0, Some(Parachain(id))) => Some(Location::new(0, [Parachain(*id)])),
-			_ => None,
-		}
-	}
-
-	fn non_chain_part(&self) -> Option<Location> {
-		let mut junctions = self.interior().clone();
-		while is_chain_junction(junctions.first()) {
-			let _ = junctions.take_first();
-		}
-
-		if junctions != Here {
-			Some(Location::new(0, junctions))
-		} else {
-			None
-		}
-	}
-}
+pub const ASSET_HUB_ID: u32 = 1000;
 
 pub trait Reserve {
 	/// Returns assets reserve location.
 	fn reserve(asset: &Asset) -> Option<Location>;
 }
 
-// Provide reserve in absolute path view
-pub struct AbsoluteReserveProvider;
-
-impl Reserve for AbsoluteReserveProvider {
-	fn reserve(asset: &Asset) -> Option<Location> {
-		let AssetId(location) = &asset.id;
-		location.chain_part()
-	}
-}
-
-// Provide reserve in relative path view
-// Self tokens are represeneted as Here
-pub struct RelativeReserveProvider;
-
-impl Reserve for RelativeReserveProvider {
-	fn reserve(asset: &Asset) -> Option<Location> {
-		let AssetId(location) = &asset.id;
-		if location.parents == 0 && !is_chain_junction(location.first_interior()) {
-			Some(Location::here())
-		} else {
-			location.chain_part()
-		}
-	}
-}
-
 pub trait RelativeLocations {
 	fn sibling_parachain_general_key(para_id: u32, general_key: BoundedVec<u8, ConstU32<32>>) -> Location;
 }
@@ -79,93 +17,3 @@ impl RelativeLocations for Location {
 		Location::new(1, [Parachain(para_id), general_key.as_bounded_slice().into()])
 	}
 }
-
-#[cfg(test)]
-mod tests {
-	use super::*;
-
-	const PARACHAIN: Junction = Parachain(1);
-	const GENERAL_INDEX: Junction = GeneralIndex(1);
-
-	fn concrete_fungible(id: Location) -> Asset {
-		(id, 1).into()
-	}
-
-	#[test]
-	fn parent_as_reserve_chain() {
-		assert_eq!(
-			AbsoluteReserveProvider::reserve(&concrete_fungible(Location::new(1, [GENERAL_INDEX]))),
-			Some(Location::parent())
-		);
-		assert_eq!(
-			RelativeReserveProvider::reserve(&concrete_fungible(Location::new(1, [GENERAL_INDEX]))),
-			Some(Location::parent())
-		);
-	}
-
-	#[test]
-	fn sibling_parachain_as_reserve_chain() {
-		assert_eq!(
-			AbsoluteReserveProvider::reserve(&concrete_fungible(Location::new(1, [PARACHAIN, GENERAL_INDEX]))),
-			Some(Location::new(1, [PARACHAIN]))
-		);
-		assert_eq!(
-			RelativeReserveProvider::reserve(&concrete_fungible(Location::new(1, [PARACHAIN, GENERAL_INDEX]))),
-			Some(Location::new(1, [PARACHAIN]))
-		);
-	}
-
-	#[test]
-	fn child_parachain_as_reserve_chain() {
-		assert_eq!(
-			AbsoluteReserveProvider::reserve(&concrete_fungible(Location::new(0, [PARACHAIN, GENERAL_INDEX]))),
-			Some(PARACHAIN.into())
-		);
-		assert_eq!(
-			RelativeReserveProvider::reserve(&concrete_fungible(Location::new(0, [PARACHAIN, GENERAL_INDEX]))),
-			Some(PARACHAIN.into())
-		);
-	}
-
-	#[test]
-	fn no_reserve_chain_for_absolute_self_for_relative() {
-		assert_eq!(
-			AbsoluteReserveProvider::reserve(&concrete_fungible(Location::new(
-				0,
-				[Junction::from(BoundedVec::try_from(b"DOT".to_vec()).unwrap())]
-			))),
-			None
-		);
-		assert_eq!(
-			RelativeReserveProvider::reserve(&concrete_fungible(Location::new(
-				0,
-				[Junction::from(BoundedVec::try_from(b"DOT".to_vec()).unwrap())]
-			))),
-			Some(Location::here())
-		);
-	}
-
-	#[test]
-	fn non_chain_part_works() {
-		assert_eq!(Location::parent().non_chain_part(), None);
-		assert_eq!(Location::new(1, [PARACHAIN]).non_chain_part(), None);
-		assert_eq!(Location::new(0, [PARACHAIN]).non_chain_part(), None);
-
-		assert_eq!(
-			Location::new(1, [GENERAL_INDEX]).non_chain_part(),
-			Some(GENERAL_INDEX.into())
-		);
-		assert_eq!(
-			Location::new(1, [GENERAL_INDEX, GENERAL_INDEX]).non_chain_part(),
-			Some((GENERAL_INDEX, GENERAL_INDEX).into())
-		);
-		assert_eq!(
-			Location::new(1, [PARACHAIN, GENERAL_INDEX]).non_chain_part(),
-			Some(GENERAL_INDEX.into())
-		);
-		assert_eq!(
-			Location::new(0, [PARACHAIN, GENERAL_INDEX]).non_chain_part(),
-			Some(GENERAL_INDEX.into())
-		);
-	}
-}

xcm-support/src/tests.rs

@@ -4,7 +4,7 @@
 
 use super::*;
 
-use orml_traits::{location::AbsoluteReserveProvider, location::RelativeLocations, ConcreteFungibleAsset};
+use orml_traits::{location::RelativeLocations, ConcreteFungibleAsset};
 
 #[derive(Debug, PartialEq, Eq)]
 pub enum TestCurrencyId {
@@ -88,22 +88,3 @@ fn is_native_concrete_does_not_matches_non_native_currencies() {
 	})
 	.is_none());
 }
-
-#[test]
-fn multi_native_asset() {
-	assert!(MultiNativeAsset::<AbsoluteReserveProvider>::contains(
-		&Asset {
-			fun: Fungible(10),
-			id: AssetId(Location::parent())
-		},
-		&Parent.into()
-	));
-	assert!(MultiNativeAsset::<AbsoluteReserveProvider>::contains(
-		&Asset::sibling_parachain_asset(1, b"TokenA".to_vec().try_into().unwrap(), 100),
-		&Location::new(1, [Parachain(1)]),
-	));
-	assert!(!MultiNativeAsset::<AbsoluteReserveProvider>::contains(
-		&Asset::sibling_parachain_asset(1, b"TokenA".to_vec().try_into().unwrap(), 100),
-		&Location::parent(),
-	));
-}

xtokens/src/lib.rs

@@ -53,7 +53,7 @@ use xcm_executor::traits::WeightBounds;
 
 pub use module::*;
 use orml_traits::{
-	location::{Parse, Reserve},
+	location::{Reserve, ASSET_HUB_ID},
 	xcm_transfer::{Transferred, XtokensWeightInfo},
 	GetByKey, RateLimiter, XcmTransfer,
 };
@@ -565,7 +565,7 @@ pub mod module {
 			if asset_len > 1 && fee_reserve != non_fee_reserve {
 				// Current only support `ToReserve` with relay-chain asset as fee. other case
 				// like `NonReserve` or `SelfReserve` with relay-chain fee is not support.
-				ensure!(non_fee_reserve == dest.chain_part(), Error::<T>::InvalidAsset);
+				ensure!(non_fee_reserve == chain_part(&dest), Error::<T>::InvalidAsset);
 
 				let reserve_location = non_fee_reserve.clone().ok_or(Error::<T>::AssetHasNoReserve)?;
 				let min_xcm_fee = T::MinXcmFee::get(&reserve_location).ok_or(Error::<T>::MinXcmFeeNotDefined)?;
@@ -590,7 +590,7 @@ pub mod module {
 
 				let mut override_recipient = T::SelfLocation::get();
 				if override_recipient == Location::here() {
-					let dest_chain_part = dest.chain_part().ok_or(Error::<T>::InvalidDest)?;
+					let dest_chain_part = chain_part(&dest).ok_or(Error::<T>::InvalidDest)?;
 					let ancestry = T::UniversalLocation::get();
 					let _ = override_recipient
 						.reanchor(&dest_chain_part, &ancestry)
@@ -600,7 +600,7 @@ pub mod module {
 				// First xcm sent to fee reserve chain and routed to dest chain.
 				// We can use `MinXcmFee` configuration to decide which target parachain use
 				// teleport. But as current there's only one case which is Parachain send back
-				// asset to Statemine/t, So we set `use_teleport` to always `true` in this case.
+				// asset to AssetHub, So we set `use_teleport` to always `true` in this case.
 				Self::execute_and_send_reserve_kind_xcm(
 					origin_location.clone(),
 					assets_to_fee_reserve,
@@ -684,10 +684,12 @@ pub mod module {
 			let mut hash = msg.using_encoded(sp_io::hashing::blake2_256);
 
 			let weight = T::Weigher::weight(&mut msg, Weight::MAX).map_err(|_| Error::<T>::UnweighableMessage)?;
-			T::XcmExecutor::prepare_and_execute(origin_location, msg, &mut hash, weight, weight)
+			T::XcmExecutor::prepare_and_execute(origin_location.clone(), msg.clone(), &mut hash, weight, weight)
 				.ensure_complete()
 				.map_err(|error| {
-					log::error!("Failed execute transfer message with {error:?}");
+					log::error!(
+						"Failed execute transfer message with {error:?}, origin: {origin_location:?}, msg: {msg:?}"
+					);
 					Error::<T>::XcmExecutionFailed
 				})?;
 
@@ -817,7 +819,7 @@ pub mod module {
 
 		/// Ensure has the `dest` has chain part and recipient part.
 		fn ensure_valid_dest(dest: &Location) -> Result<(Location, Location), DispatchError> {
-			if let (Some(dest), Some(recipient)) = (dest.chain_part(), dest.non_chain_part()) {
+			if let (Some(dest), Some(recipient)) = (chain_part(dest), non_chain_part(dest)) {
 				Ok((dest, recipient))
 			} else {
 				Err(Error::<T>::InvalidDest.into())
@@ -1066,3 +1068,60 @@ fn subtract_fee(asset: &Asset, amount: u128) -> Asset {
 		id: asset.id.clone(),
 	}
 }
+
+fn is_chain_junction(junction: Option<&Junction>) -> bool {
+	matches!(junction, Some(Parachain(_)))
+}
+
+// Provide reserve in absolute path view
+pub struct AbsoluteReserveProvider;
+
+impl Reserve for AbsoluteReserveProvider {
+	fn reserve(asset: &Asset) -> Option<Location> {
+		let AssetId(location) = &asset.id;
+		chain_part(location)
+	}
+}
+
+// Provide reserve in relative path view
+// Self tokens are represeneted as Here
+pub struct RelativeReserveProvider;
+
+impl Reserve for RelativeReserveProvider {
+	fn reserve(asset: &Asset) -> Option<Location> {
+		let AssetId(location) = &asset.id;
+		if location.parents == 0 && !is_chain_junction(location.first_interior()) {
+			Some(Location::here())
+		} else {
+			chain_part(location)
+		}
+	}
+}
+
+/// Returns the "chain" location part. It could be parent, sibling
+/// parachain, or child parachain.
+pub fn chain_part(location: &Location) -> Option<Location> {
+	match (location.parents, location.first_interior()) {
+		// sibling parachain
+		(1, Some(Parachain(id))) => Some(Location::new(1, [Parachain(*id)])),
+		// parent -> assethub
+		(1, _) => Some(Location::new(1, [Parachain(ASSET_HUB_ID)])),
+		// children parachain
+		(0, Some(Parachain(id))) => Some(Location::new(0, [Parachain(*id)])),
+		_ => None,
+	}
+}
+
+/// Returns "non-chain" location part.
+pub fn non_chain_part(location: &Location) -> Option<Location> {
+	let mut junctions = location.interior().clone();
+	while is_chain_junction(junctions.first()) {
+		let _ = junctions.take_first();
+	}
+
+	if junctions != Here {
+		Some(Location::new(0, junctions))
+	} else {
+		None
+	}
+}