XCM: Introduce versioning to dispatchables' params (#3693)

* Introduce versioning to dispatchables' params

* Fixes

* Formatting

* Bump

bridges/primitives/chain-rococo/src/lib.rs

@@ -42,7 +42,7 @@ pub const VERSION: RuntimeVersion = RuntimeVersion {
 	spec_name: sp_version::create_runtime_str!("rococo"),
 	impl_name: sp_version::create_runtime_str!("parity-rococo-v1.6"),
 	authoring_version: 0,
-	spec_version: 9004,
+	spec_version: 9100,
 	impl_version: 0,
 	apis: sp_version::create_apis_vec![[]],
 	transaction_version: 0,

runtime/rococo/src/lib.rs

@@ -103,7 +103,7 @@ pub const VERSION: RuntimeVersion = RuntimeVersion {
 	spec_name: create_runtime_str!("rococo"),
 	impl_name: create_runtime_str!("parity-rococo-v1.6"),
 	authoring_version: 0,
-	spec_version: 9004,
+	spec_version: 9100,
 	impl_version: 0,
 	#[cfg(not(feature = "disable-runtime-api"))]
 	apis: RUNTIME_API_VERSIONS,

xcm/pallet-xcm/src/lib.rs

@@ -26,8 +26,14 @@ mod tests;
 use codec::{Decode, Encode};
 use frame_support::traits::{Contains, EnsureOrigin, Get, OriginTrait};
 use sp_runtime::{traits::BadOrigin, RuntimeDebug};
-use sp_std::{boxed::Box, convert::TryInto, marker::PhantomData, prelude::*, vec};
-use xcm::latest::prelude::*;
+use sp_std::{
+	boxed::Box,
+	convert::{TryFrom, TryInto},
+	marker::PhantomData,
+	prelude::*,
+	vec,
+};
+use xcm::{latest::prelude::*, VersionedMultiAssets, VersionedMultiLocation, VersionedXcm};
 use xcm_executor::traits::ConvertOrigin;
 
 use frame_support::PalletId;
@@ -108,6 +114,8 @@ pub mod pallet {
 		TooManyAssets,
 		/// Origin is invalid for sending.
 		InvalidOrigin,
+		/// The version of the `Versioned` value used is not able to be interpreted.
+		BadVersion,
 	}
 
 	#[pallet::hooks]
@@ -116,15 +124,22 @@ pub mod pallet {
 	#[pallet::call]
 	impl<T: Config> Pallet<T> {
 		#[pallet::weight(100_000_000)]
-		pub fn send(origin: OriginFor<T>, dest: MultiLocation, message: Xcm<()>) -> DispatchResult {
+		pub fn send(
+			origin: OriginFor<T>,
+			dest: Box<VersionedMultiLocation>,
+			message: Box<VersionedXcm<()>>,
+		) -> DispatchResult {
 			let origin_location = T::SendXcmOrigin::ensure_origin(origin)?;
 			let interior =
 				origin_location.clone().try_into().map_err(|_| Error::<T>::InvalidOrigin)?;
+			let dest = MultiLocation::try_from(*dest).map_err(|()| Error::<T>::BadVersion)?;
+			let message: Xcm<()> = (*message).try_into().map_err(|()| Error::<T>::BadVersion)?;
+
 			Self::send_xcm(interior, dest.clone(), message.clone()).map_err(|e| match e {
 				XcmError::CannotReachDestination(..) => Error::<T>::Unreachable,
 				_ => Error::<T>::SendFailure,
 			})?;
-			Self::deposit_event(Event::Sent(origin_location, *dest, *message));
+			Self::deposit_event(Event::Sent(origin_location, dest, message));
 			Ok(())
 		}
 
@@ -142,25 +157,37 @@ pub mod pallet {
 		/// - `dest_weight`: Equal to the total weight on `dest` of the XCM message
 		///   `Teleport { assets, effects: [ BuyExecution{..}, DepositAsset{..} ] }`.
 		#[pallet::weight({
-			let mut message = Xcm::WithdrawAsset {
-				assets: assets.clone(),
-				effects: sp_std::vec![ InitiateTeleport {
-					assets: Wild(All),
-					dest: dest.clone(),
-					effects: sp_std::vec![],
-				} ]
-			};
-			T::Weigher::weight(&mut message).map_or(Weight::max_value(), |w| 100_000_000 + w)
+			let maybe_assets: Result<MultiAssets, ()> = (*assets.clone()).try_into();
+			let maybe_dest: Result<MultiLocation, ()> = (*dest.clone()).try_into();
+			match (maybe_assets, maybe_dest) {
+				(Ok(assets), Ok(dest)) => {
+					let mut message = Xcm::WithdrawAsset {
+						assets,
+						effects: sp_std::vec![ InitiateTeleport {
+							assets: Wild(All),
+							dest,
+							effects: sp_std::vec![],
+						} ]
+					};
+					T::Weigher::weight(&mut message).map_or(Weight::max_value(), |w| 100_000_000 + w)
+				},
+				_ => Weight::max_value(),
+			}
 		})]
 		pub fn teleport_assets(
 			origin: OriginFor<T>,
-			dest: MultiLocation,
-			beneficiary: MultiLocation,
-			assets: MultiAssets,
+			dest: Box<VersionedMultiLocation>,
+			beneficiary: Box<VersionedMultiLocation>,
+			assets: Box<VersionedMultiAssets>,
 			fee_asset_item: u32,
 			dest_weight: Weight,
 		) -> DispatchResult {
 			let origin_location = T::ExecuteXcmOrigin::ensure_origin(origin)?;
+			let dest = MultiLocation::try_from(*dest).map_err(|()| Error::<T>::BadVersion)?;
+			let beneficiary =
+				MultiLocation::try_from(*beneficiary).map_err(|()| Error::<T>::BadVersion)?;
+			let assets = MultiAssets::try_from(*assets).map_err(|()| Error::<T>::BadVersion)?;
+
 			ensure!(assets.len() <= MAX_ASSETS_FOR_TRANSFER, Error::<T>::TooManyAssets);
 			let value = (origin_location, assets.drain());
 			ensure!(T::XcmTeleportFilter::contains(&value), Error::<T>::Filtered);
@@ -215,22 +242,28 @@ pub mod pallet {
 		/// - `dest_weight`: Equal to the total weight on `dest` of the XCM message
 		///   `ReserveAssetDeposited { assets, effects: [ BuyExecution{..}, DepositAsset{..} ] }`.
 		#[pallet::weight({
-			let mut message = Xcm::TransferReserveAsset {
-				assets: assets.clone(),
-				dest: dest.clone(),
-				effects: sp_std::vec![],
-			};
-			T::Weigher::weight(&mut message).map_or(Weight::max_value(), |w| 100_000_000 + w)
+			match ((*assets.clone()).try_into(), (*dest.clone()).try_into()) {
+				(Ok(assets), Ok(dest)) => {
+					let effects = sp_std::vec![];
+					let mut message = Xcm::TransferReserveAsset { assets, dest, effects };
+					T::Weigher::weight(&mut message).map_or(Weight::max_value(), |w| 100_000_000 + w)
+				},
+				_ => Weight::max_value(),
+			}
 		})]
 		pub fn reserve_transfer_assets(
 			origin: OriginFor<T>,
-			dest: MultiLocation,
-			beneficiary: MultiLocation,
-			assets: MultiAssets,
+			dest: Box<VersionedMultiLocation>,
+			beneficiary: Box<VersionedMultiLocation>,
+			assets: Box<VersionedMultiAssets>,
 			fee_asset_item: u32,
 			dest_weight: Weight,
 		) -> DispatchResult {
 			let origin_location = T::ExecuteXcmOrigin::ensure_origin(origin)?;
+			let dest = (*dest).try_into().map_err(|()| Error::<T>::BadVersion)?;
+			let beneficiary = (*beneficiary).try_into().map_err(|()| Error::<T>::BadVersion)?;
+			let assets: MultiAssets = (*assets).try_into().map_err(|()| Error::<T>::BadVersion)?;
+
 			ensure!(assets.len() <= MAX_ASSETS_FOR_TRANSFER, Error::<T>::TooManyAssets);
 			let value = (origin_location, assets.drain());
 			ensure!(T::XcmReserveTransferFilter::contains(&value), Error::<T>::Filtered);
@@ -281,11 +314,12 @@ pub mod pallet {
 		#[pallet::weight(max_weight.saturating_add(100_000_000u64))]
 		pub fn execute(
 			origin: OriginFor<T>,
-			message: Box<Xcm<T::Call>>,
+			message: Box<VersionedXcm<T::Call>>,
 			max_weight: Weight,
 		) -> DispatchResult {
 			let origin_location = T::ExecuteXcmOrigin::ensure_origin(origin)?;
-			let value = (origin_location, *message);
+			let message = (*message).try_into().map_err(|()| Error::<T>::BadVersion)?;
+			let value = (origin_location, message);
 			ensure!(T::XcmExecuteFilter::contains(&value), Error::<T>::Filtered);
 			let (origin_location, message) = value;
 			let outcome = T::XcmExecutor::execute_xcm(origin_location, message, max_weight);

xcm/pallet-xcm/src/tests.rs

@@ -18,7 +18,7 @@ use crate::mock::*;
 use frame_support::{assert_noop, assert_ok, traits::Currency};
 use polkadot_parachain::primitives::{AccountIdConversion, Id as ParaId};
 use std::convert::TryInto;
-use xcm::v1::prelude::*;
+use xcm::{v1::prelude::*, VersionedXcm};
 
 const ALICE: AccountId = AccountId::new([0u8; 32]);
 const BOB: AccountId = AccountId::new([1u8; 32]);
@@ -44,7 +44,11 @@ fn send_works() {
 				DepositAsset { assets: All.into(), max_assets: 1, beneficiary: sender.clone() },
 			],
 		};
-		assert_ok!(XcmPallet::send(Origin::signed(ALICE), RelayLocation::get(), message.clone()));
+		assert_ok!(XcmPallet::send(
+			Origin::signed(ALICE),
+			Box::new(RelayLocation::get().into()),
+			Box::new(VersionedXcm::from(message.clone()))
+		));
 		assert_eq!(
 			sent_xcm(),
 			vec![(
@@ -84,8 +88,8 @@ fn send_fails_when_xcm_router_blocks() {
 		assert_noop!(
 			XcmPallet::send(
 				Origin::signed(ALICE),
-				Box::new(MultiLocation::ancestor(8)),
-				Box::new(message.clone())
+				Box::new(MultiLocation::ancestor(8).into()),
+				Box::new(VersionedXcm::from(message.clone())),
 			),
 			crate::Error::<Test>::SendFailure
 		);
@@ -105,9 +109,9 @@ fn teleport_assets_works() {
 		assert_eq!(Balances::total_balance(&ALICE), INITIAL_BALANCE);
 		assert_ok!(XcmPallet::teleport_assets(
 			Origin::signed(ALICE),
-			Box::new(RelayLocation::get()),
-			Box::new(AccountId32 { network: Any, id: BOB.into() }.into()),
-			(Here, SEND_AMOUNT).into(),
+			Box::new(RelayLocation::get().into()),
+			Box::new(AccountId32 { network: Any, id: BOB.into() }.into().into()),
+			Box::new((Here, SEND_AMOUNT).into()),
 			0,
 			weight,
 		));
@@ -134,9 +138,9 @@ fn reserve_transfer_assets_works() {
 		assert_eq!(Balances::total_balance(&ALICE), INITIAL_BALANCE);
 		assert_ok!(XcmPallet::reserve_transfer_assets(
 			Origin::signed(ALICE),
-			Parachain(PARA_ID).into(),
-			dest.clone(),
-			(Here, SEND_AMOUNT).into(),
+			Box::new(Parachain(PARA_ID).into().into()),
+			Box::new(dest.clone().into()),
+			Box::new((Here, SEND_AMOUNT).into()),
 			0,
 			weight
 		));
@@ -180,13 +184,13 @@ fn execute_withdraw_to_deposit_works() {
 		assert_eq!(Balances::total_balance(&ALICE), INITIAL_BALANCE);
 		assert_ok!(XcmPallet::execute(
 			Origin::signed(ALICE),
-			Box::new(Xcm::WithdrawAsset {
+			Box::new(VersionedXcm::from(Xcm::WithdrawAsset {
 				assets: (Here, SEND_AMOUNT).into(),
 				effects: vec![
 					buy_execution((Here, SEND_AMOUNT), weight),
 					DepositAsset { assets: All.into(), max_assets: 1, beneficiary: dest }
 				],
-			}),
+			})),
 			weight
 		));
 		assert_eq!(Balances::total_balance(&ALICE), INITIAL_BALANCE - SEND_AMOUNT);

xcm/src/lib.rs

@@ -23,6 +23,7 @@
 #![no_std]
 extern crate alloc;
 
+use alloc::vec::Vec;
 use core::{
 	convert::{TryFrom, TryInto},
 	result::Result,
@@ -52,6 +53,140 @@ impl Decode for Unsupported {
 	}
 }
 
+/// A single `MultiLocation` value, together with its version code.
+#[derive(Derivative, Encode, Decode)]
+#[derivative(Clone(bound = ""), Eq(bound = ""), PartialEq(bound = ""), Debug(bound = ""))]
+#[codec(encode_bound())]
+#[codec(decode_bound())]
+pub enum VersionedMultiLocation {
+	V0(v0::MultiLocation),
+	V1(v1::MultiLocation),
+}
+
+impl From<v0::MultiLocation> for VersionedMultiLocation {
+	fn from(x: v0::MultiLocation) -> Self {
+		VersionedMultiLocation::V0(x)
+	}
+}
+
+impl<T: Into<v1::MultiLocation>> From<T> for VersionedMultiLocation {
+	fn from(x: T) -> Self {
+		VersionedMultiLocation::V1(x.into())
+	}
+}
+
+impl TryFrom<VersionedMultiLocation> for v0::MultiLocation {
+	type Error = ();
+	fn try_from(x: VersionedMultiLocation) -> Result<Self, ()> {
+		use VersionedMultiLocation::*;
+		match x {
+			V0(x) => Ok(x),
+			V1(x) => x.try_into(),
+		}
+	}
+}
+
+impl TryFrom<VersionedMultiLocation> for v1::MultiLocation {
+	type Error = ();
+	fn try_from(x: VersionedMultiLocation) -> Result<Self, ()> {
+		use VersionedMultiLocation::*;
+		match x {
+			V0(x) => x.try_into(),
+			V1(x) => Ok(x),
+		}
+	}
+}
+
+/// A single `MultiAsset` value, together with its version code.
+#[derive(Derivative, Encode, Decode)]
+#[derivative(Clone(bound = ""), Eq(bound = ""), PartialEq(bound = ""), Debug(bound = ""))]
+#[codec(encode_bound())]
+#[codec(decode_bound())]
+pub enum VersionedMultiAsset {
+	V0(v0::MultiAsset),
+	V1(v1::MultiAsset),
+}
+
+impl From<v0::MultiAsset> for VersionedMultiAsset {
+	fn from(x: v0::MultiAsset) -> Self {
+		VersionedMultiAsset::V0(x)
+	}
+}
+
+impl<T: Into<v1::MultiAsset>> From<T> for VersionedMultiAsset {
+	fn from(x: T) -> Self {
+		VersionedMultiAsset::V1(x.into())
+	}
+}
+
+impl TryFrom<VersionedMultiAsset> for v0::MultiAsset {
+	type Error = ();
+	fn try_from(x: VersionedMultiAsset) -> Result<Self, ()> {
+		use VersionedMultiAsset::*;
+		match x {
+			V0(x) => Ok(x),
+			V1(x) => x.try_into(),
+		}
+	}
+}
+
+impl TryFrom<VersionedMultiAsset> for v1::MultiAsset {
+	type Error = ();
+	fn try_from(x: VersionedMultiAsset) -> Result<Self, ()> {
+		use VersionedMultiAsset::*;
+		match x {
+			V0(x) => x.try_into(),
+			V1(x) => Ok(x),
+		}
+	}
+}
+
+/// A single `MultiAssets` value, together with its version code.
+///
+/// NOTE: For XCM v0, this was `Vec<MultiAsset>`.
+#[derive(Derivative, Encode, Decode)]
+#[derivative(Clone(bound = ""), Eq(bound = ""), PartialEq(bound = ""), Debug(bound = ""))]
+#[codec(encode_bound())]
+#[codec(decode_bound())]
+pub enum VersionedMultiAssets {
+	V0(Vec<v0::MultiAsset>),
+	V1(v1::MultiAssets),
+}
+
+impl From<Vec<v0::MultiAsset>> for VersionedMultiAssets {
+	fn from(x: Vec<v0::MultiAsset>) -> Self {
+		VersionedMultiAssets::V0(x)
+	}
+}
+
+impl<T: Into<v1::MultiAssets>> From<T> for VersionedMultiAssets {
+	fn from(x: T) -> Self {
+		VersionedMultiAssets::V1(x.into())
+	}
+}
+
+impl TryFrom<VersionedMultiAssets> for Vec<v0::MultiAsset> {
+	type Error = ();
+	fn try_from(x: VersionedMultiAssets) -> Result<Self, ()> {
+		use VersionedMultiAssets::*;
+		match x {
+			V0(x) => Ok(x),
+			V1(x) => x.try_into(),
+		}
+	}
+}
+
+impl TryFrom<VersionedMultiAssets> for v1::MultiAssets {
+	type Error = ();
+	fn try_from(x: VersionedMultiAssets) -> Result<Self, ()> {
+		use VersionedMultiAssets::*;
+		match x {
+			V0(x) => x.try_into(),
+			V1(x) => Ok(x),
+		}
+	}
+}
+
 /// A single XCM message, together with its version code.
 #[derive(Derivative, Encode, Decode)]
 #[derivative(Clone(bound = ""), Eq(bound = ""), PartialEq(bound = ""), Debug(bound = ""))]