Backport XCM fixes to 1.5.0 (#3174)

- Transactional processing
- Delivery fees taken from holding

---------

Signed-off-by: Adrian Catangiu <adrian@parity.io>
Signed-off-by: Oliver Tale-Yazdi <oliver.tale-yazdi@parity.io>
Co-authored-by: Adrian Catangiu <adrian@parity.io>
Co-authored-by: Just van Stam <vstam1@users.noreply.github.com>
Co-authored-by: Keith Yeung <kungfukeith11@gmail.com>
Co-authored-by: Alexander Samusev <41779041+alvicsam@users.noreply.github.com>
Co-authored-by: Dónal Murray <donal.murray@parity.io>
Co-authored-by: Bastian Köcher <info@kchr.de>
Co-authored-by: Oliver Tale-Yazdi <oliver.tale-yazdi@parity.io>
Co-authored-by: PG Herveou <pgherveou@gmail.com>

Author: 9 people

.gitlab-ci.yml

@@ -21,7 +21,7 @@ workflow:
     - if: $CI_COMMIT_BRANCH
 
 variables:
-  CI_IMAGE: !reference [.ci-unified, variables, CI_IMAGE]
+  CI_IMAGE: "docker.io/paritytech/ci-unified:bullseye-1.74.0-2023-11-01-v20231204"
   # BUILDAH_IMAGE is defined in group variables
   BUILDAH_COMMAND: "buildah --storage-driver overlay2"
   RELENG_SCRIPTS_BRANCH: "master"

Cargo.lock

@@ -30,7 +30,7 @@ use sp_runtime::{
 	BuildStorage,
 };
 use xcm::prelude::*;
-use xcm_builder::{CurrencyAdapter, FixedWeightBounds, IsConcrete, NativeAsset, ParentIsPreset};
+use xcm_builder::{CurrencyAdapter, FixedWeightBounds, FrameTransactionalProcessor, IsConcrete, NativeAsset, ParentIsPreset};
 use xcm_executor::traits::ConvertOrigin;
 
 type Block = frame_system::mocking::MockBlock<Test>;
@@ -172,6 +172,7 @@ impl xcm_executor::Config for XcmConfig {
 	type CallDispatcher = RuntimeCall;
 	type SafeCallFilter = Everything;
 	type Aliasers = Nothing;
+	type TransactionalProcessor = FrameTransactionalProcessor;
 }
 
 pub type XcmRouter = (

cumulus/pallets/xcmp-queue/src/mock.rs

@@ -15,7 +15,7 @@ use xcm::latest::prelude::*;
 use xcm_builder::{
 	AccountId32Aliases, AllowExplicitUnpaidExecutionFrom, AllowTopLevelPaidExecutionFrom,
 	CurrencyAdapter, DenyReserveTransferToRelayChain, DenyThenTry, EnsureXcmOrigin,
-	FixedWeightBounds, IsConcrete, NativeAsset, ParentIsPreset, RelayChainAsNative,
+	FixedWeightBounds, FrameTransactionalProcessor, IsConcrete, NativeAsset, ParentIsPreset, RelayChainAsNative,
 	SiblingParachainAsNative, SiblingParachainConvertsVia, SignedAccountId32AsNative,
 	SignedToAccountId32, SovereignSignedViaLocation, TakeWeightCredit, TrailingSetTopicAsId,
 	UsingComponents, WithComputedOrigin, WithUniqueTopic,
@@ -136,6 +136,7 @@ impl xcm_executor::Config for XcmConfig {
 	type CallDispatcher = RuntimeCall;
 	type SafeCallFilter = Everything;
 	type Aliasers = Nothing;
+	type TransactionalProcessor = FrameTransactionalProcessor;
 }
 
 /// No local origins on this chain are allowed to dispatch XCM sends/executions.

cumulus/parachain-template/runtime/src/xcm_config.rs

@@ -24,8 +24,11 @@ xcm = { package = "staging-xcm", path = "../../../../../../../polkadot/xcm", def
 pallet-xcm = { path = "../../../../../../../polkadot/xcm/pallet-xcm", default-features = false, version = "6.0.0" }
 xcm-executor = { package = "staging-xcm-executor", path = "../../../../../../../polkadot/xcm/xcm-executor", default-features = false, version = "6.0.0" }
 rococo-runtime = { version = "6.0.0", path = "../../../../../../../polkadot/runtime/rococo" }
+
+# Cumulus
 asset-test-utils = { version = "6.0.0", path = "../../../../../runtimes/assets/test-utils" }
-parachains-common = { version = "6.0.0", path = "../../../../../common" }
+parachains-common = { version = "6.0.0", path = "../../../../../../parachains/common" }
+cumulus-pallet-parachain-system = { path = "../../../../../../pallets/parachain-system", default-features = false }
 asset-hub-rococo-runtime = { version = "0.11.0", path = "../../../../../runtimes/assets/asset-hub-rococo" }
 emulated-integration-tests-common = { path = "../../../common", default-features = false, version = "2.0.0" }
 rococo-system-emulated-network = { version = "0.1.0", path = "../../../networks/rococo-system" }

cumulus/parachains/integration-tests/emulated/tests/assets/asset-hub-rococo/Cargo.toml

@@ -51,8 +51,8 @@ pub use rococo_system_emulated_network::{
 	AssetHubRococoParaSender as AssetHubRococoSender, BridgeHubRococoPara as BridgeHubRococo,
 	BridgeHubRococoParaReceiver as BridgeHubRococoReceiver, PenpalAPara as PenpalA,
 	PenpalAParaReceiver as PenpalAReceiver, PenpalAParaSender as PenpalASender,
-	RococoRelay as Rococo, RococoRelayReceiver as RococoReceiver,
-	RococoRelaySender as RococoSender,
+	PenpalBPara as PenpalB, PenpalBParaReceiver as PenpalBReceiver, RococoRelay as Rococo,
+	RococoRelayReceiver as RococoReceiver, RococoRelaySender as RococoSender,
 };
 
 pub const ASSET_ID: u32 = 1;
@@ -65,6 +65,7 @@ pub type RelayToParaTest = Test<Rococo, PenpalA>;
 pub type SystemParaToRelayTest = Test<AssetHubRococo, Rococo>;
 pub type SystemParaToParaTest = Test<AssetHubRococo, PenpalA>;
 pub type ParaToSystemParaTest = Test<PenpalA, AssetHubRococo>;
+pub type ParaToParaTest = Test<PenpalA, PenpalB, Rococo>;
 
 /// Returns a `TestArgs` instance to be used for the Relay Chain across integration tests
 pub fn relay_test_args(

cumulus/parachains/integration-tests/emulated/tests/assets/asset-hub-rococo/src/lib.rs

@@ -152,6 +152,69 @@ fn system_para_to_para_assets_receiver_assertions<Test>(_: Test) {
 	);
 }
 
+fn para_to_para_sender_assertions(t: ParaToParaTest) {
+	type RuntimeEvent = <PenpalA as Chain>::RuntimeEvent;
+	PenpalA::assert_xcm_pallet_attempted_complete(None);
+	assert_expected_events!(
+		PenpalA,
+		vec![
+			// Amount to reserve transfer is transferred to Parachain's Sovereign account
+			RuntimeEvent::Balances(
+				pallet_balances::Event::Withdraw { who, amount }
+			) => {
+				who: *who == t.sender.account_id,
+				amount: *amount == t.args.amount,
+			},
+			// XCM sent to relay reserve
+			RuntimeEvent::ParachainSystem(
+				cumulus_pallet_parachain_system::Event::UpwardMessageSent { .. }
+			) => {},
+		]
+	);
+}
+
+fn para_to_para_relay_hop_assertions(t: ParaToParaTest) {
+	type RuntimeEvent = <Rococo as Chain>::RuntimeEvent;
+	let sov_penpal_a_on_rococo =
+		Rococo::sovereign_account_id_of(Rococo::child_location_of(PenpalA::para_id()));
+	let sov_penpal_b_on_rococo =
+		Rococo::sovereign_account_id_of(Rococo::child_location_of(PenpalB::para_id()));
+	assert_expected_events!(
+		Rococo,
+		vec![
+			// Withdrawn from sender parachain SA
+			RuntimeEvent::Balances(
+				pallet_balances::Event::Withdraw { who, amount }
+			) => {
+				who: *who == sov_penpal_a_on_rococo,
+				amount: *amount == t.args.amount,
+			},
+			// Deposited to receiver parachain SA
+			RuntimeEvent::Balances(
+				pallet_balances::Event::Deposit { who, .. }
+			) => {
+				who: *who == sov_penpal_b_on_rococo,
+			},
+			RuntimeEvent::MessageQueue(
+				pallet_message_queue::Event::Processed { success: true, .. }
+			) => {},
+		]
+	);
+}
+
+fn para_to_para_receiver_assertions(_: ParaToParaTest) {
+	type RuntimeEvent = <PenpalB as Chain>::RuntimeEvent;
+	assert_expected_events!(
+		PenpalB,
+		vec![
+			RuntimeEvent::Balances(pallet_balances::Event::Deposit { .. }) => {},
+			RuntimeEvent::MessageQueue(
+				pallet_message_queue::Event::Processed { success: true, .. }
+			) => {},
+		]
+	);
+}
+
 fn relay_to_para_reserve_transfer_assets(t: RelayToParaTest) -> DispatchResult {
 	<Rococo as RococoPallet>::XcmPallet::limited_reserve_transfer_assets(
 		t.signed_origin,
@@ -185,6 +248,17 @@ fn para_to_system_para_reserve_transfer_assets(t: ParaToSystemParaTest) -> Dispa
 	)
 }
 
+fn para_to_para_limited_reserve_transfer_assets(t: ParaToParaTest) -> DispatchResult {
+	<PenpalA as PenpalAPallet>::PolkadotXcm::limited_reserve_transfer_assets(
+		t.signed_origin,
+		bx!(t.args.dest.into()),
+		bx!(t.args.beneficiary.into()),
+		bx!(t.args.assets.into()),
+		t.args.fee_asset_item,
+		t.args.weight_limit,
+	)
+}
+
 /// Reserve Transfers of native asset from Relay Chain to the System Parachain shouldn't work
 #[test]
 fn reserve_transfer_native_asset_from_relay_to_system_para_fails() {
@@ -493,3 +567,51 @@ fn reserve_transfer_assets_from_system_para_to_para() {
 	// Receiver's balance is increased by exact amount
 	assert_eq!(receiver_assets_after, receiver_assets_before + asset_amount_to_send);
 }
+
+/// Reserve Transfers of native asset from Parachain to Parachain (through Relay reserve) should
+/// work
+#[test]
+fn reserve_transfer_native_asset_from_para_to_para() {
+	// Init values for Penpal Parachain
+	let destination = PenpalA::sibling_location_of(PenpalB::para_id());
+	let beneficiary_id = PenpalBReceiver::get();
+	let amount_to_send: Balance = ASSET_HUB_ROCOCO_ED * 10000;
+	let assets = (Parent, amount_to_send).into();
+
+	let test_args = TestContext {
+		sender: PenpalASender::get(),
+		receiver: PenpalBReceiver::get(),
+		args: para_test_args(destination, beneficiary_id, amount_to_send, assets, None, 0),
+	};
+
+	let mut test = ParaToParaTest::new(test_args);
+
+	let sender_balance_before = test.sender.balance;
+	let receiver_balance_before = test.receiver.balance;
+
+	let sender_as_seen_by_relay = Rococo::child_location_of(PenpalA::para_id());
+	let sov_of_sender_on_relay = Rococo::sovereign_account_id_of(sender_as_seen_by_relay);
+
+	// fund the PenpalA's SA on Rococo with the native tokens held in reserve
+	Rococo::fund_accounts(vec![(sov_of_sender_on_relay.into(), amount_to_send * 2)]);
+
+	test.set_assertion::<PenpalA>(para_to_para_sender_assertions);
+	test.set_assertion::<Rococo>(para_to_para_relay_hop_assertions);
+	test.set_assertion::<PenpalB>(para_to_para_receiver_assertions);
+	test.set_dispatchable::<PenpalA>(para_to_para_limited_reserve_transfer_assets);
+	test.assert();
+
+	let sender_balance_after = test.sender.balance;
+	let receiver_balance_after = test.receiver.balance;
+
+	let delivery_fees = PenpalA::execute_with(|| {
+		xcm_helpers::transfer_assets_delivery_fees::<
+			<PenpalRococoXcmConfig as xcm_executor::Config>::XcmSender,
+		>(test.args.assets.clone(), 0, test.args.weight_limit, test.args.beneficiary, test.args.dest)
+	});
+
+	// Sender's balance is reduced
+	assert_eq!(sender_balance_before - amount_to_send - delivery_fees, sender_balance_after);
+	// Receiver's balance is increased
+	assert!(receiver_balance_after > receiver_balance_before);
+}

cumulus/parachains/integration-tests/emulated/tests/assets/asset-hub-rococo/src/tests/reserve_transfer.rs

@@ -51,7 +51,9 @@ pub use westend_system_emulated_network::{
 	westend_emulated_chain::{genesis::ED as WESTEND_ED, WestendRelayPallet as WestendPallet},
 	AssetHubWestendPara as AssetHubWestend, AssetHubWestendParaReceiver as AssetHubWestendReceiver,
 	AssetHubWestendParaSender as AssetHubWestendSender, BridgeHubWestendPara as BridgeHubWestend,
-	BridgeHubWestendParaReceiver as BridgeHubWestendReceiver, PenpalBPara as PenpalB,
+	BridgeHubWestendParaReceiver as BridgeHubWestendReceiver,
+	PenpalAPara as PenpalA,
+	PenpalAParaReceiver as PenpalAReceiver, PenpalBPara as PenpalB,
 	PenpalBParaReceiver as PenpalBReceiver, PenpalBParaSender as PenpalBSender,
 	WestendRelay as Westend, WestendRelayReceiver as WestendReceiver,
 	WestendRelaySender as WestendSender,
@@ -67,6 +69,7 @@ pub type RelayToParaTest = Test<Westend, PenpalB>;
 pub type SystemParaToRelayTest = Test<AssetHubWestend, Westend>;
 pub type SystemParaToParaTest = Test<AssetHubWestend, PenpalB>;
 pub type ParaToSystemParaTest = Test<PenpalB, AssetHubWestend>;
+pub type ParaToParaTest = Test<PenpalB, PenpalA, Westend>;
 
 /// Returns a `TestArgs` instance to be used for the Relay Chain across integration tests
 pub fn relay_test_args(

cumulus/parachains/integration-tests/emulated/tests/assets/asset-hub-westend/src/lib.rs

@@ -162,6 +162,69 @@ fn system_para_to_para_assets_receiver_assertions<Test>(_: Test) {
 	);
 }
 
+fn para_to_para_sender_assertions(t: ParaToParaTest) {
+	type RuntimeEvent = <PenpalB as Chain>::RuntimeEvent;
+	PenpalB::assert_xcm_pallet_attempted_complete(None);
+	assert_expected_events!(
+		PenpalB,
+		vec![
+			// Amount to reserve transfer is transferred to Parachain's Sovereign account
+			RuntimeEvent::Balances(
+				pallet_balances::Event::Withdraw { who, amount }
+			) => {
+				who: *who == t.sender.account_id,
+				amount: *amount == t.args.amount,
+			},
+			// XCM sent to relay reserve
+			RuntimeEvent::ParachainSystem(
+				cumulus_pallet_parachain_system::Event::UpwardMessageSent { .. }
+			) => {},
+		]
+	);
+}
+
+fn para_to_para_relay_hop_assertions(t: ParaToParaTest) {
+	type RuntimeEvent = <Westend as Chain>::RuntimeEvent;
+	let sov_penpal_b_on_westend =
+		Westend::sovereign_account_id_of(Westend::child_location_of(PenpalB::para_id()));
+	let sov_penpal_a_on_westend =
+		Westend::sovereign_account_id_of(Westend::child_location_of(PenpalA::para_id()));
+	assert_expected_events!(
+		Westend,
+		vec![
+			// Withdrawn from sender parachain SA
+			RuntimeEvent::Balances(
+				pallet_balances::Event::Withdraw { who, amount }
+			) => {
+				who: *who == sov_penpal_b_on_westend,
+				amount: *amount == t.args.amount,
+			},
+			// Deposited to receiver parachain SA
+			RuntimeEvent::Balances(
+				pallet_balances::Event::Deposit { who, .. }
+			) => {
+				who: *who == sov_penpal_a_on_westend,
+			},
+			RuntimeEvent::MessageQueue(
+				pallet_message_queue::Event::Processed { success: true, .. }
+			) => {},
+		]
+	);
+}
+
+fn para_to_para_receiver_assertions(_: ParaToParaTest) {
+	type RuntimeEvent = <PenpalA as Chain>::RuntimeEvent;
+	assert_expected_events!(
+		PenpalA,
+		vec![
+			RuntimeEvent::Balances(pallet_balances::Event::Deposit { .. }) => {},
+			RuntimeEvent::MessageQueue(
+				pallet_message_queue::Event::Processed { success: true, .. }
+			) => {},
+		]
+	);
+}
+
 fn relay_to_para_reserve_transfer_assets(t: RelayToParaTest) -> DispatchResult {
 	<Westend as WestendPallet>::XcmPallet::limited_reserve_transfer_assets(
 		t.signed_origin,
@@ -195,6 +258,17 @@ fn para_to_system_para_reserve_transfer_assets(t: ParaToSystemParaTest) -> Dispa
 	)
 }
 
+fn para_to_para_limited_reserve_transfer_assets(t: ParaToParaTest) -> DispatchResult {
+	<PenpalB as PenpalBPallet>::PolkadotXcm::limited_reserve_transfer_assets(
+		t.signed_origin,
+		bx!(t.args.dest.into()),
+		bx!(t.args.beneficiary.into()),
+		bx!(t.args.assets.into()),
+		t.args.fee_asset_item,
+		t.args.weight_limit,
+	)
+}
+
 /// Reserve Transfers of native asset from Relay Chain to the System Parachain shouldn't work
 #[test]
 fn reserve_transfer_native_asset_from_relay_to_system_para_fails() {
@@ -503,3 +577,51 @@ fn reserve_transfer_assets_from_system_para_to_para() {
 	// Receiver's balance is increased by exact amount
 	assert_eq!(receiver_assets_after, receiver_assets_before + asset_amount_to_send);
 }
+
+/// Reserve Transfers of native asset from Parachain to Parachain (through Relay reserve) should
+/// work
+#[test]
+fn reserve_transfer_native_asset_from_para_to_para() {
+	// Init values for Penpal Parachain
+	let destination = PenpalB::sibling_location_of(PenpalA::para_id());
+	let beneficiary_id = PenpalAReceiver::get();
+	let amount_to_send: Balance = ASSET_HUB_WESTEND_ED * 1000;
+	let assets = (Parent, amount_to_send).into();
+
+	let test_args = TestContext {
+		sender: PenpalBSender::get(),
+		receiver: PenpalAReceiver::get(),
+		args: para_test_args(destination, beneficiary_id, amount_to_send, assets, None, 0),
+	};
+
+	let mut test = ParaToParaTest::new(test_args);
+
+	let sender_balance_before = test.sender.balance;
+	let receiver_balance_before = test.receiver.balance;
+
+	let sender_as_seen_by_relay = Westend::child_location_of(PenpalB::para_id());
+	let sov_of_sender_on_relay = Westend::sovereign_account_id_of(sender_as_seen_by_relay);
+
+	// fund the PenpalB's SA on Westend with the native tokens held in reserve
+	Westend::fund_accounts(vec![(sov_of_sender_on_relay.into(), amount_to_send * 2)]);
+
+	test.set_assertion::<PenpalB>(para_to_para_sender_assertions);
+	test.set_assertion::<Westend>(para_to_para_relay_hop_assertions);
+	test.set_assertion::<PenpalA>(para_to_para_receiver_assertions);
+	test.set_dispatchable::<PenpalB>(para_to_para_limited_reserve_transfer_assets);
+	test.assert();
+
+	let sender_balance_after = test.sender.balance;
+	let receiver_balance_after = test.receiver.balance;
+
+	let delivery_fees = PenpalB::execute_with(|| {
+		xcm_helpers::transfer_assets_delivery_fees::<
+			<PenpalWestendXcmConfig as xcm_executor::Config>::XcmSender,
+		>(test.args.assets.clone(), 0, test.args.weight_limit, test.args.beneficiary, test.args.dest)
+	});
+
+	// Sender's balance is reduced
+	assert_eq!(sender_balance_before - amount_to_send - delivery_fees, sender_balance_after);
+	// Receiver's balance is increased
+	assert!(receiver_balance_after > receiver_balance_before);
+}