Create thread_local in XCM executor to limit recursion depth

runtime/test-runtime/src/lib.rs

@@ -131,7 +131,7 @@ parameter_types! {
 }
 
 impl frame_system::Config for Runtime {
-	type BaseCallFilter = frame_support::traits::Everything;
+	type BaseCallFilter = Everything;
 	type BlockWeights = BlockWeights;
 	type BlockLength = BlockLength;
 	type DbWeight = ();
@@ -538,41 +538,6 @@ impl parachains_ump::Config for Runtime {
 	type WeightInfo = parachains_ump::TestWeightInfo;
 }
 
-parameter_types! {
-	pub const BaseXcmWeight: xcm::latest::Weight = Weight::from_parts(1_000, 1_000);
-	pub const AnyNetwork: Option<xcm::latest::NetworkId> = None;
-	pub const MaxInstructions: u32 = 100;
-	pub const UniversalLocation: xcm::latest::InteriorMultiLocation = xcm::latest::Junctions::Here;
-}
-
-pub type LocalOriginToLocation =
-	xcm_builder::SignedToAccountId32<RuntimeOrigin, AccountId, AnyNetwork>;
-
-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 RuntimeEvent = RuntimeEvent;
-	type ExecuteXcmOrigin = xcm_builder::EnsureXcmOrigin<RuntimeOrigin, LocalOriginToLocation>;
-	type UniversalLocation = UniversalLocation;
-	type SendXcmOrigin = xcm_builder::EnsureXcmOrigin<RuntimeOrigin, LocalOriginToLocation>;
-	type Weigher = xcm_builder::FixedWeightBounds<BaseXcmWeight, RuntimeCall, MaxInstructions>;
-	type XcmRouter = xcm_config::DoNothingRouter;
-	type XcmExecuteFilter = Everything;
-	type XcmExecutor = xcm_executor::XcmExecutor<xcm_config::XcmConfig>;
-	type XcmTeleportFilter = Everything;
-	type XcmReserveTransferFilter = Everything;
-	type RuntimeOrigin = RuntimeOrigin;
-	type RuntimeCall = RuntimeCall;
-	const VERSION_DISCOVERY_QUEUE_SIZE: u32 = 100;
-	type AdvertisedXcmVersion = pallet_xcm::CurrentXcmVersion;
-	type Currency = Balances;
-	type CurrencyMatcher = ();
-	type TrustedLockers = ();
-	type SovereignAccountOf = ();
-	type MaxLockers = frame_support::traits::ConstU32<8>;
-	type WeightInfo = pallet_xcm::TestWeightInfo;
-}
-
 impl parachains_hrmp::Config for Runtime {
 	type RuntimeOrigin = RuntimeOrigin;
 	type RuntimeEvent = RuntimeEvent;

runtime/test-runtime/src/xcm_config.rs

@@ -20,14 +20,18 @@ use frame_support::{
 	weights::Weight,
 };
 use xcm::latest::prelude::*;
-use xcm_builder::{AllowUnpaidExecutionFrom, FixedWeightBounds, SignedToAccountId32};
+use xcm_builder::{
+	AllowUnpaidExecutionFrom, EnsureXcmOrigin, FixedWeightBounds, SignedAccountId32AsNative,
+	SignedToAccountId32,
+};
 use xcm_executor::{
 	traits::{TransactAsset, WeightTrader},
 	Assets,
 };
 
 parameter_types! {
-	pub const OurNetwork: NetworkId = NetworkId::Polkadot;
+	pub const BaseXcmWeight: xcm::latest::Weight = Weight::from_parts(1_000, 1_000);
+	pub const ThisNetwork: NetworkId = NetworkId::Polkadot;
 	pub const MaxInstructions: u32 = 100;
 	pub const MaxAssetsIntoHolding: u32 = 16;
 	pub const UniversalLocation: xcm::latest::InteriorMultiLocation = xcm::latest::Junctions::Here;
@@ -37,7 +41,7 @@ parameter_types! {
 /// of this chain.
 pub type LocalOriginToLocation = (
 	// And a usual Signed origin to be used in XCM as a corresponding AccountId32
-	SignedToAccountId32<crate::RuntimeOrigin, crate::AccountId, OurNetwork>,
+	SignedToAccountId32<crate::RuntimeOrigin, crate::AccountId, ThisNetwork>,
 );
 
 pub struct DoNothingRouter;
@@ -80,17 +84,22 @@ impl WeightTrader for DummyWeightTrader {
 	}
 }
 
+type OriginConverter = (
+	pallet_xcm::XcmPassthrough<super::RuntimeOrigin>,
+	SignedAccountId32AsNative<ThisNetwork, super::RuntimeOrigin>,
+);
+
 pub struct XcmConfig;
 impl xcm_executor::Config for XcmConfig {
 	type RuntimeCall = super::RuntimeCall;
 	type XcmSender = DoNothingRouter;
 	type AssetTransactor = DummyAssetTransactor;
-	type OriginConverter = pallet_xcm::XcmPassthrough<super::RuntimeOrigin>;
+	type OriginConverter = OriginConverter;
 	type IsReserve = ();
 	type IsTeleporter = ();
 	type UniversalLocation = UniversalLocation;
 	type Barrier = Barrier;
-	type Weigher = FixedWeightBounds<super::BaseXcmWeight, super::RuntimeCall, MaxInstructions>;
+	type Weigher = FixedWeightBounds<BaseXcmWeight, super::RuntimeCall, MaxInstructions>;
 	type Trader = DummyWeightTrader;
 	type ResponseHandler = super::Xcm;
 	type AssetTrap = super::Xcm;
@@ -105,3 +114,28 @@ impl xcm_executor::Config for XcmConfig {
 	type UniversalAliases = Nothing;
 	type CallDispatcher = super::RuntimeCall;
 }
+
+impl pallet_xcm::Config for crate::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 RuntimeEvent = crate::RuntimeEvent;
+	type ExecuteXcmOrigin = EnsureXcmOrigin<crate::RuntimeOrigin, LocalOriginToLocation>;
+	type UniversalLocation = UniversalLocation;
+	type SendXcmOrigin = EnsureXcmOrigin<crate::RuntimeOrigin, LocalOriginToLocation>;
+	type Weigher = FixedWeightBounds<BaseXcmWeight, crate::RuntimeCall, MaxInstructions>;
+	type XcmRouter = DoNothingRouter;
+	type XcmExecuteFilter = Everything;
+	type XcmExecutor = xcm_executor::XcmExecutor<XcmConfig>;
+	type XcmTeleportFilter = Everything;
+	type XcmReserveTransferFilter = Everything;
+	type RuntimeOrigin = crate::RuntimeOrigin;
+	type RuntimeCall = crate::RuntimeCall;
+	const VERSION_DISCOVERY_QUEUE_SIZE: u32 = 100;
+	type AdvertisedXcmVersion = pallet_xcm::CurrentXcmVersion;
+	type Currency = crate::Balances;
+	type CurrencyMatcher = ();
+	type TrustedLockers = ();
+	type SovereignAccountOf = ();
+	type MaxLockers = frame_support::traits::ConstU32<8>;
+	type WeightInfo = pallet_xcm::TestWeightInfo;
+}

xcm/src/v3/traits.rs

@@ -150,6 +150,8 @@ pub enum Error {
 	Barrier,
 	/// The weight of an XCM message is not computable ahead of execution.
 	WeightNotComputable,
+	/// Recursion stack limit reached
+	ExceedsStackLimit,
 }
 
 impl MaxEncodedLen for Error {

xcm/xcm-executor/Cargo.toml

@@ -7,6 +7,7 @@ version = "0.9.33"
 
 [dependencies]
 impl-trait-for-tuples = "0.2.2"
+environmental = { version = "1.1.4", default-features = false }
 parity-scale-codec = { version = "3.1.5", default-features = false, features = ["derive"] }
 xcm = { path = "..", default-features = false }
 sp-std = { git = "https://github.com/paritytech/substrate", branch = "master", default-features = false }

xcm/xcm-executor/integration-tests/src/lib.rs

@@ -17,16 +17,17 @@
 #![cfg_attr(not(feature = "std"), no_std)]
 #![cfg(test)]
 
-use frame_support::weights::Weight;
+use frame_support::{codec::Encode, dispatch::GetDispatchInfo, weights::Weight};
 use polkadot_test_client::{
 	BlockBuilderExt, ClientBlockImportExt, DefaultTestClientBuilderExt, ExecutionStrategy,
 	InitPolkadotBlockBuilder, TestClientBuilder, TestClientBuilderExt,
 };
-use polkadot_test_runtime::pallet_test_notifier;
+use polkadot_test_runtime::{pallet_test_notifier, xcm_config::XcmConfig};
 use polkadot_test_service::construct_extrinsic;
 use sp_runtime::traits::Block;
 use sp_state_machine::InspectState;
 use xcm::{latest::prelude::*, VersionedResponse, VersionedXcm};
+use xcm_executor::traits::WeightBounds;
 
 #[test]
 fn basic_buy_fees_message_executes() {
@@ -71,6 +72,59 @@ fn basic_buy_fees_message_executes() {
 	});
 }
 
+#[test]
+fn transact_recursion_limit_works() {
+	sp_tracing::try_init_simple();
+	let mut client = TestClientBuilder::new()
+		.set_execution_strategy(ExecutionStrategy::AlwaysWasm)
+		.build();
+
+	let mut msg = Xcm(vec![ClearOrigin]);
+	let max_weight = <XcmConfig as xcm_executor::Config>::Weigher::weight(&mut msg).unwrap();
+	let mut call = polkadot_test_runtime::RuntimeCall::Xcm(pallet_xcm::Call::execute {
+		message: Box::new(VersionedXcm::from(msg)),
+		max_weight,
+	});
+
+	for _ in 0..11 {
+		let mut msg = Xcm(vec![
+			WithdrawAsset((Parent, 1_000).into()),
+			BuyExecution { fees: (Parent, 1).into(), weight_limit: Unlimited },
+			Transact {
+				origin_kind: OriginKind::Native,
+				require_weight_at_most: call.get_dispatch_info().weight,
+				call: call.encode().into(),
+			},
+		]);
+		let max_weight = <XcmConfig as xcm_executor::Config>::Weigher::weight(&mut msg).unwrap();
+		call = polkadot_test_runtime::RuntimeCall::Xcm(pallet_xcm::Call::execute {
+			message: Box::new(VersionedXcm::from(msg)),
+			max_weight,
+		});
+	}
+
+	let mut block_builder = client.init_polkadot_block_builder();
+
+	let execute = construct_extrinsic(&client, call, sp_keyring::Sr25519Keyring::Alice, 0);
+
+	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(block_hash).expect("state should exist").inspect_state(|| {
+		assert!(polkadot_test_runtime::System::events().iter().any(|r| matches!(
+			r.event,
+			polkadot_test_runtime::RuntimeEvent::Xcm(pallet_xcm::Event::Attempted(
+				Outcome::Incomplete(_, XcmError::ExceedsStackLimit)
+			)),
+		)));
+	});
+}
+
 #[test]
 fn query_response_fires() {
 	use pallet_test_notifier::Event::*;

xcm/xcm-executor/src/lib.rs

@@ -22,6 +22,7 @@ use frame_support::{
 	traits::{Contains, ContainsPair, Get, PalletsInfoAccess},
 };
 use parity_scale_codec::{Decode, Encode};
+use sp_core::defer;
 use sp_io::hashing::blake2_128;
 use sp_std::{marker::PhantomData, prelude::*};
 use sp_weights::Weight;
@@ -44,6 +45,10 @@ pub struct FeesMode {
 	pub jit_withdraw: bool,
 }
 
+const RECURSION_LIMIT: u8 = 10;
+
+environmental::environmental!(recursion_count: u8);
+
 /// The XCM executor.
 pub struct XcmExecutor<Config: config::Config> {
 	holding: Assets,
@@ -510,38 +515,61 @@ impl<Config: config::Config> XcmExecutor<Config> {
 				Ok(())
 			},
 			Transact { origin_kind, require_weight_at_most, mut call } => {
-				// We assume that the Relay-chain is allowed to use transact on this parachain.
-				let origin = self.origin_ref().ok_or(XcmError::BadOrigin)?.clone();
-
-				// TODO: #2841 #TRANSACTFILTER allow the trait to issue filters for the relay-chain
-				let message_call = call.take_decoded().map_err(|_| XcmError::FailedToDecode)?;
-				let dispatch_origin = Config::OriginConverter::convert_origin(origin, origin_kind)
-					.map_err(|_| XcmError::BadOrigin)?;
-				let weight = message_call.get_dispatch_info().weight;
-				ensure!(weight.all_lte(require_weight_at_most), XcmError::MaxWeightInvalid);
-				let maybe_actual_weight =
-					match Config::CallDispatcher::dispatch(message_call, dispatch_origin) {
-						Ok(post_info) => {
-							self.transact_status = MaybeErrorCode::Success;
-							post_info.actual_weight
-						},
-						Err(error_and_info) => {
-							self.transact_status = error_and_info.error.encode().into();
-							error_and_info.post_info.actual_weight
-						},
-					};
-				let actual_weight = maybe_actual_weight.unwrap_or(weight);
-				let surplus = weight.saturating_sub(actual_weight);
-				// We assume that the `Config::Weigher` will counts the `require_weight_at_most`
-				// for the estimate of how much weight this instruction will take. Now that we know
-				// that it's less, we credit it.
-				//
-				// We make the adjustment for the total surplus, which is used eventually
-				// reported back to the caller and this ensures that they account for the total
-				// weight consumed correctly (potentially allowing them to do more operations in a
-				// block than they otherwise would).
-				self.total_surplus.saturating_accrue(surplus);
-				Ok(())
+				// Initialize the recursion count only the first time we hit this code in our
+				// potential recursive execution.
+				recursion_count::using_once(&mut 1, || {
+					recursion_count::with(|count| {
+						if *count > RECURSION_LIMIT {
+							return Err(XcmError::ExceedsStackLimit)
+						}
+						*count = count.saturating_add(1);
+						Ok(())
+					})
+					// This should always return `Some`, but let's play it safe.
+					.unwrap_or(Ok(()))?;
+
+					// Ensure that we always decrement the counter whenever we finish executing
+					// `Transact`
+					defer! {
+						recursion_count::with(|count| {
+							*count = count.saturating_sub(1);
+						});
+					}
+
+					// We assume that the Relay-chain is allowed to use transact on this parachain.
+					let origin = self.origin_ref().ok_or(XcmError::BadOrigin)?.clone();
+
+					// TODO: #2841 #TRANSACTFILTER allow the trait to issue filters for the relay-chain
+					let message_call = call.take_decoded().map_err(|_| XcmError::FailedToDecode)?;
+					let dispatch_origin =
+						Config::OriginConverter::convert_origin(origin, origin_kind)
+							.map_err(|_| XcmError::BadOrigin)?;
+					let weight = message_call.get_dispatch_info().weight;
+					ensure!(weight.all_lte(require_weight_at_most), XcmError::MaxWeightInvalid);
+					let maybe_actual_weight =
+						match Config::CallDispatcher::dispatch(message_call, dispatch_origin) {
+							Ok(post_info) => {
+								self.transact_status = MaybeErrorCode::Success;
+								post_info.actual_weight
+							},
+							Err(error_and_info) => {
+								self.transact_status = error_and_info.error.encode().into();
+								error_and_info.post_info.actual_weight
+							},
+						};
+					let actual_weight = maybe_actual_weight.unwrap_or(weight);
+					let surplus = weight.saturating_sub(actual_weight);
+					// We assume that the `Config::Weigher` will counts the `require_weight_at_most`
+					// for the estimate of how much weight this instruction will take. Now that we know
+					// that it's less, we credit it.
+					//
+					// We make the adjustment for the total surplus, which is used eventually
+					// reported back to the caller and this ensures that they account for the total
+					// weight consumed correctly (potentially allowing them to do more operations in a
+					// block than they otherwise would).
+					self.total_surplus.saturating_accrue(surplus);
+					Ok(())
+				})
 			},
 			QueryResponse { query_id, response, max_weight, querier } => {
 				let origin = self.origin_ref().ok_or(XcmError::BadOrigin)?;