pallet-xcm: fix weights for all XTs and deprecate unlimited weight ones (#3927)

Fix "double-weights" for extrinsics, use only the ones benchmarked in
the runtime.

Deprecate extrinsics that don't specify WeightLimit, remove their usage
across the repo.

---------

Signed-off-by: Adrian Catangiu <adrian@parity.io>
Co-authored-by: command-bot <>
This commit is contained in:
Adrian Catangiu
2024-04-02 10:57:35 +03:00
committed by GitHub
parent 9a62de27a9
commit d0ebb850ed
10 changed files with 103 additions and 512 deletions
+15 -116
View File
@@ -940,13 +940,12 @@ pub mod pallet {
}
}
#[pallet::call]
#[pallet::call(weight(<T as Config>::WeightInfo))]
impl<T: Config> Pallet<T> {
/// WARNING: DEPRECATED. `send` will be removed after June 2024. Use `send_blob` instead.
#[allow(deprecated)]
#[deprecated(note = "`send` will be removed after June 2024. Use `send_blob` instead.")]
#[pallet::call_index(0)]
#[pallet::weight(T::WeightInfo::send())]
pub fn send(
origin: OriginFor<T>,
dest: Box<VersionedLocation>,
@@ -976,23 +975,10 @@ pub mod pallet {
/// - `fee_asset_item`: The index into `assets` of the item which should be used to pay
/// fees.
#[pallet::call_index(1)]
#[pallet::weight({
let maybe_assets: Result<Assets, ()> = (*assets.clone()).try_into();
let maybe_dest: Result<Location, ()> = (*dest.clone()).try_into();
match (maybe_assets, maybe_dest) {
(Ok(assets), Ok(dest)) => {
use sp_std::vec;
let count = assets.len() as u32;
let mut message = Xcm(vec![
WithdrawAsset(assets),
SetFeesMode { jit_withdraw: true },
InitiateTeleport { assets: Wild(AllCounted(count)), dest, xcm: Xcm(vec![]) },
]);
T::Weigher::weight(&mut message).map_or(Weight::MAX, |w| T::WeightInfo::teleport_assets().saturating_add(w))
}
_ => Weight::MAX,
}
})]
#[allow(deprecated)]
#[deprecated(
note = "This extrinsic uses `WeightLimit::Unlimited`, please migrate to `limited_teleport_assets` or `transfer_assets`"
)]
pub fn teleport_assets(
origin: OriginFor<T>,
dest: Box<VersionedLocation>,
@@ -1034,23 +1020,10 @@ pub mod pallet {
/// - `fee_asset_item`: The index into `assets` of the item which should be used to pay
/// fees.
#[pallet::call_index(2)]
#[pallet::weight({
let maybe_assets: Result<Assets, ()> = (*assets.clone()).try_into();
let maybe_dest: Result<Location, ()> = (*dest.clone()).try_into();
match (maybe_assets, maybe_dest) {
(Ok(assets), Ok(dest)) => {
use sp_std::vec;
// heaviest version of locally executed XCM program: equivalent in weight to
// transfer assets to SA, reanchor them, extend XCM program, and send onward XCM
let mut message = Xcm(vec![
SetFeesMode { jit_withdraw: true },
TransferReserveAsset { assets, dest, xcm: Xcm(vec![]) }
]);
T::Weigher::weight(&mut message).map_or(Weight::MAX, |w| T::WeightInfo::reserve_transfer_assets().saturating_add(w))
}
_ => Weight::MAX,
}
})]
#[allow(deprecated)]
#[deprecated(
note = "This extrinsic uses `WeightLimit::Unlimited`, please migrate to `limited_reserve_transfer_assets` or `transfer_assets`"
)]
pub fn reserve_transfer_assets(
origin: OriginFor<T>,
dest: Box<VersionedLocation>,
@@ -1102,7 +1075,6 @@ pub mod pallet {
/// - `location`: The destination that is being described.
/// - `xcm_version`: The latest version of XCM that `location` supports.
#[pallet::call_index(4)]
#[pallet::weight(T::WeightInfo::force_xcm_version())]
pub fn force_xcm_version(
origin: OriginFor<T>,
location: Box<Location>,
@@ -1121,7 +1093,6 @@ pub mod pallet {
/// - `origin`: Must be an origin specified by AdminOrigin.
/// - `maybe_xcm_version`: The default XCM encoding version, or `None` to disable.
#[pallet::call_index(5)]
#[pallet::weight(T::WeightInfo::force_default_xcm_version())]
pub fn force_default_xcm_version(
origin: OriginFor<T>,
maybe_xcm_version: Option<XcmVersion>,
@@ -1136,7 +1107,6 @@ pub mod pallet {
/// - `origin`: Must be an origin specified by AdminOrigin.
/// - `location`: The location to which we should subscribe for XCM version notifications.
#[pallet::call_index(6)]
#[pallet::weight(T::WeightInfo::force_subscribe_version_notify())]
pub fn force_subscribe_version_notify(
origin: OriginFor<T>,
location: Box<VersionedLocation>,
@@ -1160,7 +1130,6 @@ pub mod pallet {
/// - `location`: The location to which we are currently subscribed for XCM version
/// notifications which we no longer desire.
#[pallet::call_index(7)]
#[pallet::weight(T::WeightInfo::force_unsubscribe_version_notify())]
pub fn force_unsubscribe_version_notify(
origin: OriginFor<T>,
location: Box<VersionedLocation>,
@@ -1193,7 +1162,7 @@ pub mod pallet {
///
/// Fee payment on the destination side is made from the asset in the `assets` vector of
/// index `fee_asset_item`, up to enough to pay for `weight_limit` of weight. If more weight
/// is needed than `weight_limit`, then the operation will fail and the assets send may be
/// is needed than `weight_limit`, then the operation will fail and the sent assets may be
/// at risk.
///
/// - `origin`: Must be capable of withdrawing the `assets` and executing XCM.
@@ -1208,23 +1177,7 @@ pub mod pallet {
/// fees.
/// - `weight_limit`: The remote-side weight limit, if any, for the XCM fee purchase.
#[pallet::call_index(8)]
#[pallet::weight({
let maybe_assets: Result<Assets, ()> = (*assets.clone()).try_into();
let maybe_dest: Result<Location, ()> = (*dest.clone()).try_into();
match (maybe_assets, maybe_dest) {
(Ok(assets), Ok(dest)) => {
use sp_std::vec;
// heaviest version of locally executed XCM program: equivalent in weight to
// transfer assets to SA, reanchor them, extend XCM program, and send onward XCM
let mut message = Xcm(vec![
SetFeesMode { jit_withdraw: true },
TransferReserveAsset { assets, dest, xcm: Xcm(vec![]) }
]);
T::Weigher::weight(&mut message).map_or(Weight::MAX, |w| T::WeightInfo::reserve_transfer_assets().saturating_add(w))
}
_ => Weight::MAX,
}
})]
#[pallet::weight(T::WeightInfo::reserve_transfer_assets())]
pub fn limited_reserve_transfer_assets(
origin: OriginFor<T>,
dest: Box<VersionedLocation>,
@@ -1247,7 +1200,7 @@ pub mod pallet {
///
/// Fee payment on the destination side is made from the asset in the `assets` vector of
/// index `fee_asset_item`, up to enough to pay for `weight_limit` of weight. If more weight
/// is needed than `weight_limit`, then the operation will fail and the assets send may be
/// is needed than `weight_limit`, then the operation will fail and the sent assets may be
/// at risk.
///
/// - `origin`: Must be capable of withdrawing the `assets` and executing XCM.
@@ -1262,23 +1215,7 @@ pub mod pallet {
/// fees.
/// - `weight_limit`: The remote-side weight limit, if any, for the XCM fee purchase.
#[pallet::call_index(9)]
#[pallet::weight({
let maybe_assets: Result<Assets, ()> = (*assets.clone()).try_into();
let maybe_dest: Result<Location, ()> = (*dest.clone()).try_into();
match (maybe_assets, maybe_dest) {
(Ok(assets), Ok(dest)) => {
use sp_std::vec;
let count = assets.len() as u32;
let mut message = Xcm(vec![
WithdrawAsset(assets),
SetFeesMode { jit_withdraw: true },
InitiateTeleport { assets: Wild(AllCounted(count)), dest, xcm: Xcm(vec![]) },
]);
T::Weigher::weight(&mut message).map_or(Weight::MAX, |w| T::WeightInfo::teleport_assets().saturating_add(w))
}
_ => Weight::MAX,
}
})]
#[pallet::weight(T::WeightInfo::teleport_assets())]
pub fn limited_teleport_assets(
origin: OriginFor<T>,
dest: Box<VersionedLocation>,
@@ -1302,7 +1239,6 @@ pub mod pallet {
/// - `origin`: Must be an origin specified by AdminOrigin.
/// - `suspended`: `true` to suspend, `false` to resume.
#[pallet::call_index(10)]
#[pallet::weight(T::WeightInfo::force_suspension())]
pub fn force_suspension(origin: OriginFor<T>, suspended: bool) -> DispatchResult {
T::AdminOrigin::ensure_origin(origin)?;
XcmExecutionSuspended::<T>::set(suspended);
@@ -1315,7 +1251,7 @@ pub mod pallet {
/// Fee payment on the destination side is made from the asset in the `assets` vector of
/// index `fee_asset_item` (hence referred to as `fees`), up to enough to pay for
/// `weight_limit` of weight. If more weight is needed than `weight_limit`, then the
/// operation will fail and the assets sent may be at risk.
/// operation will fail and the sent assets may be at risk.
///
/// `assets` (excluding `fees`) must have same reserve location or otherwise be teleportable
/// to `dest`, no limitations imposed on `fees`.
@@ -1343,26 +1279,6 @@ pub mod pallet {
/// fees.
/// - `weight_limit`: The remote-side weight limit, if any, for the XCM fee purchase.
#[pallet::call_index(11)]
#[pallet::weight({
let maybe_assets: Result<Assets, ()> = (*assets.clone()).try_into();
let maybe_dest: Result<Location, ()> = (*dest.clone()).try_into();
match (maybe_assets, maybe_dest) {
(Ok(assets), Ok(dest)) => {
use sp_std::vec;
// heaviest version of locally executed XCM program: equivalent in weight to withdrawing fees,
// burning them, transferring rest of assets to SA, reanchoring them, extending XCM program,
// and sending onward XCM
let mut message = Xcm(vec![
SetFeesMode { jit_withdraw: true },
WithdrawAsset(assets.clone()),
BurnAsset(assets.clone()),
TransferReserveAsset { assets, dest, xcm: Xcm(vec![]) }
]);
T::Weigher::weight(&mut message).map_or(Weight::MAX, |w| T::WeightInfo::transfer_assets().saturating_add(w))
}
_ => Weight::MAX,
}
})]
pub fn transfer_assets(
origin: OriginFor<T>,
dest: Box<VersionedLocation>,
@@ -1452,22 +1368,6 @@ pub mod pallet {
/// was the latest when they were trapped.
/// - `beneficiary`: The location/account where the claimed assets will be deposited.
#[pallet::call_index(12)]
#[pallet::weight({
let assets_version = assets.identify_version();
let maybe_assets: Result<Assets, ()> = (*assets.clone()).try_into();
let maybe_beneficiary: Result<Location, ()> = (*beneficiary.clone()).try_into();
match (maybe_assets, maybe_beneficiary) {
(Ok(assets), Ok(beneficiary)) => {
let ticket: Location = GeneralIndex(assets_version as u128).into();
let mut message = Xcm(vec![
ClaimAsset { assets: assets.clone(), ticket },
DepositAsset { assets: AllCounted(assets.len() as u32).into(), beneficiary },
]);
T::Weigher::weight(&mut message).map_or(Weight::MAX, |w| T::WeightInfo::claim_assets().saturating_add(w))
}
_ => Weight::MAX
}
})]
pub fn claim_assets(
origin: OriginFor<T>,
assets: Box<VersionedAssets>,
@@ -1514,7 +1414,7 @@ pub mod pallet {
///
/// The message is passed in encoded. It needs to be decodable as a [`VersionedXcm`].
#[pallet::call_index(13)]
#[pallet::weight(T::WeightInfo::execute_blob())]
#[pallet::weight(max_weight.saturating_add(T::WeightInfo::execute_blob()))]
pub fn execute_blob(
origin: OriginFor<T>,
encoded_message: BoundedVec<u8, MaxXcmEncodedSize>,
@@ -1535,7 +1435,6 @@ pub mod pallet {
///
/// The message is passed in encoded. It needs to be decodable as a [`VersionedXcm`].
#[pallet::call_index(14)]
#[pallet::weight(T::WeightInfo::send_blob())]
pub fn send_blob(
origin: OriginFor<T>,
dest: Box<VersionedLocation>,
@@ -31,13 +31,17 @@ use sp_runtime::{traits::AccountIdConversion, DispatchError, ModuleError};
use xcm::prelude::*;
use xcm_executor::traits::ConvertLocation;
// Helper function to deduplicate testing different teleport types.
fn do_test_and_verify_teleport_assets<Call: FnOnce()>(
origin_location: Location,
expected_beneficiary: Location,
call: Call,
expected_weight_limit: WeightLimit,
) {
/// Test `limited_teleport_assets`
///
/// Asserts that the sender's balance is decreased as a result of execution of
/// local effects.
#[test]
fn limited_teleport_assets_works() {
let origin_location: Location = AccountId32 { network: None, id: ALICE.into() }.into();
let expected_beneficiary: Location = AccountId32 { network: None, id: BOB.into() }.into();
let weight_limit = WeightLimit::Limited(Weight::from_parts(5000, 5000));
let expected_weight_limit = weight_limit.clone();
let balances = vec![
(ALICE, INITIAL_BALANCE),
(ParaId::from(OTHER_PARA_ID).into_account_truncating(), INITIAL_BALANCE),
@@ -47,7 +51,14 @@ fn do_test_and_verify_teleport_assets<Call: FnOnce()>(
let weight = BaseXcmWeight::get() * 2;
assert_eq!(Balances::total_balance(&ALICE), INITIAL_BALANCE);
// call extrinsic
call();
assert_ok!(XcmPallet::limited_teleport_assets(
RuntimeOrigin::signed(ALICE),
Box::new(RelayLocation::get().into()),
Box::new(expected_beneficiary.clone().into()),
Box::new((Here, SEND_AMOUNT).into()),
0,
weight_limit,
));
assert_eq!(Balances::total_balance(&ALICE), INITIAL_BALANCE - SEND_AMOUNT);
assert_eq!(
sent_xcm(),
@@ -88,57 +99,6 @@ fn do_test_and_verify_teleport_assets<Call: FnOnce()>(
});
}
/// Test `teleport_assets`
///
/// Asserts that the sender's balance is decreased as a result of execution of
/// local effects.
#[test]
fn teleport_assets_works() {
let origin_location: Location = AccountId32 { network: None, id: ALICE.into() }.into();
let beneficiary: Location = AccountId32 { network: None, id: BOB.into() }.into();
do_test_and_verify_teleport_assets(
origin_location.clone(),
beneficiary.clone(),
|| {
assert_ok!(XcmPallet::teleport_assets(
RuntimeOrigin::signed(ALICE),
Box::new(RelayLocation::get().into()),
Box::new(beneficiary.into()),
Box::new((Here, SEND_AMOUNT).into()),
0,
));
},
Unlimited,
);
}
/// Test `limited_teleport_assets`
///
/// Asserts that the sender's balance is decreased as a result of execution of
/// local effects.
#[test]
fn limited_teleport_assets_works() {
let origin_location: Location = AccountId32 { network: None, id: ALICE.into() }.into();
let beneficiary: Location = AccountId32 { network: None, id: BOB.into() }.into();
let weight_limit = WeightLimit::Limited(Weight::from_parts(5000, 5000));
let expected_weight_limit = weight_limit.clone();
do_test_and_verify_teleport_assets(
origin_location.clone(),
beneficiary.clone(),
|| {
assert_ok!(XcmPallet::limited_teleport_assets(
RuntimeOrigin::signed(ALICE),
Box::new(RelayLocation::get().into()),
Box::new(beneficiary.into()),
Box::new((Here, SEND_AMOUNT).into()),
0,
weight_limit,
));
},
expected_weight_limit,
);
}
/// `limited_teleport_assets` should fail for filtered assets
#[test]
fn limited_teleport_filtered_assets_disallowed() {
@@ -184,12 +144,13 @@ fn reserve_transfer_assets_with_paid_router_works() {
let dest: Location =
Junction::AccountId32 { network: None, id: user_account.clone().into() }.into();
assert_eq!(Balances::total_balance(&user_account), INITIAL_BALANCE);
assert_ok!(XcmPallet::reserve_transfer_assets(
assert_ok!(XcmPallet::limited_reserve_transfer_assets(
RuntimeOrigin::signed(user_account.clone()),
Box::new(Parachain(paid_para_id).into()),
Box::new(dest.clone().into()),
Box::new((Here, SEND_AMOUNT).into()),
0,
Unlimited,
));
// XCM_FEES_NOT_WAIVED_USER_ACCOUNT spent amount