XCM: Remove & replace XCM Convert trait

Cargo.toml

@@ -214,6 +214,11 @@ jemalloc-allocator = ["polkadot-node-core-pvf-prepare-worker/jemalloc-allocator"
 
 
 
+
+
+
+
+
 # Configuration for building a .deb package - for use with `cargo-deb`
 [package.metadata.deb]
 name = "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());

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)
 }

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 {
-	fn convert(ml: MultiLocation) -> Result<u64, MultiLocation> {
+impl xcm_executor::traits::ConvertLocation<u64> for AccountIdConverter {
+	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()),
-			_ => Err(ml),
+				Some(<u64 as codec::Decode>::decode(&mut &*id.to_vec()).unwrap()),
+			_ => None,
 		}
 	}
-
-	fn reverse(acc: u64) -> Result<MultiLocation, u64> {
-		Err(acc)
-	}
 }
 
 parameter_types! {

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,
-		QueryResponseStatus, VersionChangeNotifier, WeightBounds,
+		CheckSuspension, ClaimAssets, ConvertLocation, DropAssets, MatchesFungible, OnResponse,
+		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();

xcm/xcm-builder/src/asset_conversion.rs

@@ -17,41 +17,45 @@
 //! 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
 /// `PalletInstance` junction.
 pub struct AsPrefixedGeneralIndex<Prefix, AssetId, ConvertAssetId>(
 	PhantomData<(Prefix, AssetId, ConvertAssetId)>,
 );
-impl<Prefix: Get<MultiLocation>, AssetId: Clone, ConvertAssetId: Convert<u128, AssetId>>
-	Convert<MultiLocation, AssetId> for AsPrefixedGeneralIndex<Prefix, AssetId, ConvertAssetId>
+impl<
+		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()
 				.iter()
 				.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),
-			_ => Err(()),
+			Some(Junction::GeneralIndex(id)) => ConvertAssetId::convert(id),
+			_ => 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)?;
-		location.push_interior(Junction::GeneralIndex(id)).map_err(|_| ())?;
-		Ok(location)
+		let id = ConvertAssetId::convert_back(what)?;
+		location.push_interior(Junction::GeneralIndex(id)).ok()?;
+		Some(location)
 	}
 }
 
@@ -61,8 +65,8 @@ pub struct ConvertedConcreteId<AssetId, Balance, ConvertAssetId, ConvertOther>(
 impl<
 		AssetId: Clone,
 		Balance: Clone,
-		ConvertAssetId: Convert<MultiLocation, AssetId>,
-		ConvertBalance: Convert<u128, Balance>,
+		ConvertAssetId: MaybeEquivalence<MultiLocation, AssetId>,
+		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 =
-			ConvertAssetId::convert_ref(id).map_err(|_| MatchError::AssetIdConversionFailed)?;
-		let amount = ConvertBalance::convert_ref(amount)
-			.map_err(|_| MatchError::AmountToBalanceConversionFailed)?;
+		let what = ConvertAssetId::convert(id).ok_or(MatchError::AssetIdConversionFailed)?;
+		let amount =
+			ConvertBalance::convert(amount).ok_or(MatchError::AmountToBalanceConversionFailed)?;
 		Ok((what, amount))
 	}
 }
 impl<
 		ClassId: Clone,
 		InstanceId: Clone,
-		ConvertClassId: Convert<MultiLocation, ClassId>,
-		ConvertInstanceId: Convert<AssetInstance, InstanceId>,
+		ConvertClassId: MaybeEquivalence<MultiLocation, ClassId>,
+		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 =
-			ConvertClassId::convert_ref(class).map_err(|_| MatchError::AssetIdConversionFailed)?;
-		let instance = ConvertInstanceId::convert_ref(instance)
-			.map_err(|_| MatchError::InstanceConversionFailed)?;
+		let what = ConvertClassId::convert(class).ok_or(MatchError::AssetIdConversionFailed)?;
+		let instance =
+			ConvertInstanceId::convert(instance).ok_or(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>,
-		ConvertBalance: Convert<u128, Balance>,
+		ConvertAssetId: MaybeEquivalence<[u8; 32], AssetId>,
+		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 =
-			ConvertAssetId::convert_ref(id).map_err(|_| MatchError::AssetIdConversionFailed)?;
-		let amount = ConvertBalance::convert_ref(amount)
-			.map_err(|_| MatchError::AmountToBalanceConversionFailed)?;
+		let what = ConvertAssetId::convert(id).ok_or(MatchError::AssetIdConversionFailed)?;
+		let amount =
+			ConvertBalance::convert(amount).ok_or(MatchError::AmountToBalanceConversionFailed)?;
 		Ok((what, amount))
 	}
 }
 impl<
 		ClassId: Clone,
 		InstanceId: Clone,
