misc(blob/service)!: return empty values in case blobs were not found

blob/service.go

@@ -5,6 +5,7 @@ import (
 	"errors"
 	"fmt"
 	"math"
+	"slices"
 	"sync"
 
 	sdkmath "cosmossdk.io/math"
@@ -163,7 +164,14 @@ func (s *Service) GetProof(
 }
 
 // GetAll returns all blobs under the given namespaces at the given height.
-// GetAll can return blobs and an error in case if some requests failed.
+// If all blobs were found without any errors, the user will receive a list of blobs.
+// If the BlobService couldn't find any blobs under the requested namespaces,
+// the user will receive an empty list of blobs along with an empty error.
+// If some of the requested namespaces were not found, the user will receive all the found blobs and an empty error.
+// If there were internal errors during some of the requests,
+// the user will receive all found blobs along with a combined error message.
+//
+// All blobs will preserve the order of the namespaces that were requested.
 func (s *Service) GetAll(ctx context.Context, height uint64, namespaces []share.Namespace) ([]*Blob, error) {
 	header, err := s.headerGetter(ctx, height)
 	if err != nil {
@@ -173,40 +181,30 @@ func (s *Service) GetAll(ctx context.Context, height uint64, namespaces []share.
 	var (
 		resultBlobs = make([][]*Blob, len(namespaces))
 		resultErr   = make([]error, len(namespaces))
+		wg          = sync.WaitGroup{}
 	)
-
-	for _, namespace := range namespaces {
-		log.Debugw("performing GetAll request", "namespace", namespace.String(), "height", height)
-	}
-
-	wg := sync.WaitGroup{}
 	for i, namespace := range namespaces {
 		wg.Add(1)
 		go func(i int, namespace share.Namespace) {
+			log.Debugw("retrieving all blobs from", "namespace", namespace.String(), "height", height)
 			defer wg.Done()
+
 			blobs, err := s.getBlobs(ctx, namespace, header)
-			if err != nil {
-				resultErr[i] = fmt.Errorf("getting blobs for namespace(%s): %w", namespace.String(), err)
-				return
+			if err != nil && !errors.Is(err, ErrBlobNotFound) {
+				log.Debugf("getting blobs for namespace(%s): %v", namespace.String(), err)
+				resultErr[i] = err
+			}
+			if len(blobs) > 0 {
+				log.Infow("retrieved blobs", "height", height, "total", len(blobs))
+				resultBlobs[i] = blobs
 			}
-
-			log.Debugw("receiving blobs", "height", height, "total", len(blobs))
-			resultBlobs[i] = blobs
 		}(i, namespace)
 	}
 	wg.Wait()
 
-	blobs := make([]*Blob, 0)
-	for _, resBlobs := range resultBlobs {
-		if len(resBlobs) > 0 {
-			blobs = append(blobs, resBlobs...)
-		}
-	}
-
-	if len(blobs) == 0 {
-		resultErr = append(resultErr, ErrBlobNotFound)
-	}
-	return blobs, errors.Join(resultErr...)
+	blobs := slices.Concat(resultBlobs...)
+	err = errors.Join(resultErr...)
+	return blobs, err
 }
 
 // Included verifies that the blob was included in a specific height.
@@ -413,8 +411,5 @@ func (s *Service) getBlobs(
 	sharesParser := &parser{verifyFn: verifyFn}
 
 	_, _, err = s.retrieve(ctx, header.Height(), namespace, sharesParser)
-	if len(blobs) == 0 {
-		return nil, ErrBlobNotFound
-	}
-	return blobs, nil
+	return blobs, err
 }

blob/service_test.go

@@ -4,24 +4,28 @@ import (
 	"bytes"
 	"context"
 	"encoding/json"
+	"errors"
 	"fmt"
 	"sort"
 	"testing"
 	"time"
 
+	"github.com/golang/mock/gomock"
 	ds "github.com/ipfs/go-datastore"
 	ds_sync "github.com/ipfs/go-datastore/sync"
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
 	tmrand "github.com/tendermint/tendermint/libs/rand"
 
 	"github.com/celestiaorg/celestia-app/pkg/appconsts"
+	appns "github.com/celestiaorg/celestia-app/pkg/namespace"
 	"github.com/celestiaorg/celestia-app/pkg/shares"
 	"github.com/celestiaorg/go-header/store"
 
 	"github.com/celestiaorg/celestia-node/blob/blobtest"
 	"github.com/celestiaorg/celestia-node/header"
 	"github.com/celestiaorg/celestia-node/header/headertest"
+	shareMock "github.com/celestiaorg/celestia-node/nodebuilder/share/mocks"
 	"github.com/celestiaorg/celestia-node/share"
 	"github.com/celestiaorg/celestia-node/share/getters"
 	"github.com/celestiaorg/celestia-node/share/ipld"
@@ -132,7 +136,9 @@ func TestBlobService_Get(t *testing.T) {
 		{
 			name: "get all with different namespaces",
 			doFn: func() (interface{}, error) {
-				b, err := service.GetAll(ctx, 1, []share.Namespace{blobs0[0].Namespace(), blobs0[1].Namespace()})
+				nid, err := share.NewBlobNamespaceV0(tmrand.Bytes(7))
+				require.NoError(t, err)
+				b, err := service.GetAll(ctx, 1, []share.Namespace{blobs0[0].Namespace(), nid, blobs0[1].Namespace()})
 				return b, err
 			},
 			expectedResult: func(res interface{}, err error) {
@@ -141,7 +147,6 @@ func TestBlobService_Get(t *testing.T) {
 				blobs, ok := res.([]*Blob)
 				assert.True(t, ok)
 				assert.NotEmpty(t, blobs)
-
 				assert.Len(t, blobs, 2)
 				// check the order
 				require.True(t, bytes.Equal(blobs[0].Namespace(), blobs0[0].Namespace()))
@@ -293,17 +298,17 @@ func TestBlobService_Get(t *testing.T) {
 			},
 		},
 		{
-			name: "get all not found",
+			name: "empty result and err when blobs were not found ",
 			doFn: func() (interface{}, error) {
-				namespace := share.Namespace(tmrand.Bytes(share.NamespaceSize))
-				return service.GetAll(ctx, 1, []share.Namespace{namespace})
+				nid, err := share.NewBlobNamespaceV0(tmrand.Bytes(appns.NamespaceVersionZeroIDSize))
+				require.NoError(t, err)
+				return service.GetAll(ctx, 1, []share.Namespace{nid})
 			},
 			expectedResult: func(i interface{}, err error) {
 				blobs, ok := i.([]*Blob)
 				require.True(t, ok)
 				assert.Empty(t, blobs)
-				require.Error(t, err)
-				require.ErrorIs(t, err, ErrBlobNotFound)
+				assert.Nil(t, err)
 			},
 		},
 		{
@@ -325,6 +330,24 @@ func TestBlobService_Get(t *testing.T) {
 				require.NoError(t, proof.equal(*newProof))
 			},
 		},
+		{
+			name: "internal error",
+			doFn: func() (interface{}, error) {
+				ctrl := gomock.NewController(t)
+				shareGetterMock := shareMock.NewMockModule(ctrl)
+				service.shareGetter = shareGetterMock
+				shareGetterMock.EXPECT().
+					GetSharesByNamespace(gomock.Any(), gomock.Any(), gomock.Any()).
+					Return(nil, errors.New("internal error"))
+				return service.GetAll(ctx, 1, []share.Namespace{blobs0[0].Namespace()})
+			},
+			expectedResult: func(res interface{}, err error) {
+				blobs, ok := res.([]*Blob)
+				require.True(t, ok)
+				assert.Empty(t, blobs)
+				require.Error(t, err)
+			},
+		},
 	}
 
 	for _, tt := range test {
@@ -522,8 +545,9 @@ func TestAllPaddingSharesInEDS(t *testing.T) {
 	}
 
 	service := NewService(nil, getters.NewIPLDGetter(bs), fn)
-	_, err = service.GetAll(ctx, 1, []share.Namespace{nid})
-	require.Error(t, err)
+	newBlobs, err := service.GetAll(ctx, 1, []share.Namespace{nid})
+	require.NoError(t, err)
+	assert.Empty(t, newBlobs)
 }
 
 func TestSkipPaddingsAndRetrieveBlob(t *testing.T) {

nodebuilder/blob/blob.go

@@ -19,7 +19,15 @@ type Module interface {
 	Submit(_ context.Context, _ []*blob.Blob, _ blob.GasPrice) (height uint64, _ error)
 	// Get retrieves the blob by commitment under the given namespace and height.
 	Get(_ context.Context, height uint64, _ share.Namespace, _ blob.Commitment) (*blob.Blob, error)
-	// GetAll returns all blobs at the given height under the given namespaces.
+	// GetAll returns all blobs under the given namespaces at the given height.
+	// If all blobs were found without any errors, the user will receive a list of blobs.
+	// If the BlobService couldn't find any blobs under the requested namespaces,
+	// the user will receive an empty list of blobs along with an empty error.
+	// If some of the requested namespaces were not found, the user will receive all the found blobs and an empty error.
+	// If there were internal errors during some of the requests,
+	// the user will receive all found blobs along with a combined error message.
+	//
+	// All blobs will preserve the order of the namespaces that were requested.
 	GetAll(_ context.Context, height uint64, _ []share.Namespace) ([]*blob.Blob, error)
 	// GetProof retrieves proofs in the given namespaces at the given height by commitment.
 	GetProof(_ context.Context, height uint64, _ share.Namespace, _ blob.Commitment) (*blob.Proof, error)

nodebuilder/tests/blob_test.go

@@ -127,6 +127,10 @@ func TestBlobModule(t *testing.T) {
 				assert.Nil(t, b)
 				require.Error(t, err)
 				require.ErrorContains(t, err, blob.ErrBlobNotFound.Error())
+
+				blobs, err := fullClient.Blob.GetAll(ctx, height, []share.Namespace{newBlob.Namespace()})
+				require.NoError(t, err)
+				assert.Empty(t, blobs)
 			},
 		},
 		{