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

[pallet-broker] Fix auto renew benchmarks #6505

Merged
merged 19 commits into from
Dec 12, 2024
Merged

Conversation

seadanda
Copy link
Contributor

@seadanda seadanda commented Nov 15, 2024

Fix the broker pallet auto-renew benchmarks which have been broken since #4424, yielding Weightless due to some prices being set too low, as reported in #6474.

Upon further investigation it turned out that the auto-renew contribution to rotate_sale was always failing but the error was mapped. This is also fixed at the cost of a bit of setup overhead.

Fixes #6474

TODO:

  • Re-run weights

@seadanda seadanda added T2-pallets This PR/Issue is related to a particular pallet. T12-benchmarks This PR/Issue is related to benchmarking and weights. labels Nov 15, 2024
@seadanda seadanda requested a review from gui1117 November 15, 2024 18:21
@seadanda seadanda self-assigned this Nov 15, 2024
@seadanda
Copy link
Contributor Author

/cmd prdoc --audience runtime_dev --bump minor

let region = Broker::<T>::do_purchase(caller.clone(), 10u32.into())
.map_err(|_| BenchmarkError::Weightless)?;
let region = Broker::<T>::do_purchase(caller.clone(), 10_000_000u32.into())
.expect("Offer not high enough for configuration.");
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Since we advance to the fixed price phase, this is independent of the price adapter implementation and depends only on the config value set earlier in this file. Therefore it's better to unwrap here so the test fails, rather than always being mapped to Weightless.

@seadanda
Copy link
Contributor Author

bot bench substrate-pallet --pallet=pallet_broker

@seadanda
Copy link
Contributor Author

bot bench cumulus-coretime --runtime=coretime-westend --pallet=pallet_broker
bot bench cumulus-coretime --runtime=coretime-rococo --pallet=pallet_broker

@seadanda
Copy link
Contributor Author

bot bench substrate-pallet --features=runtime-benchmarks --pallet=pallet_broker

@seadanda
Copy link
Contributor Author

bot bench cumulus-coretime --features=runtime-benchmarks --runtime=coretime-westend --pallet=pallet_broker
bot bench cumulus-coretime --features=runtime-benchmarks --runtime=coretime-rococo --pallet=pallet_broker

@seadanda
Copy link
Contributor Author

This will affect polkadot-fellows/runtimes#490

Until this PR is merged and backported we should filter these calls and set MaxAutoRenewals to 0

@seadanda seadanda marked this pull request as ready for review November 19, 2024 16:55
@seadanda seadanda requested a review from a team as a code owner November 19, 2024 16:55
…=dev --target_dir=substrate --features=runtime-benchmarks --pallet=pallet_broker
@seadanda
Copy link
Contributor Author

bot bench cumulus-coretime --features=runtime-benchmarks --runtime=coretime-westend --pallet=pallet_broker
bot bench cumulus-coretime --features=runtime-benchmarks --runtime=coretime-rococo --pallet=pallet_broker

command-bot and others added 4 commits December 11, 2024 17:09
…=coretime-westend --runtime_dir=coretime --target_dir=cumulus --features=runtime-benchmarks --pallet=pallet_broker
…=coretime-rococo --runtime_dir=coretime --target_dir=cumulus --features=runtime-benchmarks --pallet=pallet_broker
@seadanda
Copy link
Contributor Author

bot clean

@command-bot command-bot bot deleted a comment from paritytech-workflow-stopper bot Dec 11, 2024
@command-bot
Copy link

command-bot bot commented Dec 11, 2024

@seadanda Command "$PIPELINE_SCRIPTS_DIR/commands/bench/bench.sh" --subcommand=pallet --runtime=dev --target_dir=substrate --features=runtime-benchmarks --pallet=pallet_broker has finished. Result: https://gitlab.parity.io/parity/mirrors/polkadot-sdk/-/jobs/7905856 has finished. If any artifacts were generated, you can download them from https://gitlab.parity.io/parity/mirrors/polkadot-sdk/-/jobs/7905856/artifacts/download.

command-bot added 2 commits December 11, 2024 23:26
…=coretime-westend --runtime_dir=coretime --target_dir=cumulus --features=runtime-benchmarks --pallet=pallet_broker
…=coretime-rococo --runtime_dir=coretime --target_dir=cumulus --features=runtime-benchmarks --pallet=pallet_broker
@command-bot
Copy link

command-bot bot commented Dec 11, 2024

@seadanda Command "$PIPELINE_SCRIPTS_DIR/commands/bench/bench.sh" --subcommand=pallet --runtime=coretime-westend --runtime_dir=coretime --target_dir=cumulus --features=runtime-benchmarks --pallet=pallet_broker has finished. Result: https://gitlab.parity.io/parity/mirrors/polkadot-sdk/-/jobs/7905857 has finished. If any artifacts were generated, you can download them from https://gitlab.parity.io/parity/mirrors/polkadot-sdk/-/jobs/7905857/artifacts/download.

