Skip to content

pos: lock out-queue stake on dispute for fully-retired nodes#3524

Open
peilun-conflux wants to merge 2 commits into
Conflux-Chain:masterfrom
peilun-conflux:pos-fix-cip156-dispute-lock
Open

pos: lock out-queue stake on dispute for fully-retired nodes#3524
peilun-conflux wants to merge 2 commits into
Conflux-Chain:masterfrom
peilun-conflux:pos-fix-cip156-dispute-lock

Conversation

@peilun-conflux

@peilun-conflux peilun-conflux commented May 28, 2026

Copy link
Copy Markdown
Contributor

Summary

CIP-156 changed the PoS dispute penalty from permanently forfeiting stake to locking it for dispute_locked_views. NodeLockStatus::forfeit gates that relock on available_votes > 0, but available_votes counts only in_queue + locked, not out_queue. A validator that has retired all of its available votes — stake sitting only in out_queue, so available_votes == 0 — is therefore not relocked when disputed, and recovers its stake on the normal out-queue schedule instead of being locked for dispute_locked_views. A Byzantine validator can exploit this by self-retiring after equivocating, defeating the dispute time-lock deterrent. (Under CIP-156 the penalty is a lock rather than a burn, so this weakens the deterrent rather than enabling fund loss, and cannot split consensus since all nodes apply the same rule.)

Relocking the out-queue mutates the per-block PosState, so changing this behavior is a consensus-rule change. It is gated behind a new pos_fix_cip156_transition_view, defaulting to u64::MAX.

Related: #3513 — another consensus-rule PoS fix slated for the same next PoS hardfork.

Follow-up

No unit test is included here. The NodeLockStatus methods (including forfeit) read the chain config from a process-global singleton (POS_STATE_CONFIG), so a unit test can't supply its own config without mutating shared global state. A follow-up PR will refactor these methods to take the config as an explicit parameter, making them unit-testable in isolation; the regression test for this fix will be added there.


This change is Reviewable

CIP-156 changed the dispute penalty from forfeiting stake to locking it
for `dispute_locked_views`. `NodeLockStatus::forfeit` gated the entire
relock on `available_votes > 0`, but `available_votes` counts only
`in_queue + locked` — not `out_queue`. A node that retired all of its
available votes (stake sitting only in `out_queue`, `available_votes == 0`)
was therefore not relocked when disputed: a Byzantine validator that
self-retires after equivocating recovers its stake on the normal
out-queue schedule instead of being locked for `dispute_locked_views`.

Relocking the out-queue mutates the per-block PosState, so it is a
consensus-rule change and is gated behind a new
`pos_fix_cip156_transition_view` (u64::MAX on master; a finite activation
view is set on the mainnet/testnet branches when the hardfork is
scheduled). Before the transition behavior is unchanged; after it, a
dispute also relocks the out-queue stake.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Comment thread crates/pos/types/types/src/term_state/lock_status.rs Outdated
@kilo-code-bot

kilo-code-bot Bot commented May 28, 2026

Copy link
Copy Markdown
Contributor

Code Review Summary

Status: No Issues Found | Recommendation: Merge

Overview

Severity Count
CRITICAL 0
WARNING 0
SUGGESTION 0
Resolved Issues
File Line Issue Status
crates/pos/types/types/src/term_state/lock_status.rs 288 len() > 0is_empty() ✅ Fixed in commit 1a7dd30f7
Files Reviewed (3 files)
  • crates/pos/types/types/src/term_state/lock_status.rs — previously flagged len() > 0 now fixed with is_empty()
  • crates/config/src/configuration.rs — no issues
  • crates/pos/types/types/src/term_state/pos_state_config.rs — no issues

Reviewed by claude-4.6-sonnet-20260217 · 91,574 tokens

Addresses a review suggestion: StatusList exposed len() but not
is_empty(), so the out_queue check read `len() > 0`. Add is_empty()
and use the idiomatic `!self.out_queue.is_empty()`.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@peilun-conflux peilun-conflux requested a review from ChenxingLi May 29, 2026 01:41

@ChenxingLi ChenxingLi left a comment

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.

@ChenxingLi reviewed 3 files and all commit messages, and made 1 comment.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on peilun-conflux).


a discussion (no related file):
Does this PR require a hardfork?

@peilun-conflux peilun-conflux left a comment

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

@peilun-conflux made 1 comment.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on ChenxingLi).


a discussion (no related file):

Previously, ChenxingLi (Chenxing Li) wrote…

Does this PR require a hardfork?

Right, it is gated behind a new pos_fix_cip156_transition_view, defaulting to u64::MAX, which will be set with the next hardfork.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants