From fa6753847c068f287b71119f5f136b6316fcb5ad Mon Sep 17 00:00:00 2001 From: Shaun Wang Date: Thu, 18 Mar 2021 17:06:29 +1300 Subject: [PATCH 1/7] Handle unknown assets in TransactAsset impl. --- xcm-support/src/currency_adapter.rs | 74 ++++++++++++++++++++--------- xcm-support/src/lib.rs | 18 +++++++ 2 files changed, 69 insertions(+), 23 deletions(-) diff --git a/xcm-support/src/currency_adapter.rs b/xcm-support/src/currency_adapter.rs index 5eb76945f..ae3c20f57 100644 --- a/xcm-support/src/currency_adapter.rs +++ b/xcm-support/src/currency_adapter.rs @@ -11,12 +11,12 @@ use sp_std::{ use xcm::v0::{Error as XcmError, MultiAsset, MultiLocation, Result}; use xcm_executor::traits::{LocationConversion, MatchesFungible, TransactAsset}; -use crate::CurrencyIdConversion; +use crate::{CurrencyIdConversion, UnknownAsset as UnknownAssetT}; /// Asset transaction errors. enum Error { - /// Asset not found. - AssetNotFound, + /// Match fungible failed. + MatchFungibleFailed, /// `MultiLocation` to `AccountId` Conversion failed. AccountIdConversionFailed, /// `CurrencyId` conversion failed. @@ -26,14 +26,28 @@ enum Error { impl From for XcmError { fn from(e: Error) -> Self { match e { - Error::AssetNotFound => XcmError::FailedToTransactAsset("AssetNotFound"), + Error::MatchFungibleFailed => XcmError::FailedToTransactAsset("MatchFungibleFailed"), Error::AccountIdConversionFailed => XcmError::FailedToTransactAsset("AccountIdConversionFailed"), Error::CurrencyIdConversionFailed => XcmError::FailedToTransactAsset("CurrencyIdConversionFailed"), } } } -pub struct MultiCurrencyAdapter( +/// The `TransactAsset` implementation, to handle deposit/withdraw for XCM. +/// +/// The implementation will try deposit or withdraw on unknown asset first, so +/// that detailed error info of known asset failure could be returned, if any. +/// Thus known asset deposit/withdraw failures imply unknown asset failures as +/// well. +pub struct MultiCurrencyAdapter< + MultiCurrency, + Matcher, + AccountIdConverter, + AccountId, + CurrencyIdConverter, + CurrencyId, + UnknownAsset, +>( PhantomData<( MultiCurrency, Matcher, @@ -41,6 +55,7 @@ pub struct MultiCurrencyAdapter, ); @@ -51,30 +66,43 @@ impl< AccountId: sp_std::fmt::Debug, CurrencyIdConverter: CurrencyIdConversion, CurrencyId: FullCodec + Eq + PartialEq + Copy + MaybeSerializeDeserialize + Debug, + UnknownAsset: UnknownAssetT, > TransactAsset - for MultiCurrencyAdapter + for MultiCurrencyAdapter< + MultiCurrency, + Matcher, + AccountIdConverter, + AccountId, + CurrencyIdConverter, + CurrencyId, + UnknownAsset, + > { fn deposit_asset(asset: &MultiAsset, location: &MultiLocation) -> Result { - let who = AccountIdConverter::from_location(location) - .ok_or_else(|| XcmError::from(Error::AccountIdConversionFailed))?; - let currency_id = - CurrencyIdConverter::from_asset(asset).ok_or_else(|| XcmError::from(Error::CurrencyIdConversionFailed))?; - let amount: MultiCurrency::Balance = Matcher::matches_fungible(&asset) - .ok_or_else(|| XcmError::from(Error::AssetNotFound))? - .saturated_into(); - MultiCurrency::deposit(currency_id, &who, amount).map_err(|e| XcmError::FailedToTransactAsset(e.into()))?; - Ok(()) + UnknownAsset::deposit(asset, location).or_else(|_| { + let who = AccountIdConverter::from_location(location) + .ok_or_else(|| XcmError::from(Error::AccountIdConversionFailed))?; + let currency_id = CurrencyIdConverter::from_asset(asset) + .ok_or_else(|| XcmError::from(Error::CurrencyIdConversionFailed))?; + let amount: MultiCurrency::Balance = Matcher::matches_fungible(&asset) + .ok_or_else(|| XcmError::from(Error::MatchFungibleFailed))? + .saturated_into(); + MultiCurrency::deposit(currency_id, &who, amount).map_err(|e| XcmError::FailedToTransactAsset(e.into())) + }) } fn withdraw_asset(asset: &MultiAsset, location: &MultiLocation) -> result::Result { - let who = AccountIdConverter::from_location(location) - .ok_or_else(|| XcmError::from(Error::AccountIdConversionFailed))?; - let currency_id = - CurrencyIdConverter::from_asset(asset).ok_or_else(|| XcmError::from(Error::CurrencyIdConversionFailed))?; - let amount: MultiCurrency::Balance = Matcher::matches_fungible(&asset) - .ok_or_else(|| XcmError::from(Error::AssetNotFound))? - .saturated_into(); - MultiCurrency::withdraw(currency_id, &who, amount).map_err(|e| XcmError::FailedToTransactAsset(e.into()))?; + UnknownAsset::withdraw(asset, location).or_else(|_| { + let who = AccountIdConverter::from_location(location) + .ok_or_else(|| XcmError::from(Error::AccountIdConversionFailed))?; + let currency_id = CurrencyIdConverter::from_asset(asset) + .ok_or_else(|| XcmError::from(Error::CurrencyIdConversionFailed))?; + let amount: MultiCurrency::Balance = Matcher::matches_fungible(&asset) + .ok_or_else(|| XcmError::from(Error::MatchFungibleFailed))? + .saturated_into(); + MultiCurrency::withdraw(currency_id, &who, amount).map_err(|e| XcmError::FailedToTransactAsset(e.into())) + })?; + Ok(asset.clone()) } } diff --git a/xcm-support/src/lib.rs b/xcm-support/src/lib.rs index e3a4aaacc..120a12b6f 100644 --- a/xcm-support/src/lib.rs +++ b/xcm-support/src/lib.rs @@ -111,3 +111,21 @@ where None } } + +/// Handlers unknown asset deposit and withdraw. +pub trait UnknownAsset { + /// Deposit unknown asset. + fn deposit(asset: &MultiAsset, to: &MultiLocation) -> DispatchResult; + + /// Withdraw unknown asset. + fn withdraw(asset: &MultiAsset, from: &MultiLocation) -> DispatchResult; +} + +impl UnknownAsset for () { + fn deposit(_asset: &MultiAsset, _to: &MultiLocation) -> DispatchResult { + Ok(()) + } + fn withdraw(_asset: &MultiAsset, _from: &MultiLocation) -> DispatchResult { + Ok(()) + } +} From 9129639046893d17be63afdc722b6049f9a50928 Mon Sep 17 00:00:00 2001 From: Shaun Wang Date: Thu, 18 Mar 2021 17:12:47 +1300 Subject: [PATCH 2/7] More documentations. --- xcm-support/src/currency_adapter.rs | 13 ++++++++----- 1 file changed, 8 insertions(+), 5 deletions(-) diff --git a/xcm-support/src/currency_adapter.rs b/xcm-support/src/currency_adapter.rs index ae3c20f57..aec48815f 100644 --- a/xcm-support/src/currency_adapter.rs +++ b/xcm-support/src/currency_adapter.rs @@ -33,20 +33,23 @@ impl From for XcmError { } } -/// The `TransactAsset` implementation, to handle deposit/withdraw for XCM. +/// The `TransactAsset` implementation, to handle `MultiAsset` deposit/withdraw. +/// +/// If the asset is known, deposit/withdraw will be handled by `MultiCurrency`, +/// or by `UnknownAsset` if unknown. /// /// The implementation will try deposit or withdraw on unknown asset first, so -/// that detailed error info of known asset failure could be returned, if any. +/// that detailed error info of known asset failures could be returned if any. /// Thus known asset deposit/withdraw failures imply unknown asset failures as /// well. pub struct MultiCurrencyAdapter< MultiCurrency, + UnknownAsset, Matcher, AccountIdConverter, AccountId, CurrencyIdConverter, CurrencyId, - UnknownAsset, >( PhantomData<( MultiCurrency, @@ -61,21 +64,21 @@ pub struct MultiCurrencyAdapter< impl< MultiCurrency: orml_traits::MultiCurrency, + UnknownAsset: UnknownAssetT, Matcher: MatchesFungible, AccountIdConverter: LocationConversion, AccountId: sp_std::fmt::Debug, CurrencyIdConverter: CurrencyIdConversion, CurrencyId: FullCodec + Eq + PartialEq + Copy + MaybeSerializeDeserialize + Debug, - UnknownAsset: UnknownAssetT, > TransactAsset for MultiCurrencyAdapter< MultiCurrency, + UnknownAsset, Matcher, AccountIdConverter, AccountId, CurrencyIdConverter, CurrencyId, - UnknownAsset, > { fn deposit_asset(asset: &MultiAsset, location: &MultiLocation) -> Result { From c6512ed771485b8001b958c469cdb8ea58ad06c9 Mon Sep 17 00:00:00 2001 From: Shaun Wang Date: Thu, 18 Mar 2021 17:18:52 +1300 Subject: [PATCH 3/7] Clean code. --- xcm-support/src/currency_adapter.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/xcm-support/src/currency_adapter.rs b/xcm-support/src/currency_adapter.rs index aec48815f..b7772afb2 100644 --- a/xcm-support/src/currency_adapter.rs +++ b/xcm-support/src/currency_adapter.rs @@ -53,12 +53,12 @@ pub struct MultiCurrencyAdapter< >( PhantomData<( MultiCurrency, + UnknownAsset, Matcher, AccountIdConverter, AccountId, CurrencyIdConverter, CurrencyId, - UnknownAsset, )>, ); From dea09ccf61344043fd68a7b562898def4736fbe3 Mon Sep 17 00:00:00 2001 From: Shaun Wang Date: Thu, 18 Mar 2021 17:40:42 +1300 Subject: [PATCH 4/7] Renaming. --- xcm-support/src/currency_adapter.rs | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/xcm-support/src/currency_adapter.rs b/xcm-support/src/currency_adapter.rs index b7772afb2..7f760b1b2 100644 --- a/xcm-support/src/currency_adapter.rs +++ b/xcm-support/src/currency_adapter.rs @@ -15,8 +15,8 @@ use crate::{CurrencyIdConversion, UnknownAsset as UnknownAssetT}; /// Asset transaction errors. enum Error { - /// Match fungible failed. - MatchFungibleFailed, + /// Failed to match fungible. + FailedToMatchFungible, /// `MultiLocation` to `AccountId` Conversion failed. AccountIdConversionFailed, /// `CurrencyId` conversion failed. @@ -26,7 +26,7 @@ enum Error { impl From for XcmError { fn from(e: Error) -> Self { match e { - Error::MatchFungibleFailed => XcmError::FailedToTransactAsset("MatchFungibleFailed"), + Error::FailedToMatchFungible => XcmError::FailedToTransactAsset("FailedToMatchFungible"), Error::AccountIdConversionFailed => XcmError::FailedToTransactAsset("AccountIdConversionFailed"), Error::CurrencyIdConversionFailed => XcmError::FailedToTransactAsset("CurrencyIdConversionFailed"), } @@ -88,7 +88,7 @@ impl< let currency_id = CurrencyIdConverter::from_asset(asset) .ok_or_else(|| XcmError::from(Error::CurrencyIdConversionFailed))?; let amount: MultiCurrency::Balance = Matcher::matches_fungible(&asset) - .ok_or_else(|| XcmError::from(Error::MatchFungibleFailed))? + .ok_or_else(|| XcmError::from(Error::FailedToMatchFungible))? .saturated_into(); MultiCurrency::deposit(currency_id, &who, amount).map_err(|e| XcmError::FailedToTransactAsset(e.into())) }) @@ -101,7 +101,7 @@ impl< let currency_id = CurrencyIdConverter::from_asset(asset) .ok_or_else(|| XcmError::from(Error::CurrencyIdConversionFailed))?; let amount: MultiCurrency::Balance = Matcher::matches_fungible(&asset) - .ok_or_else(|| XcmError::from(Error::MatchFungibleFailed))? + .ok_or_else(|| XcmError::from(Error::FailedToMatchFungible))? .saturated_into(); MultiCurrency::withdraw(currency_id, &who, amount).map_err(|e| XcmError::FailedToTransactAsset(e.into())) })?; From 88982ae02c9f42e23b035d47f5f06b2bc83aee90 Mon Sep 17 00:00:00 2001 From: Shaun Wang Date: Thu, 18 Mar 2021 22:45:26 +1300 Subject: [PATCH 5/7] Should try to deposit known asset first. --- xcm-support/src/currency_adapter.rs | 29 +++++++++++++---------------- 1 file changed, 13 insertions(+), 16 deletions(-) diff --git a/xcm-support/src/currency_adapter.rs b/xcm-support/src/currency_adapter.rs index 7f760b1b2..6b8cf0b8f 100644 --- a/xcm-support/src/currency_adapter.rs +++ b/xcm-support/src/currency_adapter.rs @@ -36,12 +36,7 @@ impl From for XcmError { /// The `TransactAsset` implementation, to handle `MultiAsset` deposit/withdraw. /// /// If the asset is known, deposit/withdraw will be handled by `MultiCurrency`, -/// or by `UnknownAsset` if unknown. -/// -/// The implementation will try deposit or withdraw on unknown asset first, so -/// that detailed error info of known asset failures could be returned if any. -/// Thus known asset deposit/withdraw failures imply unknown asset failures as -/// well. +/// else by `UnknownAsset` if unknown. pub struct MultiCurrencyAdapter< MultiCurrency, UnknownAsset, @@ -82,16 +77,18 @@ impl< > { fn deposit_asset(asset: &MultiAsset, location: &MultiLocation) -> Result { - UnknownAsset::deposit(asset, location).or_else(|_| { - let who = AccountIdConverter::from_location(location) - .ok_or_else(|| XcmError::from(Error::AccountIdConversionFailed))?; - let currency_id = CurrencyIdConverter::from_asset(asset) - .ok_or_else(|| XcmError::from(Error::CurrencyIdConversionFailed))?; - let amount: MultiCurrency::Balance = Matcher::matches_fungible(&asset) - .ok_or_else(|| XcmError::from(Error::FailedToMatchFungible))? - .saturated_into(); - MultiCurrency::deposit(currency_id, &who, amount).map_err(|e| XcmError::FailedToTransactAsset(e.into())) - }) + match ( + AccountIdConverter::from_location(location), + CurrencyIdConverter::from_asset(asset), + Matcher::matches_fungible(&asset), + ) { + // known asset + (Some(who), Some(currency_id), Some(amount)) => { + MultiCurrency::deposit(currency_id, &who, amount).map_err(|e| XcmError::FailedToTransactAsset(e.into())) + } + // unknown asset + _ => UnknownAsset::deposit(asset, location).map_err(|e| XcmError::FailedToTransactAsset(e.into())), + } } fn withdraw_asset(asset: &MultiAsset, location: &MultiLocation) -> result::Result { From 3fd6d1ceb96dc7e763efa05c53ad02f7744e35ca Mon Sep 17 00:00:00 2001 From: Shaun Wang Date: Thu, 18 Mar 2021 22:49:08 +1300 Subject: [PATCH 6/7] Return error if no UnknownAsset impl. --- xcm-support/src/lib.rs | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/xcm-support/src/lib.rs b/xcm-support/src/lib.rs index 120a12b6f..0d2f96fcb 100644 --- a/xcm-support/src/lib.rs +++ b/xcm-support/src/lib.rs @@ -8,7 +8,10 @@ #![cfg_attr(not(feature = "std"), no_std)] -use frame_support::{dispatch::DispatchResult, traits::Get}; +use frame_support::{ + dispatch::{DispatchError, DispatchResult}, + traits::Get, +}; use sp_runtime::traits::{CheckedConversion, Convert}; use sp_std::{ collections::btree_set::BTreeSet, @@ -121,11 +124,13 @@ pub trait UnknownAsset { fn withdraw(asset: &MultiAsset, from: &MultiLocation) -> DispatchResult; } +const NO_UNKNOWN_ASSET_IMPL: &'static str = "NoUnknownAssetImpl"; + impl UnknownAsset for () { fn deposit(_asset: &MultiAsset, _to: &MultiLocation) -> DispatchResult { - Ok(()) + Err(DispatchError::Other(NO_UNKNOWN_ASSET_IMPL)) } fn withdraw(_asset: &MultiAsset, _from: &MultiLocation) -> DispatchResult { - Ok(()) + Err(DispatchError::Other(NO_UNKNOWN_ASSET_IMPL)) } } From b2983475d474c54e5068a97095144806bd966079 Mon Sep 17 00:00:00 2001 From: Shaun Wang Date: Thu, 18 Mar 2021 23:01:40 +1300 Subject: [PATCH 7/7] Make clippy happy. --- xcm-support/src/lib.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/xcm-support/src/lib.rs b/xcm-support/src/lib.rs index 0d2f96fcb..e95d3c2f7 100644 --- a/xcm-support/src/lib.rs +++ b/xcm-support/src/lib.rs @@ -124,7 +124,7 @@ pub trait UnknownAsset { fn withdraw(asset: &MultiAsset, from: &MultiLocation) -> DispatchResult; } -const NO_UNKNOWN_ASSET_IMPL: &'static str = "NoUnknownAssetImpl"; +const NO_UNKNOWN_ASSET_IMPL: &str = "NoUnknownAssetImpl"; impl UnknownAsset for () { fn deposit(_asset: &MultiAsset, _to: &MultiLocation) -> DispatchResult {