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

[xcm] Use Weight::MAX for reserve_asset_deposited, receive_teleported_asset benchmarks #1726

Merged
merged 20 commits into from
Oct 10, 2023

Conversation

bkontur
Copy link
Contributor

@bkontur bkontur commented Sep 27, 2023

Description

Summary

Previously, the pallet_xcm::do_reserve_transfer_assets and pallet_xcm::do_teleport_assets functions relied on weight estimation for remote chain execution, which was based on guesswork derived from the local chain. This approach led to complications for runtimes that did not provide or support specific XCM configurations for IsReserve or IsTeleporter. Consequently, such runtimes had to resort to implementing hard-coded weights for XCM instructions like reserve_asset_deposited or receive_teleported_asset to support extrinsics such as pallet_xcm::reserve_transfer_assets and pallet_xcm::teleport_assets, which depended on remote weight estimation.

The issue of remote weight estimation was addressed and resolved by Pull Request #1645, which removed the need for remote weight estimation.

Solution

As a continuation of this improvement, the current PR proposes further cleanup by removing unnecessary hard-coded values and rectifying benchmark results with Weight::MAX that previously used T::BlockWeights::get().max_block as an override for unsupported XCM instructions like ReserveAssetDeposited and ReceiveTeleportedAsset.

Questions

  • Can we remove now also Hardcoded till the XCM pallet is fixed for deposit_asset? E.g. for AssetHubKusama here
  • Are comments like this // Kusama doesn't support ReserveAssetDeposited, so this benchmark has a default weight still relevant? Shouldnt be removed/changed?

TODO

  • bench bot regenerate xcm weights for all runtimes
  • remove hard-coded stuff from system parachain weight files
  • when merged, open polkadot-fellow/runtimes PR

References

Fixes #1132
Closes #1132
Old polkadot repo PR

@bkontur bkontur added T6-XCM This PR/Issue is related to XCM. T14-system_parachains This PR/Issue is related to system parachains. labels Sep 27, 2023
@bkontur
Copy link
Contributor Author

bkontur commented Sep 27, 2023

bot help

@command-bot
Copy link

command-bot bot commented Sep 27, 2023

Here's a link to docs

@bkontur
Copy link
Contributor Author

bkontur commented Sep 27, 2023

bot bench cumulus-assets --runtime asset-hub-polkadot --subcommand xcm --pallet pallet_xcm_benchmarks::fungible

@command-bot
Copy link

command-bot bot commented Sep 27, 2023

"$PIPELINE_SCRIPTS_DIR/commands/bench/bench.sh" --subcommand=xcm --runtime=asset-hub-polkadot --runtime_dir=assets --target_dir=cumulus --pallet=pallet_xcm_benchmarks::fungible was queued.

Comment bot cancel 4-96b839ac-da56-4cf4-b942-5c65332b8266 to cancel this command or bot cancel to cancel all commands in this pull request.

@command-bot
Copy link

command-bot bot commented Sep 27, 2023

@bkontur Command "$PIPELINE_SCRIPTS_DIR/commands/bench/bench.sh" --subcommand=xcm --runtime=asset-hub-polkadot --runtime_dir=assets --target_dir=cumulus --pallet=pallet_xcm_benchmarks::fungible has finished. Result:

ValidationError: "id" is required
ValidationError: "id" is required
{"message":{"base":["Reference not found"]}}

@mordamax
Copy link
Contributor

bot bench cumulus-assets --runtime asset-hub-polkadot --subcommand xcm --pallet pallet_xcm_benchmarks::fungible

@command-bot
Copy link

command-bot bot commented Sep 27, 2023

@mordamax https://gitlab.parity.io/parity/mirrors/polkadot-sdk/-/jobs/3825853 was started for your command "$PIPELINE_SCRIPTS_DIR/commands/bench/bench.sh" --subcommand=xcm --runtime=asset-hub-polkadot --runtime_dir=assets --target_dir=cumulus --pallet=pallet_xcm_benchmarks::fungible. Check out https://gitlab.parity.io/parity/mirrors/polkadot-sdk/-/pipelines?page=1&scope=all&username=group_605_bot to know what else is being executed currently.

Comment bot cancel 8-ccb217c9-af2c-45c1-8c12-3c14f06721f1 to cancel this command or bot cancel to cancel all commands in this pull request.

command-bot added 2 commits September 27, 2023 21:45
…set-hub-polkadot --runtime_dir=assets --target_dir=cumulus --pallet=pallet_xcm_benchmarks::fungible
@command-bot
Copy link

command-bot bot commented Sep 27, 2023

