Skip to content

Commit b2db782

Browse files
authored
fix(fork-choice): make the equal-slot equivocation tie deterministic (leanSpec #1181) (#503)
## What Ports leanSpec [#1181](leanEthereum/leanSpec#1181): make the equal-slot equivocation tie in fork choice deterministic. ## Why 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. `extract_latest_attestations` then resolved the equal-slot tie with a strict `existing.slot < entry.data.slot` while iterating in **insertion (arrival) order**, so the winner was simply whichever was seen first. Two honest nodes with the same blocks and votes but different arrival order could land the equivocator's weight on different branches and pick **different heads permanently** — a determinism/safety bug that unit tests miss because it only appears across nodes. ## Changes `crates/storage/src/store.rs` — both `extract_latest_attestations` implementations (the aggregated `PayloadBuffer` and the raw `GossipSignatureBuffer`): - Process votes newest-first, breaking the equal-slot tie toward the **larger canonical attestation-data root** — the same rule the block-level fork-choice tiebreak applies to block roots. The extracted head becomes a pure function of pool contents, independent of arrival/insertion order. An equivocator is counted once, on one branch every node agrees on. - The pool key is already `hash_tree_root(data)`, so the tie needs no extra hashing. Block production is **not** changed: ethlambda's block builder already uses `data_root` as its deterministic final tiebreak (`EntryScore::ordering_key`, from leanSpec #1149), so it is already order-independent. The `drain` doc comment is updated (vote-extraction determinism no longer depends on drain order), and the unit test that asserted first-seen-wins is rewritten to assert order-independence (larger canonical root wins in both arrival orders). ## Tests - `extract_latest_attestations_canonical_root_wins_on_slot_tie` (rewritten from `..._first_inserted_wins_on_slot_tie`) `cargo fmt`, `clippy -D warnings`, and the storage lib tests (44) pass. > Since #1181 is merged in leanSpec, the released fork-choice fixtures encode the new expected head for the equivocation vectors; this change aligns ethlambda with them. Recommend a `forkchoice_spectests` run against fresh fixtures to confirm.
1 parent d96e524 commit b2db782

1 file changed

Lines changed: 77 additions & 74 deletions

File tree

crates/storage/src/store.rs

Lines changed: 77 additions & 74 deletions
Original file line numberDiff line numberDiff line change
@@ -228,8 +228,10 @@ impl PayloadBuffer {
228228
///
229229
/// Drains in insertion order (via `self.order`) so downstream consumers
230230
/// like `promote_new_aggregated_payloads` re-insert into known_payloads
231-
/// deterministically. HashMap iteration would be RandomState-seeded and
232-
/// produce non-deterministic vote ordering for same-slot equivocation.
231+
/// deterministically; `self.data` iteration alone would be RandomState-seeded.
232+
/// (Fork-choice vote extraction no longer depends on this order: it resolves
233+
/// same-slot equivocation by canonical attestation-data root, see
234+
/// `extract_latest_attestations`.)
233235
fn drain(&mut self) -> Vec<(HashedAttestationData, SingleMessageAggregate)> {
234236
self.total_proofs = 0;
235237
let mut result = Vec::with_capacity(self.data.values().map(|e| e.proofs.len()).sum());
@@ -296,26 +298,23 @@ impl PayloadBuffer {
296298

297299
/// Extract per-validator latest attestations from proofs' participation bits.
298300
///
299-
/// Iterates entries in insertion order (via `self.order`) so that, when two
300-
/// aggregations carry the same `slot` but disagree on the target (an
301-
/// equivocation by the shared validators), the first-observed aggregation
302-
/// wins. The ethrex spec relies on Python dict insertion-order semantics
303-
/// here; iterating `self.data.values()` would be RandomState-seeded and
304-
/// fail the equivocation fork-choice tests non-deterministically.
301+
/// An equivocator can cast two distinct votes at the same `slot`. To keep the
302+
/// extracted head a pure function of pool contents (independent of arrival or
303+
/// insertion order), votes are processed newest-first with an equal-slot tie
304+
/// broken toward the larger canonical attestation-data root — the same rule the
305+
/// block-level fork-choice tiebreak applies to block roots (leanSpec #1181). The
306+
/// pool key is already `hash_tree_root(data)`, so the tie needs no extra hashing.
305307
fn extract_latest_attestations(&self) -> HashMap<u64, AttestationData> {
308+
let mut ordered: Vec<(&H256, &PayloadEntry)> = self.data.iter().collect();
309+
// Descending by (slot, data_root): the larger tuple is the canonical winner.
310+
ordered.sort_unstable_by(|a, b| (b.1.data.slot, b.0).cmp(&(a.1.data.slot, a.0)));
311+
306312
let mut result: HashMap<u64, AttestationData> = HashMap::new();
307-
for data_root in &self.order {
308-
let Some(entry) = self.data.get(data_root) else {
309-
continue;
310-
};
313+
for (_data_root, entry) in ordered {
311314
for proof in &entry.proofs {
312315
for vid in proof.participant_indices() {
313-
let should_update = result
314-
.get(&vid)
315-
.is_none_or(|existing| existing.slot < entry.data.slot);
316-
if should_update {
317-
result.insert(vid, entry.data.clone());
318-
}
316+
// Descending order means the first vote seen for a validator wins.
317+
result.entry(vid).or_insert_with(|| entry.data.clone());
319318
}
320319
}
321320
}
@@ -466,25 +465,22 @@ impl GossipSignatureBuffer {
466465

467466
/// Extract per-validator latest attestations from the raw signature pool.
468467
///
469-
/// Mirrors `PayloadBuffer::extract_latest_attestations`: iterate data_roots
470-
/// in insertion order (via `self.order`) so that, when two votes share the
471-
/// same `slot`, the first-observed one wins for the validators present in
472-
/// both. This matches the leanSpec `location == "signatures"` checker, which
473-
/// folds `attestation_signatures` keeping each validator's highest-slot vote
474-
/// with first-seen-wins on slot ties.
468+
/// Mirrors `PayloadBuffer::extract_latest_attestations`: votes are processed
469+
/// newest-first with an equal-slot tie broken toward the larger canonical
470+
/// attestation-data root, so the extracted winner is independent of arrival or
471+
/// insertion order (leanSpec #1181). This matches the leanSpec
472+
/// `location == "signatures"` checker, which folds `attestation_signatures`
473+
/// keeping each validator's canonical-precedence winner.
475474
fn extract_latest_attestations(&self) -> HashMap<u64, AttestationData> {
475+
let mut ordered: Vec<(&H256, &GossipDataEntry)> = self.data.iter().collect();
476+
// Descending by (slot, data_root): the larger tuple is the canonical winner.
477+
ordered.sort_unstable_by(|a, b| (b.1.data.slot, b.0).cmp(&(a.1.data.slot, a.0)));
478+
476479
let mut result: HashMap<u64, AttestationData> = HashMap::new();
477-
for data_root in &self.order {
478-
let Some(entry) = self.data.get(data_root) else {
479-
continue;
480-
};
480+
for (_data_root, entry) in ordered {
481481
for &vid in entry.signatures.keys() {
482-
let should_update = result
483-
.get(&vid)
484-
.is_none_or(|existing| existing.slot < entry.data.slot);
485-
if should_update {
486-
result.insert(vid, entry.data.clone());
487-
}
482+
// Descending order means the first vote seen for a validator wins.
483+
result.entry(vid).or_insert_with(|| entry.data.clone());
488484
}
489485
}
490486
result
@@ -2329,51 +2325,58 @@ mod tests {
23292325
}
23302326

23312327
/// When two aggregations share `slot` but disagree on the target
2332-
/// (same-slot equivocation), the *first inserted* aggregation must win for
2333-
/// the validators that participate in both. The fork-choice spec test
2334-
/// `test_same_slot_equivocating_attesters_count_once` depends on this.
2335-
/// HashMap iteration would make this RandomState-seeded and flaky.
2328+
/// (same-slot equivocation), the vote with the larger canonical
2329+
/// attestation-data root must win for the validators present in both,
2330+
/// regardless of arrival/insertion order. This is the deterministic
2331+
/// fork-choice tiebreak (leanSpec #1181): it makes the extracted head a pure
2332+
/// function of pool contents, so two nodes that see the same votes in
2333+
/// different orders agree on the same head.
23362334
#[test]
2337-
fn extract_latest_attestations_first_inserted_wins_on_slot_tie() {
2335+
fn extract_latest_attestations_canonical_root_wins_on_slot_tie() {
23382336
let target_a = H256([0xaa; 32]);
23392337
let target_b = H256([0xbb; 32]);
23402338
let data_a = make_att_data_for_target(3, target_a);
23412339
let data_b = make_att_data_for_target(3, target_b);
2342-
assert_ne!(data_a.hash_tree_root(), data_b.hash_tree_root());
2343-
2344-
// Order 1: A then B → validators 0,1 (in both) must see A.
2345-
let mut buf = PayloadBuffer::new(10);
2346-
buf.push(
2347-
HashedAttestationData::new(data_a.clone()),
2348-
make_proof_for_validators(&[0, 1, 2]),
2349-
);
2350-
buf.push(
2351-
HashedAttestationData::new(data_b.clone()),
2352-
make_proof_for_validators(&[0, 1, 3, 4]),
2353-
);
2354-
let extracted = buf.extract_latest_attestations();
2355-
assert_eq!(extracted[&0].target.root, target_a);
2356-
assert_eq!(extracted[&1].target.root, target_a);
2357-
assert_eq!(extracted[&2].target.root, target_a);
2358-
assert_eq!(extracted[&3].target.root, target_b);
2359-
assert_eq!(extracted[&4].target.root, target_b);
2360-
2361-
// Order 2: B then A → validators 0,1 must now see B.
2362-
let mut buf = PayloadBuffer::new(10);
2363-
buf.push(
2364-
HashedAttestationData::new(data_b),
2365-
make_proof_for_validators(&[0, 1, 3, 4]),
2366-
);
2367-
buf.push(
2368-
HashedAttestationData::new(data_a),
2369-
make_proof_for_validators(&[0, 1, 2]),
2370-
);
2371-
let extracted = buf.extract_latest_attestations();
2372-
assert_eq!(extracted[&0].target.root, target_b);
2373-
assert_eq!(extracted[&1].target.root, target_b);
2374-
assert_eq!(extracted[&2].target.root, target_a);
2375-
assert_eq!(extracted[&3].target.root, target_b);
2376-
assert_eq!(extracted[&4].target.root, target_b);
2340+
// The pool keys the tie on `hash_tree_root(data)`, not on the target root.
2341+
let root_a = data_a.hash_tree_root();
2342+
let root_b = data_b.hash_tree_root();
2343+
assert_ne!(root_a, root_b);
2344+
2345+
// The larger canonical root is the deterministic winner for shared voters.
2346+
let winner_target = if root_a > root_b { target_a } else { target_b };
2347+
2348+
// Deliver the same equivocating votes in both arrival orders; the winner
2349+
// for validators present in both aggregations (0, 1) must be identical.
2350+
for insert_b_first in [false, true] {
2351+
let mut buf = PayloadBuffer::new(10);
2352+
if insert_b_first {
2353+
buf.push(
2354+
HashedAttestationData::new(data_b.clone()),
2355+
make_proof_for_validators(&[0, 1, 3, 4]),
2356+
);
2357+
buf.push(
2358+
HashedAttestationData::new(data_a.clone()),
2359+
make_proof_for_validators(&[0, 1, 2]),
2360+
);
2361+
} else {
2362+
buf.push(
2363+
HashedAttestationData::new(data_a.clone()),
2364+
make_proof_for_validators(&[0, 1, 2]),
2365+
);
2366+
buf.push(
2367+
HashedAttestationData::new(data_b.clone()),
2368+
make_proof_for_validators(&[0, 1, 3, 4]),
2369+
);
2370+
}
2371+
let extracted = buf.extract_latest_attestations();
2372+
// Shared validators: deterministic canonical winner, independent of order.
2373+
assert_eq!(extracted[&0].target.root, winner_target);
2374+
assert_eq!(extracted[&1].target.root, winner_target);
2375+
// Exclusive validators keep their only vote.
2376+
assert_eq!(extracted[&2].target.root, target_a);
2377+
assert_eq!(extracted[&3].target.root, target_b);
2378+
assert_eq!(extracted[&4].target.root, target_b);
2379+
}
23772380
}
23782381

23792382
/// `drain` must hand back entries in insertion order so that

0 commit comments

Comments
 (0)