Skip to content

Ensure withdrawals don’t bypass slashing#9079

Merged
Ank4n merged 33 commits intomasterfrom
ankn-prevent-withdraw-while-processing-offence
Jul 28, 2025
Merged

Ensure withdrawals don’t bypass slashing#9079
Ank4n merged 33 commits intomasterfrom
ankn-prevent-withdraw-while-processing-offence

Conversation

@Ank4n
Copy link
Copy Markdown
Contributor

@Ank4n Ank4n commented Jul 2, 2025

Context

The offence handling pipeline has four main stages:

  1. Reporting on RC: Offences are reported on the Relay Chain (RC) and exported to Asset Hub (AH) via RC::AHClient.
  2. Queueing: AH staking pallet receives the offence in fn on_new_offence, performs sanity checks, and enqueues it in OffenceQueue and OffenceQueueEras.
  3. Processing: Offences are processed one by one, starting from the oldest era in the queue. Processed items are stored in UnappliedSlashes.
  4. Application: Finally, slashes are applied one page per block after the slash defer duration from the offence era.

Problem

While unlikely, a spam of offence reports could slow down processing enough that some offences remain unhandled even after their bonding period ends.

This creates a rare corner case: a withdrawal could happen for an era that still has pending offences, which breaks slashing guarantees.

Also, slash application happens gradually (one page per block). If some slashes are left unapplied at the end of their application era (due to chain stalls or similar), they must be manually applied using the permissionless apply_slash call.

Both scenarios are rare, but they expose risks to the integrity of slashing.


What this PR Changes

1. Block withdrawals for eras with unprocessed offences

Withdrawals are now restricted to the minimum of:

  • The active era, and
  • The last fully processed offence era.

This ensures withdrawals don't happen for eras that still have pending offences.

Why not block withdrawals per account instead?
That would require scanning each page of ErasStakersPaged for the validator the staker is exposed to — which is costly. Since this is an edge case, blocking at the era level is simpler and sufficient.


2. Block withdrawals if unapplied slashes remain in the previous era

Introduces a new safefguard: withdrawals are blocked if the immediately concluded era has unapplied slashes. Once the era is cleared, withdrawals resume as normal. We also only care about previous era, and if this ends up not enough to nudge participants to clear the unapplied slashes, the withdrawals should resume again in the next era (provided no new unapplied slashes remain in current era as well).

When this happens, trying to withdraw would emit the error UnappliedSlashesInPreviousEra. Anyone can look up the unapplied slashes in the previous era through the storage UnappliedSlashes and apply these via the permissionless call apply_slash.

This light enforcement should be enough to maintain slashing guarantees without being too disruptive.


3. Ensure a full era for applying slashes

Previously, it was possible to receive an offence report at the very end of the era when its slashes were meant to be applied.

We now reject offences that arrive after the end of the era before their application era. An event OffenceTooOld is emitted when this happens to make the behavior visible.

Open question:
We may want to update the prune_up_to value sent from AH to RC to ActiveEra - SlashDeferDuration + 1 instead of ActiveEra - BondingDuration. This could further guarantee that late offences never reach the staking pallet.


4. Unbonding chunks are keyed by active era

We’re moving away from using CurrentEra in business logic (except for elections). This change aligns unbonding with ActiveEra. The rest of the code will be refactored in #8807.


5. More checks on offence pipeline health

Added extra try state checks to ensure the offence processing state is healthy.


Notes

This is mostly a defensive improvement. These situations are extremely rare, but the added safeguards ensure slashing guarantees are upheld even in these extreme cases.

@Ank4n Ank4n requested a review from a team as a code owner July 2, 2025 19:16
@Ank4n Ank4n added T2-pallets This PR/Issue is related to a particular pallet. R1-breaking_change This PR introduces a breaking change and should be highlighted in the upcoming release. labels Jul 3, 2025
@Ank4n Ank4n changed the title [WIP] Only allow withdraws upto eras for which offences are processed [WIP] Ensure withdrawals don’t bypass slashing Jul 3, 2025
@Ank4n Ank4n requested review from kianenigma and sigurpol July 4, 2025 13:49
@Ank4n Ank4n changed the title [WIP] Ensure withdrawals don’t bypass slashing Ensure withdrawals don’t bypass slashing Jul 4, 2025
@Ank4n Ank4n requested review from a team, seadanda and tdimitrov July 24, 2025 16:31
@Ank4n Ank4n added this pull request to the merge queue Jul 28, 2025
Merged via the queue into master with commit 204c916 Jul 28, 2025
238 of 239 checks passed
@Ank4n Ank4n deleted the ankn-prevent-withdraw-while-processing-offence branch July 28, 2025 13:27
sigurpol added a commit that referenced this pull request Jul 29, 2025
@Ank4n Ank4n added the A4-backport-unstable2507 Pull request must be backported to the unstable2507 release branch label Jul 30, 2025
@paritytech-release-backport-bot
Copy link
Copy Markdown

