pos mempool: stop duplicate pivot-decision transactions from crashing proposal construction

[peilun-conflux]

Problem

Pivot-decision transactions are signed over only the PivotBlockDecision payload, not the whole transaction, so a registered validator can produce many valid, distinct-hash transactions for one pivot decision by varying the unsigned chain_id. The mempool stored each as a separate entry for that decision, and proposal construction fed them all into one multisig, where duplicates from a single sender collapse to the same signer index and the aggregation panics — crashing the proposal path on every validator that pulled the set. One registered validator could sustain this to stall PoS proposal construction.

Fix

• Deduplicate at admission, keyed by sender: keep at most one pivot-decision transaction per (decision, sender), so later distinct-chain_id duplicates are dropped and can't produce the duplicate signer index.
• Never panic while aggregating: a failed multisig aggregation is logged and skipped, not fatal to proposal construction.
• Expire one entry at a time: TTL expiry drops only the expiring sender's transaction, not the whole set, so it no longer discards other validators' still-live entries.
• Scope the quorum check to the committee: admission accepts any registered (node_map) sender, broader than the committee, so a non-committee sender used to fail the whole set with UnknownAuthor; the check now counts only committee members, matching selection and aggregation.

Mempool-only: no change to signed bytes, hashing, or on-chain validation. The dedup leaves a committed set that a node stored under a different chain_id envelope uncleaned until TTL; that cleanup is a follow-up. Bringing chain_id into the pivot-decision signature is tracked separately.

🤖 Generated with Claude Code

---

This change is

[Pana]

retest this please

[kilo-code-bot]

Kilo Code Review could not run — your account is out of credits.

Add credits or switch to a free model to enable reviews on this change.

[kilo-code-bot]

Code Review Summary

Status: No New Issues Found | Recommendation: Merge (pending resolution of existing discussion)

Overview

This PR addresses a real consensus-safety bug: a Byzantine validator could craft many distinct-hash pivot-decision transactions (by varying the unsigned chain_id) and flood the mempool, causing SignedTransaction::new_multisig().unwrap() to panic with "Duplicate signature index" and crash every validator's proposal path.

The fixes are well-structured across two commits:

1. Commit 1 — Admission-level dedup (one pivot tx per sender), new_multisig returns Result, and selective TTL GC (drop one entry, not the whole set).
2. Commit 2 — Cleanup by committed height instead of envelope hash, PivotDecisionSet struct with height tag, and the commit notification carries committed_pivot_height.

The invariants introduced (one sender → one hash per decision, commit_pivot_height sweeps by height) are structurally enforced and are consistent throughout the call chain (state_computer → tasks → mempool → transaction_store). The seen_signers guard was correctly removed in commit 2 after structural uniqueness was guaranteed by the map key change.

Open Discussion

There is one existing active thread on mempool.rs:205 (the Err branch of new_multisig aggregation). The author has replied that an error log is sufficient since the error path should be unreachable in correct operation. No new findings supersede this discussion.

Files Reviewed (6 files)

• crates/cfxcore/core/src/pos/consensus/state_computer.rs — no new issues
• crates/cfxcore/core/src/pos/mempool/core_mempool/mempool.rs — existing discussion at line 205
• crates/cfxcore/core/src/pos/mempool/core_mempool/transaction_store.rs — no new issues
• crates/cfxcore/core/src/pos/mempool/shared_mempool/tasks.rs — no new issues
• crates/cfxcore/core/src/pos/mempool/shared_mempool/types.rs — no new issues
• crates/pos/types/types/src/transaction/mod.rs — no new issues

---

_{Reviewed by claude-sonnet-4.6 · Input: 15 · Output: 8.1K · Cached: 510.1K}