Cherry-pick paritytech#3792

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

@@ -1916,6 +1916,7 @@ impl<T: Config> Pallet<T> {
 		]);
 		Ok(Xcm(vec![
 			WithdrawAsset(assets.into()),
+			SetFeesMode { jit_withdraw: true },
 			InitiateReserveWithdraw {
 				assets: Wild(AllCounted(max_assets)),
 				reserve,

polkadot/xcm/pallet-xcm/src/mock.rs

@@ -367,13 +367,24 @@ parameter_types! {
 		parents: 0,
 		interior: X1(Parachain(FOREIGN_ASSET_RESERVE_PARA_ID))
 	};
+	pub PaidParaForeignReserveLocation: MultiLocation = MultiLocation {
+		parents: 0,
+		interior: X1(Parachain(Para3000::get()))
+	};
 	pub const ForeignAsset: MultiAsset = MultiAsset {
 		fun: Fungible(10),
 		id: Concrete(MultiLocation {
 			parents: 0,
 			interior: X2(Parachain(FOREIGN_ASSET_RESERVE_PARA_ID), FOREIGN_ASSET_INNER_JUNCTION),
 		}),
 	};
+	pub PaidParaForeignAsset: MultiAsset = MultiAsset {
+		fun: Fungible(10),
+		id: Concrete(MultiLocation {
+			parents: 0,
+			interior: X1(Parachain(Para3000::get())),
+		}),
+	};
 	pub const UsdcReserveLocation: MultiLocation = MultiLocation {
 		parents: 0,
 		interior: X1(Parachain(USDC_RESERVE_PARA_ID))
@@ -458,6 +469,8 @@ parameter_types! {
 	pub TeleportUsdtToForeign: (MultiAssetFilter, MultiLocation) = (Usdt::get().into(), ForeignReserveLocation::get());
 	pub TrustedForeign: (MultiAssetFilter, MultiLocation) = (ForeignAsset::get().into(), ForeignReserveLocation::get());
 	pub TrustedUsdc: (MultiAssetFilter, MultiLocation) = (Usdc::get().into(), UsdcReserveLocation::get());
+	pub TrustedPaidParaForeign: (MultiAssetFilter, MultiLocation) = (PaidParaForeignAsset::get().into(), PaidParaForeignReserveLocation::get());
+
 	pub const MaxInstructions: u32 = 100;
 	pub const MaxAssetsIntoHolding: u32 = 64;
 	pub XcmFeesTargetAccount: AccountId = AccountId::new([167u8; 32]);
@@ -485,7 +498,7 @@ impl xcm_executor::Config for XcmConfig {
 	type XcmSender = XcmRouter;
 	type AssetTransactor = AssetTransactors;
 	type OriginConverter = LocalOriginConverter;
-	type IsReserve = (Case<TrustedForeign>, Case<TrustedUsdc>);
+	type IsReserve = (Case<TrustedForeign>, Case<TrustedUsdc>, Case<TrustedPaidParaForeign>);
 	type IsTeleporter = (
 		Case<TrustedLocal>,
 		Case<TrustedSystemPara>,

polkadot/xcm/pallet-xcm/src/tests/assets_transfer.rs

@@ -2419,3 +2419,174 @@ fn teleport_assets_using_destination_reserve_fee_disallowed() {
 		expected_result,
 	);
 }
+
+/// Test `tested_call` transferring single asset using remote reserve.
+///
+/// Transferring Para3000 asset (`Para3000` reserve) to
+/// `OTHER_PARA_ID` (no teleport trust), therefore triggering remote reserve.
+/// Using the same asset asset (Para3000 reserve) for fees.
+///
+/// Asserts that the sender's balance is decreased and the beneficiary's balance
+/// is increased. Verifies the correct message is sent and event is emitted.
+///
+/// Verifies that XCM router fees (`SendXcm::validate` -> `Assets`) are withdrawn from correct
+/// user account and deposited to a correct target account (`XcmFeesTargetAccount`).
+/// Verifies `expected_result`.
+fn remote_asset_reserve_and_remote_fee_reserve_paid_call<Call>(
+	tested_call: Call,
+	expected_result: DispatchResult,
+) where
+	Call: FnOnce(
+		OriginFor<Test>,
+		Box<VersionedMultiLocation>,
+		Box<VersionedMultiLocation>,
+		Box<VersionedMultiAssets>,
+		u32,
+		WeightLimit,
+	) -> DispatchResult,
+{
+	let weight = BaseXcmWeight::get() * 3;
+	let user_account = AccountId::from(XCM_FEES_NOT_WAIVED_USER_ACCOUNT);
+	let xcm_router_fee_amount = Para3000PaymentAmount::get();
+	let paid_para_id = Para3000::get();
+	let balances = vec![
+		(user_account.clone(), INITIAL_BALANCE),
+		(ParaId::from(paid_para_id).into_account_truncating(), INITIAL_BALANCE),
+		(XcmFeesTargetAccount::get(), INITIAL_BALANCE),
+	];
+	let beneficiary: MultiLocation = Junction::AccountId32 { network: None, id: ALICE.into() }.into();
+	new_test_ext_with_balances(balances).execute_with(|| {
+		// create sufficient foreign asset BLA
+		let foreign_initial_amount = 142;
+		let (reserve_location, _, foreign_asset_id_location) = set_up_foreign_asset(
+			paid_para_id,
+			None,
+			user_account.clone(),
+			foreign_initial_amount,
+			true,
+		);
+
+		// transfer destination is another chain that is not the reserve location
+		// the goal is to trigger the remoteReserve case
+		let dest = RelayLocation::get().pushed_with_interior(Parachain(OTHER_PARA_ID)).unwrap();
+
+		let transferred_asset: MultiAssets = (foreign_asset_id_location.clone(), SEND_AMOUNT).into();
+
+		// balances checks before
+		assert_eq!(
+			Assets::balance(foreign_asset_id_location.clone(), user_account.clone()),
+			foreign_initial_amount
+		);
+		assert_eq!(Balances::free_balance(user_account.clone()), INITIAL_BALANCE);
+
+		// do the transfer
+		let result = tested_call(
+			RuntimeOrigin::signed(user_account.clone()),
+			Box::new(dest.clone().into()),
+			Box::new(beneficiary.clone().into()),
+			Box::new(transferred_asset.into()),
+			0 as u32,
+			Unlimited,
+		);
+		assert_eq!(result, expected_result);
+		if expected_result.is_err() {
+			// short-circuit here for tests where we expect failure
+			return;
+		}
+
+		let mut last_events = last_events(7).into_iter();
+		// asset events
+		// forceCreate
+		last_events.next().unwrap();
+		// mint tokens
+		last_events.next().unwrap();
+		// burn tokens
+		last_events.next().unwrap();
+		// balance events
+		// burn delivery fee
+		last_events.next().unwrap();
+		// mint delivery fee
+		last_events.next().unwrap();
+		assert_eq!(
+			last_events.next().unwrap(),
+			RuntimeEvent::XcmPallet(crate::Event::Attempted {
+				outcome: Outcome::Complete(weight)
+			})
+		);
+
+		// user account spent (transferred) amount
+		assert_eq!(
+			Assets::balance(foreign_asset_id_location.clone(), user_account.clone()),
+			foreign_initial_amount - SEND_AMOUNT
+		);
+
+		// user account spent delivery fees
+		assert_eq!(Balances::free_balance(user_account), INITIAL_BALANCE - xcm_router_fee_amount);
+
+		// XcmFeesTargetAccount where should lend xcm_router_fee_amount
+		assert_eq!(
+			Balances::free_balance(XcmFeesTargetAccount::get()),
+			INITIAL_BALANCE + xcm_router_fee_amount
+		);
+
+		// Verify total and active issuance of foreign BLA have decreased (burned on
+		// reserve-withdraw)
+		let expected_issuance = foreign_initial_amount - SEND_AMOUNT;
+		assert_eq!(
+			Assets::total_issuance(foreign_asset_id_location.clone()),
+			expected_issuance
+		);
+		assert_eq!(
+			Assets::active_issuance(foreign_asset_id_location.clone()),
+			expected_issuance
+		);
+
+		let context = UniversalLocation::get();
+		let foreign_id_location_reanchored =
+			foreign_asset_id_location.reanchored(&dest, context).unwrap();
+		let dest_reanchored = dest.reanchored(&reserve_location, context).unwrap();
+
+		// Verify sent XCM program
+		assert_eq!(
+			sent_xcm(),
+			vec![(
+				reserve_location,
+				// `assets` are burned on source and withdrawn from SA in remote reserve chain
+				Xcm(vec![
+					WithdrawAsset((MultiLocation::here(), SEND_AMOUNT).into()),
+					ClearOrigin,
+					buy_execution((MultiLocation::here(), SEND_AMOUNT / 2)),
+					DepositReserveAsset {
+						assets: Wild(AllCounted(1)),
+						// final destination is `dest` as seen by `reserve`
+						dest: dest_reanchored,
+						// message sent onward to `dest`
+						xcm: Xcm(vec![
+							buy_execution((foreign_id_location_reanchored, SEND_AMOUNT / 2)),
+							DepositAsset { assets: AllCounted(1).into(), beneficiary }
+						])
+					}
+				])
+			)]
+		);
+	});
+}
+/// Test `transfer_assets` with remote asset reserve and remote fee reserve.
+#[test]
+fn transfer_assets_with_remote_asset_reserve_and_remote_asset_fee_reserve_paid_works() {
+	let expected_result = Ok(());
+	remote_asset_reserve_and_remote_fee_reserve_paid_call(
+		XcmPallet::transfer_assets,
+		expected_result,
+	);
+}
+/// Test `limited_reserve_transfer_assets` with remote asset reserve and remote fee reserve.
+#[test]
+fn limited_reserve_transfer_assets_with_remote_asset_reserve_and_remote_asset_fee_reserve_paid_works(
+) {
+	let expected_result = Ok(());
+	remote_asset_reserve_and_remote_fee_reserve_paid_call(
+		XcmPallet::limited_reserve_transfer_assets,
+		expected_result,
+	);
+}

prdoc/pr_3792.prdoc

@@ -0,0 +1,19 @@
+# Schema: Polkadot SDK PRDoc Schema (prdoc) v1.0.0
+# See doc at https://raw.githubusercontent.com/paritytech/polkadot-sdk/master/prdoc/schema_user.json
+
+title: "[pallet-xcm] fix transport fees for remote reserve transfers"
+
+doc:
+  - audience: Runtime Dev
+    description: |
+      This PR fixes `pallet_xcm::transfer_assets` and
+      `pallet_xcm::limited_reserve_transfer_assets` extrinsics for transfers
+      that need to go through remote reserves. The fix is adding a
+      `SetFeesMode { jit_withdraw: true }` instruction before local execution of
+      `InitiateReserveWithdraw` so that delivery fees are correctly charged by
+      the xcm-executor. Without this change, a runtime that has implemented
+      delivery fees would not be able to execute remote reserve transfers using
+      these extrinsics.
+
+crates:
+  - name: pallet-xcm