From dd9514f7fd0467c067dd06eeaa57f02df2d1fc5b Mon Sep 17 00:00:00 2001 From: jpserrat <35823283+jpserrat@users.noreply.github.com> Date: Mon, 11 Nov 2024 13:54:37 -0300 Subject: [PATCH] add FeeManager to pallet xcm (#5363) Closes #2082 change send xcm to use `xcm::executor::FeeManager` to determine if the sender should be charged. I had to change the `FeeManager` of the penpal config to ensure the same test behaviour as before. For the other tests, I'm using the `FeeManager` from the `xcm::executor::FeeManager` as this one is used to check if the fee can be waived on the charge fees method. --------- Co-authored-by: Adrian Catangiu Co-authored-by: GitHub Action --- .../assets/asset-hub-rococo/src/xcm_config.rs | 2 ++ .../assets/asset-hub-westend/src/xcm_config.rs | 2 ++ .../bridge-hub-rococo/src/xcm_config.rs | 2 ++ .../bridge-hub-westend/src/xcm_config.rs | 2 ++ .../contracts-rococo/src/xcm_config.rs | 2 ++ .../coretime/coretime-rococo/src/xcm_config.rs | 2 ++ .../coretime/coretime-westend/src/xcm_config.rs | 2 ++ .../runtimes/testing/penpal/src/xcm_config.rs | 6 ++++-- polkadot/xcm/pallet-xcm/src/lib.rs | 17 +++++++++-------- polkadot/xcm/xcm-executor/src/lib.rs | 10 ++++++++++ prdoc/pr_5363.prdoc | 14 ++++++++++++++ 11 files changed, 51 insertions(+), 10 deletions(-) create mode 100644 prdoc/pr_5363.prdoc diff --git a/cumulus/parachains/runtimes/assets/asset-hub-rococo/src/xcm_config.rs b/cumulus/parachains/runtimes/assets/asset-hub-rococo/src/xcm_config.rs index 66743fa3a07e..08b2f520c4b9 100644 --- a/cumulus/parachains/runtimes/assets/asset-hub-rococo/src/xcm_config.rs +++ b/cumulus/parachains/runtimes/assets/asset-hub-rococo/src/xcm_config.rs @@ -66,6 +66,7 @@ use xcm_builder::{ use xcm_executor::XcmExecutor; parameter_types! { + pub const RootLocation: Location = Location::here(); pub const TokenLocation: Location = Location::parent(); pub const RelayNetwork: NetworkId = NetworkId::ByGenesis(ROCOCO_GENESIS_HASH); pub RelayChainOrigin: RuntimeOrigin = cumulus_pallet_xcm::Origin::Relay.into(); @@ -315,6 +316,7 @@ pub type ForeignAssetFeeAsExistentialDepositMultiplierFeeCharger = /// either execution or delivery. /// We only waive fees for system functions, which these locations represent. pub type WaivedLocations = ( + Equals, RelayOrOtherSystemParachains, Equals, ); diff --git a/cumulus/parachains/runtimes/assets/asset-hub-westend/src/xcm_config.rs b/cumulus/parachains/runtimes/assets/asset-hub-westend/src/xcm_config.rs index 88ccd42dff7f..b4e938f1f8b5 100644 --- a/cumulus/parachains/runtimes/assets/asset-hub-westend/src/xcm_config.rs +++ b/cumulus/parachains/runtimes/assets/asset-hub-westend/src/xcm_config.rs @@ -63,6 +63,7 @@ use xcm_builder::{ use xcm_executor::XcmExecutor; parameter_types! { + pub const RootLocation: Location = Location::here(); pub const WestendLocation: Location = Location::parent(); pub const RelayNetwork: Option = Some(NetworkId::ByGenesis(WESTEND_GENESIS_HASH)); pub RelayChainOrigin: RuntimeOrigin = cumulus_pallet_xcm::Origin::Relay.into(); @@ -336,6 +337,7 @@ pub type ForeignAssetFeeAsExistentialDepositMultiplierFeeCharger = /// either execution or delivery. /// We only waive fees for system functions, which these locations represent. pub type WaivedLocations = ( + Equals, RelayOrOtherSystemParachains, Equals, FellowshipEntities, diff --git a/cumulus/parachains/runtimes/bridge-hubs/bridge-hub-rococo/src/xcm_config.rs b/cumulus/parachains/runtimes/bridge-hubs/bridge-hub-rococo/src/xcm_config.rs index d36075444f7b..b37945317f6c 100644 --- a/cumulus/parachains/runtimes/bridge-hubs/bridge-hub-rococo/src/xcm_config.rs +++ b/cumulus/parachains/runtimes/bridge-hubs/bridge-hub-rococo/src/xcm_config.rs @@ -57,6 +57,7 @@ use xcm_executor::{ }; parameter_types! { + pub const RootLocation: Location = Location::here(); pub const TokenLocation: Location = Location::parent(); pub RelayChainOrigin: RuntimeOrigin = cumulus_pallet_xcm::Origin::Relay.into(); pub RelayNetwork: NetworkId = NetworkId::ByGenesis(ROCOCO_GENESIS_HASH); @@ -164,6 +165,7 @@ pub type Barrier = TrailingSetTopicAsId< /// either execution or delivery. /// We only waive fees for system functions, which these locations represent. pub type WaivedLocations = ( + Equals, RelayOrOtherSystemParachains, Equals, ); diff --git a/cumulus/parachains/runtimes/bridge-hubs/bridge-hub-westend/src/xcm_config.rs b/cumulus/parachains/runtimes/bridge-hubs/bridge-hub-westend/src/xcm_config.rs index e692568932fe..befb63ef9709 100644 --- a/cumulus/parachains/runtimes/bridge-hubs/bridge-hub-westend/src/xcm_config.rs +++ b/cumulus/parachains/runtimes/bridge-hubs/bridge-hub-westend/src/xcm_config.rs @@ -56,6 +56,7 @@ use xcm_executor::{ }; parameter_types! { + pub const RootLocation: Location = Location::here(); pub const WestendLocation: Location = Location::parent(); pub const RelayNetwork: NetworkId = NetworkId::ByGenesis(WESTEND_GENESIS_HASH); pub RelayChainOrigin: RuntimeOrigin = cumulus_pallet_xcm::Origin::Relay.into(); @@ -161,6 +162,7 @@ pub type Barrier = TrailingSetTopicAsId< /// either execution or delivery. /// We only waive fees for system functions, which these locations represent. pub type WaivedLocations = ( + Equals, RelayOrOtherSystemParachains, Equals, ); diff --git a/cumulus/parachains/runtimes/contracts/contracts-rococo/src/xcm_config.rs b/cumulus/parachains/runtimes/contracts/contracts-rococo/src/xcm_config.rs index 0151837aa351..532ad4ff4ce0 100644 --- a/cumulus/parachains/runtimes/contracts/contracts-rococo/src/xcm_config.rs +++ b/cumulus/parachains/runtimes/contracts/contracts-rococo/src/xcm_config.rs @@ -51,6 +51,7 @@ use xcm_builder::{ use xcm_executor::XcmExecutor; parameter_types! { + pub const RootLocation: Location = Location::here(); pub const RelayLocation: Location = Location::parent(); pub const RelayNetwork: NetworkId = NetworkId::ByGenesis(ROCOCO_GENESIS_HASH); pub RelayChainOrigin: RuntimeOrigin = cumulus_pallet_xcm::Origin::Relay.into(); @@ -166,6 +167,7 @@ pub type Barrier = TrailingSetTopicAsId< /// either execution or delivery. /// We only waive fees for system functions, which these locations represent. pub type WaivedLocations = ( + Equals, RelayOrOtherSystemParachains, Equals, ); diff --git a/cumulus/parachains/runtimes/coretime/coretime-rococo/src/xcm_config.rs b/cumulus/parachains/runtimes/coretime/coretime-rococo/src/xcm_config.rs index 37bf1e681447..33ad172962a1 100644 --- a/cumulus/parachains/runtimes/coretime/coretime-rococo/src/xcm_config.rs +++ b/cumulus/parachains/runtimes/coretime/coretime-rococo/src/xcm_config.rs @@ -52,6 +52,7 @@ use xcm_builder::{ use xcm_executor::XcmExecutor; parameter_types! { + pub const RootLocation: Location = Location::here(); pub const RocRelayLocation: Location = Location::parent(); pub const RelayNetwork: Option = Some(NetworkId::ByGenesis(ROCOCO_GENESIS_HASH)); pub RelayChainOrigin: RuntimeOrigin = cumulus_pallet_xcm::Origin::Relay.into(); @@ -177,6 +178,7 @@ parameter_types! { /// Locations that will not be charged fees in the executor, neither for execution nor delivery. /// We only waive fees for system functions, which these locations represent. pub type WaivedLocations = ( + Equals, RelayOrOtherSystemParachains, Equals, ); diff --git a/cumulus/parachains/runtimes/coretime/coretime-westend/src/xcm_config.rs b/cumulus/parachains/runtimes/coretime/coretime-westend/src/xcm_config.rs index 5616c585a13c..9f38975efae6 100644 --- a/cumulus/parachains/runtimes/coretime/coretime-westend/src/xcm_config.rs +++ b/cumulus/parachains/runtimes/coretime/coretime-westend/src/xcm_config.rs @@ -52,6 +52,7 @@ use xcm_builder::{ use xcm_executor::XcmExecutor; parameter_types! { + pub const RootLocation: Location = Location::here(); pub const TokenRelayLocation: Location = Location::parent(); pub const RelayNetwork: Option = Some(NetworkId::ByGenesis(WESTEND_GENESIS_HASH)); pub RelayChainOrigin: RuntimeOrigin = cumulus_pallet_xcm::Origin::Relay.into(); @@ -185,6 +186,7 @@ parameter_types! { /// Locations that will not be charged fees in the executor, neither for execution nor delivery. /// We only waive fees for system functions, which these locations represent. pub type WaivedLocations = ( + Equals, RelayOrOtherSystemParachains, Equals, ); diff --git a/cumulus/parachains/runtimes/testing/penpal/src/xcm_config.rs b/cumulus/parachains/runtimes/testing/penpal/src/xcm_config.rs index 375c3d509f48..10481d5d2ebc 100644 --- a/cumulus/parachains/runtimes/testing/penpal/src/xcm_config.rs +++ b/cumulus/parachains/runtimes/testing/penpal/src/xcm_config.rs @@ -34,7 +34,7 @@ use core::marker::PhantomData; use frame_support::{ parameter_types, traits::{ - tokens::imbalance::ResolveAssetTo, ConstU32, Contains, ContainsPair, Everything, + tokens::imbalance::ResolveAssetTo, ConstU32, Contains, ContainsPair, Equals, Everything, EverythingBut, Get, Nothing, PalletInfoAccess, }, weights::Weight, @@ -210,6 +210,7 @@ pub type XcmOriginToTransactDispatchOrigin = ( ); parameter_types! { + pub const RootLocation: Location = Location::here(); // One XCM operation is 1_000_000_000 weight - almost certainly a conservative estimate. pub UnitWeightCost: Weight = Weight::from_parts(1_000_000_000, 64 * 1024); pub const MaxInstructions: u32 = 100; @@ -336,6 +337,7 @@ pub type TrustedReserves = ( pub type TrustedTeleporters = (AssetFromChain,); +pub type WaivedLocations = Equals; /// `AssetId`/`Balance` converter for `TrustBackedAssets`. pub type TrustBackedAssetsConvertedConcreteId = assets_common::TrustBackedAssetsConvertedConcreteId; @@ -399,7 +401,7 @@ impl xcm_executor::Config for XcmConfig { type AssetLocker = (); type AssetExchanger = PoolAssetsExchanger; type FeeManager = XcmFeeManagerFromComponents< - (), + WaivedLocations, SendXcmFeeToAccount, >; type MessageExporter = (); diff --git a/polkadot/xcm/pallet-xcm/src/lib.rs b/polkadot/xcm/pallet-xcm/src/lib.rs index 4a97546b38d1..5e0512c6a9fd 100644 --- a/polkadot/xcm/pallet-xcm/src/lib.rs +++ b/polkadot/xcm/pallet-xcm/src/lib.rs @@ -75,6 +75,7 @@ use xcm_runtime_apis::{ #[cfg(any(feature = "try-runtime", test))] use sp_runtime::TryRuntimeError; +use xcm_executor::traits::{FeeManager, FeeReason}; pub trait WeightInfo { fn send() -> Weight; @@ -240,7 +241,7 @@ pub mod pallet { type XcmExecuteFilter: Contains<(Location, Xcm<::RuntimeCall>)>; /// Something to execute an XCM message. - type XcmExecutor: ExecuteXcm<::RuntimeCall> + XcmAssetTransfers; + type XcmExecutor: ExecuteXcm<::RuntimeCall> + XcmAssetTransfers + FeeManager; /// Our XCM filter which messages to be teleported using the dedicated extrinsic must pass. type XcmTeleportFilter: Contains<(Location, Vec)>; @@ -2468,17 +2469,17 @@ impl Pallet { mut message: Xcm<()>, ) -> Result { let interior = interior.into(); + let local_origin = interior.clone().into(); let dest = dest.into(); - let maybe_fee_payer = if interior != Junctions::Here { + let is_waived = + ::is_waived(Some(&local_origin), FeeReason::ChargeFees); + if interior != Junctions::Here { message.0.insert(0, DescendOrigin(interior.clone())); - Some(interior.into()) - } else { - None - }; + } tracing::debug!(target: "xcm::send_xcm", "{:?}, {:?}", dest.clone(), message.clone()); let (ticket, price) = validate_send::(dest, message)?; - if let Some(fee_payer) = maybe_fee_payer { - Self::charge_fees(fee_payer, price).map_err(|e| { + if !is_waived { + Self::charge_fees(local_origin, price).map_err(|e| { tracing::error!( target: "xcm::pallet_xcm::send_xcm", ?e, diff --git a/polkadot/xcm/xcm-executor/src/lib.rs b/polkadot/xcm/xcm-executor/src/lib.rs index a823dc6fec78..e33f94389b21 100644 --- a/polkadot/xcm/xcm-executor/src/lib.rs +++ b/polkadot/xcm/xcm-executor/src/lib.rs @@ -304,6 +304,16 @@ impl XcmAssetTransfers for XcmExecutor { type AssetTransactor = Config::AssetTransactor; } +impl FeeManager for XcmExecutor { + fn is_waived(origin: Option<&Location>, r: FeeReason) -> bool { + Config::FeeManager::is_waived(origin, r) + } + + fn handle_fee(fee: Assets, context: Option<&XcmContext>, r: FeeReason) { + Config::FeeManager::handle_fee(fee, context, r) + } +} + #[derive(Debug)] pub struct ExecutorError { pub index: u32, diff --git a/prdoc/pr_5363.prdoc b/prdoc/pr_5363.prdoc new file mode 100644 index 000000000000..c3ecfffb9e52 --- /dev/null +++ b/prdoc/pr_5363.prdoc @@ -0,0 +1,14 @@ +title: "[pallet-xcm] waive transport fees based on XcmConfig" + +doc: + - audience: Runtime Dev + description: | + pallet-xcm::send() no longer implicitly waives transport fees for the local root location, + but instead relies on xcm_executor::Config::FeeManager to determine whether certain locations have free transport. + + 🚨 Warning: 🚨 If your chain relies on free transport for local root, please make + sure to add Location::here() to the waived-fee locations in your configured xcm_executor::Config::FeeManager. + +crates: + - name: pallet-xcm + bump: major \ No newline at end of file