xcm-barriers: paid execution barrier supports more origin altering in…

…structions

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);