Fix attestations not getting added to the aggregation pool

[pawanjay176]

Issue Addressed

N/A

Proposed Changes

On receiving unaggregated attestations from the network, lighthouse sets the should_process variable on attestations depending on if we have a connected validator having aggregation duties for the same slot .
For should_process == false, we only validate and forward the attestation, otherwise, we add it to the naive_aggregation_pool so that we can aggregate them.

The should_process calculation happens in the subnet service where it checks if the duty slot and subnet exists in a delay map.

lighthouse/beacon_node/network/src/subnet_service/attestation_subnets.rs

Lines 390 to 393 in 3058b96

	tracked_vals.contains_key(&ExactSubnet {
	subnet_id: subnet,
	slot: attestation.data.slot,
	})

The bug is that during insertion into the delay map, we use the default expiration delay of a single slot duration(12s).

Since we implemented #4806 to reduce subscription spam, we don't send subscriptions every slot before the duty, so we get a situation where an entry into the delay map gets evicted before it has been read.

Hence, the should_process_attestation function almost always returns false unless import-all-attestations flag is turned on, leading to Lighthouse producing bad quality aggregate attestations.

[michaelsproul]

Fix looks good to me. I will add this to v5.2.0 for testing!

[pawanjay176]

@michaelsproul do you think it would be worth it to run a few testnet nodes without --import-all-attestations to observe the beacon_processor_unaggregated_attestation_verified_total metric?
I checked a holesky node(with validators attached) running without import-all-attestations and the metric was flat at 0 even when gossip_attestation work events were hovering around 500.

[michaelsproul]

Yeah I'm down to try running some nodes with --import-all-attestations.

[AgeManning]

Yep, nice find. Looks good to me.

The duration is likely to be updated multiple times, but it should do what we expect here.

[michaelsproul]

@Mergifyio queue

[mergify]

queue

✅ The pull request has been merged automatically

The pull request has been merged automatically at fb790de