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

feat(mainnet): include xcm benchmarks #479

Open
wants to merge 29 commits into
base: al3mart/feat-mainnet-all-benchmarks
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
29 commits
Select commit Hold shift + click to select a range
7cbcf93
feat(benchmark): add pallet_xcm extrinsic benchmarks
al3mart Feb 24, 2025
d01b5fd
refactor(benchmkars): amend xcm transfer_assets benchmark
al3mart Feb 24, 2025
bdbcf25
style(benchmarks): pretty comments.
al3mart Feb 24, 2025
8cc0ab1
refactor(benchmarks): fund SA of AH on pop in reserve_transfer benchmark
al3mart Feb 24, 2025
2b0ec99
feat(mainnet): add generic & fungible xcm benchmarks
al3mart Feb 24, 2025
8974a5a
refactor(benchmarks): don't define CheckAccount for pallet_xcm_benchm…
al3mart Feb 24, 2025
e92d2a0
refactor(benchmarks): amend benchmarks for pallet_xcm_benchmarks::gen…
al3mart Feb 24, 2025
08ac3ee
feat(mainnet): configure xcm Weigher as WeightInfoBounds
al3mart Feb 25, 2025
09b6582
docs(xcm): missing doc
al3mart Feb 25, 2025
9719dfd
refactor(weights): MAX_ASSETS is MaxAssetsIntoHolding
al3mart Feb 25, 2025
5ed1a0d
refactor(mainnet): remove default weights from queue pallets
al3mart Feb 25, 2025
ec8e319
docs(xcm): credit source for WeighAssets impl
al3mart Feb 26, 2025
ce9ede1
docs(xcm): reminders for updating benchmarks
al3mart Feb 26, 2025
29adfb5
refactor(mainnet): DeliveryHelper considers delivery to parent
al3mart Feb 26, 2025
18a2cee
docs(benchmarks): clarify xcm NonFungible AllOf weight comments
al3mart Feb 26, 2025
4b04f4a
refactor(api): reorg imports
al3mart Feb 28, 2025
081ca54
style(benchmark): refactor import syntax
al3mart Feb 28, 2025
31f9e63
docs(bencmark): better docs for XcmBalances
al3mart Feb 28, 2025
1f3c852
refactor(benchmark): define locations via xcm config definitions
al3mart Feb 28, 2025
8906204
test(xcm): ensure xcm encoding size
al3mart Feb 28, 2025
4863071
refactor(benchmark): improve worst_case_response
al3mart Feb 28, 2025
7269599
chore(benchmark): update results for query_response bench
al3mart Feb 28, 2025
0b83a86
style(benchmarks): remove RelayAsset
al3mart Feb 28, 2025
8640b6b
docs(benchmarks): explain ExistentialDepositAsset
al3mart Feb 28, 2025
66e3802
docs(xcm): note worst_case_holding needs updating
al3mart Feb 28, 2025
ac7e849
style(benchmark): better naming
al3mart Feb 28, 2025
f6f8af0
chore(benchmarks): update xcm benchmarks
al3mart Feb 28, 2025
9755095
test(xcm): amend router_uses_ump_for_relay_and_xcmp_for_para
al3mart Feb 28, 2025
b4824d2
test(xcm): amend router_uses_ump_for_relay_and_xcmp_for_para
al3mart Mar 1, 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
1 change: 1 addition & 0 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 1 addition & 0 deletions Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -153,6 +153,7 @@ substrate-frame-rpc-system = { git = "https://github.com/paritytech/polkadot-sdk

# Polkadot
pallet-xcm = { git = "https://github.com/paritytech/polkadot-sdk", branch = "stable2412", default-features = false }
pallet-xcm-benchmarks = { git = "https://github.com/paritytech/polkadot-sdk", branch = "stable2412", default-features = false }
polkadot-cli = { git = "https://github.com/paritytech/polkadot-sdk", branch = "stable2412" }
polkadot-parachain-primitives = { git = "https://github.com/paritytech/polkadot-sdk", branch = "stable2412", default-features = false }
polkadot-primitives = { git = "https://github.com/paritytech/polkadot-sdk", branch = "stable2412", default-features = false }
Expand Down
3 changes: 3 additions & 0 deletions runtime/mainnet/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -69,6 +69,7 @@ sp-version.workspace = true

# Polkadot
pallet-xcm.workspace = true
pallet-xcm-benchmarks.workspace = true
polkadot-parachain-primitives.workspace = true
polkadot-runtime-common.workspace = true
xcm.workspace = true
Expand Down Expand Up @@ -141,6 +142,7 @@ std = [
"pallet-transaction-payment/std",
"pallet-treasury/std",
"pallet-utility/std",
"pallet-xcm-benchmarks/std",
"pallet-xcm/std",
"parachain-info/std",
"parachains-common/std",
Expand Down Expand Up @@ -193,6 +195,7 @@ runtime-benchmarks = [
"pallet-transaction-payment/runtime-benchmarks",
"pallet-treasury/runtime-benchmarks",
"pallet-utility/runtime-benchmarks",
"pallet-xcm-benchmarks/runtime-benchmarks",
"pallet-xcm/runtime-benchmarks",
"parachains-common/runtime-benchmarks",
"polkadot-parachain-primitives/runtime-benchmarks",
Expand Down
2 changes: 1 addition & 1 deletion runtime/mainnet/src/apis.rs
Original file line number Diff line number Diff line change
Expand Up @@ -285,8 +285,7 @@
use frame_system_benchmarking::Pallet as SystemBench;
use frame_system_benchmarking::extensions::Pallet as SystemExtensionsBench;


impl frame_system_benchmarking::Config for Runtime {

Check warning on line 288 in runtime/mainnet/src/apis.rs

View workflow job for this annotation

GitHub Actions / clippy

non-local `impl` definition, `impl` blocks should be written at the same level as their item

warning: non-local `impl` definition, `impl` blocks should be written at the same level as their item --> runtime/mainnet/src/apis.rs:288:4 | 279 | / fn dispatch_benchmark( 280 | | config: frame_benchmarking::BenchmarkConfig 281 | | ) -> Result<Vec<frame_benchmarking::BenchmarkBatch>, alloc::string::String> { | |___________________________________________________________________________________- move the `impl` block outside of this associated function `dispatch_benchmark` ... 288 | impl frame_system_benchmarking::Config for Runtime { | ^^^^^---------------------------------^^^^^------- | | | | | `Runtime` is not local | `Config` is not local | = note: an `impl` is never scoped, even when it is nested inside an item, as it may impact type checking outside of that item, which can be the case if neither the trait or the self type are at the same nesting level as the `impl` = note: `#[warn(non_local_definitions)]` on by default
fn setup_set_code_requirements(code: &Vec<u8>) -> Result<(), BenchmarkError> {
ParachainSystem::initialize_for_set_code_benchmark(code.len() as u32);
Ok(())
Expand All @@ -298,9 +297,10 @@
}

use cumulus_pallet_session_benchmarking::Pallet as SessionBench;
impl cumulus_pallet_session_benchmarking::Config for Runtime {}

Check warning on line 300 in runtime/mainnet/src/apis.rs

View workflow job for this annotation

GitHub Actions / clippy

non-local `impl` definition, `impl` blocks should be written at the same level as their item

warning: non-local `impl` definition, `impl` blocks should be written at the same level as their item --> runtime/mainnet/src/apis.rs:300:4 | 279 | / fn dispatch_benchmark( 280 | | config: frame_benchmarking::BenchmarkConfig 281 | | ) -> Result<Vec<frame_benchmarking::BenchmarkBatch>, alloc::string::String> { | |___________________________________________________________________________________- move the `impl` block outside of this associated function `dispatch_benchmark` ... 300 | impl cumulus_pallet_session_benchmarking::Config for Runtime {} | ^^^^^-------------------------------------------^^^^^------- | | | | | `Runtime` is not local | `Config` is not local | = note: an `impl` is never scoped, even when it is nested inside an item, as it may impact type checking outside of that item, which can be the case if neither the trait or the self type are at the same nesting level as the `impl`

use frame_support::traits::WhitelistedStorageKeys;

let whitelist = AllPalletsWithSystem::whitelisted_storage_keys();

let mut batches = Vec::<BenchmarkBatch>::new();
Expand Down
215 changes: 213 additions & 2 deletions runtime/mainnet/src/benchmarks.rs
Original file line number Diff line number Diff line change
@@ -1,3 +1,30 @@
use alloc::{boxed::Box, vec};

use cumulus_primitives_core::ParaId;
use frame_benchmarking::BenchmarkError;
use frame_support::parameter_types;
pub use pallet_xcm::benchmarking::Pallet as PalletXcmBenchmark;
use xcm::prelude::{
Asset, AssetId, Fungible, Here, InteriorLocation, Junction, Location, NetworkId, Response,
};
use xcm_executor::traits::ConvertLocation;

use crate::{
config::{
monetary::ExistentialDeposit,
xcm::{
AssetHub, LocationToAccountId, PriceForParentDelivery, PriceForSiblingDelivery,
RelayLocation, XcmConfig,
},
},
Runtime, *,
};

/// Pallet that benchmarks XCM's `AssetTransactor` trait via `Fungible`.
pub type XcmFungible = pallet_xcm_benchmarks::fungible::Pallet<Runtime>;
/// Pallet that serves no other purpose than benchmarking raw XCMs.
pub type XcmGeneric = pallet_xcm_benchmarks::generic::Pallet<Runtime>;

frame_benchmarking::define_benchmarks!(
// Ordered as per runtime
// System
Expand All @@ -22,8 +49,9 @@
[pallet_preimage, Preimage]
// XCM
[cumulus_pallet_xcmp_queue, XcmpQueue]
// TODO: intro xcm benchmarks
//[pallet_xcm, PolkadotXcm::<Runtime>]
[pallet_xcm, PalletXcmBenchmark::<Runtime>]
[pallet_xcm_benchmarks::fungible, XcmFungible]
[pallet_xcm_benchmarks::generic, XcmGeneric]
[pallet_message_queue, MessageQueue]
// Contracts
[pallet_revive, Revive]
Expand All @@ -37,3 +65,186 @@
[pallet_nfts, Nfts]
[pallet_assets, Assets]
);

parameter_types! {
/// Delivery helpers will deposit this amount to the local origin used in the benchmarks.
pub ExistentialDepositAsset: Option<Asset> = Some((
RelayLocation::get(),
ExistentialDeposit::get()
).into());
pub const AssetHubParaId: ParaId = ParaId::new(1000);
}

type DeliveryHelper = (
cumulus_primitives_utility::ToParentDeliveryHelper<
XcmConfig,
ExistentialDepositAsset,
PriceForParentDelivery,
>,
polkadot_runtime_common::xcm_sender::ToParachainDeliveryHelper<
XcmConfig,
ExistentialDepositAsset,
PriceForSiblingDelivery,
AssetHubParaId,
ParachainSystem,
>,
);

impl pallet_xcm::benchmarking::Config for Runtime {
type DeliveryHelper = DeliveryHelper;

fn reachable_dest() -> Option<Location> {
Some(RelayLocation::get())
}

fn teleportable_asset_and_dest() -> Option<(Asset, Location)> {
// No assets can be teleported.
None
}

fn reserve_transferable_asset_and_dest() -> Option<(Asset, Location)> {
ParachainSystem::open_outbound_hrmp_channel_for_benchmarks_or_tests(AssetHubParaId::get());

let who = frame_benchmarking::whitelisted_caller();
// Give some multiple of the existential deposit.
let balance = ExistentialDeposit::get() * 10_000;
let _ =
<Balances as frame_support::traits::Currency<_>>::make_free_balance_be(&who, balance);
let ah_on_pop: AccountId = LocationToAccountId::convert_location(&AssetHub::get())?;
let _ = <Balances as frame_support::traits::Currency<_>>::make_free_balance_be(
&ah_on_pop, balance,
);
Comment on lines +114 to +116
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Funding AH sovereign account on pop because the benchmark tries to withdraw from destination as a way to verify the benchmark.

I believe that makes sense for reserve transfers happening within the reserve itself. Not quite that for our case.

Copy link
Collaborator

Choose a reason for hiding this comment

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


// We can do reserve transfers of relay native asset to AH.
Some((
Asset { fun: Fungible(ExistentialDeposit::get()), id: AssetId(RelayLocation::get()) },
AssetHub::get(),
))
}

fn set_up_complex_asset_transfer(
) -> Option<(xcm::prelude::Assets, u32, Location, Box<dyn FnOnce()>)> {
ParachainSystem::open_outbound_hrmp_channel_for_benchmarks_or_tests(ParaId::from(
AssetHubParaId::get(),
Comment on lines +127 to +128
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
ParachainSystem::open_outbound_hrmp_channel_for_benchmarks_or_tests(ParaId::from(
AssetHubParaId::get(),
ParachainSystem::open_outbound_hrmp_channel_for_benchmarks_or_tests(
AssetHubParaId::get(),

));

Check warning on line 129 in runtime/mainnet/src/benchmarks.rs

View workflow job for this annotation

GitHub Actions / clippy

useless conversion to the same type: `cumulus_primitives_core::ParaId`

warning: useless conversion to the same type: `cumulus_primitives_core::ParaId` --> runtime/mainnet/src/benchmarks.rs:127:71 | 127 | ParachainSystem::open_outbound_hrmp_channel_for_benchmarks_or_tests(ParaId::from( | _____________________________________________________________________________^ 128 | | AssetHubParaId::get(), 129 | | )); | |_________^ help: consider removing `ParaId::from()`: `AssetHubParaId::get()` | = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#useless_conversion = note: `#[warn(clippy::useless_conversion)]` on by default
// Pop can only reserve transfer DOT.
// This test needs to be adapted as the features grow.
Copy link
Collaborator

Choose a reason for hiding this comment

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

could you please add a comment on one of the XCM config unit tests?

Basically that way we have this flow:

  1. we change XCM to support other asset transfers
  2. We now have a failing unit test
  3. We fix unit test and see comment that says: "update set_up_complex_asset_transfer ..."

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

makes sense, thanks!

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Copy link
Collaborator

Choose a reason for hiding this comment

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

The worst_case_holding is also one that must be included in this cycle? Perhaps we should create a doc that we can circle back to? Just an idea not saying it as a requirement

let dest = AssetHub::get();

let fee_amount = ExistentialDeposit::get();
let fee_asset: Asset = (RelayLocation::get(), fee_amount).into();

let who = frame_benchmarking::whitelisted_caller();
// Give some multiple of the existential deposit.
let balance = fee_amount + ExistentialDeposit::get() * 10_000;
let _ =
<Balances as frame_support::traits::Currency<_>>::make_free_balance_be(&who, balance);
// Verify initial balance.
assert_eq!(Balances::free_balance(&who), balance);

let assets: xcm::prelude::Assets = vec![fee_asset.clone()].into();

// Verify transferred successfully.
let verify = Box::new(move || {
// Verify native balance after transfer, decreased by transferred fee amount
// (plus transport fees).
assert!(Balances::free_balance(&who) <= balance - fee_amount);
});
Some((assets, 0, dest, verify))
}

fn get_asset() -> Asset {
Asset { id: AssetId(RelayLocation::get()), fun: Fungible(ExistentialDeposit::get()) }
}
}

impl pallet_xcm_benchmarks::Config for Runtime {
type AccountIdConverter = LocationToAccountId;
type DeliveryHelper = DeliveryHelper;
Copy link
Collaborator

Choose a reason for hiding this comment

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

Do you know why asset hub only uses ToParentDeliveryHelper (ref)?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Had the same question. The answer I got is that our config looked good but no explanation to why AH benches are configured the way they are.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Can we ask this specific thing?

type XcmConfig = XcmConfig;

fn valid_destination() -> Result<Location, BenchmarkError> {
Ok(RelayLocation::get())
}

fn worst_case_holding(_depositable_count: u32) -> xcm::prelude::Assets {
// Pop only allows relay's native asset to be used cross chain for now.
vec![Asset { id: AssetId(RelayLocation::get()), fun: Fungible(u128::MAX) }].into()
}
}

impl pallet_xcm_benchmarks::generic::Config for Runtime {
type RuntimeCall = RuntimeCall;
type TransactAsset = Balances;

fn worst_case_response() -> (u64, Response) {
let notify = frame_system::Call::remark { remark: vec![] };
PolkadotXcm::new_notify_query(Location::here(), notify, 10, Location::here());
Comment on lines +181 to +182
Copy link
Collaborator

Choose a reason for hiding this comment

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

Nothing is done with this?

(0u64, Response::ExecutionResult(None))
}

fn worst_case_asset_exchange(
) -> Result<(xcm::prelude::Assets, xcm::prelude::Assets), BenchmarkError> {
// Pop doesn't support asset exchange for now.
Err(BenchmarkError::Skip)
}

fn universal_alias() -> Result<(Location, Junction), BenchmarkError> {
// Pop's `UniversalAliases` is configured to `Nothing`.
Err(BenchmarkError::Skip)
Copy link
Collaborator

Choose a reason for hiding this comment

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

Do we want it to emit an error and stop or skip? Just checking because I honestly don't know

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I believe the key difference is that Skip won't stop the benchmark of other extrinsics even though this one errs.
Which seems handy.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Asset hub uses it so seems to be fine.

}

fn transact_origin_and_runtime_call() -> Result<(Location, RuntimeCall), BenchmarkError> {
Ok((RelayLocation::get(), frame_system::Call::remark_with_event { remark: vec![] }.into()))
}

fn subscribe_origin() -> Result<Location, BenchmarkError> {
Ok(RelayLocation::get())
}

fn claimable_asset() -> Result<(Location, Location, xcm::prelude::Assets), BenchmarkError> {
let origin = AssetHub::get();
let assets: xcm::prelude::Assets = (AssetId(RelayLocation::get()), 1_000 * UNIT).into();
let ticket = Location { parents: 0, interior: Here };
Ok((origin, ticket, assets))
}

fn fee_asset() -> Result<Asset, BenchmarkError> {
Ok(Asset { id: AssetId(RelayLocation::get()), fun: Fungible(1_000_000 * UNIT) })
}

fn unlockable_asset() -> Result<(Location, Location, Asset), BenchmarkError> {
// Pop doesn't configure `AssetLocker` yet.
Err(BenchmarkError::Skip)
}

fn export_message_origin_and_destination(
) -> Result<(Location, NetworkId, InteriorLocation), BenchmarkError> {
// Pop doesn't configure `MessageExporter` yet.
Err(BenchmarkError::Skip)
}

fn alias_origin() -> Result<(Location, Location), BenchmarkError> {
// Pop's `Aliasers` is configured to `Nothing`.
Err(BenchmarkError::Skip)
}
}

parameter_types! {
pub TrustedReserve: Option<(Location, Asset)> = Some((AssetHub::get(), Asset::from((RelayLocation::get(), UNIT))));
// We don't set any trusted teleporters in our XCM config, but we need this for the benchmarks.
pub TrustedTeleporter: Option<(Location, Asset)> = Some((
AssetHub::get(),
Asset::from((RelayLocation::get(), UNIT)),
));
}
impl pallet_xcm_benchmarks::fungible::Config for Runtime {
type CheckedAccount = ();
type TransactAsset = Balances;
type TrustedReserve = TrustedReserve;
type TrustedTeleporter = TrustedTeleporter;

fn get_asset() -> Asset {
Asset { id: AssetId(RelayLocation::get()), fun: Fungible(10 * UNIT) }
}
}
Loading
Loading