From 960e3224fe8e93dbbb5b0d48a7c4086c7aa31322 Mon Sep 17 00:00:00 2001 From: Ankan Date: Thu, 29 May 2025 10:42:53 +0200 Subject: [PATCH 01/19] cap era duration, needs more tests. --- substrate/frame/staking-async/src/mock.rs | 1 + .../frame/staking-async/src/pallet/mod.rs | 28 +++++++++++++++++++ .../staking-async/src/session_rotation.rs | 25 ++++++++++++++++- 3 files changed, 53 insertions(+), 1 deletion(-) diff --git a/substrate/frame/staking-async/src/mock.rs b/substrate/frame/staking-async/src/mock.rs index a146e9f068344..581f4eb974633 100644 --- a/substrate/frame/staking-async/src/mock.rs +++ b/substrate/frame/staking-async/src/mock.rs @@ -405,6 +405,7 @@ impl crate::pallet::pallet::Config for Test { type EventListeners = EventListenerMock; type MaxInvulnerables = ConstU32<20>; type MaxDisabledValidators = ConstU32<100>; + type MaxEraDuration = (); type PlanningEraOffset = PlanningEraOffset; type Filter = MockedRestrictList; type RcClientInterface = session_mock::Session; diff --git a/substrate/frame/staking-async/src/pallet/mod.rs b/substrate/frame/staking-async/src/pallet/mod.rs index 4e803b07442f1..24c2d2ce1115f 100644 --- a/substrate/frame/staking-async/src/pallet/mod.rs +++ b/substrate/frame/staking-async/src/pallet/mod.rs @@ -312,6 +312,19 @@ pub mod pallet { #[pallet::constant] type MaxDisabledValidators: Get; + + /// Maximum allowed era duration in milliseconds. + /// + /// This provides a defensive upper bound to cap the effective era duration, + /// preventing excessively long eras from causing runaway inflation (e.g., due to bugs). + /// + /// If the actual era duration exceeds this value, it will be clamped to this maximum. + /// + /// Example: For an ideal era duration of 24 hours (86,400,000 ms), + /// this can be set to 604,800,000 ms (7 days). + #[pallet::constant] + type MaxEraDuration: Get; + /// Interface to talk to the RC-Client pallet, possibly sending election results to the /// relay chain. #[pallet::no_default] @@ -373,6 +386,7 @@ pub mod pallet { type MaxControllersInDeprecationBatch = ConstU32<100>; type MaxInvulnerables = ConstU32<20>; type MaxDisabledValidators = ConstU32<100>; + type MaxEraDuration = (); type EventListeners = (); type Filter = Nothing; type WeightInfo = (); @@ -1103,6 +1117,20 @@ pub mod pallet { active_era: EraIndex, planned_era: EraIndex, }, + /// Something occurred that should never happen under normal operation. + /// Logged as an event for fail-safe observability. + Unexpected(UnexpectedKind), + } + + /// Represents unexpected or invariant-breaking conditions encountered during execution. + /// + /// These variants are emitted as [`Event::Unexpected`] and indicate a defensive check has + /// failed. While these should never occur under normal operation, they are useful for + /// diagnosing issues in production or test environments. + #[derive(Clone, Encode, Decode, DecodeWithMemTracking, PartialEq, TypeInfo, RuntimeDebug)] + pub enum UnexpectedKind { + /// Emitted when calculated era duration exceeds the configured maximum. + EraDurationBoundExceeded, } #[pallet::error] diff --git a/substrate/frame/staking-async/src/session_rotation.rs b/substrate/frame/staking-async/src/session_rotation.rs index 288375ef19190..e14bcba476dbd 100644 --- a/substrate/frame/staking-async/src/session_rotation.rs +++ b/substrate/frame/staking-async/src/session_rotation.rs @@ -670,7 +670,30 @@ impl Rotator { fn end_era(ending_era: &ActiveEraInfo, new_era_start: u64) { let previous_era_start = ending_era.start.defensive_unwrap_or(new_era_start); - let era_duration = new_era_start.saturating_sub(previous_era_start); + let uncapped_era_duration = new_era_start.saturating_sub(previous_era_start); + + // maybe cap the era duration to the maximum allowed by the runtime. + let cap = T::MaxEraDuration::get(); + let era_duration = if cap == 0 { + // if the cap is zero (not set), we don't cap the era duration. + uncapped_era_duration + } else if uncapped_era_duration > cap { + Pallet::::deposit_event(Event::Unexpected(UnexpectedKind::EraDurationBoundExceeded)); + + // if the cap is set, and era duration exceeds the cap, we cap the era duration to the + // maximum allowed. + log!( + warn, + "capping era duration for era {:?} from {:?} to max allowed {:?}", + ending_era.index, + uncapped_era_duration, + cap + ); + cap + } else { + uncapped_era_duration + }; + Self::end_era_compute_payout(ending_era, era_duration); } From 5d3d02a1c852a66f798d9598ea071c25980f2064 Mon Sep 17 00:00:00 2001 From: Ankan Date: Thu, 29 May 2025 11:17:49 +0200 Subject: [PATCH 02/19] impl min bond --- .../frame/staking-async/src/pallet/impls.rs | 19 ++++++++++++++---- .../frame/staking-async/src/pallet/mod.rs | 20 ++++++++++--------- 2 files changed, 26 insertions(+), 13 deletions(-) diff --git a/substrate/frame/staking-async/src/pallet/impls.rs b/substrate/frame/staking-async/src/pallet/impls.rs index 83679103b5ad7..971ce8b132476 100644 --- a/substrate/frame/staking-async/src/pallet/impls.rs +++ b/substrate/frame/staking-async/src/pallet/impls.rs @@ -72,6 +72,18 @@ use sp_runtime::TryRuntimeError; const NPOS_MAX_ITERATIONS_COEFFICIENT: u32 = 2; impl Pallet { + /// Returns the minimum required bond for participation, considering validators, nominators, + /// and the chain’s existential deposit. + /// + /// This function computes the smallest allowed bond among `MinValidatorBond` and + /// `MinNominatorBond`, but ensures it is not below the existential deposit required to keep an + /// account alive. + pub(crate) fn min_bond() -> BalanceOf { + MinValidatorBond::::get() + .min(MinNominatorBond::::get()) + .max(asset::existential_deposit::()) + } + /// Fetches the ledger associated with a controller or stash account, if any. pub fn ledger(account: StakingAccount) -> Result, Error> { StakingLedger::::get(account) @@ -169,8 +181,8 @@ impl Pallet { ledger.total = ledger.total.checked_add(&extra).ok_or(ArithmeticError::Overflow)?; ledger.active = ledger.active.checked_add(&extra).ok_or(ArithmeticError::Overflow)?; - // last check: the new active amount of ledger must be more than ED. - ensure!(ledger.active >= asset::existential_deposit::(), Error::::InsufficientBond); + // last check: the new active amount of ledger must be more than min bond. + ensure!(ledger.active >= Self::min_bond(), Error::::InsufficientBond); // NOTE: ledger must be updated prior to calling `Self::weight_of`. ledger.update()?; @@ -193,9 +205,8 @@ impl Pallet { } let new_total = ledger.total; - let ed = asset::existential_deposit::(); let used_weight = - if ledger.unlocking.is_empty() && (ledger.active < ed || ledger.active.is_zero()) { + if ledger.unlocking.is_empty() && (ledger.active < Self::min_bond() || ledger.active.is_zero()) { // This account must have called `unbond()` with some value that caused the active // portion to fall below existential deposit + will have no more unlocking chunks // left. We can now safely remove all staking-related information. diff --git a/substrate/frame/staking-async/src/pallet/mod.rs b/substrate/frame/staking-async/src/pallet/mod.rs index 24c2d2ce1115f..c5095370a57ae 100644 --- a/substrate/frame/staking-async/src/pallet/mod.rs +++ b/substrate/frame/staking-async/src/pallet/mod.rs @@ -1307,8 +1307,8 @@ pub mod pallet { return Err(Error::::AlreadyPaired.into()); } - // Reject a bond which is considered to be _dust_. - if value < asset::existential_deposit::() { + // Reject a bond which is lower than the minimum bond. + if value < Self::min_bond() { return Err(Error::::InsufficientBond.into()); } @@ -1411,6 +1411,8 @@ pub mod pallet { } else if Validators::::contains_key(&stash) { MinValidatorBond::::get() } else { + // FAIL-CI + // TODO: port full unbond (unbond + chill) Zero::zero() }; @@ -1887,9 +1889,9 @@ pub mod pallet { let initial_unlocking = ledger.unlocking.len() as u32; let (ledger, rebonded_value) = ledger.rebond(value); - // Last check: the new active amount of ledger must be more than ED. + // Last check: the new active amount of ledger must be more than min bond. ensure!( - ledger.active >= asset::existential_deposit::(), + ledger.active >= Self::min_bond(), Error::::InsufficientBond ); @@ -1916,8 +1918,8 @@ pub mod pallet { /// Remove all data structures concerning a staker/stash once it is at a state where it can /// be considered `dust` in the staking system. The requirements are: /// - /// 1. the `total_balance` of the stash is below existential deposit. - /// 2. or, the `ledger.total` of the stash is below existential deposit. + /// 1. the `total_balance` of the stash is below minimum bond. + /// 2. or, the `ledger.total` of the stash is below minimum bond. /// 3. or, existential deposit is zero and either `total_balance` or `ledger.total` is zero. /// /// The former can happen in cases like a slash; the latter when a fully unbonded account @@ -1944,13 +1946,13 @@ pub mod pallet { // virtual stakers should not be allowed to be reaped. ensure!(!Self::is_virtual_staker(&stash), Error::::VirtualStakerNotAllowed); - let ed = asset::existential_deposit::(); + let min_bond = Self::min_bond(); let origin_balance = asset::total_balance::(&stash); let ledger_total = Self::ledger(Stash(stash.clone())).map(|l| l.total).unwrap_or_default(); - let reapable = origin_balance < ed || + let reapable = origin_balance < min_bond || origin_balance.is_zero() || - ledger_total < ed || + ledger_total < min_bond || ledger_total.is_zero(); ensure!(reapable, Error::::FundedTarget); From c47f127e3c730698cecae759f8cc072d8ab96257 Mon Sep 17 00:00:00 2001 From: Ankan Date: Thu, 29 May 2025 13:25:21 +0200 Subject: [PATCH 03/19] doc fix --- substrate/frame/staking-async/src/pallet/mod.rs | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/substrate/frame/staking-async/src/pallet/mod.rs b/substrate/frame/staking-async/src/pallet/mod.rs index c5095370a57ae..1cf078833d40a 100644 --- a/substrate/frame/staking-async/src/pallet/mod.rs +++ b/substrate/frame/staking-async/src/pallet/mod.rs @@ -315,10 +315,9 @@ pub mod pallet { /// Maximum allowed era duration in milliseconds. /// - /// This provides a defensive upper bound to cap the effective era duration, - /// preventing excessively long eras from causing runaway inflation (e.g., due to bugs). - /// - /// If the actual era duration exceeds this value, it will be clamped to this maximum. + /// This provides a defensive upper bound to cap the effective era duration, preventing + /// excessively long eras from causing runaway inflation (e.g., due to bugs). If the actual + /// era duration exceeds this value, it will be clamped to this maximum. /// /// Example: For an ideal era duration of 24 hours (86,400,000 ms), /// this can be set to 604,800,000 ms (7 days). From 873333a572896362a00282da61f698bd0d6ce97b Mon Sep 17 00:00:00 2001 From: Ankan Date: Thu, 29 May 2025 13:29:36 +0200 Subject: [PATCH 04/19] fix min bond chec --- substrate/frame/staking-async/src/tests/bonding.rs | 12 +++--------- 1 file changed, 3 insertions(+), 9 deletions(-) diff --git a/substrate/frame/staking-async/src/tests/bonding.rs b/substrate/frame/staking-async/src/tests/bonding.rs index b67b2a09e6e24..51f812917b0e7 100644 --- a/substrate/frame/staking-async/src/tests/bonding.rs +++ b/substrate/frame/staking-async/src/tests/bonding.rs @@ -1596,23 +1596,17 @@ mod staking_bounds_chill_other { .min_validator_bond(1_500) .build_and_execute(|| { // 500 is not enough for any role - assert_ok!(Staking::bond(RuntimeOrigin::signed(3), 500, RewardDestination::Stash)); assert_noop!( - Staking::nominate(RuntimeOrigin::signed(3), vec![1]), + Staking::bond(RuntimeOrigin::signed(3), 500, RewardDestination::Stash), Error::::InsufficientBond ); + // 1000 is enough for nominator but not for validator. + assert_ok!(Staking::bond(RuntimeOrigin::signed(3), 1000, RewardDestination::Stash)); assert_noop!( Staking::validate(RuntimeOrigin::signed(3), ValidatorPrefs::default()), Error::::InsufficientBond, ); - - // 1000 is enough for nominator - assert_ok!(Staking::bond_extra(RuntimeOrigin::signed(3), 500)); assert_ok!(Staking::nominate(RuntimeOrigin::signed(3), vec![1])); - assert_noop!( - Staking::validate(RuntimeOrigin::signed(3), ValidatorPrefs::default()), - Error::::InsufficientBond, - ); // 1500 is enough for validator assert_ok!(Staking::bond_extra(RuntimeOrigin::signed(3), 500)); From 1381a7855a06dc2ea0c522927f5073ce2e846582 Mon Sep 17 00:00:00 2001 From: Ankan Date: Thu, 29 May 2025 13:30:58 +0200 Subject: [PATCH 05/19] fmt --- .../frame/staking-async/src/pallet/impls.rs | 29 ++++++++++--------- .../frame/staking-async/src/pallet/mod.rs | 6 +--- 2 files changed, 16 insertions(+), 19 deletions(-) diff --git a/substrate/frame/staking-async/src/pallet/impls.rs b/substrate/frame/staking-async/src/pallet/impls.rs index 971ce8b132476..45951e26ed193 100644 --- a/substrate/frame/staking-async/src/pallet/impls.rs +++ b/substrate/frame/staking-async/src/pallet/impls.rs @@ -205,21 +205,22 @@ impl Pallet { } let new_total = ledger.total; - let used_weight = - if ledger.unlocking.is_empty() && (ledger.active < Self::min_bond() || ledger.active.is_zero()) { - // This account must have called `unbond()` with some value that caused the active - // portion to fall below existential deposit + will have no more unlocking chunks - // left. We can now safely remove all staking-related information. - Self::kill_stash(&ledger.stash)?; - - T::WeightInfo::withdraw_unbonded_kill() - } else { - // This was the consequence of a partial unbond. just update the ledger and move on. - ledger.update()?; + let used_weight = if ledger.unlocking.is_empty() && + (ledger.active < Self::min_bond() || ledger.active.is_zero()) + { + // This account must have called `unbond()` with some value that caused the active + // portion to fall below existential deposit + will have no more unlocking chunks + // left. We can now safely remove all staking-related information. + Self::kill_stash(&ledger.stash)?; - // This is only an update, so we use less overall weight. - T::WeightInfo::withdraw_unbonded_update() - }; + T::WeightInfo::withdraw_unbonded_kill() + } else { + // This was the consequence of a partial unbond. just update the ledger and move on. + ledger.update()?; + + // This is only an update, so we use less overall weight. + T::WeightInfo::withdraw_unbonded_update() + }; // `old_total` should never be less than the new total because // `consolidate_unlocked` strictly subtracts balance. diff --git a/substrate/frame/staking-async/src/pallet/mod.rs b/substrate/frame/staking-async/src/pallet/mod.rs index 1cf078833d40a..8d6d5d06883b9 100644 --- a/substrate/frame/staking-async/src/pallet/mod.rs +++ b/substrate/frame/staking-async/src/pallet/mod.rs @@ -312,7 +312,6 @@ pub mod pallet { #[pallet::constant] type MaxDisabledValidators: Get; - /// Maximum allowed era duration in milliseconds. /// /// This provides a defensive upper bound to cap the effective era duration, preventing @@ -1889,10 +1888,7 @@ pub mod pallet { let initial_unlocking = ledger.unlocking.len() as u32; let (ledger, rebonded_value) = ledger.rebond(value); // Last check: the new active amount of ledger must be more than min bond. - ensure!( - ledger.active >= Self::min_bond(), - Error::::InsufficientBond - ); + ensure!(ledger.active >= Self::min_bond(), Error::::InsufficientBond); Self::deposit_event(Event::::Bonded { stash: ledger.stash.clone(), From 345501697f3583b836a59ff9a0de749f7e14d7de Mon Sep 17 00:00:00 2001 From: Ankan Date: Tue, 3 Jun 2025 13:27:46 +0200 Subject: [PATCH 06/19] use AHStakingInterface to rotate session --- substrate/frame/staking-async/src/mock.rs | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-) diff --git a/substrate/frame/staking-async/src/mock.rs b/substrate/frame/staking-async/src/mock.rs index 6e634dc171f17..5b2809921c7ab 100644 --- a/substrate/frame/staking-async/src/mock.rs +++ b/substrate/frame/staking-async/src/mock.rs @@ -312,16 +312,22 @@ pub mod session_mock { if QueuedBufferSessions::get() == 0 { // buffer time has passed Active::set(q); - Rotator::::end_session(ending, Some((Timestamp::get(), id))); + ::on_relay_session_report( + rc_client::SessionReport::new_terminal(ending, vec![], Some((Timestamp::get(), id))) + ); Queued::reset(); QueuedId::reset(); } else { QueuedBufferSessions::mutate(|s| *s -= 1); - Rotator::::end_session(ending, None); + ::on_relay_session_report( + rc_client::SessionReport::new_terminal(ending, vec![], None) + ); } } else { // just end the session. - Rotator::::end_session(ending, None); + ::on_relay_session_report( + rc_client::SessionReport::new_terminal(ending, vec![], None) + ); } CurrentIndex::set(ending + 1); } From 35bd42345ef137f09d915b8d8840e4737af0830b Mon Sep 17 00:00:00 2001 From: Ankan Date: Tue, 3 Jun 2025 13:28:04 +0200 Subject: [PATCH 07/19] fmt --- substrate/frame/staking-async/src/mock.rs | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/substrate/frame/staking-async/src/mock.rs b/substrate/frame/staking-async/src/mock.rs index 5b2809921c7ab..5c46ae79bc47c 100644 --- a/substrate/frame/staking-async/src/mock.rs +++ b/substrate/frame/staking-async/src/mock.rs @@ -313,20 +313,24 @@ pub mod session_mock { // buffer time has passed Active::set(q); ::on_relay_session_report( - rc_client::SessionReport::new_terminal(ending, vec![], Some((Timestamp::get(), id))) + rc_client::SessionReport::new_terminal( + ending, + vec![], + Some((Timestamp::get(), id)), + ), ); Queued::reset(); QueuedId::reset(); } else { QueuedBufferSessions::mutate(|s| *s -= 1); ::on_relay_session_report( - rc_client::SessionReport::new_terminal(ending, vec![], None) + rc_client::SessionReport::new_terminal(ending, vec![], None), ); } } else { // just end the session. ::on_relay_session_report( - rc_client::SessionReport::new_terminal(ending, vec![], None) + rc_client::SessionReport::new_terminal(ending, vec![], None), ); } CurrentIndex::set(ending + 1); From b1e5247764daedf760a6e6b42f0403cb39dffbbb Mon Sep 17 00:00:00 2001 From: Ankan Date: Tue, 3 Jun 2025 14:01:05 +0200 Subject: [PATCH 08/19] wip: failing test missing events --- substrate/frame/staking-async/src/mock.rs | 3 ++- .../staking-async/src/tests/era_rotation.rs | 25 +++++++++++++++++-- 2 files changed, 25 insertions(+), 3 deletions(-) diff --git a/substrate/frame/staking-async/src/mock.rs b/substrate/frame/staking-async/src/mock.rs index 5c46ae79bc47c..1826bffe4e010 100644 --- a/substrate/frame/staking-async/src/mock.rs +++ b/substrate/frame/staking-async/src/mock.rs @@ -395,6 +395,7 @@ ord_parameter_types! { parameter_types! { pub static RemainderRatio: Perbill = Perbill::from_percent(50); + pub static MaxEraDuration: u64 = time_per_era() * 7; } pub struct OneTokenPerMillisecond; impl EraPayout for OneTokenPerMillisecond { @@ -433,7 +434,7 @@ impl crate::pallet::pallet::Config for Test { type EventListeners = EventListenerMock; type MaxInvulnerables = ConstU32<20>; type MaxDisabledValidators = ConstU32<100>; - type MaxEraDuration = (); + type MaxEraDuration = MaxEraDuration; type PlanningEraOffset = PlanningEraOffset; type Filter = MockedRestrictList; type RcClientInterface = session_mock::Session; diff --git a/substrate/frame/staking-async/src/tests/era_rotation.rs b/substrate/frame/staking-async/src/tests/era_rotation.rs index ac06eba76bbac..9b7595041b36c 100644 --- a/substrate/frame/staking-async/src/tests/era_rotation.rs +++ b/substrate/frame/staking-async/src/tests/era_rotation.rs @@ -16,6 +16,7 @@ // limitations under the License. use crate::session_rotation::Eras; +use crate::tests::session_mock::Timestamp; use super::*; @@ -192,9 +193,29 @@ fn activation_timestamp_when_era_planning_not_complete() { } #[test] -#[should_panic] fn max_era_duration_safety_guard() { - todo!("a safety guard that ensures that there is an upper bound on how long an era duration can be. Should prevent us from parabolic inflation in case of some crazy bug."); + ExtBuilder::default().build_and_execute(|| { + + // GIVEN we are at end of an era (2). + Session::roll_until_active_era(2); + System::reset_events(); + + // WHEN subsequent era takes more than MaxEraDuration. + Timestamp::set(Timestamp::get() + MaxEraDuration::get()); + + Session::roll_until_active_era(3); + + assert_eq!( + staking_events_since_last_call(), + vec![ + Event::SessionRotated { starting_session: 4, active_era: 1, planned_era: 2 }, + Event::PagedElectionProceeded { page: 0, result: Ok(2) }, + Event::SessionRotated { starting_session: 5, active_era: 1, planned_era: 2 }, + Event::EraPaid { era_index: 1, validator_payout: 7500, remainder: 7500 }, + Event::SessionRotated { starting_session: 6, active_era: 2, planned_era: 2 } + ] + ); + }); } #[test] From 971de1376532fdf759045f86b96aca5c27d6db1d Mon Sep 17 00:00:00 2001 From: Ankan Date: Tue, 3 Jun 2025 15:01:49 +0200 Subject: [PATCH 09/19] cap era duration test --- .../staking-async/src/tests/era_rotation.rs | 43 ++++++++++++++++--- 1 file changed, 36 insertions(+), 7 deletions(-) diff --git a/substrate/frame/staking-async/src/tests/era_rotation.rs b/substrate/frame/staking-async/src/tests/era_rotation.rs index 9b7595041b36c..ed7a606bf2e71 100644 --- a/substrate/frame/staking-async/src/tests/era_rotation.rs +++ b/substrate/frame/staking-async/src/tests/era_rotation.rs @@ -195,24 +195,53 @@ fn activation_timestamp_when_era_planning_not_complete() { #[test] fn max_era_duration_safety_guard() { ExtBuilder::default().build_and_execute(|| { + // let's deduce some magic numbers for the test. + let ideal_era_payout = total_payout_for(time_per_era()); + let ideal_treasury_payout = RemainderRatio::get() * ideal_era_payout; + let ideal_validator_payout = ideal_era_payout - ideal_treasury_payout; + // max era duration is capped to 7 times the ideal era duration. + let max_validator_payout = 7 * ideal_validator_payout; + let max_treasury_payout = 7 * ideal_treasury_payout; + + // these are the values we expect to see in the events. + assert_eq!(ideal_treasury_payout, 7500); + assert_eq!(ideal_validator_payout, 7500); + // when the era duration exceeds `MaxEraDuration`, the payouts should be capped to the + // following values. + assert_eq!(max_treasury_payout, 52500); + assert_eq!(max_validator_payout, 52500); // GIVEN we are at end of an era (2). Session::roll_until_active_era(2); - System::reset_events(); + assert_eq!( + staking_events_since_last_call(), + vec![ + Event::SessionRotated { starting_session: 4, active_era: 1, planned_era: 2 }, + Event::PagedElectionProceeded { page: 0, result: Ok(2) }, + Event::SessionRotated { starting_session: 5, active_era: 1, planned_era: 2 }, + Event::EraPaid { era_index: 1, validator_payout: ideal_validator_payout, remainder: ideal_treasury_payout }, + Event::SessionRotated { starting_session: 6, active_era: 2, planned_era: 2 } + ] + ); - // WHEN subsequent era takes more than MaxEraDuration. - Timestamp::set(Timestamp::get() + MaxEraDuration::get()); + // WHEN subsequent era takes longer than MaxEraDuration. + // (this can happen either because of a bug or because a long stall in the chain). + Timestamp::set(Timestamp::get() + 2 * MaxEraDuration::get()); Session::roll_until_active_era(3); assert_eq!( staking_events_since_last_call(), vec![ - Event::SessionRotated { starting_session: 4, active_era: 1, planned_era: 2 }, + Event::SessionRotated { starting_session: 7, active_era: 2, planned_era: 3 }, Event::PagedElectionProceeded { page: 0, result: Ok(2) }, - Event::SessionRotated { starting_session: 5, active_era: 1, planned_era: 2 }, - Event::EraPaid { era_index: 1, validator_payout: 7500, remainder: 7500 }, - Event::SessionRotated { starting_session: 6, active_era: 2, planned_era: 2 } + Event::SessionRotated { starting_session: 8, active_era: 2, planned_era: 3 }, + // an event is emitted to indicate something unexpected happened, i.e. the era + // duration exceeded the `MaxEraDuration` limit. + Event::Unexpected(UnexpectedKind::EraDurationBoundExceeded), + // the payouts are capped to the max values. + Event::EraPaid { era_index: 2, validator_payout: max_validator_payout, remainder: max_treasury_payout }, + Event::SessionRotated { starting_session: 9, active_era: 3, planned_era: 3 } ] ); }); From 5f0aed80caea18138472a94728cbccd7360cf6eb Mon Sep 17 00:00:00 2001 From: Ankan Date: Tue, 3 Jun 2025 16:00:53 +0200 Subject: [PATCH 10/19] refactor to handle incorrect activation stamp --- .../staking-async/ahm-test/src/ah/mock.rs | 4 +- .../frame/staking-async/src/benchmarking.rs | 10 ++--- substrate/frame/staking-async/src/mock.rs | 3 ++ .../frame/staking-async/src/pallet/mod.rs | 2 + .../staking-async/src/session_rotation.rs | 39 ++++++++++++----- .../staking-async/src/tests/era_rotation.rs | 43 ++++++++++++++++--- 6 files changed, 79 insertions(+), 22 deletions(-) diff --git a/substrate/frame/staking-async/ahm-test/src/ah/mock.rs b/substrate/frame/staking-async/ahm-test/src/ah/mock.rs index f40368eac8e93..dd548c74d4013 100644 --- a/substrate/frame/staking-async/ahm-test/src/ah/mock.rs +++ b/substrate/frame/staking-async/ahm-test/src/ah/mock.rs @@ -83,10 +83,10 @@ pub fn roll_until_matches(criteria: impl Fn() -> bool, with_rc: bool) { /// Use the given `end_index` as the first session report, and increment as per needed. pub(crate) fn roll_until_next_active(mut end_index: SessionIndex) -> Vec { // receive enough session reports, such that we plan a new era - let planned_era = pallet_staking_async::session_rotation::Rotator::::planning_era(); + let planned_era = pallet_staking_async::session_rotation::Rotator::::planned_era(); let active_era = pallet_staking_async::session_rotation::Rotator::::active_era(); - while pallet_staking_async::session_rotation::Rotator::::planning_era() == planned_era + while pallet_staking_async::session_rotation::Rotator::::planned_era() == planned_era { let report = SessionReport { end_index, diff --git a/substrate/frame/staking-async/src/benchmarking.rs b/substrate/frame/staking-async/src/benchmarking.rs index 9960fb960782f..05226d33d963b 100644 --- a/substrate/frame/staking-async/src/benchmarking.rs +++ b/substrate/frame/staking-async/src/benchmarking.rs @@ -1024,14 +1024,14 @@ mod benchmarks { let _new_validators = Rotator::::legacy_insta_plan_era(); // activate the previous one Rotator::::start_era( - crate::ActiveEraInfo { index: Rotator::::planning_era() - 1, start: Some(1) }, + crate::ActiveEraInfo { index: Rotator::::planned_era() - 1, start: Some(1) }, 42, // start session index doesn't really matter, 2, // timestamp doesn't really matter ); // ensure our offender has at least a full exposure page let offender_exposure = - Eras::::get_full_exposure(Rotator::::planning_era(), &offender); + Eras::::get_full_exposure(Rotator::::planned_era(), &offender); ensure!( offender_exposure.others.len() as u32 == 2 * T::MaxExposurePageSize::get(), "exposure not created" @@ -1073,7 +1073,7 @@ mod benchmarks { fn rc_on_offence( v: Linear<2, { T::MaxValidatorSet::get() / 2 }>, ) -> Result<(), BenchmarkError> { - let initial_era = Rotator::::planning_era(); + let initial_era = Rotator::::planned_era(); let _ = crate::testing_utils::create_validators_with_nominators_for_era::( 2 * v, // number of nominators is irrelevant here, so we hardcode these @@ -1085,7 +1085,7 @@ mod benchmarks { // plan new era let new_validators = Rotator::::legacy_insta_plan_era(); - ensure!(Rotator::::planning_era() == initial_era + 1, "era should be incremented"); + ensure!(Rotator::::planned_era() == initial_era + 1, "era should be incremented"); // activate the previous one Rotator::::start_era( crate::ActiveEraInfo { index: initial_era, start: Some(1) }, @@ -1135,7 +1135,7 @@ mod benchmarks { #[benchmark] fn rc_on_session_report() -> Result<(), BenchmarkError> { - let initial_planned_era = Rotator::::planning_era(); + let initial_planned_era = Rotator::::planned_era(); let initial_active_era = Rotator::::active_era(); // create a small, arbitrary number of stakers. This is just for sanity of the era planning, diff --git a/substrate/frame/staking-async/src/mock.rs b/substrate/frame/staking-async/src/mock.rs index 1826bffe4e010..674212649206e 100644 --- a/substrate/frame/staking-async/src/mock.rs +++ b/substrate/frame/staking-async/src/mock.rs @@ -315,6 +315,9 @@ pub mod session_mock { ::on_relay_session_report( rc_client::SessionReport::new_terminal( ending, + // TODO: currently we use `Eras::reward_active_era()` to set validator + // points in our tests. We should improve this and find a good way to + // set this value instead. vec![], Some((Timestamp::get(), id)), ), diff --git a/substrate/frame/staking-async/src/pallet/mod.rs b/substrate/frame/staking-async/src/pallet/mod.rs index 40aa59ee018b5..8a81881a455c9 100644 --- a/substrate/frame/staking-async/src/pallet/mod.rs +++ b/substrate/frame/staking-async/src/pallet/mod.rs @@ -1129,6 +1129,8 @@ pub mod pallet { pub enum UnexpectedKind { /// Emitted when calculated era duration exceeds the configured maximum. EraDurationBoundExceeded, + /// Received a validator activation event that is not recognized. + UnknownValidatorActivation, } #[pallet::error] diff --git a/substrate/frame/staking-async/src/session_rotation.rs b/substrate/frame/staking-async/src/session_rotation.rs index f0d7afd1a9142..3c014576e7608 100644 --- a/substrate/frame/staking-async/src/session_rotation.rs +++ b/substrate/frame/staking-async/src/session_rotation.rs @@ -123,7 +123,7 @@ impl Eras { } pub(crate) fn set_validator_prefs(era: EraIndex, stash: &T::AccountId, prefs: ValidatorPrefs) { - debug_assert_eq!(era, Rotator::::planning_era(), "we only set prefs for planning era"); + debug_assert_eq!(era, Rotator::::planned_era(), "we only set prefs for planning era"); >::insert(era, stash, prefs); } @@ -493,7 +493,7 @@ impl Rotator { #[cfg(any(feature = "try-runtime", test))] pub(crate) fn do_try_state() -> Result<(), sp_runtime::TryRuntimeError> { // planned era can always be at most one more than active era - let planned = Self::planning_era(); + let planned = Self::planned_era(); let active = Self::active_era(); ensure!( planned == active || planned == active + 1, @@ -511,7 +511,14 @@ impl Rotator { Ok(()) } - pub fn planning_era() -> EraIndex { + /// Latest era that was planned. + /// + /// The returned value does not necessarily indicate that planning for the era with this index + /// is underway, but rather the last era that was planned. If `Self::active_era()` is equal to + /// this value, it means that the era is currently active and no new era is planned. + /// + /// See [`Self::planning_era()`] to only get the next index that is planned. + pub fn planned_era() -> EraIndex { CurrentEra::::get().unwrap_or(0) } @@ -519,6 +526,18 @@ impl Rotator { ActiveEra::::get().map(|a| a.index).defensive_unwrap_or(0) } + /// Next era that is planned to be started. + /// + /// Returns None if no era is planned. + pub fn planning_era() -> Option { + let (active, planned) = (Self::active_era(), Self::planned_era()); + if planned.defensive_saturating_sub(active) > 1 { + defensive!("planned era must always be equal or one more than active"); + } + + (planned > active).then_some(planned) + } + /// End the session and start the next one. pub(crate) fn end_session(end_index: SessionIndex, activation_timestamp: Option<(u64, u32)>) { let Some(active_era) = ActiveEra::::get() else { @@ -541,7 +560,7 @@ impl Rotator { log!(info, "Era: active {:?}, planned {:?}", active_era.index, current_planned_era); match activation_timestamp { - Some((time, id)) if id == current_planned_era => { + Some((time, id)) if Some(id) == current_planned_era => { // We rotate the era if we have the activation timestamp. Self::start_era(active_era, starting, time); }, @@ -549,15 +568,15 @@ impl Rotator { // RC has done something wrong -- we received the wrong ID. Don't start a new era. crate::log!( warn, - "received wrong ID with activation timestamp. Got {}, expected {}", + "received wrong ID with activation timestamp. Got {}, expected {:?}", id, current_planned_era ); + Pallet::::deposit_event(Event::Unexpected(UnexpectedKind::UnknownValidatorActivation)); }, None => (), } - let active_era = Self::active_era(); // check if we should plan new era. let should_plan_era = match ForceEra::::get() { // see if it's good time to plan a new era. @@ -573,7 +592,7 @@ impl Rotator { Forcing::ForceNone => false, }; - let has_pending_era = active_era < current_planned_era; + let has_pending_era = current_planned_era.is_some(); match (should_plan_era, has_pending_era) { (false, _) => { // nothing to consider @@ -587,7 +606,7 @@ impl Rotator { // now. crate::log!( debug, - "time to plan a new era {}, but waiting for the activation of the previous.", + "time to plan a new era {:?}, but waiting for the activation of the previous.", current_planned_era ); }, @@ -596,7 +615,7 @@ impl Rotator { Pallet::::deposit_event(Event::SessionRotated { starting_session: starting, active_era: Self::active_era(), - planned_era: Self::planning_era(), + planned_era: Self::planned_era(), }); } @@ -919,7 +938,7 @@ impl EraElectionPlanner { pub(crate) fn do_elect_paged_inner( mut supports: BoundedSupportsOf, ) -> Result { - let planning_era = Rotator::::planning_era(); + let planning_era = Rotator::::planned_era(); match Self::add_electables(supports.iter().map(|(s, _)| s.clone())) { Ok(added) => { diff --git a/substrate/frame/staking-async/src/tests/era_rotation.rs b/substrate/frame/staking-async/src/tests/era_rotation.rs index ed7a606bf2e71..09921bbeaffea 100644 --- a/substrate/frame/staking-async/src/tests/era_rotation.rs +++ b/substrate/frame/staking-async/src/tests/era_rotation.rs @@ -17,6 +17,8 @@ use crate::session_rotation::Eras; use crate::tests::session_mock::Timestamp; +use crate::tests::session_mock::CurrentIndex; +use crate::session_rotation::Rotator; use super::*; @@ -179,10 +181,41 @@ fn forcing_force_new() { } #[test] -#[should_panic] fn activation_timestamp_when_no_planned_era() { // maybe not needed, as we have the id check - todo!("what if we receive an activation timestamp when there is no planned era?"); + ExtBuilder::default() + .session_per_era(6) + .build_and_execute(|| { + Session::roll_until_active_era(2); + let current_index = CurrentIndex::get(); + + // reset events until now. + let _ = staking_events_since_last_call(); + + // GIVEN: no new planned era + assert_eq!(Rotator::::active_era(), 2); + assert_eq!(Rotator::::planned_era(), 2); + + // WHEN: send a new activation timestamp (manually). + ::on_relay_session_report( + pallet_staking_async_rc_client::SessionReport::new_terminal( + current_index, + vec![], + // sending a timestamp that is in the future with identifier of the next era that + // is not planned. + Some((Timestamp::get() + time_per_session(), 3)), + ), + ); + + // THEN: No era rotation should happen, but an error event should be emitted. + assert_eq!( + staking_events_since_last_call(), + vec![ + Event::Unexpected(UnexpectedKind::UnknownValidatorActivation), + Event::SessionRotated { starting_session: current_index + 1, active_era: 2, planned_era: 2 } + ] + ); + }); } #[test] @@ -211,7 +244,7 @@ fn max_era_duration_safety_guard() { assert_eq!(max_treasury_payout, 52500); assert_eq!(max_validator_payout, 52500); - // GIVEN we are at end of an era (2). + // GIVEN: we are at end of an era (2). Session::roll_until_active_era(2); assert_eq!( staking_events_since_last_call(), @@ -224,12 +257,12 @@ fn max_era_duration_safety_guard() { ] ); - // WHEN subsequent era takes longer than MaxEraDuration. + // WHEN: subsequent era takes longer than MaxEraDuration. // (this can happen either because of a bug or because a long stall in the chain). Timestamp::set(Timestamp::get() + 2 * MaxEraDuration::get()); - Session::roll_until_active_era(3); + // THEN: we should see the payouts capped to the max values. assert_eq!( staking_events_since_last_call(), vec![ From 36448f9ae705517885cac92dfea4d0809fa97bcc Mon Sep 17 00:00:00 2001 From: Ankan Date: Tue, 3 Jun 2025 16:08:55 +0200 Subject: [PATCH 11/19] fix force always era rotation issue --- substrate/frame/staking-async/src/session_rotation.rs | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/substrate/frame/staking-async/src/session_rotation.rs b/substrate/frame/staking-async/src/session_rotation.rs index 3c014576e7608..aa07673278800 100644 --- a/substrate/frame/staking-async/src/session_rotation.rs +++ b/substrate/frame/staking-async/src/session_rotation.rs @@ -592,7 +592,9 @@ impl Rotator { Forcing::ForceNone => false, }; - let has_pending_era = current_planned_era.is_some(); + // Note: we call `planning_era` again, as a new era might have started since we checked + // it last. + let has_pending_era = Self::planning_era().is_some(); match (should_plan_era, has_pending_era) { (false, _) => { // nothing to consider From 2d7e2a2d673e0e25ff7282993a17ccedff353fac Mon Sep 17 00:00:00 2001 From: Ankan Date: Tue, 3 Jun 2025 16:10:10 +0200 Subject: [PATCH 12/19] fmt --- .../staking-async/ahm-test/src/ah/mock.rs | 3 +- .../frame/staking-async/src/pallet/mod.rs | 4 +-- .../staking-async/src/session_rotation.rs | 4 ++- .../staking-async/src/tests/era_rotation.rs | 30 ++++++++++++------- 4 files changed, 26 insertions(+), 15 deletions(-) diff --git a/substrate/frame/staking-async/ahm-test/src/ah/mock.rs b/substrate/frame/staking-async/ahm-test/src/ah/mock.rs index dd548c74d4013..7b6feae93881a 100644 --- a/substrate/frame/staking-async/ahm-test/src/ah/mock.rs +++ b/substrate/frame/staking-async/ahm-test/src/ah/mock.rs @@ -86,8 +86,7 @@ pub(crate) fn roll_until_next_active(mut end_index: SessionIndex) -> Vec::planned_era(); let active_era = pallet_staking_async::session_rotation::Rotator::::active_era(); - while pallet_staking_async::session_rotation::Rotator::::planned_era() == planned_era - { + while pallet_staking_async::session_rotation::Rotator::::planned_era() == planned_era { let report = SessionReport { end_index, activation_timestamp: None, diff --git a/substrate/frame/staking-async/src/pallet/mod.rs b/substrate/frame/staking-async/src/pallet/mod.rs index 8a81881a455c9..6a0af13e73bff 100644 --- a/substrate/frame/staking-async/src/pallet/mod.rs +++ b/substrate/frame/staking-async/src/pallet/mod.rs @@ -1129,8 +1129,8 @@ pub mod pallet { pub enum UnexpectedKind { /// Emitted when calculated era duration exceeds the configured maximum. EraDurationBoundExceeded, - /// Received a validator activation event that is not recognized. - UnknownValidatorActivation, + /// Received a validator activation event that is not recognized. + UnknownValidatorActivation, } #[pallet::error] diff --git a/substrate/frame/staking-async/src/session_rotation.rs b/substrate/frame/staking-async/src/session_rotation.rs index aa07673278800..a7a9a2eed3985 100644 --- a/substrate/frame/staking-async/src/session_rotation.rs +++ b/substrate/frame/staking-async/src/session_rotation.rs @@ -572,7 +572,9 @@ impl Rotator { id, current_planned_era ); - Pallet::::deposit_event(Event::Unexpected(UnexpectedKind::UnknownValidatorActivation)); + Pallet::::deposit_event(Event::Unexpected( + UnexpectedKind::UnknownValidatorActivation, + )); }, None => (), } diff --git a/substrate/frame/staking-async/src/tests/era_rotation.rs b/substrate/frame/staking-async/src/tests/era_rotation.rs index 09921bbeaffea..5c1406ff8ec21 100644 --- a/substrate/frame/staking-async/src/tests/era_rotation.rs +++ b/substrate/frame/staking-async/src/tests/era_rotation.rs @@ -15,10 +15,10 @@ // See the License for the specific language governing permissions and // limitations under the License. -use crate::session_rotation::Eras; -use crate::tests::session_mock::Timestamp; -use crate::tests::session_mock::CurrentIndex; -use crate::session_rotation::Rotator; +use crate::{ + session_rotation::{Eras, Rotator}, + tests::session_mock::{CurrentIndex, Timestamp}, +}; use super::*; @@ -183,9 +183,7 @@ fn forcing_force_new() { #[test] fn activation_timestamp_when_no_planned_era() { // maybe not needed, as we have the id check - ExtBuilder::default() - .session_per_era(6) - .build_and_execute(|| { + ExtBuilder::default().session_per_era(6).build_and_execute(|| { Session::roll_until_active_era(2); let current_index = CurrentIndex::get(); @@ -212,7 +210,11 @@ fn activation_timestamp_when_no_planned_era() { staking_events_since_last_call(), vec![ Event::Unexpected(UnexpectedKind::UnknownValidatorActivation), - Event::SessionRotated { starting_session: current_index + 1, active_era: 2, planned_era: 2 } + Event::SessionRotated { + starting_session: current_index + 1, + active_era: 2, + planned_era: 2 + } ] ); }); @@ -252,7 +254,11 @@ fn max_era_duration_safety_guard() { Event::SessionRotated { starting_session: 4, active_era: 1, planned_era: 2 }, Event::PagedElectionProceeded { page: 0, result: Ok(2) }, Event::SessionRotated { starting_session: 5, active_era: 1, planned_era: 2 }, - Event::EraPaid { era_index: 1, validator_payout: ideal_validator_payout, remainder: ideal_treasury_payout }, + Event::EraPaid { + era_index: 1, + validator_payout: ideal_validator_payout, + remainder: ideal_treasury_payout + }, Event::SessionRotated { starting_session: 6, active_era: 2, planned_era: 2 } ] ); @@ -273,7 +279,11 @@ fn max_era_duration_safety_guard() { // duration exceeded the `MaxEraDuration` limit. Event::Unexpected(UnexpectedKind::EraDurationBoundExceeded), // the payouts are capped to the max values. - Event::EraPaid { era_index: 2, validator_payout: max_validator_payout, remainder: max_treasury_payout }, + Event::EraPaid { + era_index: 2, + validator_payout: max_validator_payout, + remainder: max_treasury_payout + }, Event::SessionRotated { starting_session: 9, active_era: 3, planned_era: 3 } ] ); From 44d616411a4b44ec83bcd4deb0c6b37ee19d6bc6 Mon Sep 17 00:00:00 2001 From: Ankan Date: Tue, 3 Jun 2025 16:20:07 +0200 Subject: [PATCH 13/19] minor --- .../frame/staking-async/src/tests/bonding.rs | 23 +++++++++++++++---- 1 file changed, 19 insertions(+), 4 deletions(-) diff --git a/substrate/frame/staking-async/src/tests/bonding.rs b/substrate/frame/staking-async/src/tests/bonding.rs index 51f812917b0e7..0de8a3396035f 100644 --- a/substrate/frame/staking-async/src/tests/bonding.rs +++ b/substrate/frame/staking-async/src/tests/bonding.rs @@ -1426,10 +1426,12 @@ mod reap { #[test] fn reap_stash_works() { ExtBuilder::default() + .min_nominator_bond(1_000) + .min_validator_bond(1_500) .existential_deposit(10) .balance_factor(10) .build_and_execute(|| { - // given + // GIVEN: 11 is a bonded validator. assert_eq!(asset::staked::(&11), 10 * 1000); assert_eq!(Staking::bonded(&11), Some(11)); @@ -1444,14 +1446,27 @@ mod reap { Error::::FundedTarget ); + // Note: Even though the stash is a validator, the threshold to reap is min of nominator + // and validator bond // no easy way to cause an account to go below ED, we tweak their staking ledger // instead. - Ledger::::insert(11, StakingLedger::::new(11, 5)); - // reap-able + // WHEN: we set the ledger to below min validator bond but above min nominator bond. + Ledger::::insert(11, StakingLedger::::new(11, 1499)); + + // THEN: still can't reap as the balance is above min nominator bond. + assert_noop!( + Staking::reap_stash(RuntimeOrigin::signed(20), 11, 0), + Error::::FundedTarget + ); + + // WHEN: set ledger to below min nominator bond. + Ledger::::insert(11, StakingLedger::::new(11, 999)); + + // THEN: reap-able assert_ok!(Staking::reap_stash(RuntimeOrigin::signed(20), 11, 0)); - // then + // all the data is removed. assert!(!>::contains_key(&11)); assert!(!>::contains_key(&11)); assert!(!>::contains_key(&11)); From d3778846f141c3bfd02be261957aeb4e05b360d0 Mon Sep 17 00:00:00 2001 From: Ankan Date: Tue, 3 Jun 2025 16:27:38 +0200 Subject: [PATCH 14/19] no full unbond needed yet --- substrate/frame/staking-async/src/pallet/mod.rs | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/substrate/frame/staking-async/src/pallet/mod.rs b/substrate/frame/staking-async/src/pallet/mod.rs index 6a0af13e73bff..4260f4eead532 100644 --- a/substrate/frame/staking-async/src/pallet/mod.rs +++ b/substrate/frame/staking-async/src/pallet/mod.rs @@ -1411,8 +1411,7 @@ pub mod pallet { } else if Validators::::contains_key(&stash) { MinValidatorBond::::get() } else { - // FAIL-CI - // TODO: port full unbond (unbond + chill) + // staker is chilled, no min bond. Zero::zero() }; From 31c5c85a3b00cd91396ba3effbcd86beeb7efefa Mon Sep 17 00:00:00 2001 From: Ankan Date: Tue, 3 Jun 2025 16:47:40 +0200 Subject: [PATCH 15/19] fmt --- substrate/frame/staking-async/src/tests/bonding.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/substrate/frame/staking-async/src/tests/bonding.rs b/substrate/frame/staking-async/src/tests/bonding.rs index 0de8a3396035f..c18014649cbb2 100644 --- a/substrate/frame/staking-async/src/tests/bonding.rs +++ b/substrate/frame/staking-async/src/tests/bonding.rs @@ -1446,8 +1446,8 @@ mod reap { Error::::FundedTarget ); - // Note: Even though the stash is a validator, the threshold to reap is min of nominator - // and validator bond + // Note: Even though the stash is a validator, the threshold to reap is min of + // nominator and validator bond // no easy way to cause an account to go below ED, we tweak their staking ledger // instead. From 8c7c840f3a5c4a11a59e1868cefef0e6b72845bf Mon Sep 17 00:00:00 2001 From: Ankan Date: Tue, 3 Jun 2025 22:27:09 +0200 Subject: [PATCH 16/19] add era duration in non default runtimes --- substrate/frame/staking-async/ahm-test/src/ah/mock.rs | 1 + .../frame/staking-async/runtimes/parachain/src/staking.rs | 5 +++++ 2 files changed, 6 insertions(+) diff --git a/substrate/frame/staking-async/ahm-test/src/ah/mock.rs b/substrate/frame/staking-async/ahm-test/src/ah/mock.rs index 7b6feae93881a..50676faca3193 100644 --- a/substrate/frame/staking-async/ahm-test/src/ah/mock.rs +++ b/substrate/frame/staking-async/ahm-test/src/ah/mock.rs @@ -341,6 +341,7 @@ impl pallet_staking_async::Config for Runtime { type RewardRemainder = (); type Slash = (); type SlashDeferDuration = SlashDeferredDuration; + type MaxEraDuration = (); type HistoryDepth = ConstU32<7>; type MaxControllersInDeprecationBatch = (); diff --git a/substrate/frame/staking-async/runtimes/parachain/src/staking.rs b/substrate/frame/staking-async/runtimes/parachain/src/staking.rs index ce924061fa8ec..d6fc784211bbd 100644 --- a/substrate/frame/staking-async/runtimes/parachain/src/staking.rs +++ b/substrate/frame/staking-async/runtimes/parachain/src/staking.rs @@ -244,6 +244,10 @@ parameter_types! { // of nominators. pub const MaxControllersInDeprecationBatch: u32 = 751; pub const MaxNominations: u32 = ::LIMIT as u32; + // Note: In WAH, this should be set closer to the ideal era duration to trigger capping more + // frequently. On Kusama and Polkadot, a higher value like 7 × ideal_era_duration is more + // appropriate. + pub const MaxEraDuration: u64 = RelaySessionDuration::get() as u64 * RELAY_CHAIN_SLOT_DURATION_MILLIS as u64 * SessionsPerEra::get() as u64; } impl pallet_staking_async::Config for Runtime { @@ -273,6 +277,7 @@ impl pallet_staking_async::Config for Runtime { type EventListeners = (NominationPools, DelegatedStaking); type WeightInfo = weights::pallet_staking_async::WeightInfo; type MaxInvulnerables = frame_support::traits::ConstU32<20>; + type MaxEraDuration = MaxEraDuration; type MaxDisabledValidators = ConstU32<100>; type PlanningEraOffset = pallet_staking_async::PlanningEraOffsetOf>; From 84ec94badc8d83a713f50cc612ff588f21c8e166 Mon Sep 17 00:00:00 2001 From: "cmd[bot]" <41898282+github-actions[bot]@users.noreply.github.com> Date: Wed, 4 Jun 2025 07:42:12 +0000 Subject: [PATCH 17/19] Update from github-actions[bot] running command 'prdoc --audience runtime_dev' --- prdoc/pr_8701.prdoc | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) create mode 100644 prdoc/pr_8701.prdoc diff --git a/prdoc/pr_8701.prdoc b/prdoc/pr_8701.prdoc new file mode 100644 index 0000000000000..618265211cc65 --- /dev/null +++ b/prdoc/pr_8701.prdoc @@ -0,0 +1,18 @@ +title: '[Staking] Cleanups and some improvements' +doc: +- audience: Runtime Dev + description: |- + ## Changes + - Introduced a new `min_bond` value, which is the minimum of `MinValidatorBond` and `MinNominatorBond`, with a fallback to `ExistentialDeposit`. Since ED on AH is much lower than on RC, this ensures we enforce some min bonds for staking to cover storage costs for staking ledger and related data. + - Added an upper bound on era duration, protecting against anomalous conditions that could otherwise lead to excessive inflation. + - Some refactors to gracefully handle unexpected validator activation in RC. + + + ## TODO + - [ ] Set `MaxEraDuration` in WAH. + - [ ] Port [full unbond](https://github.com/paritytech/polkadot-sdk/pull/3811) (will do in a separate PR) +crates: +- name: pallet-staking-async + bump: major +- name: pallet-staking-async-parachain-runtime + bump: major From 0a3723fb35c93268e1a485e2bad4be4db3bb7e54 Mon Sep 17 00:00:00 2001 From: "cmd[bot]" <41898282+github-actions[bot]@users.noreply.github.com> Date: Mon, 9 Jun 2025 15:18:22 +0000 Subject: [PATCH 18/19] Update from github-actions[bot] running command 'fmt' --- substrate/frame/staking-async/src/session_rotation.rs | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/substrate/frame/staking-async/src/session_rotation.rs b/substrate/frame/staking-async/src/session_rotation.rs index b28b3f9e762b8..1122a864ebee1 100644 --- a/substrate/frame/staking-async/src/session_rotation.rs +++ b/substrate/frame/staking-async/src/session_rotation.rs @@ -513,10 +513,7 @@ impl Rotator { #[cfg(any(feature = "try-runtime", feature = "std", feature = "runtime-benchmarks", test))] pub fn assert_election_ongoing() { - assert!( - Self::planning_era().is_some(), - "planning era must exist" - ); + assert!(Self::planning_era().is_some(), "planning era must exist"); assert!( T::ElectionProvider::status().is_ok(), "Election provider must be in a good state during election" From 81ee0ae158a1a74606b4a0864b365dd58545ae5a Mon Sep 17 00:00:00 2001 From: Ankan Date: Mon, 9 Jun 2025 19:09:17 +0200 Subject: [PATCH 19/19] PR feedback --- prdoc/pr_8701.prdoc | 18 ------------ .../frame/staking-async/src/benchmarking.rs | 17 ++++++----- .../frame/staking-async/src/pallet/impls.rs | 28 +++++++++++++------ .../frame/staking-async/src/pallet/mod.rs | 24 ++++++++-------- .../staking-async/src/session_rotation.rs | 10 +++---- 5 files changed, 45 insertions(+), 52 deletions(-) delete mode 100644 prdoc/pr_8701.prdoc diff --git a/prdoc/pr_8701.prdoc b/prdoc/pr_8701.prdoc deleted file mode 100644 index 618265211cc65..0000000000000 --- a/prdoc/pr_8701.prdoc +++ /dev/null @@ -1,18 +0,0 @@ -title: '[Staking] Cleanups and some improvements' -doc: -- audience: Runtime Dev - description: |- - ## Changes - - Introduced a new `min_bond` value, which is the minimum of `MinValidatorBond` and `MinNominatorBond`, with a fallback to `ExistentialDeposit`. Since ED on AH is much lower than on RC, this ensures we enforce some min bonds for staking to cover storage costs for staking ledger and related data. - - Added an upper bound on era duration, protecting against anomalous conditions that could otherwise lead to excessive inflation. - - Some refactors to gracefully handle unexpected validator activation in RC. - - - ## TODO - - [ ] Set `MaxEraDuration` in WAH. - - [ ] Port [full unbond](https://github.com/paritytech/polkadot-sdk/pull/3811) (will do in a separate PR) -crates: -- name: pallet-staking-async - bump: major -- name: pallet-staking-async-parachain-runtime - bump: major diff --git a/substrate/frame/staking-async/src/benchmarking.rs b/substrate/frame/staking-async/src/benchmarking.rs index 05226d33d963b..11f70a5a8e6bf 100644 --- a/substrate/frame/staking-async/src/benchmarking.rs +++ b/substrate/frame/staking-async/src/benchmarking.rs @@ -229,7 +229,7 @@ mod benchmarks { // clean up any existing state. clear_validators_and_nominators::(); - let origin_weight = MinNominatorBond::::get().max(asset::existential_deposit::()); + let origin_weight = Staking::::min_nominator_bond(); // setup the worst case list scenario. @@ -318,7 +318,7 @@ mod benchmarks { // clean up any existing state. clear_validators_and_nominators::(); - let origin_weight = MinNominatorBond::::get().max(asset::existential_deposit::()); + let origin_weight = Staking::::min_nominator_bond(); // setup a worst case list scenario. Note that we don't care about the setup of the // destination position because we are doing a removal from the list but no insert. @@ -442,7 +442,7 @@ mod benchmarks { // clean up any existing state. clear_validators_and_nominators::(); - let origin_weight = MinNominatorBond::::get().max(asset::existential_deposit::()); + let origin_weight = Staking::::min_nominator_bond(); // setup a worst case list scenario. Note we don't care about the destination position, // because we are just doing an insert into the origin position. @@ -475,7 +475,7 @@ mod benchmarks { // clean up any existing state. clear_validators_and_nominators::(); - let origin_weight = MinNominatorBond::::get().max(asset::existential_deposit::()); + let origin_weight = Staking::::min_nominator_bond(); // setup a worst case list scenario. Note that we don't care about the setup of the // destination position because we are doing a removal from the list but no insert. @@ -633,7 +633,7 @@ mod benchmarks { // Clean up any existing state. clear_validators_and_nominators::(); - let origin_weight = MinNominatorBond::::get().max(asset::existential_deposit::()); + let origin_weight = Staking::::min_nominator_bond(); // setup a worst case list scenario. Note that we don't care about the setup of the // destination position because we are doing a removal from the list but no insert. @@ -734,8 +734,7 @@ mod benchmarks { // clean up any existing state. clear_validators_and_nominators::(); - let origin_weight = MinNominatorBond::::get() - .max(asset::existential_deposit::()) + let origin_weight = Pallet::::min_nominator_bond() // we use 100 to play friendly with the list threshold values in the mock .max(100u32.into()); @@ -781,7 +780,7 @@ mod benchmarks { // clean up any existing state. clear_validators_and_nominators::(); - let origin_weight = MinNominatorBond::::get().max(asset::existential_deposit::()); + let origin_weight = Staking::::min_nominator_bond(); // setup a worst case list scenario. Note that we don't care about the setup of the // destination position because we are doing a removal from the list but no insert. @@ -858,7 +857,7 @@ mod benchmarks { // clean up any existing state. clear_validators_and_nominators::(); - let origin_weight = MinNominatorBond::::get().max(asset::existential_deposit::()); + let origin_weight = Staking::::min_nominator_bond(); // setup a worst case list scenario. Note that we don't care about the setup of the // destination position because we are doing a removal from the list but no insert. diff --git a/substrate/frame/staking-async/src/pallet/impls.rs b/substrate/frame/staking-async/src/pallet/impls.rs index b9a117cacbd14..947a821a044de 100644 --- a/substrate/frame/staking-async/src/pallet/impls.rs +++ b/substrate/frame/staking-async/src/pallet/impls.rs @@ -72,18 +72,30 @@ use sp_runtime::TryRuntimeError; const NPOS_MAX_ITERATIONS_COEFFICIENT: u32 = 2; impl Pallet { - /// Returns the minimum required bond for participation, considering validators, nominators, + /// Returns the minimum required bond for participation, considering nominators, /// and the chain’s existential deposit. /// /// This function computes the smallest allowed bond among `MinValidatorBond` and /// `MinNominatorBond`, but ensures it is not below the existential deposit required to keep an /// account alive. - pub(crate) fn min_bond() -> BalanceOf { + pub(crate) fn min_chilled_bond() -> BalanceOf { MinValidatorBond::::get() .min(MinNominatorBond::::get()) .max(asset::existential_deposit::()) } + /// Returns the minimum required bond for validators, defaulting to `MinNominatorBond` if not + /// set or less than `MinNominatorBond`. + pub(crate) fn min_validator_bond() -> BalanceOf { + MinValidatorBond::::get().max(Self::min_nominator_bond()) + } + + /// Returns the minimum required bond for nominators, considering the chain’s existential + /// deposit. + pub(crate) fn min_nominator_bond() -> BalanceOf { + MinNominatorBond::::get().max(asset::existential_deposit::()) + } + /// Fetches the ledger associated with a controller or stash account, if any. pub fn ledger(account: StakingAccount) -> Result, Error> { StakingLedger::::get(account) @@ -182,7 +194,7 @@ impl Pallet { ledger.total = ledger.total.checked_add(&extra).ok_or(ArithmeticError::Overflow)?; ledger.active = ledger.active.checked_add(&extra).ok_or(ArithmeticError::Overflow)?; // last check: the new active amount of ledger must be more than min bond. - ensure!(ledger.active >= Self::min_bond(), Error::::InsufficientBond); + ensure!(ledger.active >= Self::min_chilled_bond(), Error::::InsufficientBond); // NOTE: ledger must be updated prior to calling `Self::weight_of`. ledger.update()?; @@ -206,7 +218,7 @@ impl Pallet { let new_total = ledger.total; let used_weight = if ledger.unlocking.is_empty() && - (ledger.active < Self::min_bond() || ledger.active.is_zero()) + (ledger.active < Self::min_chilled_bond() || ledger.active.is_zero()) { // This account must have called `unbond()` with some value that caused the active // portion to fall below existential deposit + will have no more unlocking chunks @@ -984,7 +996,7 @@ impl ElectionDataProvider for Pallet { #[cfg(feature = "runtime-benchmarks")] fn add_target(target: T::AccountId) { - let stake = (MinValidatorBond::::get() + 1u32.into()) * 100u32.into(); + let stake = (Self::min_validator_bond() + 1u32.into()) * 100u32.into(); >::insert(target.clone(), target.clone()); >::insert(target.clone(), StakingLedger::::new(target.clone(), stake)); Self::do_add_validator( @@ -1016,7 +1028,7 @@ impl ElectionDataProvider for Pallet { targets.into_iter().for_each(|v| { let stake: BalanceOf = target_stake .and_then(|w| >::try_from(w).ok()) - .unwrap_or_else(|| MinNominatorBond::::get() * 100u32.into()); + .unwrap_or_else(|| Self::min_nominator_bond() * 100u32.into()); >::insert(v.clone(), v.clone()); >::insert(v.clone(), StakingLedger::::new(v.clone(), stake)); Self::do_add_validator( @@ -1437,11 +1449,11 @@ impl StakingInterface for Pallet { type CurrencyToVote = T::CurrencyToVote; fn minimum_nominator_bond() -> Self::Balance { - MinNominatorBond::::get() + Self::min_nominator_bond() } fn minimum_validator_bond() -> Self::Balance { - MinValidatorBond::::get() + Self::min_validator_bond() } fn stash_by_ctrl(controller: &Self::AccountId) -> Result { diff --git a/substrate/frame/staking-async/src/pallet/mod.rs b/substrate/frame/staking-async/src/pallet/mod.rs index d504b5ae6daa1..525e742e1ac3e 100644 --- a/substrate/frame/staking-async/src/pallet/mod.rs +++ b/substrate/frame/staking-async/src/pallet/mod.rs @@ -1150,7 +1150,7 @@ pub mod pallet { DuplicateIndex, /// Slash record not found. InvalidSlashRecord, - /// Cannot have a validator or nominator role, with value less than the minimum defined by + /// Cannot bond, nominate or validate with value less than the minimum defined by /// governance (see `MinValidatorBond` and `MinNominatorBond`). If unbonding is the /// intention, `chill` first to remove one's role as validator/nominator. InsufficientBond, @@ -1308,7 +1308,7 @@ pub mod pallet { } // Reject a bond which is lower than the minimum bond. - if value < Self::min_bond() { + if value < Self::min_chilled_bond() { return Err(Error::::InsufficientBond.into()); } @@ -1407,9 +1407,9 @@ pub mod pallet { } let min_active_bond = if Nominators::::contains_key(&stash) { - MinNominatorBond::::get() + Self::min_nominator_bond() } else if Validators::::contains_key(&stash) { - MinValidatorBond::::get() + Self::min_validator_bond() } else { // staker is chilled, no min bond. Zero::zero() @@ -1493,7 +1493,7 @@ pub mod pallet { let ledger = Self::ledger(Controller(controller))?; - ensure!(ledger.active >= MinValidatorBond::::get(), Error::::InsufficientBond); + ensure!(ledger.active >= Self::min_validator_bond(), Error::::InsufficientBond); let stash = &ledger.stash; // ensure their commission is correct. @@ -1534,7 +1534,7 @@ pub mod pallet { let ledger = Self::ledger(StakingAccount::Controller(controller.clone()))?; - ensure!(ledger.active >= MinNominatorBond::::get(), Error::::InsufficientBond); + ensure!(ledger.active >= Self::min_nominator_bond(), Error::::InsufficientBond); let stash = &ledger.stash; // Only check limits if they are not already a nominator. @@ -1889,7 +1889,7 @@ pub mod pallet { let initial_unlocking = ledger.unlocking.len() as u32; let (ledger, rebonded_value) = ledger.rebond(value); // Last check: the new active amount of ledger must be more than min bond. - ensure!(ledger.active >= Self::min_bond(), Error::::InsufficientBond); + ensure!(ledger.active >= Self::min_chilled_bond(), Error::::InsufficientBond); Self::deposit_event(Event::::Bonded { stash: ledger.stash.clone(), @@ -1942,13 +1942,13 @@ pub mod pallet { // virtual stakers should not be allowed to be reaped. ensure!(!Self::is_virtual_staker(&stash), Error::::VirtualStakerNotAllowed); - let min_bond = Self::min_bond(); + let min_chilled_bond = Self::min_chilled_bond(); let origin_balance = asset::total_balance::(&stash); let ledger_total = Self::ledger(Stash(stash.clone())).map(|l| l.total).unwrap_or_default(); - let reapable = origin_balance < min_bond || + let reapable = origin_balance < min_chilled_bond || origin_balance.is_zero() || - ledger_total < min_bond || + ledger_total < min_chilled_bond || ledger_total.is_zero(); ensure!(reapable, Error::::FundedTarget); @@ -2123,7 +2123,7 @@ pub mod pallet { threshold * max_nominator_count < current_nominator_count, Error::::CannotChillOther ); - MinNominatorBond::::get() + Self::min_nominator_bond() } else if Validators::::contains_key(&stash) { let max_validator_count = MaxValidatorsCount::::get().ok_or(Error::::CannotChillOther)?; @@ -2132,7 +2132,7 @@ pub mod pallet { threshold * max_validator_count < current_validator_count, Error::::CannotChillOther ); - MinValidatorBond::::get() + Self::min_validator_bond() } else { Zero::zero() }; diff --git a/substrate/frame/staking-async/src/session_rotation.rs b/substrate/frame/staking-async/src/session_rotation.rs index 1122a864ebee1..fc0a1996f17bf 100644 --- a/substrate/frame/staking-async/src/session_rotation.rs +++ b/substrate/frame/staking-async/src/session_rotation.rs @@ -513,7 +513,7 @@ impl Rotator { #[cfg(any(feature = "try-runtime", feature = "std", feature = "runtime-benchmarks", test))] pub fn assert_election_ongoing() { - assert!(Self::planning_era().is_some(), "planning era must exist"); + assert!(Self::is_planning().is_some(), "planning era must exist"); assert!( T::ElectionProvider::status().is_ok(), "Election provider must be in a good state during election" @@ -526,7 +526,7 @@ impl Rotator { /// is underway, but rather the last era that was planned. If `Self::active_era()` is equal to /// this value, it means that the era is currently active and no new era is planned. /// - /// See [`Self::planning_era()`] to only get the next index that is planned. + /// See [`Self::is_planning()`] to only get the next index if planning in progress. pub fn planned_era() -> EraIndex { CurrentEra::::get().unwrap_or(0) } @@ -538,7 +538,7 @@ impl Rotator { /// Next era that is planned to be started. /// /// Returns None if no era is planned. - pub fn planning_era() -> Option { + pub fn is_planning() -> Option { let (active, planned) = (Self::active_era(), Self::planned_era()); if planned.defensive_saturating_sub(active) > 1 { defensive!("planned era must always be equal or one more than active"); @@ -553,7 +553,7 @@ impl Rotator { defensive!("Active era must always be available."); return; }; - let current_planned_era = Self::planning_era(); + let current_planned_era = Self::is_planning(); let starting = end_index + 1; // the session after the starting session. let planning = starting + 1; @@ -605,7 +605,7 @@ impl Rotator { // Note: we call `planning_era` again, as a new era might have started since we checked // it last. - let has_pending_era = Self::planning_era().is_some(); + let has_pending_era = Self::is_planning().is_some(); match (should_plan_era, has_pending_era) { (false, _) => { // nothing to consider