Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged
merged 10 commits into from
Jul 4, 2024
Merged
47 changes: 23 additions & 24 deletions blob/service.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import (
"errors"
"fmt"
"math"
"slices"
"sync"

sdkmath "cosmossdk.io/math"
Expand Down Expand Up @@ -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.
vgonkivs marked this conversation as resolved.
Show resolved Hide resolved
func (s *Service) GetAll(ctx context.Context, height uint64, namespaces []share.Namespace) ([]*Blob, error) {
header, err := s.headerGetter(ctx, height)
if err != nil {
Expand All @@ -173,40 +181,31 @@ 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
switch {
case err == nil:
log.Infow("retrieved blobs", "height", height, "total", len(blobs))
resultBlobs[i] = blobs
case errors.Is(err, ErrBlobNotFound):
default:
log.Debugf("getting blobs for namespace(%s): %v", namespace.String(), err)
resultErr[i] = err
vgonkivs marked this conversation as resolved.
Show resolved Hide resolved
}

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
vgonkivs marked this conversation as resolved.
Show resolved Hide resolved
}

// Included verifies that the blob was included in a specific height.
Expand Down
22 changes: 13 additions & 9 deletions blob/service_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -16,9 +16,11 @@ import (
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"
Expand Down Expand Up @@ -132,7 +134,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) {
Expand All @@ -141,7 +145,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()))
Expand Down Expand Up @@ -293,17 +296,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.Empty(t, err)
vgonkivs marked this conversation as resolved.
Show resolved Hide resolved
},
},
{
Expand Down Expand Up @@ -522,8 +525,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) {
Expand Down
10 changes: 9 additions & 1 deletion nodebuilder/blob/blob.go
Original file line number Diff line number Diff line change
Expand Up @@ -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,
vgonkivs marked this conversation as resolved.
Show resolved Hide resolved
// 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)
Expand Down
4 changes: 4 additions & 0 deletions nodebuilder/tests/blob_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
},
},
{
Expand Down
Loading