fix(syncing): handle incorrect da response (#2713)

<!--
Please read and fill out this form before submitting your PR.

Please make sure you have reviewed our contributors guide before
submitting your
first PR.

NOTE: PR titles should follow semantic commits:
https://www.conventionalcommits.org/en/v1.0.0/
-->

## Overview

Noticed when trying to sync from da using a wrong working endpoint.
Not all errors were handled. 

<!-- 
Please provide an explanation of the PR, including the appropriate
context,
background, goal, and rationale. If there is an issue with this
information,
please provide a tl;dr and link the issue. 

Ex: Closes #<issue number>
-->

Author: julienrbrt

block/internal/syncing/da_retriever.go

@@ -5,7 +5,6 @@ import (
 	"context"
 	"errors"
 	"fmt"
-	"strings"
 	"time"
 
 	"github.com/rs/zerolog"
@@ -72,10 +71,6 @@ func (r *DARetriever) RetrieveFromDA(ctx context.Context, daHeight uint64) ([]co
 
 	blobsResp, err := r.fetchBlobs(ctx, daHeight)
 	if err != nil {
-		if strings.Contains(err.Error(), coreda.ErrHeightFromFuture.Error()) {
-			return nil, fmt.Errorf("%w: height from future", coreda.ErrHeightFromFuture)
-		}
-
 		return nil, err
 	}
 
@@ -84,11 +79,6 @@ func (r *DARetriever) RetrieveFromDA(ctx context.Context, daHeight uint64) ([]co
 		return nil, err
 	}
 
-	if blobsResp.Code == coreda.StatusNotFound {
-		r.logger.Debug().Uint64("da_height", daHeight).Msg("no blob data found")
-		return nil, coreda.ErrBlobNotFound
-	}
-
 	r.logger.Debug().Int("blobs", len(blobsResp.Data)).Uint64("da_height", daHeight).Msg("retrieved blob data")
 	return r.processBlobs(ctx, blobsResp.Data, daHeight), nil
 }
@@ -107,13 +97,15 @@ func (r *DARetriever) fetchBlobs(ctx context.Context, daHeight uint64) (coreda.R
 
 	// Validate responses
 	headerErr := r.validateBlobResponse(headerRes, daHeight)
-	dataErr := r.validateBlobResponse(dataRes, daHeight)
+	// ignoring error not found, as data can have data
+	if headerErr != nil && !errors.Is(headerErr, coreda.ErrBlobNotFound) {
+		return headerRes, headerErr
+	}
 
-	// Handle errors
-	if errors.Is(headerErr, coreda.ErrHeightFromFuture) || errors.Is(dataErr, coreda.ErrHeightFromFuture) {
-		return coreda.ResultRetrieve{
-			BaseResult: coreda.BaseResult{Code: coreda.StatusHeightFromFuture},
-		}, fmt.Errorf("%w: height from future", coreda.ErrHeightFromFuture)
+	dataErr := r.validateBlobResponse(dataRes, daHeight)
+	// ignoring error not found, as header can have data
+	if dataErr != nil && !errors.Is(dataErr, coreda.ErrBlobNotFound) {
+		return dataRes, dataErr
 	}
 
 	// Combine successful results
@@ -127,28 +119,35 @@ func (r *DARetriever) fetchBlobs(ctx context.Context, daHeight uint64) (coreda.R
 
 	if headerRes.Code == coreda.StatusSuccess {
 		combinedResult.Data = append(combinedResult.Data, headerRes.Data...)
-		if len(headerRes.IDs) > 0 {
-			combinedResult.IDs = append(combinedResult.IDs, headerRes.IDs...)
-		}
+		combinedResult.IDs = append(combinedResult.IDs, headerRes.IDs...)
 	}
 
 	if dataRes.Code == coreda.StatusSuccess {
 		combinedResult.Data = append(combinedResult.Data, dataRes.Data...)
-		if len(dataRes.IDs) > 0 {
-			combinedResult.IDs = append(combinedResult.IDs, dataRes.IDs...)
-		}
+		combinedResult.IDs = append(combinedResult.IDs, dataRes.IDs...)
+	}
+
+	// Re-throw error not found if both were not found.
+	if len(combinedResult.Data) == 0 && len(combinedResult.IDs) == 0 {
+		r.logger.Debug().Uint64("da_height", daHeight).Msg("no blob data found")
+		combinedResult.Code = coreda.StatusNotFound
+		combinedResult.Message = coreda.ErrBlobNotFound.Error()
+		return combinedResult, coreda.ErrBlobNotFound
 	}
 
 	return combinedResult, nil
 }
 
 // validateBlobResponse validates a blob response from DA layer
+// those are the only error code returned by da.RetrieveWithHelpers
 func (r *DARetriever) validateBlobResponse(res coreda.ResultRetrieve, daHeight uint64) error {
 	switch res.Code {
 	case coreda.StatusError:
 		return fmt.Errorf("DA retrieval failed: %s", res.Message)
 	case coreda.StatusHeightFromFuture:
 		return fmt.Errorf("%w: height from future", coreda.ErrHeightFromFuture)
+	case coreda.StatusNotFound:
+		return fmt.Errorf("%w: blob not found", coreda.ErrBlobNotFound)
 	case coreda.StatusSuccess:
 		r.logger.Debug().Uint64("da_height", daHeight).Msg("successfully retrieved from DA")
 		return nil

block/internal/syncing/da_retriever_test.go

@@ -69,11 +69,28 @@ func makeSignedDataBytes(t *testing.T, chainID string, height uint64, proposer [
 	return bin, sd
 }
 
+func TestDARetriever_RetrieveFromDA_Invalid(t *testing.T) {
+	ds := dssync.MutexWrap(datastore.NewMapDatastore())
+	st := store.New(ds)
+	cm, err := cache.NewManager(config.DefaultConfig(), st, zerolog.Nop())
+	assert.NoError(t, err)
+
+	mockDA := testmocks.NewMockDA(t)
+
+	mockDA.EXPECT().GetIDs(mock.Anything, mock.Anything, mock.Anything).
+		Return(nil, errors.New("just invalid")).Maybe()
+
+	r := NewDARetriever(mockDA, cm, config.DefaultConfig(), genesis.Genesis{}, common.DefaultBlockOptions(), zerolog.Nop())
+	events, err := r.RetrieveFromDA(context.Background(), 42)
+	assert.Error(t, err)
+	assert.Len(t, events, 0)
+}
+
 func TestDARetriever_RetrieveFromDA_NotFound(t *testing.T) {
 	ds := dssync.MutexWrap(datastore.NewMapDatastore())
 	st := store.New(ds)
 	cm, err := cache.NewManager(config.DefaultConfig(), st, zerolog.Nop())
-	require.NoError(t, err)
+	assert.NoError(t, err)
 
 	mockDA := testmocks.NewMockDA(t)
 
@@ -83,7 +100,7 @@ func TestDARetriever_RetrieveFromDA_NotFound(t *testing.T) {
 
 	r := NewDARetriever(mockDA, cm, config.DefaultConfig(), genesis.Genesis{}, common.DefaultBlockOptions(), zerolog.Nop())
 	events, err := r.RetrieveFromDA(context.Background(), 42)
-	require.Error(t, err, coreda.ErrBlobNotFound)
+	assert.True(t, errors.Is(err, coreda.ErrBlobNotFound))
 	assert.Len(t, events, 0)
 }
 

block/internal/syncing/syncer.go

@@ -247,7 +247,6 @@ func (s *Syncer) syncLoop() {
 	lastDataHeight := initialHeight
 
 	// Backoff control when DA replies with errors
-	var hffDelay time.Duration
 	var nextDARequestAt time.Time
 
 	blockTicker := time.NewTicker(s.config.Node.BlockTime.Duration)
@@ -280,16 +279,16 @@ func (s *Syncer) syncLoop() {
 					nextDARequestAt = time.Time{}
 				} else {
 					// Back off exactly by DA block time to avoid overloading
-					hffDelay = s.config.DA.BlockTime.Duration
-					if hffDelay <= 0 {
-						hffDelay = 2 * time.Second
+					backoffDelay := s.config.DA.BlockTime.Duration
+					if backoffDelay <= 0 {
+						backoffDelay = 2 * time.Second
 					}
-					nextDARequestAt = now.Add(hffDelay)
+					nextDARequestAt = now.Add(backoffDelay)
 
 					if s.isHeightFromFutureError(err) {
-						s.logger.Debug().Dur("delay", hffDelay).Uint64("da_height", daHeight).Msg("height from future; backing off DA requests")
+						s.logger.Debug().Dur("delay", backoffDelay).Uint64("da_height", daHeight).Msg("height from future; backing off DA requests")
 					} else {
-						s.logger.Error().Err(err).Dur("delay", hffDelay).Uint64("da_height", daHeight).Msg("failed to retrieve from DA; backing off DA requests")
+						s.logger.Error().Err(err).Dur("delay", backoffDelay).Uint64("da_height", daHeight).Msg("failed to retrieve from DA; backing off DA requests")
 					}
 				}
 			} else {