Handle some errors from hasSeenBit and insertSeenBit differently

[rkapka]

What type of PR is this?

Change

What does this PR do? Why is it needed?

Problem

During the Holesky incident, we saw a lot errors about attestation bitlists having different lengths.

Number of attestations in the pool growing over time

{"error":"bitlists are different lengths","message":"Could not delete expired unaggregated attestation","prefix":"pool/attestations","severity":"ERROR"}

The above error results in the pool not removing old attestations:

Not being able to pack attestations into blocks

"error":"rpc error: code = Internal desc = Could not get attestations to pack into block: could not get unaggregated attestations: bitlists are different lengths","message":"Could not pack deposits and attestations","prefix":"rpc/validator","severity":"ERROR"

Other

Could not prepare attestations for fork choice error=bitlists are different lengths

ERROR could not aggregate unaggregated attestations error=could not get aggregation bits: bitlists are different lengths

time="2024-05-14 11:49:50" level=error msg="could not prune attestations from pool" error="bitlists are different lengths" prefix=blockchain

Solution

It is currently not known how we can end up in a state where committee bits of attestations have different lengths. This PR attempts to reduce the severity of negative effects of being in such a state.

• hasSeenBit returns an error inside UnaggregatedAttestations --> log the error and ignore the attestation
• insertSeenBit returns an error inside DeleteUnaggregatedAttestation --> log the error and continue (attestation will be deleted)
• hasSeenBit returns an error inside DeleteSeenUnaggregatedAttestations --> log the error and mark the attestation as seen (attestation will be deleted)

Acknowledgements

• I have read CONTRIBUTING.md.
• I have included a uniquely named changelog fragment file.
• I have added a description to this PR with sufficient context for reviewers to understand this PR.

[terencechain]

What about DeleteAggregatedAttestation?

It seems to me another way of doing this is the following, I'm not sure if we have considered it?
insertSeenBit(att ethpb.Att) error -> insertSeenBit(att ethpb.Att)
hasSeenBit(att ethpb.Att) (bool, error) -> hasSeenBit(att ethpb.Att) bool

[potuz]

LGTM

[rkapka]

@terencechain There are other usages of both functions where we discontinue on error