pallet-xcm: enhance reserve_transfer_assets to support remote reserves (#1672)

## Motivation

`pallet-xcm` is the main user-facing interface for XCM functionality,
including assets manipulation functions like `teleportAssets()` and
`reserve_transfer_assets()` calls.

While `teleportAsset()` works both ways, `reserve_transfer_assets()`
works only for sending reserve-based assets to a remote destination and
beneficiary when the reserve is the _local chain_.

## Solution

This PR enhances `pallet_xcm::(limited_)reserve_withdraw_assets` to
support transfers when reserves are other chains.
This will allow complete, **bi-directional** reserve-based asset
transfers user stories using `pallet-xcm`.

Enables following scenarios:
- transferring assets with local reserve (was previously supported iff
asset used as fee also had local reserve - now it works in all cases),
- transferring assets with reserve on destination,
- transferring assets with reserve on remote/third-party chain (iff
assets and fees have same remote reserve),
- transferring assets with reserve different than the reserve of the
asset to be used as fees - meaning can be used to transfer random asset
with local/dest reserve while using DOT for fees on all involved chains,
even if DOT local/dest reserve doesn't match asset reserve,
- transferring assets with any type of local/dest reserve while using
fees which can be teleported between involved chains.

All of the above is done by pallet inner logic without the user having
to specify which scenario/reserves/teleports/etc. The correct scenario
and corresponding XCM programs are identified, and respectively, built
automatically based on runtime configuration of trusted teleporters and
trusted reserves.

#### Current limitations:
- while `fees` and "non-fee" `assets` CAN have different reserves (or
fees CAN be teleported), the remaining "non-fee" `assets` CANNOT, among
themselves, have different reserve locations (this is also implicitly
enforced by `MAX_ASSETS_FOR_TRANSFER=2`, but this can be safely
increased in the future).
- `fees` and "non-fee" `assets` CANNOT have **different remote**
reserves (this could also be supported in the future, but adds even more
complexity while possibly not being worth it - we'll see what the future
holds).

Fixes https://github.com/paritytech/polkadot-sdk/issues/1584
Fixes https://github.com/paritytech/polkadot-sdk/issues/2055

---------

Co-authored-by: Francisco Aguirre <franciscoaguirreperez@gmail.com>
Co-authored-by: Branislav Kontur <bkontur@gmail.com>

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

@@ -65,7 +65,7 @@ pub type RelayToSystemParaTest = Test<Westend, AssetHubWestend>;
 pub type SystemParaToRelayTest = Test<AssetHubWestend, Westend>;
 pub type SystemParaToParaTest = Test<AssetHubWestend, PenpalA>;
 
-/// Returns a `TestArgs` instance to de used for the Relay Chain accross integraton tests
+/// Returns a `TestArgs` instance to be used for the Relay Chain across integration tests
 pub fn relay_test_args(amount: Balance) -> TestArgs {
 	TestArgs {
 		dest: Westend::child_location_of(AssetHubWestend::para_id()),
@@ -82,7 +82,7 @@ pub fn relay_test_args(amount: Balance) -> TestArgs {
 	}
 }
 
-/// Returns a `TestArgs` instance to de used for the System Parachain accross integraton tests
+/// Returns a `TestArgs` instance to be used for the System Parachain across integration tests
 pub fn system_para_test_args(
 	dest: MultiLocation,
 	beneficiary_id: AccountId32,

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

@@ -15,37 +15,8 @@
 
 use crate::*;
 use asset_hub_westend_runtime::xcm_config::XcmConfig;
-use westend_runtime::xcm_config::XcmConfig as WestendXcmConfig;
 
-fn relay_origin_assertions(t: RelayToSystemParaTest) {
-	type RuntimeEvent = <Westend as Chain>::RuntimeEvent;
-
-	Westend::assert_xcm_pallet_attempted_complete(Some(Weight::from_parts(629_384_000, 6_196)));
-
-	assert_expected_events!(
-		Westend,
-		vec![
-			// Amount to reserve transfer is transferred to System Parachain's Sovereign account
-			RuntimeEvent::Balances(pallet_balances::Event::Transfer { from, to, amount }) => {
-				from: *from == t.sender.account_id,
-				to: *to == Westend::sovereign_account_id_of(
-					t.args.dest
-				),
-				amount:  *amount == t.args.amount,
-			},
-		]
-	);
-}
-
-fn system_para_dest_assertions(_t: RelayToSystemParaTest) {
-	AssetHubWestend::assert_dmp_queue_incomplete(Some(Weight::from_parts(31_352_000, 1489)));
-}
-
-fn system_para_to_relay_assertions(_t: SystemParaToRelayTest) {
-	AssetHubWestend::assert_xcm_pallet_attempted_error(Some(XcmError::Barrier))
-}
-
-fn system_para_to_para_assertions(t: SystemParaToParaTest) {
+fn system_para_to_para_sender_assertions(t: SystemParaToParaTest) {
 	type RuntimeEvent = <AssetHubWestend as Chain>::RuntimeEvent;
 
 	AssetHubWestend::assert_xcm_pallet_attempted_complete(Some(Weight::from_parts(
@@ -56,7 +27,7 @@ fn system_para_to_para_assertions(t: SystemParaToParaTest) {
 	assert_expected_events!(
 		AssetHubWestend,
 		vec![
-			// Amount to reserve transfer is transferred to Parachain's Sovereing account
+			// Amount to reserve transfer is transferred to Parachain's Sovereign account
 			RuntimeEvent::Balances(
 				pallet_balances::Event::Transfer { from, to, amount }
 			) => {
@@ -70,6 +41,19 @@ fn system_para_to_para_assertions(t: SystemParaToParaTest) {
 	);
 }
 
+fn para_receiver_assertions<Test>(_: Test) {
+	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 system_para_to_para_assets_assertions(t: SystemParaToParaTest) {
 	type RuntimeEvent = <AssetHubWestend as Chain>::RuntimeEvent;
 
@@ -81,7 +65,7 @@ fn system_para_to_para_assets_assertions(t: SystemParaToParaTest) {
 	assert_expected_events!(
 		AssetHubWestend,
 		vec![
-			// Amount to reserve transfer is transferred to Parachain's Sovereing account
+			// Amount to reserve transfer is transferred to Parachain's Sovereign account
 			RuntimeEvent::Assets(
 				pallet_assets::Event::Transferred { asset_id, from, to, amount }
 			) => {
@@ -96,48 +80,6 @@ fn system_para_to_para_assets_assertions(t: SystemParaToParaTest) {
 	);
 }
 
-fn relay_limited_reserve_transfer_assets(t: RelayToSystemParaTest) -> DispatchResult {
-	<Westend as WestendPallet>::XcmPallet::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,
-	)
-}
-
-fn relay_reserve_transfer_assets(t: RelayToSystemParaTest) -> DispatchResult {
-	<Westend as WestendPallet>::XcmPallet::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,
-	)
-}
-
-fn system_para_limited_reserve_transfer_assets(t: SystemParaToRelayTest) -> DispatchResult {
-	<AssetHubWestend as AssetHubWestendPallet>::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,
-	)
-}
-
-fn system_para_reserve_transfer_assets(t: SystemParaToRelayTest) -> DispatchResult {
-	<AssetHubWestend as AssetHubWestendPallet>::PolkadotXcm::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,
-	)
-}
-
 fn system_para_to_para_limited_reserve_transfer_assets(t: SystemParaToParaTest) -> DispatchResult {
 	<AssetHubWestend as AssetHubWestendPallet>::PolkadotXcm::limited_reserve_transfer_assets(
 		t.signed_origin,
@@ -149,187 +91,72 @@ fn system_para_to_para_limited_reserve_transfer_assets(t: SystemParaToParaTest)
 	)
 }
 
-fn system_para_to_para_reserve_transfer_assets(t: SystemParaToParaTest) -> DispatchResult {
-	<AssetHubWestend as AssetHubWestendPallet>::PolkadotXcm::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,
-	)
-}
-
-/// Limited Reserve Transfers of native asset from Relay Chain to the System Parachain shouldn't
-/// work
-#[test]
-fn limited_reserve_transfer_native_asset_from_relay_to_system_para_fails() {
-	// Init values for Relay Chain
-	let amount_to_send: Balance = WESTEND_ED * 1000;
-	let test_args = TestContext {
-		sender: WestendSender::get(),
-		receiver: AssetHubWestendReceiver::get(),
-		args: relay_test_args(amount_to_send),
-	};
-
-	let mut test = RelayToSystemParaTest::new(test_args);
-
-	let sender_balance_before = test.sender.balance;
-	let receiver_balance_before = test.receiver.balance;
-
-	test.set_assertion::<Westend>(relay_origin_assertions);
-	test.set_assertion::<AssetHubWestend>(system_para_dest_assertions);
-	test.set_dispatchable::<Westend>(relay_limited_reserve_transfer_assets);
-	test.assert();
-
-	let delivery_fees = Westend::execute_with(|| {
-		xcm_helpers::transfer_assets_delivery_fees::<
-			<WestendXcmConfig as xcm_executor::Config>::XcmSender,
-		>(test.args.assets.clone(), 0, test.args.weight_limit, test.args.beneficiary, test.args.dest)
-	});
-
-	let sender_balance_after = test.sender.balance;
-	let receiver_balance_after = test.receiver.balance;
-
-	assert_eq!(sender_balance_before - amount_to_send - delivery_fees, sender_balance_after);
-	assert_eq!(receiver_balance_before, receiver_balance_after);
-}
-
-/// Limited Reserve Transfers of native asset from System Parachain to Relay Chain shoudln't work
-#[test]
-fn limited_reserve_transfer_native_asset_from_system_para_to_relay_fails() {
-	// Init values for System Parachain
-	let destination = AssetHubWestend::parent_location();
-	let beneficiary_id = WestendReceiver::get();
-	let amount_to_send: Balance = ASSET_HUB_WESTEND_ED * 1000;
-	let assets = (Parent, amount_to_send).into();
-
-	let test_args = TestContext {
-		sender: AssetHubWestendSender::get(),
-		receiver: WestendReceiver::get(),
-		args: system_para_test_args(destination, beneficiary_id, amount_to_send, assets, None),
-	};
-
-	let mut test = SystemParaToRelayTest::new(test_args);
-
-	let sender_balance_before = test.sender.balance;
-	let receiver_balance_before = test.receiver.balance;
-
-	test.set_assertion::<AssetHubWestend>(system_para_to_relay_assertions);
-	test.set_dispatchable::<AssetHubWestend>(system_para_limited_reserve_transfer_assets);
-	test.assert();
-
-	let sender_balance_after = test.sender.balance;
-	let receiver_balance_after = test.receiver.balance;
-
-	assert_eq!(sender_balance_before, sender_balance_after);
-	assert_eq!(receiver_balance_before, receiver_balance_after);
-}
-
 /// 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() {
-	// Init values for Relay Chain
+	let signed_origin = <Westend as Chain>::RuntimeOrigin::signed(WestendSender::get().into());
+	let destination = Westend::child_location_of(AssetHubWestend::para_id());
+	let beneficiary: MultiLocation =
+		AccountId32Junction { network: None, id: AssetHubWestendReceiver::get().into() }.into();
 	let amount_to_send: Balance = WESTEND_ED * 1000;
-	let test_args = TestContext {
-		sender: WestendSender::get(),
-		receiver: AssetHubWestendReceiver::get(),
-		args: relay_test_args(amount_to_send),
-	};
+	let assets: MultiAssets = (Here, amount_to_send).into();
+	let fee_asset_item = 0;
 
-	let mut test = RelayToSystemParaTest::new(test_args);
-
-	let sender_balance_before = test.sender.balance;
-	let receiver_balance_before = test.receiver.balance;
-
-	test.set_assertion::<Westend>(relay_origin_assertions);
-	test.set_assertion::<AssetHubWestend>(system_para_dest_assertions);
-	test.set_dispatchable::<Westend>(relay_reserve_transfer_assets);
-	test.assert();
-
-	let delivery_fees = Westend::execute_with(|| {
-		xcm_helpers::transfer_assets_delivery_fees::<
-			<WestendXcmConfig as xcm_executor::Config>::XcmSender,
-		>(test.args.assets.clone(), 0, test.args.weight_limit, test.args.beneficiary, test.args.dest)
+	// this should fail
+	Westend::execute_with(|| {
+		let result = <Westend as WestendPallet>::XcmPallet::limited_reserve_transfer_assets(
+			signed_origin,
+			bx!(destination.into()),
+			bx!(beneficiary.into()),
+			bx!(assets.into()),
+			fee_asset_item,
+			WeightLimit::Unlimited,
+		);
+		assert_err!(
+			result,
+			DispatchError::Module(sp_runtime::ModuleError {
+				index: 99,
+				error: [2, 0, 0, 0],
+				message: Some("Filtered")
+			})
+		);
 	});
-
-	let sender_balance_after = test.sender.balance;
-	let receiver_balance_after = test.receiver.balance;
-
-	assert_eq!(sender_balance_before - amount_to_send - delivery_fees, sender_balance_after);
-	assert_eq!(receiver_balance_before, receiver_balance_after);
 }
 
 /// Reserve Transfers of native asset from System Parachain to Relay Chain shouldn't work
 #[test]
 fn reserve_transfer_native_asset_from_system_para_to_relay_fails() {
 	// Init values for System Parachain
+	let signed_origin =
+		<AssetHubWestend as Chain>::RuntimeOrigin::signed(AssetHubWestendSender::get().into());
 	let destination = AssetHubWestend::parent_location();
 	let beneficiary_id = WestendReceiver::get();
+	let beneficiary: MultiLocation =
+		AccountId32Junction { network: None, id: beneficiary_id.into() }.into();
 	let amount_to_send: Balance = ASSET_HUB_WESTEND_ED * 1000;
-	let assets = (Parent, amount_to_send).into();
+	let assets: MultiAssets = (Parent, amount_to_send).into();
+	let fee_asset_item = 0;
 
-	let test_args = TestContext {
-		sender: AssetHubWestendSender::get(),
-		receiver: WestendReceiver::get(),
-		args: system_para_test_args(destination, beneficiary_id, amount_to_send, assets, None),
-	};
-
-	let mut test = SystemParaToRelayTest::new(test_args);
-
-	let sender_balance_before = test.sender.balance;
-	let receiver_balance_before = test.receiver.balance;
-
-	test.set_assertion::<AssetHubWestend>(system_para_to_relay_assertions);
-	test.set_dispatchable::<AssetHubWestend>(system_para_reserve_transfer_assets);
-	test.assert();
-
-	let sender_balance_after = test.sender.balance;
-	let receiver_balance_after = test.receiver.balance;
-
-	assert_eq!(sender_balance_before, sender_balance_after);
-	assert_eq!(receiver_balance_before, receiver_balance_after);
-}
-
-/// Limited Reserve Transfers of native asset from System Parachain to Parachain should work
-#[test]
-fn limited_reserve_transfer_native_asset_from_system_para_to_para() {
-	// Init values for System Parachain
-	let destination = AssetHubWestend::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: AssetHubWestendSender::get(),
-		receiver: PenpalAReceiver::get(),
-		args: system_para_test_args(destination, beneficiary_id, amount_to_send, assets, None),
-	};
-
-	let mut test = SystemParaToParaTest::new(test_args);
-
-	let sender_balance_before = test.sender.balance;
-
-	test.set_assertion::<AssetHubWestend>(system_para_to_para_assertions);
-	// TODO: Add assertion for Penpal runtime. Right now message is failing with
-	// `UntrustedReserveLocation`
-	test.set_dispatchable::<AssetHubWestend>(system_para_to_para_limited_reserve_transfer_assets);
-	test.assert();
-
-	let sender_balance_after = test.sender.balance;
-
-	let delivery_fees = AssetHubWestend::execute_with(|| {
-		xcm_helpers::transfer_assets_delivery_fees::<<XcmConfig as xcm_executor::Config>::XcmSender>(
-			test.args.assets.clone(),
-			0,
-			test.args.weight_limit,
-			test.args.beneficiary,
-			test.args.dest,
-		)
+	// this should fail
+	AssetHubWestend::execute_with(|| {
+		let result =
+			<AssetHubWestend as AssetHubWestendPallet>::PolkadotXcm::limited_reserve_transfer_assets(
+				signed_origin,
+				bx!(destination.into()),
+				bx!(beneficiary.into()),
+				bx!(assets.into()),
+				fee_asset_item,
+				WeightLimit::Unlimited,
+			);
+		assert_err!(
+			result,
+			DispatchError::Module(sp_runtime::ModuleError {
+				index: 31,
+				error: [2, 0, 0, 0],
+				message: Some("Filtered")
+			})
+		);
 	});
-
-	assert_eq!(sender_balance_before - amount_to_send - delivery_fees, sender_balance_after);
-	// TODO: Check receiver balance when Penpal runtime is improved to propery handle reserve
-	// transfers
 }
 
 /// Reserve Transfers of native asset from System Parachain to Parachain should work
@@ -350,14 +177,15 @@ fn reserve_transfer_native_asset_from_system_para_to_para() {
 	let mut test = SystemParaToParaTest::new(test_args);
 
 	let sender_balance_before = test.sender.balance;
+	let receiver_balance_before = test.receiver.balance;
 
-	test.set_assertion::<AssetHubWestend>(system_para_to_para_assertions);
-	// TODO: Add assertion for Penpal runtime. Right now message is failing with
-	// `UntrustedReserveLocation`
-	test.set_dispatchable::<AssetHubWestend>(system_para_to_para_reserve_transfer_assets);
+	test.set_assertion::<AssetHubWestend>(system_para_to_para_sender_assertions);
+	test.set_assertion::<PenpalA>(para_receiver_assertions);
+	test.set_dispatchable::<AssetHubWestend>(system_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 = AssetHubWestend::execute_with(|| {
 		xcm_helpers::transfer_assets_delivery_fees::<<XcmConfig as xcm_executor::Config>::XcmSender>(
@@ -369,45 +197,10 @@ fn reserve_transfer_native_asset_from_system_para_to_para() {
 		)
 	});
 
+	// Sender's balance is reduced
 	assert_eq!(sender_balance_before - amount_to_send - delivery_fees, sender_balance_after);
-	// TODO: Check receiver balance when Penpal runtime is improved to propery handle reserve
-	// transfers
-}
-
-/// Limited Reserve Transfers of a local asset from System Parachain to Parachain should work
-#[test]
-fn limited_reserve_transfer_asset_from_system_para_to_para() {
-	// Force create asset from Relay Chain and mint assets for System Parachain's sender account
-	AssetHubWestend::force_create_and_mint_asset(
-		ASSET_ID,
-		ASSET_MIN_BALANCE,
-		true,
-		AssetHubWestendSender::get(),
-		Some(Weight::from_parts(1_019_445_000, 200_000)),
-		ASSET_MIN_BALANCE * 1000000,
-	);
-
-	// Init values for System Parachain
-	let destination = AssetHubWestend::sibling_location_of(PenpalA::para_id());
-	let beneficiary_id = PenpalAReceiver::get();
-	let amount_to_send = ASSET_MIN_BALANCE * 1000;
-	let assets =
-		(X2(PalletInstance(ASSETS_PALLET_ID), GeneralIndex(ASSET_ID.into())), amount_to_send)
-			.into();
-
-	let system_para_test_args = TestContext {
-		sender: AssetHubWestendSender::get(),
-		receiver: PenpalAReceiver::get(),
-		args: system_para_test_args(destination, beneficiary_id, amount_to_send, assets, None),
-	};
-
-	let mut system_para_test = SystemParaToParaTest::new(system_para_test_args);
-
-	system_para_test.set_assertion::<AssetHubWestend>(system_para_to_para_assets_assertions);
-	// TODO: Add assertions when Penpal is able to manage assets
-	system_para_test
-		.set_dispatchable::<AssetHubWestend>(system_para_to_para_limited_reserve_transfer_assets);
-	system_para_test.assert();
+	// Receiver's balance is increased
+	assert!(receiver_balance_after > receiver_balance_before);
 }
 
 /// Reserve Transfers of a local asset from System Parachain to Parachain should work
@@ -442,6 +235,6 @@ fn reserve_transfer_asset_from_system_para_to_para() {
 	system_para_test.set_assertion::<AssetHubWestend>(system_para_to_para_assets_assertions);
 	// TODO: Add assertions when Penpal is able to manage assets
 	system_para_test
-		.set_dispatchable::<AssetHubWestend>(system_para_to_para_reserve_transfer_assets);
+		.set_dispatchable::<AssetHubWestend>(system_para_to_para_limited_reserve_transfer_assets);
 	system_para_test.assert();
 }

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

@@ -51,7 +51,7 @@ fn relay_dest_assertions(t: SystemParaToRelayTest) {
 	assert_expected_events!(
 		Westend,
 		vec![
-			// Amount is witdrawn from Relay Chain's `CheckAccount`
+			// Amount is withdrawn from Relay Chain's `CheckAccount`
 			RuntimeEvent::Balances(pallet_balances::Event::Withdraw { who, amount }) => {
 				who: *who == <Westend as WestendPallet>::XcmPallet::check_account(),
 				amount: *amount == t.args.amount,