XCM: Remove & replace XCM Convert trait (#7329)

* Introduce an extensible location-to-hash-account

* Convert becomes RevFallRefConvert

* Use ConvertLocation trait

* Remove Convert usage

* Builds

* Fix warnings

* Remove unused types

* Bump lock

* No need for aliasing

* Remove unused

* Deprecate legacy conversion

* Fixes

* Fixes

* Update Cargo.toml

Co-authored-by: Muharem Ismailov <ismailov.m.h@gmail.com>

* Update xcm/xcm-builder/src/location_conversion.rs

Co-authored-by: Liam Aharon <liam.aharon@hotmail.com>

* Update xcm/xcm-builder/src/location_conversion.rs

Co-authored-by: Liam Aharon <liam.aharon@hotmail.com>

* Update xcm/xcm-builder/src/location_conversion.rs

Co-authored-by: Liam Aharon <liam.aharon@hotmail.com>

* Update xcm/xcm-builder/src/location_conversion.rs

Co-authored-by: Liam Aharon <liam.aharon@hotmail.com>

* Update xcm/xcm-builder/src/location_conversion.rs

Co-authored-by: Liam Aharon <liam.aharon@hotmail.com>

* Update xcm/xcm-builder/src/location_conversion.rs

Co-authored-by: Liam Aharon <liam.aharon@hotmail.com>

---------

Co-authored-by: Muharem Ismailov <ismailov.m.h@gmail.com>
Co-authored-by: Liam Aharon <liam.aharon@hotmail.com>

polkadot/xcm/pallet-xcm-benchmarks/src/fungible/benchmarking.rs

@@ -24,7 +24,7 @@ use frame_support::{
 use sp_runtime::traits::{Bounded, Zero};
 use sp_std::{prelude::*, vec};
 use xcm::latest::prelude::*;
-use xcm_executor::traits::{Convert, TransactAsset};
+use xcm_executor::traits::{ConvertLocation, TransactAsset};
 
 benchmarks_instance_pallet! {
 	where_clause { where
@@ -75,7 +75,7 @@ benchmarks_instance_pallet! {
 		// this xcm doesn't use holding
 
 		let dest_location = T::valid_destination()?;
-		let dest_account = T::AccountIdConverter::convert(dest_location.clone()).unwrap();
+		let dest_account = T::AccountIdConverter::convert_location(&dest_location).unwrap();
 
 		<AssetTransactorOf<T>>::deposit_asset(
 			&asset,
@@ -101,7 +101,7 @@ benchmarks_instance_pallet! {
 	transfer_reserve_asset {
 		let (sender_account, sender_location) = account_and_location::<T>(1);
 		let dest_location = T::valid_destination()?;
-		let dest_account = T::AccountIdConverter::convert(dest_location.clone()).unwrap();
+		let dest_account = T::AccountIdConverter::convert_location(&dest_location).unwrap();
 
 		let asset = T::get_multi_asset();
 		<AssetTransactorOf<T>>::deposit_asset(
@@ -171,7 +171,7 @@ benchmarks_instance_pallet! {
 
 		// our dest must have no balance initially.
 		let dest_location = T::valid_destination()?;
-		let dest_account = T::AccountIdConverter::convert(dest_location.clone()).unwrap();
+		let dest_account = T::AccountIdConverter::convert_location(&dest_location).unwrap();
 		assert!(T::TransactAsset::balance(&dest_account).is_zero());
 
 		let mut executor = new_executor::<T>(Default::default());
@@ -197,7 +197,7 @@ benchmarks_instance_pallet! {
 
 		// our dest must have no balance initially.
 		let dest_location = T::valid_destination()?;
-		let dest_account = T::AccountIdConverter::convert(dest_location.clone()).unwrap();
+		let dest_account = T::AccountIdConverter::convert_location(&dest_location).unwrap();
 		assert!(T::TransactAsset::balance(&dest_account).is_zero());
 
 		let mut executor = new_executor::<T>(Default::default());

polkadot/xcm/pallet-xcm-benchmarks/src/lib.rs

@@ -22,7 +22,7 @@ use codec::Encode;
 use frame_benchmarking::{account, BenchmarkError};
 use sp_std::prelude::*;
 use xcm::latest::prelude::*;
-use xcm_executor::{traits::Convert, Config as XcmConfig};
+use xcm_executor::{traits::ConvertLocation, Config as XcmConfig};
 
 pub mod fungible;
 pub mod generic;
@@ -39,7 +39,7 @@ pub trait Config: frame_system::Config {
 	type XcmConfig: XcmConfig;
 
 	/// A converter between a multi-location to a sovereign account.
-	type AccountIdConverter: Convert<MultiLocation, Self::AccountId>;
+	type AccountIdConverter: ConvertLocation<Self::AccountId>;
 
 	/// Does any necessary setup to create a valid destination for XCM messages.
 	/// Returns that destination's multi-location to be used in benchmarks.
@@ -104,7 +104,7 @@ fn account_id_junction<T: frame_system::Config>(index: u32) -> Junction {
 
 pub fn account_and_location<T: Config>(index: u32) -> (T::AccountId, MultiLocation) {
 	let location: MultiLocation = account_id_junction::<T>(index).into();
-	let account = T::AccountIdConverter::convert(location.clone()).unwrap();
+	let account = T::AccountIdConverter::convert_location(&location).unwrap();
 
 	(account, location)
 }

polkadot/xcm/pallet-xcm-benchmarks/src/mock.rs

@@ -47,18 +47,14 @@ impl xcm_executor::traits::OnResponse for DevNull {
 }
 
 pub struct AccountIdConverter;
-impl xcm_executor::traits::Convert<MultiLocation, u64> for AccountIdConverter {
+impl xcm_executor::traits::ConvertLocation<u64> for AccountIdConverter {
-	fn convert(ml: MultiLocation) -> Result<u64, MultiLocation> {
+	fn convert_location(ml: &MultiLocation) -> Option<u64> {
 		match ml {
 			MultiLocation { parents: 0, interior: X1(Junction::AccountId32 { id, .. }) } =>
-				Ok(<u64 as codec::Decode>::decode(&mut &*id.to_vec()).unwrap()),
+				Some(<u64 as codec::Decode>::decode(&mut &*id.to_vec()).unwrap()),
-			_ => Err(ml),
+			_ => None,
 		}
 	}
-
-	fn reverse(acc: u64) -> Result<MultiLocation, u64> {
-		Err(acc)
-	}
 }
 
 parameter_types! {

polkadot/xcm/pallet-xcm/src/lib.rs

@@ -40,7 +40,7 @@ use sp_runtime::{
 };
 use sp_std::{boxed::Box, marker::PhantomData, prelude::*, result::Result, vec};
 use xcm::{latest::QueryResponseInfo, prelude::*};
-use xcm_executor::traits::{Convert, ConvertOrigin, Properties};
+use xcm_executor::traits::{ConvertOrigin, Properties};
 
 use frame_support::{
 	dispatch::{Dispatchable, GetDispatchInfo},
@@ -52,8 +52,8 @@ use frame_system::pallet_prelude::*;
 pub use pallet::*;
 use xcm_executor::{
 	traits::{
-		CheckSuspension, ClaimAssets, DropAssets, MatchesFungible, OnResponse, QueryHandler,
+		CheckSuspension, ClaimAssets, ConvertLocation, DropAssets, MatchesFungible, OnResponse,
-		QueryResponseStatus, VersionChangeNotifier, WeightBounds,
+		QueryHandler, QueryResponseStatus, VersionChangeNotifier, WeightBounds,
 	},
 	Assets,
 };
@@ -247,7 +247,7 @@ pub mod pallet {
 		type TrustedLockers: ContainsPair<MultiLocation, MultiAsset>;
 
 		/// How to get an `AccountId` value from a `MultiLocation`, useful for handling asset locks.
-		type SovereignAccountOf: Convert<MultiLocation, Self::AccountId>;
+		type SovereignAccountOf: ConvertLocation<Self::AccountId>;
 
 		/// The maximum number of local XCM locks that a single account may have.
 		type MaxLockers: Get<u32>;
@@ -1750,7 +1750,7 @@ impl<T: Config> xcm_executor::traits::AssetLock for Pallet<T> {
 		owner: MultiLocation,
 	) -> Result<LockTicket<T>, xcm_executor::traits::LockError> {
 		use xcm_executor::traits::LockError::*;
-		let sovereign_account = T::SovereignAccountOf::convert_ref(&owner).map_err(|_| BadOwner)?;
+		let sovereign_account = T::SovereignAccountOf::convert_location(&owner).ok_or(BadOwner)?;
 		let amount = T::CurrencyMatcher::matches_fungible(&asset).ok_or(UnknownAsset)?;
 		ensure!(T::Currency::free_balance(&sovereign_account) >= amount, AssetNotOwned);
 		let locks = LockedFungibles::<T>::get(&sovereign_account).unwrap_or_default();
@@ -1765,7 +1765,7 @@ impl<T: Config> xcm_executor::traits::AssetLock for Pallet<T> {
 		owner: MultiLocation,
 	) -> Result<UnlockTicket<T>, xcm_executor::traits::LockError> {
 		use xcm_executor::traits::LockError::*;
-		let sovereign_account = T::SovereignAccountOf::convert_ref(&owner).map_err(|_| BadOwner)?;
+		let sovereign_account = T::SovereignAccountOf::convert_location(&owner).ok_or(BadOwner)?;
 		let amount = T::CurrencyMatcher::matches_fungible(&asset).ok_or(UnknownAsset)?;
 		ensure!(T::Currency::free_balance(&sovereign_account) >= amount, AssetNotOwned);
 		let locks = LockedFungibles::<T>::get(&sovereign_account).unwrap_or_default();
@@ -1787,7 +1787,7 @@ impl<T: Config> xcm_executor::traits::AssetLock for Pallet<T> {
 			NonFungible(_) => return Err(Unimplemented),
 		};
 		owner.remove_network_id();
-		let account = T::SovereignAccountOf::convert_ref(&owner).map_err(|_| BadOwner)?;
+		let account = T::SovereignAccountOf::convert_location(&owner).ok_or(BadOwner)?;
 		let locker = locker.into();
 		let owner = owner.into();
 		let id: VersionedAssetId = asset.id.into();
@@ -1815,7 +1815,7 @@ impl<T: Config> xcm_executor::traits::AssetLock for Pallet<T> {
 			NonFungible(_) => return Err(Unimplemented),
 		};
 		owner.remove_network_id();
-		let sovereign_account = T::SovereignAccountOf::convert_ref(&owner).map_err(|_| BadOwner)?;
+		let sovereign_account = T::SovereignAccountOf::convert_location(&owner).ok_or(BadOwner)?;
 		let locker = locker.into();
 		let owner = owner.into();
 		let id: VersionedAssetId = asset.id.into();

polkadot/xcm/xcm-builder/src/asset_conversion.rs

@@ -17,9 +17,10 @@
 //! Adapters to work with `frame_support::traits::tokens::fungibles` through XCM.
 
 use frame_support::traits::{Contains, Get};
-use sp_std::{borrow::Borrow, marker::PhantomData, prelude::*, result};
+use sp_runtime::traits::MaybeEquivalence;
+use sp_std::{marker::PhantomData, prelude::*, result};
 use xcm::latest::prelude::*;
-use xcm_executor::traits::{Convert, Error as MatchError, MatchesFungibles, MatchesNonFungibles};
+use xcm_executor::traits::{Error as MatchError, MatchesFungibles, 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
@@ -27,12 +28,15 @@ use xcm_executor::traits::{Convert, Error as MatchError, MatchesFungibles, Match
 pub struct AsPrefixedGeneralIndex<Prefix, AssetId, ConvertAssetId>(
 	PhantomData<(Prefix, AssetId, ConvertAssetId)>,
 );
-impl<Prefix: Get<MultiLocation>, AssetId: Clone, ConvertAssetId: Convert<u128, AssetId>>
+impl<
-	Convert<MultiLocation, AssetId> for AsPrefixedGeneralIndex<Prefix, AssetId, ConvertAssetId>
+		Prefix: Get<MultiLocation>,
+		AssetId: Clone,
+		ConvertAssetId: MaybeEquivalence<u128, AssetId>,
+	> MaybeEquivalence<MultiLocation, AssetId>
+	for AsPrefixedGeneralIndex<Prefix, AssetId, ConvertAssetId>
 {
-	fn convert_ref(id: impl Borrow<MultiLocation>) -> result::Result<AssetId, ()> {
+	fn convert(id: &MultiLocation) -> Option<AssetId> {
 		let prefix = Prefix::get();
-		let id = id.borrow();
 		if prefix.parent_count() != id.parent_count() ||
 			prefix
 				.interior()
@@ -40,18 +44,18 @@ impl<Prefix: Get<MultiLocation>, AssetId: Clone, ConvertAssetId: Convert<u128, A
 				.enumerate()
 				.any(|(index, junction)| id.interior().at(index) != Some(junction))
 		{
-			return Err(())
+			return None
 		}
 		match id.interior().at(prefix.interior().len()) {
-			Some(Junction::GeneralIndex(id)) => ConvertAssetId::convert_ref(id),
+			Some(Junction::GeneralIndex(id)) => ConvertAssetId::convert(id),
-			_ => Err(()),
+			_ => None,
 		}
 	}
-	fn reverse_ref(what: impl Borrow<AssetId>) -> result::Result<MultiLocation, ()> {
+	fn convert_back(what: &AssetId) -> Option<MultiLocation> {
 		let mut location = Prefix::get();
-		let id = ConvertAssetId::reverse_ref(what)?;
+		let id = ConvertAssetId::convert_back(what)?;
-		location.push_interior(Junction::GeneralIndex(id)).map_err(|_| ())?;
+		location.push_interior(Junction::GeneralIndex(id)).ok()?;
-		Ok(location)
+		Some(location)
 	}
 }
 
@@ -61,8 +65,8 @@ pub struct ConvertedConcreteId<AssetId, Balance, ConvertAssetId, ConvertOther>(
 impl<
 		AssetId: Clone,
 		Balance: Clone,
-		ConvertAssetId: Convert<MultiLocation, AssetId>,
+		ConvertAssetId: MaybeEquivalence<MultiLocation, AssetId>,
-		ConvertBalance: Convert<u128, Balance>,
+		ConvertBalance: MaybeEquivalence<u128, Balance>,
 	> MatchesFungibles<AssetId, Balance>
 	for ConvertedConcreteId<AssetId, Balance, ConvertAssetId, ConvertBalance>
 {
@@ -71,18 +75,17 @@ impl<
 			(Fungible(ref amount), Concrete(ref id)) => (amount, id),
 			_ => return Err(MatchError::AssetNotHandled),
 		};
-		let what =
+		let what = ConvertAssetId::convert(id).ok_or(MatchError::AssetIdConversionFailed)?;
-			ConvertAssetId::convert_ref(id).map_err(|_| MatchError::AssetIdConversionFailed)?;
+		let amount =
-		let amount = ConvertBalance::convert_ref(amount)
+			ConvertBalance::convert(amount).ok_or(MatchError::AmountToBalanceConversionFailed)?;
-			.map_err(|_| MatchError::AmountToBalanceConversionFailed)?;
 		Ok((what, amount))
 	}
 }
 impl<
 		ClassId: Clone,
 		InstanceId: Clone,
-		ConvertClassId: Convert<MultiLocation, ClassId>,
+		ConvertClassId: MaybeEquivalence<MultiLocation, ClassId>,
-		ConvertInstanceId: Convert<AssetInstance, InstanceId>,
+		ConvertInstanceId: MaybeEquivalence<AssetInstance, InstanceId>,
 	> MatchesNonFungibles<ClassId, InstanceId>
 	for ConvertedConcreteId<ClassId, InstanceId, ConvertClassId, ConvertInstanceId>
 {
@@ -91,10 +94,9 @@ impl<
 			(NonFungible(ref instance), Concrete(ref class)) => (instance, class),
 			_ => return Err(MatchError::AssetNotHandled),
 		};
-		let what =
+		let what = ConvertClassId::convert(class).ok_or(MatchError::AssetIdConversionFailed)?;
-			ConvertClassId::convert_ref(class).map_err(|_| MatchError::AssetIdConversionFailed)?;
+		let instance =
-		let instance = ConvertInstanceId::convert_ref(instance)
+			ConvertInstanceId::convert(instance).ok_or(MatchError::InstanceConversionFailed)?;
-			.map_err(|_| MatchError::InstanceConversionFailed)?;
 		Ok((what, instance))
 	}
 }
@@ -105,8 +107,8 @@ pub struct ConvertedAbstractId<AssetId, Balance, ConvertAssetId, ConvertOther>(
 impl<
 		AssetId: Clone,
 		Balance: Clone,
-		ConvertAssetId: Convert<[u8; 32], AssetId>,
+		ConvertAssetId: MaybeEquivalence<[u8; 32], AssetId>,
-		ConvertBalance: Convert<u128, Balance>,
+		ConvertBalance: MaybeEquivalence<u128, Balance>,
 	> MatchesFungibles<AssetId, Balance>
 	for ConvertedAbstractId<AssetId, Balance, ConvertAssetId, ConvertBalance>
 {
@@ -115,18 +117,17 @@ impl<
 			(Fungible(ref amount), Abstract(ref id)) => (amount, id),
 			_ => return Err(MatchError::AssetNotHandled),
 		};
-		let what =
+		let what = ConvertAssetId::convert(id).ok_or(MatchError::AssetIdConversionFailed)?;
-			ConvertAssetId::convert_ref(id).map_err(|_| MatchError::AssetIdConversionFailed)?;
+		let amount =
-		let amount = ConvertBalance::convert_ref(amount)
+			ConvertBalance::convert(amount).ok_or(MatchError::AmountToBalanceConversionFailed)?;
-			.map_err(|_| MatchError::AmountToBalanceConversionFailed)?;
 		Ok((what, amount))
 	}
 }
 impl<
 		ClassId: Clone,
 		InstanceId: Clone,
-		ConvertClassId: Convert<[u8; 32], ClassId>,
+		ConvertClassId: MaybeEquivalence<[u8; 32], ClassId>,
-		ConvertInstanceId: Convert<AssetInstance, InstanceId>,
+		ConvertInstanceId: MaybeEquivalence<AssetInstance, InstanceId>,
 	> MatchesNonFungibles<ClassId, InstanceId>
 	for ConvertedAbstractId<ClassId, InstanceId, ConvertClassId, ConvertInstanceId>
 {
@@ -135,10 +136,9 @@ impl<
 			(NonFungible(ref instance), Abstract(ref class)) => (instance, class),
 			_ => return Err(MatchError::AssetNotHandled),
 		};
-		let what =
+		let what = ConvertClassId::convert(class).ok_or(MatchError::AssetIdConversionFailed)?;
-			ConvertClassId::convert_ref(class).map_err(|_| MatchError::AssetIdConversionFailed)?;
+		let instance =
-		let instance = ConvertInstanceId::convert_ref(instance)
+			ConvertInstanceId::convert(instance).ok_or(MatchError::InstanceConversionFailed)?;
-			.map_err(|_| MatchError::InstanceConversionFailed)?;
 		Ok((what, instance))
 	}
 }
@@ -155,8 +155,8 @@ impl<
 		AssetId: Clone,
 		Balance: Clone,
 		MatchAssetId: Contains<MultiLocation>,
-		ConvertAssetId: Convert<MultiLocation, AssetId>,
+		ConvertAssetId: MaybeEquivalence<MultiLocation, AssetId>,
-		ConvertBalance: Convert<u128, Balance>,
+		ConvertBalance: MaybeEquivalence<u128, Balance>,
 	> MatchesFungibles<AssetId, Balance>
 	for MatchedConvertedConcreteId<AssetId, Balance, MatchAssetId, ConvertAssetId, ConvertBalance>
 {
@@ -165,10 +165,9 @@ impl<
 			(Fungible(ref amount), Concrete(ref id)) if MatchAssetId::contains(id) => (amount, id),
 			_ => return Err(MatchError::AssetNotHandled),
 		};
-		let what =
+		let what = ConvertAssetId::convert(id).ok_or(MatchError::AssetIdConversionFailed)?;
-			ConvertAssetId::convert_ref(id).map_err(|_| MatchError::AssetIdConversionFailed)?;
+		let amount =
-		let amount = ConvertBalance::convert_ref(amount)
+			ConvertBalance::convert(amount).ok_or(MatchError::AmountToBalanceConversionFailed)?;
-			.map_err(|_| MatchError::AmountToBalanceConversionFailed)?;
 		Ok((what, amount))
 	}
 }
@@ -176,8 +175,8 @@ impl<
 		ClassId: Clone,
 		InstanceId: Clone,
 		MatchClassId: Contains<MultiLocation>,
-		ConvertClassId: Convert<MultiLocation, ClassId>,
+		ConvertClassId: MaybeEquivalence<MultiLocation, ClassId>,
-		ConvertInstanceId: Convert<AssetInstance, InstanceId>,
+		ConvertInstanceId: MaybeEquivalence<AssetInstance, InstanceId>,
 	> MatchesNonFungibles<ClassId, InstanceId>
 	for MatchedConvertedConcreteId<ClassId, InstanceId, MatchClassId, ConvertClassId, ConvertInstanceId>
 {
@@ -187,10 +186,9 @@ impl<
 				(instance, class),
 			_ => return Err(MatchError::AssetNotHandled),
 		};
-		let what =
+		let what = ConvertClassId::convert(class).ok_or(MatchError::AssetIdConversionFailed)?;
-			ConvertClassId::convert_ref(class).map_err(|_| MatchError::AssetIdConversionFailed)?;
+		let instance =
-		let instance = ConvertInstanceId::convert_ref(instance)
+			ConvertInstanceId::convert(instance).ok_or(MatchError::InstanceConversionFailed)?;
-			.map_err(|_| MatchError::InstanceConversionFailed)?;
 		Ok((what, instance))
 	}
 }
@@ -286,13 +284,13 @@ mod tests {
 
 		// ConvertedConcreteId cfg
 		struct ClassInstanceIdConverter;
-		impl Convert<AssetInstance, ClassInstanceId> for ClassInstanceIdConverter {
+		impl MaybeEquivalence<AssetInstance, ClassInstanceId> for ClassInstanceIdConverter {
-			fn convert_ref(value: impl Borrow<AssetInstance>) -> Result<ClassInstanceId, ()> {
+			fn convert(value: &AssetInstance) -> Option<ClassInstanceId> {
-				value.borrow().clone().try_into().map_err(|_| ())
+				value.clone().try_into().ok()
 			}
 
-			fn reverse_ref(value: impl Borrow<ClassInstanceId>) -> Result<AssetInstance, ()> {
+			fn convert_back(value: &ClassInstanceId) -> Option<AssetInstance> {
-				Ok(AssetInstance::from(value.borrow().clone()))
+				Some(AssetInstance::from(value.clone()))
 			}
 		}
 

polkadot/xcm/xcm-builder/src/currency_adapter.rs

@@ -22,7 +22,7 @@ use sp_runtime::traits::CheckedSub;
 use sp_std::{marker::PhantomData, result};
 use xcm::latest::{Error as XcmError, MultiAsset, MultiLocation, Result, XcmContext};
 use xcm_executor::{
-	traits::{Convert, MatchesFungible, TransactAsset},
+	traits::{ConvertLocation, MatchesFungible, TransactAsset},
 	Assets,
 };
 
@@ -64,7 +64,7 @@ impl From<Error> for XcmError {
 ///     pub CheckingAccount: AccountId = PalletId(*b"checking").into_account_truncating();
 /// }
 ///
-/// /// Some items that implement `Convert<MultiLocation, AccountId>`. Can be more, but for now we just assume we accept
+/// /// Some items that implement `ConvertLocation<AccountId>`. Can be more, but for now we just assume we accept
 /// /// messages from the parent (relay chain).
 /// pub type LocationConverter = (ParentIsPreset<AccountId>);
 ///
@@ -92,7 +92,7 @@ pub struct CurrencyAdapter<Currency, Matcher, AccountIdConverter, AccountId, Che
 impl<
 		Currency: frame_support::traits::Currency<AccountId>,
 		Matcher: MatchesFungible<Currency::Balance>,
-		AccountIdConverter: Convert<MultiLocation, AccountId>,
+		AccountIdConverter: ConvertLocation<AccountId>,
 		AccountId: Clone, // can't get away without it since Currency is generic over it.
 		CheckedAccount: Get<Option<(AccountId, MintLocation)>>,
 	> CurrencyAdapter<Currency, Matcher, AccountIdConverter, AccountId, CheckedAccount>
@@ -133,7 +133,7 @@ impl<
 impl<
 		Currency: frame_support::traits::Currency<AccountId>,
 		Matcher: MatchesFungible<Currency::Balance>,
-		AccountIdConverter: Convert<MultiLocation, AccountId>,
+		AccountIdConverter: ConvertLocation<AccountId>,
 		AccountId: Clone, // can't get away without it since Currency is generic over it.
 		CheckedAccount: Get<Option<(AccountId, MintLocation)>>,
 	> TransactAsset
@@ -196,7 +196,7 @@ impl<
 		// Check we handle this asset.
 		let amount = Matcher::matches_fungible(&what).ok_or(Error::AssetNotHandled)?;
 		let who =
-			AccountIdConverter::convert_ref(who).map_err(|()| Error::AccountIdConversionFailed)?;
+			AccountIdConverter::convert_location(who).ok_or(Error::AccountIdConversionFailed)?;
 		let _imbalance = Currency::deposit_creating(&who, amount);
 		Ok(())
 	}
@@ -210,7 +210,7 @@ impl<
 		// Check we handle this asset.
 		let amount = Matcher::matches_fungible(what).ok_or(Error::AssetNotHandled)?;
 		let who =
-			AccountIdConverter::convert_ref(who).map_err(|()| Error::AccountIdConversionFailed)?;
+			AccountIdConverter::convert_location(who).ok_or(Error::AccountIdConversionFailed)?;
 		Currency::withdraw(&who, amount, WithdrawReasons::TRANSFER, AllowDeath)
 			.map_err(|e| XcmError::FailedToTransactAsset(e.into()))?;
 		Ok(what.clone().into())
@@ -225,9 +225,9 @@ impl<
 		log::trace!(target: "xcm::currency_adapter", "internal_transfer_asset asset: {:?}, from: {:?}, to: {:?}", asset, from, to);
 		let amount = Matcher::matches_fungible(asset).ok_or(Error::AssetNotHandled)?;
 		let from =
-			AccountIdConverter::convert_ref(from).map_err(|()| Error::AccountIdConversionFailed)?;
+			AccountIdConverter::convert_location(from).ok_or(Error::AccountIdConversionFailed)?;
 		let to =
-			AccountIdConverter::convert_ref(to).map_err(|()| Error::AccountIdConversionFailed)?;
+			AccountIdConverter::convert_location(to).ok_or(Error::AccountIdConversionFailed)?;
 		Currency::transfer(&from, &to, amount, AllowDeath)
 			.map_err(|e| XcmError::FailedToTransactAsset(e.into()))?;
 		Ok(asset.clone().into())

polkadot/xcm/xcm-builder/src/fungibles_adapter.rs

@@ -24,7 +24,7 @@ use frame_support::traits::{
 };
 use sp_std::{marker::PhantomData, prelude::*, result};
 use xcm::latest::prelude::*;
-use xcm_executor::traits::{Convert, Error as MatchError, MatchesFungibles, TransactAsset};
+use xcm_executor::traits::{ConvertLocation, Error as MatchError, MatchesFungibles, TransactAsset};
 
 /// `TransactAsset` implementation to convert a `fungibles` implementation to become usable in XCM.
 pub struct FungiblesTransferAdapter<Assets, Matcher, AccountIdConverter, AccountId>(
@@ -33,7 +33,7 @@ pub struct FungiblesTransferAdapter<Assets, Matcher, AccountIdConverter, Account
 impl<
 		Assets: fungibles::Mutate<AccountId>,
 		Matcher: MatchesFungibles<Assets::AssetId, Assets::Balance>,
-		AccountIdConverter: Convert<MultiLocation, AccountId>,
+		AccountIdConverter: ConvertLocation<AccountId>,
 		AccountId: Clone, // can't get away without it since Currency is generic over it.
 	> TransactAsset for FungiblesTransferAdapter<Assets, Matcher, AccountIdConverter, AccountId>
 {
@@ -50,10 +50,10 @@ impl<
 		);
 		// Check we handle this asset.
 		let (asset_id, amount) = Matcher::matches_fungibles(what)?;
-		let source = AccountIdConverter::convert_ref(from)
+		let source = AccountIdConverter::convert_location(from)
-			.map_err(|()| MatchError::AccountIdConversionFailed)?;
+			.ok_or(MatchError::AccountIdConversionFailed)?;
-		let dest = AccountIdConverter::convert_ref(to)
+		let dest = AccountIdConverter::convert_location(to)
-			.map_err(|()| MatchError::AccountIdConversionFailed)?;
+			.ok_or(MatchError::AccountIdConversionFailed)?;
 		Assets::transfer(asset_id, &source, &dest, amount, Preserve)
 			.map_err(|e| XcmError::FailedToTransactAsset(e.into()))?;
 		Ok(what.clone().into())
@@ -149,7 +149,7 @@ pub struct FungiblesMutateAdapter<
 impl<
 		Assets: fungibles::Mutate<AccountId>,
 		Matcher: MatchesFungibles<Assets::AssetId, Assets::Balance>,
-		AccountIdConverter: Convert<MultiLocation, AccountId>,
+		AccountIdConverter: ConvertLocation<AccountId>,
 		AccountId: Clone, // can't get away without it since Currency is generic over it.
 		CheckAsset: AssetChecking<Assets::AssetId>,
 		CheckingAccount: Get<AccountId>,
@@ -184,7 +184,7 @@ impl<
 impl<
 		Assets: fungibles::Mutate<AccountId>,
 		Matcher: MatchesFungibles<Assets::AssetId, Assets::Balance>,
-		AccountIdConverter: Convert<MultiLocation, AccountId>,
+		AccountIdConverter: ConvertLocation<AccountId>,
 		AccountId: Clone, // can't get away without it since Currency is generic over it.
 		CheckAsset: AssetChecking<Assets::AssetId>,
 		CheckingAccount: Get<AccountId>,
@@ -282,8 +282,8 @@ impl<
 		);
 		// Check we handle this asset.
 		let (asset_id, amount) = Matcher::matches_fungibles(what)?;
-		let who = AccountIdConverter::convert_ref(who)
+		let who = AccountIdConverter::convert_location(who)
-			.map_err(|()| MatchError::AccountIdConversionFailed)?;
+			.ok_or(MatchError::AccountIdConversionFailed)?;
 		Assets::mint_into(asset_id, &who, amount)
 			.map_err(|e| XcmError::FailedToTransactAsset(e.into()))?;
 		Ok(())
@@ -301,8 +301,8 @@ impl<
 		);
 		// Check we handle this asset.
 		let (asset_id, amount) = Matcher::matches_fungibles(what)?;
-		let who = AccountIdConverter::convert_ref(who)
+		let who = AccountIdConverter::convert_location(who)
-			.map_err(|()| MatchError::AccountIdConversionFailed)?;
+			.ok_or(MatchError::AccountIdConversionFailed)?;
 		Assets::burn_from(asset_id, &who, amount, Exact, Polite)
 			.map_err(|e| XcmError::FailedToTransactAsset(e.into()))?;
 		Ok(what.clone().into())
@@ -320,7 +320,7 @@ pub struct FungiblesAdapter<
 impl<
 		Assets: fungibles::Mutate<AccountId>,
 		Matcher: MatchesFungibles<Assets::AssetId, Assets::Balance>,
-		AccountIdConverter: Convert<MultiLocation, AccountId>,
+		AccountIdConverter: ConvertLocation<AccountId>,
 		AccountId: Clone, // can't get away without it since Currency is generic over it.
 		CheckAsset: AssetChecking<Assets::AssetId>,
 		CheckingAccount: Get<AccountId>,

polkadot/xcm/xcm-builder/src/lib.rs

@@ -27,10 +27,14 @@ mod tests;
 pub mod test_utils;
 
 mod location_conversion;
+#[allow(deprecated)]
+pub use location_conversion::ForeignChainAliasAccount;
 pub use location_conversion::{
 	Account32Hash, AccountId32Aliases, AccountKey20Aliases, AliasesIntoAccountId32,
-	ChildParachainConvertsVia, GlobalConsensusParachainConvertsFor, ParentIsPreset,
+	ChildParachainConvertsVia, DescribeAccountId32Terminal, DescribeAccountIdTerminal,
-	SiblingParachainConvertsVia,
+	DescribeAccountKey20Terminal, DescribeAllTerminal, DescribeFamily, DescribeLocation,
+	DescribePalletTerminal, DescribeTerminus, GlobalConsensusParachainConvertsFor,
+	HashedDescription, ParentIsPreset, SiblingParachainConvertsVia,
 };
 
 mod origin_conversion;

polkadot/xcm/xcm-builder/src/location_conversion.rs

@@ -16,12 +16,155 @@
 
 use crate::universal_exports::ensure_is_remote;
 use frame_support::traits::Get;
-use parity_scale_codec::{Decode, Encode};
+use parity_scale_codec::{Compact, Decode, Encode};
 use sp_io::hashing::blake2_256;
-use sp_runtime::traits::{AccountIdConversion, TrailingZeroInput};
+use sp_runtime::traits::{AccountIdConversion, Convert, TrailingZeroInput};
-use sp_std::{borrow::Borrow, marker::PhantomData};
+use sp_std::{marker::PhantomData, prelude::*};
 use xcm::latest::prelude::*;
-use xcm_executor::traits::Convert;
+use xcm_executor::traits::ConvertLocation;
+
+/// Means of converting a location into a stable and unique descriptive identifier.
+pub trait DescribeLocation {
+	/// Create a description of the given `location` if possible. No two locations should have the
+	/// same descriptor.
+	fn describe_location(location: &MultiLocation) -> Option<Vec<u8>>;
+}
+
+#[impl_trait_for_tuples::impl_for_tuples(30)]
+impl DescribeLocation for Tuple {
+	fn describe_location(l: &MultiLocation) -> Option<Vec<u8>> {
+		for_tuples!( #(
+			match Tuple::describe_location(l) {
+				Some(result) => return Some(result),
+				None => {},
+			}
+		)* );
+		None
+	}
+}
+
+pub struct DescribeTerminus;
+impl DescribeLocation for DescribeTerminus {
+	fn describe_location(l: &MultiLocation) -> Option<Vec<u8>> {
+		match (l.parents, &l.interior) {
+			(0, Here) => Some(Vec::new()),
+			_ => return None,
+		}
+	}
+}
+
+pub struct DescribePalletTerminal;
+impl DescribeLocation for DescribePalletTerminal {
+	fn describe_location(l: &MultiLocation) -> Option<Vec<u8>> {
+		match (l.parents, &l.interior) {
+			(0, X1(PalletInstance(i))) =>
+				Some((b"Pallet", Compact::<u32>::from(*i as u32)).encode()),
+			_ => return None,
+		}
+	}
+}
+
+pub struct DescribeAccountId32Terminal;
+impl DescribeLocation for DescribeAccountId32Terminal {
+	fn describe_location(l: &MultiLocation) -> Option<Vec<u8>> {
+		match (l.parents, &l.interior) {
+			(0, X1(AccountId32 { id, .. })) => Some((b"AccountId32", id).encode()),
+			_ => return None,
+		}
+	}
+}
+
+pub struct DescribeAccountKey20Terminal;
+impl DescribeLocation for DescribeAccountKey20Terminal {
+	fn describe_location(l: &MultiLocation) -> Option<Vec<u8>> {
+		match (l.parents, &l.interior) {
+			(0, X1(AccountKey20 { key, .. })) => Some((b"AccountKey20", key).encode()),
+			_ => return None,
+		}
+	}
+}
+
+pub type DescribeAccountIdTerminal = (DescribeAccountId32Terminal, DescribeAccountKey20Terminal);
+
+pub type DescribeAllTerminal = (
+	DescribeTerminus,
+	DescribePalletTerminal,
+	DescribeAccountId32Terminal,
+	DescribeAccountKey20Terminal,
+);
+
+pub struct DescribeFamily<DescribeInterior>(PhantomData<DescribeInterior>);
+impl<Suffix: DescribeLocation> DescribeLocation for DescribeFamily<Suffix> {
+	fn describe_location(l: &MultiLocation) -> Option<Vec<u8>> {
+		match (l.parents, l.interior.first()) {
+			(0, Some(Parachain(index))) => {
+				let tail = l.interior.split_first().0;
+				let interior = Suffix::describe_location(&tail.into())?;
+				Some((b"ChildChain", Compact::<u32>::from(*index), interior).encode())
+			},
+			(1, Some(Parachain(index))) => {
+				let tail = l.interior.split_first().0;
+				let interior = Suffix::describe_location(&tail.into())?;
+				Some((b"SiblingChain", Compact::<u32>::from(*index), interior).encode())
+			},
+			(1, _) => {
+				let tail = l.interior.into();
+				let interior = Suffix::describe_location(&tail)?;
+				Some((b"ParentChain", interior).encode())
+			},
+			_ => return None,
+		}
+	}
+}
+
+pub struct HashedDescription<AccountId, Describe>(PhantomData<(AccountId, Describe)>);
+impl<AccountId: From<[u8; 32]> + Clone, Describe: DescribeLocation> ConvertLocation<AccountId>
+	for HashedDescription<AccountId, Describe>
+{
+	fn convert_location(value: &MultiLocation) -> Option<AccountId> {
+		Some(blake2_256(&Describe::describe_location(value)?).into())
+	}
+}
+
+/// This is a describer for legacy support of the `ForeignChainAliasAccount` preimage. New chains
+/// are recommended to use the more extensible `HashedDescription` type.
+pub struct LegacyDescribeForeignChainAccount;
+impl DescribeLocation for LegacyDescribeForeignChainAccount {
+	fn describe_location(location: &MultiLocation) -> Option<Vec<u8>> {
+		Some(match location {
+			// Used on the relay chain for sending paras that use 32 byte accounts
+			MultiLocation {
+				parents: 0,
+				interior: X2(Parachain(para_id), AccountId32 { id, .. }),
+			} => LegacyDescribeForeignChainAccount::from_para_32(para_id, id, 0),
+
+			// Used on the relay chain for sending paras that use 20 byte accounts
+			MultiLocation {
+				parents: 0,
+				interior: X2(Parachain(para_id), AccountKey20 { key, .. }),
+			} => LegacyDescribeForeignChainAccount::from_para_20(para_id, key, 0),
+
+			// Used on para-chain for sending paras that use 32 byte accounts
+			MultiLocation {
+				parents: 1,
+				interior: X2(Parachain(para_id), AccountId32 { id, .. }),
+			} => LegacyDescribeForeignChainAccount::from_para_32(para_id, id, 1),
+
+			// Used on para-chain for sending paras that use 20 byte accounts
+			MultiLocation {
+				parents: 1,
+				interior: X2(Parachain(para_id), AccountKey20 { key, .. }),
+			} => LegacyDescribeForeignChainAccount::from_para_20(para_id, key, 1),
+
+			// Used on para-chain for sending from the relay chain
+			MultiLocation { parents: 1, interior: X1(AccountId32 { id, .. }) } =>
+				LegacyDescribeForeignChainAccount::from_relay_32(id, 1),
+
+			// No other conversions provided
+			_ => return None,
+		})
+	}
+}
 
 /// Prefix for generating alias account for accounts coming
 /// from chains that use 32 byte long representations.
@@ -35,6 +178,39 @@ pub const FOREIGN_CHAIN_PREFIX_PARA_20: [u8; 37] = *b"ForeignChainAliasAccountPr
 /// from the relay chain using 32 byte long representations.
 pub const FOREIGN_CHAIN_PREFIX_RELAY: [u8; 36] = *b"ForeignChainAliasAccountPrefix_Relay";
 
+impl LegacyDescribeForeignChainAccount {
+	fn from_para_32(para_id: &u32, id: &[u8; 32], parents: u8) -> Vec<u8> {
+		(FOREIGN_CHAIN_PREFIX_PARA_32, para_id, id, parents).encode()
+	}
+
+	fn from_para_20(para_id: &u32, id: &[u8; 20], parents: u8) -> Vec<u8> {
+		(FOREIGN_CHAIN_PREFIX_PARA_20, para_id, id, parents).encode()
+	}
+
+	fn from_relay_32(id: &[u8; 32], parents: u8) -> Vec<u8> {
+		(FOREIGN_CHAIN_PREFIX_RELAY, id, parents).encode()
+	}
+}
+
+/// This is deprecated in favour of the more modular `HashedDescription` converter. If
+/// your chain has previously used this, then you can retain backwards compatibility using
+/// `HashedDescription` and a tuple with `LegacyDescribeForeignChainAccount` as the first
+/// element. For example:
+///
+/// ```nocompile
+/// pub type LocationToAccount = HashedDescription<
+///   // Legacy conversion - MUST BE FIRST!
+///   LegacyDescribeForeignChainAccount,
+///   // Other conversions
+///   DescribeTerminus,
+///   DescribePalletTerminal,
+/// >;
+/// ```
+///
+/// This type is equivalent to the above but without any other conversions.
+///
+/// ### Old documentation
+///
 /// This converter will for a given `AccountId32`/`AccountKey20`
 /// always generate the same "remote" account for a specific
 /// sending chain.
@@ -68,145 +244,58 @@ pub const FOREIGN_CHAIN_PREFIX_RELAY: [u8; 36] = *b"ForeignChainAliasAccountPref
 ///
 /// Note that the alias accounts have overlaps but never on the same
 /// chain when the sender comes from different chains.
-pub struct ForeignChainAliasAccount<AccountId>(PhantomData<AccountId>);
+#[deprecated = "Use `HashedDescription<AccountId, LegacyDescribeForeignChainAccount>` instead"]
-impl<AccountId: From<[u8; 32]> + Clone> Convert<MultiLocation, AccountId>
+pub type ForeignChainAliasAccount<AccountId> =
-	for ForeignChainAliasAccount<AccountId>
+	HashedDescription<AccountId, LegacyDescribeForeignChainAccount>;
-{
-	fn convert_ref(location: impl Borrow<MultiLocation>) -> Result<AccountId, ()> {
-		let entropy = match location.borrow() {
-			// Used on the relay chain for sending paras that use 32 byte accounts
-			MultiLocation {
-				parents: 0,
-				interior: X2(Parachain(para_id), AccountId32 { id, .. }),
-			} => ForeignChainAliasAccount::<AccountId>::from_para_32(para_id, id, 0),
-
-			// Used on the relay chain for sending paras that use 20 byte accounts
-			MultiLocation {
-				parents: 0,
-				interior: X2(Parachain(para_id), AccountKey20 { key, .. }),
-			} => ForeignChainAliasAccount::<AccountId>::from_para_20(para_id, key, 0),
-
-			// Used on para-chain for sending paras that use 32 byte accounts
-			MultiLocation {
-				parents: 1,
-				interior: X2(Parachain(para_id), AccountId32 { id, .. }),
-			} => ForeignChainAliasAccount::<AccountId>::from_para_32(para_id, id, 1),
-
-			// Used on para-chain for sending paras that use 20 byte accounts
-			MultiLocation {
-				parents: 1,
-				interior: X2(Parachain(para_id), AccountKey20 { key, .. }),
-			} => ForeignChainAliasAccount::<AccountId>::from_para_20(para_id, key, 1),
-
-			// Used on para-chain for sending from the relay chain
-			MultiLocation { parents: 1, interior: X1(AccountId32 { id, .. }) } =>
-				ForeignChainAliasAccount::<AccountId>::from_relay_32(id, 1),
-
-			// No other conversions provided
-			_ => return Err(()),
-		};
-
-		Ok(entropy.into())
-	}
-
-	fn reverse_ref(_: impl Borrow<AccountId>) -> Result<MultiLocation, ()> {
-		Err(())
-	}
-}
-
-impl<AccountId> ForeignChainAliasAccount<AccountId> {
-	fn from_para_32(para_id: &u32, id: &[u8; 32], parents: u8) -> [u8; 32] {
-		(FOREIGN_CHAIN_PREFIX_PARA_32, para_id, id, parents).using_encoded(blake2_256)
-	}
-
-	fn from_para_20(para_id: &u32, id: &[u8; 20], parents: u8) -> [u8; 32] {
-		(FOREIGN_CHAIN_PREFIX_PARA_20, para_id, id, parents).using_encoded(blake2_256)
-	}
-
-	fn from_relay_32(id: &[u8; 32], parents: u8) -> [u8; 32] {
-		(FOREIGN_CHAIN_PREFIX_RELAY, id, parents).using_encoded(blake2_256)
-	}
-}
 
 pub struct Account32Hash<Network, AccountId>(PhantomData<(Network, AccountId)>);
 impl<Network: Get<Option<NetworkId>>, AccountId: From<[u8; 32]> + Into<[u8; 32]> + Clone>
-	Convert<MultiLocation, AccountId> for Account32Hash<Network, AccountId>
+	ConvertLocation<AccountId> for Account32Hash<Network, AccountId>
 {
-	fn convert_ref(location: impl Borrow<MultiLocation>) -> Result<AccountId, ()> {
+	fn convert_location(location: &MultiLocation) -> Option<AccountId> {
-		Ok(("multiloc", location.borrow()).using_encoded(blake2_256).into())
+		Some(("multiloc", location).using_encoded(blake2_256).into())
-	}
-
-	fn reverse_ref(_: impl Borrow<AccountId>) -> Result<MultiLocation, ()> {
-		Err(())
 	}
 }
 
 /// A [`MultiLocation`] consisting of a single `Parent` [`Junction`] will be converted to the
 /// parent `AccountId`.
 pub struct ParentIsPreset<AccountId>(PhantomData<AccountId>);
-impl<AccountId: Decode + Eq + Clone> Convert<MultiLocation, AccountId>
+impl<AccountId: Decode + Eq + Clone> ConvertLocation<AccountId> for ParentIsPreset<AccountId> {
-	for ParentIsPreset<AccountId>
+	fn convert_location(location: &MultiLocation) -> Option<AccountId> {
-{
+		if location.contains_parents_only(1) {
-	fn convert_ref(location: impl Borrow<MultiLocation>) -> Result<AccountId, ()> {
+			Some(
-		if location.borrow().contains_parents_only(1) {
+				b"Parent"
-			Ok(b"Parent"
+					.using_encoded(|b| AccountId::decode(&mut TrailingZeroInput::new(b)))
-				.using_encoded(|b| AccountId::decode(&mut TrailingZeroInput::new(b)))
+					.expect("infinite length input; no invalid inputs for type; qed"),
-				.expect("infinite length input; no invalid inputs for type; qed"))
+			)
 		} else {
-			Err(())
+			None
-		}
-	}
-
-	fn reverse_ref(who: impl Borrow<AccountId>) -> Result<MultiLocation, ()> {
-		let parent_account = b"Parent"
-			.using_encoded(|b| AccountId::decode(&mut TrailingZeroInput::new(b)))
-			.expect("infinite length input; no invalid inputs for type; qed");
-		if who.borrow() == &parent_account {
-			Ok(Parent.into())
-		} else {
-			Err(())
 		}
 	}
 }
 
 pub struct ChildParachainConvertsVia<ParaId, AccountId>(PhantomData<(ParaId, AccountId)>);
 impl<ParaId: From<u32> + Into<u32> + AccountIdConversion<AccountId>, AccountId: Clone>
-	Convert<MultiLocation, AccountId> for ChildParachainConvertsVia<ParaId, AccountId>
+	ConvertLocation<AccountId> for ChildParachainConvertsVia<ParaId, AccountId>
 {
-	fn convert_ref(location: impl Borrow<MultiLocation>) -> Result<AccountId, ()> {
+	fn convert_location(location: &MultiLocation) -> Option<AccountId> {
-		match location.borrow() {
+		match location {
 			MultiLocation { parents: 0, interior: X1(Parachain(id)) } =>
-				Ok(ParaId::from(*id).into_account_truncating()),
+				Some(ParaId::from(*id).into_account_truncating()),
-			_ => Err(()),
+			_ => None,
-		}
-	}
-
-	fn reverse_ref(who: impl Borrow<AccountId>) -> Result<MultiLocation, ()> {
-		if let Some(id) = ParaId::try_from_account(who.borrow()) {
-			Ok(Parachain(id.into()).into())
-		} else {
-			Err(())
 		}
 	}
 }
 
 pub struct SiblingParachainConvertsVia<ParaId, AccountId>(PhantomData<(ParaId, AccountId)>);
 impl<ParaId: From<u32> + Into<u32> + AccountIdConversion<AccountId>, AccountId: Clone>
-	Convert<MultiLocation, AccountId> for SiblingParachainConvertsVia<ParaId, AccountId>
+	ConvertLocation<AccountId> for SiblingParachainConvertsVia<ParaId, AccountId>
 {
-	fn convert_ref(location: impl Borrow<MultiLocation>) -> Result<AccountId, ()> {
+	fn convert_location(location: &MultiLocation) -> Option<AccountId> {
-		match location.borrow() {
+		match location {
 			MultiLocation { parents: 1, interior: X1(Parachain(id)) } =>
-				Ok(ParaId::from(*id).into_account_truncating()),
+				Some(ParaId::from(*id).into_account_truncating()),
-			_ => Err(()),
+			_ => None,
-		}
-	}
-
-	fn reverse_ref(who: impl Borrow<AccountId>) -> Result<MultiLocation, ()> {
-		if let Some(id) = ParaId::try_from_account(who.borrow()) {
-			Ok(MultiLocation::new(1, X1(Parachain(id.into()))))
-		} else {
-			Err(())
 		}
 	}
 }
@@ -214,21 +303,17 @@ impl<ParaId: From<u32> + Into<u32> + AccountIdConversion<AccountId>, AccountId:
 /// Extracts the `AccountId32` from the passed `location` if the network matches.
 pub struct AccountId32Aliases<Network, AccountId>(PhantomData<(Network, AccountId)>);
 impl<Network: Get<Option<NetworkId>>, AccountId: From<[u8; 32]> + Into<[u8; 32]> + Clone>
-	Convert<MultiLocation, AccountId> for AccountId32Aliases<Network, AccountId>
+	ConvertLocation<AccountId> for AccountId32Aliases<Network, AccountId>
 {
-	fn convert(location: MultiLocation) -> Result<AccountId, MultiLocation> {
+	fn convert_location(location: &MultiLocation) -> Option<AccountId> {
-		let id = match location {
+		let id = match *location {
 			MultiLocation { parents: 0, interior: X1(AccountId32 { id, network: None }) } => id,
 			MultiLocation { parents: 0, interior: X1(AccountId32 { id, network }) }
 				if network == Network::get() =>
 				id,
-			_ => return Err(location),
+			_ => return None,
 		};
-		Ok(id.into())
+		Some(id.into())
-	}
-
-	fn reverse(who: AccountId) -> Result<MultiLocation, AccountId> {
-		Ok(AccountId32 { id: who.into(), network: Network::get() }.into())
 	}
 }
 
@@ -237,8 +322,7 @@ impl<Network: Get<Option<NetworkId>>, AccountId: From<[u8; 32]> + Into<[u8; 32]>
 /// network (provided by `Network`) and the `AccountId`'s `[u8; 32]` datum for the `id`.
 pub struct AliasesIntoAccountId32<Network, AccountId>(PhantomData<(Network, AccountId)>);
 impl<'a, Network: Get<Option<NetworkId>>, AccountId: Clone + Into<[u8; 32]> + Clone>
-	sp_runtime::traits::Convert<&'a AccountId, MultiLocation>
+	Convert<&'a AccountId, MultiLocation> for AliasesIntoAccountId32<Network, AccountId>
-	for AliasesIntoAccountId32<Network, AccountId>
 {
 	fn convert(who: &AccountId) -> MultiLocation {
 		AccountId32 { network: Network::get(), id: who.clone().into() }.into()
@@ -247,22 +331,17 @@ impl<'a, Network: Get<Option<NetworkId>>, AccountId: Clone + Into<[u8; 32]> + Cl
 
 pub struct AccountKey20Aliases<Network, AccountId>(PhantomData<(Network, AccountId)>);
 impl<Network: Get<Option<NetworkId>>, AccountId: From<[u8; 20]> + Into<[u8; 20]> + Clone>
-	Convert<MultiLocation, AccountId> for AccountKey20Aliases<Network, AccountId>
+	ConvertLocation<AccountId> for AccountKey20Aliases<Network, AccountId>
 {
-	fn convert(location: MultiLocation) -> Result<AccountId, MultiLocation> {
+	fn convert_location(location: &MultiLocation) -> Option<AccountId> {
-		let key = match location {
+		let key = match *location {
 			MultiLocation { parents: 0, interior: X1(AccountKey20 { key, network: None }) } => key,
 			MultiLocation { parents: 0, interior: X1(AccountKey20 { key, network }) }
 				if network == Network::get() =>
 				key,
-			_ => return Err(location),
+			_ => return None,
 		};
-		Ok(key.into())
+		Some(key.into())
-	}
-
-	fn reverse(who: AccountId) -> Result<MultiLocation, AccountId> {
-		let j = AccountKey20 { key: who.into(), network: Network::get() };
-		Ok(j.into())
 	}
 }
 
@@ -285,31 +364,24 @@ pub struct GlobalConsensusParachainConvertsFor<UniversalLocation, AccountId>(
 	PhantomData<(UniversalLocation, AccountId)>,
 );
 impl<UniversalLocation: Get<InteriorMultiLocation>, AccountId: From<[u8; 32]> + Clone>
-	Convert<MultiLocation, AccountId>
+	ConvertLocation<AccountId> for GlobalConsensusParachainConvertsFor<UniversalLocation, AccountId>
-	for GlobalConsensusParachainConvertsFor<UniversalLocation, AccountId>
 {
-	fn convert_ref(location: impl Borrow<MultiLocation>) -> Result<AccountId, ()> {
+	fn convert_location(location: &MultiLocation) -> Option<AccountId> {
 		let universal_source = UniversalLocation::get();
 		log::trace!(
 			target: "xcm::location_conversion",
 			"GlobalConsensusParachainConvertsFor universal_source: {:?}, location: {:?}",
-			universal_source, location.borrow(),
+			universal_source, location,
 		);
-		let devolved = ensure_is_remote(universal_source, *location.borrow()).map_err(|_| ())?;
+		let devolved = ensure_is_remote(universal_source, *location).ok()?;
 		let (remote_network, remote_location) = devolved;
 
 		match remote_location {
 			X1(Parachain(remote_network_para_id)) =>
-				Ok(AccountId::from(Self::from_params(&remote_network, &remote_network_para_id))),
+				Some(AccountId::from(Self::from_params(&remote_network, &remote_network_para_id))),
-			_ => Err(()),
+			_ => None,
 		}
 	}
-
-	fn reverse_ref(_: impl Borrow<AccountId>) -> Result<MultiLocation, ()> {
-		// if this is ever be needed, we could implement some kind of guessing, if we have
-		// configuration for supported networkId+paraId
-		Err(())
-	}
 }
 impl<UniversalLocation, AccountId>
 	GlobalConsensusParachainConvertsFor<UniversalLocation, AccountId>
@@ -323,6 +395,9 @@ impl<UniversalLocation, AccountId>
 mod tests {
 	use super::*;
 
+	pub type ForeignChainAliasAccount<AccountId> =
+		HashedDescription<AccountId, LegacyDescribeForeignChainAccount>;
+
 	use frame_support::parameter_types;
 	use xcm::latest::Junction;
 
@@ -441,11 +516,11 @@ mod tests {
 
 		for (location, expected_result) in test_data {
 			let result =
-				GlobalConsensusParachainConvertsFor::<UniversalLocation, [u8; 32]>::convert_ref(
+				GlobalConsensusParachainConvertsFor::<UniversalLocation, [u8; 32]>::convert_location(
 					&location,
 				);
 			match result {
-				Ok(account) => {
+				Some(account) => {
 					assert_eq!(
 						true, expected_result,
 						"expected_result: {}, but conversion passed: {:?}, location: {:?}",
@@ -465,7 +540,7 @@ mod tests {
 						)
 					}
 				},
-				Err(_) => {
+				None => {
 					assert_eq!(
 						false, expected_result,
 						"expected_result: {} - but conversion failed, location: {:?}",
@@ -477,23 +552,23 @@ mod tests {
 
 		// all success
 		let res_gc_a_p1000 =
-			GlobalConsensusParachainConvertsFor::<UniversalLocation, [u8; 32]>::convert_ref(
+			GlobalConsensusParachainConvertsFor::<UniversalLocation, [u8; 32]>::convert_location(
-				MultiLocation::new(2, X2(GlobalConsensus(ByGenesis([3; 32])), Parachain(1000))),
+				&MultiLocation::new(2, X2(GlobalConsensus(ByGenesis([3; 32])), Parachain(1000))),
 			)
 			.expect("conversion is ok");
 		let res_gc_a_p1001 =
-			GlobalConsensusParachainConvertsFor::<UniversalLocation, [u8; 32]>::convert_ref(
+			GlobalConsensusParachainConvertsFor::<UniversalLocation, [u8; 32]>::convert_location(
-				MultiLocation::new(2, X2(GlobalConsensus(ByGenesis([3; 32])), Parachain(1001))),
+				&MultiLocation::new(2, X2(GlobalConsensus(ByGenesis([3; 32])), Parachain(1001))),
 			)
 			.expect("conversion is ok");
 		let res_gc_b_p1000 =
-			GlobalConsensusParachainConvertsFor::<UniversalLocation, [u8; 32]>::convert_ref(
+			GlobalConsensusParachainConvertsFor::<UniversalLocation, [u8; 32]>::convert_location(
-				MultiLocation::new(2, X2(GlobalConsensus(ByGenesis([4; 32])), Parachain(1000))),
+				&MultiLocation::new(2, X2(GlobalConsensus(ByGenesis([4; 32])), Parachain(1000))),
 			)
 			.expect("conversion is ok");
 		let res_gc_b_p1001 =
-			GlobalConsensusParachainConvertsFor::<UniversalLocation, [u8; 32]>::convert_ref(
+			GlobalConsensusParachainConvertsFor::<UniversalLocation, [u8; 32]>::convert_location(
-				MultiLocation::new(2, X2(GlobalConsensus(ByGenesis([4; 32])), Parachain(1001))),
+				&MultiLocation::new(2, X2(GlobalConsensus(ByGenesis([4; 32])), Parachain(1001))),
 			)
 			.expect("conversion is ok");
 		assert_ne!(res_gc_a_p1000, res_gc_a_p1001);
@@ -510,7 +585,7 @@ mod tests {
 			parents: 1,
 			interior: X2(Parachain(1), AccountId32 { network: None, id: [0u8; 32] }),
 		};
-		let rem_1 = ForeignChainAliasAccount::<[u8; 32]>::convert(mul).unwrap();
+		let rem_1 = ForeignChainAliasAccount::<[u8; 32]>::convert_location(&mul).unwrap();
 
 		assert_eq!(
 			[
@@ -528,13 +603,13 @@ mod tests {
 			),
 		};
 
-		assert_eq!(ForeignChainAliasAccount::<[u8; 32]>::convert(mul).unwrap(), rem_1);
+		assert_eq!(ForeignChainAliasAccount::<[u8; 32]>::convert_location(&mul).unwrap(), rem_1);
 
 		let mul = MultiLocation {
 			parents: 1,
 			interior: X2(Parachain(2), AccountId32 { network: None, id: [0u8; 32] }),
 		};
-		let rem_2 = ForeignChainAliasAccount::<[u8; 32]>::convert(mul).unwrap();
+		let rem_2 = ForeignChainAliasAccount::<[u8; 32]>::convert_location(&mul).unwrap();
 
 		assert_eq!(
 			[
@@ -553,7 +628,7 @@ mod tests {
 			parents: 1,
 			interior: X2(Parachain(1), AccountKey20 { network: None, key: [0u8; 20] }),
 		};
-		let rem_1 = ForeignChainAliasAccount::<[u8; 32]>::convert(mul).unwrap();
+		let rem_1 = ForeignChainAliasAccount::<[u8; 32]>::convert_location(&mul).unwrap();
 
 		assert_eq!(
 			[
@@ -571,13 +646,13 @@ mod tests {
 			),
 		};
 
-		assert_eq!(ForeignChainAliasAccount::<[u8; 32]>::convert(mul).unwrap(), rem_1);
+		assert_eq!(ForeignChainAliasAccount::<[u8; 32]>::convert_location(&mul).unwrap(), rem_1);
 
 		let mul = MultiLocation {
 			parents: 1,
 			interior: X2(Parachain(2), AccountKey20 { network: None, key: [0u8; 20] }),
 		};
-		let rem_2 = ForeignChainAliasAccount::<[u8; 32]>::convert(mul).unwrap();
+		let rem_2 = ForeignChainAliasAccount::<[u8; 32]>::convert_location(&mul).unwrap();
 
 		assert_eq!(
 			[
@@ -596,7 +671,7 @@ mod tests {
 			parents: 1,
 			interior: X1(AccountId32 { network: None, id: [0u8; 32] }),
 		};
-		let rem_1 = ForeignChainAliasAccount::<[u8; 32]>::convert(mul).unwrap();
+		let rem_1 = ForeignChainAliasAccount::<[u8; 32]>::convert_location(&mul).unwrap();
 
 		assert_eq!(
 			[
@@ -611,13 +686,13 @@ mod tests {
 			interior: X1(AccountId32 { network: Some(NetworkId::Polkadot), id: [0u8; 32] }),
 		};
 
-		assert_eq!(ForeignChainAliasAccount::<[u8; 32]>::convert(mul).unwrap(), rem_1);
+		assert_eq!(ForeignChainAliasAccount::<[u8; 32]>::convert_location(&mul).unwrap(), rem_1);
 
 		let mul = MultiLocation {
 			parents: 1,
 			interior: X1(AccountId32 { network: None, id: [1u8; 32] }),
 		};
-		let rem_2 = ForeignChainAliasAccount::<[u8; 32]>::convert(mul).unwrap();
+		let rem_2 = ForeignChainAliasAccount::<[u8; 32]>::convert_location(&mul).unwrap();
 
 		assert_eq!(
 			[
@@ -636,7 +711,7 @@ mod tests {
 			parents: 0,
 			interior: X2(Parachain(1), AccountKey20 { network: None, key: [0u8; 20] }),
 		};
-		let rem_1 = ForeignChainAliasAccount::<[u8; 32]>::convert(mul).unwrap();
+		let rem_1 = ForeignChainAliasAccount::<[u8; 32]>::convert_location(&mul).unwrap();
 
 		assert_eq!(
 			[
@@ -650,7 +725,7 @@ mod tests {
 			parents: 0,
 			interior: X2(Parachain(2), AccountKey20 { network: None, key: [0u8; 20] }),
 		};
-		let rem_2 = ForeignChainAliasAccount::<[u8; 32]>::convert(mul).unwrap();
+		let rem_2 = ForeignChainAliasAccount::<[u8; 32]>::convert_location(&mul).unwrap();
 
 		assert_eq!(
 			[
@@ -669,7 +744,7 @@ mod tests {
 			parents: 0,
 			interior: X2(Parachain(1), AccountId32 { network: None, id: [0u8; 32] }),
 		};
-		let rem_1 = ForeignChainAliasAccount::<[u8; 32]>::convert(mul).unwrap();
+		let rem_1 = ForeignChainAliasAccount::<[u8; 32]>::convert_location(&mul).unwrap();
 
 		assert_eq!(
 			[
@@ -687,13 +762,13 @@ mod tests {
 			),
 		};
 
-		assert_eq!(ForeignChainAliasAccount::<[u8; 32]>::convert(mul).unwrap(), rem_1);
+		assert_eq!(ForeignChainAliasAccount::<[u8; 32]>::convert_location(&mul).unwrap(), rem_1);
 
 		let mul = MultiLocation {
 			parents: 0,
 			interior: X2(Parachain(2), AccountId32 { network: None, id: [0u8; 32] }),
 		};
-		let rem_2 = ForeignChainAliasAccount::<[u8; 32]>::convert(mul).unwrap();
+		let rem_2 = ForeignChainAliasAccount::<[u8; 32]>::convert_location(&mul).unwrap();
 
 		assert_eq!(
 			[
@@ -712,6 +787,6 @@ mod tests {
 			parents: 1,
 			interior: X1(AccountKey20 { network: None, key: [0u8; 20] }),
 		};
-		assert!(ForeignChainAliasAccount::<[u8; 32]>::convert(mul).is_err());
+		assert!(ForeignChainAliasAccount::<[u8; 32]>::convert_location(&mul).is_none());
 	}
 }

polkadot/xcm/xcm-builder/src/nonfungibles_adapter.rs

@@ -23,7 +23,9 @@ use frame_support::{
 };
 use sp_std::{marker::PhantomData, prelude::*, result};
 use xcm::latest::prelude::*;
-use xcm_executor::traits::{Convert, Error as MatchError, MatchesNonFungibles, TransactAsset};
+use xcm_executor::traits::{
+	ConvertLocation, Error as MatchError, MatchesNonFungibles, TransactAsset,
+};
 
 const LOG_TARGET: &str = "xcm::nonfungibles_adapter";
 
@@ -33,7 +35,7 @@ pub struct NonFungiblesTransferAdapter<Assets, Matcher, AccountIdConverter, Acco
 impl<
 		Assets: nonfungibles::Transfer<AccountId>,
 		Matcher: MatchesNonFungibles<Assets::CollectionId, Assets::ItemId>,
-		AccountIdConverter: Convert<MultiLocation, AccountId>,
+		AccountIdConverter: ConvertLocation<AccountId>,
 		AccountId: Clone, // can't get away without it since Currency is generic over it.
 	> TransactAsset for NonFungiblesTransferAdapter<Assets, Matcher, AccountIdConverter, AccountId>
 {
@@ -53,8 +55,8 @@ impl<
 		);
 		// Check we handle this asset.
 		let (class, instance) = Matcher::matches_nonfungibles(what)?;
-		let destination = AccountIdConverter::convert_ref(to)
+		let destination = AccountIdConverter::convert_location(to)
-			.map_err(|()| MatchError::AccountIdConversionFailed)?;
+			.ok_or(MatchError::AccountIdConversionFailed)?;
 		Assets::transfer(&class, &instance, &destination)
 			.map_err(|e| XcmError::FailedToTransactAsset(e.into()))?;
 		Ok(what.clone().into())
@@ -73,7 +75,7 @@ pub struct NonFungiblesMutateAdapter<
 impl<
 		Assets: nonfungibles::Mutate<AccountId>,
 		Matcher: MatchesNonFungibles<Assets::CollectionId, Assets::ItemId>,
-		AccountIdConverter: Convert<MultiLocation, AccountId>,
+		AccountIdConverter: ConvertLocation<AccountId>,
 		AccountId: Clone + Eq, // can't get away without it since Currency is generic over it.
 		CheckAsset: AssetChecking<Assets::CollectionId>,
 		CheckingAccount: Get<Option<AccountId>>,
@@ -115,7 +117,7 @@ impl<
 impl<
 		Assets: nonfungibles::Mutate<AccountId>,
 		Matcher: MatchesNonFungibles<Assets::CollectionId, Assets::ItemId>,
-		AccountIdConverter: Convert<MultiLocation, AccountId>,
+		AccountIdConverter: ConvertLocation<AccountId>,
 		AccountId: Clone + Eq, // can't get away without it since Currency is generic over it.
 		CheckAsset: AssetChecking<Assets::CollectionId>,
 		CheckingAccount: Get<Option<AccountId>>,
@@ -215,8 +217,8 @@ impl<
 		);
 		// Check we handle this asset.
 		let (class, instance) = Matcher::matches_nonfungibles(what)?;
-		let who = AccountIdConverter::convert_ref(who)
+		let who = AccountIdConverter::convert_location(who)
-			.map_err(|()| MatchError::AccountIdConversionFailed)?;
+			.ok_or(MatchError::AccountIdConversionFailed)?;
 		Assets::mint_into(&class, &instance, &who)
 			.map_err(|e| XcmError::FailedToTransactAsset(e.into()))
 	}
@@ -234,8 +236,8 @@ impl<
 			maybe_context,
 		);
 		// Check we handle this asset.
-		let who = AccountIdConverter::convert_ref(who)
+		let who = AccountIdConverter::convert_location(who)
-			.map_err(|()| MatchError::AccountIdConversionFailed)?;
+			.ok_or(MatchError::AccountIdConversionFailed)?;
 		let (class, instance) = Matcher::matches_nonfungibles(what)?;
 		Assets::burn(&class, &instance, Some(&who))
 			.map_err(|e| XcmError::FailedToTransactAsset(e.into()))?;
@@ -254,7 +256,7 @@ pub struct NonFungiblesAdapter<
 impl<
 		Assets: nonfungibles::Mutate<AccountId> + nonfungibles::Transfer<AccountId>,
 		Matcher: MatchesNonFungibles<Assets::CollectionId, Assets::ItemId>,
-		AccountIdConverter: Convert<MultiLocation, AccountId>,
+		AccountIdConverter: ConvertLocation<AccountId>,
 		AccountId: Clone + Eq, // can't get away without it since Currency is generic over it.
 		CheckAsset: AssetChecking<Assets::CollectionId>,
 		CheckingAccount: Get<Option<AccountId>>,

polkadot/xcm/xcm-builder/src/origin_conversion.rs

@@ -19,18 +19,17 @@
 use frame_support::traits::{EnsureOrigin, Get, GetBacking, OriginTrait};
 use frame_system::RawOrigin as SystemRawOrigin;
 use polkadot_parachain::primitives::IsSystem;
+use sp_runtime::traits::TryConvert;
 use sp_std::marker::PhantomData;
 use xcm::latest::{BodyId, BodyPart, Junction, Junctions::*, MultiLocation, NetworkId, OriginKind};
-use xcm_executor::traits::{Convert, ConvertOrigin};
+use xcm_executor::traits::{ConvertLocation, ConvertOrigin};
 
 /// Sovereign accounts use the system's `Signed` origin with an account ID derived from the `LocationConverter`.
 pub struct SovereignSignedViaLocation<LocationConverter, RuntimeOrigin>(
 	PhantomData<(LocationConverter, RuntimeOrigin)>,
 );
-impl<
+impl<LocationConverter: ConvertLocation<RuntimeOrigin::AccountId>, RuntimeOrigin: OriginTrait>
-		LocationConverter: Convert<MultiLocation, RuntimeOrigin::AccountId>,
+	ConvertOrigin<RuntimeOrigin> for SovereignSignedViaLocation<LocationConverter, RuntimeOrigin>
-		RuntimeOrigin: OriginTrait,
-	> ConvertOrigin<RuntimeOrigin> for SovereignSignedViaLocation<LocationConverter, RuntimeOrigin>
 where
 	RuntimeOrigin::AccountId: Clone,
 {
@@ -45,7 +44,7 @@ where
 			origin, kind,
 		);
 		if let OriginKind::SovereignAccount = kind {
-			let location = LocationConverter::convert(origin)?;
+			let location = LocationConverter::convert_location(&origin).ok_or(origin)?;
 			Ok(RuntimeOrigin::signed(location).into())
 		} else {
 			Err(origin)
@@ -244,14 +243,14 @@ where
 
 /// `EnsureOrigin` barrier to convert from dispatch origin to XCM origin, if one exists.
 pub struct EnsureXcmOrigin<RuntimeOrigin, Conversion>(PhantomData<(RuntimeOrigin, Conversion)>);
-impl<RuntimeOrigin: OriginTrait + Clone, Conversion: Convert<RuntimeOrigin, MultiLocation>>
+impl<RuntimeOrigin: OriginTrait + Clone, Conversion: TryConvert<RuntimeOrigin, MultiLocation>>
 	EnsureOrigin<RuntimeOrigin> for EnsureXcmOrigin<RuntimeOrigin, Conversion>
 where
 	RuntimeOrigin::PalletsOrigin: PartialEq,
 {
 	type Success = MultiLocation;
 	fn try_origin(o: RuntimeOrigin) -> Result<Self::Success, RuntimeOrigin> {
-		let o = match Conversion::convert(o) {
+		let o = match Conversion::try_convert(o) {
 			Ok(location) => return Ok(location),
 			Err(o) => o,
 		};
@@ -281,12 +280,13 @@ impl<
 		RuntimeOrigin: OriginTrait + Clone,
 		AccountId: Into<[u8; 32]>,
 		Network: Get<Option<NetworkId>>,
-	> Convert<RuntimeOrigin, MultiLocation> for SignedToAccountId32<RuntimeOrigin, AccountId, Network>
+	> TryConvert<RuntimeOrigin, MultiLocation>
+	for SignedToAccountId32<RuntimeOrigin, AccountId, Network>
 where
 	RuntimeOrigin::PalletsOrigin: From<SystemRawOrigin<AccountId>>
 		+ TryInto<SystemRawOrigin<AccountId>, Error = RuntimeOrigin::PalletsOrigin>,
 {
-	fn convert(o: RuntimeOrigin) -> Result<MultiLocation, RuntimeOrigin> {
+	fn try_convert(o: RuntimeOrigin) -> Result<MultiLocation, RuntimeOrigin> {
 		o.try_with_caller(|caller| match caller.try_into() {
 			Ok(SystemRawOrigin::Signed(who)) =>
 				Ok(Junction::AccountId32 { network: Network::get(), id: who.into() }.into()),
@@ -305,12 +305,12 @@ pub struct BackingToPlurality<RuntimeOrigin, COrigin, Body>(
 	PhantomData<(RuntimeOrigin, COrigin, Body)>,
 );
 impl<RuntimeOrigin: OriginTrait + Clone, COrigin: GetBacking, Body: Get<BodyId>>
-	Convert<RuntimeOrigin, MultiLocation> for BackingToPlurality<RuntimeOrigin, COrigin, Body>
+	TryConvert<RuntimeOrigin, MultiLocation> for BackingToPlurality<RuntimeOrigin, COrigin, Body>
 where
 	RuntimeOrigin::PalletsOrigin:
 		From<COrigin> + TryInto<COrigin, Error = RuntimeOrigin::PalletsOrigin>,
 {
-	fn convert(o: RuntimeOrigin) -> Result<MultiLocation, RuntimeOrigin> {
+	fn try_convert(o: RuntimeOrigin) -> Result<MultiLocation, RuntimeOrigin> {
 		o.try_with_caller(|caller| match caller.try_into() {
 			Ok(co) => match co.get_backing() {
 				Some(backing) => Ok(Junction::Plurality {
@@ -331,10 +331,10 @@ pub struct OriginToPluralityVoice<RuntimeOrigin, EnsureBodyOrigin, Body>(
 	PhantomData<(RuntimeOrigin, EnsureBodyOrigin, Body)>,
 );
 impl<RuntimeOrigin: Clone, EnsureBodyOrigin: EnsureOrigin<RuntimeOrigin>, Body: Get<BodyId>>
-	Convert<RuntimeOrigin, MultiLocation>
+	TryConvert<RuntimeOrigin, MultiLocation>
 	for OriginToPluralityVoice<RuntimeOrigin, EnsureBodyOrigin, Body>
 {
-	fn convert(o: RuntimeOrigin) -> Result<MultiLocation, RuntimeOrigin> {
+	fn try_convert(o: RuntimeOrigin) -> Result<MultiLocation, RuntimeOrigin> {
 		match EnsureBodyOrigin::try_origin(o) {
 			Ok(_) => Ok(Junction::Plurality { id: Body::get(), part: BodyPart::Voice }.into()),
 			Err(o) => Err(o),

polkadot/xcm/xcm-executor/src/traits/conversion.rs

@@ -15,122 +15,26 @@
 // along with Polkadot.  If not, see <http://www.gnu.org/licenses/>.
 
 use frame_support::traits::{Contains, OriginTrait};
-use parity_scale_codec::{Decode, Encode};
 use sp_runtime::{traits::Dispatchable, DispatchErrorWithPostInfo};
-use sp_std::{borrow::Borrow, marker::PhantomData, prelude::*, result::Result};
+use sp_std::{marker::PhantomData, result::Result};
 use xcm::latest::prelude::*;
 
-/// Generic third-party conversion trait. Use this when you don't want to force the user to use default
+/// Means of converting a location into an account identifier.
-/// implementations of `From` and `Into` for the types you wish to convert between.
+pub trait ConvertLocation<AccountId> {
-///
+	/// Convert the `location` into `Some` account ID, or `None` if not possible.
-/// One of `convert`/`convert_ref` and `reverse`/`reverse_ref` MUST be implemented. If possible, implement
+	fn convert_location(location: &MultiLocation) -> Option<AccountId>;
-/// `convert_ref`, since this will never result in a clone. Use `convert` when you definitely need to consume
-/// the source value.
-///
-/// Can be amalgamated into tuples. If any of the tuple elements converts into `Ok(_)` it short circuits. Otherwise returns
-/// the `Err(_)` of the last failing conversion (or `Err(())` for ref conversions).
-pub trait Convert<A: Clone, B: Clone> {
-	/// Convert from `value` (of type `A`) into an equivalent value of type `B`, `Err` if not possible.
-	fn convert(value: A) -> Result<B, A> {
-		Self::convert_ref(&value).map_err(|_| value)
-	}
-	fn convert_ref(value: impl Borrow<A>) -> Result<B, ()> {
-		Self::convert(value.borrow().clone()).map_err(|_| ())
-	}
-	/// Convert from `value` (of type `B`) into an equivalent value of type `A`, `Err` if not possible.
-	fn reverse(value: B) -> Result<A, B> {
-		Self::reverse_ref(&value).map_err(|_| value)
-	}
-	fn reverse_ref(value: impl Borrow<B>) -> Result<A, ()> {
-		Self::reverse(value.borrow().clone()).map_err(|_| ())
-	}
 }
 
 #[impl_trait_for_tuples::impl_for_tuples(30)]
-impl<A: Clone, B: Clone> Convert<A, B> for Tuple {
+impl<AccountId> ConvertLocation<AccountId> for Tuple {
-	fn convert(value: A) -> Result<B, A> {
+	fn convert_location(l: &MultiLocation) -> Option<AccountId> {
 		for_tuples!( #(
-			let value = match Tuple::convert(value) {
+			match Tuple::convert_location(l) {
-				Ok(result) => return Ok(result),
+				Some(result) => return Some(result),
-				Err(v) => v,
+				None => {},
-			};
-		)* );
-		Err(value)
-	}
-	fn reverse(value: B) -> Result<A, B> {
-		for_tuples!( #(
-			let value = match Tuple::reverse(value) {
-				Ok(result) => return Ok(result),
-				Err(v) => v,
-			};
-		)* );
-		Err(value)
-	}
-	fn convert_ref(value: impl Borrow<A>) -> Result<B, ()> {
-		let value = value.borrow();
-		for_tuples!( #(
-			match Tuple::convert_ref(value) {
-				Ok(result) => return Ok(result),
-				Err(_) => (),
 			}
 		)* );
-		Err(())
+		None
-	}
-	fn reverse_ref(value: impl Borrow<B>) -> Result<A, ()> {
-		let value = value.borrow();
-		for_tuples!( #(
-			match Tuple::reverse_ref(value.clone()) {
-				Ok(result) => return Ok(result),
-				Err(_) => (),
-			}
-		)* );
-		Err(())
-	}
-}
-
-/// Simple pass-through which implements `BytesConversion` while not doing any conversion.
-pub struct Identity;
-impl<T: Clone> Convert<T, T> for Identity {
-	fn convert(value: T) -> Result<T, T> {
-		Ok(value)
-	}
-	fn reverse(value: T) -> Result<T, T> {
-		Ok(value)
-	}
-}
-
-/// Implementation of `Convert` trait using `TryFrom`.
-pub struct JustTry;
-impl<Source: TryFrom<Dest> + Clone, Dest: TryFrom<Source> + Clone> Convert<Source, Dest>
-	for JustTry
-{
-	fn convert(value: Source) -> Result<Dest, Source> {
-		Dest::try_from(value.clone()).map_err(|_| value)
-	}
-	fn reverse(value: Dest) -> Result<Source, Dest> {
-		Source::try_from(value.clone()).map_err(|_| value)
-	}
-}
-
-/// Implementation of `Convert<_, Vec<u8>>` using the parity scale codec.
-pub struct Encoded;
-impl<T: Clone + Encode + Decode> Convert<T, Vec<u8>> for Encoded {
-	fn convert_ref(value: impl Borrow<T>) -> Result<Vec<u8>, ()> {
-		Ok(value.borrow().encode())
-	}
-	fn reverse_ref(bytes: impl Borrow<Vec<u8>>) -> Result<T, ()> {
-		T::decode(&mut &bytes.borrow()[..]).map_err(|_| ())
-	}
-}
-
-/// Implementation of `Convert<Vec<u8>, _>` using the parity scale codec.
-pub struct Decoded;
-impl<T: Clone + Encode + Decode> Convert<Vec<u8>, T> for Decoded {
-	fn convert_ref(bytes: impl Borrow<Vec<u8>>) -> Result<T, ()> {
-		T::decode(&mut &bytes.borrow()[..]).map_err(|_| ())
-	}
-	fn reverse_ref(value: impl Borrow<T>) -> Result<Vec<u8>, ()> {
-		Ok(value.borrow().encode())
 	}
 }
 

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

@@ -17,9 +17,7 @@
 //! Various traits used in configuring the executor.
 
 mod conversion;
-pub use conversion::{
+pub use conversion::{CallDispatcher, ConvertLocation, ConvertOrigin, WithOriginFilter};
-	CallDispatcher, Convert, ConvertOrigin, Decoded, Encoded, Identity, JustTry, WithOriginFilter,
-};
 mod drop_assets;
 pub use drop_assets::{ClaimAssets, DropAssets};
 mod asset_lock;
@@ -44,14 +42,17 @@ pub use should_execute::{CheckSuspension, Properties, ShouldExecute};
 mod transact_asset;
 pub use transact_asset::TransactAsset;
 mod weight;
+#[deprecated = "Use `sp_runtime::traits::` instead"]
+pub use sp_runtime::traits::{Identity, TryConvertInto as JustTry};
 pub use weight::{WeightBounds, WeightTrader};
 
 pub mod prelude {
 	pub use super::{
-		export_xcm, validate_export, AssetExchange, AssetLock, ClaimAssets, Convert, ConvertOrigin,
+		export_xcm, validate_export, AssetExchange, AssetLock, ClaimAssets, ConvertOrigin,
-		Decoded, DropAssets, Enact, Encoded, Error, ExportXcm, FeeManager, FeeReason, Identity,
+		DropAssets, Enact, Error, ExportXcm, FeeManager, FeeReason, LockError, MatchesFungible,
-		JustTry, LockError, MatchesFungible, MatchesFungibles, MatchesNonFungible,
+		MatchesFungibles, MatchesNonFungible, MatchesNonFungibles, OnResponse, ShouldExecute,
-		MatchesNonFungibles, OnResponse, ShouldExecute, TransactAsset, VersionChangeNotifier,
+		TransactAsset, VersionChangeNotifier, WeightBounds, WeightTrader, WithOriginFilter,
-		WeightBounds, WeightTrader, WithOriginFilter,
 	};
+	#[allow(deprecated)]
+	pub use super::{Identity, JustTry};
 }

polkadot/xcm/xcm-simulator/example/src/lib.rs

@@ -19,7 +19,7 @@ mod relay_chain;
 
 use frame_support::sp_tracing;
 use xcm::prelude::*;
-use xcm_executor::traits::Convert;
+use xcm_executor::traits::ConvertLocation;
 use xcm_simulator::{decl_test_network, decl_test_parachain, decl_test_relay_chain, TestExt};
 
 pub const ALICE: sp_runtime::AccountId32 = sp_runtime::AccountId32::new([0u8; 32]);
@@ -67,27 +67,27 @@ decl_test_network! {
 
 pub fn parent_account_id() -> parachain::AccountId {
 	let location = (Parent,);
-	parachain::LocationToAccountId::convert(location.into()).unwrap()
+	parachain::LocationToAccountId::convert_location(&location.into()).unwrap()
 }
 
 pub fn child_account_id(para: u32) -> relay_chain::AccountId {
 	let location = (Parachain(para),);
-	relay_chain::LocationToAccountId::convert(location.into()).unwrap()
+	relay_chain::LocationToAccountId::convert_location(&location.into()).unwrap()
 }
 
 pub fn child_account_account_id(para: u32, who: sp_runtime::AccountId32) -> relay_chain::AccountId {
 	let location = (Parachain(para), AccountId32 { network: None, id: who.into() });
-	relay_chain::LocationToAccountId::convert(location.into()).unwrap()
+	relay_chain::LocationToAccountId::convert_location(&location.into()).unwrap()
 }
 
 pub fn sibling_account_account_id(para: u32, who: sp_runtime::AccountId32) -> parachain::AccountId {
 	let location = (Parent, Parachain(para), AccountId32 { network: None, id: who.into() });
-	parachain::LocationToAccountId::convert(location.into()).unwrap()
+	parachain::LocationToAccountId::convert_location(&location.into()).unwrap()
 }
 
 pub fn parent_account_account_id(who: sp_runtime::AccountId32) -> parachain::AccountId {
 	let location = (Parent, AccountId32 { network: None, id: who.into() });
-	parachain::LocationToAccountId::convert(location.into()).unwrap()
+	parachain::LocationToAccountId::convert_location(&location.into()).unwrap()
 }
 
 pub fn para_ext(para_id: u32) -> sp_io::TestExternalities {

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

@@ -47,7 +47,7 @@ use xcm_builder::{
 	SovereignSignedViaLocation,
 };
 use xcm_executor::{
-	traits::{Convert, JustTry},
+	traits::{ConvertLocation, JustTry},
 	Config, XcmExecutor,
 };
 
@@ -160,7 +160,7 @@ impl EnsureOriginWithArg<RuntimeOrigin, MultiLocation> for ForeignCreators {
 		if !a.starts_with(&origin_location) {
 			return Err(o)
 		}
-		SovereignAccountOf::convert(origin_location).map_err(|_| o)
+		SovereignAccountOf::convert_location(&origin_location).ok_or(o)
 	}
 
 	#[cfg(feature = "runtime-benchmarks")]