@mordamax Command "$PIPELINE_SCRIPTS_DIR/commands/bench/bench.sh" --subcommand=xcm --runtime=asset-hub-polkadot --runtime_dir=assets --target_dir=cumulus --pallet=pallet_xcm_benchmarks::fungible has finished. Result: https://gitlab.parity.io/parity/mirrors/polkadot-sdk/-/jobs/3825853 has finished. If any artifacts were generated, you can download them from https://gitlab.parity.io/parity/mirrors/polkadot-sdk/-/jobs/3825853/artifacts/download.

@bkontur
Copy link
Contributor Author

bkontur commented Sep 28, 2023

bot bench cumulus-assets --runtime asset-hub-kusama --subcommand xcm --pallet pallet_xcm_benchmarks::fungible
bot bench cumulus-assets --runtime asset-hub-westend --subcommand xcm --pallet pallet_xcm_benchmarks::fungible

@command-bot
Copy link

command-bot bot commented Sep 28, 2023

@bkontur https://gitlab.parity.io/parity/mirrors/polkadot-sdk/-/jobs/3828304 was started for your command "$PIPELINE_SCRIPTS_DIR/commands/bench/bench.sh" --subcommand=xcm --runtime=asset-hub-kusama --runtime_dir=assets --target_dir=cumulus --pallet=pallet_xcm_benchmarks::fungible. Check out https://gitlab.parity.io/parity/mirrors/polkadot-sdk/-/pipelines?page=1&scope=all&username=group_605_bot to know what else is being executed currently.

Comment bot cancel 9-73db97ef-a019-4e0f-8888-eec6d6baf23e to cancel this command or bot cancel to cancel all commands in this pull request.

@command-bot
Copy link

command-bot bot commented Sep 28, 2023

@bkontur https://gitlab.parity.io/parity/mirrors/polkadot-sdk/-/jobs/3828305 was started for your command "$PIPELINE_SCRIPTS_DIR/commands/bench/bench.sh" --subcommand=xcm --runtime=asset-hub-westend --runtime_dir=assets --target_dir=cumulus --pallet=pallet_xcm_benchmarks::fungible. Check out https://gitlab.parity.io/parity/mirrors/polkadot-sdk/-/pipelines?page=1&scope=all&username=group_605_bot to know what else is being executed currently.

Comment bot cancel 10-c920392d-e8a5-4463-92ff-7ec209d710a4 to cancel this command or bot cancel to cancel all commands in this pull request.

@bkontur
Copy link
Contributor Author

bkontur commented Sep 28, 2023

bot bench cumulus-bridge-hubs --runtime bridge-hub-polkadot --subcommand xcm --pallet pallet_xcm_benchmarks::fungible
bot bench cumulus-bridge-hubs --runtime bridge-hub-kusama --subcommand xcm --pallet pallet_xcm_benchmarks::fungible
bot bench cumulus-bridge-hubs --runtime bridge-hub-rococo --subcommand xcm --pallet pallet_xcm_benchmarks::fungible

@command-bot
Copy link

command-bot bot commented Sep 28, 2023

@bkontur https://gitlab.parity.io/parity/mirrors/polkadot-sdk/-/jobs/3828306 was started for your command "$PIPELINE_SCRIPTS_DIR/commands/bench/bench.sh" --subcommand=xcm --runtime=bridge-hub-polkadot --runtime_dir=bridge-hubs --target_dir=cumulus --pallet=pallet_xcm_benchmarks::fungible. Check out https://gitlab.parity.io/parity/mirrors/polkadot-sdk/-/pipelines?page=1&scope=all&username=group_605_bot to know what else is being executed currently.

Comment bot cancel 11-6987b38b-0a4b-4eb6-95d9-8e53765d1b30 to cancel this command or bot cancel to cancel all commands in this pull request.

@command-bot
Copy link

command-bot bot commented Sep 28, 2023

@bkontur https://gitlab.parity.io/parity/mirrors/polkadot-sdk/-/jobs/3828307 was started for your command "$PIPELINE_SCRIPTS_DIR/commands/bench/bench.sh" --subcommand=xcm --runtime=bridge-hub-kusama --runtime_dir=bridge-hubs --target_dir=cumulus --pallet=pallet_xcm_benchmarks::fungible. Check out https://gitlab.parity.io/parity/mirrors/polkadot-sdk/-/pipelines?page=1&scope=all&username=group_605_bot to know what else is being executed currently.

Comment bot cancel 12-4bbbb6e2-2410-4040-a444-b0033b4a6422 to cancel this command or bot cancel to cancel all commands in this pull request.

@command-bot
Copy link

command-bot bot commented Sep 28, 2023

@bkontur https://gitlab.parity.io/parity/mirrors/polkadot-sdk/-/jobs/3828308 was started for your command "$PIPELINE_SCRIPTS_DIR/commands/bench/bench.sh" --subcommand=xcm --runtime=bridge-hub-rococo --runtime_dir=bridge-hubs --target_dir=cumulus --pallet=pallet_xcm_benchmarks::fungible. Check out https://gitlab.parity.io/parity/mirrors/polkadot-sdk/-/pipelines?page=1&scope=all&username=group_605_bot to know what else is being executed currently.