Successfully created backport PR for unstable2507:

paritytech-release-backport-bot Bot pushed a commit that referenced this pull request Jul 30, 2025
## Context

The offence handling pipeline has four main stages:

1. **Reporting on RC**: Offences are reported on the Relay Chain (RC)
and exported to Asset Hub (AH) via RC::AHClient.
2. **Queueing**: AH staking pallet receives the offence in `fn
on_new_offence`, performs sanity checks, and enqueues it in
`OffenceQueue` and `OffenceQueueEras`.
3. **Processing**: Offences are processed one by one, starting from the
oldest era in the queue. Processed items are stored in
`UnappliedSlashes`.
4. **Application**: Finally, slashes are applied one page per block
after the slash defer duration from the offence era.

---

## Problem

While unlikely, a spam of offence reports could slow down processing
enough that some offences remain unhandled even after their bonding
period ends.

This creates a rare corner case: a withdrawal could happen for an era
that still has pending offences, which breaks slashing guarantees.

Also, slash application happens gradually (one page per block). If some
slashes are left unapplied at the end of their application era (due to
chain stalls or similar), they must be manually applied using the
permissionless `apply_slash` call.

Both scenarios are rare, but they expose risks to the integrity of
slashing.

---

## What this PR Changes

### 1. Block withdrawals for eras with unprocessed offences
Withdrawals are now restricted to the **minimum of:**

- The active era, and
- The last fully processed offence era.

This ensures withdrawals don't happen for eras that still have pending
offences.

**Why not block withdrawals per account instead?**
That would require scanning each page of `ErasStakersPaged` for the
validator the staker is exposed to — which is costly. Since this is an
edge case, blocking at the era level is simpler and sufficient.

---

### 2. Block withdrawals if unapplied slashes remain in the previous era
Introduces a new safefguard: withdrawals are blocked if the immediately
concluded era has unapplied slashes. Once the era is cleared,
withdrawals resume as normal. We also only care about previous era, and
if this ends up not enough to nudge participants to clear the unapplied
slashes, the withdrawals should resume again in the next era (provided
no new unapplied slashes remain in current era as well).

When this happens, trying to withdraw would emit the error
`UnappliedSlashesInPreviousEra`. Anyone can look up the unapplied
slashes in the previous era through the storage `UnappliedSlashes` and
apply these via the permissionless call `apply_slash`.

This light enforcement should be enough to maintain slashing guarantees
without being too disruptive.

---

### 3. Ensure a full era for applying slashes
Previously, it was possible to receive an offence report at the very end
of the era when its slashes were meant to be applied.

We now reject offences that arrive **after** the end of the era *before*
their application era. An event `OffenceTooOld` is emitted when this
happens to make the behavior visible.

**Open question:**
We may want to update the `prune_up_to` value sent from AH to RC to
`ActiveEra - SlashDeferDuration + 1` instead of `ActiveEra -
BondingDuration`. This could further guarantee that late offences never
reach the staking pallet.

---

