Skip to content
This repository has been archived by the owner on Nov 15, 2023. It is now read-only.

[pallet_xcm::reserve_transfer_assets] NotHoldingFees issue in case of paid XcmRouter is used #7585

Open
wants to merge 18 commits into
base: kckyeung/xcm-fee-manager
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 10 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
39 changes: 39 additions & 0 deletions runtime/kusama/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2108,6 +2108,8 @@ sp_api::impl_runtime_apis! {
use xcm_config::{
LocalCheckAccount, SovereignAccountOf, AssetHub, TokenLocation, XcmConfig,
};
use xcm_executor::FeesMode;
use xcm_executor::traits::FeeReason;

impl pallet_session_benchmarking::Config for Runtime {}
impl pallet_offences_benchmarking::Config for Runtime {}
Expand All @@ -2130,6 +2132,43 @@ sp_api::impl_runtime_apis! {
fun: Fungible(1_000_000 * UNITS),
}].into()
}
fn ensure_for_send(origin_ref: &MultiLocation, _dest: &MultiLocation, fee_reason: FeeReason) -> Option<FeesMode> {
use xcm_executor::traits::{FeeManager, TransactAsset};

let mut fees_mode = None;
if !<XcmConfig as xcm_executor::Config>::FeeManager::is_waived(Some(origin_ref), fee_reason) {
// if not waived, we need to set up accounts for paying and receiving fees

// mint ED to origin
let ed = MultiAsset {
id: Concrete(TokenLocation::get()),
fun: Fungible(EXISTENTIAL_DEPOSIT.into())
};
<XcmConfig as xcm_executor::Config>::AssetTransactor::deposit_asset(&ed, &origin_ref, None).unwrap();

// overestimate delivery fee
use runtime_common::xcm_sender::PriceForParachainDelivery;
let overestimated_xcm = vec![ClearOrigin; 128].into();
let overestimated_fees = runtime_common::xcm_sender::ExponentialPrice::<
xcm_config::FeeAssetId,
xcm_config::BaseDeliveryFee,
TransactionByteFee,
Dmp
>::price_for_parachain_delivery(
kusama_runtime_constants::system_parachain::ASSET_HUB_ID.into(),
&overestimated_xcm
);

// mint overestimated fee to origin
for fee in overestimated_fees.inner() {
<XcmConfig as xcm_executor::Config>::AssetTransactor::deposit_asset(&fee, &origin_ref, None).unwrap();
}

// expected worst case
fees_mode = Some(FeesMode { jit_withdraw: true });
}
fees_mode
}
bkontur marked this conversation as resolved.
Show resolved Hide resolved
}