Comment bot cancel 13-dd78d0f6-2473-490f-ad51-adefb5654068 to cancel this command or bot cancel to cancel all commands in this pull request.

@bkontur
Copy link
Contributor Author

bkontur commented Sep 28, 2023

bot bench polkadot-pallet --runtime polkadot --subcommand xcm --pallet pallet_xcm_benchmarks::fungible
bot bench polkadot-pallet --runtime kusama --subcommand xcm --pallet pallet_xcm_benchmarks::fungible
bot bench polkadot-pallet --runtime westend --subcommand xcm --pallet pallet_xcm_benchmarks::fungible
bot bench polkadot-pallet --runtime rococo --subcommand xcm --pallet pallet_xcm_benchmarks::fungible

@command-bot
Copy link

command-bot bot commented Sep 28, 2023

@bkontur https://gitlab.parity.io/parity/mirrors/polkadot-sdk/-/jobs/3828310 was started for your command "$PIPELINE_SCRIPTS_DIR/commands/bench/bench.sh" --subcommand=xcm --runtime=polkadot --target_dir=polkadot --pallet=pallet_xcm_benchmarks::fungible. Check out https://gitlab.parity.io/parity/mirrors/polkadot-sdk/-/pipelines?page=1&scope=all&username=group_605_bot to know what else is being executed currently.

Comment bot cancel 14-025a4d49-e87e-44dd-bf8b-b0ea664c6ab0 to cancel this command or bot cancel to cancel all commands in this pull request.

@command-bot
Copy link

command-bot bot commented Sep 28, 2023

@bkontur https://gitlab.parity.io/parity/mirrors/polkadot-sdk/-/jobs/3828319 was started for your command "$PIPELINE_SCRIPTS_DIR/commands/bench/bench.sh" --subcommand=xcm --runtime=kusama --target_dir=polkadot --pallet=pallet_xcm_benchmarks::fungible. Check out https://gitlab.parity.io/parity/mirrors/polkadot-sdk/-/pipelines?page=1&scope=all&username=group_605_bot to know what else is being executed currently.

Comment bot cancel 15-80878031-f6c9-4d08-b660-5a9caa5fcd2e to cancel this command or bot cancel to cancel all commands in this pull request.

@command-bot
Copy link

command-bot bot commented Sep 28, 2023

@bkontur https://gitlab.parity.io/parity/mirrors/polkadot-sdk/-/jobs/3828322 was started for your command "$PIPELINE_SCRIPTS_DIR/commands/bench/bench.sh" --subcommand=xcm --runtime=westend --target_dir=polkadot --pallet=pallet_xcm_benchmarks::fungible. Check out https://gitlab.parity.io/parity/mirrors/polkadot-sdk/-/pipelines?page=1&scope=all&username=group_605_bot to know what else is being executed currently.

Comment bot cancel 16-de8dba29-ab4d-46b3-ae83-ab5b97bda174 to cancel this command or bot cancel to cancel all commands in this pull request.

@command-bot
Copy link

command-bot bot commented Sep 28, 2023

@bkontur https://gitlab.parity.io/parity/mirrors/polkadot-sdk/-/jobs/3828323 was started for your command "$PIPELINE_SCRIPTS_DIR/commands/bench/bench.sh" --subcommand=xcm --runtime=rococo --target_dir=polkadot --pallet=pallet_xcm_benchmarks::fungible. Check out https://gitlab.parity.io/parity/mirrors/polkadot-sdk/-/pipelines?page=1&scope=all&username=group_605_bot to know what else is being executed currently.

Comment bot cancel 17-f267a616-de32-4ebd-ba13-922a59156c3d to cancel this command or bot cancel to cancel all commands in this pull request.

command-bot added 2 commits September 28, 2023 07:31
@command-bot
Copy link

command-bot bot commented Sep 28, 2023

@bkontur Command "$PIPELINE_SCRIPTS_DIR/commands/bench/bench.sh" --subcommand=xcm --runtime=westend --target_dir=polkadot --pallet=pallet_xcm_benchmarks::fungible has finished. Result: https://gitlab.parity.io/parity/mirrors/polkadot-sdk/-/jobs/3828322 has finished. If any artifacts were generated, you can download them from https://gitlab.parity.io/parity/mirrors/polkadot-sdk/-/jobs/3828322/artifacts/download.

@command-bot
Copy link

command-bot bot commented Sep 28, 2023

