From cdd7b9f484fed691d2c06c56833662422e64fa00 Mon Sep 17 00:00:00 2001
From: Adrian Catangiu <adrian@parity.io>
Date: Mon, 9 Dec 2024 21:58:10 +0200
Subject: [PATCH] xcm-executor: take transport fee from transferred assets if
necessary (#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 https://github.com/paritytech/polkadot-sdk/pull/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 https://github.com/paritytech/polkadot-sdk/issues/4832
Fixes https://github.com/paritytech/polkadot-sdk/issues/6637
---------
Signed-off-by: Adrian Catangiu <adrian@parity.io>
Co-authored-by: Francisco Aguirre <franciscoaguirreperez@gmail.com>
(cherry picked from commit e79fd2bb9926be7d94be0c002676fca57b6116bf)
---
.../primitives/router/src/inbound/mod.rs | 4 +-
.../src/tests/reserve_transfer.rs | 6 +-
.../tests/assets/asset-hub-westend/src/lib.rs | 1 +
.../src/tests/hybrid_transfers.rs | 141 +++++++++++++++++-
.../src/tests/reserve_transfer.rs | 6 +-
.../asset-hub-westend/src/tests/transact.rs | 2 +-
.../src/tests/asset_transfers.rs | 2 +-
.../src/tests/register_bridged_assets.rs | 2 +-
.../bridge-hub-rococo/src/tests/send_xcm.rs | 2 +-
.../src/tests/asset_transfers.rs | 4 +-
.../src/tests/register_bridged_assets.rs | 2 +-
.../bridge-hub-westend/src/tests/send_xcm.rs | 2 +-
.../bridge-hub-westend/src/tests/transact.rs | 2 +-
polkadot/xcm/xcm-executor/src/lib.rs | 83 ++++++++---
prdoc/pr_4834.prdoc | 15 ++
15 files changed, 230 insertions(+), 44 deletions(-)
create mode 100644 prdoc/pr_4834.prdoc
diff --git a/bridges/snowbridge/primitives/router/src/inbound/mod.rs b/bridges/snowbridge/primitives/router/src/inbound/mod.rs
index 756ee7a0459..bc5d401cd4f 100644
--- a/bridges/snowbridge/primitives/router/src/inbound/mod.rs
+++ b/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.
diff --git a/cumulus/parachains/integration-tests/emulated/tests/assets/asset-hub-rococo/src/tests/reserve_transfer.rs b/cumulus/parachains/integration-tests/emulated/tests/assets/asset-hub-rococo/src/tests/reserve_transfer.rs
index 698ef2c9e79..d642e877f00 100644
--- a/cumulus/parachains/integration-tests/emulated/tests/assets/asset-hub-rococo/src/tests/reserve_transfer.rs
+++ b/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 { .. }
) => {},
diff --git a/cumulus/parachains/integration-tests/emulated/tests/assets/asset-hub-westend/src/lib.rs b/cumulus/parachains/integration-tests/emulated/tests/assets/asset-hub-westend/src/lib.rs
index 3cca99fbfe5..36630e2d222 100644
--- a/cumulus/parachains/integration-tests/emulated/tests/assets/asset-hub-westend/src/lib.rs
+++ b/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)]
diff --git a/cumulus/parachains/integration-tests/emulated/tests/assets/asset-hub-westend/src/tests/hybrid_transfers.rs b/cumulus/parachains/integration-tests/emulated/tests/assets/asset-hub-westend/src/tests/hybrid_transfers.rs
index a0fc82fba6e..0686bd71d08 100644
--- a/cumulus/parachains/integration-tests/emulated/tests/assets/asset-hub-westend/src/tests/hybrid_transfers.rs
+++ b/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 },
diff --git a/cumulus/parachains/integration-tests/emulated/tests/assets/asset-hub-westend/src/tests/reserve_transfer.rs b/cumulus/parachains/integration-tests/emulated/tests/assets/asset-hub-westend/src/tests/reserve_transfer.rs
index 558eab13e5c..707e8adc8a5 100644
--- a/cumulus/parachains/integration-tests/emulated/tests/assets/asset-hub-westend/src/tests/reserve_transfer.rs
+++ b/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 { .. }
) => {},
diff --git a/cumulus/parachains/integration-tests/emulated/tests/assets/asset-hub-westend/src/tests/transact.rs b/cumulus/parachains/integration-tests/emulated/tests/assets/asset-hub-westend/src/tests/transact.rs
index 592c2845255..7e881a332a5 100644
--- a/cumulus/parachains/integration-tests/emulated/tests/assets/asset-hub-westend/src/tests/transact.rs
+++ b/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 },
diff --git a/cumulus/parachains/integration-tests/emulated/tests/bridges/bridge-hub-rococo/src/tests/asset_transfers.rs b/cumulus/parachains/integration-tests/emulated/tests/bridges/bridge-hub-rococo/src/tests/asset_transfers.rs
index 33ab1e70b97..a2a61660aff 100644
--- a/cumulus/parachains/integration-tests/emulated/tests/bridges/bridge-hub-rococo/src/tests/asset_transfers.rs
+++ b/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
diff --git a/cumulus/parachains/integration-tests/emulated/tests/bridges/bridge-hub-rococo/src/tests/register_bridged_assets.rs b/cumulus/parachains/integration-tests/emulated/tests/bridges/bridge-hub-rococo/src/tests/register_bridged_assets.rs
index 1ae3a1b1580..70e7a7a3ddd 100644
--- a/cumulus/parachains/integration-tests/emulated/tests/bridges/bridge-hub-rococo/src/tests/register_bridged_assets.rs
+++ b/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
diff --git a/cumulus/parachains/integration-tests/emulated/tests/bridges/bridge-hub-rococo/src/tests/send_xcm.rs b/cumulus/parachains/integration-tests/emulated/tests/bridges/bridge-hub-rococo/src/tests/send_xcm.rs
index 931a3128f82..116ec4dc0e5 100644
--- a/cumulus/parachains/integration-tests/emulated/tests/bridges/bridge-hub-rococo/src/tests/send_xcm.rs
+++ b/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)]);
diff --git a/cumulus/parachains/integration-tests/emulated/tests/bridges/bridge-hub-westend/src/tests/asset_transfers.rs b/cumulus/parachains/integration-tests/emulated/tests/bridges/bridge-hub-westend/src/tests/asset_transfers.rs
index ab09517339d..cc90c10b54b 100644
--- a/cumulus/parachains/integration-tests/emulated/tests/bridges/bridge-hub-westend/src/tests/asset_transfers.rs
+++ b/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
diff --git a/cumulus/parachains/integration-tests/emulated/tests/bridges/bridge-hub-westend/src/tests/register_bridged_assets.rs b/cumulus/parachains/integration-tests/emulated/tests/bridges/bridge-hub-westend/src/tests/register_bridged_assets.rs
index 424f1e55956..952fc35e670 100644
--- a/cumulus/parachains/integration-tests/emulated/tests/bridges/bridge-hub-westend/src/tests/register_bridged_assets.rs
+++ b/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
diff --git a/cumulus/parachains/integration-tests/emulated/tests/bridges/bridge-hub-westend/src/tests/send_xcm.rs b/cumulus/parachains/integration-tests/emulated/tests/bridges/bridge-hub-westend/src/tests/send_xcm.rs
index 787d7dc842c..acce60b4fa7 100644
--- a/cumulus/parachains/integration-tests/emulated/tests/bridges/bridge-hub-westend/src/tests/send_xcm.rs
+++ b/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)]);
diff --git a/cumulus/parachains/integration-tests/emulated/tests/bridges/bridge-hub-westend/src/tests/transact.rs b/cumulus/parachains/integration-tests/emulated/tests/bridges/bridge-hub-westend/src/tests/transact.rs
index 7831c8d6635..f6a3c53c4bf 100644
--- a/cumulus/parachains/integration-tests/emulated/tests/bridges/bridge-hub-westend/src/tests/transact.rs
+++ b/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 },
diff --git a/polkadot/xcm/xcm-executor/src/lib.rs b/polkadot/xcm/xcm-executor/src/lib.rs
index 11fd4e04761..ba7de72b794 100644
--- a/polkadot/xcm/xcm-executor/src/lib.rs
+++ b/polkadot/xcm/xcm-executor/src/lib.rs
@@ -1087,18 +1087,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`, delivery fees are paid from
+ // transferred assets.
+ let maybe_delivery_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 delivery 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),
@@ -1107,7 +1108,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_delivery_fee_from_assets {
// Put back delivery_fee in holding register to be charged by XcmSender.
self.holding.subsume_assets(delivery_fee);
}
@@ -1122,7 +1123,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`, delivery fees are paid from
+ // transferred assets.
+ let maybe_delivery_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 delivery 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,
@@ -1134,6 +1143,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_delivery_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(())
});
@@ -1145,13 +1158,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`, delivery fees are paid from
+ // transferred assets.
+ let maybe_delivery_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 delivery 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_delivery_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(())
});
@@ -1708,36 +1733,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 delivery 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)
diff --git a/prdoc/pr_4834.prdoc b/prdoc/pr_4834.prdoc
new file mode 100644
index 00000000000..b7c8b15cb07
--- /dev/null
+++ b/prdoc/pr_4834.prdoc
@@ -0,0 +1,15 @@
+title: "xcm-executor: take delivery fee from transferred assets if necessary"
+
+doc:
+ - audience: Runtime Dev
+ description: |
+ In asset transfers, as a last resort, XCM delivery fees are taken from
+ transferred assets rather than failing the transfer.
+
+crates:
+ - name: staging-xcm-executor
+ bump: patch
+ - name: snowbridge-router-primitives
+ bump: patch
+ - name: snowbridge-pallet-inbound-queue
+ bump: patch
--
GitLab