pos mempool: stop duplicate pivot-decision votes from crashing proposal construction#3540
Open
peilun-conflux wants to merge 1 commit into
Open
Conversation
A registered validator could submit many distinct mempool transactions for
the same pivot decision by varying the unsigned `chain_id`: the pivot-decision
signature covers only the `PivotBlockDecision` payload, not the rest of the
`RawTransaction`, so each variant carries a valid signature but a different tx
hash. Those entries were counted as separate voting power in `get_block`'s
quorum check and produced a repeated signer index, making
`SignedTransaction::new_multisig().unwrap()` panic ("Duplicate signature
index") and crashing proposal construction on every validator that pulled the
poisoned set.
- Enforce one pivot-decision per (decision, sender) at admission.
- `new_multisig` returns `Result`; `get_block` dedups signer indices and
logs-and-skips on aggregation error instead of panicking.
- `gc_by_system_ttl` drops only the expiring (sender, hash) entry, not the
whole pivot-decision set, so one vote's TTL expiry no longer discards other
validators' live votes.
- `get_block`'s quorum check counts only current-committee voters (matching
selection and aggregation), so a registered non-committee node's vote is
ignored rather than failing the set with `UnknownAuthor`.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
![kilo-code-bot[bot]](https://avatars.githubusercontent.com/in/2193792?s=60&v=4){kind=small}
Member
|
retest this please |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
Pivot-decision transactions are signed over only the
PivotBlockDecisionpayload, not the wholeRawTransaction. A registered validator can therefore produce many valid, distinct-hash transactions for the same vote by varying the unsignedchain_id. The mempool stored each as a separate entry for the same pivot decision, so during proposal construction (Mempool::get_block) they were counted as separate voting power and produced a repeated signer index — makingSignedTransaction::new_multisig().unwrap()panic withBitVecError("Duplicate signature index")and crashing the proposal path on every validator that pulled the poisoned set. A single registered validator could sustain this to stall PoS proposal construction.Fix
new_multisigreturnsResult, andget_blockdedups signer indices and logs-and-skips on aggregation error instead ofunwrap()-panicking.gc_by_system_ttlremoves only the expiring(sender, hash)entry, not the whole pivot-decision set, so one vote's TTL expiry no longer discards other validators' still-live votes.get_block's quorum check counts only current-committee voters (consistent with selection and aggregation, which already filter), so a registered non-committee node's valid vote is ignored instead of failing the set withUnknownAuthorand stalling pivot decisions.Whether
chain_id(and the rest of the envelope) should be brought into the pivot-decision signature is tracked separately; this PR is the mempool-level hotfix and changes no on-chain validation rules.🤖 Generated with Claude Code
This change is