### 4. Unbonding chunks are keyed by active era
We’re moving away from using `CurrentEra` in business logic (except for
elections). This change aligns unbonding with `ActiveEra`. The rest of
the code will be refactored in
[#8807](#8807).

---

### 5. More checks on offence pipeline health
Added extra try state checks to ensure the offence processing state is
healthy.

---

## Notes

This is mostly a defensive improvement. These situations are extremely
rare, but the added safeguards ensure slashing guarantees are upheld
even in these extreme cases.

(cherry picked from commit 204c916)
Ank4n added a commit that referenced this pull request Jul 30, 2025
Backport #9079 into `unstable2507` from Ank4n.

See the
[documentation](https://github.com/paritytech/polkadot-sdk/blob/master/docs/BACKPORT.md)
on how to use this bot.

<!--
  # To be used by other automation, do not modify:
  original-pr-number: #${pull_number}
-->

Co-authored-by: Ankan <10196091+Ank4n@users.noreply.github.com>
@redzsina redzsina moved this from Backlog to Scheduled in Security Audit (PRs) - SRLabs Aug 4, 2025
alvicsam pushed a commit that referenced this pull request Oct 17, 2025
## Context

The offence handling pipeline has four main stages:

1. **Reporting on RC**: Offences are reported on the Relay Chain (RC)
and exported to Asset Hub (AH) via RC::AHClient.
2. **Queueing**: AH staking pallet receives the offence in `fn
on_new_offence`, performs sanity checks, and enqueues it in
`OffenceQueue` and `OffenceQueueEras`.
3. **Processing**: Offences are processed one by one, starting from the
oldest era in the queue. Processed items are stored in
`UnappliedSlashes`.
4. **Application**: Finally, slashes are applied one page per block
after the slash defer duration from the offence era.

---

## Problem

While unlikely, a spam of offence reports could slow down processing
enough that some offences remain unhandled even after their bonding
period ends.

This creates a rare corner case: a withdrawal could happen for an era
that still has pending offences, which breaks slashing guarantees.

Also, slash application happens gradually (one page per block). If some
slashes are left unapplied at the end of their application era (due to
chain stalls or similar), they must be manually applied using the
permissionless `apply_slash` call.

Both scenarios are rare, but they expose risks to the integrity of
slashing.

---

## What this PR Changes

### 1. Block withdrawals for eras with unprocessed offences
Withdrawals are now restricted to the **minimum of:**

- The active era, and
- The last fully processed offence era.

This ensures withdrawals don't happen for eras that still have pending
offences.

**Why not block withdrawals per account instead?**  
That would require scanning each page of `ErasStakersPaged` for the
validator the staker is exposed to — which is costly. Since this is an
edge case, blocking at the era level is simpler and sufficient.

---

### 2. Block withdrawals if unapplied slashes remain in the previous era
Introduces a new safefguard: withdrawals are blocked if the immediately
concluded era has unapplied slashes. Once the era is cleared,
withdrawals resume as normal. We also only care about previous era, and
if this ends up not enough to nudge participants to clear the unapplied
slashes, the withdrawals should resume again in the next era (provided
no new unapplied slashes remain in current era as well).

When this happens, trying to withdraw would emit the error
`UnappliedSlashesInPreviousEra`. Anyone can look up the unapplied
slashes in the previous era through the storage `UnappliedSlashes` and
apply these via the permissionless call `apply_slash`.

This light enforcement should be enough to maintain slashing guarantees
without being too disruptive.

---

### 3. Ensure a full era for applying slashes
Previously, it was possible to receive an offence report at the very end
of the era when its slashes were meant to be applied.

We now reject offences that arrive **after** the end of the era *before*
their application era. An event `OffenceTooOld` is emitted when this
happens to make the behavior visible.

**Open question:**  
We may want to update the `prune_up_to` value sent from AH to RC to
`ActiveEra - SlashDeferDuration + 1` instead of `ActiveEra -
BondingDuration`. This could further guarantee that late offences never
reach the staking pallet.

---

### 4. Unbonding chunks are keyed by active era
We’re moving away from using `CurrentEra` in business logic (except for
elections). This change aligns unbonding with `ActiveEra`. The rest of
the code will be refactored in
[#8807](#8807).

---

### 5. More checks on offence pipeline health
Added extra try state checks to ensure the offence processing state is
healthy.

---

## Notes

This is mostly a defensive improvement. These situations are extremely
rare, but the added safeguards ensure slashing guarantees are upheld
even in these extreme cases.
@louismerlin louismerlin moved this from Scheduled to In progress in Security Audit (PRs) - SRLabs Nov 3, 2025
@redzsina redzsina moved this from In progress to Audited in Security Audit (PRs) - SRLabs Nov 10, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

A4-backport-unstable2507 Pull request must be backported to the unstable2507 release branch R1-breaking_change This PR introduces a breaking change and should be highlighted in the upcoming release. T2-pallets This PR/Issue is related to a particular pallet.

Projects

Status: Audited
Status: Done

Development

Successfully merging this pull request may close these issues.

6 participants