fix(fork-choice): make the equal-slot equivocation tie deterministic

[tcoratger]

What

Fixes #1172: an equivocation tie-break in fork choice was insertion-order dependent, so two honest nodes with the same blocks and votes but different arrival order could pick different heads permanently. This is a genuine consensus-safety / determinism bug — all unit tests passed; the split only appeared between nodes on a live network.

The bug

An equivocating validator can sign two distinct votes A and B for the same slot. Nothing rejects the second (the pool is keyed by attestation data, and the two data differ), so both are admitted and aggregated. The latest-vote extraction then resolved the equal-slot tie with a strict < on slot:

if previous_vote is None or previous_vote.slot < attestation_data.slot:  # s < s is False on a tie
    latest_vote_by_validator[validator_index] = attestation_data

On the tie the second vote never overwrites the first, so the winner is simply whichever was iterated first — i.e. dict insertion = arrival order. The block-level tiebreak max(children, key=(weights[r], r)) is one layer too late: the equivocator's weight has already landed on different branches on the two nodes, so the branch weights are unequal and the tiebreak never fires. The heads diverge and stay diverged.

The fix

Resolve the equal-slot tie deterministically by the largest canonical attestation-data root, the same rule the block-level tiebreak applies to block roots. The extraction now sorts the pool by (slot, hash_tree_root(data)) descending and keeps the first vote per validator:

• The head becomes a pure function of store contents — independent of arrival or insertion order.
• An equivocator is counted once, on one branch every node agrees on. That is strictly safer than the status quo, where first-seen let one Byzantine validator land its weight on branch A at one node and branch B at another (an effective cross-network double-count). Fork-choice head weight is not the finality mechanism, so counting one deterministic vote changes no finalization.
• hash_tree_root(attestation_data) is computed once per distinct vote (in the sort key), never per validator.

This is chosen over the consensus-specs approach of dropping equivocators via an equivocating_indices set: that requires slashing / equivocation-detection infrastructure this fork does not have, whereas a canonical tiebreak needs none.

Block production gains the same (target.slot, hash_tree_root) secondary sort key, so a proposer builds the same block regardless of arrival order (a related determinism gap where distinct data at one target slot previously fell back to arrival order under truncation).

Determinism note (supersedes an earlier choice)

An earlier change (PR #892) made the tie keep the first-seen vote via an insertion-order-preserving pool merge. That only guaranteed intra-run reproducibility (identical output across PYTHONHASHSEED for a single scripted fill); it did not give inter-node agreement, which is what this issue is about. The canonical-root tiebreak is a strict superset: it keeps the hash-seed reproducibility PR #892 wanted and adds inter-node determinism. Verified by filling the equivocation vectors under PYTHONHASHSEED=0 and 7 — byte-identical.

Tests

Consensus vectors only (fork tests are vector-driven):

• The testing fixture's own vote checker mirrored the old first-seen rule; it is brought into lockstep with the spec so attestation_checks match the fixed store.
• An existing vector asserted the arrival-order winner (fork_a, first-gossiped); it is corrected to the deterministic winner (fork_b, larger data root), with its Given/When/Then docstring rewritten to describe the canonical rule.
• Two new vectors deliver the same equivocating votes in opposite arrival orders and assert an identical head — directly encoding order-independence (pre-fix they diverged, post-fix they agree).

Full tests/consensus/lstar/fork_choice/ re-fill passes (119 vectors), no other vector relied on arrival order; just check is clean.

Credit

Found via the Lean 4 formalization of this spec (NyxFoundation/formal-leanSpec): modeling the store as an unordered map made "the head is a function of store contents" unprovable, because the result depended on an insertion order a map does not carry.

Refs #1172

🤖 Generated with Claude Code