Skip to content

Commit

Permalink
!refactor(api/share): use height in share module (#3870)
Browse files Browse the repository at this point in the history
  • Loading branch information
walldiss authored Oct 23, 2024
1 parent 01de405 commit cfb363c
Show file tree
Hide file tree
Showing 10 changed files with 80 additions and 122 deletions.
8 changes: 1 addition & 7 deletions api/gateway/availability.go
Original file line number Diff line number Diff line change
Expand Up @@ -27,13 +27,7 @@ func (h *Handler) handleHeightAvailabilityRequest(w http.ResponseWriter, r *http
return
}

header, err := h.header.GetByHeight(r.Context(), uint64(height))
if err != nil {
writeError(w, http.StatusInternalServerError, heightAvailabilityEndpoint, err)
return
}

err = h.share.SharesAvailable(r.Context(), header)
err = h.share.SharesAvailable(r.Context(), uint64(height))
switch {
case err == nil:
resp, err := json.Marshal(&AvailabilityResponse{Available: true})
Expand Down
7 changes: 1 addition & 6 deletions api/gateway/share.go
Original file line number Diff line number Diff line change
Expand Up @@ -93,12 +93,7 @@ func (h *Handler) getShares(
height uint64,
namespace libshare.Namespace,
) ([]libshare.Share, error) {
header, err := h.header.GetByHeight(ctx, height)
if err != nil {
return nil, err
}

shares, err := h.share.GetSharesByNamespace(ctx, header, namespace)
shares, err := h.share.GetSharesByNamespace(ctx, height, namespace)
if err != nil {
return nil, err
}
Expand Down
33 changes: 0 additions & 33 deletions nodebuilder/node_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -15,9 +15,7 @@ import (
collectormetricpb "go.opentelemetry.io/proto/otlp/collector/metrics/v1"
"google.golang.org/protobuf/proto"

"github.com/celestiaorg/celestia-node/header/headertest"
"github.com/celestiaorg/celestia-node/nodebuilder/node"
"github.com/celestiaorg/celestia-node/share"
)

func TestLifecycle(t *testing.T) {
Expand Down Expand Up @@ -125,34 +123,3 @@ func StartMockOtelCollectorHTTPServer(t *testing.T) (string, func()) {
server.EnableHTTP2 = true
return server.URL, server.Close
}

func TestEmptyBlockExists(t *testing.T) {
ctx, cancel := context.WithCancel(context.Background())
defer cancel()

test := []struct {
tp node.Type
}{
{tp: node.Bridge},
{tp: node.Full},
// technically doesn't need to be tested as a SharesAvailable call to
// light node short circuits on an empty EDS
{tp: node.Light},
}
for i, tt := range test {
t.Run(strconv.Itoa(i), func(t *testing.T) {
node := TestNode(t, tt.tp)
err := node.Start(ctx)
require.NoError(t, err)

// ensure an empty block exists in store

eh := headertest.RandExtendedHeaderWithRoot(t, share.EmptyEDSRoots())
err = node.ShareServ.SharesAvailable(ctx, eh)
require.NoError(t, err)

err = node.Stop(ctx)
require.NoError(t, err)
})
}
}
34 changes: 8 additions & 26 deletions nodebuilder/share/cmd/share.go
Original file line number Diff line number Diff line change
@@ -1,18 +1,14 @@
package cmd

import (
"context"
"encoding/hex"
"fmt"
"strconv"

"github.com/spf13/cobra"

libshare "github.com/celestiaorg/go-square/v2/share"

rpc "github.com/celestiaorg/celestia-node/api/rpc/client"
cmdnode "github.com/celestiaorg/celestia-node/cmd"
"github.com/celestiaorg/celestia-node/header"
)

func init() {
Expand Down Expand Up @@ -42,12 +38,12 @@ var sharesAvailableCmd = &cobra.Command{
}
defer client.Close()

eh, err := getExtendedHeaderFromCmdArg(cmd.Context(), client, args[0])
height, err := strconv.ParseUint(args[0], 10, 64)
if err != nil {
return err
}

err = client.Share.SharesAvailable(cmd.Context(), eh)
err = client.Share.SharesAvailable(cmd.Context(), height)
formatter := func(data interface{}) interface{} {
err, ok := data.(error)
available := false
Expand Down Expand Up @@ -79,7 +75,7 @@ var getSharesByNamespaceCmd = &cobra.Command{
}
defer client.Close()

eh, err := getExtendedHeaderFromCmdArg(cmd.Context(), client, args[0])
height, err := strconv.ParseUint(args[0], 10, 64)
if err != nil {
return err
}
Expand All @@ -89,7 +85,7 @@ var getSharesByNamespaceCmd = &cobra.Command{
return err
}

shares, err := client.Share.GetSharesByNamespace(cmd.Context(), eh, ns)
shares, err := client.Share.GetSharesByNamespace(cmd.Context(), height, ns)
return cmdnode.PrintOutput(shares, err, nil)
},
}
Expand All @@ -105,7 +101,7 @@ var getShare = &cobra.Command{
}
defer client.Close()

eh, err := getExtendedHeaderFromCmdArg(cmd.Context(), client, args[0])
height, err := strconv.ParseUint(args[0], 10, 64)
if err != nil {
return err
}
Expand All @@ -120,7 +116,7 @@ var getShare = &cobra.Command{
return err
}

s, err := client.Share.GetShare(cmd.Context(), eh, int(row), int(col))
s, err := client.Share.GetShare(cmd.Context(), height, int(row), int(col))

formatter := func(data interface{}) interface{} {
sh, ok := data.(libshare.Share)
Expand Down Expand Up @@ -153,26 +149,12 @@ var getEDS = &cobra.Command{
}
defer client.Close()

eh, err := getExtendedHeaderFromCmdArg(cmd.Context(), client, args[0])
height, err := strconv.ParseUint(args[0], 10, 64)
if err != nil {
return err
}

shares, err := client.Share.GetEDS(cmd.Context(), eh)
shares, err := client.Share.GetEDS(cmd.Context(), height)
return cmdnode.PrintOutput(shares, err, nil)
},
}

func getExtendedHeaderFromCmdArg(ctx context.Context, client *rpc.Client, arg string) (*header.ExtendedHeader, error) {
height, err := strconv.ParseUint(arg, 10, 64)
if err == nil {
return client.Header.GetByHeight(ctx, height)
}

hash, err := hex.DecodeString(arg)
if err != nil {
return nil, fmt.Errorf("can't parse the height/hash argument: %w", err)
}

return client.Header.GetByHash(ctx, hash)
}
9 changes: 4 additions & 5 deletions nodebuilder/share/mocks/api.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

73 changes: 46 additions & 27 deletions nodebuilder/share/share.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,6 @@ import (
libshare "github.com/celestiaorg/go-square/v2/share"
"github.com/celestiaorg/rsmt2d"

"github.com/celestiaorg/celestia-node/header"
headerServ "github.com/celestiaorg/celestia-node/nodebuilder/header"
"github.com/celestiaorg/celestia-node/share"
"github.com/celestiaorg/celestia-node/share/eds"
Expand Down Expand Up @@ -43,15 +42,15 @@ type GetRangeResult struct {
type Module interface {
// SharesAvailable subjectively validates if Shares committed to the given
// ExtendedHeader are available on the Network.
SharesAvailable(context.Context, *header.ExtendedHeader) error
SharesAvailable(ctx context.Context, height uint64) error
// GetShare gets a Share by coordinates in EDS.
GetShare(ctx context.Context, header *header.ExtendedHeader, row, col int) (libshare.Share, error)
GetShare(ctx context.Context, height uint64, row, col int) (libshare.Share, error)
// GetEDS gets the full EDS identified by the given extended header.
GetEDS(ctx context.Context, header *header.ExtendedHeader) (*rsmt2d.ExtendedDataSquare, error)
GetEDS(ctx context.Context, height uint64) (*rsmt2d.ExtendedDataSquare, error)
// 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.
GetSharesByNamespace(
ctx context.Context, header *header.ExtendedHeader, namespace libshare.Namespace,
ctx context.Context, height uint64, namespace libshare.Namespace,
) (shwap.NamespaceData, error)
// GetRange gets a list of shares and their corresponding proof.
GetRange(ctx context.Context, height uint64, start, end int) (*GetRangeResult, error)
Expand All @@ -60,19 +59,19 @@ type Module interface {
// API is a wrapper around Module for the RPC.
type API struct {
Internal struct {
SharesAvailable func(context.Context, *header.ExtendedHeader) error `perm:"read"`
SharesAvailable func(ctx context.Context, height uint64) error `perm:"read"`
GetShare func(
ctx context.Context,
header *header.ExtendedHeader,
height uint64,
row, col int,
) (libshare.Share, error) `perm:"read"`
GetEDS func(
ctx context.Context,
header *header.ExtendedHeader,
height uint64,
) (*rsmt2d.ExtendedDataSquare, error) `perm:"read"`
GetSharesByNamespace func(
ctx context.Context,
header *header.ExtendedHeader,
height uint64,
namespace libshare.Namespace,
) (shwap.NamespaceData, error) `perm:"read"`
GetRange func(
Expand All @@ -83,16 +82,16 @@ type API struct {
}
}

func (api *API) SharesAvailable(ctx context.Context, header *header.ExtendedHeader) error {
return api.Internal.SharesAvailable(ctx, header)
func (api *API) SharesAvailable(ctx context.Context, height uint64) error {
return api.Internal.SharesAvailable(ctx, height)
}

func (api *API) GetShare(ctx context.Context, header *header.ExtendedHeader, row, col int) (libshare.Share, error) {
return api.Internal.GetShare(ctx, header, row, col)
func (api *API) GetShare(ctx context.Context, height uint64, row, col int) (libshare.Share, error) {
return api.Internal.GetShare(ctx, height, row, col)
}

func (api *API) GetEDS(ctx context.Context, header *header.ExtendedHeader) (*rsmt2d.ExtendedDataSquare, error) {
return api.Internal.GetEDS(ctx, header)
func (api *API) GetEDS(ctx context.Context, height uint64) (*rsmt2d.ExtendedDataSquare, error) {
return api.Internal.GetEDS(ctx, height)
}

func (api *API) GetRange(ctx context.Context, height uint64, start, end int) (*GetRangeResult, error) {
Expand All @@ -101,28 +100,44 @@ func (api *API) GetRange(ctx context.Context, height uint64, start, end int) (*G

func (api *API) GetSharesByNamespace(
ctx context.Context,
header *header.ExtendedHeader,
height uint64,
namespace libshare.Namespace,
) (shwap.NamespaceData, error) {
return api.Internal.GetSharesByNamespace(ctx, header, namespace)
return api.Internal.GetSharesByNamespace(ctx, height, namespace)
}

type module struct {
shwap.Getter
share.Availability
hs headerServ.Module
getter shwap.Getter
avail share.Availability
hs headerServ.Module
}

func (m module) SharesAvailable(ctx context.Context, header *header.ExtendedHeader) error {
return m.Availability.SharesAvailable(ctx, header)
func (m module) GetShare(ctx context.Context, height uint64, row, col int) (libshare.Share, error) {
header, err := m.hs.GetByHeight(ctx, height)
if err != nil {
return libshare.Share{}, err
}
return m.getter.GetShare(ctx, header, row, col)
}

func (m module) GetRange(ctx context.Context, height uint64, start, end int) (*GetRangeResult, error) {
extendedHeader, err := m.hs.GetByHeight(ctx, height)
func (m module) GetEDS(ctx context.Context, height uint64) (*rsmt2d.ExtendedDataSquare, error) {
header, err := m.hs.GetByHeight(ctx, height)
if err != nil {
return nil, err
}
extendedDataSquare, err := m.GetEDS(ctx, extendedHeader)
return m.getter.GetEDS(ctx, header)
}

func (m module) SharesAvailable(ctx context.Context, height uint64) error {
header, err := m.hs.GetByHeight(ctx, height)
if err != nil {
return err
}
return m.avail.SharesAvailable(ctx, header)
}

func (m module) GetRange(ctx context.Context, height uint64, start, end int) (*GetRangeResult, error) {
extendedDataSquare, err := m.GetEDS(ctx, height)
if err != nil {
return nil, err
}
Expand All @@ -144,8 +159,12 @@ func (m module) GetRange(ctx context.Context, height uint64, start, end int) (*G

func (m module) GetSharesByNamespace(
ctx context.Context,
header *header.ExtendedHeader,
height uint64,
namespace libshare.Namespace,
) (shwap.NamespaceData, error) {
return m.Getter.GetSharesByNamespace(ctx, header, namespace)
header, err := m.hs.GetByHeight(ctx, height)
if err != nil {
return nil, err
}
return m.getter.GetSharesByNamespace(ctx, header, namespace)
}
12 changes: 7 additions & 5 deletions nodebuilder/tests/nd_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -72,9 +72,10 @@ func TestShrexNDFromLights(t *testing.T) {
ns, err := libshare.NewNamespaceFromBytes(namespace)
require.NoError(t, err)

expected, err := bridgeClient.Share.GetSharesByNamespace(reqCtx, h, ns)
height := h.Height()
expected, err := bridgeClient.Share.GetSharesByNamespace(reqCtx, height, ns)
require.NoError(t, err)
got, err := lightClient.Share.GetSharesByNamespace(reqCtx, h, ns)
got, err := lightClient.Share.GetSharesByNamespace(reqCtx, height, ns)
require.NoError(t, err)

require.True(t, len(got[0].Shares) > 0)
Expand Down Expand Up @@ -145,20 +146,21 @@ func TestShrexNDFromLightsWithBadFulls(t *testing.T) {

// ensure to fetch random namespace (not the reserved namespace)
namespace := h.DAH.RowRoots[1][:libshare.NamespaceSize]
height := h.Height()
ns, err := libshare.NewNamespaceFromBytes(namespace)
require.NoError(t, err)
expected, err := bridgeClient.Share.GetSharesByNamespace(reqCtx, h, ns)
expected, err := bridgeClient.Share.GetSharesByNamespace(reqCtx, height, ns)
require.NoError(t, err)
require.True(t, len(expected[0].Shares) > 0)

// choose a random full to test
fN := fulls[len(fulls)/2]
fnClient := getAdminClient(ctx, fN, t)
gotFull, err := fnClient.Share.GetSharesByNamespace(reqCtx, h, ns)
gotFull, err := fnClient.Share.GetSharesByNamespace(reqCtx, height, ns)
require.NoError(t, err)
require.True(t, len(gotFull[0].Shares) > 0)

gotLight, err := lightClient.Share.GetSharesByNamespace(reqCtx, h, ns)
gotLight, err := lightClient.Share.GetSharesByNamespace(reqCtx, height, ns)
require.NoError(t, err)
require.True(t, len(gotLight[0].Shares) > 0)

Expand Down
Loading

0 comments on commit cfb363c

Please sign in to comment.