From a7edeaa03df2c883ad4526a5df189d5a37f624f6 Mon Sep 17 00:00:00 2001
From: Tumas <tumas@grandine.io>
Date: Fri, 6 Dec 2024 02:09:53 +0200
Subject: [PATCH] iterate over packing results and collect attestation with
 disjoint committee bits before computing on chain aggregate

---
 block_producer/src/block_producer.rs | 53 +++++++++++++++++++---------
 1 file changed, 37 insertions(+), 16 deletions(-)

diff --git a/block_producer/src/block_producer.rs b/block_producer/src/block_producer.rs
index 4c182413..90329b2d 100644
--- a/block_producer/src/block_producer.rs
+++ b/block_producer/src/block_producer.rs
@@ -75,13 +75,13 @@ use types::{
     phase0::{
         consts::{FAR_FUTURE_EPOCH, GENESIS_SLOT},
         containers::{
-            Attestation, AttesterSlashing as Phase0AttesterSlashing,
+            Attestation, AttestationData, AttesterSlashing as Phase0AttesterSlashing,
             BeaconBlock as Phase0BeaconBlock, BeaconBlockBody as Phase0BeaconBlockBody, Deposit,
             Eth1Data, ProposerSlashing, SignedVoluntaryExit,
         },
         primitives::{
-            DepositIndex, Epoch, ExecutionAddress, ExecutionBlockHash, Slot, Uint256,
-            ValidatorIndex, H256,
+            CommitteeIndex, DepositIndex, Epoch, ExecutionAddress, ExecutionBlockHash, Slot,
+            Uint256, ValidatorIndex, H256,
         },
     },
     preset::{Preset, SyncSubcommitteeSize},
@@ -847,28 +847,49 @@ impl<P: Preset, W: Wait> BlockBuildContext<P, W> {
                 {
                     ContiguousList::default()
                 } else {
-                    let attestations = attestations
-                        .into_iter()
-                        .filter_map(|attestation| {
+                    // Store results in a vec to preserve insertion order and thus the results of the packing algorithm
+                    let mut results: Vec<(
+                        AttestationData,
+                        HashSet<CommitteeIndex>,
+                        Vec<ElectraAttestation<P>>,
+                    )> = Vec::new();
+
+                    for (electra_attestation, committee_index) in
+                        attestations.into_iter().filter_map(|attestation| {
+                            let committee_index = attestation.data.index;
+
                             match operation_pools::convert_to_electra_attestation(attestation) {
-                                Ok(electra_attestation) => Some(electra_attestation),
+                                Ok(electra_attestation) => {
+                                    Some((electra_attestation, committee_index))
+                                }
                                 Err(error) => {
                                     warn!("unable to convert to electra attestation: {error:?}");
                                     None
                                 }
                             }
                         })
-                        .chunk_by(|attestation| {
-                            (
-                                attestation.data,
-                                misc::get_committee_indices::<P>(attestation.committee_bits).next(),
-                            )
-                        });
+                    {
+                        if let Some((_, indices, attestations)) =
+                            results.iter_mut().find(|(data, indices, _)| {
+                                *data == electra_attestation.data
+                                    && !indices.contains(&committee_index)
+                            })
+                        {
+                            indices.insert(committee_index);
+                            attestations.push(electra_attestation);
+                        } else {
+                            results.push((
+                                electra_attestation.data,
+                                HashSet::from([committee_index]),
+                                vec![electra_attestation],
+                            ))
+                        }
+                    }
 
-                    let attestations = attestations
+                    let attestations = results
                         .into_iter()
-                        .filter_map(|(_, attestations)| {
-                            match Self::compute_on_chain_aggregate(attestations) {
+                        .filter_map(|(_, _, attestations)| {
+                            match Self::compute_on_chain_aggregate(attestations.into_iter()) {
                                 Ok(electra_aggregate) => Some(electra_aggregate),
                                 Err(error) => {
                                     warn!("unable to compute on chain aggregate: {error:?}");