From d859734ed9ca34407b83747563fd77efa9fdafaf Mon Sep 17 00:00:00 2001
From: Shaun Wang <spxwang@gmail.com>
Date: Wed, 10 Mar 2021 03:46:30 +1300
Subject: [PATCH] Replace XCM `Error::Undefined` usage (#2580)

* Replace undefined error in currency adapter.

* Update tranact asset errors.

* Update TransactAsset trait documentations.

* Update currency adapter error documentation.
---
 polkadot/xcm/src/v0/traits.rs                 |  1 +
 .../xcm/xcm-builder/src/currency_adapter.rs   | 54 +++++++++++++++----
 polkadot/xcm/xcm-executor/src/traits.rs       |  6 ++-
 3 files changed, 51 insertions(+), 10 deletions(-)

diff --git a/polkadot/xcm/src/v0/traits.rs b/polkadot/xcm/src/v0/traits.rs
index eafeca8661c..542db15dd03 100644
--- a/polkadot/xcm/src/v0/traits.rs
+++ b/polkadot/xcm/src/v0/traits.rs
@@ -37,6 +37,7 @@ pub enum Error {
 	FailedToDecode,
 	BadOrigin,
 	ExceedsMaxMessageSize,
+	FailedToTransactAsset(#[codec(skip)] &'static str),
 }
 
 impl From<()> for Error {
diff --git a/polkadot/xcm/xcm-builder/src/currency_adapter.rs b/polkadot/xcm/xcm-builder/src/currency_adapter.rs
index 09b61ab6bb5..7883d5959d7 100644
--- a/polkadot/xcm/xcm-builder/src/currency_adapter.rs
+++ b/polkadot/xcm/xcm-builder/src/currency_adapter.rs
@@ -15,11 +15,33 @@
 // along with Polkadot.  If not, see <http://www.gnu.org/licenses/>.
 
 use sp_std::{result, convert::TryInto, marker::PhantomData};
-use xcm::v0::{Error, Result, MultiAsset, MultiLocation};
+use xcm::v0::{Error as XcmError, Result, MultiAsset, MultiLocation};
 use sp_arithmetic::traits::SaturatedConversion;
 use frame_support::traits::{ExistenceRequirement::AllowDeath, WithdrawReasons};
 use xcm_executor::traits::{MatchesFungible, LocationConversion, TransactAsset};
 
+/// Asset transaction errors.
+enum Error {
+	/// Asset not found.
+	AssetNotFound,
+	/// `MultiLocation` to `AccountId` Conversion failed.
+	AccountIdConversionFailed,
+	/// `u128` amount to currency `Balance` conversion failed.
+	AmountToBalanceConversionFailed,
+}
+
+impl From<Error> for XcmError {
+	fn from(e: Error) -> Self {
+		match e {
+			Error::AssetNotFound => XcmError::FailedToTransactAsset("AssetNotFound"),
+			Error::AccountIdConversionFailed =>
+				XcmError::FailedToTransactAsset("AccountIdConversionFailed"),
+			Error::AmountToBalanceConversionFailed =>
+				XcmError::FailedToTransactAsset("AmountToBalanceConversionFailed"),
+		}
+	}
+}
+
 pub struct CurrencyAdapter<Currency, Matcher, AccountIdConverter, AccountId>(
 	PhantomData<Currency>,
 	PhantomData<Matcher>,
@@ -36,19 +58,33 @@ impl<
 
 	fn deposit_asset(what: &MultiAsset, who: &MultiLocation) -> Result {
 		// Check we handle this asset.
-		let amount: u128 = Matcher::matches_fungible(&what).ok_or(())?.saturated_into();
-		let who = AccountIdConverter::from_location(who).ok_or(())?;
-		let balance_amount = amount.try_into().map_err(|_| ())?;
+		let amount: u128 = Matcher::matches_fungible(&what)
+			.ok_or(XcmError::from(Error::AssetNotFound))?
+			.saturated_into();
+		let who = AccountIdConverter::from_location(who)
+			.ok_or(XcmError::from(Error::AccountIdConversionFailed))?;
+		let balance_amount = amount
+			.try_into()
+			.map_err(|_| XcmError::from(Error::AmountToBalanceConversionFailed))?;
 		let _imbalance = Currency::deposit_creating(&who, balance_amount);
 		Ok(())
 	}
 
-	fn withdraw_asset(what: &MultiAsset, who: &MultiLocation) -> result::Result<MultiAsset, Error> {
+	fn withdraw_asset(
+		what: &MultiAsset,
+		who: &MultiLocation
+	) -> result::Result<MultiAsset, XcmError> {
 		// Check we handle this asset.
-		let amount: u128 = Matcher::matches_fungible(&what).ok_or(())?.saturated_into();
-		let who = AccountIdConverter::from_location(who).ok_or(())?;
-		let balance_amount = amount.try_into().map_err(|_| ())?;
-		Currency::withdraw(&who, balance_amount, WithdrawReasons::TRANSFER, AllowDeath).map_err(|_| ())?;
+		let amount: u128 = Matcher::matches_fungible(&what)
+			.ok_or(XcmError::from(Error::AssetNotFound))?
+			.saturated_into();
+		let who = AccountIdConverter::from_location(who)
+			.ok_or(XcmError::from(Error::AccountIdConversionFailed))?;
+		let balance_amount = amount
+			.try_into()
+			.map_err(|_| XcmError::from(Error::AmountToBalanceConversionFailed))?;
+		Currency::withdraw(&who, balance_amount, WithdrawReasons::TRANSFER, AllowDeath)
+			.map_err(|e| XcmError::FailedToTransactAsset(e.into()))?;
 		Ok(what.clone())
 	}
 }
diff --git a/polkadot/xcm/xcm-executor/src/traits.rs b/polkadot/xcm/xcm-executor/src/traits.rs
index d01791dfa23..a5fc2040f66 100644
--- a/polkadot/xcm/xcm-executor/src/traits.rs
+++ b/polkadot/xcm/xcm-executor/src/traits.rs
@@ -57,15 +57,19 @@ impl<T: Get<(MultiAsset, MultiLocation)>> FilterAssetLocation for Case<T> {
 /// different ways.
 pub trait TransactAsset {
 	/// Deposit the `what` asset into the account of `who`.
+	///
+	/// Implementations should return `XcmError::FailedToTransactAsset` if deposit failed.
 	fn deposit_asset(what: &MultiAsset, who: &MultiLocation) -> XcmResult;
 
 	/// Withdraw the given asset from the consensus system. Return the actual asset withdrawn. In
 	/// the case of `what` being a wildcard, this may be something more specific.
+	///
+	/// Implementations should return `XcmError::FailedToTransactAsset` if withdraw failed.
 	fn withdraw_asset(what: &MultiAsset, who: &MultiLocation) -> Result<MultiAsset, XcmError>;
 
 	/// Move an `asset` `from` one location in `to` another location.
 	///
-	/// Undefined if from account doesn't own this asset.
+	/// Returns `XcmError::FailedToTransactAsset` if transfer failed.
 	fn transfer_asset(asset: &MultiAsset, from: &MultiLocation, to: &MultiLocation) -> Result<MultiAsset, XcmError> {
 		let withdrawn = Self::withdraw_asset(asset, from)?;
 		Self::deposit_asset(&withdrawn, to)?;
-- 
GitLab