From 30a6c7fc3a850f598f2ab61252a6647d017f89a5 Mon Sep 17 00:00:00 2001 From: Gavin Wood Date: Mon, 23 Aug 2021 19:53:59 +0200 Subject: [PATCH] XCM: Introduce versioning to dispatchables' params (#3693) * Introduce versioning to dispatchables' params * Fixes * Formatting * Bump --- bridges/primitives/chain-rococo/src/lib.rs | 2 +- runtime/kusama/src/lib.rs | 2 +- runtime/polkadot/src/lib.rs | 2 +- runtime/rococo/src/lib.rs | 2 +- runtime/westend/src/lib.rs | 2 +- xcm/pallet-xcm/src/lib.rs | 96 ++++++++----- xcm/pallet-xcm/src/tests.rs | 26 ++-- xcm/src/lib.rs | 135 ++++++++++++++++++ xcm/xcm-executor/integration-tests/src/lib.rs | 6 +- xcm/xcm-simulator/example/src/lib.rs | 6 +- 10 files changed, 222 insertions(+), 57 deletions(-) diff --git a/bridges/primitives/chain-rococo/src/lib.rs b/bridges/primitives/chain-rococo/src/lib.rs index d76ec8e679d3..b4faae00eeb3 100644 --- a/bridges/primitives/chain-rococo/src/lib.rs +++ b/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, diff --git a/runtime/kusama/src/lib.rs b/runtime/kusama/src/lib.rs index f259fc8285da..350d70a94a7e 100644 --- a/runtime/kusama/src/lib.rs +++ b/runtime/kusama/src/lib.rs @@ -115,7 +115,7 @@ pub const VERSION: RuntimeVersion = RuntimeVersion { spec_name: create_runtime_str!("kusama"), impl_name: create_runtime_str!("parity-kusama"), authoring_version: 2, - spec_version: 9090, + spec_version: 9100, impl_version: 0, #[cfg(not(feature = "disable-runtime-api"))] apis: RUNTIME_API_VERSIONS, diff --git a/runtime/polkadot/src/lib.rs b/runtime/polkadot/src/lib.rs index 2388edcc8878..ea4d64f133ab 100644 --- a/runtime/polkadot/src/lib.rs +++ b/runtime/polkadot/src/lib.rs @@ -96,7 +96,7 @@ pub const VERSION: RuntimeVersion = RuntimeVersion { spec_name: create_runtime_str!("polkadot"), impl_name: create_runtime_str!("parity-polkadot"), authoring_version: 0, - spec_version: 9090, + spec_version: 9100, impl_version: 0, #[cfg(not(feature = "disable-runtime-api"))] apis: RUNTIME_API_VERSIONS, diff --git a/runtime/rococo/src/lib.rs b/runtime/rococo/src/lib.rs index 68d4ee3cbf85..35d78487eca9 100644 --- a/runtime/rococo/src/lib.rs +++ b/runtime/rococo/src/lib.rs @@ -105,7 +105,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, diff --git a/runtime/westend/src/lib.rs b/runtime/westend/src/lib.rs index 77be0b1dafe9..dc7604a5ea3a 100644 --- a/runtime/westend/src/lib.rs +++ b/runtime/westend/src/lib.rs @@ -114,7 +114,7 @@ pub const VERSION: RuntimeVersion = RuntimeVersion { spec_name: create_runtime_str!("westend"), impl_name: create_runtime_str!("parity-westend"), authoring_version: 2, - spec_version: 9090, + spec_version: 9100, impl_version: 0, #[cfg(not(feature = "disable-runtime-api"))] apis: RUNTIME_API_VERSIONS, diff --git a/xcm/pallet-xcm/src/lib.rs b/xcm/pallet-xcm/src/lib.rs index 2ba7e6a68b53..c6772cb6df37 100644 --- a/xcm/pallet-xcm/src/lib.rs +++ b/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] @@ -118,17 +126,20 @@ pub mod pallet { #[pallet::weight(100_000_000)] pub fn send( origin: OriginFor, - dest: Box, - message: Box>, + dest: Box, + message: Box>, ) -> DispatchResult { let origin_location = T::SendXcmOrigin::ensure_origin(origin)?; let interior = origin_location.clone().try_into().map_err(|_| Error::::InvalidOrigin)?; - Self::send_xcm(interior, *dest.clone(), *message.clone()).map_err(|e| match e { + let dest = MultiLocation::try_from(*dest).map_err(|()| Error::::BadVersion)?; + let message: Xcm<()> = (*message).try_into().map_err(|()| Error::::BadVersion)?; + + Self::send_xcm(interior, dest.clone(), message.clone()).map_err(|e| match e { XcmError::CannotReachDestination(..) => Error::::Unreachable, _ => Error::::SendFailure, })?; - Self::deposit_event(Event::Sent(origin_location, *dest, *message)); + Self::deposit_event(Event::Sent(origin_location, dest, message)); Ok(()) } @@ -146,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 = (*assets.clone()).try_into(); + let maybe_dest: Result = (*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, - dest: Box, - beneficiary: Box, - assets: MultiAssets, + dest: Box, + beneficiary: Box, + assets: Box, 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::::BadVersion)?; + let beneficiary = + MultiLocation::try_from(*beneficiary).map_err(|()| Error::::BadVersion)?; + let assets = MultiAssets::try_from(*assets).map_err(|()| Error::::BadVersion)?; + ensure!(assets.len() <= MAX_ASSETS_FOR_TRANSFER, Error::::TooManyAssets); let value = (origin_location, assets.drain()); ensure!(T::XcmTeleportFilter::contains(&value), Error::::Filtered); @@ -182,7 +205,7 @@ pub mod pallet { assets, effects: vec![InitiateTeleport { assets: Wild(All), - dest: *dest, + dest, effects: vec![ BuyExecution { fees, @@ -192,7 +215,7 @@ pub mod pallet { halt_on_error: false, instructions: vec![], }, - DepositAsset { assets: Wild(All), max_assets, beneficiary: *beneficiary }, + DepositAsset { assets: Wild(All), max_assets, beneficiary }, ], }], }; @@ -219,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, - dest: Box, - beneficiary: Box, - assets: MultiAssets, + dest: Box, + beneficiary: Box, + assets: Box, fee_asset_item: u32, dest_weight: Weight, ) -> DispatchResult { let origin_location = T::ExecuteXcmOrigin::ensure_origin(origin)?; + let dest = (*dest).try_into().map_err(|()| Error::::BadVersion)?; + let beneficiary = (*beneficiary).try_into().map_err(|()| Error::::BadVersion)?; + let assets: MultiAssets = (*assets).try_into().map_err(|()| Error::::BadVersion)?; + ensure!(assets.len() <= MAX_ASSETS_FOR_TRANSFER, Error::::TooManyAssets); let value = (origin_location, assets.drain()); ensure!(T::XcmReserveTransferFilter::contains(&value), Error::::Filtered); @@ -250,7 +279,7 @@ pub mod pallet { let assets = assets.into(); let mut message = Xcm::TransferReserveAsset { assets, - dest: *dest, + dest, effects: vec![ BuyExecution { fees, @@ -260,7 +289,7 @@ pub mod pallet { halt_on_error: false, instructions: vec![], }, - DepositAsset { assets: Wild(All), max_assets, beneficiary: *beneficiary }, + DepositAsset { assets: Wild(All), max_assets, beneficiary }, ], }; let weight = @@ -285,11 +314,12 @@ pub mod pallet { #[pallet::weight(max_weight.saturating_add(100_000_000u64))] pub fn execute( origin: OriginFor, - message: Box>, + message: Box>, 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::::BadVersion)?; + let value = (origin_location, message); ensure!(T::XcmExecuteFilter::contains(&value), Error::::Filtered); let (origin_location, message) = value; let outcome = T::XcmExecutor::execute_xcm(origin_location, message, max_weight); diff --git a/xcm/pallet-xcm/src/tests.rs b/xcm/pallet-xcm/src/tests.rs index 62d5f87807bb..a790cf7374ce 100644 --- a/xcm/pallet-xcm/src/tests.rs +++ b/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]); @@ -46,8 +46,8 @@ fn send_works() { }; assert_ok!(XcmPallet::send( Origin::signed(ALICE), - Box::new(RelayLocation::get()), - Box::new(message.clone()) + Box::new(RelayLocation::get().into()), + Box::new(VersionedXcm::from(message.clone())) )); assert_eq!( sent_xcm(), @@ -88,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::::SendFailure ); @@ -109,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, )); @@ -138,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), - Box::new(Parachain(PARA_ID).into()), - Box::new(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 )); @@ -184,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); diff --git a/xcm/src/lib.rs b/xcm/src/lib.rs index fa9e0c3b4b45..7e460e792f31 100644 --- a/xcm/src/lib.rs +++ b/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 for VersionedMultiLocation { + fn from(x: v0::MultiLocation) -> Self { + VersionedMultiLocation::V0(x) + } +} + +impl> From for VersionedMultiLocation { + fn from(x: T) -> Self { + VersionedMultiLocation::V1(x.into()) + } +} + +impl TryFrom for v0::MultiLocation { + type Error = (); + fn try_from(x: VersionedMultiLocation) -> Result { + use VersionedMultiLocation::*; + match x { + V0(x) => Ok(x), + V1(x) => x.try_into(), + } + } +} + +impl TryFrom for v1::MultiLocation { + type Error = (); + fn try_from(x: VersionedMultiLocation) -> Result { + 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 for VersionedMultiAsset { + fn from(x: v0::MultiAsset) -> Self { + VersionedMultiAsset::V0(x) + } +} + +impl> From for VersionedMultiAsset { + fn from(x: T) -> Self { + VersionedMultiAsset::V1(x.into()) + } +} + +impl TryFrom for v0::MultiAsset { + type Error = (); + fn try_from(x: VersionedMultiAsset) -> Result { + use VersionedMultiAsset::*; + match x { + V0(x) => Ok(x), + V1(x) => x.try_into(), + } + } +} + +impl TryFrom for v1::MultiAsset { + type Error = (); + fn try_from(x: VersionedMultiAsset) -> Result { + 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`. +#[derive(Derivative, Encode, Decode)] +#[derivative(Clone(bound = ""), Eq(bound = ""), PartialEq(bound = ""), Debug(bound = ""))] +#[codec(encode_bound())] +#[codec(decode_bound())] +pub enum VersionedMultiAssets { + V0(Vec), + V1(v1::MultiAssets), +} + +impl From> for VersionedMultiAssets { + fn from(x: Vec) -> Self { + VersionedMultiAssets::V0(x) + } +} + +impl> From for VersionedMultiAssets { + fn from(x: T) -> Self { + VersionedMultiAssets::V1(x.into()) + } +} + +impl TryFrom for Vec { + type Error = (); + fn try_from(x: VersionedMultiAssets) -> Result { + use VersionedMultiAssets::*; + match x { + V0(x) => Ok(x), + V1(x) => x.try_into(), + } + } +} + +impl TryFrom for v1::MultiAssets { + type Error = (); + fn try_from(x: VersionedMultiAssets) -> Result { + 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 = ""))] diff --git a/xcm/xcm-executor/integration-tests/src/lib.rs b/xcm/xcm-executor/integration-tests/src/lib.rs index 66a997e11a62..78b03dd82ccd 100644 --- a/xcm/xcm-executor/integration-tests/src/lib.rs +++ b/xcm/xcm-executor/integration-tests/src/lib.rs @@ -24,7 +24,7 @@ use polkadot_test_client::{ use polkadot_test_service::construct_extrinsic; use sp_runtime::{generic::BlockId, traits::Block}; use sp_state_machine::InspectState; -use xcm::latest::prelude::*; +use xcm::{latest::prelude::*, VersionedXcm}; use xcm_executor::MAX_RECURSION_LIMIT; // This is the inflection point where the test should either fail or pass. @@ -57,7 +57,7 @@ fn execute_within_recursion_limit() { let execute = construct_extrinsic( &client, polkadot_test_runtime::Call::Xcm(pallet_xcm::Call::execute( - Box::new(msg.clone()), + Box::new(VersionedXcm::from(msg.clone())), 1_000_000_000, )), sp_keyring::Sr25519Keyring::Alice, @@ -111,7 +111,7 @@ fn exceed_recursion_limit() { let execute = construct_extrinsic( &client, polkadot_test_runtime::Call::Xcm(pallet_xcm::Call::execute( - Box::new(msg.clone()), + Box::new(VersionedXcm::from(msg.clone())), 1_000_000_000, )), sp_keyring::Sr25519Keyring::Alice, diff --git a/xcm/xcm-simulator/example/src/lib.rs b/xcm/xcm-simulator/example/src/lib.rs index 6fee8d08aab2..964046d6f2f0 100644 --- a/xcm/xcm-simulator/example/src/lib.rs +++ b/xcm/xcm-simulator/example/src/lib.rs @@ -211,9 +211,9 @@ mod tests { Relay::execute_with(|| { assert_ok!(RelayChainPalletXcm::reserve_transfer_assets( relay_chain::Origin::signed(ALICE), - Box::new(X1(Parachain(1)).into()), - Box::new(X1(AccountId32 { network: Any, id: ALICE.into() }).into()), - (Here, withdraw_amount).into(), + Box::new(X1(Parachain(1)).into().into()), + Box::new(X1(AccountId32 { network: Any, id: ALICE.into() }).into().into()), + Box::new((Here, withdraw_amount).into()), 0, max_weight_for_execution, ));