-
Notifications
You must be signed in to change notification settings - Fork 865
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
Use non-native token to benchmark xcm on asset hub #7893
Conversation
/cmd fmt |
/cmd prdoc --audience runtime_dev --bump patch |
…time_dev --bump patch'
/cmd bench --runtime asset-hub-westend --pallet pallet_xcm_benchmarks::fungible |
Command "bench --runtime asset-hub-westend --pallet pallet_xcm_benchmarks::generic" has started 🚀 See logs here |
Command "bench --runtime asset-hub-westend --pallet pallet_xcm_benchmarks::fungible" has started 🚀 See logs here |
…t-hub-westend --pallet pallet_xcm_benchmarks::generic'
Command "bench --runtime asset-hub-westend --pallet pallet_xcm_benchmarks::generic" has finished ✅ See logs here Subweight results:
Command output:✅ Successful benchmarks of runtimes/pallets: |
…t-hub-westend --pallet pallet_xcm_benchmarks::fungible'
Command "bench --runtime asset-hub-westend --pallet pallet_xcm_benchmarks::fungible" has finished ✅ See logs here Subweight results:
Command output:✅ Successful benchmarks of runtimes/pallets: |
&account, | ||
<Balances as Inspect<_>>::minimum_balance(), | ||
)); | ||
let asset_id = 1984; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
1984
: Big Brother is watching you! 👀
@bkontur we could merge it but I'd rather just merge this once it's acceptable since it's only for asset hubs. |
/cmd bench --runtime asset-hub-rococo --pallet pallet_xcm_benchmarks::fungible |
Command "bench --runtime asset-hub-rococo --pallet pallet_xcm_benchmarks::fungible" has started 🚀 See logs here |
/cmd bench --runtime asset-hub-westend asset-hub-rococo --pallet pallet_xcm |
Command "bench --runtime asset-hub-westend asset-hub-rococo --pallet pallet_xcm" has started 🚀 See logs here |
@@ -354,7 +328,6 @@ benchmarks_instance_pallet! { | |||
}: { | |||
executor.bench_process(xcm)?; | |||
} verify { | |||
assert!(T::TransactAsset::balance(&sender_account) <= sender_account_balance_before); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we at least check/assert that executor.holding()
is changed? Should it change all the time?
@@ -46,8 +46,6 @@ benchmarks_instance_pallet! { | |||
let asset = T::get_asset(); | |||
|
|||
<AssetTransactorOf<T>>::deposit_asset(&asset, &sender_location, None).unwrap(); | |||
// check the assets of origin. | |||
assert!(!T::TransactAsset::balance(&sender_account).is_zero()); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@franciscoaguirre I don't see it from PR, but did you remove all T::TransactAsset
? If we don't need it, maybe we could remove it also or at least where_clause { where
at the beginning?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I left one where we test that we take delivery fees, which we are only using the native asset for now.
Command "bench --runtime asset-hub-rococo --pallet pallet_xcm_benchmarks::fungible" has finished ✅ See logs here Subweight results:
Command output:✅ Successful benchmarks of runtimes/pallets: |
…t-hub-westend asset-hub-rococo --pallet pallet_xcm'
Command "bench --runtime asset-hub-westend asset-hub-rococo --pallet pallet_xcm" has finished ✅ See logs here Subweight results:
Command output:✅ Successful benchmarks of runtimes/pallets: |
All GitHub workflows were cancelled due to failure one of the required jobs. |
Asset Hub was using the native token for benchmarking xcm instructions. This is not the best since it's cheaper than using something in `pallet-assets` for example. Had to remove some restrictive checks from `pallet-xcm-benchmarks`. I'll bring back the checks with a better framework in the future that allows for handling multiple assets (`fungibles::*` traits). --------- Co-authored-by: cmd[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Created backport PR for
Please cherry-pick the changes locally and resolve any conflicts. git fetch origin backport-7893-to-stable2407
git worktree add --checkout .worktree/backport-7893-to-stable2407 backport-7893-to-stable2407
cd .worktree/backport-7893-to-stable2407
git reset --hard HEAD^
git cherry-pick -x c4b8ec123afcef596fbc4ea3239ff9e392bcaf36
git push --force-with-lease |
Created backport PR for
Please cherry-pick the changes locally and resolve any conflicts. git fetch origin backport-7893-to-stable2409
git worktree add --checkout .worktree/backport-7893-to-stable2409 backport-7893-to-stable2409
cd .worktree/backport-7893-to-stable2409
git reset --hard HEAD^
git cherry-pick -x c4b8ec123afcef596fbc4ea3239ff9e392bcaf36
git push --force-with-lease |
Created backport PR for
Please cherry-pick the changes locally and resolve any conflicts. git fetch origin backport-7893-to-stable2412
git worktree add --checkout .worktree/backport-7893-to-stable2412 backport-7893-to-stable2412
cd .worktree/backport-7893-to-stable2412
git reset --hard HEAD^
git cherry-pick -x c4b8ec123afcef596fbc4ea3239ff9e392bcaf36
git push --force-with-lease |
Created backport PR for
Please cherry-pick the changes locally and resolve any conflicts. git fetch origin backport-7893-to-stable2503
git worktree add --checkout .worktree/backport-7893-to-stable2503 backport-7893-to-stable2503
cd .worktree/backport-7893-to-stable2503
git reset --hard HEAD^
git cherry-pick -x c4b8ec123afcef596fbc4ea3239ff9e392bcaf36
git push --force-with-lease |
* origin: (325 commits) Add an extra_constant to pallet-treasury (#7918) Bump the ci_dependencies group across 1 directory with 4 updates (#7855) remove compromised action (#7934) Fixing token-economics dead link (#5302) [pallet-revive] Fix pallet-revive-fixtures build.rs (#7928) cumulus: fix pov exporter format (#7923) sp-api: Support `mut` in `impl_runtime_apis!` (#7924) Remove clones from block seal function (#7917) [pallet-revive] precompiles 2->9 (#7810) Use non-native token to benchmark xcm on asset hub (#7893) [CI] bump timeout wait for build in zombienet workflows. (#7871) taplo: split long array line to multiline array (#7905) [pallet-revive] fixture as dev dep (#7844) notifications/libp2p: Punish notification protocol misbehavior on outbound substreams (#7781) [Release|CI/CD] Update version of the cache action in the Publish docker ci (#7892) Remove `pallet::getter` usage from bridges/modules (#7120) [pallet-revive] Support blocktag in eth_getLogs RPC (#7879) Improve error message in benchmark macro (#7873) staking: add `manual_slash` extrinsic (#7805) Remove execute_with_origin implementation in the XCM executor (#7889) ...
* master: (58 commits) Upgrade link-checker cache to v4 (#7874) Updating readmes (#7950) Cumulus: Remove some old scripts (#7946) pallet-bounties: allow bounties to never expire (#7723) run frame-omni-bencher overhead command in CI for all runtimes in the runtime matrix (#7459) Update README.md for Cumulus (#7930) FRAME: Meta Transaction (#6428) Follow up for: Use the umbrella crate for the parachain template #5993 (#7464) Add an extra_constant to pallet-treasury (#7918) Bump the ci_dependencies group across 1 directory with 4 updates (#7855) remove compromised action (#7934) Fixing token-economics dead link (#5302) [pallet-revive] Fix pallet-revive-fixtures build.rs (#7928) cumulus: fix pov exporter format (#7923) sp-api: Support `mut` in `impl_runtime_apis!` (#7924) Remove clones from block seal function (#7917) [pallet-revive] precompiles 2->9 (#7810) Use non-native token to benchmark xcm on asset hub (#7893) [CI] bump timeout wait for build in zombienet workflows. (#7871) taplo: split long array line to multiline array (#7905) ...
Backport #7893 into `stable2412` from franciscoaguirre. See the [documentation](https://github.com/paritytech/polkadot-sdk/blob/master/docs/BACKPORT.md) on how to use this bot. <!-- # To be used by other automation, do not modify: original-pr-number: #${pull_number} --> --------- Co-authored-by: Francisco Aguirre <[email protected]>
Backport #7893 into `stable2503` from franciscoaguirre. See the [documentation](https://github.com/paritytech/polkadot-sdk/blob/master/docs/BACKPORT.md) on how to use this bot. <!-- # To be used by other automation, do not modify: original-pr-number: #${pull_number} --> --------- Co-authored-by: Francisco Aguirre <[email protected]> Co-authored-by: Adrian Catangiu <[email protected]>
Asset Hub was using the native token for benchmarking xcm instructions. This is not the best since it's cheaper than using something in
pallet-assets
for example.Had to remove some restrictive checks from
pallet-xcm-benchmarks
.I'll bring back the checks with a better framework in the future that allows for handling multiple assets (
fungibles::*
traits).