You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
Context: MinimumValidatorSetSize on the RCs is not a parameter that controls the validator set-size in any way (that's staking.ValidatorCount in AHs). It is only a safety mechanism that ensures no matter what AH does, RC ignores a set smaller than this size.
The StakingAsync on AHs already has strict checks where any set smaller than ValidatorCount are rejected on the AH side.
Why keeping MinimumValidatorSetSize flexible via a storage item vs. const? Because if not, any reduction of validator set requires a runtime upgrade on the RC first, slowing down the process.
Why pallet-parameters for the flexibility? Unlike parameter_types { pub storage } the origin can be flexible, and the proposal to upgrade it is more transparent than a system.setStorage.
Why lower defaults?
As mentioned above, these lower limits won't have any meaningful impact, until Staking.ValidatorCount on AH is changed.
Since reduction of sets is already envisioned as a part of the DAP proposal, this flexibility + lower defaults speeds up that effort.
Why 100 and 250? They follow the the core count and usage in each respective network.
The new dynamic parameter AhClient::MinimumValidatorSetSize is added, but the #[cfg(feature = "runtime-benchmarks")] impl Default for RuntimeParameters was not updated to provide a default value for it. That means pallet-parameters benchmarking/support code will not have a successful default parameter case for the new variant, unlike the Polkadot runtime. This can break runtime-benchmark behavior for the new pallet parameter and leaves Kusama inconsistent with the new wiring. Add an AhClient default variant here, or otherwise extend the benchmark-only default selection so the new parameter is covered.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
kianenigma
changed the title
And, this PR lowers the default limits to:
Reasoning:
MinimumValidatorSetSizeon the RCs is not a parameter that controls the validator set-size in any way (that'sstaking.ValidatorCountin AHs). It is only a safety mechanism that ensures no matter what AH does, RC ignores a set smaller than this size.StakingAsyncon AHs already has strict checks where any set smaller thanValidatorCountare rejected on the AH side.MinimumValidatorSetSizeflexible via a storage item vs.const? Because if not, any reduction of validator set requires a runtime upgrade on the RC first, slowing down the process.pallet-parametersfor the flexibility? Unlikeparameter_types { pub storage }the origin can be flexible, and the proposal to upgrade it is more transparent than asystem.setStorage.Staking.ValidatorCounton AH is changed.