@bkontur Command "$PIPELINE_SCRIPTS_DIR/commands/bench/bench.sh" --subcommand=xcm --runtime=polkadot --target_dir=polkadot --pallet=pallet_xcm_benchmarks::fungible has finished. Result: https://gitlab.parity.io/parity/mirrors/polkadot-sdk/-/jobs/3828310 has finished. If any artifacts were generated, you can download them from https://gitlab.parity.io/parity/mirrors/polkadot-sdk/-/jobs/3828310/artifacts/download.

@command-bot
Copy link

command-bot bot commented Sep 28, 2023

@bkontur Command "$PIPELINE_SCRIPTS_DIR/commands/bench/bench.sh" --subcommand=xcm --runtime=kusama --target_dir=polkadot --pallet=pallet_xcm_benchmarks::fungible has finished. Result: https://gitlab.parity.io/parity/mirrors/polkadot-sdk/-/jobs/3828319 has finished. If any artifacts were generated, you can download them from https://gitlab.parity.io/parity/mirrors/polkadot-sdk/-/jobs/3828319/artifacts/download.

@bkontur bkontur marked this pull request as ready for review October 3, 2023 21:22
@bkontur bkontur requested a review from a team as a code owner October 3, 2023 21:22
@KiChjang
Copy link
Contributor

KiChjang commented Oct 4, 2023

Can we remove now also Hardcoded till the XCM pallet is fixed for deposit_asset

IIRC this was there because the XCM pallet did not have proper benchmarked weights, so once we do, then we can indeed remove it.

@bkontur bkontur merged commit e3c97e4 into master Oct 10, 2023
12 checks passed
@bkontur bkontur deleted the bko-xcm-max-weight-for-benchmarks branch October 10, 2023 11:26
ordian added a commit that referenced this pull request Oct 12, 2023
* master: (33 commits)
  ci: set CI_IMAGE back to (now updated) .ci-unified (#1854)
  ci: bump ci image to rust 1.73.0 (#1830)
  Refactor Identity to benchmark v2 (#1838)
  PVF worker: bump landlock, update ABI docs (#1850)
  Xcm emulator nits (#1649)
  Fixes path issue in derive-impl (#1823)
  upgrade to macro_magic 0.4.3 (#1832)
  Use safe math when pruning statuses (#1835)
  remote-ext: fix state download stall on slow connections and reduce memory usage (#1295)
  Update testnet bootnode dns name (#1712)
  [FRAME] Warn on unchecked weight witness (#1818)
  [xcm] Use `Weight::MAX` for `reserve_asset_deposited`, `receive_teleported_asset` benchmarks (#1726)
  Update bridges subtree (#1803)
  Check for parent of first ready block being on chain (#1812)
  Make CheckNonce refuse transactions signed by accounts with no providers (#1578)
  Fix Asset Hub collator crashing when starting from genesis (#1788)
  Mixnet integration (#1346)
  [xcm-emulator] Decouple the `AccountId` type from `AccountId32` (#1458)
  Treasury spends various asset kinds (#1333)
  chore: bump zombienter version (#1806)
  ...
bkontur added a commit that referenced this pull request Oct 25, 2023
…yLocation` and `SystemParachains` in the same way (#2023)

This PR addresses several issues:
- simplify referencing `RelayTreasuryLocation` without needing
additional `RelayTreasury` struct
- fix for referencing `SystemParachains` from parachain with `parents:
1` instead of `parents: 0`
- removed hard-coded constants and fix tests for `asset-hub-rococo`
which was merged to master after
#1726

---------

Co-authored-by: command-bot <>
s0me0ne-unkn0wn pushed a commit that referenced this pull request Oct 29, 2023
…yLocation` and `SystemParachains` in the same way (#2023)

This PR addresses several issues:
- simplify referencing `RelayTreasuryLocation` without needing
additional `RelayTreasury` struct
- fix for referencing `SystemParachains` from parachain with `parents:
1` instead of `parents: 0`
- removed hard-coded constants and fix tests for `asset-hub-rococo`
which was merged to master after
#1726

---------

Co-authored-by: command-bot <>
EgorPopelyaev pushed a commit that referenced this pull request Nov 3, 2023
…yLocation` and `SystemParachains` in the same way (#2023)

This PR addresses several issues:
- simplify referencing `RelayTreasuryLocation` without needing
additional `RelayTreasury` struct
- fix for referencing `SystemParachains` from parachain with `parents:
1` instead of `parents: 0`
- removed hard-coded constants and fix tests for `asset-hub-rococo`
which was merged to master after
#1726

---------

Co-authored-by: command-bot <>
svyatonik added a commit to svyatonik/runtimes that referenced this pull request Nov 9, 2023
bkchr pushed a commit that referenced this pull request Apr 10, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
T6-XCM This PR/Issue is related to XCM. T14-system_parachains This PR/Issue is related to system parachains.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

XCM Weights/Benchmarks TODOs
5 participants