xcm-executor: take transport fee from transferred assets if necessary (…

…paritytech#4834)

# Description

Sending XCM messages to other chains requires paying a "transport fee".
This can be paid either:
- from `origin` local account if `jit_withdraw = true`,
- taken from Holding register otherwise.

This currently works for following hops/scenarios:
1. On destination no transport fee needed (only sending costs, not
receiving),
2. Local/originating chain: just set JIT=true and fee will be paid from
signed account,
3. Intermediary hops - only if intermediary is acting as reserve between
two untrusted chains (aka only for `DepositReserveAsset` instruction) -
this was fixed in paritytech#3142

But now we're seeing more complex asset transfers that are mixing
reserve transfers with teleports depending on the involved chains.

# Example

E.g. transferring DOT between Relay and parachain, but through AH (using
AH instead of the Relay chain as parachain's DOT reserve).

In the `Parachain --1--> AssetHub --2--> Relay` scenario, DOT has to be
reserve-withdrawn in leg `1`, then teleported in leg `2`.
On the intermediary hop (AssetHub), `InitiateTeleport` fails to send
onward message because of missing transport fees. We also can't rely on
`jit_withdraw` because the original origin is lost on the way, and even
if it weren't we can't rely on the user having funded accounts on each
hop along the way.

# Solution/Changes

- Charge the transport fee in the executor from the transferred assets
(if available),
- Only charge from transferred assets if JIT_WITHDRAW was not set,
- Only charge from transferred assets if unless using XCMv5 `PayFees`
where we do not have this problem.

# Testing

Added regression tests in emulated transfers.

Fixes paritytech#4832
Fixes paritytech#6637

---------

Signed-off-by: Adrian Catangiu <[email protected]>
Co-authored-by: Francisco Aguirre <[email protected]>

bridges/snowbridge/primitives/router/src/inbound/mod.rs

@@ -358,7 +358,9 @@ where
 					}])),
 					// Perform a deposit reserve to send to destination chain.
 					DepositReserveAsset {
-						assets: Definite(vec![dest_para_fee_asset.clone(), asset].into()),
+						// Send over assets and unspent fees, XCM delivery fee will be charged from
+						// here.
+						assets: Wild(AllCounted(2)),
 						dest: Location::new(1, [Parachain(dest_para_id)]),
 						xcm: vec![
 							// Buy execution on target.

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

@@ -115,7 +115,7 @@ pub fn system_para_to_para_sender_assertions(t: SystemParaToParaTest) {
 	assert_expected_events!(
 		AssetHubRococo,
 		vec![
-			// Transport fees are paid
+			// Delivery fees are paid
 			RuntimeEvent::PolkadotXcm(pallet_xcm::Event::FeesPaid { .. }) => {},
 		]
 	);
@@ -274,7 +274,7 @@ fn system_para_to_para_assets_sender_assertions(t: SystemParaToParaTest) {
 					t.args.dest.clone()
 				),
 			},
-			// Transport fees are paid
+			// Delivery fees are paid
 			RuntimeEvent::PolkadotXcm(
 				pallet_xcm::Event::FeesPaid { .. }
 			) => {},
@@ -305,7 +305,7 @@ fn para_to_system_para_assets_sender_assertions(t: ParaToSystemParaTest) {
 				owner: *owner == t.sender.account_id,
 				balance: *balance == t.args.amount,
 			},
-			// Transport fees are paid
+			// Delivery fees are paid
 			RuntimeEvent::PolkadotXcm(
 				pallet_xcm::Event::FeesPaid { .. }
 			) => {},

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

@@ -106,6 +106,7 @@ mod imports {
 	pub type ParaToParaThroughRelayTest = Test<PenpalA, PenpalB, Westend>;
 	pub type ParaToParaThroughAHTest = Test<PenpalA, PenpalB, AssetHubWestend>;
 	pub type RelayToParaThroughAHTest = Test<Westend, PenpalA, AssetHubWestend>;
+	pub type PenpalToRelayThroughAHTest = Test<PenpalA, Westend, AssetHubWestend>;
 }
 
 #[cfg(test)]

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

@@ -658,13 +658,13 @@ fn bidirectional_teleport_foreign_asset_between_para_and_asset_hub_using_explici
 }
 
 // ===============================================================
-// ===== Transfer - Native Asset - Relay->AssetHub->Parachain ====
+// ====== Transfer - Native Asset - Relay->AssetHub->Penpal ======
 // ===============================================================
-/// Transfers of native asset Relay to Parachain (using AssetHub reserve). Parachains want to avoid
+/// Transfers of native asset Relay to Penpal (using AssetHub reserve). Parachains want to avoid
 /// managing SAs on all system chains, thus want all their DOT-in-reserve to be held in their
 /// Sovereign Account on Asset Hub.
 #[test]
-fn transfer_native_asset_from_relay_to_para_through_asset_hub() {
+fn transfer_native_asset_from_relay_to_penpal_through_asset_hub() {
 	// Init values for Relay
 	let destination = Westend::child_location_of(PenpalA::para_id());
 	let sender = WestendSender::get();
@@ -820,6 +820,137 @@ fn transfer_native_asset_from_relay_to_para_through_asset_hub() {
 	assert!(receiver_assets_after < receiver_assets_before + amount_to_send);
 }
 
+// ===============================================================
+// ===== Transfer - Native Asset - Penpal->AssetHub->Relay =======
+// ===============================================================
+/// Transfers of native asset Penpal to Relay (using AssetHub reserve). Parachains want to avoid
+/// managing SAs on all system chains, thus want all their DOT-in-reserve to be held in their
+/// Sovereign Account on Asset Hub.
+#[test]
+fn transfer_native_asset_from_penpal_to_relay_through_asset_hub() {
+	// Init values for Penpal
+	let destination = RelayLocation::get();
+	let sender = PenpalASender::get();
+	let amount_to_send: Balance = WESTEND_ED * 100;
+
+	// Init values for Penpal
+	let relay_native_asset_location = RelayLocation::get();
+	let receiver = WestendReceiver::get();
+
+	// Init Test
+	let test_args = TestContext {
+		sender: sender.clone(),
+		receiver: receiver.clone(),
+		args: TestArgs::new_para(
+			destination.clone(),
+			receiver.clone(),
+			amount_to_send,
+			(Parent, amount_to_send).into(),
+			None,
+			0,
+		),
+	};
+	let mut test = PenpalToRelayThroughAHTest::new(test_args);
+
+	let sov_penpal_on_ah = AssetHubWestend::sovereign_account_id_of(
+		AssetHubWestend::sibling_location_of(PenpalA::para_id()),
+	);
+	// fund Penpal's sender account
+	PenpalA::mint_foreign_asset(
+		<PenpalA as Chain>::RuntimeOrigin::signed(PenpalAssetOwner::get()),
+		relay_native_asset_location.clone(),
+		sender.clone(),
+		amount_to_send * 2,
+	);
+	// fund Penpal's SA on AssetHub with the assets held in reserve
+	AssetHubWestend::fund_accounts(vec![(sov_penpal_on_ah.clone().into(), amount_to_send * 2)]);
+
+	// prefund Relay checking account so we accept teleport "back" from AssetHub
+	let check_account =
+		Westend::execute_with(|| <Westend as WestendPallet>::XcmPallet::check_account());
+	Westend::fund_accounts(vec![(check_account, amount_to_send)]);
+
+	// Query initial balances
+	let sender_balance_before = PenpalA::execute_with(|| {
+		type ForeignAssets = <PenpalA as PenpalAPallet>::ForeignAssets;
+		<ForeignAssets as Inspect<_>>::balance(relay_native_asset_location.clone(), &sender)
+	});
+	let sov_penpal_on_ah_before = AssetHubWestend::execute_with(|| {
+		<AssetHubWestend as AssetHubWestendPallet>::Balances::free_balance(sov_penpal_on_ah.clone())
+	});
+	let receiver_balance_before = Westend::execute_with(|| {
+		<Westend as WestendPallet>::Balances::free_balance(receiver.clone())
+	});
+
+	fn transfer_assets_dispatchable(t: PenpalToRelayThroughAHTest) -> DispatchResult {
+		let fee_idx = t.args.fee_asset_item as usize;
+		let fee: Asset = t.args.assets.inner().get(fee_idx).cloned().unwrap();
+		let asset_hub_location = PenpalA::sibling_location_of(AssetHubWestend::para_id());
+		let context = PenpalUniversalLocation::get();
+
+		// reanchor fees to the view of destination (Westend Relay)
+		let mut remote_fees = fee.clone().reanchored(&t.args.dest, &context).unwrap();
+		if let Fungible(ref mut amount) = remote_fees.fun {
+			// we already spent some fees along the way, just use half of what we started with
+			*amount = *amount / 2;
+		}
+		let xcm_on_final_dest = Xcm::<()>(vec![
+			BuyExecution { fees: remote_fees, weight_limit: t.args.weight_limit.clone() },
+			DepositAsset {
+				assets: Wild(AllCounted(t.args.assets.len() as u32)),
+				beneficiary: t.args.beneficiary,
+			},
+		]);
+
+		// reanchor final dest (Westend Relay) to the view of hop (Asset Hub)
+		let mut dest = t.args.dest.clone();
+		dest.reanchor(&asset_hub_location, &context).unwrap();
+		// on Asset Hub
+		let xcm_on_hop = Xcm::<()>(vec![InitiateTeleport {
+			assets: Wild(AllCounted(t.args.assets.len() as u32)),
+			dest,
+			xcm: xcm_on_final_dest,
+		}]);
+
+		// First leg is a reserve-withdraw, from there a teleport to final dest
+		<PenpalA as PenpalAPallet>::PolkadotXcm::transfer_assets_using_type_and_then(
+			t.signed_origin,
+			bx!(asset_hub_location.into()),
+			bx!(t.args.assets.into()),
+			bx!(TransferType::DestinationReserve),
+			bx!(fee.id.into()),
+			bx!(TransferType::DestinationReserve),
+			bx!(VersionedXcm::from(xcm_on_hop)),
+			t.args.weight_limit,
+		)
+	}
+	test.set_dispatchable::<PenpalA>(transfer_assets_dispatchable);
+	test.assert();
+
+	// Query final balances
+	let sender_balance_after = PenpalA::execute_with(|| {
+		type ForeignAssets = <PenpalA as PenpalAPallet>::ForeignAssets;
+		<ForeignAssets as Inspect<_>>::balance(relay_native_asset_location.clone(), &sender)
+	});
+	let sov_penpal_on_ah_after = AssetHubWestend::execute_with(|| {
+		<AssetHubWestend as AssetHubWestendPallet>::Balances::free_balance(sov_penpal_on_ah.clone())
+	});
+	let receiver_balance_after = Westend::execute_with(|| {
+		<Westend as WestendPallet>::Balances::free_balance(receiver.clone())
+	});
+
+	// Sender's asset balance is reduced by amount sent plus delivery fees
+	assert!(sender_balance_after < sender_balance_before - amount_to_send);
+	// SA on AH balance is decreased by `amount_to_send`
+	assert_eq!(sov_penpal_on_ah_after, sov_penpal_on_ah_before - amount_to_send);
+	// Receiver's balance is increased
+	assert!(receiver_balance_after > receiver_balance_before);
+	// Receiver's balance increased by `amount_to_send - delivery_fees - bought_execution`;
+	// `delivery_fees` might be paid from transfer or JIT, also `bought_execution` is unknown but
+	// should be non-zero
+	assert!(receiver_balance_after < receiver_balance_before + amount_to_send);
+}
+
 // ==============================================================================================
 // ==== Bidirectional Transfer - Native + Teleportable Foreign Assets - Parachain<->AssetHub ====
 // ==============================================================================================
@@ -839,7 +970,7 @@ fn bidirectional_transfer_multiple_assets_between_penpal_and_asset_hub() {
 		// xcm to be executed at dest
 		let xcm_on_dest = Xcm(vec![
 			// since this is the last hop, we don't need to further use any assets previously
-			// reserved for fees (there are no further hops to cover transport fees for); we
+			// reserved for fees (there are no further hops to cover delivery fees for); we
 			// RefundSurplus to get back any unspent fees
 			RefundSurplus,
 			DepositAsset { assets: Wild(All), beneficiary: t.args.beneficiary },
@@ -875,7 +1006,7 @@ fn bidirectional_transfer_multiple_assets_between_penpal_and_asset_hub() {
 		// xcm to be executed at dest
 		let xcm_on_dest = Xcm(vec![
 			// since this is the last hop, we don't need to further use any assets previously
-			// reserved for fees (there are no further hops to cover transport fees for); we
+			// reserved for fees (there are no further hops to cover delivery fees for); we
 			// RefundSurplus to get back any unspent fees
 			RefundSurplus,
 			DepositAsset { assets: Wild(All), beneficiary: t.args.beneficiary },

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

@@ -115,7 +115,7 @@ pub fn system_para_to_para_sender_assertions(t: SystemParaToParaTest) {
 	assert_expected_events!(
 		AssetHubWestend,
 		vec![
-			// Transport fees are paid
+			// Delivery fees are paid
 			RuntimeEvent::PolkadotXcm(pallet_xcm::Event::FeesPaid { .. }) => {},
 		]
 	);
@@ -274,7 +274,7 @@ fn system_para_to_para_assets_sender_assertions(t: SystemParaToParaTest) {
 					t.args.dest.clone()
 				),
 			},
-			// Transport fees are paid
+			// Delivery fees are paid
 			RuntimeEvent::PolkadotXcm(
 				pallet_xcm::Event::FeesPaid { .. }
 			) => {},
@@ -305,7 +305,7 @@ fn para_to_system_para_assets_sender_assertions(t: ParaToSystemParaTest) {
 				owner: *owner == t.sender.account_id,
 				balance: *balance == t.args.amount,
 			},
-			// Transport fees are paid
+			// Delivery fees are paid
 			RuntimeEvent::PolkadotXcm(
 				pallet_xcm::Event::FeesPaid { .. }
 			) => {},

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

@@ -46,7 +46,7 @@ fn transfer_and_transact_in_same_xcm(
 		Transact { origin_kind: OriginKind::Xcm, call, fallback_max_weight: None },
 		ExpectTransactStatus(MaybeErrorCode::Success),
 		// since this is the last hop, we don't need to further use any assets previously
-		// reserved for fees (there are no further hops to cover transport fees for); we
+		// reserved for fees (there are no further hops to cover delivery fees for); we
 		// RefundSurplus to get back any unspent fees
 		RefundSurplus,
 		DepositAsset { assets: Wild(All), beneficiary },

cumulus/parachains/integration-tests/emulated/tests/bridges/bridge-hub-rococo/src/tests/asset_transfers.rs

@@ -16,7 +16,7 @@
 use crate::tests::*;
 
 fn send_assets_over_bridge<F: FnOnce()>(send_fn: F) {
-	// fund the AHR's SA on BHR for paying bridge transport fees
+	// fund the AHR's SA on BHR for paying bridge delivery fees
 	BridgeHubRococo::fund_para_sovereign(AssetHubRococo::para_id(), 10_000_000_000_000u128);
 
 	// set XCM versions

cumulus/parachains/integration-tests/emulated/tests/bridges/bridge-hub-rococo/src/tests/register_bridged_assets.rs

@@ -58,7 +58,7 @@ fn register_rococo_asset_on_wah_from_rah() {
 
 	let destination = asset_hub_westend_location();
 
-	// fund the RAH's SA on RBH for paying bridge transport fees
+	// fund the RAH's SA on RBH for paying bridge delivery fees
 	BridgeHubRococo::fund_para_sovereign(AssetHubRococo::para_id(), 10_000_000_000_000u128);
 
 	// set XCM versions

cumulus/parachains/integration-tests/emulated/tests/bridges/bridge-hub-rococo/src/tests/send_xcm.rs

@@ -65,7 +65,7 @@ fn send_xcm_through_opened_lane_with_different_xcm_version_on_hops_works() {
 	let native_token = Location::parent();
 	let amount = ASSET_HUB_ROCOCO_ED * 1_000;
 
-	// fund the AHR's SA on BHR for paying bridge transport fees
+	// fund the AHR's SA on BHR for paying bridge delivery fees
 	BridgeHubRococo::fund_para_sovereign(AssetHubRococo::para_id(), 10_000_000_000_000u128);
 	// fund sender
 	AssetHubRococo::fund_accounts(vec![(AssetHubRococoSender::get().into(), amount * 10)]);

cumulus/parachains/integration-tests/emulated/tests/bridges/bridge-hub-westend/src/tests/asset_transfers.rs

@@ -17,7 +17,7 @@ use crate::{create_pool_with_native_on, tests::*};
 use xcm::latest::AssetTransferFilter;
 
 fn send_assets_over_bridge<F: FnOnce()>(send_fn: F) {
-	// fund the AHW's SA on BHW for paying bridge transport fees
+	// fund the AHW's SA on BHW for paying bridge delivery fees
 	BridgeHubWestend::fund_para_sovereign(AssetHubWestend::para_id(), 10_000_000_000_000u128);
 
 	// set XCM versions
@@ -592,7 +592,7 @@ fn do_send_pens_and_wnds_from_penpal_westend_via_ahw_to_asset_hub_rococo(
 			// XCM to be executed at dest (Rococo Asset Hub)
 			let xcm_on_dest = Xcm(vec![
 				// since this is the last hop, we don't need to further use any assets previously
-				// reserved for fees (there are no further hops to cover transport fees for); we
+				// reserved for fees (there are no further hops to cover delivery fees for); we
 				// RefundSurplus to get back any unspent fees
 				RefundSurplus,
 				// deposit everything to final beneficiary

cumulus/parachains/integration-tests/emulated/tests/bridges/bridge-hub-westend/src/tests/register_bridged_assets.rs

@@ -82,7 +82,7 @@ fn register_asset_on_rah_from_wah(bridged_asset_at_rah: Location) {
 
 	let destination = asset_hub_rococo_location();
 
-	// fund the WAH's SA on WBH for paying bridge transport fees
+	// fund the WAH's SA on WBH for paying bridge delivery fees
 	BridgeHubWestend::fund_para_sovereign(AssetHubWestend::para_id(), 10_000_000_000_000u128);
 
 	// set XCM versions

cumulus/parachains/integration-tests/emulated/tests/bridges/bridge-hub-westend/src/tests/send_xcm.rs

@@ -65,7 +65,7 @@ fn send_xcm_through_opened_lane_with_different_xcm_version_on_hops_works() {
 	let native_token = Location::parent();
 	let amount = ASSET_HUB_WESTEND_ED * 1_000;
 
-	// fund the AHR's SA on BHR for paying bridge transport fees
+	// fund the AHR's SA on BHR for paying bridge delivery fees
 	BridgeHubWestend::fund_para_sovereign(AssetHubWestend::para_id(), 10_000_000_000_000u128);
 	// fund sender
 	AssetHubWestend::fund_accounts(vec![(AssetHubWestendSender::get().into(), amount * 10)]);

cumulus/parachains/integration-tests/emulated/tests/bridges/bridge-hub-westend/src/tests/transact.rs

@@ -52,7 +52,7 @@ fn transfer_and_transact_in_same_xcm(
 		Transact { origin_kind: OriginKind::Xcm, call, fallback_max_weight: None },
 		ExpectTransactStatus(MaybeErrorCode::Success),
 		// since this is the last hop, we don't need to further use any assets previously
-		// reserved for fees (there are no further hops to cover transport fees for); we
+		// reserved for fees (there are no further hops to cover delivery fees for); we
 		// RefundSurplus to get back any unspent fees
 		RefundSurplus,
 		DepositAsset { assets: Wild(All), beneficiary },