refactor: prefer blob_sidecars endpoint to avoid recomputing KZG proofs

Switch to prioritizing /eth/v1/beacon/blob_sidecars over /eth/v1/beacon/blobs
since the sidecars endpoint provides KZG commitments and proofs directly,
avoiding unnecessary computation.

Author: niran

archiver/service/archiver.go

@@ -125,8 +125,9 @@ func (a *Archiver) Stop(ctx context.Context) error {
 // perform any validation of the blobs, it assumes a trusted beacon node. See:
 // https://github.com/base/blob-archiver/issues/4.
 //
-// The function prefers the /eth/v1/beacon/blobs endpoint but falls back to /eth/v1/beacon/blob_sidecars
-// if the blobs endpoint fails.
+// The function prefers the /eth/v1/beacon/blob_sidecars endpoint but falls back to /eth/v1/beacon/blobs
+// if the blob sidecars endpoint fails. This avoids recomputing KZG commitments and proofs when they're
+// available from the beacon node.
 func (a *Archiver) persistBlobsForBlockToS3(ctx context.Context, blockIdentifier string, overwrite bool) (*v1.BeaconBlockHeader, bool, error) {
 	currentHeader, err := a.beaconClient.BeaconBlockHeader(ctx, &api.BeaconBlockHeaderOpts{
 		Block: blockIdentifier,
@@ -148,38 +149,44 @@ func (a *Archiver) persistBlobsForBlockToS3(ctx context.Context, blockIdentifier
 		return currentHeader.Data, true, nil
 	}
 
-	// Try the new blobs endpoint first
+	// Try the blob sidecars endpoint first to get commitments and proofs directly
 	var blobSidecarData []*deneb.BlobSidecar
-	blobs, err := a.beaconClient.Blobs(ctx, &api.BlobsOpts{
+	blobSidecarsResp, err := a.beaconClient.BlobSidecars(ctx, &api.BlobSidecarsOpts{
 		Block: currentHeader.Data.Root.String(),
 	})
 
-	if err == nil && blobs != nil && blobs.Data != nil && len(blobs.Data) > 0 {
-		// Successfully fetched blobs, compute commitments and proofs from blob data
-		a.log.Debug("fetched blobs from blobs endpoint, computing KZG commitments and proofs", "count", len(blobs.Data))
-
-		var err error
-		blobSidecarData, err = blobsToSidecars(blobs.Data, currentHeader.Data)
-		if err != nil {
-			a.log.Error("failed to compute KZG commitments and proofs for blobs", "err", err)
-			return nil, false, err
-		}
+	if err == nil && blobSidecarsResp != nil && blobSidecarsResp.Data != nil {
+		// Successfully fetched blob sidecars with KZG commitments and proofs
+		a.log.Debug("fetched blob sidecars from blob_sidecars endpoint", "count", len(blobSidecarsResp.Data))
+		blobSidecarData = blobSidecarsResp.Data
 	} else {
-		// Fall back to blob sidecars endpoint
+		// Fall back to blobs endpoint and compute commitments and proofs
 		if err != nil {
-			a.log.Debug("blobs endpoint failed, falling back to blob sidecars", "err", err)
+			a.log.Debug("blob sidecars endpoint failed, falling back to blobs", "err", err)
 		}
 
-		blobSidecarsResp, fallbackErr := a.beaconClient.BlobSidecars(ctx, &api.BlobSidecarsOpts{
+		blobs, fallbackErr := a.beaconClient.Blobs(ctx, &api.BlobsOpts{
 			Block: currentHeader.Data.Root.String(),
 		})
 
 		if fallbackErr != nil {
-			a.log.Error("failed to fetch blob sidecars", "err", fallbackErr)
+			a.log.Error("failed to fetch blobs", "err", fallbackErr)
 			return nil, false, fallbackErr
 		}
 
-		blobSidecarData = blobSidecarsResp.Data
+		if blobs == nil || blobs.Data == nil {
+			a.log.Error("blobs endpoint returned nil data")
+			return nil, false, errors.New("blobs endpoint returned nil data")
+		}
+
+		a.log.Debug("fetched blobs from blobs endpoint, computing KZG commitments and proofs", "count", len(blobs.Data))
+
+		var computeErr error
+		blobSidecarData, computeErr = blobsToSidecars(blobs.Data, currentHeader.Data)
+		if computeErr != nil {
+			a.log.Error("failed to compute KZG commitments and proofs for blobs", "err", computeErr)
+			return nil, false, computeErr
+		}
 	}
 
 	a.log.Debug("fetched blob sidecars", "count", len(blobSidecarData))

archiver/service/archiver_test.go

@@ -475,14 +475,14 @@ func TestArchiver_RearchiveRange(t *testing.T) {
 	require.Equal(t, fs.ReadOrFail(t, blobtest.Three).BlobSidecars.Data, beacon.SidecarsByBlock[blobtest.Three.String()])
 }
 
-func TestArchiver_FetchBlobs_FallbackToSidecars(t *testing.T) {
+func TestArchiver_FetchBlobSidecars_Success(t *testing.T) {
 	beacon := beacontest.NewDefaultStubBeaconClient(t)
-	beacon.FailBlobs = true // Make blobs endpoint fail
+	// FailSidecars = false (default) - blob sidecars endpoint should succeed
 	svc, fs := setup(t, beacon)
 
 	fs.CheckNotExistsOrFail(t, blobtest.One)
 
-	// Should fall back to blob sidecars endpoint
+	// Should use blob sidecars endpoint successfully
 	header, alreadyExists, err := svc.persistBlobsForBlockToS3(context.Background(), blobtest.One.String(), false)
 	require.False(t, alreadyExists)
 	require.NoError(t, err)
@@ -493,14 +493,14 @@ func TestArchiver_FetchBlobs_FallbackToSidecars(t *testing.T) {
 	require.Equal(t, len(beacon.SidecarsByBlock[blobtest.One.String()]), len(stored.BlobSidecars.Data))
 }
 
-func TestArchiver_FetchBlobs_Success(t *testing.T) {
+func TestArchiver_FetchBlobs_FallbackToBlobs(t *testing.T) {
 	beacon := beacontest.NewDefaultStubBeaconClient(t)
-	// FailBlobs = false (default) - blobs endpoint should succeed
+	beacon.FailSidecars = true // Make blob sidecars endpoint fail
 	svc, fs := setup(t, beacon)
 
 	fs.CheckNotExistsOrFail(t, blobtest.Two)
 
-	// Should use blobs endpoint successfully
+	// Should fall back to blobs endpoint and compute KZG commitments/proofs
 	header, alreadyExists, err := svc.persistBlobsForBlockToS3(context.Background(), blobtest.Two.String(), false)
 	require.False(t, alreadyExists)
 	require.NoError(t, err)
@@ -511,7 +511,7 @@ func TestArchiver_FetchBlobs_Success(t *testing.T) {
 	// Should have stored the correct number of blobs
 	require.Equal(t, len(beacon.SidecarsByBlock[blobtest.Two.String()]), len(stored.BlobSidecars.Data))
 
-	// Verify that KZG commitments and proofs were derived correctly
+	// Verify that KZG commitments and proofs were computed correctly from blob data
 	for i, storedSidecar := range stored.BlobSidecars.Data {
 		originalSidecar := beacon.SidecarsByBlock[blobtest.Two.String()][i]
 		// The blob data should match

common/beacon/beacontest/stub.go

@@ -17,7 +17,7 @@ import (
 type StubBeaconClient struct {
 	Headers         map[string]*v1.BeaconBlockHeader
 	SidecarsByBlock map[string][]*deneb.BlobSidecar
-	FailBlobs       bool
+	FailSidecars    bool
 }
 
 func (s *StubBeaconClient) BeaconBlockHeader(ctx context.Context, opts *api.BeaconBlockHeaderOpts) (*api.Response[*v1.BeaconBlockHeader], error) {
@@ -31,6 +31,10 @@ func (s *StubBeaconClient) BeaconBlockHeader(ctx context.Context, opts *api.Beac
 }
 
 func (s *StubBeaconClient) BlobSidecars(ctx context.Context, opts *api.BlobSidecarsOpts) (*api.Response[[]*deneb.BlobSidecar], error) {
+	if s.FailSidecars {
+		return nil, fmt.Errorf("blob sidecars endpoint unavailable")
+	}
+
 	blobs, found := s.SidecarsByBlock[opts.Block]
 	if !found {
 		return nil, fmt.Errorf("block not found")
@@ -42,10 +46,6 @@ func (s *StubBeaconClient) BlobSidecars(ctx context.Context, opts *api.BlobSidec
 
 // Blobs implements the BlobsProvider interface, converting sidecars to blobs
 func (s *StubBeaconClient) Blobs(ctx context.Context, opts *api.BlobsOpts) (*api.Response[v1.Blobs], error) {
-	if s.FailBlobs {
-		return nil, fmt.Errorf("blobs endpoint unavailable")
-	}
-
 	sidecars, found := s.SidecarsByBlock[opts.Block]
 	if !found {
 		return nil, fmt.Errorf("block not found")