-
Notifications
You must be signed in to change notification settings - Fork 739
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
Add storage bounds for pallet staking
and clean up deprecated non paged exposure storages
#6445
base: master
Are you sure you want to change the base?
Changes from 122 commits
6c808b6
1b18bb1
0ac60af
4edb8e1
65c7de6
35069c9
d4b15af
eb93a70
58cf45d
292c5d0
74f2055
63f2c5e
b249a67
362f505
4f21d53
e258baf
4b7d896
01e2233
243bec8
a599afc
9a25fbc
98daabc
aec5087
36204da
18e9bee
cbfd4a5
b5cbfec
9ca25a1
10e7295
b3186b1
336137a
c1911e1
2ac794e
583e46d
6347a8c
9e82b85
1db7f39
f996a97
e030506
3ab45d9
a980daa
827e898
c4d5142
df8e567
c066632
686b6b3
9cf2263
21ad9ea
ca881ac
e986264
ea59383
9a21b8f
47863bb
50ce1a1
40ccdec
c3365a6
584fe53
5333a7d
31a9300
898d469
32df05d
7bc9b98
9c4d874
536d5c5
608ec88
76469ea
fb1a4fa
181fe4d
b1f5f97
9c002c1
503a7d9
63a18b1
49059c0
efde7a7
8186680
3fb6ca7
66923a4
bec0544
3708085
73af22a
1139689
2bda8eb
fe3147b
cd8a0c3
5b15e24
9d055a8
07b021b
87e0e46
473e0ff
eb64ffc
74d6c7a
94f5b31
7b1fc5e
cf9d8c3
c265974
bb5229c
6588714
4754629
76a2d4b
67c41fa
bd28db5
043b7d6
68481f4
312904e
d508f8d
529fa0f
17eeef7
21a12d1
e2fa0ca
4bf3cd6
f7c1fe4
c881bb0
306cd41
2da736a
831f1b0
3702ce5
489fc9d
65fc95b
9c88d39
a5e98e6
7b82520
30520af
0d47dc5
4b58f0a
33e59b2
c889905
300b7f9
fc63dc7
720c9eb
a065a91
483eb03
7aeb8a5
c51d09b
1a9be6b
235a260
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -33,7 +33,7 @@ use sp_consensus_grandpa::AuthorityId as GrandpaId; | |
use sp_core::{crypto::get_public_from_string_or_panic, sr25519}; | ||
use sp_genesis_builder::PresetId; | ||
use sp_keyring::Sr25519Keyring; | ||
use sp_runtime::Perbill; | ||
use sp_runtime::{BoundedVec, Perbill}; | ||
use westend_runtime_constants::currency::UNITS as WND; | ||
|
||
/// Helper function to generate stash, controller and session key from seed | ||
|
@@ -202,7 +202,10 @@ fn westend_testnet_genesis( | |
.iter() | ||
.map(|x| (x.0.clone(), x.0.clone(), STASH, StakerStatus::<AccountId>::Validator)) | ||
.collect::<Vec<_>>(), | ||
invulnerables: initial_authorities.iter().map(|x| x.0.clone()).collect::<Vec<_>>(), | ||
invulnerables: BoundedVec::try_from( | ||
initial_authorities.iter().map(|x| x.0.clone()).collect::<Vec<_>>() | ||
) | ||
.expect("Too many invulnerable validators!"), | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Error message can be a bit more informative, hinting at which config should be tweaked if this There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. error message improved in 0d47dc5 |
||
force_era: Forcing::NotForcing, | ||
slash_reward_fraction: Perbill::from_percent(10), | ||
}, | ||
|
@@ -373,7 +376,10 @@ fn westend_staging_testnet_config_genesis() -> serde_json::Value { | |
.iter() | ||
.map(|x| (x.0.clone(), x.0.clone(), STASH, StakerStatus::<AccountId>::Validator)) | ||
.collect::<Vec<_>>(), | ||
invulnerables: initial_authorities.iter().map(|x| x.0.clone()).collect::<Vec<_>>(), | ||
invulnerables: BoundedVec::try_from( | ||
initial_authorities.iter().map(|x| x.0.clone()).collect::<Vec<_>>() | ||
) | ||
.expect("Too many invulnerable validators!"), | ||
force_era: Forcing::ForceNone, | ||
slash_reward_fraction: Perbill::from_percent(10), | ||
}, | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -287,6 +287,14 @@ pub mod dynamic_params { | |
#[codec(index = 4)] | ||
pub static UseAuctionSlots: bool = false; | ||
} | ||
|
||
#[dynamic_pallet_params] | ||
#[codec(index = 1)] | ||
pub mod staking { | ||
/// Maximum number of validators allowed. | ||
#[codec(index = 0)] | ||
pub static MaxValidatorsCount: u32 = 300; | ||
} | ||
} | ||
|
||
#[cfg(feature = "runtime-benchmarks")] | ||
|
@@ -319,6 +327,7 @@ impl EnsureOriginWithArg<RuntimeOrigin, RuntimeParametersKey> for DynamicParamet | |
|
||
match key { | ||
Inflation(_) => frame_system::ensure_root(origin.clone()), | ||
Staking(_) => frame_system::ensure_root(origin.clone()), | ||
} | ||
.map_err(|_| origin) | ||
} | ||
|
@@ -716,6 +725,7 @@ parameter_types! { | |
pub const SessionsPerEra: SessionIndex = prod_or_fast!(6, 1); | ||
// 2 eras for unbonding (12 hours). | ||
pub const BondingDuration: sp_staking::EraIndex = 2; | ||
pub const MaxBondedEras: u32 = (BondingDuration::get() as u32) + 1; | ||
// 1 era in which slashes can be cancelled (6 hours). | ||
pub const SlashDeferDuration: sp_staking::EraIndex = 1; | ||
pub const MaxExposurePageSize: u32 = 64; | ||
|
@@ -738,6 +748,7 @@ impl pallet_staking::Config for Runtime { | |
type Reward = (); | ||
type SessionsPerEra = SessionsPerEra; | ||
type BondingDuration = BondingDuration; | ||
type MaxBondedEras = MaxBondedEras; | ||
type SlashDeferDuration = SlashDeferDuration; | ||
type AdminOrigin = EitherOf<EnsureRoot<AccountId>, StakingAdmin>; | ||
type SessionInterface = Self; | ||
|
@@ -756,6 +767,10 @@ impl pallet_staking::Config for Runtime { | |
type EventListeners = (NominationPools, DelegatedStaking); | ||
type WeightInfo = weights::pallet_staking::WeightInfo<Runtime>; | ||
type DisablingStrategy = pallet_staking::UpToLimitWithReEnablingDisablingStrategy; | ||
type MaxInvulnerables = frame_support::traits::ConstU32<20>; | ||
type MaxRewardPagesPerValidator = frame_support::traits::ConstU32<20>; | ||
type MaxValidatorsCount = dynamic_params::staking::MaxValidatorsCount; | ||
type MaxDisabledValidators = ConstU32<100>; | ||
} | ||
|
||
impl pallet_fast_unstake::Config for Runtime { | ||
|
@@ -1417,7 +1432,6 @@ impl parachains_slashing::Config for Runtime { | |
ReportLongevity, | ||
>; | ||
type WeightInfo = weights::polkadot_runtime_parachains_disputes_slashing::WeightInfo<Runtime>; | ||
type BenchmarkingConfig = parachains_slashing::BenchConfig<300>; | ||
} | ||
|
||
parameter_types! { | ||
|
@@ -1841,6 +1855,7 @@ pub mod migrations { | |
parachains_shared::migration::MigrateToV1<Runtime>, | ||
parachains_scheduler::migration::MigrateV2ToV3<Runtime>, | ||
pallet_staking::migrations::v16::MigrateV15ToV16<Runtime>, | ||
pallet_staking::migrations::v17::MigrateV16ToV17<Runtime>, | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. You can check the logs for this migration in this CI job: https://github.com/paritytech/polkadot-sdk/actions/runs/12412281493/job/34651846644?pr=6445 there seems to be some error-ish logs in there:
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Solved with EDIT: I can also see a third option: merge pages for old items until they are less than 20. |
||
// permanent | ||
pallet_xcm::migration::MigrateToLatestXcmVersion<Runtime>, | ||
); | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,51 @@ | ||
title: Add storage bounds for pallet `staking` and clean up deprecated non paged exposure storages | ||
doc: | ||
- audience: Runtime Dev | ||
description: |- | ||
This is part of #6289 and necessary for the Asset Hub migration. | ||
|
||
Building on the observations and suggestions from #255 . | ||
|
||
**Changes** | ||
|
||
- Add `MaxInvulnerables` to bound `Invulnerables` Vec -> `BoundedVec`. | ||
- Set to constant 20 in the pallet (must be >= 17 for backward compatibility of runtime `westend`). | ||
- Add `MaxDisabledValidators` to bound `DisabledValidators` Vec -> `BoundedVec` | ||
- Set to constant 100 in the pallet (it should be <= 1/3 * `MaxValidatorsCount` according to the current disabling strategy) | ||
- Remove `ErasStakers` and `ErasStakersClipped` (see #433 ) | ||
- They were deprecated in v14 and could have been removed since staking era 1504 (now it's > 1600) | ||
- Completing the task from #5986 | ||
Comment on lines
+15
to
+17
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think this should be included in the title, there might be other chain using the staking pallet, There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think it lacks guideline in regards to the clean up of deprecated non paged exposure storage items. The issue #433 state that 84 eras must pass for the migration to finish. I think the prdoc should mention what are the consequences of this cleanup.
I don't have the exact details of the consequences of not waiting for the 84 eras. But if you know what are the consequences then you can also write it down. We should keep in mind that prdoc is used by other projects using the staking pallets not only polkadot/kusama. |
||
- Use `MaxExposurePageSize` to bound `ErasStakersPaged` mapping to exposure pages: each `ExposurePage.others` Vec is turned into a `WeakBoundedVec` to allow easy and quick changes to this bound | ||
- Add `MaxBondedEras` to bound `BondedEras` Vec -> `BoundedVec` | ||
- Set to `BondingDuration::get() + 1` everywhere to include both time interval endpoints in [`current_era - BondingDuration::get()`, `current_era`]. Notice that this was done manually in every test and runtime. | ||
- Add `MaxRewardPagesPerValidator` to bound `ClaimedRewards` Vec of pages -> `WeakBoundedVec` | ||
- Set to constant 20 in the pallet. The vector of pages is now a `WeakBoundedVec` to allow easy and quick changes to this parameter | ||
- Remove `MaxValidatorsCount` optional storage item to add `MaxValidatorsCount` mandatory config parameter | ||
- Using it to to bound `EraRewardPoints.individual` BTreeMap -> `BoundedBTreeMap`; | ||
- Set to dynamic parameter in runtime westend so that changing it should not require migrations for it | ||
|
||
crates: | ||
- name: pallet-staking | ||
bump: major | ||
- name: westend-runtime | ||
bump: minor | ||
- name: sp-staking | ||
bump: minor | ||
- name: polkadot-runtime-common | ||
bump: patch | ||
- name: pallet-fast-unstake | ||
bump: patch | ||
- name: pallet-babe | ||
bump: patch | ||
- name: pallet-beefy | ||
bump: patch | ||
- name: pallet-delegated-staking | ||
bump: patch | ||
- name: pallet-grandpa | ||
bump: patch | ||
- name: pallet-root-offences | ||
bump: patch | ||
- name: polkadot-runtime-parachains | ||
bump: major | ||
- name: rococo-runtime | ||
bump: patch |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -694,6 +694,7 @@ pallet_staking_reward_curve::build! { | |
parameter_types! { | ||
pub const SessionsPerEra: sp_staking::SessionIndex = 6; | ||
pub const BondingDuration: sp_staking::EraIndex = 24 * 28; | ||
pub const MaxBondedEras: u32 = (BondingDuration::get() as u32) + 1; | ||
pub const SlashDeferDuration: sp_staking::EraIndex = 24 * 7; // 1/4 the bonding duration. | ||
pub const RewardCurve: &'static PiecewiseLinear<'static> = &REWARD_CURVE; | ||
pub const MaxNominators: u32 = 64; | ||
|
@@ -708,7 +709,6 @@ const MAX_QUOTA_NOMINATIONS: u32 = 16; | |
pub struct StakingBenchmarkingConfig; | ||
impl pallet_staking::BenchmarkingConfig for StakingBenchmarkingConfig { | ||
type MaxNominators = ConstU32<1000>; | ||
type MaxValidators = ConstU32<1000>; | ||
} | ||
|
||
impl pallet_staking::Config for Runtime { | ||
|
@@ -722,6 +722,7 @@ impl pallet_staking::Config for Runtime { | |
type Reward = (); // rewards are minted from the void | ||
type SessionsPerEra = SessionsPerEra; | ||
type BondingDuration = BondingDuration; | ||
type MaxBondedEras = MaxBondedEras; | ||
type SlashDeferDuration = SlashDeferDuration; | ||
/// A super-majority of the council can cancel the slash. | ||
type AdminOrigin = EitherOfDiverse< | ||
|
@@ -745,6 +746,10 @@ impl pallet_staking::Config for Runtime { | |
type WeightInfo = pallet_staking::weights::SubstrateWeight<Runtime>; | ||
type BenchmarkingConfig = StakingBenchmarkingConfig; | ||
type DisablingStrategy = pallet_staking::UpToLimitWithReEnablingDisablingStrategy; | ||
type MaxInvulnerables = ConstU32<20>; | ||
type MaxRewardPagesPerValidator = ConstU32<20>; | ||
type MaxValidatorsCount = ConstU32<300>; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Other pallets like session, babe, grandpa and such also have a notion of within this file, you can use one Taking it a step further, you can expose this as a part of
This might be too much for this PR, but good for you to be familiar with the pattern. Whenever they are multiple parameters within two pallets that have a logical dependency (they have to be equal, or one has to be larger than the other), you can remove the implicitness like this. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. |
||
type MaxDisabledValidators = ConstU32<100>; | ||
} | ||
|
||
impl pallet_fast_unstake::Config for Runtime { | ||
|
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.
The overall observation is that we had to add temporarily a lot of bounds to pallets just for benchmarks, but as staking has now proper bounds for all its storage, they are not needed ✅