diff --git a/blob/service.go b/blob/service.go index 0b43493e27..0876a45a0c 100644 --- a/blob/service.go +++ b/blob/service.go @@ -193,8 +193,7 @@ func (s *Service) getByCommitment( namespacedShares, err := s.shareGetter.GetSharesByNamespace(ctx, header.DAH, nID) if err != nil { - if errors.Is(err, share.ErrNamespaceNotFound) || - errors.Is(err, share.ErrNotFound) { + if errors.Is(err, share.ErrNotFound) { err = ErrBlobNotFound } return nil, nil, err diff --git a/share/availability/light/availability_test.go b/share/availability/light/availability_test.go index 4980af031e..ca72596816 100644 --- a/share/availability/light/availability_test.go +++ b/share/availability/light/availability_test.go @@ -141,8 +141,9 @@ func TestService_GetSharesByNamespaceNotFound(t *testing.T) { getter, root := GetterWithRandSquare(t, 1) root.RowRoots = nil - _, err := getter.GetSharesByNamespace(context.Background(), root, namespace.RandomNamespace().Bytes()) - assert.ErrorIs(t, err, share.ErrNamespaceNotFound) + emptyShares, err := getter.GetSharesByNamespace(context.Background(), root, namespace.RandomNamespace().Bytes()) + require.NoError(t, err) + require.Empty(t, emptyShares.Flatten()) } func BenchmarkService_GetSharesByNamespace(b *testing.B) { diff --git a/share/getter.go b/share/getter.go index b157e12f8d..a4bb22995e 100644 --- a/share/getter.go +++ b/share/getter.go @@ -17,13 +17,6 @@ import ( var ( // ErrNotFound is used to indicate that requested data could not be found. ErrNotFound = errors.New("share: data not found") - // ErrNamespaceNotFound is returned by GetSharesByNamespace when data for the requested root does - // not include any shares from the given namespace. If the target namespace belongs to the - // namespace range of any rows of the requested root, the error will be returned along with - // collected non-inclusion proofs for those rows. It is the obligation of the requester to verify - // non-inclusion proofs for all rows that could possibly contain the target namespace by calling - // the Verify method. - ErrNamespaceNotFound = errors.New("share: namespace not found in data") ) // Getter interface provides a set of accessors for shares by the Root. @@ -39,8 +32,8 @@ type Getter interface { // GetSharesByNamespace gets all shares from an EDS within the given namespace. // Shares are returned in a row-by-row order if the namespace spans multiple rows. - // If data for the requested root does not include any shares from the given namespace - // ErrNamespaceNotFound will be returned along with non-inclusion proofs if there are any. + // Inclusion of returned data could be verified using Verify method on NamespacedShares. + // If no shares are found for target namespace non-inclusion could be also verified by calling Verify method. GetSharesByNamespace(context.Context, *Root, namespace.ID) (NamespacedShares, error) } diff --git a/share/getters/cascade.go b/share/getters/cascade.go index 1a0d8fb274..725a924e85 100644 --- a/share/getters/cascade.go +++ b/share/getters/cascade.go @@ -122,7 +122,7 @@ func cascadeGetters[V any]( getCtx, cancel := ctxWithSplitTimeout(ctx, len(getters)-i, 0) val, getErr := get(getCtx, getter) cancel() - if getErr == nil || errors.Is(getErr, share.ErrNamespaceNotFound) { + if getErr == nil { return val, getErr } diff --git a/share/getters/getter_test.go b/share/getters/getter_test.go index c9bf82031a..20b7e25191 100644 --- a/share/getters/getter_test.go +++ b/share/getters/getter_test.go @@ -138,8 +138,9 @@ func TestStoreGetter(t *testing.T) { // nid not found nID = make([]byte, namespace.NamespaceSize) - _, err = sg.GetSharesByNamespace(ctx, &dah, nID) - require.ErrorIs(t, err, share.ErrNamespaceNotFound) + emptyShares, err := sg.GetSharesByNamespace(ctx, &dah, nID) + require.NoError(t, err) + require.Empty(t, emptyShares.Flatten()) // root not found root := share.Root{} @@ -216,13 +217,14 @@ func TestIPLDGetter(t *testing.T) { // nid not found nID = make([]byte, namespace.NamespaceSize) emptyShares, err := sg.GetSharesByNamespace(ctx, &dah, nID) - require.ErrorIs(t, err, share.ErrNamespaceNotFound) + require.NoError(t, err) require.Nil(t, emptyShares) // nid doesnt exist in root root := share.Root{} - _, err = sg.GetSharesByNamespace(ctx, &root, nID) - require.ErrorIs(t, err, share.ErrNamespaceNotFound) + emptyShares, err = sg.GetSharesByNamespace(ctx, &root, nID) + require.NoError(t, err) + require.Empty(t, emptyShares.Flatten()) }) } diff --git a/share/getters/shrex.go b/share/getters/shrex.go index b0aa790103..99da2f8758 100644 --- a/share/getters/shrex.go +++ b/share/getters/shrex.go @@ -191,7 +191,7 @@ func (sg *ShrexGetter) GetSharesByNamespace( // verify that the namespace could exist inside the roots before starting network requests roots := filterRootsByNamespace(root, id) if len(roots) == 0 { - return nil, share.ErrNamespaceNotFound + return nil, nil } for { @@ -217,15 +217,16 @@ func (sg *ShrexGetter) GetSharesByNamespace( nd, getErr := sg.ndClient.RequestND(reqCtx, root, id, peer) cancel() switch { - case getErr == nil, errors.Is(getErr, share.ErrNamespaceNotFound): + case getErr == nil: // both inclusion and non-inclusion cases needs verification if verErr := nd.Verify(root, id); verErr != nil { + getErr = verErr setStatus(peers.ResultBlacklistPeer) break } setStatus(peers.ResultNoop) sg.metrics.recordNDAttempt(ctx, attempt, true) - return nd, getErr + return nd, nil case errors.Is(getErr, context.DeadlineExceeded), errors.Is(getErr, context.Canceled): setStatus(peers.ResultCooldownPeer) diff --git a/share/getters/shrex_test.go b/share/getters/shrex_test.go index cb6efc3422..4026692381 100644 --- a/share/getters/shrex_test.go +++ b/share/getters/shrex_test.go @@ -108,11 +108,11 @@ func TestShrexGetter(t *testing.T) { // check for namespace to be between max and min namespace in root require.Len(t, filterRootsByNamespace(&dah, maxNID), 1) - sh, err := getter.GetSharesByNamespace(ctx, &dah, nID) - require.ErrorIs(t, err, share.ErrNamespaceNotFound) + emptyShares, err := getter.GetSharesByNamespace(ctx, &dah, nID) + require.NoError(t, err) // no shares should be returned - require.Len(t, sh.Flatten(), 0) - require.Nil(t, sh.Verify(&dah, nID)) + require.Empty(t, emptyShares.Flatten()) + require.Nil(t, emptyShares.Verify(&dah, nID)) }) t.Run("ND_namespace_not_in_root", func(t *testing.T) { @@ -134,11 +134,11 @@ func TestShrexGetter(t *testing.T) { // check for namespace to be not in root require.Len(t, filterRootsByNamespace(&dah, nID), 0) - sh, err := getter.GetSharesByNamespace(ctx, &dah, nID) - require.ErrorIs(t, err, share.ErrNamespaceNotFound) + emptyShares, err := getter.GetSharesByNamespace(ctx, &dah, nID) + require.NoError(t, err) // no shares should be returned - require.Len(t, sh.Flatten(), 0) - require.Nil(t, sh.Verify(&dah, nID)) + require.Empty(t, emptyShares.Flatten()) + require.Nil(t, emptyShares.Verify(&dah, nID)) }) t.Run("EDS_Available", func(t *testing.T) { diff --git a/share/getters/store.go b/share/getters/store.go index e76a1c19cc..f39f41fc80 100644 --- a/share/getters/store.go +++ b/share/getters/store.go @@ -122,9 +122,9 @@ func (sg *StoreGetter) GetSharesByNamespace( // wrap the read-only CAR blockstore in a getter blockGetter := eds.NewBlockGetter(bs) shares, err = collectSharesByNamespace(ctx, blockGetter, root, nID) - if err != nil && !errors.Is(err, share.ErrNamespaceNotFound) { + if err != nil { return nil, fmt.Errorf("getter/store: failed to retrieve shares by namespace: %w", err) } - return shares, err + return shares, nil } diff --git a/share/getters/utils.go b/share/getters/utils.go index 116acf2795..79438204bd 100644 --- a/share/getters/utils.go +++ b/share/getters/utils.go @@ -59,7 +59,7 @@ func collectSharesByNamespace( rootCIDs := filterRootsByNamespace(root, nID) if len(rootCIDs) == 0 { - return nil, share.ErrNamespaceNotFound + return nil, nil } errGroup, ctx := errgroup.WithContext(ctx) @@ -83,13 +83,6 @@ func collectSharesByNamespace( if err := errGroup.Wait(); err != nil { return nil, err } - - // return ErrNamespaceNotFound along with collected proofs if no shares are found for the - // namespace.ID - if len(rootCIDs) == 1 && len(shares[0].Shares) == 0 { - return shares, share.ErrNamespaceNotFound - } - return shares, nil } diff --git a/share/p2p/shrexnd/client.go b/share/p2p/shrexnd/client.go index 72c445913d..f591a0970d 100644 --- a/share/p2p/shrexnd/client.go +++ b/share/p2p/shrexnd/client.go @@ -55,8 +55,8 @@ func (c *Client) RequestND( peer peer.ID, ) (share.NamespacedShares, error) { shares, err := c.doRequest(ctx, root, nID, peer) - if err == nil || errors.Is(err, share.ErrNamespaceNotFound) { - return shares, err + if err == nil { + return shares, nil } if errors.Is(err, context.DeadlineExceeded) || errors.Is(err, context.Canceled) { c.metrics.ObserveRequests(ctx, 1, p2p.StatusTimeout) @@ -119,24 +119,11 @@ func (c *Client) doRequest( return nil, fmt.Errorf("client-nd: reading response: %w", err) } - err = c.statusToErr(ctx, resp.Status) - if errors.Is(err, share.ErrNamespaceNotFound) { - shares := convertToNonInclusionProofs(resp.Rows) - return shares, err - } - if err != nil { - return nil, fmt.Errorf("client-nd: response code is not OK: %w", err) - } - - shares, err := convertToNamespacedShares(resp.Rows) - if err != nil { - return nil, fmt.Errorf("client-nd: converting response to shares: %w", err) - } - return shares, nil + return c.convertResponse(ctx, resp) } // convertToNamespacedShares converts proto Rows to share.NamespacedShares -func convertToNamespacedShares(rows []*pb.Row) (share.NamespacedShares, error) { +func convertToNamespacedShares(rows []*pb.Row) share.NamespacedShares { shares := make([]share.NamespacedRow, 0, len(rows)) for _, row := range rows { var proof *nmt.Proof @@ -155,7 +142,7 @@ func convertToNamespacedShares(rows []*pb.Row) (share.NamespacedShares, error) { Proof: proof, }) } - return shares, nil + return shares } func convertToNonInclusionProofs(rows []*pb.Row) share.NamespacedShares { @@ -203,22 +190,22 @@ func (c *Client) setStreamDeadlines(ctx context.Context, stream network.Stream) } } -func (c *Client) statusToErr(ctx context.Context, code pb.StatusCode) error { - switch code { +func (c *Client) convertResponse(ctx context.Context, resp pb.GetSharesByNamespaceResponse) (share.NamespacedShares, error) { + switch resp.Status { case pb.StatusCode_OK: c.metrics.ObserveRequests(ctx, 1, p2p.StatusSuccess) - return nil + return convertToNamespacedShares(resp.Rows), nil + case pb.StatusCode_NAMESPACE_NOT_FOUND: + return convertToNonInclusionProofs(resp.Rows), nil case pb.StatusCode_NOT_FOUND: c.metrics.ObserveRequests(ctx, 1, p2p.StatusNotFound) - return p2p.ErrNotFound - case pb.StatusCode_NAMESPACE_NOT_FOUND: - return share.ErrNamespaceNotFound + return nil, p2p.ErrNotFound case pb.StatusCode_INVALID: log.Debug("client-nd: invalid request") fallthrough case pb.StatusCode_INTERNAL: fallthrough default: - return p2p.ErrInvalidResponse + return nil, p2p.ErrInvalidResponse } } diff --git a/share/p2p/shrexnd/exchange_test.go b/share/p2p/shrexnd/exchange_test.go index 8c5a132fdc..25528595d7 100644 --- a/share/p2p/shrexnd/exchange_test.go +++ b/share/p2p/shrexnd/exchange_test.go @@ -49,8 +49,9 @@ func TestExchange_RequestND_NotFound(t *testing.T) { require.NoError(t, edsStore.Put(ctx, dah.Hash(), eds)) randNID := dah.RowRoots[(len(dah.RowRoots)-1)/2][:namespace.NamespaceSize] - _, err := client.RequestND(ctx, &dah, randNID, server.host.ID()) - require.ErrorIs(t, err, share.ErrNamespaceNotFound) + emptyShares, err := client.RequestND(ctx, &dah, randNID, server.host.ID()) + require.NoError(t, err) + require.Empty(t, emptyShares.Flatten()) }) } @@ -119,7 +120,7 @@ func (m notFoundGetter) GetEDS( func (m notFoundGetter) GetSharesByNamespace( _ context.Context, _ *share.Root, _ nmtnamespace.ID, ) (share.NamespacedShares, error) { - return nil, share.ErrNamespaceNotFound + return nil, nil } func newStore(t *testing.T) *eds.Store { diff --git a/share/p2p/shrexnd/server.go b/share/p2p/shrexnd/server.go index 2e08cf0cb1..72c5231eff 100644 --- a/share/p2p/shrexnd/server.go +++ b/share/p2p/shrexnd/server.go @@ -136,17 +136,13 @@ func (srv *Server) handleNamespacedData(ctx context.Context, stream network.Stre logger.Warn("server: nd not found") srv.respondNotFoundError(ctx, logger, stream) return - case errors.Is(err, share.ErrNamespaceNotFound): - resp := namespacedSharesToResponse(shares, false) - srv.respond(ctx, logger, stream, resp) - return case err != nil: logger.Errorw("server: retrieving shares", "err", err) srv.respondInternalError(ctx, logger, stream) return } - resp := namespacedSharesToResponse(shares, true) + resp := namespacedSharesToResponse(shares) srv.respond(ctx, logger, stream, resp) } @@ -181,26 +177,24 @@ func (srv *Server) respondInternalError(ctx context.Context, } // namespacedSharesToResponse encodes shares into proto and sends it to client with OK status code -func namespacedSharesToResponse(shares share.NamespacedShares, found bool) *pb.GetSharesByNamespaceResponse { +func namespacedSharesToResponse(shares share.NamespacedShares) *pb.GetSharesByNamespaceResponse { rows := make([]*pb.Row, 0, len(shares)) for _, row := range shares { - proof := &pb.Proof{ - Start: int64(row.Proof.Start()), - End: int64(row.Proof.End()), - Nodes: row.Proof.Nodes(), - Hashleaf: row.Proof.LeafHash(), - } - row := &pb.Row{ Shares: row.Shares, - Proof: proof, + Proof: &pb.Proof{ + Start: int64(row.Proof.Start()), + End: int64(row.Proof.End()), + Nodes: row.Proof.Nodes(), + Hashleaf: row.Proof.LeafHash(), + }, } rows = append(rows, row) } status := pb.StatusCode_OK - if !found { + if len(shares) == 0 || (len(shares) == 1 && len(shares[0].Shares) == 0) { status = pb.StatusCode_NAMESPACE_NOT_FOUND }