Repository: ObolNetwork/charon

Issue:
Fetcher fetch only attestation data with committee index 0
# 🎯 Problem to be solved
Currently our fetcher fetches attestation data for all committee indices that need to attest. This is done, because some VCs are requesting attester data for the specific committee indices and not 0. Recently though they have started fixing that (check #3913 ). As a start, it would be nice if we add a feature flag that makes fetcher fetch only attestation with committee index 0 for the users that are already using VCs. This will improve couple of things:
- Reduce the amount of the requests to the BN;
- Reduce memory usage of Charon;
- Avoid issues where we have different head for the same slot, that caused issues before (most likely it will result in more head vote misses though).

# 🛠️ Proposed solution
- [ ] Add a new feature flag. Or simply a flag? If it we choose a feature flag we know we will move it to stable soon, when all VCs fix the issue.
- [ ] Set BN to only ask for attestation data with committee index 0.
- [ ] Do not save twice attestation data in DB, as we currently do. We do save attestation data for the actual attestation committee index + committee index 0. No reason to do that anymore.

# 🧪 Tests
- [ ] Tested by new automated unit/integration/smoke tests
- [ ] Manually tested on core team/canary/test clusters
- [ ] Manually tested on local compose simnet


Hints from issue comments:

Solution patch:
diff --git a/app/app.go b/app/app.go
index b860c664..ac79a686 100644
--- a/app/app.go
+++ b/app/app.go
@@ -553,7 +553,7 @@ func wireCoreWorkflow(ctx context.Context, life *lifecycle.Manager, conf Config,
 
 	electraSlot := eth2p0.Slot(uint64(forkSchedule[eth2wrap.Electra].Epoch) * slotsPerEpoch)
 
-	fetch, err := fetcher.New(eth2Cl, feeRecipientFunc, conf.BuilderAPI, graffitiBuilder, electraSlot)
+	fetch, err := fetcher.New(eth2Cl, feeRecipientFunc, conf.BuilderAPI, graffitiBuilder, electraSlot, featureset.Enabled(featureset.FetchOnlyCommIdx0))
 	if err != nil {
 		return err
 	}
diff --git a/app/featureset/featureset.go b/app/featureset/featureset.go
index d0e89561..c1244a8a 100644
--- a/app/featureset/featureset.go
+++ b/app/featureset/featureset.go
@@ -63,6 +63,9 @@ const (
 
 	// QUIC enables the QUIC transport protocol in libp2p.
 	QUIC = "quic"
+
+	// FetchOnlyCommIdx0 enables querying the beacon node for attestation data only for committee index 0.
+	FetchOnlyCommIdx0 = "fetch_only_commidx_0"
 )
 
 var (
@@ -79,6 +82,7 @@ var (
 		AttestationInclusion: statusAlpha,
 		ProposalTimeout:      statusAlpha,
 		QUIC:                 statusAlpha,
+		FetchOnlyCommIdx0:    statusAlpha,
 		// Add all features and there status here.
 	}
 
diff --git a/core/fetcher/fetcher.go b/core/fetcher/fetcher.go
index 024cbbb6..a55149ca 100644
--- a/core/fetcher/fetcher.go
+++ b/core/fetcher/fetcher.go
@@ -21,26 +21,28 @@ import (
 )
 
 // New returns a new fetcher instance.
-func New(eth2Cl eth2wrap.Client, feeRecipientFunc func(core.PubKey) string, builderEnabled bool, graffitiBuilder *GraffitiBuilder, electraSlot eth2p0.Slot) (*Fetcher, error) {
+func New(eth2Cl eth2wrap.Client, feeRecipientFunc func(core.PubKey) string, builderEnabled bool, graffitiBuilder *GraffitiBuilder, electraSlot eth2p0.Slot, fetchOnlyCommIdx0 bool) (*Fetcher, error) {
 	return &Fetcher{
-		eth2Cl:           eth2Cl,
-		feeRecipientFunc: feeRecipientFunc,
-		builderEnabled:   builderEnabled,
-		graffitiBuilder:  graffitiBuilder,
-		electraSlot:      electraSlot,
+		eth2Cl:            eth2Cl,
+		feeRecipientFunc:  feeRecipientFunc,
+		builderEnabled:    builderEnabled,
+		graffitiBuilder:   graffitiBuilder,
+		electraSlot:       electraSlot,
+		fetchOnlyCommIdx0: fetchOnlyCommIdx0,
 	}, nil
 }
 
 // Fetcher fetches proposed duty data.
 type Fetcher struct {
-	eth2Cl           eth2wrap.Client
-	feeRecipientFunc func(core.PubKey) string
-	subs             []func(context.Context, core.Duty, core.UnsignedDataSet) error
-	aggSigDBFunc     func(context.Context, core.Duty, core.PubKey) (core.SignedData, error)
-	awaitAttDataFunc func(ctx context.Context, slot, commIdx uint64) (*eth2p0.AttestationData, error)
-	builderEnabled   bool
-	graffitiBuilder  *GraffitiBuilder
-	electraSlot      eth2p0.Slot
+	eth2Cl            eth2wrap.Client
+	feeRecipientFunc  func(core.PubKey) string
+	subs              []func(context.Context, core.Duty, core.UnsignedDataSet) error
+	aggSigDBFunc      func(context.Context, core.Duty, core.PubKey) (core.SignedData, error)
+	awaitAttDataFunc  func(ctx context.Context, slot, commIdx uint64) (*eth2p0.AttestationData, error)
+	builderEnabled    bool
+	graffitiBuilder   *GraffitiBuilder
+	electraSlot       eth2p0.Slot
+	fetchOnlyCommIdx0 bool
 }
 
 // Subscribe registers a callback for fetched duties.
@@ -128,18 +130,15 @@ func (f *Fetcher) fetchAttesterData(ctx context.Context, slot uint64, defSet cor
 
 		commIdx := attDuty.CommitteeIndex
 
-		// TODO(kalo):
 		// Attestation data for Electra is not bound by committee index.
 		// Committee index is still persisted in the request but should be set to 0.
 		// https://ethereum.github.io/beacon-APIs/#/Validator/produceAttestationData
 		// However, some validator clients are still sending attestation_data requests for each committee index.
 		// Because of that, we should continue asking for all + 0 committee indices for the ones that work correctly.
-		// In a future releases, when we know all validator clients ask only for committee index 0,
-		// we can start doing the same and uncomment the code block below.
-
-		// if slot >= uint64(f.electraSlot) {
-		// 	commIdx = 0
-		// }
+		// After all VCs start asking for committee index 0, we should change the default scenario to that.
+		if slot >= uint64(f.electraSlot) && f.fetchOnlyCommIdx0 {
+			commIdx = 0
+		}
 
 		eth2AttData, ok := dataByCommIdx[commIdx]
 		if !ok {



Test patch:
diff --git a/core/fetcher/fetcher_test.go b/core/fetcher/fetcher_test.go
index 93d575bd..3d7200ff 100644
--- a/core/fetcher/fetcher_test.go
+++ b/core/fetcher/fetcher_test.go
@@ -553,7 +553,7 @@ func TestFetchSyncContribution(t *testing.T) {
 func mustCreateFetcher(t *testing.T, bmock beaconmock.Mock) *fetcher.Fetcher {
 	t.Helper()
 
-	fetch, err := fetcher.New(bmock, nil, true, &fetcher.GraffitiBuilder{}, 5)
+	fetch, err := fetcher.New(bmock, nil, true, &fetcher.GraffitiBuilder{}, 5, false)
 	require.NoError(t, err)
 
 	return fetch
@@ -564,7 +564,7 @@ func mustCreateFetcherWithAddressAndGraffiti(t *testing.T, bmock beaconmock.Mock
 
 	fetch, err := fetcher.New(bmock, func(core.PubKey) string {
 		return addr
-	}, true, graffitiBuilder, 5)
+	}, true, graffitiBuilder, 5, false)
 	require.NoError(t, err)
 
 	return fetch