Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
26 commits
Select commit Hold shift + click to select a range
26a5a50
raise offence queue eras bound limit
cirko33 Mar 19, 2026
1ec3d82
Update from github-actions[bot] running command 'prdoc --audience run…
github-actions[bot] Mar 19, 2026
83a0e95
Update from github-actions[bot] running command 'prdoc --audience run…
github-actions[bot] Mar 19, 2026
ed2e2cb
Merge branch 'master' into cirko33-offence-queue-raise-limit
cirko33 Mar 19, 2026
11f0815
Merge branch 'master' into cirko33-offence-queue-raise-limit
cirko33 Mar 20, 2026
b821916
Update from github-actions[bot] running command 'bench --pallet palle…
github-actions[bot] Mar 20, 2026
75bae3b
update prdoc
cirko33 Mar 20, 2026
74dcf8d
Merge branch 'master' into cirko33-offence-queue-raise-limit
cirko33 Mar 24, 2026
753358e
Merge branch 'master' into cirko33-offence-queue-raise-limit
cirko33 Mar 25, 2026
3bd444a
Merge branch 'master' into cirko33-offence-queue-raise-limit
Ank4n Mar 25, 2026
2ad136e
update offence processing limits in staking-async pallet
cirko33 Mar 25, 2026
7a4788f
add explaination comment & add unit test
cirko33 Mar 26, 2026
3526dfc
Merge branch 'master' into cirko33-offence-queue-raise-limit
cirko33 Mar 26, 2026
4729384
naming and comment refactor
cirko33 Mar 26, 2026
dc40626
fix test and fmt
cirko33 Mar 26, 2026
ee007e9
Merge branch 'master' into cirko33-offence-queue-raise-limit
cirko33 Mar 26, 2026
b6b7015
comment fix
cirko33 Mar 26, 2026
ce9b59e
update prdoc
cirko33 Mar 27, 2026
c60f124
sub instead of add & fix tests
cirko33 Mar 27, 2026
005bc7f
fmt
cirko33 Mar 27, 2026
d3adf1d
Merge branch 'master' into cirko33-offence-queue-raise-limit
cirko33 Mar 27, 2026
a8b5acd
fixed comment
cirko33 Mar 27, 2026
339c342
adjust test to use 2 eras instead of 3
cirko33 Mar 27, 2026
89226e2
update comment
cirko33 Mar 29, 2026
bbea93f
Merge branch 'master' into cirko33-offence-queue-raise-limit
cirko33 Mar 31, 2026
7a253fe
Merge branch 'master' into cirko33-offence-queue-raise-limit
cirko33 Mar 31, 2026
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view

Large diffs are not rendered by default.

