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

Allow for 0 existential deposit in benchmarks for pallet_staking, pallet_session, and pallet_balances #4346

Merged
merged 8 commits into from
May 2, 2024

Conversation

krisbitney
Copy link
Contributor

@krisbitney krisbitney commented May 1, 2024

This PR ensures non-zero values are available in benchmarks for pallet_staking, pallet_session, and pallet_balances where required for them to run.

This small change makes it possible to run the benchmarks for pallet_staking, pallet_session, and pallet_balances in a runtime for which existential deposit is set to 0.

The benchmarks for pallet_staking and pallet_session will still fail in runtimes that use U128CurrencyToVote, but that is easy to work around by creating a new CurrencyToVote implementation for benchmarking.

The changes are implemented by checking if existential deposit equals 0 and using 1 if so.

@krisbitney krisbitney requested a review from a team as a code owner May 1, 2024 17:47
@cla-bot-2021
Copy link

cla-bot-2021 bot commented May 1, 2024

User @krisbitney, please sign the CLA here.

@benefacto
Copy link

For additional context: The changes ensure that the minimum values used in benchmarks are never less than one. This is achieved by using the .max() function to set a floor value of 1u32 for balances and 1u64 for amounts, respectively.

@bkchr bkchr added the T1-FRAME This PR/Issue is related to core FRAME, the framework. label May 2, 2024
@bkchr
Copy link
Member

bkchr commented May 2, 2024

bot fmt

@command-bot
Copy link

command-bot bot commented May 2, 2024

@bkchr https://gitlab.parity.io/parity/mirrors/polkadot-sdk/-/jobs/6109389 was started for your command "$PIPELINE_SCRIPTS_DIR/commands/fmt/fmt.sh". 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 7-0eb34928-a9e1-4db2-9ff9-415ec037f2d4 to cancel this command or bot cancel to cancel all commands in this pull request.

@command-bot
Copy link

command-bot bot commented May 2, 2024

@bkchr Command "$PIPELINE_SCRIPTS_DIR/commands/fmt/fmt.sh" has finished. Result: https://gitlab.parity.io/parity/mirrors/polkadot-sdk/-/jobs/6109389 has finished. If any artifacts were generated, you can download them from https://gitlab.parity.io/parity/mirrors/polkadot-sdk/-/jobs/6109389/artifacts/download.

@bkchr
Copy link
Member

bkchr commented May 2, 2024

@krisbitney could you please write a prdoc

Copy link

github-actions bot commented May 2, 2024

Review required! Latest push from author must always be reviewed

@krisbitney
Copy link
Contributor Author

@krisbitney could you please write a prdoc

Done!

Comment on lines 14 to 15
- name: pallet-session
bump: patch
Copy link
Contributor

Choose a reason for hiding this comment

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

maybe I am missing something, but I don't see any changes to pallet-session here?

Copy link
Contributor Author

@krisbitney krisbitney May 2, 2024

Choose a reason for hiding this comment

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

I just pushed a commit changing it to pallet-session-benchmarking, which depends on pallet_staking::test_utils. Thanks for the correction.

@bkchr bkchr enabled auto-merge May 2, 2024 19:51
@bkchr bkchr added this pull request to the merge queue May 2, 2024
Merged via the queue into paritytech:master with commit a9aeabe May 2, 2024
140 of 142 checks passed
dcolley added a commit to metaspan/polkadot-sdk that referenced this pull request May 6, 2024
* 'master' of https://github.com/metaspan/polkadot-sdk: (65 commits)
  Introduces `TypeWithDefault<T, D: Get<T>>` (paritytech#4034)
  Publish `polkadot-sdk-frame`  crate (paritytech#4370)
  Add validate field to prdoc (paritytech#4368)
  State trie migration on asset-hub westend and collectives westend (paritytech#4185)
  Fix: dust unbonded for zero existential deposit (paritytech#4364)
  Bridge: added subcommand to relay single parachain header (paritytech#4365)
  Bridge: fix zombienet tests (paritytech#4367)
  [WIP][CI] Add more GHA jobs (paritytech#4270)
  Allow for 0 existential deposit in benchmarks for `pallet_staking`, `pallet_session`, and `pallet_balances` (paritytech#4346)
  Deprecate `NativeElseWasmExecutor` (paritytech#4329)
  More `xcm::v4` cleanup and `xcm_fee_payment_runtime_api::XcmPaymentApi` nits (paritytech#4355)
  sc-tracing: enable env-filter feature (paritytech#4357)
  deps: update jsonrpsee to v0.22.5 (paritytech#4330)
  Add PoV-reclaim enablement guide to polkadot-sdk-docs (paritytech#4244)
  cargo: Update experimental litep2p to latest version (paritytech#4344)
  Bridge: ignore client errors when calling recently added `*_free_headers_interval` methods (paritytech#4350)
  Make parachain template great again (and async backing ready) (paritytech#4295)
  [Backport] Version bumps and reorg prdocs from 1.11.0 (paritytech#4336)
  HRMP - set `DefaultChannelSizeAndCapacityWithSystem` with dynamic values according to the `ActiveConfig` (paritytech#4332)
  Statement Distribution Per Peer Rate Limit (paritytech#3444)
  ...
TarekkMA pushed a commit to moonbeam-foundation/polkadot-sdk that referenced this pull request Aug 2, 2024
…pallet_session`, and `pallet_balances` (paritytech#4346)

This PR ensures non-zero values are available in benchmarks for
`pallet_staking`, `pallet_session`, and `pallet_balances` where required
for them to run.

This small change makes it possible to run the benchmarks for
`pallet_staking`, `pallet_session`, and `pallet_balances` in a runtime
for which existential deposit is set to 0.

The benchmarks for `pallet_staking` and `pallet_session` will still fail
in runtimes that use `U128CurrencyToVote`, but that is easy to work
around by creating a new `CurrencyToVote` implementation for
benchmarking.

The changes are implemented by checking if existential deposit equals 0
and using 1 if so.

---------

Co-authored-by: command-bot <>
Co-authored-by: Bastian Köcher <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
T1-FRAME This PR/Issue is related to core FRAME, the framework.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants