Use CurrencyId convert. (#437)

* Use CurrencyId convert.

* Apply review suggestions.

Author: shaunxw

xcm-support/src/currency_adapter.rs

@@ -1,8 +1,7 @@
 use codec::FullCodec;
-use sp_runtime::traits::{MaybeSerializeDeserialize, SaturatedConversion};
+use sp_runtime::traits::{Convert, MaybeSerializeDeserialize, SaturatedConversion};
 use sp_std::{
 	cmp::{Eq, PartialEq},
-	convert::{TryFrom, TryInto},
 	fmt::Debug,
 	marker::PhantomData,
 	prelude::*,
@@ -38,35 +37,53 @@ impl From<Error> for XcmError {
 ///
 /// If the asset is known, deposit/withdraw will be handled by `MultiCurrency`,
 /// else by `UnknownAsset` if unknown.
-pub struct MultiCurrencyAdapter<MultiCurrency, UnknownAsset, Matcher, AccountIdConverter, AccountId, CurrencyId>(
+pub struct MultiCurrencyAdapter<
+	MultiCurrency,
+	UnknownAsset,
+	Matcher,
+	AccountId,
+	AccountIdConvert,
+	CurrencyId,
+	CurrencyIdConvert,
+>(
 	PhantomData<(
 		MultiCurrency,
 		UnknownAsset,
 		Matcher,
-		AccountIdConverter,
 		AccountId,
+		AccountIdConvert,
 		CurrencyId,
+		CurrencyIdConvert,
 	)>,
 );
 
 impl<
 		MultiCurrency: orml_traits::MultiCurrency<AccountId, CurrencyId = CurrencyId>,
 		UnknownAsset: UnknownAssetT,
 		Matcher: MatchesFungible<MultiCurrency::Balance>,
-		AccountIdConverter: LocationConversion<AccountId>,
 		AccountId: sp_std::fmt::Debug,
-		CurrencyId: FullCodec + Eq + PartialEq + Copy + MaybeSerializeDeserialize + Debug + TryFrom<MultiAsset>,
+		AccountIdConvert: LocationConversion<AccountId>,
+		CurrencyId: FullCodec + Eq + PartialEq + Copy + MaybeSerializeDeserialize + Debug,
+		CurrencyIdConvert: Convert<MultiAsset, Option<CurrencyId>>,
 	> TransactAsset
-	for MultiCurrencyAdapter<MultiCurrency, UnknownAsset, Matcher, AccountIdConverter, AccountId, CurrencyId>
+	for MultiCurrencyAdapter<
+		MultiCurrency,
+		UnknownAsset,
+		Matcher,
+		AccountId,
+		AccountIdConvert,
+		CurrencyId,
+		CurrencyIdConvert,
+	>
 {
 	fn deposit_asset(asset: &MultiAsset, location: &MultiLocation) -> Result {
 		match (
-			AccountIdConverter::from_location(location),
-			asset.clone().try_into(),
+			AccountIdConvert::from_location(location),
+			CurrencyIdConvert::convert(asset.clone()),
 			Matcher::matches_fungible(&asset),
 		) {
 			// known asset
-			(Some(who), Ok(currency_id), Some(amount)) => {
+			(Some(who), Some(currency_id), Some(amount)) => {
 				MultiCurrency::deposit(currency_id, &who, amount).map_err(|e| XcmError::FailedToTransactAsset(e.into()))
 			}
 			// unknown asset
@@ -76,12 +93,10 @@ impl<
 
 	fn withdraw_asset(asset: &MultiAsset, location: &MultiLocation) -> result::Result<MultiAsset, XcmError> {
 		UnknownAsset::withdraw(asset, location).or_else(|_| {
-			let who = AccountIdConverter::from_location(location)
+			let who = AccountIdConvert::from_location(location)
 				.ok_or_else(|| XcmError::from(Error::AccountIdConversionFailed))?;
-			let currency_id = asset
-				.clone()
-				.try_into()
-				.map_err(|_| XcmError::from(Error::CurrencyIdConversionFailed))?;
+			let currency_id = CurrencyIdConvert::convert(asset.clone())
+				.ok_or_else(|| XcmError::from(Error::CurrencyIdConversionFailed))?;
 			let amount: MultiCurrency::Balance = Matcher::matches_fungible(&asset)
 				.ok_or_else(|| XcmError::from(Error::FailedToMatchFungible))?
 				.saturated_into();

xcm-support/src/lib.rs

@@ -10,7 +10,7 @@
 #![allow(clippy::unused_unit)]
 
 use frame_support::dispatch::{DispatchError, DispatchResult};
-use sp_runtime::traits::CheckedConversion;
+use sp_runtime::traits::{CheckedConversion, Convert};
 use sp_std::{convert::TryFrom, marker::PhantomData, prelude::*};
 
 use xcm::v0::{MultiAsset, MultiLocation, Xcm};
@@ -31,15 +31,15 @@ pub trait XcmHandler<AccountId> {
 
 /// A `MatchesFungible` implementation. It matches concrete fungible assets
 /// whose `id` could be converted into `CurrencyId`.
-pub struct IsNativeConcrete<CurrencyId>(PhantomData<CurrencyId>);
-impl<CurrencyId, Amount> MatchesFungible<Amount> for IsNativeConcrete<CurrencyId>
+pub struct IsNativeConcrete<CurrencyId, CurrencyIdConvert>(PhantomData<(CurrencyId, CurrencyIdConvert)>);
+impl<CurrencyId, CurrencyIdConvert, Amount> MatchesFungible<Amount> for IsNativeConcrete<CurrencyId, CurrencyIdConvert>
 where
-	CurrencyId: TryFrom<MultiLocation>,
+	CurrencyIdConvert: Convert<MultiLocation, Option<CurrencyId>>,
 	Amount: TryFrom<u128>,
 {
 	fn matches_fungible(a: &MultiAsset) -> Option<Amount> {
 		if let MultiAsset::ConcreteFungible { id, amount } = a {
-			if CurrencyId::try_from(id.clone()).is_ok() {
+			if CurrencyIdConvert::convert(id.clone()).is_some() {
 				return CheckedConversion::checked_from(*amount);
 			}
 		}

xcm-support/src/tests.rs

@@ -4,7 +4,7 @@
 
 use super::*;
 
-use xcm::v0::{Junction::*, MultiAsset::*, MultiLocation::*};
+use xcm::v0::{Junction::*, MultiAsset::ConcreteFungible, MultiLocation::*};
 
 #[derive(Debug, PartialEq, Eq)]
 pub enum TestCurrencyId {
@@ -13,22 +13,22 @@ pub enum TestCurrencyId {
 	RelayChainToken,
 }
 
-impl TryFrom<MultiLocation> for TestCurrencyId {
-	type Error = ();
-	fn try_from(l: MultiLocation) -> Result<TestCurrencyId, ()> {
+pub struct CurrencyIdConvert;
+impl Convert<MultiLocation, Option<TestCurrencyId>> for CurrencyIdConvert {
+	fn convert(l: MultiLocation) -> Option<TestCurrencyId> {
 		use TestCurrencyId::*;
 		let token_a: Vec<u8> = "TokenA".into();
 		let token_b: Vec<u8> = "TokenB".into();
 		match l {
-			X1(Parent) => Ok(RelayChainToken),
-			X3(Parent, Parachain { id: 1 }, GeneralKey(k)) if k == token_a => Ok(TokenA),
-			X3(Parent, Parachain { id: 2 }, GeneralKey(k)) if k == token_b => Ok(TokenB),
-			_ => Err(()),
+			X1(Parent) => Some(RelayChainToken),
+			X3(Parent, Parachain { id: 1 }, GeneralKey(k)) if k == token_a => Some(TokenA),
+			X3(Parent, Parachain { id: 2 }, GeneralKey(k)) if k == token_b => Some(TokenB),
+			_ => None,
 		}
 	}
 }
 
-type MatchesCurrencyId = IsNativeConcrete<TestCurrencyId>;
+type MatchesCurrencyId = IsNativeConcrete<TestCurrencyId, CurrencyIdConvert>;
 
 #[test]
 fn is_native_concrete_matches_native_currencies() {

xtokens/src/lib.rs

@@ -63,7 +63,10 @@ pub mod module {
 			+ Into<u128>;
 
 		/// Currency Id.
-		type CurrencyId: Parameter + Member + Clone + Into<MultiLocation>;
+		type CurrencyId: Parameter + Member + Clone;
+
+		/// Convert `T::CurrencyIn` to `MultiLocation`.
+		type CurrencyIdConvert: Convert<Self::CurrencyId, Option<MultiLocation>>;
 
 		/// Convert `Self::Account` to `AccountId32`
 		type AccountId32Convert: Convert<Self::AccountId, [u8; 32]>;
@@ -94,6 +97,8 @@ pub mod module {
 		NotCrossChainTransfer,
 		/// Invalid transfer destination.
 		InvalidDest,
+		/// Currency is not cross-chain transferable.
+		NotCrossChainTransferableCurrency,
 	}
 
 	#[pallet::hooks]
@@ -119,8 +124,10 @@ pub mod module {
 				return Ok(().into());
 			}
 
+			let id: MultiLocation = T::CurrencyIdConvert::convert(currency_id.clone())
+				.ok_or(Error::<T>::NotCrossChainTransferableCurrency)?;
 			let asset = MultiAsset::ConcreteFungible {
-				id: currency_id.clone().into(),
+				id,
 				amount: amount.into(),
 			};
 			Self::do_transfer_multiasset(who.clone(), asset, dest.clone())?;

xtokens/src/mock.rs

@@ -10,7 +10,6 @@ use polkadot_parachain::primitives::Sibling;
 use serde::{Deserialize, Serialize};
 use sp_io::TestExternalities;
 use sp_runtime::AccountId32;
-use sp_std::convert::TryFrom;
 use xcm::v0::{Junction, MultiLocation::*, NetworkId};
 use xcm_builder::{
 	AccountId32Aliases, LocationInverter, ParentIsDefault, RelayChainAsNative, SiblingParachainAsNative,
@@ -33,48 +32,52 @@ pub enum CurrencyId {
 	B,
 }
 
-impl From<CurrencyId> for MultiLocation {
-	fn from(id: CurrencyId) -> Self {
+pub struct CurrencyIdConvert;
+impl Convert<CurrencyId, Option<MultiLocation>> for CurrencyIdConvert {
+	fn convert(id: CurrencyId) -> Option<MultiLocation> {
 		match id {
-			CurrencyId::R => Junction::Parent.into(),
-			CurrencyId::A => (
-				Junction::Parent,
-				Junction::Parachain { id: 1 },
-				Junction::GeneralKey("A".into()),
-			)
-				.into(),
-			CurrencyId::B => (
-				Junction::Parent,
-				Junction::Parachain { id: 2 },
-				Junction::GeneralKey("B".into()),
-			)
-				.into(),
+			CurrencyId::R => Some(Junction::Parent.into()),
+			CurrencyId::A => Some(
+				(
+					Junction::Parent,
+					Junction::Parachain { id: 1 },
+					Junction::GeneralKey("A".into()),
+				)
+					.into(),
+			),
+			CurrencyId::B => Some(
+				(
+					Junction::Parent,
+					Junction::Parachain { id: 2 },
+					Junction::GeneralKey("B".into()),
+				)
+					.into(),
+			),
 		}
 	}
 }
-
-impl TryFrom<MultiLocation> for CurrencyId {
-	type Error = ();
-	fn try_from(l: MultiLocation) -> Result<Self, Self::Error> {
+impl Convert<MultiLocation, Option<CurrencyId>> for CurrencyIdConvert {
+	fn convert(l: MultiLocation) -> Option<CurrencyId> {
 		let a: Vec<u8> = "A".into();
 		let b: Vec<u8> = "B".into();
 		match l {
-			X1(Parent) => Ok(CurrencyId::R),
-			X3(Junction::Parent, Junction::Parachain { id: 1 }, Junction::GeneralKey(k)) if k == a => Ok(CurrencyId::A),
-			X3(Junction::Parent, Junction::Parachain { id: 2 }, Junction::GeneralKey(k)) if k == b => Ok(CurrencyId::B),
-
-			_ => Err(()),
+			X1(Parent) => Some(CurrencyId::R),
+			X3(Junction::Parent, Junction::Parachain { id: 1 }, Junction::GeneralKey(k)) if k == a => {
+				Some(CurrencyId::A)
+			}
+			X3(Junction::Parent, Junction::Parachain { id: 2 }, Junction::GeneralKey(k)) if k == b => {
+				Some(CurrencyId::B)
+			}
+			_ => None,
 		}
 	}
 }
-
-impl TryFrom<MultiAsset> for CurrencyId {
-	type Error = ();
-	fn try_from(a: MultiAsset) -> Result<Self, Self::Error> {
+impl Convert<MultiAsset, Option<CurrencyId>> for CurrencyIdConvert {
+	fn convert(a: MultiAsset) -> Option<CurrencyId> {
 		if let MultiAsset::ConcreteFungible { id, amount: _ } = a {
-			Self::try_from(id)
+			Self::convert(id)
 		} else {
-			Err(())
+			None
 		}
 	}
 }
@@ -110,10 +113,11 @@ decl_test_parachain! {
 			pub type LocalAssetTransactor = MultiCurrencyAdapter<
 				Tokens,
 				(),
-				IsNativeConcrete<CurrencyId>,
-				LocationConverter,
+				IsNativeConcrete<CurrencyId, CurrencyIdConvert>,
 				AccountId,
+				LocationConverter,
 				CurrencyId,
+				CurrencyIdConvert,
 			>;
 
 			pub type LocalOriginConverter = (
@@ -173,6 +177,7 @@ decl_test_parachain! {
 				type Event = Event;
 				type Balance = Balance;
 				type CurrencyId = CurrencyId;
+				type CurrencyIdConvert = CurrencyIdConvert;
 				type AccountId32Convert = AccountId32Convert;
 				type SelfLocation = SelfLocation;
 				type XcmHandler = HandleXcm;
@@ -213,10 +218,11 @@ decl_test_parachain! {
 			pub type LocalAssetTransactor = MultiCurrencyAdapter<
 				Tokens,
 				(),
-				IsNativeConcrete<CurrencyId>,
-				LocationConverter,
+				IsNativeConcrete<CurrencyId, CurrencyIdConvert>,
 				AccountId,
+				LocationConverter,
 				CurrencyId,
+				CurrencyIdConvert,
 			>;
 
 			pub type LocalOriginConverter = (
@@ -276,6 +282,7 @@ decl_test_parachain! {
 				type Event = Event;
 				type Balance = Balance;
 				type CurrencyId = CurrencyId;
+				type CurrencyIdConvert = CurrencyIdConvert;
 				type AccountId32Convert = AccountId32Convert;
 				type SelfLocation = SelfLocation;
 				type XcmHandler = HandleXcm;
@@ -316,10 +323,11 @@ decl_test_parachain! {
 			pub type LocalAssetTransactor = MultiCurrencyAdapter<
 				Tokens,
 				(),
-				IsNativeConcrete<CurrencyId>,
-				LocationConverter,
+				IsNativeConcrete<CurrencyId, CurrencyIdConvert>,
 				AccountId,
+				LocationConverter,
 				CurrencyId,
+				CurrencyIdConvert,
 			>;
 
 			pub type LocalOriginConverter = (
@@ -379,6 +387,7 @@ decl_test_parachain! {
 				type Event = Event;
 				type Balance = Balance;
 				type CurrencyId = CurrencyId;
+				type CurrencyIdConvert = CurrencyIdConvert;
 				type AccountId32Convert = AccountId32Convert;
 				type SelfLocation = SelfLocation;
 				type XcmHandler = HandleXcm;