9 changes: 9 additions & 0 deletions prdoc/pr_11435.prdoc
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
title: Raise offence queue eras bound limit
doc:
- audience: Runtime Dev
description: Fixes a bug where OffenceQueueEras bound (BondingDuration) was incorrect when SlashDeferDuration=0. The oldest reportable offence era formula allowed more eras than the bound could hold.
crates:
- name: pallet-staking-async
bump: minor
- name: asset-hub-westend-runtime
bump: patch
Original file line number Diff line number Diff line change
Expand Up @@ -942,11 +942,10 @@ fn on_offence_previous_era_instant_apply() {
.build()
.execute_with(|| {
let _ = roll_until_next_active(0);
let _ = roll_until_next_active(5);
let active_validators = roll_until_next_active(10);
let active_validators = roll_until_next_active(7);

assert_eq!(active_validators, vec![3, 5, 6, 8]);
assert_eq!(Rotator::<Runtime>::active_era(), 3);
assert_eq!(Rotator::<Runtime>::active_era(), 2);

// flush the events.
let _ = staking_events_since_last_call();
Expand Down
5 changes: 4 additions & 1 deletion substrate/frame/staking-async/src/pallet/impls.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1194,7 +1194,10 @@ impl<T: Config> rc_client::AHStakingInterface for Pallet<T> {
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())
// Align with the SlashDeferDuration > 0 branch: accept offences from at most
// BondingDuration - 1 distinct eras, ensuring the count fits within the
// OffenceQueueEras bound.
active_era.index.saturating_sub(T::BondingDuration::get().saturating_sub(2))
} else {
// slashes are deffered, so we only accept offences that are not older than the
// defferal duration.
Expand Down
21 changes: 19 additions & 2 deletions substrate/frame/staking-async/src/pallet/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -583,6 +583,21 @@ pub mod pallet {
}
}

const OFFENCE_QUEUE_ERAS_BOUND: u32 = 10;
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What is the purpose of this? Why is it 10? Why isn't it used anywhere besides the Get impl below?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The scope could be limited since it's used only in the get impl below indeed - that came from (my 😅 ) comment where magic number 10 was used in two different places - but can be revised and kept where it is used only.
About why 10 and not 20, or 2*bonding_duration or anything else - this is very arbitrary. My understanding of this issue - but please @Ank4n , and @cirko33 (these days at EthCc so might not answer promptly) is that

  1. the original issue reporting during audit is not a realistic one since we have 1 era to apply all slashes in an era and so it is practically impossible for count to exceed BondingDuration.
  2. considering the above, the scope of WeakBoundedVector here is not to protect against offence spam vs slow processing but rather being robust in case during a runtime upgrade we lower BondingDuration and then we still want to be sure to be able to process offences from old eras that wouldn't fit in the post-RU OffenceQueueEras. Now, 10 is a magic number that might or might not work in this case and whatever number might not be good enough here. If we want to be more robust, and we still don't want to go for the hackish way of going through Vec conversion + force_from() discussed in other comments in this PR, one could assume that in the case someone wants to lower BondingDuration, it must provides also an ad-hoc migration or something like that.

Wdyt?

/// Custom bound for [`OffenceQueueEras`] which is equal to `Config::BondingDuration +
/// OFFENCE_QUEUE_ERAS_BOUND`.
pub struct OffenceQueueErasBound<T>(core::marker::PhantomData<T>);
impl<T: Config> Get<u32> for OffenceQueueErasBound<T> {
fn get() -> u32 {
let bonding_duration = T::BondingDuration::get();
bonding_duration.saturating_add(OFFENCE_QUEUE_ERAS_BOUND) // adding OFFENCE_QUEUE_ERAS_BOUND eras
// to add headroom to
// the bound for runtime upgrades that
// lower BondingDuration so we avoid
// the try_into trap.
}
}

/// A mapping from still-bonded eras to the first session index of that era.
///
/// Must contains information for eras for the range:
Expand Down Expand Up @@ -801,12 +816,14 @@ pub mod pallet {
/// - When a new offence is added to `OffenceQueue`, its era is **inserted in sorted order**
/// if not already present.
/// - When all offences for an era are processed, it is **removed** from this list.
/// - The maximum length of this vector is bounded by `BondingDuration`.
/// - The maximum length of this vector is bounded by `BondingDuration +
/// OFFENCE_QUEUE_ERAS_BOUND`.
///
/// This eliminates the need for expensive iteration and sorting when fetching the next offence
/// to process.
#[pallet::storage]
pub type OffenceQueueEras<T: Config> = StorageValue<_, WeakBoundedVec<u32, T::BondingDuration>>;
pub type OffenceQueueEras<T: Config> =
StorageValue<_, WeakBoundedVec<u32, OffenceQueueErasBound<T>>>;

/// Tracks the currently processed offence record from the `OffenceQueue`.
///
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -569,14 +569,14 @@ fn mixed_era_offences_processed_based_on_era_specific_setting() {

let nominator_stake_before = Staking::ledger(101.into()).unwrap().active;

// Report offence from era 1 (slashable) - nominator should be slashed
add_slash_in_era(11, 1, Perbill::from_percent(5));
// Report offence from era 2 (slashable) - nominator should be slashed
add_slash_in_era(11, 2, Perbill::from_percent(5));
Session::roll_next();

let nominator_stake_after_era1_slash = Staking::ledger(101.into()).unwrap().active;
let nominator_stake_after_era2_slash = Staking::ledger(101.into()).unwrap().active;
assert!(
nominator_stake_after_era1_slash < nominator_stake_before,
"Nominator should be slashed for era 1 offence"
nominator_stake_after_era2_slash < nominator_stake_before,
"Nominator should be slashed for era 2 offence"
);

// Report offence from era 3 (NOT slashable) - nominator should NOT be slashed
Expand All @@ -585,7 +585,7 @@ fn mixed_era_offences_processed_based_on_era_specific_setting() {

let nominator_stake_after_era3_slash = Staking::ledger(101.into()).unwrap().active;
assert_eq!(
nominator_stake_after_era3_slash, nominator_stake_after_era1_slash,
nominator_stake_after_era3_slash, nominator_stake_after_era2_slash,
"Nominator should NOT be slashed for era 3 offence"
);
});
Expand Down
69 changes: 69 additions & 0 deletions substrate/frame/staking-async/src/tests/slashing.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2147,3 +2147,72 @@ mod paged_slashing {
});
}
}

