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.

Sign up for GitHub

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

Allow configuration of worst case buy execution weight #7944

Open
wants to merge 15 commits into
base: master
Choose a base branch
from
+89 −45
Open

Allow configuration of worst case buy execution weight #7944

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
15 commits
Select commit Hold shift + click to select a range
ce2d375
Allow to set a worst case buy execution fee asset and weight
girazoki Jan 17, 2024
e58b767
Merge remote-tracking branch 'origin/master' into girazoki-worst-case…
girazoki Feb 19, 2024
0b426c7
add prdoc
girazoki Feb 19, 2024
c982b26
remove default impl
girazoki Feb 19, 2024
04d2623
Merge remote-tracking branch 'origin/master' into girazoki-worst-case…
girazoki Mar 17, 2025
b9b9b11
reopen and merging back
girazoki Mar 17, 2025
bb54f66
adapt pr number
girazoki Mar 17, 2025
ddafc1e
generate worst case for all cases, and include it as part of fee asset
girazoki Mar 24, 2025
aebfc30
add limited weight for benchmarks
girazoki Mar 25, 2025
b8dc917
remove non-used
girazoki Mar 25, 2025
d8492ce
sanitize prdoc
girazoki Mar 26, 2025
5e63326
fmt
girazoki Mar 26, 2025
c8c4cc0
adapt prdoc
girazoki Mar 28, 2025
a45f7f7
prdoc cleanup again
girazoki Mar 28, 2025
2df61db
Merge branch 'master' into girazoki-worst-case-buy-execution-weight
girazoki Mar 31, 2025
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
8 changes: 4 additions & 4 deletions cumulus/parachains/runtimes/assets/asset-hub-rococo/src/lib.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -101,7 +101,7 @@ use polkadot_runtime_common::{BlockHashCount, SlowAdjustingFeeUpdate};
#[cfg(feature = "runtime-benchmarks")]
use xcm::latest::prelude::{
Asset, Assets as XcmAssets, Fungible, Here, InteriorLocation, Junction, Junction::*, Location,
NetworkId, NonFungible, Parent, ParentThen, Response, XCM_VERSION,
NetworkId, NonFungible, Parent, ParentThen, Response, WeightLimit, XCM_VERSION,
};
use xcm::{
latest::prelude::{AssetId, BodyId},
Expand Down Expand Up @@ -1989,11 +1989,11 @@ impl_runtime_apis! {
Ok((origin, ticket, assets))
}

fn fee_asset() -> Result<Asset, BenchmarkError> {
Ok(Asset {
fn worst_case_for_trader() -> Result<(Asset, WeightLimit), BenchmarkError> {
Ok((Asset {
id: AssetId(TokenLocation::get()),
fun: Fungible(1_000_000 * UNITS),
})
}, WeightLimit::Limited(Weight::from_parts(5000, 5000))))
}

fn unlockable_asset() -> Result<(Location, Location, Asset), BenchmarkError> {
Expand Down
8 changes: 4 additions & 4 deletions cumulus/parachains/runtimes/assets/asset-hub-westend/src/lib.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -112,7 +112,7 @@ use frame_support::traits::PalletInfoAccess;
#[cfg(feature = "runtime-benchmarks")]
use xcm::latest::prelude::{
Asset, Assets as XcmAssets, Fungible, Here, InteriorLocation, Junction, Junction::*, Location,
NetworkId, NonFungible, Parent, ParentThen, Response, XCM_VERSION,
NetworkId, NonFungible, Parent, ParentThen, Response, WeightLimit, XCM_VERSION,
};

use xcm_runtime_apis::{
Expand Down Expand Up @@ -2236,11 +2236,11 @@ impl_runtime_apis! {
Ok((origin, ticket, assets))
}

fn fee_asset() -> Result<Asset, BenchmarkError> {
Ok(Asset {
fn worst_case_for_trader() -> Result<(Asset, WeightLimit), BenchmarkError> {
Ok((Asset {
id: AssetId(WestendLocation::get()),
fun: Fungible(1_000 * UNITS),
})
}, WeightLimit::Limited(Weight::from_parts(5000, 5000))))
}

fn unlockable_asset() -> Result<(Location, Location, Asset), BenchmarkError> {
Expand Down
6 changes: 3 additions & 3 deletions cumulus/parachains/runtimes/bridge-hubs/bridge-hub-rococo/src/lib.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1247,11 +1247,11 @@ impl_runtime_apis! {
Ok((origin, ticket, assets))
}

fn fee_asset() -> Result<Asset, BenchmarkError> {
Ok(Asset {
fn worst_case_for_trader() -> Result<(Asset, WeightLimit), BenchmarkError> {
Ok((Asset {
id: AssetId(TokenLocation::get()),
fun: Fungible(1_000_000 * UNITS),
})
}, WeightLimit::Limited(Weight::from_parts(5000, 5000))))
}

fn unlockable_asset() -> Result<(Location, Location, Asset), BenchmarkError> {
Expand Down
6 changes: 3 additions & 3 deletions cumulus/parachains/runtimes/bridge-hubs/bridge-hub-westend/src/lib.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1163,11 +1163,11 @@ impl_runtime_apis! {
Ok((origin, ticket, assets))
}

fn fee_asset() -> Result<Asset, BenchmarkError> {
Ok(Asset {
fn worst_case_for_trader() -> Result<(Asset, WeightLimit), BenchmarkError> {
Ok((Asset {
id: AssetId(WestendLocation::get()),
fun: Fungible(1_000_000 * UNITS),
})
}, WeightLimit::Limited(Weight::from_parts(5000, 5000))))
}

fn unlockable_asset() -> Result<(Location, Location, Asset), BenchmarkError> {
Expand Down
6 changes: 3 additions & 3 deletions cumulus/parachains/runtimes/collectives/collectives-westend/src/lib.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1246,11 +1246,11 @@ impl_runtime_apis! {
Ok((origin, ticket, assets))
}

fn fee_asset() -> Result<Asset, BenchmarkError> {
Ok(Asset {
fn worst_case_for_trader() -> Result<(Asset, WeightLimit), BenchmarkError> {
Ok((Asset {
id: AssetId(WndLocation::get()),
fun: Fungible(1_000_000 * UNITS),
})
}, WeightLimit::Limited(Weight::from_parts(5000, 5000))))
}

fn unlockable_asset() -> Result<(Location, Location, Asset), BenchmarkError> {
Expand Down
6 changes: 3 additions & 3 deletions cumulus/parachains/runtimes/coretime/coretime-rococo/src/lib.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1133,11 +1133,11 @@ impl_runtime_apis! {
Ok((origin, ticket, assets))
}

fn fee_asset() -> Result<Asset, BenchmarkError> {
Ok(Asset {
fn worst_case_for_trader() -> Result<(Asset, WeightLimit), BenchmarkError> {
Ok((Asset {
id: AssetId(RocRelayLocation::get()),
fun: Fungible(1_000_000 * UNITS),
})
}, WeightLimit::Limited(Weight::from_parts(5000, 5000))))
}

fn unlockable_asset() -> Result<(Location, Location, Asset), BenchmarkError> {
Expand Down
6 changes: 3 additions & 3 deletions cumulus/parachains/runtimes/coretime/coretime-westend/src/lib.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1135,11 +1135,11 @@ impl_runtime_apis! {
Ok((origin, ticket, assets))
}

fn fee_asset() -> Result<Asset, BenchmarkError> {
Ok(Asset {
fn worst_case_for_trader() -> Result<(Asset, WeightLimit), BenchmarkError> {
Ok((Asset {
id: AssetId(TokenRelayLocation::get()),
fun: Fungible(1_000_000 * UNITS),
})
}, WeightLimit::Limited(Weight::from_parts(5000, 5000))))
}

fn unlockable_asset() -> Result<(Location, Location, Asset), BenchmarkError> {
Expand Down
6 changes: 3 additions & 3 deletions cumulus/parachains/runtimes/people/people-rococo/src/lib.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1048,11 +1048,11 @@ impl_runtime_apis! {
Ok((origin, ticket, assets))
}

fn fee_asset() -> Result<Asset, BenchmarkError> {
Ok(Asset {
fn worst_case_for_trader() -> Result<(Asset, WeightLimit), BenchmarkError> {
Ok((Asset {
id: AssetId(RelayLocation::get()),
fun: Fungible(1_000_000 * UNITS),
})
}, WeightLimit::Limited(Weight::from_parts(5000, 5000))))
}

fn unlockable_asset() -> Result<(Location, Location, Asset), BenchmarkError> {
Expand Down
6 changes: 3 additions & 3 deletions cumulus/parachains/runtimes/people/people-westend/src/lib.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1046,11 +1046,11 @@ impl_runtime_apis! {
Ok((origin, ticket, assets))
}

fn fee_asset() -> Result<Asset, BenchmarkError> {
Ok(Asset {
fn worst_case_for_trader() -> Result<(Asset, WeightLimit), BenchmarkError> {
Ok((Asset {
id: AssetId(RelayLocation::get()),
fun: Fungible(1_000_000 * UNITS),
})
}, WeightLimit::Limited(Weight::from_parts(5000, 5000))))
}

fn unlockable_asset() -> Result<(Location, Location, Asset), BenchmarkError> {
Expand Down
6 changes: 3 additions & 3 deletions polkadot/runtime/rococo/src/lib.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2645,11 +2645,11 @@ sp_api::impl_runtime_apis! {
Ok((origin, ticket, assets))
}

fn fee_asset() -> Result<Asset, BenchmarkError> {
Ok(Asset {
fn worst_case_for_trader() -> Result<(Asset, WeightLimit), BenchmarkError> {
Ok((Asset {
id: AssetId(TokenLocation::get()),
fun: Fungible(1_000_000 * UNITS),
})
}, WeightLimit::Limited(Weight::from_parts(5000, 5000))))
}

fn unlockable_asset() -> Result<(Location, Location, Asset), BenchmarkError> {
Expand Down
6 changes: 3 additions & 3 deletions polkadot/runtime/westend/src/lib.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2867,11 +2867,11 @@ sp_api::impl_runtime_apis! {
Ok((origin, ticket, assets))
}

fn fee_asset() -> Result<Asset, BenchmarkError> {
Ok(Asset {
fn worst_case_for_trader() -> Result<(Asset, WeightLimit), BenchmarkError> {
Ok((Asset {
id: AssetId(TokenLocation::get()),
fun: Fungible(1_000_000 * UNITS),
})
}, WeightLimit::Limited(Weight::from_parts(5000, 5000))))
}

fn unlockable_asset() -> Result<(Location, Location, Asset), BenchmarkError> {
Expand Down
13 changes: 8 additions & 5 deletions polkadot/xcm/pallet-xcm-benchmarks/src/generic/benchmarking.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -102,11 +102,13 @@ mod benchmarks {
let mut executor = new_executor::<T>(Default::default());
executor.set_holding(holding);

let fee_asset = AssetId(Here.into());
// The worst case we want for buy execution in terms of
// fee asset and weight
let (fee_asset, weight_limit) = T::worst_case_for_trader()?;

let instruction = Instruction::<XcmCallOf<T>>::BuyExecution {
fees: (fee_asset, 100_000_000u128).into(), // should be something inside of holding
weight_limit: WeightLimit::Unlimited,
fees: fee_asset,
weight_limit: weight_limit.into(),
};

let xcm = Xcm(vec![instruction]);
Expand All @@ -127,7 +129,7 @@ mod benchmarks {
// Set some weight to be paid for.
executor.set_message_weight(Weight::from_parts(100_000_000, 100_000));

let fee_asset: Asset = T::fee_asset().unwrap();
let (fee_asset, _): (Asset, WeightLimit) = T::worst_case_for_trader().unwrap();

let instruction = Instruction::<XcmCallOf<T>>::PayFees { asset: fee_asset };

Expand Down Expand Up @@ -207,7 +209,8 @@ mod benchmarks {
let mut executor = new_executor::<T>(Default::default());
let holding_assets = T::worst_case_holding(1);
// We can already buy execution since we'll load the holding register manually
let asset_for_fees = T::fee_asset().unwrap();
let (asset_for_fees, _): (Asset, WeightLimit) = T::worst_case_for_trader().unwrap();

let previous_xcm = Xcm(vec![BuyExecution {
fees: asset_for_fees,
weight_limit: Limited(Weight::from_parts(1337, 1337)),
Expand Down
4 changes: 2 additions & 2 deletions polkadot/xcm/pallet-xcm-benchmarks/src/generic/mock.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -173,8 +173,8 @@ impl generic::Config for Test {
Ok((Default::default(), ticket, assets))
}

fn fee_asset() -> Result<Asset, BenchmarkError> {
Ok(Asset { id: AssetId(Here.into()), fun: Fungible(1_000_000) })
fn worst_case_for_trader() -> Result<(Asset, WeightLimit), BenchmarkError> {
Ok((Asset { id: AssetId(Here.into()), fun: Fungible(1_000_000) }, WeightLimit::Unlimited))
}

fn unlockable_asset() -> Result<(Location, Location, Asset), BenchmarkError> {
Expand Down
10 changes: 7 additions & 3 deletions polkadot/xcm/pallet-xcm-benchmarks/src/generic/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,9 @@ pub mod pallet {
use frame_benchmarking::BenchmarkError;
use frame_support::{dispatch::GetDispatchInfo, pallet_prelude::Encode};
use sp_runtime::traits::Dispatchable;
use xcm::latest::{Asset, Assets, InteriorLocation, Junction, Location, NetworkId, Response};
use xcm::latest::{
Asset, Assets, InteriorLocation, Junction, Location, NetworkId, Response, WeightLimit,
};

#[pallet::config]
pub trait Config<I: 'static = ()>: frame_system::Config + crate::Config {
Expand Down Expand Up @@ -73,9 +75,11 @@ pub mod pallet {
/// Return an origin, ticket, and assets that can be trapped and claimed.
fn claimable_asset() -> Result<(Location, Location, Assets), BenchmarkError>;

/// Asset used to pay for fees. Used to buy weight in benchmarks, for example in
/// The worst case buy execution weight limit and
/// asset to trigger the Trader::buy_execution in the XCM executor
/// Used to buy weight in benchmarks, for example in
/// `refund_surplus`.
fn fee_asset() -> Result<Asset, BenchmarkError>;
fn worst_case_for_trader() -> Result<(Asset, WeightLimit), BenchmarkError>;

/// Return an unlocker, owner and assets that can be locked and unlocked.
fn unlockable_asset() -> Result<(Location, Location, Asset), BenchmarkError>;
Expand Down
37 changes: 37 additions & 0 deletions prdoc/pr_7944.prdoc
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,37 @@
title: Allow to set a worst case buy execution fee asset and weight
doc:
- audience: Runtime Dev
description: |-
Allows to set a worst case for the `BuyExecution` XCM instruction
benchmark. Currently the benchmark assumes best case scenario (i.e.)
the case where does not even need to go into the Trader. This PR allows
developers to set the worst-case scenario as they wish.

Breaking changes:
- Change `fee_asset` in the benchmarking helper to `worst_case_for_trader` which
now should return both the `fee_asset` and the weight to use
crates:
- name: pallet-xcm-benchmarks
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
- name: pallet-xcm-benchmarks
- name: pallet-xcm-benchmarks
- bump: major

It's major since you're adding a function with no default.

Copy link
Contributor

Choose a reason for hiding this comment

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

well.. shit! how are we gonna backport then?

I guess we'll have to limit "backport" to just 2503 which hasn't been published yet

Copy link
Contributor

Choose a reason for hiding this comment

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

ah, nevermind, that's fine, we don't really need to backport this too far back

Copy link
Contributor Author

@girazoki girazoki Mar 24, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

I am actually adding a default value, unless you want me to change it. If I dont add a default value I need to put a worst case scenario in all runtimes, and I dont really know what is the worst case scenario in each of them. I can try to guess, but a few hints would be good

Copy link
Contributor

Choose a reason for hiding this comment

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

Now that you changed fee_asset, there's no default. Everyone needs to change that function so it's a breaking change.
If we don't need to backport then it's fine.

bump: major
- name: rococo-runtime
bump: patch
- name: westend-runtime
bump: patch
- name: asset-hub-rococo-runtime
bump: patch
- name: asset-hub-westend-runtime
bump: patch
- name: bridge-hub-rococo-runtime
bump: patch
- name: bridge-hub-westend-runtime
bump: patch
- name: collectives-westend-runtime
bump: patch
- name: coretime-rococo-runtime
bump: patch
- name: coretime-westend-runtime
bump: patch
- name: people-rococo-runtime
bump: patch
- name: people-westend-runtime
bump: patch
Loading