Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

add FeeManager to pallet xcm #5363

Merged
Merged
Show file tree
Hide file tree
Changes from 4 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
Original file line number Diff line number Diff line change
Expand Up @@ -485,6 +485,7 @@ impl pallet_xcm::Config for Runtime {
type AdminOrigin = EnsureRoot<AccountId>;
type MaxRemoteLockConsumers = ConstU32<0>;
type RemoteLockConsumerIdentifier = ();
type FeeManager = <XcmConfig as xcm_executor::Config>::FeeManager;
}

impl cumulus_pallet_xcm::Config for Runtime {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -509,6 +509,7 @@ impl pallet_xcm::Config for Runtime {
type AdminOrigin = EnsureRoot<AccountId>;
type MaxRemoteLockConsumers = ConstU32<0>;
type RemoteLockConsumerIdentifier = ();
type FeeManager = <XcmConfig as xcm_executor::Config>::FeeManager;
}

impl cumulus_pallet_xcm::Config for Runtime {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -287,6 +287,7 @@ impl pallet_xcm::Config for Runtime {
type AdminOrigin = EnsureRoot<AccountId>;
type MaxRemoteLockConsumers = ConstU32<0>;
type RemoteLockConsumerIdentifier = ();
type FeeManager = <XcmConfig as xcm_executor::Config>::FeeManager;
}

impl cumulus_pallet_xcm::Config for Runtime {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -273,6 +273,7 @@ impl pallet_xcm::Config for Runtime {
type AdminOrigin = EnsureRoot<AccountId>;
type MaxRemoteLockConsumers = ConstU32<0>;
type RemoteLockConsumerIdentifier = ();
type FeeManager = <XcmConfig as xcm_executor::Config>::FeeManager;
}

impl cumulus_pallet_xcm::Config for Runtime {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -273,6 +273,7 @@ impl pallet_xcm::Config for Runtime {
type AdminOrigin = EnsureRoot<AccountId>;
type MaxRemoteLockConsumers = ConstU32<0>;
type RemoteLockConsumerIdentifier = ();
type FeeManager = <XcmConfig as xcm_executor::Config>::FeeManager;
}

impl cumulus_pallet_xcm::Config for Runtime {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -249,6 +249,7 @@ impl pallet_xcm::Config for Runtime {
type AdminOrigin = EnsureRoot<AccountId>;
type MaxRemoteLockConsumers = ConstU32<0>;
type RemoteLockConsumerIdentifier = ();
type FeeManager = <XcmConfig as xcm_executor::Config>::FeeManager;
}

impl cumulus_pallet_xcm::Config for Runtime {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -273,6 +273,7 @@ impl pallet_xcm::Config for Runtime {
type AdminOrigin = EnsureRoot<AccountId>;
type MaxRemoteLockConsumers = ConstU32<0>;
type RemoteLockConsumerIdentifier = ();
type FeeManager = <XcmConfig as xcm_executor::Config>::FeeManager;
}

impl cumulus_pallet_xcm::Config for Runtime {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -281,6 +281,7 @@ impl pallet_xcm::Config for Runtime {
type AdminOrigin = EnsureRoot<AccountId>;
type MaxRemoteLockConsumers = ConstU32<0>;
type RemoteLockConsumerIdentifier = ();
type FeeManager = <XcmConfig as xcm_executor::Config>::FeeManager;
}

impl cumulus_pallet_xcm::Config for Runtime {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -276,6 +276,7 @@ impl pallet_xcm::Config for Runtime {
type AdminOrigin = EnsureRoot<AccountId>;
type MaxRemoteLockConsumers = ConstU32<0>;
type RemoteLockConsumerIdentifier = ();
type FeeManager = <XcmConfig as xcm_executor::Config>::FeeManager;
}

impl cumulus_pallet_xcm::Config for Runtime {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -284,6 +284,7 @@ impl pallet_xcm::Config for Runtime {
type AdminOrigin = EnsureRoot<AccountId>;
type MaxRemoteLockConsumers = ConstU32<0>;
type RemoteLockConsumerIdentifier = ();
type FeeManager = <XcmConfig as xcm_executor::Config>::FeeManager;
}

impl cumulus_pallet_xcm::Config for Runtime {
Expand Down
9 changes: 8 additions & 1 deletion cumulus/parachains/runtimes/testing/penpal/src/xcm_config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -316,6 +316,12 @@ pub type TrustedReserves = (
pub type TrustedTeleporters =
(AssetFromChain<LocalTeleportableToAssetHub, SystemAssetHubLocation>,);

pub struct WaivedLocations;
impl Contains<Location> for WaivedLocations {
fn contains(location: &Location) -> bool {
*location == Location::here()
}
}
Copy link
Contributor

Choose a reason for hiding this comment

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

There is already something you can use here:

Suggested change
pub struct WaivedLocations;
impl Contains<Location> for WaivedLocations {
fn contains(location: &Location) -> bool {
*location == Location::here()
}
}
pub type WaivedLocations = Equals<RootLocation>;

Also please make sure all the other system chain runtimes have Equals<RootLocation> part of their WaivedLocations so as not to break functionality.

Hint: they currently do not, for example collectives-westend has it, make sure the others do too.

pub struct XcmConfig;
impl xcm_executor::Config for XcmConfig {
type RuntimeCall = RuntimeCall;
Expand Down Expand Up @@ -354,7 +360,7 @@ impl xcm_executor::Config for XcmConfig {
type AssetLocker = ();
type AssetExchanger = ();
type FeeManager = XcmFeeManagerFromComponents<
(),
WaivedLocations,
SendXcmFeeToAccount<Self::AssetTransactor, TreasuryAccount>,
>;
type MessageExporter = ();
Expand Down Expand Up @@ -419,6 +425,7 @@ impl pallet_xcm::Config for Runtime {
type AdminOrigin = EnsureRoot<AccountId>;
type MaxRemoteLockConsumers = ConstU32<0>;
type RemoteLockConsumerIdentifier = ();
type FeeManager = <XcmConfig as xcm_executor::Config>::FeeManager;
}

impl cumulus_pallet_xcm::Config for Runtime {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -531,6 +531,7 @@ impl pallet_xcm::Config for Runtime {
type AdminOrigin = EnsureRoot<AccountId>;
type MaxRemoteLockConsumers = ConstU32<0>;
type RemoteLockConsumerIdentifier = ();
type FeeManager = <XcmConfig as xcm_executor::Config>::FeeManager;
}

impl cumulus_pallet_xcm::Config for Runtime {
Expand Down
1 change: 1 addition & 0 deletions polkadot/runtime/rococo/src/xcm_config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -287,4 +287,5 @@ impl pallet_xcm::Config for Runtime {
type RemoteLockConsumerIdentifier = ();
type WeightInfo = crate::weights::pallet_xcm::WeightInfo<Runtime>;
type AdminOrigin = EnsureRoot<AccountId>;
type FeeManager = <XcmConfig as xcm_executor::Config>::FeeManager;
}
1 change: 1 addition & 0 deletions polkadot/runtime/test-runtime/src/xcm_config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -185,4 +185,5 @@ impl pallet_xcm::Config for crate::Runtime {
type RemoteLockConsumerIdentifier = ();
type WeightInfo = pallet_xcm::TestWeightInfo;
type AdminOrigin = EnsureRoot<crate::AccountId>;
type FeeManager = <XcmConfig as xcm_executor::Config>::FeeManager;
}
1 change: 1 addition & 0 deletions polkadot/runtime/westend/src/xcm_config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -301,4 +301,5 @@ impl pallet_xcm::Config for Runtime {
type RemoteLockConsumerIdentifier = ();
type WeightInfo = crate::weights::pallet_xcm::WeightInfo<Runtime>;
type AdminOrigin = EnsureRoot<AccountId>;
type FeeManager = <XcmConfig as xcm_executor::Config>::FeeManager;
}
Original file line number Diff line number Diff line change
Expand Up @@ -152,7 +152,7 @@ impl pallet_xcm::Config for Runtime {
// We turn off sending for these tests
type SendXcmOrigin = EnsureXcmOrigin<RuntimeOrigin, ()>;
type XcmRouter = super::super::network::ParachainXcmRouter<MessageQueue>; // Provided by xcm-simulator
// Anyone can execute XCM programs
// Anyone can execute XCM programs
type ExecuteXcmOrigin = EnsureXcmOrigin<RuntimeOrigin, LocalOriginToLocation>;
// We execute any type of program
type XcmExecuteFilter = Everything;
Expand Down Expand Up @@ -186,4 +186,5 @@ impl pallet_xcm::Config for Runtime {
type RuntimeOrigin = RuntimeOrigin;
type RuntimeCall = RuntimeCall;
type RuntimeEvent = RuntimeEvent;
type FeeManager = <XcmConfig as xcm_executor::Config>::FeeManager;
}
Original file line number Diff line number Diff line change
Expand Up @@ -125,7 +125,7 @@ impl pallet_xcm::Config for Runtime {
// No one can call `send`
type SendXcmOrigin = EnsureXcmOrigin<RuntimeOrigin, ()>;
type XcmRouter = super::super::network::RelayChainXcmRouter; // Provided by xcm-simulator
// Anyone can execute XCM programs
// Anyone can execute XCM programs
type ExecuteXcmOrigin = EnsureXcmOrigin<RuntimeOrigin, LocalOriginToLocation>;
// We execute any type of program
type XcmExecuteFilter = Everything;
Expand Down Expand Up @@ -160,4 +160,5 @@ impl pallet_xcm::Config for Runtime {
type RuntimeOrigin = RuntimeOrigin;
type RuntimeCall = RuntimeCall;
type RuntimeEvent = RuntimeEvent;
type FeeManager = <XcmConfig as xcm_executor::Config>::FeeManager;
}
26 changes: 14 additions & 12 deletions polkadot/xcm/pallet-xcm/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -61,9 +61,9 @@ use xcm_builder::{
use xcm_executor::{
traits::{
AssetTransferError, CheckSuspension, ClaimAssets, ConvertLocation, ConvertOrigin,
DropAssets, MatchesFungible, OnResponse, Properties, QueryHandler, QueryResponseStatus,
RecordXcm, TransactAsset, TransferType, VersionChangeNotifier, WeightBounds,
XcmAssetTransfers,
DropAssets, FeeManager, FeeReason, MatchesFungible, OnResponse, Properties, QueryHandler,
QueryResponseStatus, RecordXcm, TransactAsset, TransferType, VersionChangeNotifier,
WeightBounds, XcmAssetTransfers,
},
AssetsInHolding,
};
Expand Down Expand Up @@ -192,7 +192,7 @@ pub mod pallet {
use frame_system::Config as SysConfig;
use sp_core::H256;
use sp_runtime::traits::Dispatchable;
use xcm_executor::traits::{MatchesFungible, WeightBounds};
use xcm_executor::traits::{FeeManager, MatchesFungible, WeightBounds};

parameter_types! {
/// An implementation of `Get<u32>` which just returns the latest XCM version which we can
Expand Down Expand Up @@ -292,6 +292,9 @@ pub mod pallet {

/// Weight information for extrinsics in this pallet.
type WeightInfo: WeightInfo;

/// Configure the fees.
type FeeManager: FeeManager;
Copy link
Contributor

Choose a reason for hiding this comment

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

I am in favor of re-exposing FeeManager from XcmExecutor type instead of creating yet another configuration type here.

I don't see why pallet-xcm would be configured with fee manager different than the xcm-executor. The benefit outweighs the overhead cost of configuring it twice and thus also the risk of misconfiguring it.

type XcmExecutor here can take a trait FeeManager bound.

xcm-executor implementation can impl FeeManager where it would simply re-expose its inner configured type FeeManager.

This will help with ergonomics and allow most runtimes to ingest this change with no breakages and needs to adjust configs.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done!

}

impl<T: Config> ExecuteControllerWeightInfo for Pallet<T> {
Expand Down Expand Up @@ -2421,17 +2424,16 @@ impl<T: Config> Pallet<T> {
mut message: Xcm<()>,
) -> Result<XcmHash, SendError> {
let interior = interior.into();
let origin_dest = interior.clone().into();
Copy link
Contributor

Choose a reason for hiding this comment

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

This is the origin as seen by the local chain, relative to Here.

Suggested change
let origin_dest = interior.clone().into();
let local_origin = interior.clone().into();

let dest = dest.into();
let maybe_fee_payer = if interior != Junctions::Here {
message.0.insert(0, DescendOrigin(interior.clone()));
Some(interior.into())
} else {
None
};
let is_waived = T::FeeManager::is_waived(Some(&origin_dest), FeeReason::ChargeFees);
if !is_waived {
message.0.insert(0, DescendOrigin(interior));
}
Copy link
Contributor

Choose a reason for hiding this comment

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

This is not really what we want.

Whether or not DescendOrigin(interior) is inserted is orthogonal to whether fees are paid; it should depend on the same condition as before:

Suggested change
if !is_waived {
message.0.insert(0, DescendOrigin(interior));
}
if interior != Junctions::Here {
message.0.insert(0, DescendOrigin(interior));
}

log::debug!(target: "xcm::send_xcm", "dest: {:?}, message: {:?}", &dest, &message);
let (ticket, price) = validate_send::<T::XcmRouter>(dest, message)?;
if let Some(fee_payer) = maybe_fee_payer {
Self::charge_fees(fee_payer, price).map_err(|_| SendError::Fees)?;
if !is_waived {
Self::charge_fees(origin_dest, price).map_err(|_| SendError::Fees)?;
}
Copy link
Contributor

Choose a reason for hiding this comment

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

This condition is good! 👍

Just rename the var

Suggested change
if !is_waived {
Self::charge_fees(origin_dest, price).map_err(|_| SendError::Fees)?;
}
if !is_waived {
Self::charge_fees(local_origin, price).map_err(|_| SendError::Fees)?;
}

T::XcmRouter::deliver(ticket)
}
Expand Down
1 change: 1 addition & 0 deletions polkadot/xcm/pallet-xcm/src/mock.rs
Original file line number Diff line number Diff line change
Expand Up @@ -556,6 +556,7 @@ impl pallet_xcm::Config for Test {
type MaxRemoteLockConsumers = frame_support::traits::ConstU32<0>;
type RemoteLockConsumerIdentifier = ();
type WeightInfo = TestWeightInfo;
type FeeManager = <XcmConfig as xcm_executor::Config>::FeeManager;
}

impl origin::Config for Test {}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -330,6 +330,7 @@ impl pallet_xcm::Config for Runtime {
type RuntimeOrigin = RuntimeOrigin;
type RuntimeCall = RuntimeCall;
type RuntimeEvent = RuntimeEvent;
type FeeManager = <XcmConfig as xcm_executor::Config>::FeeManager;
}

pub const INITIAL_BALANCE: Balance = 1_000_000_000;
Expand Down
1 change: 1 addition & 0 deletions polkadot/xcm/xcm-builder/src/tests/pay/mock.rs
Original file line number Diff line number Diff line change
Expand Up @@ -268,6 +268,7 @@ impl pallet_xcm::Config for Test {
type RemoteLockConsumerIdentifier = ();
type WeightInfo = pallet_xcm::TestWeightInfo;
type AdminOrigin = EnsureRoot<AccountId>;
type FeeManager = <XcmConfig as xcm_executor::Config>::FeeManager;
}

pub const UNITS: Balance = 1_000_000_000_000;
Expand Down
1 change: 1 addition & 0 deletions polkadot/xcm/xcm-builder/tests/mock/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -224,6 +224,7 @@ impl pallet_xcm::Config for Runtime {
type RemoteLockConsumerIdentifier = ();
type WeightInfo = pallet_xcm::TestWeightInfo;
type AdminOrigin = EnsureRoot<AccountId>;
type FeeManager = <XcmConfig as xcm_executor::Config>::FeeManager;
}

impl origin::Config for Runtime {}
Expand Down
1 change: 1 addition & 0 deletions polkadot/xcm/xcm-runtime-apis/tests/mock.rs
Original file line number Diff line number Diff line change
Expand Up @@ -351,6 +351,7 @@ impl pallet_xcm::Config for TestRuntime {
type MaxRemoteLockConsumers = ConstU32<0>;
type RemoteLockConsumerIdentifier = ();
type WeightInfo = TestWeightInfo;
type FeeManager = <XcmConfig as xcm_executor::Config>::FeeManager;
}

#[allow(dead_code)]
Expand Down
1 change: 1 addition & 0 deletions polkadot/xcm/xcm-simulator/example/src/parachain/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -166,6 +166,7 @@ impl pallet_xcm::Config for Runtime {
type RemoteLockConsumerIdentifier = ();
type WeightInfo = pallet_xcm::TestWeightInfo;
type AdminOrigin = EnsureRoot<AccountId>;
type FeeManager = <XcmConfig as xcm_executor::Config>::FeeManager;
}

type Block = frame_system::mocking::MockBlock<Runtime>;
Expand Down
1 change: 1 addition & 0 deletions polkadot/xcm/xcm-simulator/example/src/relay_chain/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -119,6 +119,7 @@ impl pallet_xcm::Config for Runtime {
type RemoteLockConsumerIdentifier = ();
type WeightInfo = pallet_xcm::TestWeightInfo;
type AdminOrigin = EnsureRoot<AccountId>;
type FeeManager = <XcmConfig as xcm_executor::Config>::FeeManager;
}

impl origin::Config for Runtime {}
Expand Down
1 change: 1 addition & 0 deletions polkadot/xcm/xcm-simulator/fuzzer/src/parachain.rs
Original file line number Diff line number Diff line change
Expand Up @@ -334,6 +334,7 @@ impl pallet_xcm::Config for Runtime {
type RemoteLockConsumerIdentifier = ();
type WeightInfo = pallet_xcm::TestWeightInfo;
type AdminOrigin = EnsureRoot<AccountId>;
type FeeManager = <XcmConfig as xcm_executor::Config>::FeeManager;
}

construct_runtime!(
Expand Down
1 change: 1 addition & 0 deletions polkadot/xcm/xcm-simulator/fuzzer/src/relay_chain.rs
Original file line number Diff line number Diff line change
Expand Up @@ -179,6 +179,7 @@ impl pallet_xcm::Config for Runtime {
type RemoteLockConsumerIdentifier = ();
type WeightInfo = pallet_xcm::TestWeightInfo;
type AdminOrigin = EnsureRoot<AccountId>;
type FeeManager = <XcmConfig as xcm_executor::Config>::FeeManager;
}

impl origin::Config for Runtime {}
Expand Down
1 change: 1 addition & 0 deletions substrate/frame/contracts/mock-network/src/parachain.rs
Original file line number Diff line number Diff line change
Expand Up @@ -321,6 +321,7 @@ impl pallet_xcm::Config for Runtime {
type RemoteLockConsumerIdentifier = ();
type WeightInfo = pallet_xcm::TestWeightInfo;
type AdminOrigin = EnsureRoot<AccountId>;
type FeeManager = <XcmConfig as xcm_executor::Config>::FeeManager;
}

type Block = frame_system::mocking::MockBlock<Runtime>;
Expand Down
1 change: 1 addition & 0 deletions substrate/frame/contracts/mock-network/src/relay_chain.rs
Original file line number Diff line number Diff line change
Expand Up @@ -209,6 +209,7 @@ impl pallet_xcm::Config for Runtime {
type RemoteLockConsumerIdentifier = ();
type WeightInfo = pallet_xcm::TestWeightInfo;
type AdminOrigin = EnsureRoot<AccountId>;
type FeeManager = <XcmConfig as xcm_executor::Config>::FeeManager;
}

impl origin::Config for Runtime {}
Expand Down
1 change: 1 addition & 0 deletions templates/parachain/runtime/src/configs/xcm_config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -185,6 +185,7 @@ impl pallet_xcm::Config for Runtime {
type AdminOrigin = EnsureRoot<AccountId>;
type MaxRemoteLockConsumers = ConstU32<0>;
type RemoteLockConsumerIdentifier = ();
type FeeManager = <XcmConfig as xcm_executor::Config>::FeeManager;
}

impl cumulus_pallet_xcm::Config for Runtime {
Expand Down
Loading