[XCM] Multiple FungiblesAdapters support + WeightTrader::buy_weight more accurate error

xcm/xcm-builder/src/asset_conversion.rs

@@ -19,7 +19,9 @@
 use frame_support::traits::Get;
 use sp_std::{borrow::Borrow, marker::PhantomData, prelude::*, result};
 use xcm::latest::prelude::*;
-use xcm_executor::traits::{Convert, Error as MatchError, MatchesFungibles, MatchesNonFungibles};
+use xcm_executor::traits::{
+	Convert, Error as MatchError, MatchesFungibles, MatchesMultiAssetId, MatchesNonFungibles,
+};
 
 /// Converter struct implementing `AssetIdConversion` converting a numeric asset ID (must be `TryFrom/TryInto<u128>`) into
 /// a `GeneralIndex` junction, prefixed by some `MultiLocation` value. The `MultiLocation` value will typically be a
@@ -147,3 +149,104 @@ impl<
 pub type ConvertedConcreteAssetId<A, B, C, O> = ConvertedConcreteId<A, B, C, O>;
 #[deprecated = "Use `ConvertedAbstractId` instead"]
 pub type ConvertedAbstractAssetId<A, B, C, O> = ConvertedAbstractId<A, B, C, O>;
+
+pub struct MatchedConvertedConcreteId<AssetId, Balance, MatchAssetId, ConvertAssetId, ConvertOther>(
+	PhantomData<(AssetId, Balance, MatchAssetId, ConvertAssetId, ConvertOther)>,
+);
+impl<
+		AssetId: Clone,
+		Balance: Clone,
+		MatchAssetId: MatchesMultiAssetId<MultiLocation>,
+		ConvertAssetId: Convert<MultiLocation, AssetId>,
+		ConvertBalance: Convert<u128, Balance>,
+	> MatchesFungibles<AssetId, Balance>
+	for MatchedConvertedConcreteId<AssetId, Balance, MatchAssetId, ConvertAssetId, ConvertBalance>
+{
+	fn matches_fungibles(a: &MultiAsset) -> result::Result<(AssetId, Balance), MatchError> {
+		let (amount, id) = match (&a.fun, &a.id) {
+			(Fungible(ref amount), Concrete(ref id)) => match MatchAssetId::matches(id) {
+				true => (amount, id),
+				false => return Err(MatchError::AssetNotFound),
+			},
+			_ => return Err(MatchError::AssetNotFound),
+		};
+		let what =
+			ConvertAssetId::convert_ref(id).map_err(|_| MatchError::AssetIdConversionFailed)?;
+		let amount = ConvertBalance::convert_ref(amount)
+			.map_err(|_| MatchError::AmountToBalanceConversionFailed)?;
+		Ok((what, amount))
+	}
+}
+
+#[cfg(test)]
+mod tests {
+	use super::*;
+
+	use xcm_executor::traits::JustTry;
+
+	struct OnlyParentZero;
+	impl MatchesMultiAssetId<MultiLocation> for OnlyParentZero {
+		fn matches(a: &MultiLocation) -> bool {
+			match a {
+				MultiLocation { parents: 0, .. } => true,
+				_ => false,
+			}
+		}
+	}
+
+	#[test]
+	fn matched_converted_concrete_id_works() {
+		type AssetIdForTrustBackedAssets = u32;
+		type Balance = u128;
+		frame_support::parameter_types! {
+			pub TrustBackedAssetsPalletLocation: MultiLocation = PalletInstance(50).into();
+		}
+
+		// ConvertedConcreteId cfg
+		type Converter = MatchedConvertedConcreteId<
+			AssetIdForTrustBackedAssets,
+			Balance,
+			OnlyParentZero,
+			AsPrefixedGeneralIndex<
+				TrustBackedAssetsPalletLocation,
+				AssetIdForTrustBackedAssets,
+				JustTry,
+			>,
+			JustTry,
+		>;
+		assert_eq!(
+			TrustBackedAssetsPalletLocation::get(),
+			MultiLocation { parents: 0, interior: X1(PalletInstance(50)) }
+		);
+
+		// err - matches, but convert failed
+		assert_eq!(
+			Converter::matches_fungibles(&MultiAsset {
+				id: Concrete(MultiLocation::new(
+					0,
+					X2(PalletInstance(50), GeneralKey { length: 1, data: [1; 32] })
+				)),
+				fun: Fungible(12345),
+			}),
+			Err(MatchError::AssetIdConversionFailed)
+		);
+
+		// err - does not match
+		assert_eq!(
+			Converter::matches_fungibles(&MultiAsset {
+				id: Concrete(MultiLocation::new(1, X2(PalletInstance(50), GeneralIndex(1)))),
+				fun: Fungible(12345),
+			}),
+			Err(MatchError::AssetNotFound)
+		);
+
+		// ok
+		assert_eq!(
+			Converter::matches_fungibles(&MultiAsset {
+				id: Concrete(MultiLocation::new(0, X2(PalletInstance(50), GeneralIndex(1)))),
+				fun: Fungible(12345),
+			}),
+			Ok((1, 12345))
+		);
+	}
+}

