diff --git a/polkadot/Cargo.lock b/polkadot/Cargo.lock index 6d5673ca93..d618a7940e 100644 --- a/polkadot/Cargo.lock +++ b/polkadot/Cargo.lock @@ -7216,6 +7216,7 @@ dependencies = [ "pallet-transaction-payment", "pallet-transaction-payment-rpc-runtime-api", "pallet-vesting", + "pallet-xcm", "parity-scale-codec", "polkadot-parachain", "polkadot-primitives", @@ -7244,6 +7245,9 @@ dependencies = [ "sp-version", "substrate-wasm-builder", "tiny-keccak", + "xcm", + "xcm-builder", + "xcm-executor", ] [[package]] @@ -12303,6 +12307,26 @@ dependencies = [ "xcm", ] +[[package]] +name = "xcm-executor-integration-tests" +version = "0.9.9" +dependencies = [ + "frame-support", + "frame-system", + "futures 0.3.16", + "pallet-xcm", + "polkadot-test-client", + "polkadot-test-runtime", + "polkadot-test-service", + "sp-consensus", + "sp-keyring", + "sp-runtime", + "sp-state-machine", + "sp-tracing", + "xcm", + "xcm-executor", +] + [[package]] name = "xcm-simulator" version = "0.9.9" diff --git a/polkadot/Cargo.toml b/polkadot/Cargo.toml index cfd1ef2d75..02c1d06224 100644 --- a/polkadot/Cargo.toml +++ b/polkadot/Cargo.toml @@ -39,6 +39,7 @@ members = [ "xcm", "xcm/xcm-builder", "xcm/xcm-executor", + "xcm/xcm-executor/integration-tests", "xcm/xcm-simulator", "xcm/xcm-simulator/example", "xcm/pallet-xcm", diff --git a/polkadot/runtime/parachains/src/ump.rs b/polkadot/runtime/parachains/src/ump.rs index 31a7287540..40f1a5a3ac 100644 --- a/polkadot/runtime/parachains/src/ump.rs +++ b/polkadot/runtime/parachains/src/ump.rs @@ -84,14 +84,18 @@ impl, C: Config> UmpSink for XcmSi data: &[u8], max_weight: Weight, ) -> Result { + use parity_scale_codec::DecodeLimit; use xcm::{ latest::{Error as XcmError, Junction, MultiLocation, Xcm}, VersionedXcm, }; let id = sp_io::hashing::blake2_256(&data[..]); - let maybe_msg = - VersionedXcm::::decode(&mut &data[..]).map(Xcm::::try_from); + let maybe_msg = VersionedXcm::::decode_all_with_depth_limit( + xcm::MAX_XCM_DECODE_DEPTH, + &mut &data[..], + ) + .map(Xcm::::try_from); match maybe_msg { Err(_) => { Pallet::::deposit_event(Event::InvalidFormat(id)); diff --git a/polkadot/runtime/test-runtime/Cargo.toml b/polkadot/runtime/test-runtime/Cargo.toml index 853eacc685..88cece7375 100644 --- a/polkadot/runtime/test-runtime/Cargo.toml +++ b/polkadot/runtime/test-runtime/Cargo.toml @@ -55,8 +55,12 @@ pallet-vesting = { git = "https://github.com/paritytech/substrate", branch = "ma runtime-common = { package = "polkadot-runtime-common", path = "../common", default-features = false } primitives = { package = "polkadot-primitives", path = "../../primitives", default-features = false } +pallet-xcm = { path = "../../xcm/pallet-xcm", default-features = false } polkadot-parachain = { path = "../../parachain", default-features = false } polkadot-runtime-parachains = { path = "../parachains", default-features = false } +xcm-builder = { path = "../../xcm/xcm-builder", default-features = false } +xcm-executor = { path = "../../xcm/xcm-executor", default-features = false } +xcm = { path = "../../xcm", default-features = false } [dev-dependencies] hex-literal = "0.3.3" @@ -82,6 +86,10 @@ std = [ "inherents/std", "sp-core/std", "polkadot-parachain/std", + "pallet-xcm/std", + "xcm-builder/std", + "xcm-executor/std", + "xcm/std", "sp-api/std", "tx-pool-api/std", "block-builder-api/std", diff --git a/polkadot/runtime/test-runtime/src/lib.rs b/polkadot/runtime/test-runtime/src/lib.rs index abc87e9af9..f69e18cd89 100644 --- a/polkadot/runtime/test-runtime/src/lib.rs +++ b/polkadot/runtime/test-runtime/src/lib.rs @@ -79,6 +79,7 @@ pub use sp_runtime::BuildStorage; /// Constant values used within the runtime. pub mod constants; +pub mod xcm_config; use constants::{currency::*, fee::*, time::*}; // Make the WASM binary available. @@ -488,6 +489,31 @@ impl parachains_ump::Config for Runtime { type FirstMessageFactorPercent = FirstMessageFactorPercent; } +parameter_types! { + pub const BaseXcmWeight: frame_support::weights::Weight = 1_000; + pub const AnyNetwork: xcm::latest::NetworkId = xcm::latest::NetworkId::Any; +} + +pub type LocalOriginToLocation = xcm_builder::SignedToAccountId32; + +impl pallet_xcm::Config for Runtime { + // The config types here are entirely configurable, since the only one that is sorely needed + // is `XcmExecutor`, which will be used in unit tests located in xcm-executor. + type Event = Event; + type ExecuteXcmOrigin = xcm_builder::EnsureXcmOrigin; + type LocationInverter = xcm_config::InvertNothing; + type SendXcmOrigin = xcm_builder::EnsureXcmOrigin; + type Weigher = xcm_builder::FixedWeightBounds; + type XcmRouter = xcm_config::DoNothingRouter; + type XcmExecuteFilter = + frame_support::traits::All<(xcm::latest::MultiLocation, xcm::latest::Xcm)>; + type XcmExecutor = xcm_executor::XcmExecutor; + type XcmTeleportFilter = + frame_support::traits::All<(xcm::latest::MultiLocation, Vec)>; + type XcmReserveTransferFilter = + frame_support::traits::All<(xcm::latest::MultiLocation, Vec)>; +} + impl parachains_hrmp::Config for Runtime { type Event = Event; type Origin = Origin; @@ -543,6 +569,7 @@ construct_runtime! { Hrmp: parachains_hrmp::{Pallet, Call, Storage, Event}, Ump: parachains_ump::{Pallet, Call, Storage, Event}, Dmp: parachains_dmp::{Pallet, Call, Storage}, + Xcm: pallet_xcm::{Pallet, Call, Event, Origin}, ParasDisputes: parachains_disputes::{Pallet, Storage, Event}, Sudo: pallet_sudo::{Pallet, Call, Storage, Config, Event}, diff --git a/polkadot/runtime/test-runtime/src/xcm_config.rs b/polkadot/runtime/test-runtime/src/xcm_config.rs new file mode 100644 index 0000000000..deaaef47ed --- /dev/null +++ b/polkadot/runtime/test-runtime/src/xcm_config.rs @@ -0,0 +1,91 @@ +// Copyright 2021 Parity Technologies (UK) Ltd. +// This file is part of Polkadot. + +// Polkadot is free software: you can redistribute it and/or modify +// it under the terms of the GNU General Public License as published by +// the Free Software Foundation, either version 3 of the License, or +// (at your option) any later version. + +// Polkadot is distributed in the hope that it will be useful, +// but WITHOUT ANY WARRANTY; without even the implied warranty of +// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +// GNU General Public License for more details. + +// You should have received a copy of the GNU General Public License +// along with Polkadot. If not, see . + +use frame_support::{parameter_types, traits::All, weights::Weight}; +use xcm::latest::{ + Error as XcmError, Junction::*, MultiAsset, MultiLocation, MultiLocation::*, NetworkId, + Result as XcmResult, SendXcm, Xcm, +}; +use xcm_builder::{AllowUnpaidExecutionFrom, FixedWeightBounds, SignedToAccountId32}; +use xcm_executor::{ + traits::{InvertLocation, TransactAsset, WeightTrader}, + Assets, +}; + +parameter_types! { + pub const OurNetwork: NetworkId = NetworkId::Polkadot; +} + +/// Type to convert an `Origin` type value into a `MultiLocation` value which represents an interior location +/// of this chain. +pub type LocalOriginToLocation = ( + // And a usual Signed origin to be used in XCM as a corresponding AccountId32 + SignedToAccountId32, +); + +pub struct DoNothingRouter; +impl SendXcm for DoNothingRouter { + fn send_xcm(_dest: MultiLocation, _msg: Xcm<()>) -> XcmResult { + Ok(()) + } +} + +pub type Barrier = AllowUnpaidExecutionFrom>; + +pub struct DummyAssetTransactor; +impl TransactAsset for DummyAssetTransactor { + fn deposit_asset(_what: &MultiAsset, _who: &MultiLocation) -> XcmResult { + Ok(()) + } + + fn withdraw_asset(_what: &MultiAsset, _who: &MultiLocation) -> Result { + let asset: MultiAsset = (X1(Parent), 100_000).into(); + Ok(asset.into()) + } +} + +pub struct DummyWeightTrader; +impl WeightTrader for DummyWeightTrader { + fn new() -> Self { + DummyWeightTrader + } + + fn buy_weight(&mut self, _weight: Weight, _payment: Assets) -> Result { + Ok(Assets::default()) + } +} + +pub struct InvertNothing; +impl InvertLocation for InvertNothing { + fn invert_location(_: &MultiLocation) -> MultiLocation { + MultiLocation::Here + } +} + +pub struct XcmConfig; +impl xcm_executor::Config for XcmConfig { + type Call = super::Call; + type XcmSender = DoNothingRouter; + type AssetTransactor = DummyAssetTransactor; + type OriginConverter = pallet_xcm::XcmPassthrough; + type IsReserve = (); + type IsTeleporter = (); + type LocationInverter = InvertNothing; + type Barrier = Barrier; + type Weigher = FixedWeightBounds; + type Trader = DummyWeightTrader; + type ResponseHandler = (); +} diff --git a/polkadot/xcm/src/double_encoded.rs b/polkadot/xcm/src/double_encoded.rs index 70592580d3..ae21336d13 100644 --- a/polkadot/xcm/src/double_encoded.rs +++ b/polkadot/xcm/src/double_encoded.rs @@ -14,12 +14,10 @@ // You should have received a copy of the GNU General Public License // along with Polkadot. If not, see . +use crate::MAX_XCM_DECODE_DEPTH; use alloc::vec::Vec; use parity_scale_codec::{Decode, DecodeLimit, Encode}; -/// Maximum nesting level for XCM decoding. -pub const MAX_XCM_DECODE_DEPTH: u32 = 8; - /// Wrapper around the encoded and decoded versions of a value. /// Caches the decoded value once computed. #[derive(Encode, Decode)] diff --git a/polkadot/xcm/src/lib.rs b/polkadot/xcm/src/lib.rs index 05443fc76b..fa9e0c3b4b 100644 --- a/polkadot/xcm/src/lib.rs +++ b/polkadot/xcm/src/lib.rs @@ -40,6 +40,9 @@ pub mod latest { mod double_encoded; pub use double_encoded::DoubleEncoded; +/// Maximum nesting level for XCM decoding. +pub const MAX_XCM_DECODE_DEPTH: u32 = 8; + #[derive(Clone, Eq, PartialEq, Debug)] pub enum Unsupported {} impl Encode for Unsupported {} diff --git a/polkadot/xcm/src/v0/traits.rs b/polkadot/xcm/src/v0/traits.rs index e1673fd5d9..58f395716f 100644 --- a/polkadot/xcm/src/v0/traits.rs +++ b/polkadot/xcm/src/v0/traits.rs @@ -87,6 +87,8 @@ pub enum Error { TooExpensive, /// The given asset is not handled. AssetNotFound, + /// `execute_xcm` has been called too many times recursively. + RecursionLimitReached, } impl From<()> for Error { diff --git a/polkadot/xcm/src/v1/traits.rs b/polkadot/xcm/src/v1/traits.rs index 94fd4449f2..a029d2cc64 100644 --- a/polkadot/xcm/src/v1/traits.rs +++ b/polkadot/xcm/src/v1/traits.rs @@ -89,6 +89,8 @@ pub enum Error { AssetNotFound, /// The given message cannot be translated into a format that the destination can be expected to interpret. DestinationUnsupported, + /// `execute_xcm` has been called too many times recursively. + RecursionLimitReached, } impl From<()> for Error { diff --git a/polkadot/xcm/xcm-executor/integration-tests/Cargo.toml b/polkadot/xcm/xcm-executor/integration-tests/Cargo.toml new file mode 100644 index 0000000000..d803373366 --- /dev/null +++ b/polkadot/xcm/xcm-executor/integration-tests/Cargo.toml @@ -0,0 +1,30 @@ +[package] +authors = ["Parity Technologies "] +edition = "2018" +name = "xcm-executor-integration-tests" +description = "Integration tests for the XCM Executor" +version = "0.9.9" + +[dependencies] +frame-support = { git = "https://github.com/paritytech/substrate", branch = "master", default-features = false } +frame-system = { git = "https://github.com/paritytech/substrate", branch = "master" } +futures = "0.3.15" +pallet-xcm = { path = "../../pallet-xcm" } +polkadot-test-client = { path = "../../../node/test/client" } +polkadot-test-runtime = { path = "../../../runtime/test-runtime" } +polkadot-test-service = { path = "../../../node/test/service" } +sp-consensus = { git = "https://github.com/paritytech/substrate", branch = "master" } +sp-keyring = { git = "https://github.com/paritytech/substrate", branch = "master" } +sp-runtime = { git = "https://github.com/paritytech/substrate", branch = "master", default-features = false } +sp-state-machine = { git = "https://github.com/paritytech/substrate", branch = "master" } +xcm = { path = "../..", default-features = false } +xcm-executor = { path = ".." } +sp-tracing = { git = "https://github.com/paritytech/substrate", branch = "master" } + +[features] +default = ["std"] +std = [ + "xcm/std", + "sp-runtime/std", + "frame-support/std", +] diff --git a/polkadot/xcm/xcm-executor/integration-tests/src/lib.rs b/polkadot/xcm/xcm-executor/integration-tests/src/lib.rs new file mode 100644 index 0000000000..209b44b898 --- /dev/null +++ b/polkadot/xcm/xcm-executor/integration-tests/src/lib.rs @@ -0,0 +1,141 @@ +// Copyright 2021 Parity Technologies (UK) Ltd. +// This file is part of Polkadot. + +// Polkadot is free software: you can redistribute it and/or modify +// it under the terms of the GNU General Public License as published by +// the Free Software Foundation, either version 3 of the License, or +// (at your option) any later version. + +// Polkadot is distributed in the hope that it will be useful, +// but WITHOUT ANY WARRANTY; without even the implied warranty of +// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +// GNU General Public License for more details. + +// You should have received a copy of the GNU General Public License +// along with Polkadot. If not, see . + +#![cfg_attr(not(feature = "std"), no_std)] +#![cfg(test)] + +use polkadot_test_client::{ + BlockBuilderExt, ClientBlockImportExt, DefaultTestClientBuilderExt, ExecutionStrategy, + InitPolkadotBlockBuilder, TestClientBuilder, TestClientBuilderExt, +}; +use polkadot_test_service::construct_extrinsic; +use sp_runtime::{generic::BlockId, traits::Block}; +use sp_state_machine::InspectState; +use xcm::latest::{Error as XcmError, Junction::*, MultiLocation::*, Order, Outcome, Xcm::*}; +use xcm_executor::MAX_RECURSION_LIMIT; + +// This is the inflection point where the test should either fail or pass. +const MAX_RECURSION_CHECK: u32 = MAX_RECURSION_LIMIT / 2; + +#[test] +fn execute_within_recursion_limit() { + sp_tracing::try_init_simple(); + let mut client = TestClientBuilder::new() + .set_execution_strategy(ExecutionStrategy::AlwaysWasm) + .build(); + + let mut msg = WithdrawAsset { assets: (X1(Parent), 100).into(), effects: vec![] }; + for _ in 0..MAX_RECURSION_CHECK { + msg = WithdrawAsset { + assets: (X1(Parent), 100).into(), + effects: vec![Order::BuyExecution { + fees: (X1(Parent), 1).into(), + weight: 0, + debt: 0, + halt_on_error: true, + orders: vec![], + // nest `msg` into itself on each iteration. + instructions: vec![msg], + }], + }; + } + + let mut block_builder = client.init_polkadot_block_builder(); + + let execute = construct_extrinsic( + &client, + polkadot_test_runtime::Call::Xcm(pallet_xcm::Call::execute( + Box::new(msg.clone()), + 1_000_000_000, + )), + sp_keyring::Sr25519Keyring::Alice, + ); + + block_builder.push_polkadot_extrinsic(execute).expect("pushes extrinsic"); + + let block = block_builder.build().expect("Finalizes the block").block; + let block_hash = block.hash(); + + futures::executor::block_on(client.import(sp_consensus::BlockOrigin::Own, block)) + .expect("imports the block"); + + client + .state_at(&BlockId::Hash(block_hash)) + .expect("state should exist") + .inspect_state(|| { + assert!(polkadot_test_runtime::System::events().iter().any(|r| matches!( + r.event, + polkadot_test_runtime::Event::Xcm(pallet_xcm::Event::Attempted(Outcome::Complete( + _ + )),), + ))); + }); +} + +#[test] +fn exceed_recursion_limit() { + sp_tracing::try_init_simple(); + let mut client = TestClientBuilder::new() + .set_execution_strategy(ExecutionStrategy::AlwaysWasm) + .build(); + + let mut msg = WithdrawAsset { assets: (X1(Parent), 100).into(), effects: vec![] }; + for _ in 0..(MAX_RECURSION_CHECK + 1) { + msg = WithdrawAsset { + assets: (X1(Parent), 100).into(), + effects: vec![Order::BuyExecution { + fees: (X1(Parent), 1).into(), + weight: 0, + debt: 0, + halt_on_error: true, + orders: vec![], + // nest `msg` into itself on each iteration. + instructions: vec![msg], + }], + }; + } + + let mut block_builder = client.init_polkadot_block_builder(); + + let execute = construct_extrinsic( + &client, + polkadot_test_runtime::Call::Xcm(pallet_xcm::Call::execute( + Box::new(msg.clone()), + 1_000_000_000, + )), + sp_keyring::Sr25519Keyring::Alice, + ); + + block_builder.push_polkadot_extrinsic(execute).expect("pushes extrinsic"); + + let block = block_builder.build().expect("Finalizes the block").block; + let block_hash = block.hash(); + + futures::executor::block_on(client.import(sp_consensus::BlockOrigin::Own, block)) + .expect("imports the block"); + + client + .state_at(&BlockId::Hash(block_hash)) + .expect("state should exist") + .inspect_state(|| { + assert!(polkadot_test_runtime::System::events().iter().any(|r| matches!( + r.event, + polkadot_test_runtime::Event::Xcm(pallet_xcm::Event::Attempted( + Outcome::Incomplete(_, XcmError::RecursionLimitReached), + )), + ))); + }); +} diff --git a/polkadot/xcm/xcm-executor/src/lib.rs b/polkadot/xcm/xcm-executor/src/lib.rs index 102f241fbc..1ba86fd334 100644 --- a/polkadot/xcm/xcm-executor/src/lib.rs +++ b/polkadot/xcm/xcm-executor/src/lib.rs @@ -41,6 +41,9 @@ pub use config::Config; /// The XCM executor. pub struct XcmExecutor(PhantomData); +/// The maximum recursion limit for `execute_xcm` and `execute_effects`. +pub const MAX_RECURSION_LIMIT: u32 = 8; + impl ExecuteXcm for XcmExecutor { fn execute_xcm_in_credit( origin: MultiLocation, @@ -56,7 +59,6 @@ impl ExecuteXcm for XcmExecutor { weight_limit, weight_credit, ); - // TODO: #2841 #HARDENXCM We should identify recursive bombs here and bail. let mut message = Xcm::::from(message); let shallow_weight = match Config::Weigher::shallow(&mut message) { Ok(x) => x, @@ -81,6 +83,7 @@ impl ExecuteXcm for XcmExecutor { &mut weight_credit, Some(shallow_weight), &mut trader, + 0, ); drop(trader); log::trace!(target: "xcm::execute_xcm", "result: {:?}", &result); @@ -110,16 +113,23 @@ impl XcmExecutor { weight_credit: &mut Weight, maybe_shallow_weight: Option, trader: &mut Config::Trader, + num_recursions: u32, ) -> Result { log::trace!( target: "xcm::do_execute_xcm", - "origin: {:?}, top_level: {:?}, message: {:?}, weight_credit: {:?}, maybe_shallow_weight: {:?}", + "origin: {:?}, top_level: {:?}, message: {:?}, weight_credit: {:?}, maybe_shallow_weight: {:?}, recursion: {:?}", origin, top_level, message, weight_credit, maybe_shallow_weight, + num_recursions, ); + + if num_recursions > MAX_RECURSION_LIMIT { + return Err(XcmError::RecursionLimitReached) + } + // This is the weight of everything that cannot be paid for. This basically means all computation // except any XCM which is behind an Order::BuyExecution. let shallow_weight = maybe_shallow_weight @@ -237,8 +247,15 @@ impl XcmExecutor { ensure!(who.is_interior(), XcmError::EscalationOfPrivilege); let mut origin = origin; origin.append_with(who).map_err(|_| XcmError::MultiLocationFull)?; - let surplus = - Self::do_execute_xcm(origin, top_level, *message, weight_credit, None, trader)?; + let surplus = Self::do_execute_xcm( + origin, + top_level, + *message, + weight_credit, + None, + trader, + num_recursions + 1, + )?; total_surplus = total_surplus.saturating_add(surplus); None }, @@ -247,7 +264,13 @@ impl XcmExecutor { if let Some((mut holding, effects)) = maybe_holding_effects { for effect in effects.into_iter() { - total_surplus += Self::execute_orders(&origin, &mut holding, effect, trader)?; + total_surplus += Self::execute_orders( + &origin, + &mut holding, + effect, + trader, + num_recursions + 1, + )?; } } @@ -259,14 +282,21 @@ impl XcmExecutor { holding: &mut Assets, order: Order, trader: &mut Config::Trader, + num_recursions: u32, ) -> Result { log::trace!( target: "xcm::execute_orders", - "origin: {:?}, holding: {:?}, effect: {:?}", + "origin: {:?}, holding: {:?}, order: {:?}, recursion: {:?}", origin, holding, order, + num_recursions, ); + + if num_recursions > MAX_RECURSION_LIMIT { + return Err(XcmError::RecursionLimitReached) + } + let mut total_surplus = 0; match order { Order::DepositAsset { assets, max_assets, beneficiary } => { @@ -314,7 +344,7 @@ impl XcmExecutor { let mut remaining_weight = weight; for order in orders.into_iter() { - match Self::execute_orders(origin, holding, order, trader) { + match Self::execute_orders(origin, holding, order, trader, num_recursions + 1) { Err(e) if halt_on_error => return Err(e), Err(_) => {}, Ok(surplus) => total_surplus += surplus, @@ -328,6 +358,7 @@ impl XcmExecutor { &mut remaining_weight, None, trader, + num_recursions + 1, ) { Err(e) if halt_on_error => return Err(e), Err(_) => {}, diff --git a/polkadot/xcm/xcm-simulator/example/Cargo.toml b/polkadot/xcm/xcm-simulator/example/Cargo.toml index 77b8b8e369..d7e8710e26 100644 --- a/polkadot/xcm/xcm-simulator/example/Cargo.toml +++ b/polkadot/xcm/xcm-simulator/example/Cargo.toml @@ -24,4 +24,4 @@ xcm-builder = { path = "../../xcm-builder" } pallet-xcm = { path = "../../pallet-xcm" } polkadot-core-primitives = { path = "../../../core-primitives"} polkadot-runtime-parachains = { path = "../../../runtime/parachains" } -polkadot-parachain = { path = "../../../parachain" } \ No newline at end of file +polkadot-parachain = { path = "../../../parachain" }