Ensure /eth/v2/beacon/pool/attestations honors committee_index

[eserilev]

Issue Addressed

#7294

Proposed Changes

Fix the filtering logic so that we actually filter by committee index for both Base and Electra attestations.

Added a tiny optimization when calculating committee_index to prevent unneeded memory allocations

Added a regression test

[Copilot]

Copilot reviewed 5 out of 5 changed files in this pull request and generated no comments.

Comments suppressed due to low confidence (4)

consensus/types/src/attestation.rs:296

• [nitpick] Ensure that returning only the first active committee bit is intentional. If multiple bits can be active, verify that using find to return only the first one matches the expected behavior.

.iter()

beacon_node/operation_pool/src/lib.rs:676

• Ensure that all invocations of get_filtered_attestations have been updated to supply the additional committee_index parameter correctly to avoid potential runtime issues.

F: Fn(&AttestationData, Option<u64>) -> bool,

beacon_node/http_api/src/lib.rs:2005

• [nitpick] Review the conditional logic for committee_index filtering to ensure that attestations with a None committee index are correctly excluded when a committee filter is provided; consider refactoring for improved clarity.

&& query.committee_index.is_none_or(|index| {

beacon_node/operation_pool/src/attestation_storage.rs:122

• [nitpick] Verify that using self.data.index for Base attestations produces the intended committee index, aligning with the processing logic for Electra attestations to maintain consistency.

pub fn committee_index(&self) -> Option<u64> {

[michaelsproul]

I think we need to account for aggregation here. Attestations in the op pool can cover multiple committee indices and if any of these match our query index, we should return that attestation.

The aggregation happens in aggregate_across_committees, which persists the aggregates in the attestation storage:

lighthouse/beacon_node/operation_pool/src/attestation_storage.rs

Line 335 in bf955c7

pub fn aggregate_across_committees(&mut self, checkpoint_key: CheckpointKey) {

We could use the committee_indices vec and check committee_indices.contains(&query.committee_index)? We should also add a test covering this case (might need to call aggregate_across_committees to prompt aggregation).

(We should maybe remove the committee_index methods here so we don't accidentally misuse them)

[eserilev]

One of the issues with out test harness is that we only have attestations across a single committee. so the aggregation isnt doing much. I've added it anyways, but I had to make a field public which isnt the best solution

We should clean this up and get the test harness to create attestations across more than a single committee. I just didnt want to block v7 testing because of this. Let me know what you think

[michaelsproul]

Oh yeah good point. We can come back to test this more thoroughly, I've opened an issue:

• Test getAttestations API with multiple committees/aggregation #7312