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

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,18 +1086,19 @@ 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)?
+					let mut assets = self.holding.saturating_take(assets);
+					// When not using `PayFees`, nor `JIT_WITHDRAW`, transport fees are paid from
+					// transferred assets.
+					let maybe_transport_fee_from_assets = if self.fees.is_empty() && !self.fees_mode.jit_withdraw {
+						// Deduct and return the part of `assets` that shall be used for transport fees.
+						self.take_delivery_fee_from_assets(&mut 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);
-					tracing::trace!(target: "xcm::DepositReserveAsset", ?deposited, "Assets except delivery fee");
+					tracing::trace!(target: "xcm::DepositReserveAsset", ?assets, "Assets except delivery fee");
 					Self::do_reserve_deposit_assets(
-						deposited,
+						assets,
 						&dest,
 						&mut message,
 						Some(&self.context),
@@ -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_assets {
 						// Put back delivery_fee in holding register to be charged by XcmSender.
 						self.holding.subsume_assets(delivery_fee);
 					}
@@ -1121,7 +1122,15 @@ impl<Config: config::Config> XcmExecutor<Config> {
 			InitiateReserveWithdraw { assets, reserve, xcm } => {
 				let old_holding = self.holding.clone();
 				let result = Config::TransactionalProcessor::process(|| {
-					let assets = self.holding.saturating_take(assets);
+					let mut assets = self.holding.saturating_take(assets);
+					// When not using `PayFees`, nor `JIT_WITHDRAW`, transport fees are paid from
+					// transferred assets.
+					let maybe_transport_fee_from_assets = if self.fees.is_empty() && !self.fees_mode.jit_withdraw {
+						// Deduct and return the part of `assets` that shall be used for transport fees.
+						self.take_delivery_fee_from_assets(&mut assets, &reserve, FeeReason::InitiateReserveWithdraw, &xcm)?
+					} else {
+						None
+					};
 					let mut message = Vec::with_capacity(xcm.len() + 2);
 					Self::do_reserve_withdraw_assets(
 						assets,
@@ -1133,6 +1142,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_assets {
+						// 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 +1157,25 @@ impl<Config: config::Config> XcmExecutor<Config> {
 			InitiateTeleport { assets, dest, xcm } => {
 				let old_holding = self.holding.clone();
 				let result = Config::TransactionalProcessor::process(|| {
-					let assets = self.holding.saturating_take(assets);
+					let mut assets = self.holding.saturating_take(assets);
+					// When not using `PayFees`, nor `JIT_WITHDRAW`, transport fees are paid from
+					// transferred assets.
+					let maybe_transport_fee_from_assets = if self.fees.is_empty() && !self.fees_mode.jit_withdraw {
+						// Deduct and return the part of `assets` that shall be used for transport fees.
+						self.take_delivery_fee_from_assets(&mut assets, &dest, FeeReason::InitiateTeleport, &xcm)?
+					} else {
+						None
+					};
 					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_assets {
+						// 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 +1732,48 @@ impl<Config: config::Config> XcmExecutor<Config> {
 		Ok(())
 	}
 
-	/// Gets the necessary delivery fee to send a reserve transfer message to `destination` from
-	/// holding.
+	/// Take from transferred `assets` the delivery fee required 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,
+	fn take_delivery_fee_from_assets(
+		&self,
+		assets: &mut AssetsInHolding,
 		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 = assets.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::take_delivery_fee_from_assets",
+					"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::take_delivery_fee_from_assets",
 				"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);
+			let delivery_fee = assets.saturating_take(asset_to_pay_for_fees.into());
+			tracing::trace!(target: "xcm::fees::take_delivery_fee_from_assets", ?delivery_fee);
 			delivery_fee
 		});
 		Ok(maybe_delivery_fee)

prdoc/pr_4834.prdoc

@@ -0,0 +1,11 @@
+title: "xcm-executor: take transport fee from transferred assets if necessary"
+
+doc:
+  - audience: Runtime Dev
+    description: |
+      In asset transfers, as a last resort, XCM transport fees are taken from
+      transferred assets rather than failing the transfer.
+
+crates:
+  - name: staging-xcm-executor
+    bump: patch