XCM paid execution barrier supports more origin altering instructions (…

…#5917)

The AllowTopLevelPaidExecutionFrom allows ClearOrigin instructions
before the expected BuyExecution instruction, it also allows messages
without any origin altering instructions.

This commit enhances the barrier to also support messages that use
AliasOrigin, or DescendOrigin. This is sometimes desired in asset
transfer XCM programs that need to run the inbound assets instructions
using the origin chain root origin, but then want to drop privileges for
the rest of the program. Currently these programs drop privileges by
clearing the origin completely, but that also unnecessarily limits the
range of actions available to the rest of the program. Using
DescendOrigin or AliasOrigin allows the sending chain to instruct the
receiving chain what the deprivileged real origin is.

See polkadot-fellows/RFCs#109 and
polkadot-fellows/RFCs#122 for more details on
how DescendOrigin and AliasOrigin could be used instead of ClearOrigin.

---------

Signed-off-by: Adrian Catangiu <adrian@parity.io>

polkadot/xcm/xcm-builder/src/barriers.rs

@@ -57,8 +57,9 @@ const MAX_ASSETS_FOR_BUY_EXECUTION: usize = 2;
 /// Allows execution from `origin` if it is contained in `T` (i.e. `T::Contains(origin)`) taking
 /// payments into account.
 ///
-/// Only allows for `TeleportAsset`, `WithdrawAsset`, `ClaimAsset` and `ReserveAssetDeposit` XCMs
-/// because they are the only ones that place assets in the Holding Register to pay for execution.
+/// Only allows for `WithdrawAsset`, `ReceiveTeleportedAsset`, `ReserveAssetDeposited` and
+/// `ClaimAsset` XCMs because they are the only ones that place assets in the Holding Register to
+/// pay for execution.
 pub struct AllowTopLevelPaidExecutionFrom<T>(PhantomData<T>);
 impl<T: Contains<Location>> ShouldExecute for AllowTopLevelPaidExecutionFrom<T> {
 	fn should_execute<RuntimeCall>(
@@ -81,9 +82,9 @@ impl<T: Contains<Location>> ShouldExecute for AllowTopLevelPaidExecutionFrom<T>
 		instructions[..end]
 			.matcher()
 			.match_next_inst(|inst| match inst {
+				WithdrawAsset(ref assets) |
 				ReceiveTeleportedAsset(ref assets) |
 				ReserveAssetDeposited(ref assets) |
-				WithdrawAsset(ref assets) |
 				ClaimAsset { ref assets, .. } =>
 					if assets.len() <= MAX_ASSETS_FOR_BUY_EXECUTION {
 						Ok(())
@@ -92,7 +93,10 @@ impl<T: Contains<Location>> ShouldExecute for AllowTopLevelPaidExecutionFrom<T>
 					},
 				_ => Err(ProcessMessageError::BadFormat),
 			})?
-			.skip_inst_while(|inst| matches!(inst, ClearOrigin))?
+			.skip_inst_while(|inst| {
+				matches!(inst, ClearOrigin | AliasOrigin(..)) ||
+					matches!(inst, DescendOrigin(child) if child != &Here)
+			})?
 			.match_next_inst(|inst| match inst {
 				BuyExecution { weight_limit: Limited(ref mut weight), .. }
 					if weight.all_gte(max_weight) =>

polkadot/xcm/xcm-builder/src/tests/barriers.rs

@@ -283,6 +283,56 @@ fn allow_paid_should_work() {
 	assert_eq!(r, Ok(()))
 }
 
+#[test]
+fn allow_paid_should_deprivilege_origin() {
+	AllowPaidFrom::set(vec![Parent.into()]);
+	let fees = (Parent, 1).into();
+
+	let mut paying_message_clears_origin = Xcm::<()>(vec![
+		ReserveAssetDeposited((Parent, 100).into()),
+		ClearOrigin,
+		BuyExecution { fees, weight_limit: Limited(Weight::from_parts(30, 30)) },
+		DepositAsset { assets: AllCounted(1).into(), beneficiary: Here.into() },
+	]);
+	let r = AllowTopLevelPaidExecutionFrom::<IsInVec<AllowPaidFrom>>::should_execute(
+		&Parent.into(),
+		paying_message_clears_origin.inner_mut(),
+		Weight::from_parts(30, 30),
+		&mut props(Weight::zero()),
+	);
+	assert_eq!(r, Ok(()));
+
+	let mut paying_message_aliases_origin = paying_message_clears_origin.clone();
+	paying_message_aliases_origin.0[1] = AliasOrigin(Parachain(1).into());
+	let r = AllowTopLevelPaidExecutionFrom::<IsInVec<AllowPaidFrom>>::should_execute(
+		&Parent.into(),
+		paying_message_aliases_origin.inner_mut(),
+		Weight::from_parts(30, 30),
+		&mut props(Weight::zero()),
+	);
+	assert_eq!(r, Ok(()));
+
+	let mut paying_message_descends_origin = paying_message_clears_origin.clone();
+	paying_message_descends_origin.0[1] = DescendOrigin(Parachain(1).into());
+	let r = AllowTopLevelPaidExecutionFrom::<IsInVec<AllowPaidFrom>>::should_execute(
+		&Parent.into(),
+		paying_message_descends_origin.inner_mut(),
+		Weight::from_parts(30, 30),
+		&mut props(Weight::zero()),
+	);
+	assert_eq!(r, Ok(()));
+
+	let mut paying_message_fake_descends_origin = paying_message_clears_origin.clone();
+	paying_message_fake_descends_origin.0[1] = DescendOrigin(Here.into());
+	let r = AllowTopLevelPaidExecutionFrom::<IsInVec<AllowPaidFrom>>::should_execute(
+		&Parent.into(),
+		paying_message_fake_descends_origin.inner_mut(),
+		Weight::from_parts(30, 30),
+		&mut props(Weight::zero()),
+	);
+	assert_eq!(r, Err(ProcessMessageError::Overweight(Weight::from_parts(30, 30))));
+}
+
 #[test]
 fn suspension_should_work() {
 	TestSuspender::set_suspended(true);

prdoc/pr_5917.prdoc

@@ -0,0 +1,14 @@
+# Schema: Polkadot SDK PRDoc Schema (prdoc) v1.0.0
+# See doc at https://raw.githubusercontent.com/paritytech/polkadot-sdk/master/prdoc/schema_user.json
+
+title: "XCM paid execution barrier supports more origin altering instructions"
+
+doc:
+  - audience: Runtime Dev
+    description: |
+      Updates the `AllowTopLevelPaidExecutionFrom` barrier to also support messages that
+      use `DescendOrigin` or `AliasOrigin` for altering the computed origin during execution.
+
+crates:
+  - name: staging-xcm-builder
+    bump: patch