diff --git a/xcm/xcm-builder/src/asset_conversion.rs b/xcm/xcm-builder/src/asset_conversion.rs index 1a13f1ffb444..cadbb8381cfa 100644 --- a/xcm/xcm-builder/src/asset_conversion.rs +++ b/xcm/xcm-builder/src/asset_conversion.rs @@ -16,7 +16,7 @@ //! Adapters to work with `frame_support::traits::tokens::fungibles` through XCM. -use frame_support::traits::Get; +use frame_support::traits::{Contains, Get}; use sp_std::{borrow::Borrow, marker::PhantomData, prelude::*, result}; use xcm::latest::prelude::*; use xcm_executor::traits::{Convert, Error as MatchError, MatchesFungibles, MatchesNonFungibles}; @@ -69,7 +69,7 @@ impl< fn matches_fungibles(a: &MultiAsset) -> result::Result<(AssetId, Balance), MatchError> { let (amount, id) = match (&a.fun, &a.id) { (Fungible(ref amount), Concrete(ref id)) => (amount, id), - _ => return Err(MatchError::AssetNotFound), + _ => return Err(MatchError::AssetNotHandled), }; let what = ConvertAssetId::convert_ref(id).map_err(|_| MatchError::AssetIdConversionFailed)?; @@ -89,7 +89,7 @@ impl< fn matches_nonfungibles(a: &MultiAsset) -> result::Result<(ClassId, InstanceId), MatchError> { let (instance, class) = match (&a.fun, &a.id) { (NonFungible(ref instance), Concrete(ref class)) => (instance, class), - _ => return Err(MatchError::AssetNotFound), + _ => return Err(MatchError::AssetNotHandled), }; let what = ConvertClassId::convert_ref(class).map_err(|_| MatchError::AssetIdConversionFailed)?; @@ -113,7 +113,7 @@ impl< fn matches_fungibles(a: &MultiAsset) -> result::Result<(AssetId, Balance), MatchError> { let (amount, id) = match (&a.fun, &a.id) { (Fungible(ref amount), Abstract(ref id)) => (amount, id), - _ => return Err(MatchError::AssetNotFound), + _ => return Err(MatchError::AssetNotHandled), }; let what = ConvertAssetId::convert_ref(id).map_err(|_| MatchError::AssetIdConversionFailed)?; @@ -133,7 +133,7 @@ impl< fn matches_nonfungibles(a: &MultiAsset) -> result::Result<(ClassId, InstanceId), MatchError> { let (instance, class) = match (&a.fun, &a.id) { (NonFungible(ref instance), Abstract(ref class)) => (instance, class), - _ => return Err(MatchError::AssetNotFound), + _ => return Err(MatchError::AssetNotHandled), }; let what = ConvertClassId::convert_ref(class).map_err(|_| MatchError::AssetIdConversionFailed)?; @@ -147,3 +147,204 @@ impl< pub type ConvertedConcreteAssetId = ConvertedConcreteId; #[deprecated = "Use `ConvertedAbstractId` instead"] pub type ConvertedAbstractAssetId = ConvertedAbstractId; + +pub struct MatchedConvertedConcreteId( + PhantomData<(AssetId, Balance, MatchAssetId, ConvertAssetId, ConvertOther)>, +); +impl< + AssetId: Clone, + Balance: Clone, + MatchAssetId: Contains, + ConvertAssetId: Convert, + ConvertBalance: Convert, + > MatchesFungibles + for MatchedConvertedConcreteId +{ + fn matches_fungibles(a: &MultiAsset) -> result::Result<(AssetId, Balance), MatchError> { + let (amount, id) = match (&a.fun, &a.id) { + (Fungible(ref amount), Concrete(ref id)) if MatchAssetId::contains(id) => (amount, id), + _ => return Err(MatchError::AssetNotHandled), + }; + let what = + ConvertAssetId::convert_ref(id).map_err(|_| MatchError::AssetIdConversionFailed)?; + let amount = ConvertBalance::convert_ref(amount) + .map_err(|_| MatchError::AmountToBalanceConversionFailed)?; + Ok((what, amount)) + } +} +impl< + ClassId: Clone, + InstanceId: Clone, + MatchClassId: Contains, + ConvertClassId: Convert, + ConvertInstanceId: Convert, + > MatchesNonFungibles + for MatchedConvertedConcreteId +{ + fn matches_nonfungibles(a: &MultiAsset) -> result::Result<(ClassId, InstanceId), MatchError> { + let (instance, class) = match (&a.fun, &a.id) { + (NonFungible(ref instance), Concrete(ref class)) if MatchClassId::contains(class) => + (instance, class), + _ => return Err(MatchError::AssetNotHandled), + }; + let what = + ConvertClassId::convert_ref(class).map_err(|_| MatchError::AssetIdConversionFailed)?; + let instance = ConvertInstanceId::convert_ref(instance) + .map_err(|_| MatchError::InstanceConversionFailed)?; + Ok((what, instance)) + } +} + +#[cfg(test)] +mod tests { + use super::*; + + use xcm_executor::traits::JustTry; + + struct OnlyParentZero; + impl Contains for OnlyParentZero { + fn contains(a: &MultiLocation) -> bool { + match a { + MultiLocation { parents: 0, .. } => true, + _ => false, + } + } + } + + #[test] + fn matched_converted_concrete_id_for_fungibles_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 - does not match + assert_eq!( + Converter::matches_fungibles(&MultiAsset { + id: Concrete(MultiLocation::new(1, X2(PalletInstance(50), GeneralIndex(1)))), + fun: Fungible(12345), + }), + Err(MatchError::AssetNotHandled) + ); + + // err - matches, but convert fails + 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 - matches, but NonFungible + assert_eq!( + Converter::matches_fungibles(&MultiAsset { + id: Concrete(MultiLocation::new(0, X2(PalletInstance(50), GeneralIndex(1)))), + fun: NonFungible(Index(54321)), + }), + Err(MatchError::AssetNotHandled) + ); + + // ok + assert_eq!( + Converter::matches_fungibles(&MultiAsset { + id: Concrete(MultiLocation::new(0, X2(PalletInstance(50), GeneralIndex(1)))), + fun: Fungible(12345), + }), + Ok((1, 12345)) + ); + } + + #[test] + fn matched_converted_concrete_id_for_nonfungibles_works() { + type ClassId = u32; + type ClassInstanceId = u64; + frame_support::parameter_types! { + pub TrustBackedAssetsPalletLocation: MultiLocation = PalletInstance(50).into(); + } + + // ConvertedConcreteId cfg + struct ClassInstanceIdConverter; + impl Convert for ClassInstanceIdConverter { + fn convert_ref(value: impl Borrow) -> Result { + value.borrow().clone().try_into().map_err(|_| ()) + } + + fn reverse_ref(value: impl Borrow) -> Result { + Ok(AssetInstance::from(value.borrow().clone())) + } + } + + type Converter = MatchedConvertedConcreteId< + ClassId, + ClassInstanceId, + OnlyParentZero, + AsPrefixedGeneralIndex, + ClassInstanceIdConverter, + >; + assert_eq!( + TrustBackedAssetsPalletLocation::get(), + MultiLocation { parents: 0, interior: X1(PalletInstance(50)) } + ); + + // err - does not match + assert_eq!( + Converter::matches_nonfungibles(&MultiAsset { + id: Concrete(MultiLocation::new(1, X2(PalletInstance(50), GeneralIndex(1)))), + fun: NonFungible(Index(54321)), + }), + Err(MatchError::AssetNotHandled) + ); + + // err - matches, but convert fails + assert_eq!( + Converter::matches_nonfungibles(&MultiAsset { + id: Concrete(MultiLocation::new( + 0, + X2(PalletInstance(50), GeneralKey { length: 1, data: [1; 32] }) + )), + fun: NonFungible(Index(54321)), + }), + Err(MatchError::AssetIdConversionFailed) + ); + + // err - matches, but Fungible vs NonFungible + assert_eq!( + Converter::matches_nonfungibles(&MultiAsset { + id: Concrete(MultiLocation::new(0, X2(PalletInstance(50), GeneralIndex(1)))), + fun: Fungible(12345), + }), + Err(MatchError::AssetNotHandled) + ); + + // ok + assert_eq!( + Converter::matches_nonfungibles(&MultiAsset { + id: Concrete(MultiLocation::new(0, X2(PalletInstance(50), GeneralIndex(1)))), + fun: NonFungible(Index(54321)), + }), + Ok((1, 54321)) + ); + } +} diff --git a/xcm/xcm-builder/src/currency_adapter.rs b/xcm/xcm-builder/src/currency_adapter.rs index 2815d8bd3c47..bb7ac7a648da 100644 --- a/xcm/xcm-builder/src/currency_adapter.rs +++ b/xcm/xcm-builder/src/currency_adapter.rs @@ -28,8 +28,8 @@ use xcm_executor::{ /// Asset transaction errors. enum Error { - /// Asset not found. - AssetNotFound, + /// The given asset is not handled. (According to [`XcmError::AssetNotFound`]) + AssetNotHandled, /// `MultiLocation` to `AccountId` conversion failed. AccountIdConversionFailed, } @@ -38,7 +38,7 @@ impl From for XcmError { fn from(e: Error) -> Self { use XcmError::FailedToTransactAsset; match e { - Error::AssetNotFound => XcmError::AssetNotFound, + Error::AssetNotHandled => XcmError::AssetNotFound, Error::AccountIdConversionFailed => FailedToTransactAsset("AccountIdConversionFailed"), } } @@ -143,7 +143,7 @@ impl< log::trace!(target: "xcm::currency_adapter", "can_check_in origin: {:?}, what: {:?}", _origin, what); // Check we handle this asset. let amount: Currency::Balance = - Matcher::matches_fungible(what).ok_or(Error::AssetNotFound)?; + Matcher::matches_fungible(what).ok_or(Error::AssetNotHandled)?; match CheckedAccount::get() { Some((checked_account, MintLocation::Local)) => Self::can_reduce_checked(checked_account, amount), @@ -168,7 +168,7 @@ impl< fn can_check_out(_dest: &MultiLocation, what: &MultiAsset, _context: &XcmContext) -> Result { log::trace!(target: "xcm::currency_adapter", "check_out dest: {:?}, what: {:?}", _dest, what); - let amount = Matcher::matches_fungible(what).ok_or(Error::AssetNotFound)?; + let amount = Matcher::matches_fungible(what).ok_or(Error::AssetNotHandled)?; match CheckedAccount::get() { Some((checked_account, MintLocation::Local)) => Self::can_accrue_checked(checked_account, amount), @@ -194,7 +194,7 @@ impl< fn deposit_asset(what: &MultiAsset, who: &MultiLocation, _context: &XcmContext) -> Result { log::trace!(target: "xcm::currency_adapter", "deposit_asset what: {:?}, who: {:?}", what, who); // Check we handle this asset. - let amount = Matcher::matches_fungible(&what).ok_or(Error::AssetNotFound)?; + let amount = Matcher::matches_fungible(&what).ok_or(Error::AssetNotHandled)?; let who = AccountIdConverter::convert_ref(who).map_err(|()| Error::AccountIdConversionFailed)?; let _imbalance = Currency::deposit_creating(&who, amount); @@ -208,7 +208,7 @@ impl< ) -> result::Result { log::trace!(target: "xcm::currency_adapter", "withdraw_asset what: {:?}, who: {:?}", what, who); // Check we handle this asset. - let amount = Matcher::matches_fungible(what).ok_or(Error::AssetNotFound)?; + let amount = Matcher::matches_fungible(what).ok_or(Error::AssetNotHandled)?; let who = AccountIdConverter::convert_ref(who).map_err(|()| Error::AccountIdConversionFailed)?; Currency::withdraw(&who, amount, WithdrawReasons::TRANSFER, AllowDeath) @@ -223,7 +223,7 @@ impl< _context: &XcmContext, ) -> result::Result { log::trace!(target: "xcm::currency_adapter", "internal_transfer_asset asset: {:?}, from: {:?}, to: {:?}", asset, from, to); - let amount = Matcher::matches_fungible(asset).ok_or(Error::AssetNotFound)?; + let amount = Matcher::matches_fungible(asset).ok_or(Error::AssetNotHandled)?; let from = AccountIdConverter::convert_ref(from).map_err(|()| Error::AccountIdConversionFailed)?; let to = diff --git a/xcm/xcm-executor/src/traits/token_matching.rs b/xcm/xcm-executor/src/traits/token_matching.rs index befff6b1b726..f1bbbe68025b 100644 --- a/xcm/xcm-executor/src/traits/token_matching.rs +++ b/xcm/xcm-executor/src/traits/token_matching.rs @@ -48,9 +48,10 @@ impl MatchesNonFungible for Tuple { } /// Errors associated with [`MatchesFungibles`] operation. +#[derive(Debug, PartialEq, Eq)] pub enum Error { - /// Asset not found. - AssetNotFound, + /// The given asset is not handled. (According to [`XcmError::AssetNotFound`]) + AssetNotHandled, /// `MultiLocation` to `AccountId` conversion failed. AccountIdConversionFailed, /// `u128` amount to currency `Balance` conversion failed. @@ -65,7 +66,7 @@ impl From for XcmError { fn from(e: Error) -> Self { use XcmError::FailedToTransactAsset; match e { - Error::AssetNotFound => XcmError::AssetNotFound, + Error::AssetNotHandled => XcmError::AssetNotFound, Error::AccountIdConversionFailed => FailedToTransactAsset("AccountIdConversionFailed"), Error::AmountToBalanceConversionFailed => FailedToTransactAsset("AmountToBalanceConversionFailed"), @@ -86,7 +87,7 @@ impl MatchesFungibles for Tuple { match Tuple::matches_fungibles(a) { o @ Ok(_) => return o, _ => () } )* ); log::trace!(target: "xcm::matches_fungibles", "did not match fungibles asset: {:?}", &a); - Err(Error::AssetNotFound) + Err(Error::AssetNotHandled) } } @@ -101,6 +102,6 @@ impl MatchesNonFungibles for Tuple { match Tuple::matches_nonfungibles(a) { o @ Ok(_) => return o, _ => () } )* ); log::trace!(target: "xcm::matches_non_fungibles", "did not match fungibles asset: {:?}", &a); - Err(Error::AssetNotFound) + Err(Error::AssetNotHandled) } } diff --git a/xcm/xcm-executor/src/traits/weight.rs b/xcm/xcm-executor/src/traits/weight.rs index 76f411e5c1f5..6e111f93e806 100644 --- a/xcm/xcm-executor/src/traits/weight.rs +++ b/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 { + 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 { diff --git a/xcm/xcm-simulator/example/src/parachain.rs b/xcm/xcm-simulator/example/src/parachain.rs index 029771bce10d..e2a5364e9080 100644 --- a/xcm/xcm-simulator/example/src/parachain.rs +++ b/xcm/xcm-simulator/example/src/parachain.rs @@ -49,7 +49,7 @@ use xcm_executor::{ }; pub type SovereignAccountOf = ( - SiblingParachainConvertsVia, + SiblingParachainConvertsVia, AccountId32Aliases, ParentIsPreset, );