Unverified Commit 67fd660f authored by Gavin Wood's avatar Gavin Wood Committed by GitHub
Browse files

Avoid bad pattern of wildcard fees (#3578)

* Avoid bad pattern of wildcard fees

* spelling

* fmt

* Fix runtimes

* spelling
parent 5d462ca2
Pipeline #151370 passed with stages
in 39 minutes and 21 seconds
...@@ -1380,6 +1380,7 @@ impl pallet_xcm::Config for Runtime { ...@@ -1380,6 +1380,7 @@ impl pallet_xcm::Config for Runtime {
type XcmTeleportFilter = All<(MultiLocation, Vec<MultiAsset>)>; type XcmTeleportFilter = All<(MultiLocation, Vec<MultiAsset>)>;
type XcmReserveTransferFilter = All<(MultiLocation, Vec<MultiAsset>)>; type XcmReserveTransferFilter = All<(MultiLocation, Vec<MultiAsset>)>;
type Weigher = FixedWeightBounds<BaseXcmWeight, Call>; type Weigher = FixedWeightBounds<BaseXcmWeight, Call>;
type LocationInverter = LocationInverter<Ancestry>;
} }
parameter_types! { parameter_types! {
......
...@@ -755,6 +755,7 @@ impl pallet_xcm::Config for Runtime { ...@@ -755,6 +755,7 @@ impl pallet_xcm::Config for Runtime {
type XcmTeleportFilter = All<(MultiLocation, Vec<MultiAsset>)>; type XcmTeleportFilter = All<(MultiLocation, Vec<MultiAsset>)>;
type XcmReserveTransferFilter = All<(MultiLocation, Vec<MultiAsset>)>; type XcmReserveTransferFilter = All<(MultiLocation, Vec<MultiAsset>)>;
type Weigher = FixedWeightBounds<BaseXcmWeight, Call>; type Weigher = FixedWeightBounds<BaseXcmWeight, Call>;
type LocationInverter = LocationInverter<Ancestry>;
} }
impl parachains_session_info::Config for Runtime {} impl parachains_session_info::Config for Runtime {}
......
...@@ -1012,6 +1012,7 @@ impl pallet_xcm::Config for Runtime { ...@@ -1012,6 +1012,7 @@ impl pallet_xcm::Config for Runtime {
type XcmTeleportFilter = All<(MultiLocation, Vec<MultiAsset>)>; type XcmTeleportFilter = All<(MultiLocation, Vec<MultiAsset>)>;
type XcmReserveTransferFilter = All<(MultiLocation, Vec<MultiAsset>)>; type XcmReserveTransferFilter = All<(MultiLocation, Vec<MultiAsset>)>;
type Weigher = FixedWeightBounds<BaseXcmWeight, Call>; type Weigher = FixedWeightBounds<BaseXcmWeight, Call>;
type LocationInverter = LocationInverter<Ancestry>;
} }
construct_runtime! { construct_runtime! {
......
...@@ -38,7 +38,7 @@ pub mod pallet { ...@@ -38,7 +38,7 @@ pub mod pallet {
use frame_support::pallet_prelude::*; use frame_support::pallet_prelude::*;
use frame_system::pallet_prelude::*; use frame_system::pallet_prelude::*;
use sp_runtime::traits::AccountIdConversion; use sp_runtime::traits::AccountIdConversion;
use xcm_executor::traits::WeightBounds; use xcm_executor::traits::{InvertLocation, WeightBounds};
#[pallet::pallet] #[pallet::pallet]
#[pallet::generate_store(pub(super) trait Store)] #[pallet::generate_store(pub(super) trait Store)]
...@@ -76,6 +76,9 @@ pub mod pallet { ...@@ -76,6 +76,9 @@ pub mod pallet {
/// Means of measuring the weight consumed by an XCM message locally. /// Means of measuring the weight consumed by an XCM message locally.
type Weigher: WeightBounds<Self::Call>; type Weigher: WeightBounds<Self::Call>;
/// Means of inverting a location.
type LocationInverter: InvertLocation;
} }
#[pallet::event] #[pallet::event]
...@@ -93,6 +96,10 @@ pub mod pallet { ...@@ -93,6 +96,10 @@ pub mod pallet {
Filtered, Filtered,
/// The message's weight could not be determined. /// The message's weight could not be determined.
UnweighableMessage, UnweighableMessage,
/// The assets to be sent are empty.
Empty,
/// Could not reanchor the assets to declare the fees for the destination chain.
CannotReanchor,
} }
#[pallet::hooks] #[pallet::hooks]
...@@ -115,6 +122,8 @@ pub mod pallet { ...@@ -115,6 +122,8 @@ pub mod pallet {
/// Teleport some assets from the local chain to some destination chain. /// Teleport some assets from the local chain to some destination chain.
/// ///
/// Fee payment on the destination side is made from the first asset listed in the `assets` vector.
///
/// - `origin`: Must be capable of withdrawing the `assets` and executing XCM. /// - `origin`: Must be capable of withdrawing the `assets` and executing XCM.
/// - `dest`: Destination context for the assets. Will typically be `X2(Parent, Parachain(..))` to send /// - `dest`: Destination context for the assets. Will typically be `X2(Parent, Parachain(..))` to send
/// from parachain to parachain, or `X1(Parachain(..))` to send from relay to parachain. /// from parachain to parachain, or `X1(Parachain(..))` to send from relay to parachain.
...@@ -146,6 +155,9 @@ pub mod pallet { ...@@ -146,6 +155,9 @@ pub mod pallet {
let value = (origin_location, assets); let value = (origin_location, assets);
ensure!(T::XcmTeleportFilter::contains(&value), Error::<T>::Filtered); ensure!(T::XcmTeleportFilter::contains(&value), Error::<T>::Filtered);
let (origin_location, assets) = value; let (origin_location, assets) = value;
let inv_dest = T::LocationInverter::invert_location(&dest);
let mut fees = assets.first().ok_or(Error::<T>::Empty)?.clone();
fees.reanchor(&inv_dest).map_err(|_| Error::<T>::CannotReanchor)?;
let mut message = Xcm::WithdrawAsset { let mut message = Xcm::WithdrawAsset {
assets, assets,
effects: vec![InitiateTeleport { effects: vec![InitiateTeleport {
...@@ -153,7 +165,7 @@ pub mod pallet { ...@@ -153,7 +165,7 @@ pub mod pallet {
dest, dest,
effects: vec![ effects: vec![
BuyExecution { BuyExecution {
fees: All, fees,
// Zero weight for additional XCM (since there are none to execute) // Zero weight for additional XCM (since there are none to execute)
weight: 0, weight: 0,
debt: dest_weight, debt: dest_weight,
...@@ -175,6 +187,8 @@ pub mod pallet { ...@@ -175,6 +187,8 @@ pub mod pallet {
/// Transfer some assets from the local chain to the sovereign account of a destination chain and forward /// Transfer some assets from the local chain to the sovereign account of a destination chain and forward
/// a notification XCM. /// a notification XCM.
/// ///
/// Fee payment on the destination side is made from the first asset listed in the `assets` vector.
///
/// - `origin`: Must be capable of withdrawing the `assets` and executing XCM. /// - `origin`: Must be capable of withdrawing the `assets` and executing XCM.
/// - `dest`: Destination context for the assets. Will typically be `X2(Parent, Parachain(..))` to send /// - `dest`: Destination context for the assets. Will typically be `X2(Parent, Parachain(..))` to send
/// from parachain to parachain, or `X1(Parachain(..))` to send from relay to parachain. /// from parachain to parachain, or `X1(Parachain(..))` to send from relay to parachain.
...@@ -203,12 +217,15 @@ pub mod pallet { ...@@ -203,12 +217,15 @@ pub mod pallet {
let value = (origin_location, assets); let value = (origin_location, assets);
ensure!(T::XcmReserveTransferFilter::contains(&value), Error::<T>::Filtered); ensure!(T::XcmReserveTransferFilter::contains(&value), Error::<T>::Filtered);
let (origin_location, assets) = value; let (origin_location, assets) = value;
let inv_dest = T::LocationInverter::invert_location(&dest);
let mut fees = assets.first().ok_or(Error::<T>::Empty)?.clone();
fees.reanchor(&inv_dest).map_err(|_| Error::<T>::CannotReanchor)?;
let mut message = Xcm::TransferReserveAsset { let mut message = Xcm::TransferReserveAsset {
assets, assets,
dest, dest,
effects: vec![ effects: vec![
BuyExecution { BuyExecution {
fees: All, fees,
// Zero weight for additional XCM (since there are none to execute) // Zero weight for additional XCM (since there are none to execute)
weight: 0, weight: 0,
debt: dest_weight, debt: dest_weight,
......
...@@ -186,6 +186,7 @@ impl pallet_xcm::Config for Test { ...@@ -186,6 +186,7 @@ impl pallet_xcm::Config for Test {
type XcmTeleportFilter = All<(MultiLocation, Vec<MultiAsset>)>; type XcmTeleportFilter = All<(MultiLocation, Vec<MultiAsset>)>;
type XcmReserveTransferFilter = All<(MultiLocation, Vec<MultiAsset>)>; type XcmReserveTransferFilter = All<(MultiLocation, Vec<MultiAsset>)>;
type Weigher = FixedWeightBounds<BaseXcmWeight, Call>; type Weigher = FixedWeightBounds<BaseXcmWeight, Call>;
type LocationInverter = LocationInverter<Ancestry>;
} }
impl origin::Config for Test {} impl origin::Config for Test {}
...@@ -194,9 +195,9 @@ pub(crate) fn last_event() -> Event { ...@@ -194,9 +195,9 @@ pub(crate) fn last_event() -> Event {
System::events().pop().expect("Event expected").event System::events().pop().expect("Event expected").event
} }
pub(crate) fn buy_execution<C>(debt: Weight) -> Order<C> { pub(crate) fn buy_execution<C>(debt: Weight, fees: MultiAsset) -> Order<C> {
use xcm::opaque::v0::prelude::*; use xcm::opaque::v0::prelude::*;
Order::BuyExecution { fees: All, weight: 0, debt, halt_on_error: false, xcm: vec![] } Order::BuyExecution { fees, weight: 0, debt, halt_on_error: false, xcm: vec![] }
} }
pub(crate) fn new_test_ext_with_balances( pub(crate) fn new_test_ext_with_balances(
......
...@@ -42,7 +42,10 @@ fn send_works() { ...@@ -42,7 +42,10 @@ fn send_works() {
let message = Xcm::ReserveAssetDeposit { let message = Xcm::ReserveAssetDeposit {
assets: vec![ConcreteFungible { id: Parent.into(), amount: SEND_AMOUNT }], assets: vec![ConcreteFungible { id: Parent.into(), amount: SEND_AMOUNT }],
effects: vec![ effects: vec![
buy_execution(weight), buy_execution(
weight,
ConcreteFungible { id: MultiLocation::Null, amount: SEND_AMOUNT },
),
DepositAsset { assets: vec![All], dest: sender.clone() }, DepositAsset { assets: vec![All], dest: sender.clone() },
], ],
}; };
...@@ -76,7 +79,10 @@ fn send_fails_when_xcm_router_blocks() { ...@@ -76,7 +79,10 @@ fn send_fails_when_xcm_router_blocks() {
let message = Xcm::ReserveAssetDeposit { let message = Xcm::ReserveAssetDeposit {
assets: vec![ConcreteFungible { id: Parent.into(), amount: SEND_AMOUNT }], assets: vec![ConcreteFungible { id: Parent.into(), amount: SEND_AMOUNT }],
effects: vec![ effects: vec![
buy_execution(weight), buy_execution(
weight,
ConcreteFungible { id: MultiLocation::Null, amount: SEND_AMOUNT },
),
DepositAsset { assets: vec![All], dest: sender.clone() }, DepositAsset { assets: vec![All], dest: sender.clone() },
], ],
}; };
...@@ -157,7 +163,13 @@ fn reserve_transfer_assets_works() { ...@@ -157,7 +163,13 @@ fn reserve_transfer_assets_works() {
Parachain(PARA_ID).into(), Parachain(PARA_ID).into(),
Xcm::ReserveAssetDeposit { Xcm::ReserveAssetDeposit {
assets: vec![ConcreteFungible { id: Parent.into(), amount: SEND_AMOUNT }], assets: vec![ConcreteFungible { id: Parent.into(), amount: SEND_AMOUNT }],
effects: vec![buy_execution(weight), DepositAsset { assets: vec![All], dest },] effects: vec![
buy_execution(
weight,
ConcreteFungible { id: Parent.into(), amount: SEND_AMOUNT }
),
DepositAsset { assets: vec![All], dest },
]
} }
)] )]
); );
...@@ -185,7 +197,13 @@ fn execute_withdraw_to_deposit_works() { ...@@ -185,7 +197,13 @@ fn execute_withdraw_to_deposit_works() {
Origin::signed(ALICE), Origin::signed(ALICE),
Box::new(Xcm::WithdrawAsset { Box::new(Xcm::WithdrawAsset {
assets: vec![ConcreteFungible { id: MultiLocation::Null, amount: SEND_AMOUNT }], assets: vec![ConcreteFungible { id: MultiLocation::Null, amount: SEND_AMOUNT }],
effects: vec![buy_execution(weight), DepositAsset { assets: vec![All], dest }], effects: vec![
buy_execution(
weight,
ConcreteFungible { id: MultiLocation::Null, amount: SEND_AMOUNT }
),
DepositAsset { assets: vec![All], dest },
],
}), }),
weight weight
)); ));
......
...@@ -187,7 +187,7 @@ pub mod mock_msg_queue { ...@@ -187,7 +187,7 @@ pub mod mock_msg_queue {
#[pallet::generate_deposit(pub(super) fn deposit_event)] #[pallet::generate_deposit(pub(super) fn deposit_event)]
pub enum Event<T: Config> { pub enum Event<T: Config> {
// XCMP // XCMP
/// Some XCM was executed ok. /// Some XCM was executed OK.
Success(Option<T::Hash>), Success(Option<T::Hash>),
/// Some XCM failed. /// Some XCM failed.
Fail(Option<T::Hash>, XcmError), Fail(Option<T::Hash>, XcmError),
...@@ -302,6 +302,7 @@ impl pallet_xcm::Config for Runtime { ...@@ -302,6 +302,7 @@ impl pallet_xcm::Config for Runtime {
type XcmTeleportFilter = (); type XcmTeleportFilter = ();
type XcmReserveTransferFilter = All<(MultiLocation, Vec<MultiAsset>)>; type XcmReserveTransferFilter = All<(MultiLocation, Vec<MultiAsset>)>;
type Weigher = FixedWeightBounds<UnitWeightCost, Call>; type Weigher = FixedWeightBounds<UnitWeightCost, Call>;
type LocationInverter = LocationInverter<Ancestry>;
} }
type UncheckedExtrinsic = frame_system::mocking::MockUncheckedExtrinsic<Runtime>; type UncheckedExtrinsic = frame_system::mocking::MockUncheckedExtrinsic<Runtime>;
......
...@@ -148,6 +148,7 @@ impl pallet_xcm::Config for Runtime { ...@@ -148,6 +148,7 @@ impl pallet_xcm::Config for Runtime {
type XcmTeleportFilter = All<(MultiLocation, Vec<MultiAsset>)>; type XcmTeleportFilter = All<(MultiLocation, Vec<MultiAsset>)>;
type XcmReserveTransferFilter = All<(MultiLocation, Vec<MultiAsset>)>; type XcmReserveTransferFilter = All<(MultiLocation, Vec<MultiAsset>)>;
type Weigher = FixedWeightBounds<BaseXcmWeight, Call>; type Weigher = FixedWeightBounds<BaseXcmWeight, Call>;
type LocationInverter = LocationInverter<Ancestry>;
} }
parameter_types! { parameter_types! {
......
Supports Markdown
0% or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment