From 941fb484ac032898301f72e76b41c4ba47328456 Mon Sep 17 00:00:00 2001 From: Ankan Date: Wed, 2 Jul 2025 21:15:42 +0200 Subject: [PATCH 01/25] only allow withdraw until the eras for which offences have been processed --- .../frame/staking-async/src/pallet/impls.rs | 27 ++++++++++++++++--- .../frame/staking-async/src/pallet/mod.rs | 18 ++++++++----- 2 files changed, 35 insertions(+), 10 deletions(-) diff --git a/substrate/frame/staking-async/src/pallet/impls.rs b/substrate/frame/staking-async/src/pallet/impls.rs index 54bdc8195f1a5..d9a2eddc92f35 100644 --- a/substrate/frame/staking-async/src/pallet/impls.rs +++ b/substrate/frame/staking-async/src/pallet/impls.rs @@ -212,10 +212,31 @@ impl Pallet { pub(super) fn do_withdraw_unbonded(controller: &T::AccountId) -> Result { let mut ledger = Self::ledger(Controller(controller.clone()))?; let (stash, old_total) = (ledger.stash.clone(), ledger.total); - if let Some(current_era) = CurrentEra::::get() { - ledger = ledger.consolidate_unlocked(current_era) - } + let active_era = Rotator::::active_era(); + + // check the oldest era for which offences are not processed. + let oldest_unprocessed_offence_era = OffenceQueueEras::::get() + .as_ref() + .and_then(|eras| eras.first()) + .copied() + .unwrap_or(u32::MAX); + + // If there are unprocessed offences older than the current active era, withdrawals are only + // allowed up to the last era for which offences have been processed. + // Note: This situation is extremely unlikely, since offences have `SlashDeferDuration` eras + // to be processed. If it ever occurs, it likely indicates offence spam and that we're + // struggling to keep up with processing. + let earliest_era_to_withdraw = active_era + .min(oldest_unprocessed_offence_era.saturating_sub(1)); + + // withdraw unbonded balance from the ledger until earliest_era_to_withdraw. + ledger = ledger.consolidate_unlocked(earliest_era_to_withdraw); + let new_total = ledger.total; + debug_assert!( + new_total <= old_total, + "consolidate_unlocked should never increase the total balance of the ledger" + ); let used_weight = if ledger.unlocking.is_empty() && (ledger.active < Self::min_chilled_bond() || ledger.active.is_zero()) diff --git a/substrate/frame/staking-async/src/pallet/mod.rs b/substrate/frame/staking-async/src/pallet/mod.rs index 5a30ae00362dd..0a8ea3c549ae4 100644 --- a/substrate/frame/staking-async/src/pallet/mod.rs +++ b/substrate/frame/staking-async/src/pallet/mod.rs @@ -1414,8 +1414,8 @@ pub mod pallet { let unlocking = Self::ledger(Controller(controller.clone())).map(|l| l.unlocking.len())?; - // if there are no unlocking chunks available, try to withdraw chunks older than - // `BondingDuration` to proceed with the unbonding. + // if there are no unlocking chunks available, try to remove any chunks by withdrawing + // funds that have fully unbonded. let maybe_withdraw_weight = { if unlocking == T::MaxUnlockingChunks::get() as usize { Some(Self::do_withdraw_unbonded(&controller)?) @@ -1492,10 +1492,14 @@ pub mod pallet { Ok(actual_weight.into()) } - /// Remove any unlocked chunks from the `unlocking` queue from our management. + /// Remove any stake that has been fully unbonded and is ready for withdrawal. /// - /// This essentially frees up that balance to be used by the stash account to do whatever - /// it wants. + /// Stake is considered fully unbonded once [`Config::BondingDuration`] has elapsed since + /// the unbonding was initiated. In rare cases—such as when offences for the unbonded era + /// have been reported but not yet processed—withdrawal is restricted to eras for which + /// all offences have been processed. + /// + /// The unlocked stake will be returned as free balance in the stash account. /// /// The dispatch origin for this call must be _Signed_ by the controller. /// @@ -1505,8 +1509,8 @@ pub mod pallet { /// /// ## Parameters /// - /// - `num_slashing_spans`: **Deprecated**. This parameter is retained for backward - /// compatibility. It no longer has any effect. + /// - `num_slashing_spans`: **Deprecated**. Retained only for backward compatibility; + /// this parameter has no effect. #[pallet::call_index(3)] #[pallet::weight(T::WeightInfo::withdraw_unbonded_kill())] pub fn withdraw_unbonded( From 857016b68433a0e5172e1da9210ee3ebe3157164 Mon Sep 17 00:00:00 2001 From: Ankan Date: Wed, 2 Jul 2025 21:31:13 +0200 Subject: [PATCH 02/25] cargo fmt --- substrate/frame/staking-async/src/pallet/impls.rs | 4 ++-- substrate/frame/staking-async/src/pallet/mod.rs | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/substrate/frame/staking-async/src/pallet/impls.rs b/substrate/frame/staking-async/src/pallet/impls.rs index d9a2eddc92f35..ceaab6e40e82e 100644 --- a/substrate/frame/staking-async/src/pallet/impls.rs +++ b/substrate/frame/staking-async/src/pallet/impls.rs @@ -226,8 +226,8 @@ impl Pallet { // Note: This situation is extremely unlikely, since offences have `SlashDeferDuration` eras // to be processed. If it ever occurs, it likely indicates offence spam and that we're // struggling to keep up with processing. - let earliest_era_to_withdraw = active_era - .min(oldest_unprocessed_offence_era.saturating_sub(1)); + let earliest_era_to_withdraw = + active_era.min(oldest_unprocessed_offence_era.saturating_sub(1)); // withdraw unbonded balance from the ledger until earliest_era_to_withdraw. ledger = ledger.consolidate_unlocked(earliest_era_to_withdraw); diff --git a/substrate/frame/staking-async/src/pallet/mod.rs b/substrate/frame/staking-async/src/pallet/mod.rs index 0a8ea3c549ae4..7c147fb67d11d 100644 --- a/substrate/frame/staking-async/src/pallet/mod.rs +++ b/substrate/frame/staking-async/src/pallet/mod.rs @@ -1509,8 +1509,8 @@ pub mod pallet { /// /// ## Parameters /// - /// - `num_slashing_spans`: **Deprecated**. Retained only for backward compatibility; - /// this parameter has no effect. + /// - `num_slashing_spans`: **Deprecated**. Retained only for backward compatibility; this + /// parameter has no effect. #[pallet::call_index(3)] #[pallet::weight(T::WeightInfo::withdraw_unbonded_kill())] pub fn withdraw_unbonded( From 34e5ca6098d502b7c4810d2651927f4fe764f635 Mon Sep 17 00:00:00 2001 From: Ankan Date: Thu, 3 Jul 2025 09:37:45 +0200 Subject: [PATCH 03/25] set active era instead of current --- substrate/frame/staking-async/src/benchmarking.rs | 4 ++-- substrate/frame/staking-async/src/testing_utils.rs | 8 ++++++++ 2 files changed, 10 insertions(+), 2 deletions(-) diff --git a/substrate/frame/staking-async/src/benchmarking.rs b/substrate/frame/staking-async/src/benchmarking.rs index 987bbbbc49cf7..1b133f1b88f29 100644 --- a/substrate/frame/staking-async/src/benchmarking.rs +++ b/substrate/frame/staking-async/src/benchmarking.rs @@ -311,7 +311,7 @@ mod benchmarks { let (_, controller) = create_stash_controller::(0, 100, RewardDestination::Staked)?; let amount = asset::existential_deposit::() * 5u32.into(); // Half of total Staking::::unbond(RawOrigin::Signed(controller.clone()).into(), amount)?; - CurrentEra::::put(EraIndex::max_value()); + set_active_era::(EraIndex::max_value()); let ledger = Ledger::::get(&controller).ok_or("ledger not created before")?; let original_total: BalanceOf = ledger.total; whitelist_account!(controller); @@ -345,7 +345,7 @@ mod benchmarks { let mut ledger = Ledger::::get(&controller).unwrap(); ledger.active = ed - One::one(); Ledger::::insert(&controller, ledger); - CurrentEra::::put(EraIndex::max_value()); + set_active_era::(EraIndex::max_value()); whitelist_account!(controller); diff --git a/substrate/frame/staking-async/src/testing_utils.rs b/substrate/frame/staking-async/src/testing_utils.rs index a5ef580414aba..3028b3a4e55aa 100644 --- a/substrate/frame/staking-async/src/testing_utils.rs +++ b/substrate/frame/staking-async/src/testing_utils.rs @@ -259,3 +259,11 @@ pub fn migrate_to_old_currency(who: T::AccountId) { // replicate old behaviour of explicit increment of consumer. frame_system::Pallet::::inc_consumers(&who).expect("increment consumer failed"); } + +/// Set active era to the given era index. +pub fn set_active_era(era: EraIndex) { + // set the current era. + CurrentEra::::put(era); + // set the active era. + ActiveEra::::put(ActiveEraInfo { index: era, start: None }); +} From 737fe41b56b31619bd27e16c509525c8ba550df2 Mon Sep 17 00:00:00 2001 From: Ankan Date: Thu, 3 Jul 2025 10:46:52 +0200 Subject: [PATCH 04/25] withdrawal blocking test --- .../frame/staking-async/src/tests/bonding.rs | 2 + .../frame/staking-async/src/tests/slashing.rs | 59 +++++++++++++++++++ 2 files changed, 61 insertions(+) diff --git a/substrate/frame/staking-async/src/tests/bonding.rs b/substrate/frame/staking-async/src/tests/bonding.rs index 28fc6c7229624..3f8b66ff3f774 100644 --- a/substrate/frame/staking-async/src/tests/bonding.rs +++ b/substrate/frame/staking-async/src/tests/bonding.rs @@ -15,6 +15,8 @@ // See the License for the specific language governing permissions and // limitations under the License. +//! Tests concerning bond, bond_extra, unbond, rebond, withdraw and chill for stakers. + use super::*; use frame_support::{hypothetically_ok, traits::Currency}; diff --git a/substrate/frame/staking-async/src/tests/slashing.rs b/substrate/frame/staking-async/src/tests/slashing.rs index c55004cc9c409..7d97ce6ef09da 100644 --- a/substrate/frame/staking-async/src/tests/slashing.rs +++ b/substrate/frame/staking-async/src/tests/slashing.rs @@ -1320,6 +1320,65 @@ fn proportional_ledger_slash_works() { }); } +#[test] +fn withdrawals_are_blocked_for_unprocessed_offence_eras() { + ExtBuilder::default() + .slash_defer_duration(2) + .bonding_duration(3) + .build_and_execute(|| { + let bonding_duration = BondingDuration::get(); + assert_eq!(bonding_duration, 3); + // we have 15 blocks per era. + let blocks_per_era = Period::get() * SessionsPerEra::get() as u64; + assert_eq!(blocks_per_era, 15); + + // Set up nominator and validator + let validator = 300; + let nominator = 301; + bond_validator(validator, 1000); + bond_nominator(nominator, 500, vec![validator]); + + // create unbonding chunks for next 4 eras. + let mut unbond_val_in_era = vec![0 as Balance; 6]; + let mut block_height = System::block_number(); + + for e in 2..=5 { + Session::roll_until_active_era(e); + + // sanity check that we have expected number of blocks in the era. + assert_eq!(System::block_number(), block_height + blocks_per_era); + block_height = System::block_number(); + + // initiate unbonding in each era. + unbond_val_in_era[e as usize] = e as Balance * 5; + assert_ok!(Staking::unbond(RuntimeOrigin::signed(nominator), unbond_val_in_era[e as usize])); + } + + // current era is 5. + assert_eq!(active_era(), 5); + + // ensure the unbond chunks are created correctly. + assert_eq!(unbond_val_in_era, vec![0, 0, 10, 15, 20, 25]); + let expected_chunks: BoundedVec, MaxUnlockingChunks> = + bounded_vec![ + // era is unbond_era + bonding_duration, starting from era 2 + 3. + UnlockChunk { era: 5, value: 10}, + UnlockChunk { era: 6, value: 15}, + UnlockChunk { era: 7, value: 20}, + UnlockChunk { era: 8, value: 25}, + ]; + + assert_eq!( + Ledger::::get(nominator).unwrap().unlocking, + expected_chunks + ); + + + + + }); +} + mod paged_slashing { use super::*; use crate::slashing::OffenceRecord; From 5276b422294cdd1b31d4714b0f48b96807e736b4 Mon Sep 17 00:00:00 2001 From: Ankan Date: Thu, 3 Jul 2025 13:07:33 +0200 Subject: [PATCH 05/25] add test --- substrate/frame/staking-async/src/mock.rs | 4 + .../frame/staking-async/src/pallet/impls.rs | 29 +++-- .../frame/staking-async/src/tests/slashing.rs | 105 ++++++++++++------ 3 files changed, 99 insertions(+), 39 deletions(-) diff --git a/substrate/frame/staking-async/src/mock.rs b/substrate/frame/staking-async/src/mock.rs index 9d4f0c999a423..f2dd344b8dc4c 100644 --- a/substrate/frame/staking-async/src/mock.rs +++ b/substrate/frame/staking-async/src/mock.rs @@ -1004,3 +1004,7 @@ pub(crate) fn restrict(who: &AccountId) { pub(crate) fn remove_from_restrict_list(who: &AccountId) { RestrictedAccounts::mutate(|l| l.retain(|x| x != who)); } + +pub(crate) fn era_offence_count(era: EraIndex) -> u32 { + OffenceQueue::::iter_prefix_values(era).count() as u32 +} diff --git a/substrate/frame/staking-async/src/pallet/impls.rs b/substrate/frame/staking-async/src/pallet/impls.rs index ceaab6e40e82e..ef932ce7fb4c8 100644 --- a/substrate/frame/staking-async/src/pallet/impls.rs +++ b/substrate/frame/staking-async/src/pallet/impls.rs @@ -214,20 +214,35 @@ impl Pallet { let (stash, old_total) = (ledger.stash.clone(), ledger.total); let active_era = Rotator::::active_era(); - // check the oldest era for which offences are not processed. - let oldest_unprocessed_offence_era = OffenceQueueEras::::get() + // get lowest era for which all offences are processed and withdrawals can be allowed. + let earliest_unlock_era_by_offence_queue = OffenceQueueEras::::get() .as_ref() .and_then(|eras| eras.first()) .copied() - .unwrap_or(u32::MAX); - - // If there are unprocessed offences older than the current active era, withdrawals are only + .unwrap_or(active_era) + // above returns earliest era for which offences are processed, so we subtract one to + // get the last era for which all offences are processed. + .saturating_sub(1) + // Unlock chunks are keyed by the era they were initiated plus Bonding Duration. + // We do the same to processed offence era so they can be compared. + .saturating_add(T::BondingDuration::get()); + + // If there are unprocessed offences older than the active era, withdrawals are only // allowed up to the last era for which offences have been processed. // Note: This situation is extremely unlikely, since offences have `SlashDeferDuration` eras // to be processed. If it ever occurs, it likely indicates offence spam and that we're // struggling to keep up with processing. - let earliest_era_to_withdraw = - active_era.min(oldest_unprocessed_offence_era.saturating_sub(1)); + let earliest_era_to_withdraw = active_era.min(earliest_unlock_era_by_offence_queue); + + log!( + debug, + "Withdrawing unbonded stake. Active_era is: {:?} | \ + Earliest era for which all offences are processed: {:?} | \ + Earliest era we can allow withdrawing: {:?}", + active_era, + earliest_unlock_era_by_offence_queue, + earliest_era_to_withdraw + ); // withdraw unbonded balance from the ledger until earliest_era_to_withdraw. ledger = ledger.consolidate_unlocked(earliest_era_to_withdraw); diff --git a/substrate/frame/staking-async/src/tests/slashing.rs b/substrate/frame/staking-async/src/tests/slashing.rs index 7d97ce6ef09da..d970c428c4007 100644 --- a/substrate/frame/staking-async/src/tests/slashing.rs +++ b/substrate/frame/staking-async/src/tests/slashing.rs @@ -1325,6 +1325,11 @@ fn withdrawals_are_blocked_for_unprocessed_offence_eras() { ExtBuilder::default() .slash_defer_duration(2) .bonding_duration(3) + .add_staker(61, 1000, StakerStatus::Validator) + .add_staker(71, 1000, StakerStatus::Validator) + .add_staker(81, 1000, StakerStatus::Validator) + .add_staker(91, 1000, StakerStatus::Validator) + .validator_count(6) .build_and_execute(|| { let bonding_duration = BondingDuration::get(); assert_eq!(bonding_duration, 3); @@ -1332,51 +1337,87 @@ fn withdrawals_are_blocked_for_unprocessed_offence_eras() { let blocks_per_era = Period::get() * SessionsPerEra::get() as u64; assert_eq!(blocks_per_era, 15); - // Set up nominator and validator - let validator = 300; + // Set up nominator. + let validator = 11; let nominator = 301; - bond_validator(validator, 1000); bond_nominator(nominator, 500, vec![validator]); - // create unbonding chunks for next 4 eras. - let mut unbond_val_in_era = vec![0 as Balance; 6]; - let mut block_height = System::block_number(); - - for e in 2..=5 { - Session::roll_until_active_era(e); + // create unbonding chunks for the next two eras. + Session::roll_until_active_era(2); + assert_ok!(Staking::unbond(RuntimeOrigin::signed(nominator), 100)); + Session::roll_until_active_era(3); + assert_ok!(Staking::unbond(RuntimeOrigin::signed(nominator), 150)); - // sanity check that we have expected number of blocks in the era. - assert_eq!(System::block_number(), block_height + blocks_per_era); - block_height = System::block_number(); + Session::roll_until_active_era(5); - // initiate unbonding in each era. - unbond_val_in_era[e as usize] = e as Balance * 5; - assert_ok!(Staking::unbond(RuntimeOrigin::signed(nominator), unbond_val_in_era[e as usize])); + // Lets go to one block before start of era 6. + for _i in 1..blocks_per_era { + Session::roll_next(); } - // current era is 5. - assert_eq!(active_era(), 5); + // one block before era 6 starts, lets spam the offence pipeline. + add_slash_in_era(21, 3, Perbill::from_percent(10)); + add_slash_in_era(61, 3, Perbill::from_percent(10)); + add_slash_in_era(71, 3, Perbill::from_percent(10)); + add_slash_in_era(81, 3, Perbill::from_percent(10)); + add_slash_in_era(91, 3, Perbill::from_percent(10)); - // ensure the unbond chunks are created correctly. - assert_eq!(unbond_val_in_era, vec![0, 0, 10, 15, 20, 25]); - let expected_chunks: BoundedVec, MaxUnlockingChunks> = - bounded_vec![ - // era is unbond_era + bonding_duration, starting from era 2 + 3. - UnlockChunk { era: 5, value: 10}, - UnlockChunk { era: 6, value: 15}, - UnlockChunk { era: 7, value: 20}, - UnlockChunk { era: 8, value: 25}, - ]; + // lets roll to era 6. + Session::roll_next(); + assert_eq!(active_era(), 6); - assert_eq!( - Ledger::::get(nominator).unwrap().unlocking, - expected_chunks - ); + // and we created 5 offences, of which 1 would be processed since we rolled one block. + assert_eq!(era_offence_count(3), 5 - 1); + assert_eq!(OffenceQueueEras::::get().unwrap(), vec![3]); + // GIVEN Scenario: Our nominator has unbonding chunks scheduled for eras 5 and 6. + // The current active era is 6, so both chunks would normally be eligible for + // withdrawal. However, offences from era 3 (which can affect withdrawals in era 6) is + // unprocessed.Therefore, withdrawal from era 6 should be blocked until those offences + // are handled. + // Unbonding chunks + let expected_chunks: BoundedVec, MaxUnlockingChunks> = bounded_vec![ + // era is unbond_era + bonding_duration, starting from era 2 + 3. + UnlockChunk { era: 5, value: 100 }, + UnlockChunk { era: 6, value: 150 }, + ]; + assert_eq!(Ledger::::get(nominator).unwrap().unlocking, expected_chunks); - }); + // all nominator balance other than ED is staked. + let nominator_balance_pre_withdraw = Balances::free_balance(&nominator); + assert_eq!(nominator_balance_pre_withdraw, 1); + + // WHEN the nominator tries to withdraw unbonded funds. + assert_ok!(Staking::withdraw_unbonded(RuntimeOrigin::signed(nominator), 0)); + + // THEN only the first unbonding chunk is withdrawn, as the second one is blocked by + // unprocessed offences. + let nominator_balance_post_withdraw_1 = Balances::free_balance(&nominator); + // free balance increases by unlock chunk 1 value. + assert_eq!(nominator_balance_post_withdraw_1, nominator_balance_pre_withdraw + 100); + + // lets roll one more block. + Session::roll_next(); + // there are still offences for era 3. + assert_eq!(era_offence_count(3), 3); + // withdrawal is still blocked. + assert_ok!(Staking::withdraw_unbonded(RuntimeOrigin::signed(nominator), 0)); + assert_eq!(Balances::free_balance(&nominator), nominator_balance_post_withdraw_1); + + // WHEN: all offences are processed. + while era_offence_count(3) > 0 { + Session::roll_next(); + } + + assert_eq!(era_offence_count(3), 0); + assert_eq!(OffenceQueueEras::::get(), None); + + // now withdrawing for era 3 should be unblocked. + assert_ok!(Staking::withdraw_unbonded(RuntimeOrigin::signed(nominator), 0)); + assert_eq!(Balances::free_balance(&nominator), nominator_balance_post_withdraw_1 + 150); + }); } mod paged_slashing { From f69886cc07b9ad353f0b1887bb8e90b1a9847157 Mon Sep 17 00:00:00 2001 From: Ankan Date: Thu, 3 Jul 2025 13:10:48 +0200 Subject: [PATCH 06/25] minor fixes --- substrate/frame/staking-async/src/pallet/impls.rs | 1 + substrate/frame/staking-async/src/tests/slashing.rs | 4 ++++ 2 files changed, 5 insertions(+) diff --git a/substrate/frame/staking-async/src/pallet/impls.rs b/substrate/frame/staking-async/src/pallet/impls.rs index ef932ce7fb4c8..d9e1be8ce0e14 100644 --- a/substrate/frame/staking-async/src/pallet/impls.rs +++ b/substrate/frame/staking-async/src/pallet/impls.rs @@ -219,6 +219,7 @@ impl Pallet { .as_ref() .and_then(|eras| eras.first()) .copied() + // if nothing in queue, use the active era. .unwrap_or(active_era) // above returns earliest era for which offences are processed, so we subtract one to // get the last era for which all offences are processed. diff --git a/substrate/frame/staking-async/src/tests/slashing.rs b/substrate/frame/staking-async/src/tests/slashing.rs index d970c428c4007..2f980189ff02d 100644 --- a/substrate/frame/staking-async/src/tests/slashing.rs +++ b/substrate/frame/staking-async/src/tests/slashing.rs @@ -1411,12 +1411,16 @@ fn withdrawals_are_blocked_for_unprocessed_offence_eras() { Session::roll_next(); } + // ensure all offences are processed. assert_eq!(era_offence_count(3), 0); assert_eq!(OffenceQueueEras::::get(), None); // now withdrawing for era 3 should be unblocked. assert_ok!(Staking::withdraw_unbonded(RuntimeOrigin::signed(nominator), 0)); assert_eq!(Balances::free_balance(&nominator), nominator_balance_post_withdraw_1 + 150); + + // final check we are all this time still in Era 6. + assert_eq!(active_era(), 6); }); } From f9b3af9bb0dcedb596d5fabe8dcb2b01b38acad7 Mon Sep 17 00:00:00 2001 From: Ankan Date: Thu, 3 Jul 2025 13:15:53 +0200 Subject: [PATCH 07/25] make offence queue eras to be weak bounded vec as technically this can grow above bonding duration size, though highly unlikely --- substrate/frame/staking-async/src/pallet/impls.rs | 2 +- substrate/frame/staking-async/src/pallet/mod.rs | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/substrate/frame/staking-async/src/pallet/impls.rs b/substrate/frame/staking-async/src/pallet/impls.rs index d9e1be8ce0e14..ddf49df7fcf3b 100644 --- a/substrate/frame/staking-async/src/pallet/impls.rs +++ b/substrate/frame/staking-async/src/pallet/impls.rs @@ -1261,7 +1261,7 @@ impl rc_client::AHStakingInterface for Pallet { ) }); } else { - let mut eras = BoundedVec::default(); + let mut eras = WeakBoundedVec::default(); log!(debug, "🦹 inserting offence era {} into empty queue", offence_era); let _ = eras .try_push(offence_era) diff --git a/substrate/frame/staking-async/src/pallet/mod.rs b/substrate/frame/staking-async/src/pallet/mod.rs index 7c147fb67d11d..98828738878ce 100644 --- a/substrate/frame/staking-async/src/pallet/mod.rs +++ b/substrate/frame/staking-async/src/pallet/mod.rs @@ -734,7 +734,7 @@ pub mod pallet { /// This eliminates the need for expensive iteration and sorting when fetching the next offence /// to process. #[pallet::storage] - pub type OffenceQueueEras = StorageValue<_, BoundedVec>; + pub type OffenceQueueEras = StorageValue<_, WeakBoundedVec>; /// Tracks the currently processed offence record from the `OffenceQueue`. /// From c65d61c827f3a1e4f097a9f62dd74c28c84ca493 Mon Sep 17 00:00:00 2001 From: Ankan Date: Thu, 3 Jul 2025 14:57:44 +0200 Subject: [PATCH 08/25] try state checks to ensure slash health --- .../frame/staking-async/src/pallet/impls.rs | 60 ++++++++++++++++++- 1 file changed, 59 insertions(+), 1 deletion(-) diff --git a/substrate/frame/staking-async/src/pallet/impls.rs b/substrate/frame/staking-async/src/pallet/impls.rs index ddf49df7fcf3b..0b588857f4c5b 100644 --- a/substrate/frame/staking-async/src/pallet/impls.rs +++ b/substrate/frame/staking-async/src/pallet/impls.rs @@ -36,7 +36,7 @@ use frame_election_provider_support::{ use frame_support::{ defensive, dispatch::WithPostDispatchInfo, - pallet_prelude::*, + pallet_prelude::*, StorageDoubleMap, traits::{ Defensive, DefensiveSaturating, Get, Imbalance, InspectLockableCurrency, LockableCurrency, OnUnbalanced, @@ -1719,6 +1719,7 @@ impl Pallet { Self::check_payees()?; Self::check_paged_exposures()?; Self::check_count()?; + Self::check_slash_health()?; Ok(()) } @@ -1952,6 +1953,63 @@ impl Pallet { .collect::>() } + /// Ensures offence pipeline and slashing is in a healthy state. + fn check_slash_health() -> Result<(), TryRuntimeError> { + // (1) Ensure offence queue is sorted + let offence_queue_eras = OffenceQueueEras::::get().unwrap_or_default().into_inner(); + let mut sorted_offence_queue_eras = offence_queue_eras.clone(); + sorted_offence_queue_eras.sort(); + ensure!(sorted_offence_queue_eras == offence_queue_eras, "Offence queue eras are not sorted"); + + // (2) Ensure oldest offence queue era is old enough. + let active_era = Rotator::::active_era(); + let oldest_unprocessed_offence_era = offence_queue_eras.first().cloned().unwrap_or(active_era); + + // how old is the oldest unprocessed offence era? + // given bonding duration = 28, the ideal value is between 0 and 2 eras. + // anything close to bonding duration is terrible. + let oldest_unprocessed_offence_age = + active_era.saturating_sub(oldest_unprocessed_offence_era); + + // warn if less than 26 eras old. + if oldest_unprocessed_offence_age > 2.min(T::BondingDuration::get()) { + log!( + warn, + "Offence queue has unprocessed offences from older than 2 eras: oldest offence era in queue {:?} (active era: {:?})", + oldest_unprocessed_offence_era, + active_era + ); + } + + // error if the oldest unprocessed offence era closer to bonding duration. + ensure!( + oldest_unprocessed_offence_age < T::BondingDuration::get() - 1, + "offences from era less than 3 eras old from active era not processed yet" + ); + + // (3) Report count of offences in the queue. + for e in offence_queue_eras { + let count = OffenceQueue::::iter_prefix(e).count(); + ensure!(count > 0, "Offence queue is empty for era listed in offence queue eras"); + log!( + warn, + "Offence queue for era {:?} has {:?} offences queued", + e, + count + ); + } + + // (4) Ensure all slashes older than (active era - 1) are applied. + // We will look at all eras before the active era as it can take 1 era for slashes + // to be applied. + for era in (active_era.saturating_sub(T::BondingDuration::get()))..(active_era) { + // all unapplied slashes are expected to be applied in 1 era. + ensure!(!UnappliedSlashes::::contains_prefix(era), "Unapplied slashes for recently passed era found"); + } + + Ok(()) + } + fn ensure_ledger_role_and_min_bond(ctrl: &T::AccountId) -> Result<(), TryRuntimeError> { let ledger = Self::ledger(StakingAccount::Controller(ctrl.clone()))?; let stash = ledger.stash; From ed04173f115ad4490688cb1f62dc44424991d879 Mon Sep 17 00:00:00 2001 From: Ankan Date: Thu, 3 Jul 2025 15:15:28 +0200 Subject: [PATCH 09/25] drop offences older than slashdeferduration - 1 old. 4 test failing --- .../frame/staking-async/src/pallet/impls.rs | 47 +++++++++++++++---- .../frame/staking-async/src/pallet/mod.rs | 6 +++ 2 files changed, 43 insertions(+), 10 deletions(-) diff --git a/substrate/frame/staking-async/src/pallet/impls.rs b/substrate/frame/staking-async/src/pallet/impls.rs index 0b588857f4c5b..e4d8e35a73d4b 100644 --- a/substrate/frame/staking-async/src/pallet/impls.rs +++ b/substrate/frame/staking-async/src/pallet/impls.rs @@ -36,12 +36,13 @@ use frame_election_provider_support::{ use frame_support::{ defensive, dispatch::WithPostDispatchInfo, - pallet_prelude::*, StorageDoubleMap, + pallet_prelude::*, traits::{ Defensive, DefensiveSaturating, Get, Imbalance, InspectLockableCurrency, LockableCurrency, OnUnbalanced, }, weights::Weight, + StorageDoubleMap, }; use frame_system::{pallet_prelude::BlockNumberFor, RawOrigin}; use pallet_staking_async_rc_client::{self as rc_client}; @@ -1126,6 +1127,16 @@ impl rc_client::AHStakingInterface for Pallet { Self::register_weight(consumed_weight); } + /// Accepts offences only if they are from era `active_era - (SlashDeferDuration - 1)` or newer. + /// + /// Slashes for offences are applied `SlashDeferDuration` eras after the offence occurred. + /// Accepting offences older than this range would not leave enough time for slashes to be + /// applied. + /// + /// Note: The validator set report that we send to the relay chain contains the pruning + /// information for a relay chain, but we conservatively keep some extra sessions, so it is + /// possible that an offence report is created for a session between SlashDeferDuration and + /// BondingDuration eras before the active era. But they will be dropped here. fn on_new_offences( slash_session: SessionIndex, offences: Vec>, @@ -1162,6 +1173,9 @@ impl rc_client::AHStakingInterface for Pallet { } }; + let oldest_reportable_offence_era = + active_era.index.saturating_sub(T::SlashDeferDuration::get().saturating_sub(1)); + let invulnerables = Invulnerables::::get(); for o in offences { @@ -1173,6 +1187,17 @@ impl rc_client::AHStakingInterface for Pallet { continue } + // ignore offence if too old to report. + if offence_era < oldest_reportable_offence_era { + log!(warn, "🦹 on_new_offences: offence era {:?} too old; Can only accept offences from era {:?} or newer", offence_era, oldest_reportable_offence_era); + Self::deposit_event(Event::::OffenceIgnored { + validator: validator.clone(), + fraction: slash_fraction, + offence_era, + }); + // will emit an event for each validator in the report. + continue; + } let Some(exposure_overview) = >::get(&offence_era, &validator) else { // defensive: this implies offence is for a discarded era, and should already be @@ -1959,11 +1984,15 @@ impl Pallet { let offence_queue_eras = OffenceQueueEras::::get().unwrap_or_default().into_inner(); let mut sorted_offence_queue_eras = offence_queue_eras.clone(); sorted_offence_queue_eras.sort(); - ensure!(sorted_offence_queue_eras == offence_queue_eras, "Offence queue eras are not sorted"); + ensure!( + sorted_offence_queue_eras == offence_queue_eras, + "Offence queue eras are not sorted" + ); // (2) Ensure oldest offence queue era is old enough. let active_era = Rotator::::active_era(); - let oldest_unprocessed_offence_era = offence_queue_eras.first().cloned().unwrap_or(active_era); + let oldest_unprocessed_offence_era = + offence_queue_eras.first().cloned().unwrap_or(active_era); // how old is the oldest unprocessed offence era? // given bonding duration = 28, the ideal value is between 0 and 2 eras. @@ -1991,12 +2020,7 @@ impl Pallet { for e in offence_queue_eras { let count = OffenceQueue::::iter_prefix(e).count(); ensure!(count > 0, "Offence queue is empty for era listed in offence queue eras"); - log!( - warn, - "Offence queue for era {:?} has {:?} offences queued", - e, - count - ); + log!(warn, "Offence queue for era {:?} has {:?} offences queued", e, count); } // (4) Ensure all slashes older than (active era - 1) are applied. @@ -2004,7 +2028,10 @@ impl Pallet { // to be applied. for era in (active_era.saturating_sub(T::BondingDuration::get()))..(active_era) { // all unapplied slashes are expected to be applied in 1 era. - ensure!(!UnappliedSlashes::::contains_prefix(era), "Unapplied slashes for recently passed era found"); + ensure!( + !UnappliedSlashes::::contains_prefix(era), + "Unapplied slashes for recently passed era found" + ); } Ok(()) diff --git a/substrate/frame/staking-async/src/pallet/mod.rs b/substrate/frame/staking-async/src/pallet/mod.rs index 98828738878ce..170c24b175ec5 100644 --- a/substrate/frame/staking-async/src/pallet/mod.rs +++ b/substrate/frame/staking-async/src/pallet/mod.rs @@ -1156,6 +1156,12 @@ pub mod pallet { /// Something occurred that should never happen under normal operation. /// Logged as an event for fail-safe observability. Unexpected(UnexpectedKind), + /// An offence was reported that was too old to be processed, and thus was dropped. + OffenceIgnored { + offence_era: EraIndex, + validator: T::AccountId, + fraction: Perbill, + }, } /// Represents unexpected or invariant-breaking conditions encountered during execution. From 5520308ad0b14c694b2d17d52d7d9bd9b15bae82 Mon Sep 17 00:00:00 2001 From: Ankan Date: Thu, 3 Jul 2025 15:44:14 +0200 Subject: [PATCH 10/25] test for discarding old offences --- .../frame/staking-async/src/tests/slashing.rs | 85 +++++++++++++++++++ 1 file changed, 85 insertions(+) diff --git a/substrate/frame/staking-async/src/tests/slashing.rs b/substrate/frame/staking-async/src/tests/slashing.rs index 2f980189ff02d..6eae65d24ee56 100644 --- a/substrate/frame/staking-async/src/tests/slashing.rs +++ b/substrate/frame/staking-async/src/tests/slashing.rs @@ -627,6 +627,91 @@ fn only_slash_validator_for_max_in_era() { }) } +#[test] +fn really_old_offences_are_ignored() { + ExtBuilder::default() + .slash_defer_duration(27) + .bonding_duration(28) + .build_and_execute(|| { + Session::roll_until_active_era(100); + + let expected_oldest_reportable_offence = + active_era() - (SlashDeferDuration::get() - 1); + + assert_eq!(expected_oldest_reportable_offence, 74); + + // clear staking events until now + staking_events_since_last_call(); + + // WHEN: reporting offence for era 72 and 73, which are too old. + add_slash_in_era(11, 72, Perbill::from_percent(10)); + add_slash_in_era(21, 73, Perbill::from_percent(10)); + + // THEN: offence is ignored. + assert_eq!( + staking_events_since_last_call(), + vec![ + Event::OffenceIgnored { + offence_era: 72, + validator: 11, + fraction: Perbill::from_percent(10) + }, + Event::OffenceIgnored { + offence_era: 73, + validator: 21, + fraction: Perbill::from_percent(10) + }, + ] + ); + + // WHEN: reporting offence for era 74. + add_slash_in_era(11, 74, Perbill::from_percent(10)); + + // THEN: offence is reported. + assert_eq!( + staking_events_since_last_call(), + vec![ + Event::OffenceReported { + offence_era: 74, + validator: 11, + fraction: Perbill::from_percent(10) + } + ] + ); + + // AND: computed in the next block. + Session::roll_next(); + + assert_eq!( + staking_events_since_last_call(), + vec![ + Event::SlashComputed { + offence_era: 74, + slash_era: 101, + offender: 11, + page: 0 + }, + ] + ); + + // Slash is applied at the start of the next era. + Session::roll_until_active_era(101); + // clear staking events until now + staking_events_since_last_call(); + + // this should apply the slash. + Session::roll_next(); + assert_eq!( + staking_events_since_last_call(), + vec![ + Event::Slashed { staker: 11, amount: 100 }, + // Nominator 101 is exposed to 11, so they are slashed too. + Event::Slashed { staker: 101, amount: 25 } + ] + ); + }); +} + #[test] fn nominator_is_slashed_by_max_for_validator_in_era() { ExtBuilder::default().build_and_execute(|| { From 899d8f91a21eb2e27e6fa52681de9191e3e50b5f Mon Sep 17 00:00:00 2001 From: Ankan Date: Thu, 3 Jul 2025 15:44:33 +0200 Subject: [PATCH 11/25] fmt --- .../frame/staking-async/src/tests/slashing.rs | 31 ++++++++----------- 1 file changed, 13 insertions(+), 18 deletions(-) diff --git a/substrate/frame/staking-async/src/tests/slashing.rs b/substrate/frame/staking-async/src/tests/slashing.rs index 6eae65d24ee56..18f4565cc7cc9 100644 --- a/substrate/frame/staking-async/src/tests/slashing.rs +++ b/substrate/frame/staking-async/src/tests/slashing.rs @@ -635,8 +635,7 @@ fn really_old_offences_are_ignored() { .build_and_execute(|| { Session::roll_until_active_era(100); - let expected_oldest_reportable_offence = - active_era() - (SlashDeferDuration::get() - 1); + let expected_oldest_reportable_offence = active_era() - (SlashDeferDuration::get() - 1); assert_eq!(expected_oldest_reportable_offence, 74); @@ -670,13 +669,11 @@ fn really_old_offences_are_ignored() { // THEN: offence is reported. assert_eq!( staking_events_since_last_call(), - vec![ - Event::OffenceReported { - offence_era: 74, - validator: 11, - fraction: Perbill::from_percent(10) - } - ] + vec![Event::OffenceReported { + offence_era: 74, + validator: 11, + fraction: Perbill::from_percent(10) + }] ); // AND: computed in the next block. @@ -684,14 +681,12 @@ fn really_old_offences_are_ignored() { assert_eq!( staking_events_since_last_call(), - vec![ - Event::SlashComputed { - offence_era: 74, - slash_era: 101, - offender: 11, - page: 0 - }, - ] + vec![Event::SlashComputed { + offence_era: 74, + slash_era: 101, + offender: 11, + page: 0 + },] ); // Slash is applied at the start of the next era. @@ -709,7 +704,7 @@ fn really_old_offences_are_ignored() { Event::Slashed { staker: 101, amount: 25 } ] ); - }); + }); } #[test] From 0bb1281b9e11d92022ffa4e89eba8c35d92aa667 Mon Sep 17 00:00:00 2001 From: Ankan Date: Thu, 3 Jul 2025 15:54:28 +0200 Subject: [PATCH 12/25] only ignore offence if slash deferral set --- substrate/frame/staking-async/src/pallet/impls.rs | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/substrate/frame/staking-async/src/pallet/impls.rs b/substrate/frame/staking-async/src/pallet/impls.rs index e4d8e35a73d4b..ce3290fc0bbcc 100644 --- a/substrate/frame/staking-async/src/pallet/impls.rs +++ b/substrate/frame/staking-async/src/pallet/impls.rs @@ -1173,8 +1173,15 @@ impl rc_client::AHStakingInterface for Pallet { } }; - let oldest_reportable_offence_era = - active_era.index.saturating_sub(T::SlashDeferDuration::get().saturating_sub(1)); + let oldest_reportable_offence_era = if T::SlashDeferDuration::get() == 0 { + // this implies that slashes are applied immediately, so we can accept any offence up to + // bonding duration old. + active_era.index.saturating_sub(T::BondingDuration::get()) + } else { + // slashes are deffered, so we only accept offences that are not older than the + // defferal duration. + active_era.index.saturating_sub(T::SlashDeferDuration::get().saturating_sub(1)) + }; let invulnerables = Invulnerables::::get(); From 1ff876afe8aab8185a66b85bad98d767791ac1d4 Mon Sep 17 00:00:00 2001 From: Ankan Date: Thu, 3 Jul 2025 16:44:03 +0200 Subject: [PATCH 13/25] almost there, need to block if unapplied slashes from last era. --- .../frame/staking-async/src/pallet/impls.rs | 2 +- .../frame/staking-async/src/pallet/mod.rs | 8 +-- .../frame/staking-async/src/tests/slashing.rs | 70 +++++++++---------- 3 files changed, 40 insertions(+), 40 deletions(-) diff --git a/substrate/frame/staking-async/src/pallet/impls.rs b/substrate/frame/staking-async/src/pallet/impls.rs index ce3290fc0bbcc..3ebc7d9a970d6 100644 --- a/substrate/frame/staking-async/src/pallet/impls.rs +++ b/substrate/frame/staking-async/src/pallet/impls.rs @@ -42,7 +42,6 @@ use frame_support::{ OnUnbalanced, }, weights::Weight, - StorageDoubleMap, }; use frame_system::{pallet_prelude::BlockNumberFor, RawOrigin}; use pallet_staking_async_rc_client::{self as rc_client}; @@ -2033,6 +2032,7 @@ impl Pallet { // (4) Ensure all slashes older than (active era - 1) are applied. // We will look at all eras before the active era as it can take 1 era for slashes // to be applied. + use frame_support::StorageDoubleMap; for era in (active_era.saturating_sub(T::BondingDuration::get()))..(active_era) { // all unapplied slashes are expected to be applied in 1 era. ensure!( diff --git a/substrate/frame/staking-async/src/pallet/mod.rs b/substrate/frame/staking-async/src/pallet/mod.rs index 170c24b175ec5..579f3cfd489eb 100644 --- a/substrate/frame/staking-async/src/pallet/mod.rs +++ b/substrate/frame/staking-async/src/pallet/mod.rs @@ -1463,10 +1463,10 @@ pub mod pallet { // If a user runs into this error, they should chill first. ensure!(ledger.active >= min_active_bond, Error::::InsufficientBond); - // Note: in case there is no current era it is fine to bond one era more. - let era = CurrentEra::::get() - .unwrap_or(0) - .defensive_saturating_add(T::BondingDuration::get()); + // Note: we used current era before, but that is meant to be used for only election. + // The right value to use here is the active era. + + let era = session_rotation::Rotator::::active_era().saturating_add(T::BondingDuration::get()); if let Some(chunk) = ledger.unlocking.last_mut().filter(|chunk| chunk.era == era) { // To keep the chunk count down, we only keep one chunk per era. Since // `unlocking` is a FiFo queue, if a chunk exists for `era` we know that it will diff --git a/substrate/frame/staking-async/src/tests/slashing.rs b/substrate/frame/staking-async/src/tests/slashing.rs index 18f4565cc7cc9..0a89cbf128cb8 100644 --- a/substrate/frame/staking-async/src/tests/slashing.rs +++ b/substrate/frame/staking-async/src/tests/slashing.rs @@ -1409,13 +1409,14 @@ fn withdrawals_are_blocked_for_unprocessed_offence_eras() { .add_staker(71, 1000, StakerStatus::Validator) .add_staker(81, 1000, StakerStatus::Validator) .add_staker(91, 1000, StakerStatus::Validator) + // we want to replicate a scenario where all offences could not be processed in 1 era, so we + // reduce the era length to 1 block. + .session_per_era(1) + .period(1) .validator_count(6) .build_and_execute(|| { - let bonding_duration = BondingDuration::get(); - assert_eq!(bonding_duration, 3); - // we have 15 blocks per era. - let blocks_per_era = Period::get() * SessionsPerEra::get() as u64; - assert_eq!(blocks_per_era, 15); + // NOTE for curious reader: Era change still takes 2 blocks... don't ask why ¯\_(ツ)_/¯ + let _expected_era_length = 2; // Set up nominator. let validator = 11; @@ -1428,51 +1429,55 @@ fn withdrawals_are_blocked_for_unprocessed_offence_eras() { Session::roll_until_active_era(3); assert_ok!(Staking::unbond(RuntimeOrigin::signed(nominator), 150)); - Session::roll_until_active_era(5); - // Lets go to one block before start of era 6. - for _i in 1..blocks_per_era { - Session::roll_next(); - } + // Rationale: We want to simulate a backlog of offences from era 3 that remain + // unprocessed by the time unbonding becomes possible in era 6. + // + // Offences for era 3 must be reported no later than era 4, since slashing application + // starts in era 5. To achieve this, we flood era 3 with more than 4 offences, all + // reported just before the end of era 4. Given there are only 2 blocks per era + // (limiting processing throughput), this ensures not all offences will be processed by + // era 6 — blocking withdrawal as intended. + + // go to era 4. + Session::roll_until_active_era(4); + + // roll one block of 2 of era 4. + Session::roll_next(); - // one block before era 6 starts, lets spam the offence pipeline. + // flood offence pipeline with offences for era 3. + // Note: our validator 11 is not slashed. add_slash_in_era(21, 3, Perbill::from_percent(10)); add_slash_in_era(61, 3, Perbill::from_percent(10)); add_slash_in_era(71, 3, Perbill::from_percent(10)); add_slash_in_era(81, 3, Perbill::from_percent(10)); add_slash_in_era(91, 3, Perbill::from_percent(10)); - // lets roll to era 6. - Session::roll_next(); + // lets roll to era 6 where all unbonding chunks are available to withdraw. + Session::roll_until_active_era(6); assert_eq!(active_era(), 6); - // and we created 5 offences, of which 1 would be processed since we rolled one block. - assert_eq!(era_offence_count(3), 5 - 1); - assert_eq!(OffenceQueueEras::::get().unwrap(), vec![3]); - - // GIVEN Scenario: Our nominator has unbonding chunks scheduled for eras 5 and 6. - // The current active era is 6, so both chunks would normally be eligible for - // withdrawal. However, offences from era 3 (which can affect withdrawals in era 6) is - // unprocessed.Therefore, withdrawal from era 6 should be blocked until those offences - // are handled. - - // Unbonding chunks + // Ensure unbonding chunks can all be withdrawn by era 6. let expected_chunks: BoundedVec, MaxUnlockingChunks> = bounded_vec![ // era is unbond_era + bonding_duration, starting from era 2 + 3. UnlockChunk { era: 5, value: 100 }, UnlockChunk { era: 6, value: 150 }, ]; - assert_eq!(Ledger::::get(nominator).unwrap().unlocking, expected_chunks); + // and we created 5 offences, of which 3 would be processed in last block of era 4, and + // 2 blocks of era 5. + assert_eq!(era_offence_count(3), 5 - 3); + assert_eq!(OffenceQueueEras::::get().unwrap(), vec![3]); + // all nominator balance other than ED is staked. let nominator_balance_pre_withdraw = Balances::free_balance(&nominator); assert_eq!(nominator_balance_pre_withdraw, 1); - // WHEN the nominator tries to withdraw unbonded funds. + // WHEN: the nominator tries to withdraw unbonded funds. assert_ok!(Staking::withdraw_unbonded(RuntimeOrigin::signed(nominator), 0)); - // THEN only the first unbonding chunk is withdrawn, as the second one is blocked by + // THEN: only the first unbonding chunk is withdrawn, as the second one is blocked by // unprocessed offences. let nominator_balance_post_withdraw_1 = Balances::free_balance(&nominator); // free balance increases by unlock chunk 1 value. @@ -1480,16 +1485,14 @@ fn withdrawals_are_blocked_for_unprocessed_offence_eras() { // lets roll one more block. Session::roll_next(); - // there are still offences for era 3. - assert_eq!(era_offence_count(3), 3); + // there are still one offence left for era 3. + assert_eq!(era_offence_count(3), 1); // withdrawal is still blocked. assert_ok!(Staking::withdraw_unbonded(RuntimeOrigin::signed(nominator), 0)); assert_eq!(Balances::free_balance(&nominator), nominator_balance_post_withdraw_1); // WHEN: all offences are processed. - while era_offence_count(3) > 0 { - Session::roll_next(); - } + Session::roll_next(); // ensure all offences are processed. assert_eq!(era_offence_count(3), 0); @@ -1498,9 +1501,6 @@ fn withdrawals_are_blocked_for_unprocessed_offence_eras() { // now withdrawing for era 3 should be unblocked. assert_ok!(Staking::withdraw_unbonded(RuntimeOrigin::signed(nominator), 0)); assert_eq!(Balances::free_balance(&nominator), nominator_balance_post_withdraw_1 + 150); - - // final check we are all this time still in Era 6. - assert_eq!(active_era(), 6); }); } From fe438979b6bebecc7053c8037f69547536cebaaf Mon Sep 17 00:00:00 2001 From: Ankan Date: Thu, 3 Jul 2025 17:44:13 +0200 Subject: [PATCH 14/25] finally everything works (fingers crossed); I am dead --- substrate/frame/staking-async/src/mock.rs | 21 +++++++- .../frame/staking-async/src/pallet/impls.rs | 26 +++++++--- .../frame/staking-async/src/pallet/mod.rs | 22 ++++++-- .../frame/staking-async/src/tests/slashing.rs | 51 +++++++++++++++---- 4 files changed, 99 insertions(+), 21 deletions(-) diff --git a/substrate/frame/staking-async/src/mock.rs b/substrate/frame/staking-async/src/mock.rs index f2dd344b8dc4c..88ad4d3e290f7 100644 --- a/substrate/frame/staking-async/src/mock.rs +++ b/substrate/frame/staking-async/src/mock.rs @@ -1005,6 +1005,25 @@ pub(crate) fn remove_from_restrict_list(who: &AccountId) { RestrictedAccounts::mutate(|l| l.retain(|x| x != who)); } -pub(crate) fn era_offence_count(era: EraIndex) -> u32 { +pub(crate) fn era_unprocessed_offence_count(era: EraIndex) -> u32 { OffenceQueue::::iter_prefix_values(era).count() as u32 } + +pub(crate) fn era_unapplied_slash_count(era: EraIndex) -> u32 { + UnappliedSlashes::::iter_prefix_values(era).count() as u32 +} + +/// A pending slash from the previous era blocks withdrawal. Use this to apply them. +pub(crate) fn apply_pending_slashes_from_previous_era() { + apply_pending_slashes_from_era(active_era() - 1); +} + +pub(crate) fn apply_pending_slashes_from_era(era: EraIndex) { + for (key, _) in UnappliedSlashes::::iter_prefix(era) { + assert_ok!(Staking::apply_slash( + RuntimeOrigin::signed(1), + era, + key + )); + } +} \ No newline at end of file diff --git a/substrate/frame/staking-async/src/pallet/impls.rs b/substrate/frame/staking-async/src/pallet/impls.rs index 3ebc7d9a970d6..1bf9289a1e5aa 100644 --- a/substrate/frame/staking-async/src/pallet/impls.rs +++ b/substrate/frame/staking-async/src/pallet/impls.rs @@ -37,6 +37,7 @@ use frame_support::{ defensive, dispatch::WithPostDispatchInfo, pallet_prelude::*, + StorageDoubleMap, traits::{ Defensive, DefensiveSaturating, Get, Imbalance, InspectLockableCurrency, LockableCurrency, OnUnbalanced, @@ -214,6 +215,11 @@ impl Pallet { let (stash, old_total) = (ledger.stash.clone(), ledger.total); let active_era = Rotator::::active_era(); + // Ensure last era slashes are applied. Else we block the withdrawals. + if active_era > 1 { + Self::ensure_era_slashes_applied(active_era.saturating_sub(1))?; + } + // get lowest era for which all offences are processed and withdrawals can be allowed. let earliest_unlock_era_by_offence_queue = OffenceQueueEras::::get() .as_ref() @@ -285,6 +291,16 @@ impl Pallet { Ok(used_weight) } + fn ensure_era_slashes_applied( + era: EraIndex, + ) -> Result<(), DispatchError> { + ensure!( + !UnappliedSlashes::::contains_prefix(era), + Error::::UnappliedSlashesInPreviousEra + ); + Ok(()) + } + pub(super) fn do_payout_stakers( validator_stash: T::AccountId, era: EraIndex, @@ -2032,13 +2048,11 @@ impl Pallet { // (4) Ensure all slashes older than (active era - 1) are applied. // We will look at all eras before the active era as it can take 1 era for slashes // to be applied. - use frame_support::StorageDoubleMap; for era in (active_era.saturating_sub(T::BondingDuration::get()))..(active_era) { - // all unapplied slashes are expected to be applied in 1 era. - ensure!( - !UnappliedSlashes::::contains_prefix(era), - "Unapplied slashes for recently passed era found" - ); + // all unapplied slashes are expected to be applied until the active era. If this is not + // the case, then we need to use a permissionless call to apply all of them. + // See `Call::apply_slash` for more details. + Self::ensure_era_slashes_applied(era)?; } Ok(()) diff --git a/substrate/frame/staking-async/src/pallet/mod.rs b/substrate/frame/staking-async/src/pallet/mod.rs index 579f3cfd489eb..b8f662a365988 100644 --- a/substrate/frame/staking-async/src/pallet/mod.rs +++ b/substrate/frame/staking-async/src/pallet/mod.rs @@ -1251,6 +1251,9 @@ pub mod pallet { /// Account is restricted from participation in staking. This may happen if the account is /// staking in another way already, such as via pool. Restricted, + /// Unapplied slashes in the recently concluded era is blocking this operation. + /// See `Call::apply_slash` to apply them. + UnappliedSlashesInPreviousEra, } impl Pallet { @@ -2450,11 +2453,21 @@ pub mod pallet { Ok(Pays::No.into()) } - /// Manually applies a deferred slash for a given era. + /// Manually and permissionlessly applies a deferred slash for a given era. /// /// Normally, slashes are automatically applied shortly after the start of the `slash_era`. - /// This function exists as a **fallback mechanism** in case slashes were not applied due to - /// unexpected reasons. It allows anyone to manually apply an unapplied slash. + /// The automatic application of slashes is handled by the pallet's internal logic, and it + /// tries to apply one slash page per block of the era. + /// If for some reason, one era is not enough for applying all slash pages, the remaining + /// slashes need to be manually (permissionlessly) applied. + /// + /// For a given era x, if at era x+1, slashes are still unapplied, all withdrawals get + /// blocked, and these need to be manually applied by calling this function. + /// This function exists as a **fallback mechanism** for this extreme situation, but we + /// never expect to encounter this in normal scenarios. + /// + /// The parameters for this call can be queried by looking at the `UnappliedSlashes` storage + /// for eras older than the active era. /// /// ## Parameters /// - `slash_era`: The staking era in which the slash was originally scheduled. @@ -2465,7 +2478,8 @@ pub mod pallet { /// /// ## Behavior /// - The function is **permissionless**—anyone can call it. - /// - The `slash_era` **must be the current era or a past era**. If it is in the future, the + /// - The `slash_era` **must be the current era or a past era**. + /// If it is in the future, the /// call fails with `EraNotStarted`. /// - The fee is waived if the slash is successfully applied. /// diff --git a/substrate/frame/staking-async/src/tests/slashing.rs b/substrate/frame/staking-async/src/tests/slashing.rs index 0a89cbf128cb8..0643603c31a5b 100644 --- a/substrate/frame/staking-async/src/tests/slashing.rs +++ b/substrate/frame/staking-async/src/tests/slashing.rs @@ -1401,7 +1401,7 @@ fn proportional_ledger_slash_works() { } #[test] -fn withdrawals_are_blocked_for_unprocessed_offence_eras() { +fn withdrawals_are_blocked_for_unprocessed_and_unapplied_slashes() { ExtBuilder::default() .slash_defer_duration(2) .bonding_duration(3) @@ -1467,13 +1467,27 @@ fn withdrawals_are_blocked_for_unprocessed_offence_eras() { // and we created 5 offences, of which 3 would be processed in last block of era 4, and // 2 blocks of era 5. - assert_eq!(era_offence_count(3), 5 - 3); + assert_eq!(era_unprocessed_offence_count(3), 5 - 3); assert_eq!(OffenceQueueEras::::get().unwrap(), vec![3]); // all nominator balance other than ED is staked. let nominator_balance_pre_withdraw = Balances::free_balance(&nominator); assert_eq!(nominator_balance_pre_withdraw, 1); + // Since the eras are too short, the offences that needed to be applied for last era 5 + // are still unapplied. This will block the withdrawal. + assert_eq!(era_unapplied_slash_count(5), 1); + + // WHEN: the nominator tries to withdraw unbonded funds while there are unapplied + // offence in the last era. + assert_noop!(Staking::withdraw_unbonded(RuntimeOrigin::signed(nominator), 0), + Error::::UnappliedSlashesInPreviousEra); + + // let's clear the slashes by manually applying them. + apply_pending_slashes_from_previous_era(); + // ensure unapplied slashes are cleared. + assert_eq!(era_unapplied_slash_count(5), 0); + // WHEN: the nominator tries to withdraw unbonded funds. assert_ok!(Staking::withdraw_unbonded(RuntimeOrigin::signed(nominator), 0)); @@ -1483,24 +1497,41 @@ fn withdrawals_are_blocked_for_unprocessed_offence_eras() { // free balance increases by unlock chunk 1 value. assert_eq!(nominator_balance_post_withdraw_1, nominator_balance_pre_withdraw + 100); - // lets roll one more block. + // rolling a block creates another unapplied slash for era 3 as well as process a + // remaining offence. Session::roll_next(); - // there are still one offence left for era 3. - assert_eq!(era_offence_count(3), 1); - // withdrawal is still blocked. + assert_eq!(era_unapplied_slash_count(5), 1); + // clear the pending slashes. + apply_pending_slashes_from_previous_era(); + + // there is still one offence unprocessed for era 3. + assert_eq!(era_unprocessed_offence_count(3), 1); + + // withdrawals are still not possible for era (3 + 3 =) 6. assert_ok!(Staking::withdraw_unbonded(RuntimeOrigin::signed(nominator), 0)); assert_eq!(Balances::free_balance(&nominator), nominator_balance_post_withdraw_1); // WHEN: all offences are processed. Session::roll_next(); - - // ensure all offences are processed. - assert_eq!(era_offence_count(3), 0); + // Note that active_era has bumped to 7. + assert_eq!(active_era(), 7); + // The previous block created another unapplied slash for era 5, but we only block + // withdrawals upto 1 block (to give enough time for offchain actors to apply slashes + // manually). So, we dont need to apply pending slashes for era 5. + assert_eq!(era_unapplied_slash_count(5), 1); + // But era 6 (last era) has no unapplied slashes. + assert_eq!(era_unapplied_slash_count(6), 0); + // We also ensure all offences in the queue for era 3 are now processed. + assert_eq!(era_unprocessed_offence_count(3), 0); assert_eq!(OffenceQueueEras::::get(), None); - // now withdrawing for era 3 should be unblocked. + // Withdrawing for era 3 should be possible. assert_ok!(Staking::withdraw_unbonded(RuntimeOrigin::signed(nominator), 0)); assert_eq!(Balances::free_balance(&nominator), nominator_balance_post_withdraw_1 + 150); + + // Finally, we clear the unapplied slashes for era 5. Otherwise our try state checks + // will fail. (Try by commenting the next line :)) + apply_pending_slashes_from_era(5); }); } From c0ae8aa9b0a6b1b557a316adf8af344d4141f46b Mon Sep 17 00:00:00 2001 From: Ankan Date: Thu, 3 Jul 2025 17:44:32 +0200 Subject: [PATCH 15/25] fmt --- substrate/frame/staking-async/src/mock.rs | 8 ++------ substrate/frame/staking-async/src/pallet/impls.rs | 10 ++++------ substrate/frame/staking-async/src/pallet/mod.rs | 3 ++- substrate/frame/staking-async/src/tests/slashing.rs | 7 ++++--- 4 files changed, 12 insertions(+), 16 deletions(-) diff --git a/substrate/frame/staking-async/src/mock.rs b/substrate/frame/staking-async/src/mock.rs index 88ad4d3e290f7..3c1d075519568 100644 --- a/substrate/frame/staking-async/src/mock.rs +++ b/substrate/frame/staking-async/src/mock.rs @@ -1020,10 +1020,6 @@ pub(crate) fn apply_pending_slashes_from_previous_era() { pub(crate) fn apply_pending_slashes_from_era(era: EraIndex) { for (key, _) in UnappliedSlashes::::iter_prefix(era) { - assert_ok!(Staking::apply_slash( - RuntimeOrigin::signed(1), - era, - key - )); + assert_ok!(Staking::apply_slash(RuntimeOrigin::signed(1), era, key)); } -} \ No newline at end of file +} diff --git a/substrate/frame/staking-async/src/pallet/impls.rs b/substrate/frame/staking-async/src/pallet/impls.rs index 1bf9289a1e5aa..dacb85a2ddfdf 100644 --- a/substrate/frame/staking-async/src/pallet/impls.rs +++ b/substrate/frame/staking-async/src/pallet/impls.rs @@ -37,12 +37,12 @@ use frame_support::{ defensive, dispatch::WithPostDispatchInfo, pallet_prelude::*, - StorageDoubleMap, traits::{ Defensive, DefensiveSaturating, Get, Imbalance, InspectLockableCurrency, LockableCurrency, OnUnbalanced, }, weights::Weight, + StorageDoubleMap, }; use frame_system::{pallet_prelude::BlockNumberFor, RawOrigin}; use pallet_staking_async_rc_client::{self as rc_client}; @@ -291,12 +291,10 @@ impl Pallet { Ok(used_weight) } - fn ensure_era_slashes_applied( - era: EraIndex, - ) -> Result<(), DispatchError> { + fn ensure_era_slashes_applied(era: EraIndex) -> Result<(), DispatchError> { ensure!( - !UnappliedSlashes::::contains_prefix(era), - Error::::UnappliedSlashesInPreviousEra + !UnappliedSlashes::::contains_prefix(era), + Error::::UnappliedSlashesInPreviousEra ); Ok(()) } diff --git a/substrate/frame/staking-async/src/pallet/mod.rs b/substrate/frame/staking-async/src/pallet/mod.rs index b8f662a365988..93e0a59247f80 100644 --- a/substrate/frame/staking-async/src/pallet/mod.rs +++ b/substrate/frame/staking-async/src/pallet/mod.rs @@ -1469,7 +1469,8 @@ pub mod pallet { // Note: we used current era before, but that is meant to be used for only election. // The right value to use here is the active era. - let era = session_rotation::Rotator::::active_era().saturating_add(T::BondingDuration::get()); + let era = session_rotation::Rotator::::active_era() + .saturating_add(T::BondingDuration::get()); if let Some(chunk) = ledger.unlocking.last_mut().filter(|chunk| chunk.era == era) { // To keep the chunk count down, we only keep one chunk per era. Since // `unlocking` is a FiFo queue, if a chunk exists for `era` we know that it will diff --git a/substrate/frame/staking-async/src/tests/slashing.rs b/substrate/frame/staking-async/src/tests/slashing.rs index 0643603c31a5b..6d03639cf288e 100644 --- a/substrate/frame/staking-async/src/tests/slashing.rs +++ b/substrate/frame/staking-async/src/tests/slashing.rs @@ -1429,7 +1429,6 @@ fn withdrawals_are_blocked_for_unprocessed_and_unapplied_slashes() { Session::roll_until_active_era(3); assert_ok!(Staking::unbond(RuntimeOrigin::signed(nominator), 150)); - // Rationale: We want to simulate a backlog of offences from era 3 that remain // unprocessed by the time unbonding becomes possible in era 6. // @@ -1480,8 +1479,10 @@ fn withdrawals_are_blocked_for_unprocessed_and_unapplied_slashes() { // WHEN: the nominator tries to withdraw unbonded funds while there are unapplied // offence in the last era. - assert_noop!(Staking::withdraw_unbonded(RuntimeOrigin::signed(nominator), 0), - Error::::UnappliedSlashesInPreviousEra); + assert_noop!( + Staking::withdraw_unbonded(RuntimeOrigin::signed(nominator), 0), + Error::::UnappliedSlashesInPreviousEra + ); // let's clear the slashes by manually applying them. apply_pending_slashes_from_previous_era(); From 822f6ab7a4abd945b151b5aa0637dfeb4af557e9 Mon Sep 17 00:00:00 2001 From: Ankan Date: Fri, 4 Jul 2025 14:39:24 +0200 Subject: [PATCH 16/25] fix ahm test --- .../staking-async/ahm-test/src/ah/mock.rs | 2 +- .../staking-async/ahm-test/src/ah/test.rs | 55 +++++++++++++++++-- 2 files changed, 50 insertions(+), 7 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 aee4e304ef747..baeef5f1d4f72 100644 --- a/substrate/frame/staking-async/ahm-test/src/ah/mock.rs +++ b/substrate/frame/staking-async/ahm-test/src/ah/mock.rs @@ -121,7 +121,7 @@ pub(crate) fn roll_until_next_active(mut end_index: SessionIndex) -> Vec::era_start_session_index(1), Some(5)); + // 1 era is reserved for the application of slashes. + let oldest_reportable_era = Rotator::::active_era() - (SlashDeferredDuration::get() - 1); + assert_eq!(oldest_reportable_era, 2); + // WHEN we report an offence older than Era 2 (oldest reportable era). assert_ok!(rc_client::Pallet::::relay_new_offence( RuntimeOrigin::root(), // offence is in era 1 5, + vec![rc_client::Offence { + offender: 3, + reporters: vec![], + slash_fraction: Perbill::from_percent(30), + }] + )); + + // THEN offence is ignored. + assert_eq!( + staking_events_since_last_call(), + vec![staking_async::Event::OffenceIgnored { + offence_era: 1, + validator: 3, + fraction: Perbill::from_percent(30) + }] + ); + + // WHEN: report an offence for the session belonging to the previous era + assert_eq!(Rotator::::era_start_session_index(2), Some(10)); + assert_ok!(rc_client::Pallet::::relay_new_offence( + RuntimeOrigin::root(), + // offence is in era 2 + 10, vec![rc_client::Offence { offender: 3, reporters: vec![], @@ -688,27 +716,42 @@ fn on_offence_previous_era() { }] )); - // reported + // THEN: offence is reported. assert_eq!( staking_events_since_last_call(), vec![staking_async::Event::OffenceReported { - offence_era: 1, + offence_era: 2, validator: 3, fraction: Perbill::from_percent(50) }] ); - // computed, and instantly applied, as we are already on era 3 (slash era = 1, defer = 2) + // computed in the next block (will be applied in era 4) roll_next(); assert_eq!( staking_events_since_last_call(), vec![ staking_async::Event::SlashComputed { - offence_era: 1, - slash_era: 3, + offence_era: 2, + slash_era: 4, offender: 3, page: 0 }, + ] + ); + + // roll to the next era. + roll_until_next_active(15); + // ensure we are in era 4. + assert_eq!(Rotator::::active_era(), 4); + // clear staking events. + let _ = staking_events_since_last_call(); + + // the next block applies the slashes. + roll_next(); + assert_eq!( + staking_events_since_last_call(), + vec![ staking_async::Event::Slashed { staker: 3, amount: 50 } ] ); From 6d174b6e37da0bd826fc041c0bf69ab44911ccc2 Mon Sep 17 00:00:00 2001 From: Ankan Date: Fri, 4 Jul 2025 14:40:16 +0200 Subject: [PATCH 17/25] fmt --- .../staking-async/ahm-test/src/ah/test.rs | 21 ++++++++----------- 1 file changed, 9 insertions(+), 12 deletions(-) diff --git a/substrate/frame/staking-async/ahm-test/src/ah/test.rs b/substrate/frame/staking-async/ahm-test/src/ah/test.rs index d238f863165e5..c9ccfc44cc0f9 100644 --- a/substrate/frame/staking-async/ahm-test/src/ah/test.rs +++ b/substrate/frame/staking-async/ahm-test/src/ah/test.rs @@ -678,7 +678,8 @@ fn on_offence_previous_era() { assert_eq!(SlashDeferredDuration::get(), 2); assert_eq!(Rotator::::era_start_session_index(1), Some(5)); // 1 era is reserved for the application of slashes. - let oldest_reportable_era = Rotator::::active_era() - (SlashDeferredDuration::get() - 1); + let oldest_reportable_era = + Rotator::::active_era() - (SlashDeferredDuration::get() - 1); assert_eq!(oldest_reportable_era, 2); // WHEN we report an offence older than Era 2 (oldest reportable era). @@ -730,14 +731,12 @@ fn on_offence_previous_era() { roll_next(); assert_eq!( staking_events_since_last_call(), - vec![ - staking_async::Event::SlashComputed { - offence_era: 2, - slash_era: 4, - offender: 3, - page: 0 - }, - ] + vec![staking_async::Event::SlashComputed { + offence_era: 2, + slash_era: 4, + offender: 3, + page: 0 + },] ); // roll to the next era. @@ -751,9 +750,7 @@ fn on_offence_previous_era() { roll_next(); assert_eq!( staking_events_since_last_call(), - vec![ - staking_async::Event::Slashed { staker: 3, amount: 50 } - ] + vec![staking_async::Event::Slashed { staker: 3, amount: 50 }] ); // nothing left From 9208866bf219de68f9f463f17039b27a25865440 Mon Sep 17 00:00:00 2001 From: Ankan Date: Sun, 6 Jul 2025 12:16:03 +0200 Subject: [PATCH 18/25] Update crate doc --- substrate/frame/staking-async/src/lib.rs | 112 ++++++++++++++++++++++- 1 file changed, 110 insertions(+), 2 deletions(-) diff --git a/substrate/frame/staking-async/src/lib.rs b/substrate/frame/staking-async/src/lib.rs index 981acd6ba574d..3c685fc6723fc 100644 --- a/substrate/frame/staking-async/src/lib.rs +++ b/substrate/frame/staking-async/src/lib.rs @@ -43,9 +43,117 @@ //! //! TODO //! -//! ## Slashing of Validators and Exposures +//! ## Slashing Pipeline and Withdrawal Restrictions //! -//! TODO +//! This pallet implements a robust slashing mechanism that ensures the integrity of the staking +//! system while preventing stakers from withdrawing funds that might still be subject to slashing. +//! +//! ### Overview of the Slashing Pipeline +//! +//! The slashing process consists of multiple phases: +//! +//! 1. **Offence Reporting**: Offences are reported from the relay chain through `on_new_offences` +//! 2. **Queuing**: Valid offences are added to the `OffenceQueue` for processing +//! 3. **Processing**: Offences are processed incrementally over multiple blocks +//! 4. **Application**: Slashes are either applied immediately or deferred based on configuration +//! +//! ### Phase 1: Offence Reporting +//! +//! Offences are reported from the relay chain (e.g., from BABE, GRANDPA, BEEFY, or parachain +//! modules) through the `on_new_offences` function: +//! +//! ```text +//! struct Offence { +//! offender: AccountId, // The validator being slashed +//! reporters: Vec, // Who reported the offence (may be empty) +//! slash_fraction: Perbill, // Percentage of stake to slash +//! } +//! ``` +//! +//! **Reporting Deadlines**: +//! - With deferred slashing: Offences must be reported within `SlashDeferDuration - 1` eras +//! - With immediate slashing: Offences can be reported up to `BondingDuration` eras old +//! +//! Example: If `SlashDeferDuration = 27` and current era is 100: +//! - Oldest reportable offence: Era 74 (100 - 26) +//! - Offences from era 73 or earlier are rejected +//! +//! ### Phase 2: Queuing +//! +//! When an offence passes validation, it's added to the queue: +//! +//! 1. **Storage**: Added to `OffenceQueue`: `(EraIndex, AccountId) -> OffenceRecord` +//! 2. **Era Tracking**: Era added to `OffenceQueueEras` (sorted vector of eras with offences) +//! 3. **Duplicate Handling**: If an offence already exists for the same validator in the same era, +//! only the higher slash fraction is kept +//! +//! ### Phase 3: Processing +//! +//! Offences are processed incrementally in `on_initialize` each block: +//! +//! ```text +//! 1. Load oldest offence from queue +//! 2. Move to ProcessingOffence storage +//! 3. For each exposure page (from last to first): +//! - Calculate slash for validator's own stake +//! - Calculate slash for each nominator (pro-rata based on exposure) +//! - Track total slash and reward amounts +//! 4. Once all pages processed, create UnappliedSlash +//! ``` +//! +//! **Key Features**: +//! - **Page-by-page processing**: Large validator sets don't overwhelm a single block +//! - **Pro-rata slashing**: Nominators slashed proportionally to their stake +//! - **Reward calculation**: A portion goes to reporters (if any) +//! +//! ### Phase 4: Application +//! +//! Based on `SlashDeferDuration`, slashes are either: +//! +//! **Immediate (SlashDeferDuration = 0)**: +//! - Applied right away in the same block +//! - Funds deducted from staking ledger immediately +//! +//! **Deferred (SlashDeferDuration > 0)**: +//! - Stored in `UnappliedSlashes` for future application +//! - Applied at era: `offence_era + SlashDeferDuration` +//! - Can be cancelled by governance before application +//! +//! ### Storage Items Involved +//! +//! - `OffenceQueue`: Pending offences to process +//! - `OffenceQueueEras`: Sorted list of eras with offences +//! - `ProcessingOffence`: Currently processing offence +//! - `ValidatorSlashInEra`: Tracks highest slash per validator per era +//! - `UnappliedSlashes`: Deferred slashes waiting for application +//! +//! ### Withdrawal Restrictions +//! +//! To maintain slashing guarantees, withdrawals are restricted: +//! +//! **Withdrawal Era Calculation**: +//! ```text +//! earliest_era_to_withdraw = min( +//! active_era, +//! last_fully_processed_offence_era + BondingDuration +//! ) +//! ``` +//! +//! **Example**: +//! - Active era: 100 +//! - Oldest unprocessed offence: Era 70 +//! - BondingDuration: 28 +//! - Withdrawal allowed only for chunks with era ≤ 97 (70 - 1 + 28) +//! +//! **Key Restrictions**: +//! 1. Cannot withdraw if previous era has unapplied slashes +//! 2. Cannot withdraw funds from eras with unprocessed offences +//! +//! ### Error Handling +//! +//! - `UnappliedSlashesInPreviousEra`: Withdrawal blocked due to pending slashes +//! - Offences arriving after deadline emit `OffenceIgnored` event +//! - Missing exposure data causes offence to be discarded #![cfg_attr(not(feature = "std"), no_std)] #![recursion_limit = "256"] From e824c107b0269d873ef791f1aa5558648de27686 Mon Sep 17 00:00:00 2001 From: Ankan Date: Sun, 6 Jul 2025 12:52:05 +0200 Subject: [PATCH 19/25] update doc --- substrate/frame/staking-async/src/lib.rs | 25 ++++++++++++++++++++++++ 1 file changed, 25 insertions(+) diff --git a/substrate/frame/staking-async/src/lib.rs b/substrate/frame/staking-async/src/lib.rs index 3c685fc6723fc..49f799097413b 100644 --- a/substrate/frame/staking-async/src/lib.rs +++ b/substrate/frame/staking-async/src/lib.rs @@ -145,6 +145,31 @@ //! - BondingDuration: 28 //! - Withdrawal allowed only for chunks with era ≤ 97 (70 - 1 + 28) //! +//! **Withdrawal Timeline Example with an Offence**: +//! ```text +//! Era: 90 91 92 93 94 95 96 97 98 99 100 ... 117 118 +//! | | | | | | | | | | | | | +//! Unbond: U +//! Offence: X +//! Reported: R +//! Processed: P (within next few blocks) +//! Slash Applied: S +//! Withdraw: ❌ ✓ +//! +//! With BondingDuration = 28 and SlashDeferDuration = 27: +//! - User unbonds in era 90 +//! - Offence occurs in era 90 +//! - Reported in era 92 (typically within 2 days) +//! - Processed in era 92 (within next few blocks after reporting) +//! - Slash deferred for 27 eras, applied at era 117 (90 + 27) +//! - Cannot withdraw unbonded chunks until era 118 (90 + 28) +//! +//! The 28-era bonding duration ensures that any offences committed before or during +//! unbonding have time to be reported, processed, and applied before funds can be +//! withdrawn. This provides a window for governance to cancel slashes that may have +//! resulted from software bugs. +//! ``` +//! //! **Key Restrictions**: //! 1. Cannot withdraw if previous era has unapplied slashes //! 2. Cannot withdraw funds from eras with unprocessed offences From e2d364046075f08372d87e08685906034b14c83a Mon Sep 17 00:00:00 2001 From: Ankan Date: Sun, 6 Jul 2025 14:50:08 +0200 Subject: [PATCH 20/25] fmt --- substrate/frame/staking-async/src/lib.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/substrate/frame/staking-async/src/lib.rs b/substrate/frame/staking-async/src/lib.rs index 49f799097413b..fb6c34285ea05 100644 --- a/substrate/frame/staking-async/src/lib.rs +++ b/substrate/frame/staking-async/src/lib.rs @@ -155,7 +155,7 @@ //! Processed: P (within next few blocks) //! Slash Applied: S //! Withdraw: ❌ ✓ -//! +//! //! With BondingDuration = 28 and SlashDeferDuration = 27: //! - User unbonds in era 90 //! - Offence occurs in era 90 @@ -163,7 +163,7 @@ //! - Processed in era 92 (within next few blocks after reporting) //! - Slash deferred for 27 eras, applied at era 117 (90 + 27) //! - Cannot withdraw unbonded chunks until era 118 (90 + 28) -//! +//! //! The 28-era bonding duration ensures that any offences committed before or during //! unbonding have time to be reported, processed, and applied before funds can be //! withdrawn. This provides a window for governance to cancel slashes that may have From 12e0c435bfdff7a60807158268e9d0e9e1b4ca72 Mon Sep 17 00:00:00 2001 From: Ankan Date: Mon, 7 Jul 2025 01:11:17 +0200 Subject: [PATCH 21/25] minor edits --- substrate/frame/staking-async/src/lib.rs | 8 +------- 1 file changed, 1 insertion(+), 7 deletions(-) diff --git a/substrate/frame/staking-async/src/lib.rs b/substrate/frame/staking-async/src/lib.rs index fb6c34285ea05..41a516835adb7 100644 --- a/substrate/frame/staking-async/src/lib.rs +++ b/substrate/frame/staking-async/src/lib.rs @@ -159,7 +159,7 @@ //! With BondingDuration = 28 and SlashDeferDuration = 27: //! - User unbonds in era 90 //! - Offence occurs in era 90 -//! - Reported in era 92 (typically within 2 days) +//! - Reported in era 92 (typically within 2 days, but reportable until Era 116) //! - Processed in era 92 (within next few blocks after reporting) //! - Slash deferred for 27 eras, applied at era 117 (90 + 27) //! - Cannot withdraw unbonded chunks until era 118 (90 + 28) @@ -173,12 +173,6 @@ //! **Key Restrictions**: //! 1. Cannot withdraw if previous era has unapplied slashes //! 2. Cannot withdraw funds from eras with unprocessed offences -//! -//! ### Error Handling -//! -//! - `UnappliedSlashesInPreviousEra`: Withdrawal blocked due to pending slashes -//! - Offences arriving after deadline emit `OffenceIgnored` event -//! - Missing exposure data causes offence to be discarded #![cfg_attr(not(feature = "std"), no_std)] #![recursion_limit = "256"] From ed8bbd865fedde6effb688f04d57e2ec97d830d6 Mon Sep 17 00:00:00 2001 From: Ankan Date: Mon, 7 Jul 2025 12:00:24 +0200 Subject: [PATCH 22/25] prdoc --- prdoc/pr_9079.prdoc | 32 ++++++++++++++++++++++++++++++++ 1 file changed, 32 insertions(+) create mode 100644 prdoc/pr_9079.prdoc diff --git a/prdoc/pr_9079.prdoc b/prdoc/pr_9079.prdoc new file mode 100644 index 0000000000000..536183d791741 --- /dev/null +++ b/prdoc/pr_9079.prdoc @@ -0,0 +1,32 @@ +title: "Prevent withdrawals while processing offences" + +doc: + - audience: Runtime Dev + description: | + Adds withdrawal restrictions to prevent users from withdrawing unbonded funds while + there are unprocessed offences that could result in slashing. This is a defensive + measure that ensures slashing guarantees are maintained even in extreme edge cases. + + Key changes: + - Withdrawals are blocked if there are unapplied slashes from the previous era + (returns `UnappliedSlashesInPreviousEra` error). This occurs when all unapplied + slashes for an era could not be applied within one era worth of blocks. While + one era is reserved for applying slashes page by page, if the era rolls over + before completion, these slashes can only be applied via the permissionless + `apply_slash` call. + - Withdrawals are restricted to the minimum of the active era and the last fully + processed offence era + - Unbonding chunks are now keyed by active era instead of current era + - Offences arriving after their intended application era are rejected and emit + `OffenceIgnored` event + + Both the `UnappliedSlashesInPreviousEra` error and withdrawal restrictions due to + delayed offence processing are extremely rare scenarios that should not occur under + normal operation. These are defensive measures to handle edge cases where slash + processing is delayed beyond expected timelines. + +crates: + - name: pallet-staking-async + bump: major + - name: staking-async-rc-client + bump: patch \ No newline at end of file From 1a34989cc7d5cf3cb77052df47acb6ce7389ce79 Mon Sep 17 00:00:00 2001 From: Ankan Date: Mon, 7 Jul 2025 13:02:29 +0200 Subject: [PATCH 23/25] fix pallet name in prdoc --- prdoc/pr_9079.prdoc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/prdoc/pr_9079.prdoc b/prdoc/pr_9079.prdoc index 536183d791741..10b31f2cfb124 100644 --- a/prdoc/pr_9079.prdoc +++ b/prdoc/pr_9079.prdoc @@ -28,5 +28,5 @@ doc: crates: - name: pallet-staking-async bump: major - - name: staking-async-rc-client + - name: pallet-staking-async-rc-client bump: patch \ No newline at end of file From ca46e1aab1174be0367f691d7adfbe2136f29948 Mon Sep 17 00:00:00 2001 From: Ankan Date: Mon, 7 Jul 2025 13:03:34 +0200 Subject: [PATCH 24/25] remove rc client from prdoc --- prdoc/pr_9079.prdoc | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/prdoc/pr_9079.prdoc b/prdoc/pr_9079.prdoc index 10b31f2cfb124..9bf01fa3cf3ef 100644 --- a/prdoc/pr_9079.prdoc +++ b/prdoc/pr_9079.prdoc @@ -27,6 +27,4 @@ doc: crates: - name: pallet-staking-async - bump: major - - name: pallet-staking-async-rc-client - bump: patch \ No newline at end of file + bump: major \ No newline at end of file From 7361c29fc03d12fe26b93a6fc1c7efc1886342c4 Mon Sep 17 00:00:00 2001 From: Ankan Date: Thu, 24 Jul 2025 18:13:09 +0200 Subject: [PATCH 25/25] fix kian feedback --- prdoc/pr_9079.prdoc | 2 +- .../staking-async/ahm-test/src/ah/test.rs | 2 +- substrate/frame/staking-async/src/lib.rs | 4 +- .../frame/staking-async/src/pallet/impls.rs | 40 +++++++++++-------- .../frame/staking-async/src/pallet/mod.rs | 2 +- .../frame/staking-async/src/tests/slashing.rs | 10 ++++- 6 files changed, 36 insertions(+), 24 deletions(-) diff --git a/prdoc/pr_9079.prdoc b/prdoc/pr_9079.prdoc index 9bf01fa3cf3ef..c303ae09d54d2 100644 --- a/prdoc/pr_9079.prdoc +++ b/prdoc/pr_9079.prdoc @@ -18,7 +18,7 @@ doc: processed offence era - Unbonding chunks are now keyed by active era instead of current era - Offences arriving after their intended application era are rejected and emit - `OffenceIgnored` event + `OffenceTooOld` event Both the `UnappliedSlashesInPreviousEra` error and withdrawal restrictions due to delayed offence processing are extremely rare scenarios that should not occur under diff --git a/substrate/frame/staking-async/ahm-test/src/ah/test.rs b/substrate/frame/staking-async/ahm-test/src/ah/test.rs index c9ccfc44cc0f9..2894de24b5a2e 100644 --- a/substrate/frame/staking-async/ahm-test/src/ah/test.rs +++ b/substrate/frame/staking-async/ahm-test/src/ah/test.rs @@ -697,7 +697,7 @@ fn on_offence_previous_era() { // THEN offence is ignored. assert_eq!( staking_events_since_last_call(), - vec![staking_async::Event::OffenceIgnored { + vec![staking_async::Event::OffenceTooOld { offence_era: 1, validator: 3, fraction: Perbill::from_percent(30) diff --git a/substrate/frame/staking-async/src/lib.rs b/substrate/frame/staking-async/src/lib.rs index 41a516835adb7..571f6257c92ed 100644 --- a/substrate/frame/staking-async/src/lib.rs +++ b/substrate/frame/staking-async/src/lib.rs @@ -93,12 +93,12 @@ //! //! ```text //! 1. Load oldest offence from queue -//! 2. Move to ProcessingOffence storage +//! 2. Move to `ProcessingOffence` storage //! 3. For each exposure page (from last to first): //! - Calculate slash for validator's own stake //! - Calculate slash for each nominator (pro-rata based on exposure) //! - Track total slash and reward amounts -//! 4. Once all pages processed, create UnappliedSlash +//! 4. Once all pages processed, create `UnappliedSlash` //! ``` //! //! **Key Features**: diff --git a/substrate/frame/staking-async/src/pallet/impls.rs b/substrate/frame/staking-async/src/pallet/impls.rs index dacb85a2ddfdf..e9acf54bb2b59 100644 --- a/substrate/frame/staking-async/src/pallet/impls.rs +++ b/substrate/frame/staking-async/src/pallet/impls.rs @@ -210,16 +210,10 @@ impl Pallet { Ok(()) } - pub(super) fn do_withdraw_unbonded(controller: &T::AccountId) -> Result { - let mut ledger = Self::ledger(Controller(controller.clone()))?; - let (stash, old_total) = (ledger.stash.clone(), ledger.total); - let active_era = Rotator::::active_era(); - - // Ensure last era slashes are applied. Else we block the withdrawals. - if active_era > 1 { - Self::ensure_era_slashes_applied(active_era.saturating_sub(1))?; - } - + /// Calculate the earliest era that withdrawals are allowed for, considering: + /// - The current active era + /// - Any unprocessed offences in the queue + fn calculate_earliest_withdrawal_era(active_era: EraIndex) -> EraIndex { // get lowest era for which all offences are processed and withdrawals can be allowed. let earliest_unlock_era_by_offence_queue = OffenceQueueEras::::get() .as_ref() @@ -227,8 +221,8 @@ impl Pallet { .copied() // if nothing in queue, use the active era. .unwrap_or(active_era) - // above returns earliest era for which offences are processed, so we subtract one to - // get the last era for which all offences are processed. + // above returns earliest era for which offences are NOT processed yet, so we subtract + // one from it which gives us the oldest era for which all offences are processed. .saturating_sub(1) // Unlock chunks are keyed by the era they were initiated plus Bonding Duration. // We do the same to processed offence era so they can be compared. @@ -239,15 +233,26 @@ impl Pallet { // Note: This situation is extremely unlikely, since offences have `SlashDeferDuration` eras // to be processed. If it ever occurs, it likely indicates offence spam and that we're // struggling to keep up with processing. - let earliest_era_to_withdraw = active_era.min(earliest_unlock_era_by_offence_queue); + active_era.min(earliest_unlock_era_by_offence_queue) + } + + pub(super) fn do_withdraw_unbonded(controller: &T::AccountId) -> Result { + let mut ledger = Self::ledger(Controller(controller.clone()))?; + let (stash, old_total) = (ledger.stash.clone(), ledger.total); + let active_era = Rotator::::active_era(); + + // Ensure last era slashes are applied. Else we block the withdrawals. + if active_era > 1 { + Self::ensure_era_slashes_applied(active_era.saturating_sub(1))?; + } + + let earliest_era_to_withdraw = Self::calculate_earliest_withdrawal_era(active_era); log!( debug, "Withdrawing unbonded stake. Active_era is: {:?} | \ - Earliest era for which all offences are processed: {:?} | \ Earliest era we can allow withdrawing: {:?}", active_era, - earliest_unlock_era_by_offence_queue, earliest_era_to_withdraw ); @@ -1210,7 +1215,7 @@ impl rc_client::AHStakingInterface for Pallet { // ignore offence if too old to report. if offence_era < oldest_reportable_offence_era { log!(warn, "🦹 on_new_offences: offence era {:?} too old; Can only accept offences from era {:?} or newer", offence_era, oldest_reportable_offence_era); - Self::deposit_event(Event::::OffenceIgnored { + Self::deposit_event(Event::::OffenceTooOld { validator: validator.clone(), fraction: slash_fraction, offence_era, @@ -2008,6 +2013,7 @@ impl Pallet { sorted_offence_queue_eras == offence_queue_eras, "Offence queue eras are not sorted" ); + drop(sorted_offence_queue_eras); // (2) Ensure oldest offence queue era is old enough. let active_era = Rotator::::active_era(); @@ -2040,7 +2046,7 @@ impl Pallet { for e in offence_queue_eras { let count = OffenceQueue::::iter_prefix(e).count(); ensure!(count > 0, "Offence queue is empty for era listed in offence queue eras"); - log!(warn, "Offence queue for era {:?} has {:?} offences queued", e, count); + log!(info, "Offence queue for era {:?} has {:?} offences queued", e, count); } // (4) Ensure all slashes older than (active era - 1) are applied. diff --git a/substrate/frame/staking-async/src/pallet/mod.rs b/substrate/frame/staking-async/src/pallet/mod.rs index 6744d1bb05309..aa59eff5ff548 100644 --- a/substrate/frame/staking-async/src/pallet/mod.rs +++ b/substrate/frame/staking-async/src/pallet/mod.rs @@ -1156,7 +1156,7 @@ pub mod pallet { /// Logged as an event for fail-safe observability. Unexpected(UnexpectedKind), /// An offence was reported that was too old to be processed, and thus was dropped. - OffenceIgnored { + OffenceTooOld { offence_era: EraIndex, validator: T::AccountId, fraction: Perbill, diff --git a/substrate/frame/staking-async/src/tests/slashing.rs b/substrate/frame/staking-async/src/tests/slashing.rs index 6d03639cf288e..b90b74fb138a8 100644 --- a/substrate/frame/staking-async/src/tests/slashing.rs +++ b/substrate/frame/staking-async/src/tests/slashing.rs @@ -650,12 +650,12 @@ fn really_old_offences_are_ignored() { assert_eq!( staking_events_since_last_call(), vec![ - Event::OffenceIgnored { + Event::OffenceTooOld { offence_era: 72, validator: 11, fraction: Perbill::from_percent(10) }, - Event::OffenceIgnored { + Event::OffenceTooOld { offence_era: 73, validator: 21, fraction: Perbill::from_percent(10) @@ -663,6 +663,12 @@ fn really_old_offences_are_ignored() { ] ); + // also check that the ignored offences are not stored anywhere + assert!(OffenceQueue::::iter_prefix(72).next().is_none()); + assert!(OffenceQueue::::iter_prefix(73).next().is_none()); + assert!(!OffenceQueueEras::::get().unwrap_or_default().contains(&72)); + assert!(!OffenceQueueEras::::get().unwrap_or_default().contains(&73)); + // WHEN: reporting offence for era 74. add_slash_in_era(11, 74, Perbill::from_percent(10));