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..50676faca3193 100644 --- a/substrate/frame/staking-async/ahm-test/src/ah/mock.rs +++ b/substrate/frame/staking-async/ahm-test/src/ah/mock.rs @@ -83,11 +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, activation_timestamp: None, @@ -342,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>; diff --git a/substrate/frame/staking-async/src/benchmarking.rs b/substrate/frame/staking-async/src/benchmarking.rs index 9960fb960782f..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. @@ -1024,14 +1023,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 +1072,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 +1084,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 +1134,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 07f9c997eb061..674212649206e 100644 --- a/substrate/frame/staking-async/src/mock.rs +++ b/substrate/frame/staking-async/src/mock.rs @@ -312,16 +312,29 @@ 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, + // 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)), + ), + ); 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); } @@ -385,6 +398,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 { @@ -423,6 +437,7 @@ impl crate::pallet::pallet::Config for Test { type EventListeners = EventListenerMock; type MaxInvulnerables = ConstU32<20>; type MaxDisabledValidators = ConstU32<100>; + type MaxEraDuration = MaxEraDuration; type PlanningEraOffset = PlanningEraOffset; type Filter = MockedRestrictList; type RcClientInterface = session_mock::Session; diff --git a/substrate/frame/staking-async/src/pallet/impls.rs b/substrate/frame/staking-async/src/pallet/impls.rs index df66f2b45fda8..947a821a044de 100644 --- a/substrate/frame/staking-async/src/pallet/impls.rs +++ b/substrate/frame/staking-async/src/pallet/impls.rs @@ -72,6 +72,30 @@ use sp_runtime::TryRuntimeError; const NPOS_MAX_ITERATIONS_COEFFICIENT: u32 = 2; impl Pallet { + /// 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_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) @@ -169,8 +193,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_chilled_bond(), Error::::InsufficientBond); // NOTE: ledger must be updated prior to calling `Self::weight_of`. ledger.update()?; @@ -193,22 +217,22 @@ 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()) { - // 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)?; + let used_weight = if ledger.unlocking.is_empty() && + (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 + // 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()?; + 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() - }; + // 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. @@ -972,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( @@ -1004,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( @@ -1425,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 1996b5df8396a..525e742e1ac3e 100644 --- a/substrate/frame/staking-async/src/pallet/mod.rs +++ b/substrate/frame/staking-async/src/pallet/mod.rs @@ -312,6 +312,17 @@ 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 +384,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 +1115,22 @@ 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, + /// Received a validator activation event that is not recognized. + UnknownValidatorActivation, } #[pallet::error] @@ -1122,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, @@ -1279,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_chilled_bond() { return Err(Error::::InsufficientBond.into()); } @@ -1379,10 +1407,11 @@ 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() }; @@ -1464,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. @@ -1505,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. @@ -1859,11 +1888,8 @@ 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. - 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_chilled_bond(), Error::::InsufficientBond); Self::deposit_event(Event::::Bonded { stash: ledger.stash.clone(), @@ -1888,8 +1914,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 @@ -1916,13 +1942,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_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 < ed || + let reapable = origin_balance < min_chilled_bond || origin_balance.is_zero() || - ledger_total < ed || + ledger_total < min_chilled_bond || ledger_total.is_zero(); ensure!(reapable, Error::::FundedTarget); @@ -2097,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)?; @@ -2106,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 3cbe52a327bd1..fc0a1996f17bf 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, @@ -513,17 +513,21 @@ impl Rotator { #[cfg(any(feature = "try-runtime", feature = "std", feature = "runtime-benchmarks", test))] pub fn assert_election_ongoing() { - assert!( - Self::planning_era() == Self::active_era() + 1, - "planning era must be one more than active era during election" - ); + 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" ); } - 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::is_planning()`] to only get the next index if planning in progress. + pub fn planned_era() -> EraIndex { CurrentEra::::get().unwrap_or(0) } @@ -531,13 +535,25 @@ 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 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"); + } + + (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 { 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; @@ -553,7 +569,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); }, @@ -561,15 +577,17 @@ 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. @@ -585,7 +603,9 @@ impl Rotator { Forcing::ForceNone => false, }; - let has_pending_era = active_era < current_planned_era; + // Note: we call `planning_era` again, as a new era might have started since we checked + // it last. + let has_pending_era = Self::is_planning().is_some(); match (should_plan_era, has_pending_era) { (false, _) => { // nothing to consider @@ -599,7 +619,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 ); }, @@ -608,7 +628,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(), }); } @@ -682,7 +702,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); } @@ -908,7 +951,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/bonding.rs b/substrate/frame/staking-async/src/tests/bonding.rs index b67b2a09e6e24..c18014649cbb2 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)); @@ -1596,23 +1611,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)); diff --git a/substrate/frame/staking-async/src/tests/era_rotation.rs b/substrate/frame/staking-async/src/tests/era_rotation.rs index ac06eba76bbac..5c1406ff8ec21 100644 --- a/substrate/frame/staking-async/src/tests/era_rotation.rs +++ b/substrate/frame/staking-async/src/tests/era_rotation.rs @@ -15,7 +15,10 @@ // See the License for the specific language governing permissions and // limitations under the License. -use crate::session_rotation::Eras; +use crate::{ + session_rotation::{Eras, Rotator}, + tests::session_mock::{CurrentIndex, Timestamp}, +}; use super::*; @@ -178,10 +181,43 @@ 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] @@ -192,9 +228,66 @@ 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(|| { + // 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); + 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 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![ + Event::SessionRotated { starting_session: 7, active_era: 2, planned_era: 3 }, + Event::PagedElectionProceeded { page: 0, result: Ok(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 } + ] + ); + }); } #[test]