-
Notifications
You must be signed in to change notification settings - Fork 1.2k
[Staking] Rotate era reward pots through a fixed-size pool #11930
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
Changes from all commits
3a80f94
31a2454
c9de4ac
7890f2b
7c0d831
2c7024a
1e74eb9
71e9739
8bfec51
057f96c
13d8d4c
ee77d62
9d3e301
54c3b01
2134eec
b1d6fc0
ed793f4
f3f4614
7bc958e
d06ad71
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 |
|---|---|---|
| @@ -0,0 +1,16 @@ | ||
| title: 'pallet-staking-async: Rotate era reward pots through a fixed-size pool' | ||
| doc: | ||
| - audience: Runtime Dev | ||
| description: |- | ||
| Era reward pot accounts are now drawn from a fixed pool of `POT_POOL_SIZE = 200` | ||
| accounts, indexed by `era % POT_POOL_SIZE`, instead of one fresh account per era. | ||
| This ensure we only use a fixed size of pot accounts for the lifetime of the | ||
| chain rather than growing per era. | ||
|
|
||
| An `integrity_test` enforces `POT_POOL_SIZE > HistoryDepth` so a slot is only | ||
| reused after its previous era has been pruned. | ||
| crates: | ||
| - name: pallet-staking-async | ||
| bump: minor | ||
| - name: asset-hub-westend-runtime | ||
| bump: minor | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -211,6 +211,7 @@ mod tests; | |
| pub mod asset; | ||
| pub mod election_size_tracker; | ||
| pub mod ledger; | ||
| pub mod migrations; | ||
| mod pallet; | ||
| pub mod reward; | ||
| pub mod session_rotation; | ||
|
|
@@ -567,14 +568,33 @@ impl<T: Config> Contains<T::AccountId> for AllStakers<T> { | |
| } | ||
| } | ||
|
|
||
| /// Size of the rotating pool of era-specific pot accounts. | ||
| /// | ||
| /// Era pots are addressed by `era % POT_POOL_SIZE`, so a pot account is reused | ||
| /// every `POT_POOL_SIZE` eras instead of a fresh account being created per era. | ||
| /// This bounds the total storage footprint contributed by era pot accounts to a | ||
| /// constant rather than growing with chain age. | ||
| /// | ||
| /// Must be strictly greater than [`Config::HistoryDepth`] so that a slot is only | ||
| /// reused after its previous era has been pruned and drained. The | ||
| /// [`integrity_test`] enforces this invariant at runtime startup. | ||
| pub(crate) const POT_POOL_SIZE: u32 = 200; | ||
|
Contributor
Author
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. Didn't expose this as config since it doesn't practically need to be configured. Only requirement is that it's bigger than HistoryDepth (enforced by integrity_test). Also intended to keep it independent of HistoryDepth so changing it doesn't affect pot account assignment. |
||
|
|
||
| /// Maps an era index to its slot in the rotating pot pool. | ||
| pub(crate) fn pot_slot(era: EraIndex) -> u32 { | ||
| era % POT_POOL_SIZE | ||
| } | ||
|
|
||
| /// Kind of reward managed by staking pots. | ||
| #[derive( | ||
| Debug, Clone, Copy, PartialEq, Eq, Encode, Decode, codec::DecodeWithMemTracking, TypeInfo, | ||
| )] | ||
| pub enum RewardKind { | ||
| /// Staker rewards (nominators + validators). | ||
| #[codec(index = 0)] | ||
|
Contributor
Author
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. Pinned variant indices to be safe from any future mishappenings |
||
| StakerRewards, | ||
| /// Pot for validator self-stake incentive. | ||
| #[codec(index = 1)] | ||
| ValidatorSelfStake, | ||
| } | ||
|
|
||
|
|
@@ -585,8 +605,11 @@ pub enum RewardKind { | |
| pub enum RewardPot { | ||
| /// General pot: funded by an external source (e.g. pallet-dap). | ||
| /// At era boundaries, staking snapshots the balance into an era-specific pot. | ||
| #[codec(index = 0)] | ||
| General(RewardKind), | ||
| /// Era-specific pot: snapshotted from the general pot at era boundaries. | ||
| /// See `POT_POOL_SIZE` for the slot rotation scheme. | ||
| #[codec(index = 1)] | ||
| Era(EraIndex, RewardKind), | ||
| } | ||
|
|
||
|
|
@@ -596,6 +619,9 @@ pub trait PotAccountProvider<AccountId> { | |
| } | ||
|
|
||
| /// Seed-based pot account provider for production use. | ||
| /// | ||
| /// Era pots are derived from `(slot, kind)` where `slot = era % POT_POOL_SIZE`, | ||
| /// so a fixed pool of accounts rotates instead of creating one per era. | ||
| pub struct Seed<S>(core::marker::PhantomData<S>); | ||
|
|
||
| impl<AccountId, S> PotAccountProvider<AccountId> for Seed<S> | ||
|
|
@@ -605,11 +631,21 @@ where | |
| { | ||
| fn pot_account(pot: RewardPot) -> AccountId { | ||
| use sp_runtime::traits::AccountIdConversion; | ||
| S::get().into_sub_account_truncating(pot) | ||
| // Era pots are addressed by slot (`era % POT_POOL_SIZE`), not by the | ||
| // raw era index, so a fixed pool of accounts rotates instead of | ||
| // growing per era. | ||
| let normalized = match pot { | ||
| RewardPot::Era(era, kind) => RewardPot::Era(pot_slot(era), kind), | ||
| other => other, | ||
| }; | ||
| S::get().into_sub_account_truncating(normalized) | ||
| } | ||
| } | ||
|
|
||
| /// Sequential pot account provider for testing. | ||
| /// | ||
| /// Mirrors the production rotation: era pots collide every `POT_POOL_SIZE` | ||
| /// eras so tests exercise the same pool reuse path. | ||
| #[cfg(feature = "std")] | ||
| pub struct SequentialTest; | ||
|
|
||
|
|
@@ -623,10 +659,10 @@ where | |
| RewardPot::General(RewardKind::StakerRewards) => AccountId::from(200_000u64), | ||
| RewardPot::General(RewardKind::ValidatorSelfStake) => AccountId::from(200_001u64), | ||
| RewardPot::Era(era, RewardKind::StakerRewards) => { | ||
| AccountId::from(100_000 + (era as u64 * 10)) | ||
| AccountId::from(100_000 + (pot_slot(era) as u64 * 10)) | ||
| }, | ||
| RewardPot::Era(era, RewardKind::ValidatorSelfStake) => { | ||
| AccountId::from(100_000 + (era as u64 * 10) + 1) | ||
| AccountId::from(100_000 + (pot_slot(era) as u64 * 10) + 1) | ||
| }, | ||
| } | ||
| } | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,194 @@ | ||
| // This file is part of Substrate. | ||
|
|
||
| // Copyright (C) Parity Technologies (UK) Ltd. | ||
| // SPDX-License-Identifier: Apache-2.0 | ||
|
|
||
| // Licensed under the Apache License, Version 2.0 (the "License"); | ||
| // you may not use this file except in compliance with the License. | ||
| // You may obtain a copy of the License at | ||
| // | ||
| // http://www.apache.org/licenses/LICENSE-2.0 | ||
| // | ||
| // Unless required by applicable law or agreed to in writing, software | ||
| // distributed under the License is distributed on an "AS IS" BASIS, | ||
| // WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. | ||
| // See the License for the specific language governing permissions and | ||
| // limitations under the License. | ||
|
|
||
| //! Storage migrations for the staking-async pallet. | ||
|
|
||
| use crate::{log, reward::EraRewardManager, Config, DisableMintingGuard, RewardKind, RewardPot}; | ||
| use frame_support::{ | ||
| pallet_prelude::*, | ||
| traits::{ | ||
| fungible::{Inspect, Mutate}, | ||
| tokens::Preservation, | ||
| Get, OnRuntimeUpgrade, | ||
| }, | ||
| PalletId, | ||
| }; | ||
| use sp_runtime::{traits::AccountIdConversion, Saturating}; | ||
| use sp_staking::EraIndex; | ||
|
|
||
| /// One-shot migration relocating already-funded era pots after the seed-derivation | ||
| /// change (#11930) so existing rewards stay claimable. For runtimes that activated | ||
| /// DAP before the slot-based rotation of era pot accounts landed. | ||
| /// | ||
| /// Migrates a single [`RewardKind`] per instance — list it twice in `Migrations` | ||
| /// if both kinds need migrating. | ||
| /// | ||
| /// Idempotent: skips eras whose old account has no balance. | ||
| /// | ||
| /// Generic params: | ||
| /// - `T`: pallet config. | ||
| /// - `S`: same `Get<PalletId>` used by [`crate::Seed`] to derive pot accounts. | ||
| /// - `K`: which [`RewardKind`] to migrate. | ||
| pub struct MigrateEraPotsToPool<T, S, K>(core::marker::PhantomData<(T, S, K)>); | ||
|
|
||
| impl<T: Config, S: Get<PalletId>, K: Get<RewardKind>> MigrateEraPotsToPool<T, S, K> { | ||
| /// Reproduces the historical seed derivation used before the slot-based | ||
| /// rotation, needed to locate pre-migration balances. | ||
| fn old_pot_account(era: EraIndex) -> T::AccountId { | ||
| S::get().into_sub_account_truncating(RewardPot::Era(era, K::get())) | ||
| } | ||
| } | ||
|
|
||
| impl<T: Config, S: Get<PalletId>, K: Get<RewardKind>> OnRuntimeUpgrade | ||
| for MigrateEraPotsToPool<T, S, K> | ||
| { | ||
| fn on_runtime_upgrade() -> Weight { | ||
| let mut weight = T::DbWeight::get().reads(2); | ||
|
|
||
| let Some(guard_era) = DisableMintingGuard::<T>::get() else { | ||
| log!(info, "EraPotsToPool: guard unset, nothing to migrate"); | ||
| return weight; | ||
| }; | ||
|
|
||
| let active_era_idx = crate::session_rotation::Rotator::<T>::active_era(); | ||
| debug_assert!( | ||
| active_era_idx >= guard_era, | ||
| "active_era should always be past DisableMintingGuard once set" | ||
| ); | ||
| if active_era_idx <= guard_era { | ||
| return weight; | ||
| } | ||
|
|
||
| // Anything older than `HistoryDepth` was already cleaned up via the | ||
| // normal payout flow. | ||
| let oldest = active_era_idx.saturating_sub(T::HistoryDepth::get()).max(guard_era); | ||
|
|
||
| let kind = K::get(); | ||
| let mut migrated = 0u32; | ||
| for era in oldest..active_era_idx { | ||
| let old = Self::old_pot_account(era); | ||
| weight.saturating_accrue(T::DbWeight::get().reads(1)); | ||
| if frame_system::Pallet::<T>::providers(&old) == 0 { | ||
| continue; | ||
| } | ||
|
|
||
| // `create` is idempotent: increments the provider on the new | ||
| // slot account only if not already provided. | ||
| let new = EraRewardManager::<T>::create(era, kind); | ||
| weight.saturating_accrue(T::DbWeight::get().reads_writes(1, 1)); | ||
|
|
||
| let balance = T::Currency::balance(&old); | ||
| weight.saturating_accrue(T::DbWeight::get().reads(1)); | ||
| if !balance.is_zero() { | ||
| if let Err(e) = T::Currency::transfer(&old, &new, balance, Preservation::Expendable) | ||
| { | ||
| log!( | ||
| error, | ||
| "EraPotsToPool: era {} kind {:?}: transfer failed: {:?}", | ||
| era, | ||
| kind, | ||
| e, | ||
| ); | ||
| // Keep providers on the old account; balance is still there | ||
| // and the account remains queryable for manual recovery. | ||
| continue; | ||
| } | ||
| weight.saturating_accrue(T::DbWeight::get().reads_writes(2, 2)); | ||
| } | ||
|
|
||
| // Try to release the old drained account so it can be reaped. | ||
| let _ = frame_system::Pallet::<T>::dec_providers(&old); | ||
| weight.saturating_accrue(T::DbWeight::get().writes(1)); | ||
| migrated.saturating_accrue(1); | ||
| } | ||
|
|
||
| log!( | ||
| info, | ||
| "EraPotsToPool: migrated {} eras of kind {:?} from guard {} to active {}", | ||
| migrated, | ||
| kind, | ||
| guard_era, | ||
| active_era_idx, | ||
| ); | ||
| weight | ||
| } | ||
|
|
||
| #[cfg(feature = "try-runtime")] | ||
| fn pre_upgrade() -> Result<alloc::vec::Vec<u8>, sp_runtime::TryRuntimeError> { | ||
| use crate::{BalanceOf, PotAccountProvider}; | ||
| use codec::Encode; | ||
| use sp_runtime::traits::Zero; | ||
|
|
||
| let kind = K::get(); | ||
| let mut total_old: BalanceOf<T> = Zero::zero(); | ||
| let mut total_new_pre: BalanceOf<T> = Zero::zero(); | ||
| for era in Self::migrated_eras() { | ||
| let old = Self::old_pot_account(era); | ||
| total_old.saturating_accrue(T::Currency::balance(&old)); | ||
| let new = T::RewardPots::pot_account(RewardPot::Era(era, kind)); | ||
| total_new_pre.saturating_accrue(T::Currency::balance(&new)); | ||
| } | ||
| Ok((total_old, total_new_pre).encode()) | ||
| } | ||
|
|
||
| #[cfg(feature = "try-runtime")] | ||
| fn post_upgrade(state: alloc::vec::Vec<u8>) -> Result<(), sp_runtime::TryRuntimeError> { | ||
| use crate::{BalanceOf, PotAccountProvider}; | ||
| use codec::Decode; | ||
| use sp_runtime::traits::Zero; | ||
|
|
||
| let (total_old, total_new_pre): (BalanceOf<T>, BalanceOf<T>) = | ||
| Decode::decode(&mut &state[..]).map_err(|_| "decode pre_upgrade state")?; | ||
|
|
||
| let kind = K::get(); | ||
| let mut remaining_old: BalanceOf<T> = Zero::zero(); | ||
| let mut total_new_post: BalanceOf<T> = Zero::zero(); | ||
| for era in Self::migrated_eras() { | ||
| let old = Self::old_pot_account(era); | ||
| remaining_old.saturating_accrue(T::Currency::balance(&old)); | ||
| let new = T::RewardPots::pot_account(RewardPot::Era(era, kind)); | ||
| total_new_post.saturating_accrue(T::Currency::balance(&new)); | ||
| } | ||
|
|
||
| frame_support::ensure!( | ||
| remaining_old.is_zero(), | ||
| "old pot accounts still hold balance after migration" | ||
| ); | ||
| // Funds must have landed in the new pots, accounting for whatever was | ||
| // already there pre-migration (if anything). | ||
| frame_support::ensure!( | ||
| total_new_post.saturating_sub(total_new_pre) == total_old, | ||
| "new pot balances did not increase by total_old after migration" | ||
| ); | ||
| Ok(()) | ||
| } | ||
| } | ||
|
|
||
| #[cfg(feature = "try-runtime")] | ||
| impl<T: Config, S: Get<PalletId>, K: Get<RewardKind>> MigrateEraPotsToPool<T, S, K> { | ||
| /// Returns the eras the migration touches. Only used for pre/post state checks. | ||
| fn migrated_eras() -> core::ops::Range<EraIndex> { | ||
| let active = crate::session_rotation::Rotator::<T>::active_era(); | ||
| match DisableMintingGuard::<T>::get() { | ||
| Some(guard) if active > guard => { | ||
| let oldest = active.saturating_sub(T::HistoryDepth::get()).max(guard); | ||
| oldest..active | ||
| }, | ||
| _ => 0..0, | ||
| } | ||
| } | ||
| } |
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.
Since
HistoryDepthis 84 for Westend / Kusama / Polkadot, isn't 200 too high? Why not 100 maybe?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 was looking to make it at least 2x to protect against history depth changing, which tbh is unlikely in itself. That said, I think 200 is a safe number. Much larger than history depth, and still gives the bounded protection.