From bde983c21f9b71ad4752db26b89a1d6678bf5326 Mon Sep 17 00:00:00 2001
From: Pablo Deymonnaz <pdeymon@fi.uba.ar>
Date: Mon, 24 Aug 2026 14:55:41 -0300
Subject: [PATCH] fix(blockchain): make proof selection tie-breaking
 deterministic

extend_proofs_greedily kept its remaining candidate proofs in a
HashSet<usize> and picked the best-coverage proof with max_by_key over
the set's randomized iteration order, so equal-coverage ties were broken
arbitrarily per process: the same store state could produce blocks with
different aggregation bits from one run to the next.

Iterate candidates in index order and break coverage ties toward the
lowest index (pool insertion order), making block building reproducible
for a given pool. The regression test builds a six-way coverage tie, so
an arbitrary order cannot reproduce pool order by luck; against the
previous code it fails on most runs.

Found while building an offline block-building benchmark, whose
same-seed determinism check reported differing block roots across runs
of an identical workload.
---
 crates/blockchain/src/block_builder.rs | 39 +++++++++++++++++++++++---
 1 file changed, 35 insertions(+), 4 deletions(-)

diff --git a/crates/blockchain/src/block_builder.rs b/crates/blockchain/src/block_builder.rs
index f064b3d7..6df8cda1 100644
--- a/crates/blockchain/src/block_builder.rs
+++ b/crates/blockchain/src/block_builder.rs
@@ -806,10 +806,14 @@ fn extend_proofs_greedily(
     }
 
     let mut covered: HashSet<u64> = HashSet::new();
-    let mut remaining_indices: HashSet<usize> = (0..proofs.len()).collect();
+    let mut remaining_indices: Vec<usize> = (0..proofs.len()).collect();
 
     while !remaining_indices.is_empty() {
-        // Pick proof covering the most uncovered validators (count only, no allocation)
+        // Pick proof covering the most uncovered validators (count only, no
+        // allocation). Coverage ties break to the lowest index (pool insertion
+        // order): a HashSet here would let hash-iteration order pick an
+        // arbitrary equal-coverage winner, making the built block's
+        // aggregation bits differ from run to run.
         let best = remaining_indices
             .iter()
             .map(|&idx| {
@@ -819,7 +823,7 @@ fn extend_proofs_greedily(
                     .count();
                 (idx, count)
             })
-            .max_by_key(|&(_, count)| count);
+            .max_by_key(|&(idx, count)| (count, Reverse(idx)));
 
         let Some((best_idx, best_count)) = best else {
             break;
@@ -846,7 +850,7 @@ fn extend_proofs_greedily(
 
         covered.extend(new_covered);
         selected.push((att, proof.clone()));
-        remaining_indices.remove(&best_idx);
+        remaining_indices.retain(|&idx| idx != best_idx);
     }
 }
 
@@ -1958,4 +1962,31 @@ mod tests {
         let covered: HashSet<u64> = selected[0].1.participant_indices().collect();
         assert_eq!(covered, HashSet::from([0, 1, 2, 3]));
     }
+
+    /// Equal-coverage proofs must be selected in pool order, so that the same
+    /// pool always yields the same block. Iterating a `HashSet` of candidate
+    /// indices made the winner depend on hash order, which varies per process.
+    #[test]
+    fn extend_proofs_greedily_breaks_coverage_ties_by_pool_order() {
+        let data = make_att_data(1);
+
+        // Six proofs of identical coverage size, disjoint so that every round
+        // is again a six-way tie: selection order is decided purely by the
+        // tie-break, and must follow the order they sit in the pool. Six of
+        // them rather than two so an arbitrary order cannot match pool order
+        // by luck (1 in 720 rather than 1 in 2).
+        let proofs: Vec<SingleMessageAggregate> = (0..6)
+            .map(|group| SingleMessageAggregate::empty(make_bits(&[group * 2, group * 2 + 1])))
+            .collect();
+
+        let mut selected = Vec::new();
+        extend_proofs_greedily(&proofs, &mut selected, &data);
+
+        let order: Vec<Vec<u64>> = selected
+            .iter()
+            .map(|(_, proof)| proof.participant_indices().collect())
+            .collect();
+        let pool_order: Vec<Vec<u64>> = (0..6).map(|g| vec![g * 2, g * 2 + 1]).collect();
+        assert_eq!(order, pool_order);
+    }
 }