#[test]
fn old_offences_rejected_with_zero_slash_defer_duration() {
Comment thread
sigurpol marked this conversation as resolved.
// Regression test: with SlashDeferDuration=0, the oldest reportable offence era is
// `active_era - (BondingDuration - 2)`. Offences older than that are rejected.
ExtBuilder::default().nominate(false).build_and_execute(|| {
assert_eq!(SlashDeferDuration::get(), 0);
assert_eq!(BondingDuration::get(), 3);

// advance to era 5.
Session::roll_until_active_era(5);
assert_eq!(active_era(), 5);

// clear events from era transitions.
staking_events_since_last_call();

let offence_era = 3u32;

// WHEN: reporting offence for era 3 (outside the valid window).
// oldest_reportable = 5 - (3 - 2) = 4, so era 3 < 4 is too old.
add_slash_in_era(11, offence_era, Perbill::from_percent(10));

// THEN: correctly rejected as too old.
assert_eq!(
staking_events_since_last_call(),
vec![Event::OffenceTooOld {
offence_era,
validator: 11,
fraction: Perbill::from_percent(10),
}]
);

// OffenceQueue and OffenceQueueEras remain consistent: no orphaned records.
assert!(OffenceQueue::<Test>::iter_prefix(offence_era).next().is_none());
assert!(!OffenceQueueEras::<Test>::get().unwrap_or_default().contains(&offence_era));

// WHEN: reporting offence for era 4 (within the valid window, 4 >= 4).
add_slash_in_era(21, 4, Perbill::from_percent(10));

// THEN: offence is accepted and stored consistently.
assert_eq!(
staking_events_since_last_call(),
vec![Event::OffenceReported {
offence_era: 4,
validator: 21,
fraction: Perbill::from_percent(10),
}]
);

// OffenceQueue has the record for era 4.
assert!(OffenceQueue::<Test>::iter_prefix(4).next().is_some());
// OffenceQueueEras tracks era 4.
assert!(OffenceQueueEras::<Test>::get().unwrap_or_default().contains(&4));

// AND: computed in the next block.
Session::roll_next();
assert_eq!(
staking_events_since_last_call(),
vec![
Event::SlashComputed { offence_era: 4, slash_era: 4, offender: 21, page: 0 },
Event::Slashed { staker: 21, amount: 100 },
]
);

// After processing, both storages are cleaned up consistently.
assert!(OffenceQueue::<Test>::iter_prefix(4).next().is_none());
assert!(!OffenceQueueEras::<Test>::get().unwrap_or_default().contains(&4));
});
}
Loading