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.

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.

- 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 Holding doesn't already contain enough (other)
assets to pay for the transport fee.

Added regression tests in emulated transfers.

Fixes paritytech#4832

Signed-off-by: Adrian Catangiu <[email protected]>

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 ====
 // ==============================================================================================

polkadot/xcm/xcm-executor/src/lib.rs

@@ -1086,12 +1086,13 @@ impl<Config: config::Config> XcmExecutor<Config> {
 			DepositReserveAsset { assets, dest, xcm } => {
 				let old_holding = self.holding.clone();
 				let result = Config::TransactionalProcessor::process(|| {
-					let maybe_delivery_fee_from_holding = if self.fees.is_empty() {
-						self.get_delivery_fee_from_holding(&assets, &dest, &xcm)?
+					// When not using `PayFees`, nor `JIT_WITHDRAW`, transport fees are paid from
+					// transferred assets.
+					let maybe_transport_fee_from_holding = if self.fees.is_empty() && !self.fees_mode.jit_withdraw {
+						self.get_delivery_fee_from_holding(&assets, &dest, FeeReason::DepositReserveAsset, &xcm)?
 					} else {
 						None
 					};
-
 					let mut message = Vec::with_capacity(xcm.len() + 2);
 					// now take assets to deposit (after having taken delivery fees)
 					let deposited = self.holding.saturating_take(assets);
@@ -1106,7 +1107,7 @@ impl<Config: config::Config> XcmExecutor<Config> {
 					message.push(ClearOrigin);
 					// append custom instructions
 					message.extend(xcm.0.into_iter());
-					if let Some(delivery_fee) = maybe_delivery_fee_from_holding {
+					if let Some(delivery_fee) = maybe_transport_fee_from_holding {
 						// Put back delivery_fee in holding register to be charged by XcmSender.
 						self.holding.subsume_assets(delivery_fee);
 					}
@@ -1121,6 +1122,13 @@ impl<Config: config::Config> XcmExecutor<Config> {
 			InitiateReserveWithdraw { assets, reserve, xcm } => {
 				let old_holding = self.holding.clone();
 				let result = Config::TransactionalProcessor::process(|| {
+					// When not using `PayFees`, nor `JIT_WITHDRAW`, transport fees are paid from
+					// transferred assets.
+					let maybe_transport_fee_from_holding = if self.fees.is_empty() && !self.fees_mode.jit_withdraw {
+						self.get_delivery_fee_from_holding(&assets, &reserve, FeeReason::InitiateReserveWithdraw, &xcm)?
+					} else {
+						None
+					};
 					let assets = self.holding.saturating_take(assets);
 					let mut message = Vec::with_capacity(xcm.len() + 2);
 					Self::do_reserve_withdraw_assets(
@@ -1133,6 +1141,10 @@ impl<Config: config::Config> XcmExecutor<Config> {
 					message.push(ClearOrigin);
 					// append custom instructions
 					message.extend(xcm.0.into_iter());
+					if let Some(delivery_fee) = maybe_transport_fee_from_holding {
+						// Put back delivery_fee in holding register to be charged by XcmSender.
+						self.holding.subsume_assets(delivery_fee);
+					}
 					self.send(reserve, Xcm(message), FeeReason::InitiateReserveWithdraw)?;
 					Ok(())
 				});
@@ -1144,13 +1156,24 @@ impl<Config: config::Config> XcmExecutor<Config> {
 			InitiateTeleport { assets, dest, xcm } => {
 				let old_holding = self.holding.clone();
 				let result = Config::TransactionalProcessor::process(|| {
+					// When not using `PayFees`, nor `JIT_WITHDRAW`, transport fees are paid from
+					// transferred assets.
+					let maybe_transport_fee_from_holding = if self.fees.is_empty() && !self.fees_mode.jit_withdraw {
+						self.get_delivery_fee_from_holding(&assets, &dest, FeeReason::InitiateTeleport, &xcm)?
+					} else {
+						None
+					};
 					let assets = self.holding.saturating_take(assets);
 					let mut message = Vec::with_capacity(xcm.len() + 2);
 					Self::do_teleport_assets(assets, &dest, &mut message, &self.context)?;
 					// clear origin for subsequent custom instructions
 					message.push(ClearOrigin);
 					// append custom instructions
 					message.extend(xcm.0.into_iter());
+					if let Some(delivery_fee) = maybe_transport_fee_from_holding {
+						// Put back delivery_fee in holding register to be charged by XcmSender.
+						self.holding.subsume_assets(delivery_fee);
+					}
 					self.send(dest.clone(), Xcm(message), FeeReason::InitiateTeleport)?;
 					Ok(())
 				});
@@ -1707,36 +1730,51 @@ impl<Config: config::Config> XcmExecutor<Config> {
 		Ok(())
 	}
 
-	/// Gets the necessary delivery fee to send a reserve transfer message to `destination` from
-	/// holding.
+	/// Gets the necessary delivery fee from holding to send an onward transfer message to
+	/// `destination`.
 	///
 	/// Will be removed once the transition from `BuyExecution` to `PayFees` is complete.
 	fn get_delivery_fee_from_holding(
 		&mut self,
 		assets: &AssetFilter,
 		destination: &Location,
+		reason: FeeReason,
 		xcm: &Xcm<()>,
 	) -> Result<Option<AssetsInHolding>, XcmError> {
 		// we need to do this take/put cycle to solve wildcards and get exact assets to
 		// be weighed
 		let to_weigh = self.holding.saturating_take(assets.clone());
 		self.holding.subsume_assets(to_weigh.clone());
 		let to_weigh_reanchored = Self::reanchored(to_weigh, &destination, None);
-		let mut message_to_weigh = vec![ReserveAssetDeposited(to_weigh_reanchored), ClearOrigin];
+		let remote_instruction = match reason {
+			FeeReason::DepositReserveAsset => ReserveAssetDeposited(to_weigh_reanchored),
+			FeeReason::InitiateReserveWithdraw => WithdrawAsset(to_weigh_reanchored),
+			FeeReason::InitiateTeleport => ReceiveTeleportedAsset(to_weigh_reanchored),
+			_ => {
+				tracing::debug!(
+					target: "xcm::get_delivery_fee_from_holding",
+					"Unexpected transport fee reason",
+				);
+				return Err(XcmError::NotHoldingFees);
+			},
+		};
+		let mut message_to_weigh = Vec::with_capacity(xcm.len() + 2);
+		message_to_weigh.push(remote_instruction);
+		message_to_weigh.push(ClearOrigin);
 		message_to_weigh.extend(xcm.0.clone().into_iter());
 		let (_, fee) =
 			validate_send::<Config::XcmSender>(destination.clone(), Xcm(message_to_weigh))?;
 		let maybe_delivery_fee = fee.get(0).map(|asset_needed_for_fees| {
 			tracing::trace!(
-				target: "xcm::fees::DepositReserveAsset",
+				target: "xcm::fees::get_delivery_fee_from_holding",
 				"Asset provided to pay for fees {:?}, asset required for delivery fees: {:?}",
 				self.asset_used_in_buy_execution, asset_needed_for_fees,
 			);
 			let asset_to_pay_for_fees =
 				self.calculate_asset_for_delivery_fees(asset_needed_for_fees.clone());
 			// set aside fee to be charged by XcmSender
 			let delivery_fee = self.holding.saturating_take(asset_to_pay_for_fees.into());
-			tracing::trace!(target: "xcm::fees::DepositReserveAsset", ?delivery_fee);
+			tracing::trace!(target: "xcm::fees::get_delivery_fee_from_holding", ?delivery_fee);
 			delivery_fee
 		});
 		Ok(maybe_delivery_fee)