-		ConvertClassId: Convert<[u8; 32], ClassId>,
-		ConvertInstanceId: Convert<AssetInstance, InstanceId>,
+		ConvertClassId: MaybeEquivalence<[u8; 32], ClassId>,
+		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 =
-			ConvertClassId::convert_ref(class).map_err(|_| MatchError::AssetIdConversionFailed)?;
-		let instance = ConvertInstanceId::convert_ref(instance)
-			.map_err(|_| MatchError::InstanceConversionFailed)?;
+		let what = ConvertClassId::convert(class).ok_or(MatchError::AssetIdConversionFailed)?;
+		let instance =
+			ConvertInstanceId::convert(instance).ok_or(MatchError::InstanceConversionFailed)?;
 		Ok((what, instance))
 	}
 }
@@ -155,8 +155,8 @@ impl<
 		AssetId: Clone,
 		Balance: Clone,
 		MatchAssetId: Contains<MultiLocation>,
-		ConvertAssetId: Convert<MultiLocation, AssetId>,
-		ConvertBalance: Convert<u128, Balance>,
+		ConvertAssetId: MaybeEquivalence<MultiLocation, AssetId>,
+		ConvertBalance: MaybeEquivalence<u128, Balance>,
 	> MatchesFungibles<AssetId, Balance>
 	for MatchedConvertedConcreteId<AssetId, Balance, MatchAssetId, ConvertAssetId, ConvertBalance>
 {
@@ -165,19 +165,18 @@ impl<
 			(Fungible(ref amount), Concrete(ref id)) if MatchAssetId::contains(id) => (amount, id),
 			_ => return Err(MatchError::AssetNotHandled),
 		};
-		let what =
-			ConvertAssetId::convert_ref(id).map_err(|_| MatchError::AssetIdConversionFailed)?;
-		let amount = ConvertBalance::convert_ref(amount)
-			.map_err(|_| MatchError::AmountToBalanceConversionFailed)?;
+		let what = ConvertAssetId::convert(id).ok_or(MatchError::AssetIdConversionFailed)?;
+		let amount =
+			ConvertBalance::convert(amount).ok_or(MatchError::AmountToBalanceConversionFailed)?;
 		Ok((what, amount))
 	}
 }
 impl<
 		ClassId: Clone,
 		InstanceId: Clone,
 		MatchClassId: Contains<MultiLocation>,
-		ConvertClassId: Convert<MultiLocation, ClassId>,
-		ConvertInstanceId: Convert<AssetInstance, InstanceId>,
+		ConvertClassId: MaybeEquivalence<MultiLocation, ClassId>,
+		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 =
-			ConvertClassId::convert_ref(class).map_err(|_| MatchError::AssetIdConversionFailed)?;
-		let instance = ConvertInstanceId::convert_ref(instance)
-			.map_err(|_| MatchError::InstanceConversionFailed)?;
+		let what = ConvertClassId::convert(class).ok_or(MatchError::AssetIdConversionFailed)?;
+		let instance =
+			ConvertInstanceId::convert(instance).ok_or(MatchError::InstanceConversionFailed)?;
 		Ok((what, instance))
 	}
 }
@@ -286,13 +284,13 @@ mod tests {
 
 		// ConvertedConcreteId cfg
 		struct ClassInstanceIdConverter;
-		impl Convert<AssetInstance, ClassInstanceId> for ClassInstanceIdConverter {
-			fn convert_ref(value: impl Borrow<AssetInstance>) -> Result<ClassInstanceId, ()> {
-				value.borrow().clone().try_into().map_err(|_| ())
+		impl MaybeEquivalence<AssetInstance, ClassInstanceId> for ClassInstanceIdConverter {
+			fn convert(value: &AssetInstance) -> Option<ClassInstanceId> {
+				value.clone().try_into().ok()
 			}
 
-			fn reverse_ref(value: impl Borrow<ClassInstanceId>) -> Result<AssetInstance, ()> {
-				Ok(AssetInstance::from(value.borrow().clone()))
+			fn convert_back(value: &ClassInstanceId) -> Option<AssetInstance> {
+				Some(AssetInstance::from(value.clone()))
 			}
 		}