parameter_types! {
Expand Down
6 changes: 5 additions & 1 deletion runtime/kusama/src/xcm_config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -146,6 +146,9 @@ match_types! {
pub type OnlyParachains: impl Contains<MultiLocation> = {
MultiLocation { parents: 0, interior: X1(Parachain(_)) }
};
pub type HereLocation: impl Contains<MultiLocation> = {
MultiLocation { parents: 0, interior: Here }
};
}

/// The barriers one of which must be passed for an XCM message to be executed.
Expand Down Expand Up @@ -346,7 +349,8 @@ impl xcm_executor::Config for XcmConfig {
type SubscriptionService = XcmPallet;
type PalletInstancesInfo = AllPalletsWithSystem;
type MaxAssetsIntoHolding = MaxAssetsIntoHolding;
type FeeManager = XcmFeesToAccount<Self, SystemParachains, AccountId, TreasuryAccount>;
type FeeManager =
XcmFeesToAccount<Self, (SystemParachains, HereLocation), AccountId, TreasuryAccount>;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please explain.

// No bridges yet...
type MessageExporter = ();
type UniversalAliases = Nothing;
Expand Down
39 changes: 39 additions & 0 deletions runtime/polkadot/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2085,6 +2085,8 @@ sp_api::impl_runtime_apis! {
use frame_benchmarking::baseline::Pallet as Baseline;
use xcm::latest::prelude::*;
use xcm_config::{XcmConfig, AssetHubLocation, TokenLocation, LocalCheckAccount, SovereignAccountOf};
use xcm_executor::FeesMode;
use xcm_executor::traits::FeeReason;

impl pallet_session_benchmarking::Config for Runtime {}
impl pallet_offences_benchmarking::Config for Runtime {}
Expand All @@ -2108,6 +2110,43 @@ sp_api::impl_runtime_apis! {
// Polkadot only knows about DOT
vec![MultiAsset { id: Concrete(TokenLocation::get()), fun: Fungible(1_000_000 * UNITS) }].into()
}
fn ensure_for_send(origin_ref: &MultiLocation, _dest: &MultiLocation, fee_reason: FeeReason) -> Option<FeesMode> {
use xcm_executor::traits::{FeeManager, TransactAsset};

let mut fees_mode = None;
if !<XcmConfig as xcm_executor::Config>::FeeManager::is_waived(Some(origin_ref), fee_reason) {
// if not waived, we need to set up accounts for paying and receiving fees

// mint ED to origin
let ed = MultiAsset {
id: Concrete(TokenLocation::get()),
fun: Fungible(EXISTENTIAL_DEPOSIT.into())
};
<XcmConfig as xcm_executor::Config>::AssetTransactor::deposit_asset(&ed, &origin_ref, None).unwrap();

// overestimate delivery fee
use runtime_common::xcm_sender::PriceForParachainDelivery;
let overestimated_xcm = vec![ClearOrigin; 128].into();
let overestimated_fees = runtime_common::xcm_sender::ExponentialPrice::<
xcm_config::FeeAssetId,
xcm_config::BaseDeliveryFee,
TransactionByteFee,
Dmp
>::price_for_parachain_delivery(
polkadot_runtime_constants::system_parachain::ASSET_HUB_ID.into(),
&overestimated_xcm
);

// mint overestimated fee to origin
for fee in overestimated_fees.inner() {
<XcmConfig as xcm_executor::Config>::AssetTransactor::deposit_asset(&fee, &origin_ref, None).unwrap();
}

// expected worst case
fees_mode = Some(FeesMode { jit_withdraw: true });
}
fees_mode
}
bkontur marked this conversation as resolved.
Show resolved Hide resolved
}

parameter_types! {
Expand Down
6 changes: 5 additions & 1 deletion runtime/polkadot/src/xcm_config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -157,6 +157,9 @@ match_types! {
MultiLocation { parents: 0, interior: X1(Parachain(COLLECTIVES_ID)) } |
MultiLocation { parents: 0, interior: X2(Parachain(COLLECTIVES_ID), Plurality { id: BodyId::Technical, .. }) }
};
pub type HereLocation: impl Contains<MultiLocation> = {
MultiLocation { parents: 0, interior: Here }
};
}

/// The barriers one of which must be passed for an XCM message to be executed.
Expand Down Expand Up @@ -347,7 +350,8 @@ impl xcm_executor::Config for XcmConfig {
type SubscriptionService = XcmPallet;
type PalletInstancesInfo = AllPalletsWithSystem;
type MaxAssetsIntoHolding = MaxAssetsIntoHolding;
type FeeManager = XcmFeesToAccount<Self, SystemParachains, AccountId, TreasuryAccount>;
type FeeManager =
XcmFeesToAccount<Self, (SystemParachains, HereLocation), AccountId, TreasuryAccount>;
// No bridges yet...
type MessageExporter = ();
type UniversalAliases = Nothing;
Expand Down
39 changes: 39 additions & 0 deletions runtime/rococo/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2094,6 +2094,8 @@ sp_api::impl_runtime_apis! {
use xcm_config::{
LocalCheckAccount, LocationConverter, AssetHub, TokenLocation, XcmConfig,
};
use xcm_executor::FeesMode;
use xcm_executor::traits::FeeReason;

impl frame_system_benchmarking::Config for Runtime {}
impl frame_benchmarking::baseline::Config for Runtime {}
Expand All @@ -2110,6 +2112,43 @@ sp_api::impl_runtime_apis! {
fun: Fungible(1_000_000 * UNITS),
}].into()
}
fn ensure_for_send(origin_ref: &MultiLocation, _dest: &MultiLocation, fee_reason: FeeReason) -> Option<FeesMode> {
use xcm_executor::traits::{FeeManager, TransactAsset};

let mut fees_mode = None;
if !<XcmConfig as xcm_executor::Config>::FeeManager::is_waived(Some(origin_ref), fee_reason) {
// if not waived, we need to set up accounts for paying and receiving fees

// mint ED to origin
let ed = MultiAsset {
id: Concrete(TokenLocation::get()),
fun: Fungible(EXISTENTIAL_DEPOSIT.into())
};
<XcmConfig as xcm_executor::Config>::AssetTransactor::deposit_asset(&ed, &origin_ref, None).unwrap();

// overestimate delivery fee
use runtime_common::xcm_sender::PriceForParachainDelivery;
let overestimated_xcm = vec![ClearOrigin; 128].into();
let overestimated_fees = runtime_common::xcm_sender::ExponentialPrice::<
xcm_config::FeeAssetId,
xcm_config::BaseDeliveryFee,
TransactionByteFee,
Dmp
>::price_for_parachain_delivery(
rococo_runtime_constants::system_parachain::ASSET_HUB_ID.into(),
&overestimated_xcm
);

// mint overestimated fee to origin
for fee in overestimated_fees.inner() {
<XcmConfig as xcm_executor::Config>::AssetTransactor::deposit_asset(&fee, &origin_ref, None).unwrap();
}

// expected worst case
fees_mode = Some(FeesMode { jit_withdraw: true });
}
fees_mode
}
bkontur marked this conversation as resolved.
Show resolved Hide resolved
}

parameter_types! {
Expand Down
6 changes: 5 additions & 1 deletion runtime/rococo/src/xcm_config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -140,6 +140,9 @@ match_types! {
pub type OnlyParachains: impl Contains<MultiLocation> = {
MultiLocation { parents: 0, interior: X1(Parachain(_)) }
};
pub type HereLocation: impl Contains<MultiLocation> = {
MultiLocation { parents: 0, interior: Here }
};
}

/// The barriers one of which must be passed for an XCM message to be executed.
Expand Down Expand Up @@ -319,7 +322,8 @@ impl xcm_executor::Config for XcmConfig {
type SubscriptionService = XcmPallet;
type PalletInstancesInfo = AllPalletsWithSystem;
type MaxAssetsIntoHolding = MaxAssetsIntoHolding;
type FeeManager = XcmFeesToAccount<Self, SystemParachains, AccountId, TreasuryAccount>;
type FeeManager =
XcmFeesToAccount<Self, (SystemParachains, HereLocation), AccountId, TreasuryAccount>;
type MessageExporter = ();
type UniversalAliases = Nothing;
type CallDispatcher = WithOriginFilter<SafeCallFilter>;
Expand Down
6 changes: 6 additions & 0 deletions runtime/westend/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1858,6 +1858,8 @@ sp_api::impl_runtime_apis! {
MultiAsset, MultiAssets, MultiLocation, NetworkId, Response,
};
use xcm_config::{AssetHub, TokenLocation};
use xcm_executor::FeesMode;
use xcm_executor::traits::FeeReason;

impl pallet_xcm_benchmarks::Config for Runtime {
type XcmConfig = xcm_config::XcmConfig;
Expand All @@ -1872,6 +1874,10 @@ sp_api::impl_runtime_apis! {
fun: Fungible(1_000_000 * UNITS),
}].into()
}
fn ensure_for_send(_origin_ref: &MultiLocation, _dest: &MultiLocation, _fee_reason: FeeReason) -> Option<FeesMode> {
// doing nothing
None
}
}

parameter_types! {
Expand Down
11 changes: 9 additions & 2 deletions xcm/pallet-xcm-benchmarks/src/fungible/benchmarking.rs
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ use frame_support::{
use sp_runtime::traits::{Bounded, Zero};
use sp_std::{prelude::*, vec};
use xcm::latest::prelude::*;
use xcm_executor::traits::{ConvertLocation, TransactAsset};
use xcm_executor::traits::{ConvertLocation, FeeReason, TransactAsset};

benchmarks_instance_pallet! {
where_clause { where
Expand Down Expand Up @@ -87,12 +87,19 @@ benchmarks_instance_pallet! {
let dest_location = T::valid_destination()?;
let dest_account = T::AccountIdConverter::convert_location(&dest_location).unwrap();

let expected_fees_mode = T::ensure_for_send(&sender_location, &dest_location, FeeReason::TransferReserveAsset);
let sender_account_balance_before = T::TransactAsset::balance(&sender_account);

let asset = T::get_multi_asset();
<AssetTransactorOf<T>>::deposit_asset(&asset, &sender_location, None).unwrap();
assert!(T::TransactAsset::balance(&sender_account) > sender_account_balance_before);
let assets: MultiAssets = vec![ asset ].into();
assert!(T::TransactAsset::balance(&dest_account).is_zero());

let mut executor = new_executor::<T>(sender_location);
if let Some(expected_fees_mode) = expected_fees_mode {
executor.set_fees_mode(expected_fees_mode);
}
let instruction = Instruction::TransferReserveAsset {
assets,
dest: dest_location,
Expand All @@ -102,7 +109,7 @@ benchmarks_instance_pallet! {
}: {
executor.bench_process(xcm)?;
} verify {
assert!(T::TransactAsset::balance(&sender_account).is_zero());
assert!(T::TransactAsset::balance(&sender_account) <= sender_account_balance_before);
assert!(!T::TransactAsset::balance(&dest_account).is_zero());
// TODO: Check sender queue is not empty. #4426
}
Expand Down
10 changes: 10 additions & 0 deletions xcm/pallet-xcm-benchmarks/src/fungible/mock.rs
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ use sp_core::H256;
use sp_runtime::traits::{BlakeTwo256, IdentityLookup};
use xcm::latest::prelude::*;
use xcm_builder::{AllowUnpaidExecutionFrom, MintLocation};
use xcm_executor::{traits::FeeReason, FeesMode};

type Block = frame_system::mocking::MockBlock<Test>;

Expand Down Expand Up @@ -169,6 +170,15 @@ impl crate::Config for Test {
<XcmConfig as xcm_executor::Config>::MaxAssetsIntoHolding::get(),
)
}

fn ensure_for_send(
_origin_ref: &MultiLocation,
_dest: &MultiLocation,
_fee_reason: FeeReason,
) -> Option<FeesMode> {
// doing nothing
None
}
}

pub type TrustedTeleporters = xcm_builder::Case<TeleportConcreteFungible>;
Expand Down
9 changes: 9 additions & 0 deletions xcm/pallet-xcm-benchmarks/src/generic/mock.rs
Original file line number Diff line number Diff line change
Expand Up @@ -152,6 +152,15 @@ impl crate::Config for Test {
<XcmConfig as xcm_executor::Config>::MaxAssetsIntoHolding::get(),
)
}

fn ensure_for_send(
_origin_ref: &MultiLocation,
_dest: &MultiLocation,
_fee_reason: FeeReason,
) -> Option<FeesMode> {
// doing nothing
None
}
}

impl generic::Config for Test {
Expand Down
13 changes: 12 additions & 1 deletion xcm/pallet-xcm-benchmarks/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,10 @@ use codec::Encode;
use frame_benchmarking::{account, BenchmarkError};
use sp_std::prelude::*;
use xcm::latest::prelude::*;
use xcm_executor::{traits::ConvertLocation, Config as XcmConfig};
use xcm_executor::{
traits::{ConvertLocation, FeeReason},
Config as XcmConfig, FeesMode,
};

pub mod fungible;
pub mod generic;
Expand All @@ -47,6 +50,14 @@ pub trait Config: frame_system::Config {

/// Worst case scenario for a holding account in this runtime.
fn worst_case_holding(depositable_count: u32) -> MultiAssets;

/// Prepare all requirements for successful `XcmSender: SendXcm` passing (accounts, balances ...).
/// Returns possible `FeesMode` which is expected to be set to executor.
fn ensure_for_send(
origin_ref: &MultiLocation,
dest: &MultiLocation,
fee_reason: FeeReason,
) -> Option<FeesMode>;
}

const SEED: u32 = 0;
Expand Down
Loading
Loading