*: fix attestation committee index

[KaloyanTanev]

During our testing of v1.5.0-rc1 and rc2, we found an issue with clusters that had Nimbus VCs. Upon closer look an incompatibility was found.

Pre electra, VCs upon asking for attestation data from the BN, they were required to specify the committee_index of the validator. Given that the committee index was removed from the signed data post electra, the endpoint staid the same, but it is now required to simply put committee_index=0. However, most VCs do not comply with that and continue asking for specific committee index. BNs on their hand, simply ignore this field and return the data.

On our end as Charon we store the unsigned attestation data in a map, where part of its key structure is the committee index itself. We continued asking for all committee indices in our fetcher, so we saved multiple entries as pre electra. This worked fine, as VCs were asking as well for specific committee indices. BNs on their end are caching the result, so the responses were almost instant.

Nimbus VC though was the only VC working correctly and asking only for attestation data for committee_index=0. This resulted in failures to fetch.

Example:

1. Fetcher of a leader non-Nimbus fetched for indices 1, 5, 10.
2. Nimbus VC asked for attestation data for index 0.
3. Charon of Nimbus VC had only for indices 1, 5, 10 and its request to fetch attestation data for index 0 timed out.

Given that some VCs ask for the specific committee indices and some ask for only 0, this fix keeps the existing fetcher logic to ask for all, but also adds committee index 0 in Charon's DB. This way we have duplicated data, but we ensure compatibility. Note that Charon's memory usage will increase slightly. Once we can confirm all VCs will ask only for committee index 0, we can remove the fetcher logic to ask for all committee indices and reduce Charon's memory usage by a lot.

Note that Lodestar had this enabled only for DVs. It should be fixed in their next release. It is likely the case is similar with other clients as well.

category: bug
ticket: none

[codecov]

Codecov Report

Attention: Patch coverage is 60.41667% with 38 lines in your changes missing coverage. Please review.

Project coverage is 54.13%. Comparing base (f48d03a) to head (2e506f9).
Report is 4 commits behind head on main.

Files with missing lines	Patch %	Lines
app/eth2wrap/utils.go	47.50%	16 Missing and 5 partials ⚠️
app/app.go	0.00%	11 Missing ⚠️
core/dutydb/memory.go	80.00%	4 Missing and 2 partials ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #3820      +/-   ##
==========================================
- Coverage   54.15%   54.13%   -0.03%     
==========================================
  Files         227      227              
  Lines       35735    35806      +71     
==========================================
+ Hits        19354    19383      +29     
- Misses      14254    14289      +35     
- Partials     2127     2134       +7     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[KaloyanTanev]

Note that this is currently not utilised. It will be once all VCs start asking. Then, if we are post electra, we can safely ask the BN only for 1 attestation data - with a committee index 0.

If there are strong objections not to keep it in the meantime, I'm OK with removing it.

[sonarqubecloud]

Quality Gate failed

Failed conditions
27.5% Duplication on New Code (required ≤ 15%)

See analysis details on SonarQube Cloud

[KaloyanTanev]

Sonarcloud is complaining about the repeated Endpoint: "/eth/v1/config/spec", in beaconmock.go, however, I don't see this as an issue at all... If it has to be refactored not to be duplicated, it will make it more unreadable.