@command-bot
Copy link

command-bot bot commented Dec 11, 2024

@seadanda Command "$PIPELINE_SCRIPTS_DIR/commands/bench/bench.sh" --subcommand=pallet --runtime=coretime-rococo --runtime_dir=coretime --target_dir=cumulus --features=runtime-benchmarks --pallet=pallet_broker has finished. Result: https://gitlab.parity.io/parity/mirrors/polkadot-sdk/-/jobs/7905858 has finished. If any artifacts were generated, you can download them from https://gitlab.parity.io/parity/mirrors/polkadot-sdk/-/jobs/7905858/artifacts/download.

@bkchr bkchr enabled auto-merge December 12, 2024 19:06
@bkchr bkchr added this pull request to the merge queue Dec 12, 2024
@seadanda seadanda removed this pull request from the merge queue due to a manual request Dec 12, 2024
@seadanda seadanda enabled auto-merge December 12, 2024 19:44
@seadanda seadanda added this pull request to the merge queue Dec 12, 2024
Merged via the queue into master with commit 459b4a6 Dec 12, 2024
198 of 199 checks passed
@seadanda seadanda deleted the donal-auto-renew-benchmarks branch December 12, 2024 21:01
@seadanda seadanda added the A4-needs-backport Pull request must be backported to all maintained releases. label Dec 13, 2024
@paritytech-cmd-bot-polkadot-sdk

Created backport PR for stable2407:

Please cherry-pick the changes locally and resolve any conflicts.

git fetch origin backport-6505-to-stable2407
git worktree add --checkout .worktree/backport-6505-to-stable2407 backport-6505-to-stable2407
cd .worktree/backport-6505-to-stable2407
git reset --hard HEAD^
git cherry-pick -x 459b4a6521d35f3e84a036262e64fa547a5b1ff4
git push --force-with-lease

@paritytech-cmd-bot-polkadot-sdk

Created backport PR for stable2409:

Please cherry-pick the changes locally and resolve any conflicts.

git fetch origin backport-6505-to-stable2409
git worktree add --checkout .worktree/backport-6505-to-stable2409 backport-6505-to-stable2409
cd .worktree/backport-6505-to-stable2409
git reset --hard HEAD^
git cherry-pick -x 459b4a6521d35f3e84a036262e64fa547a5b1ff4
git push --force-with-lease

@paritytech-cmd-bot-polkadot-sdk

Created backport PR for stable2412:

Please cherry-pick the changes locally and resolve any conflicts.

git fetch origin backport-6505-to-stable2412
git worktree add --checkout .worktree/backport-6505-to-stable2412 backport-6505-to-stable2412
cd .worktree/backport-6505-to-stable2412
git reset --hard HEAD^
git cherry-pick -x 459b4a6521d35f3e84a036262e64fa547a5b1ff4
git push --force-with-lease

seadanda added a commit that referenced this pull request Dec 13, 2024
Fix the broker pallet auto-renew benchmarks which have been broken since
reported in #6474.

Upon further investigation it turned out that the auto-renew
contribution to `rotate_sale` was always failing but the error was
mapped. This is also fixed at the cost of a bit of setup overhead.

Fixes #6474

TODO:
- [x] Re-run weights

---------

Co-authored-by: GitHub Action <[email protected]>
Co-authored-by: command-bot <>
Co-authored-by: Bastian Köcher <[email protected]>
(cherry picked from commit 459b4a6)
@seadanda seadanda had a problem deploying to subsystem-benchmarks December 13, 2024 17:17 — with GitHub Actions Failure
seadanda added a commit that referenced this pull request Dec 13, 2024
Fix the broker pallet auto-renew benchmarks which have been broken since
reported in #6474.

Upon further investigation it turned out that the auto-renew
contribution to `rotate_sale` was always failing but the error was
mapped. This is also fixed at the cost of a bit of setup overhead.

Fixes #6474

TODO:
- [x] Re-run weights

---------

Co-authored-by: GitHub Action <[email protected]>
Co-authored-by: command-bot <>
Co-authored-by: Bastian Köcher <[email protected]>
(cherry picked from commit 459b4a6)
EgorPopelyaev pushed a commit that referenced this pull request Dec 16, 2024
Backport #6505 into `stable2412` from seadanda.

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: Dónal Murray <[email protected]>
EgorPopelyaev pushed a commit that referenced this pull request Dec 17, 2024
Backport #6505 into `stable2409` from seadanda.

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: Dónal Murray <[email protected]>
Co-authored-by: command-bot <>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A4-needs-backport Pull request must be backported to all maintained releases. T2-pallets This PR/Issue is related to a particular pallet. T12-benchmarks This PR/Issue is related to benchmarking and weights.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

pallet broker: some benchmarks are weightless for kitchensink runtime. Resulting in substrate weight being 0
4 participants