Skip to content

Allow to set a worst case buy execution fee asset and weight #2962

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

Closed
girazoki wants to merge 4 commits into from
Closed

Allow to set a worst case buy execution fee asset and weight #2962

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 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
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: 5 additions & 3 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 @@ -77,11 +77,13 @@ 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_buy_execution()?;
Copy link
Copy Markdown
Contributor

@franciscoaguirre franciscoaguirre Jan 17, 2024

Choose a reason for hiding this comment

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

I definitely like the idea of benchmarking with WeightLimit::Limited so we can hit more logic. There's now a fee_asset configuration item that is used to buying execution in the benchmark for refund_surplus, this could be merged with that to include the weight_limit.

We also have to make sure this is in holding, don't know if there's a good way to do it.

Copy link
Copy Markdown
Contributor Author

@girazoki girazoki Jan 17, 2024

Choose a reason for hiding this comment

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

ok I can try to do this!

Copy link
Copy Markdown
Contributor Author

@girazoki girazoki Jan 17, 2024

Choose a reason for hiding this comment

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

For holding, yes we rely on worst_case_holding having the fees, and I guess that if it is not the benchmark will fail. So I think this is good enough.

The other way we could do it is by injecting the fee_asset into holding directly, without using worst_case_holding at all

Copy link
Copy Markdown
Contributor Author

@girazoki girazoki Jan 17, 2024

Choose a reason for hiding this comment

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

@franciscoaguirre I dont see the fee_asset setup in refund_surplus. all I see is that the total_surplus is artificially set

polkadot-sdk/polkadot/xcm/pallet-xcm-benchmarks/src/generic/benchmarking.rs

Line 131 in c01dbeb

executor.set_total_surplus(Weight::from_parts(1337, 1337));


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 Down
13 changes: 12 additions & 1 deletion 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,13 @@ 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, AssetId, Assets, InteriorLocation, Junction, Junctions, Location, NetworkId,
Response,
},
v2::WeightLimit,
};

#[pallet::config]
pub trait Config<I: 'static = ()>: frame_system::Config + crate::Config {
Expand Down Expand Up @@ -104,6 +110,11 @@ pub mod pallet {
crate_version: <frame_system::Pallet<Self> as frame_support::traits::PalletInfoAccess>::crate_version(),
}
}

/// The worst case buy execution weight limit and
/// asset to trigger the Trader::buy_execution in the XCM executor
/// By default returns ((AssetId(Here.into()), 100_000_000u128), Unlimited)
fn worst_case_buy_execution() -> Result<(Asset, WeightLimit), BenchmarkError>;
}

#[pallet::pallet]
Expand Down
11 changes: 11 additions & 0 deletions prdoc/pr_2962.prdoc
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
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.
crates:
- name: pallet-xcm-benchmarks