xcm/xcm-executor/src/traits/mod.rs

@@ -35,7 +35,8 @@ mod filter_asset_location;
 pub use filter_asset_location::FilterAssetLocation;
 mod token_matching;
 pub use token_matching::{
-	Error, MatchesFungible, MatchesFungibles, MatchesNonFungible, MatchesNonFungibles,
+	Error, MatchesFungible, MatchesFungibles, MatchesMultiAssetId, MatchesNonFungible,
+	MatchesNonFungibles,
 };
 mod on_response;
 pub use on_response::{OnResponse, VersionChangeNotifier};
@@ -50,8 +51,8 @@ pub mod prelude {
 	pub use super::{
 		export_xcm, validate_export, AssetExchange, AssetLock, ClaimAssets, Convert, ConvertOrigin,
 		Decoded, DropAssets, Enact, Encoded, Error, ExportXcm, FeeManager, FeeReason, Identity,
-		JustTry, LockError, MatchesFungible, MatchesFungibles, MatchesNonFungible,
-		MatchesNonFungibles, OnResponse, ShouldExecute, TransactAsset, VersionChangeNotifier,
-		WeightBounds, WeightTrader, WithOriginFilter,
+		JustTry, LockError, MatchesFungible, MatchesFungibles, MatchesMultiAssetId,
+		MatchesNonFungible, MatchesNonFungibles, OnResponse, ShouldExecute, TransactAsset,
+		VersionChangeNotifier, WeightBounds, WeightTrader, WithOriginFilter,
 	};
 }

xcm/xcm-executor/src/traits/token_matching.rs

@@ -48,8 +48,9 @@ impl<Instance> MatchesNonFungible<Instance> for Tuple {
 }
 
 /// Errors associated with [`MatchesFungibles`] operation.
+#[derive(Debug, PartialEq, Eq)]
 pub enum Error {
-	/// Asset not found.
+	/// The given asset is not handled. (According to [`XcmError::AssetNotFound`])
 	AssetNotFound,
 	/// `MultiLocation` to `AccountId` conversion failed.
 	AccountIdConversionFailed,
@@ -104,3 +105,18 @@ impl<AssetId, Instance> MatchesNonFungibles<AssetId, Instance> for Tuple {
 		Err(Error::AssetNotFound)
 	}
 }
+
+pub trait MatchesMultiAssetId<Id> {
+	fn matches(a: &Id) -> bool;
+}
+
+#[impl_trait_for_tuples::impl_for_tuples(30)]
+impl<Id: sp_std::fmt::Debug> MatchesMultiAssetId<Id> for Tuple {
+	fn matches(a: &Id) -> bool {
+		for_tuples!( #(
+			match Tuple::matches(a) { o @ true => return o, _ => () }
+		)* );
+		log::trace!(target: "xcm::matches", "did not match asset location: {:?}", &a);
+		false
+	}
+}

xcm/xcm-executor/src/traits/weight.rs

@@ -67,16 +67,29 @@ impl WeightTrader for Tuple {
 	}
 
 	fn buy_weight(&mut self, weight: Weight, payment: Assets) -> Result<Assets, XcmError> {
+		let mut too_expensive_error_found = false;
 		let mut last_error = None;
 		for_tuples!( #(
 			match Tuple.buy_weight(weight, payment.clone()) {
 				Ok(assets) => return Ok(assets),
-				Err(e) => { last_error = Some(e) }
+				Err(e) => {
+					if let XcmError::TooExpensive = e {
+						too_expensive_error_found = true;
+					}
+					last_error = Some(e)
+				}
 			}
 		)* );
-		let last_error = last_error.unwrap_or(XcmError::TooExpensive);
-		log::trace!(target: "xcm::buy_weight", "last_error: {:?}", last_error);
-		Err(last_error)
+
+		log::trace!(target: "xcm::buy_weight", "last_error: {:?}, too_expensive_error_found: {}", last_error, too_expensive_error_found);
+
+		// if we have multiple traders, and first one returns `TooExpensive` and others fail e.g. `AssetNotFound`
+		// then it is more accurate to return `TooExpensive` then `AssetNotFound`
+		Err(if too_expensive_error_found {
+			XcmError::TooExpensive
+		} else {
+			last_error.unwrap_or(XcmError::TooExpensive)
+		})
 	}
 
 	fn refund_weight(&mut self, weight: Weight) -> Option<MultiAsset> {

xcm/xcm-simulator/example/src/parachain.rs

@@ -49,7 +49,7 @@ use xcm_executor::{
 };
 
 pub type SovereignAccountOf = (
-	SiblingParachainConvertsVia<ParaId, AccountId>,
+	SiblingParachainConvertsVia<Sibling, AccountId>,
 	AccountId32Aliases<RelayNetwork, AccountId>,
 	ParentIsPreset<AccountId>,
 );