Skip to content

Commit

Permalink
remove generics and type assert
Browse files Browse the repository at this point in the history
  • Loading branch information
walldiss authored and Wondertan committed May 27, 2024
1 parent 0a63a38 commit b4ee8c2
Show file tree
Hide file tree
Showing 7 changed files with 96 additions and 80 deletions.
27 changes: 10 additions & 17 deletions share/shwap/p2p/bitswap/bitswap.go
Original file line number Diff line number Diff line change
Expand Up @@ -25,12 +25,14 @@ var log = logger.Logger("shwap/bitswap")
// * godoc
// * document steps required to add new id/container type

type ID[C any] interface {
type ID interface {
String() string
CID() cid.Cid
UnmarshalContainer(*share.Root, []byte) (C, error)
Verifier(root *share.Root) verify
}

type verify func(data []byte) error

func RegisterID(mhcode, codec uint64, size int, bldrFn func(cid2 cid.Cid) (blockBuilder, error)) {
mh.Register(mhcode, func() hash.Hash {
return &hasher{IDSize: size}
Expand All @@ -45,33 +47,24 @@ func RegisterID(mhcode, codec uint64, size int, bldrFn func(cid2 cid.Cid) (block
// GetContainers
// Does not guarantee synchronization. Calling this func simultaneously with the same ID may cause
// issues. TODO: Describe motivation
func GetContainers[C any](ctx context.Context, fetcher exchange.Fetcher, root *share.Root, ids ...ID[C]) ([]C, error) {
func GetContainers(ctx context.Context, fetcher exchange.Fetcher, root *share.Root, ids ...ID) error {
ctx, cancel := context.WithCancel(ctx)
defer cancel()

cids := make([]cid.Cid, len(ids))
cntrs := make([]C, len(ids))
for i, id := range ids {
i := i
cids[i] = id.CID()

idStr := id.String()
globalVerifiers.add(idStr, func(data []byte) error {
cntr, err := id.UnmarshalContainer(root, data)
if err != nil {
return err
}

cntrs[i] = cntr
return nil
})
globalVerifiers.add(idStr, id.Verifier(root))
defer globalVerifiers.release(idStr)
}

// must start getting only after verifiers are registered
blkCh, err := fetcher.GetBlocks(ctx, cids)
if err != nil {
return nil, fmt.Errorf("fetching bitswap blocks: %w", err)
return fmt.Errorf("fetching bitswap blocks: %w", err)
}

// GetBlocks handles ctx and closes blkCh, so we don't have to
Expand All @@ -81,12 +74,12 @@ func GetContainers[C any](ctx context.Context, fetcher exchange.Fetcher, root *s
}
if len(blks) != len(cids) {
if ctx.Err() != nil {
return nil, ctx.Err()
return ctx.Err()
}
return nil, fmt.Errorf("not all the containers were found")
return fmt.Errorf("not all the containers were found")
}

return cntrs, nil
return nil
}

var globalVerifiers verifiers
Expand Down
32 changes: 20 additions & 12 deletions share/shwap/p2p/bitswap/row.go
Original file line number Diff line number Diff line change
Expand Up @@ -90,18 +90,26 @@ func (rid RowID) BlockFromEDS(eds *rsmt2d.ExtendedDataSquare) (blocks.Block, err
return blk, nil
}

func (rid RowID) UnmarshalContainer(root *share.Root, data []byte) (shwap.Row, error) {
var rowBlk bitswappb.RowBlock
if err := rowBlk.Unmarshal(data); err != nil {
return shwap.Row{}, fmt.Errorf("unmarshaling RowBlock: %w", err)
}
type RowBlock struct {
RowID
Row shwap.Row
}

row := shwap.RowFromProto(rowBlk.Row)
if err := row.Validate(root, rid.RowIndex); err != nil {
return shwap.Row{}, fmt.Errorf("validating Row: %w", err)
func (r *RowBlock) Verifier(root *share.Root) verify {

Check warning on line 98 in share/shwap/p2p/bitswap/row.go

View workflow job for this annotation

GitHub Actions / go-ci / Lint

unexported-return: exported method Verifier returns unexported type bitswap.verify, which can be annoying to use (revive)
return func(data []byte) error {
var rowBlk bitswappb.RowBlock
if err := rowBlk.Unmarshal(data); err != nil {
return fmt.Errorf("unmarshaling RowBlock: %w", err)
}

r.Row = shwap.RowFromProto(rowBlk.Row)
if err := r.Row.Validate(root, r.RowID.RowIndex); err != nil {
fmt.Errorf("validating Row: %w", err)

Check failure on line 107 in share/shwap/p2p/bitswap/row.go

View workflow job for this annotation

GitHub Actions / go-ci / Lint

unusedresult: result of fmt.Errorf call not used (govet)
}

// NOTE: We don't have to validate ID in the RowBlock, as it's implicitly verified by string
// equality of globalVerifiers entry key(requesting side) and hasher accessing the entry(response
// verification)
return nil
}
// NOTE: We don't have to validate ID in the RowBlock, as it's implicitly verified by string
// equality of globalVerifiers entry key(requesting side) and hasher accessing the entry(response
// verification)
return row, nil
}
32 changes: 20 additions & 12 deletions share/shwap/p2p/bitswap/row_namespace_data.go
Original file line number Diff line number Diff line change
Expand Up @@ -95,18 +95,26 @@ func (rndid RowNamespaceDataID) BlockFromEDS(eds *rsmt2d.ExtendedDataSquare) (bl
return blk, nil
}

func (rndid RowNamespaceDataID) UnmarshalContainer(root *share.Root, data []byte) (shwap.RowNamespaceData, error) {
var rndBlk bitswapb.RowNamespaceDataBlock
if err := rndBlk.Unmarshal(data); err != nil {
return shwap.RowNamespaceData{}, fmt.Errorf("unmarshaling RowNamespaceDataBlock: %w", err)
}
type RowNamespaceDataBlock struct {
RowNamespaceDataID
Data shwap.RowNamespaceData
}

rnd := shwap.RowNamespaceDataFromProto(rndBlk.Data)
if err := rnd.Validate(root, rndid.DataNamespace, rndid.RowIndex); err != nil {
return shwap.RowNamespaceData{}, fmt.Errorf("validating RowNamespaceData: %w", err)
func (r *RowNamespaceDataBlock) Verifier(root *share.Root) verify {

Check warning on line 103 in share/shwap/p2p/bitswap/row_namespace_data.go

View workflow job for this annotation

GitHub Actions / go-ci / Lint

unexported-return: exported method Verifier returns unexported type bitswap.verify, which can be annoying to use (revive)
return func(data []byte) error {
var rndBlk bitswapb.RowNamespaceDataBlock
if err := rndBlk.Unmarshal(data); err != nil {
return fmt.Errorf("unmarshaling RowNamespaceDataBlock: %w", err)
}

r.Data = shwap.RowNamespaceDataFromProto(rndBlk.Data)
if err := r.Data.Validate(root, r.DataNamespace, r.RowIndex); err != nil {
return fmt.Errorf("validating RowNamespaceData: %w", err)
}

// NOTE: We don't have to validate ID in the RowBlock, as it's implicitly verified by string
// equality of globalVerifiers entry key(requesting side) and hasher accessing the entry(response
// verification)
return nil
}
// NOTE: We don't have to validate ID in the RowBlock, as it's implicitly verified by string
// equality of globalVerifiers entry key(requesting side) and hasher accessing the entry(response
// verification)
return rnd, nil
}
15 changes: 8 additions & 7 deletions share/shwap/p2p/bitswap/row_namespace_data_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -22,20 +22,21 @@ func TestRowNamespaceDataRoundtrip_GetContainers(t *testing.T) {
client := remoteClient(ctx, t, newTestBlockstore(eds))

rowIdxs := share.RowsWithNamespace(root, namespace)
ids := make([]ID[shwap.RowNamespaceData], len(rowIdxs))
nds := make([]*RowNamespaceDataBlock, len(rowIdxs))
ids := make([]ID, len(rowIdxs))
for i, rowIdx := range rowIdxs {
rid, err := shwap.NewRowNamespaceDataID(1, rowIdx, namespace, root)
require.NoError(t, err)
ids[i] = RowNamespaceDataID(rid)
//TODO(@walldiss): not sure if copy of RowNamespaceDataID type is needed in bitswap

Check failure on line 30 in share/shwap/p2p/bitswap/row_namespace_data_test.go

View workflow job for this annotation

GitHub Actions / go-ci / Lint

commentFormatting: put a space between `//` and comment text (gocritic)
nds[i] = &RowNamespaceDataBlock{RowNamespaceDataID: RowNamespaceDataID(rid)}
ids[i] = nds[i]
}

cntrs, err := GetContainers(ctx, client, root, ids...)
err := GetContainers(ctx, client, root, ids...)
require.NoError(t, err)
require.Len(t, cntrs, len(ids))

for i, cntr := range cntrs {
sid := ids[i].(RowNamespaceDataID)
err = cntr.Validate(root, sid.DataNamespace, sid.RowIndex)
for _, nd := range nds {
err = nd.Data.Validate(root, nd.RowNamespaceDataID.DataNamespace, nd.RowNamespaceDataID.RowIndex)
require.NoError(t, err)
}
}
16 changes: 8 additions & 8 deletions share/shwap/p2p/bitswap/row_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -16,26 +16,26 @@ func TestRowRoundtrip_GetContainers(t *testing.T) {
ctx, cancel := context.WithTimeout(context.Background(), time.Second*5)
defer cancel()

eds := edstest.RandEDS(t, 2)
eds := edstest.RandEDS(t, 4)
root, err := share.NewRoot(eds)
require.NoError(t, err)
client := remoteClient(ctx, t, newTestBlockstore(eds))

ids := make([]ID[shwap.Row], eds.Width())
ids := make([]ID, eds.Width())
data := make([]*RowBlock, eds.Width())
for i := range ids {
rid, err := shwap.NewRowID(1, i, root)
require.NoError(t, err)
log.Debugf("%X", RowID(rid).CID())
ids[i] = RowID(rid)
data[i] = &RowBlock{RowID: RowID(rid)}
ids[i] = data[i]
}

cntrs, err := GetContainers(ctx, client, root, ids...)
err = GetContainers(ctx, client, root, ids...)
require.NoError(t, err)
require.Len(t, cntrs, len(ids))

for i, cntr := range cntrs {
rid := ids[i].(RowID)
err = cntr.Validate(root, rid.RowIndex)
for _, row := range data {
err = row.Row.Validate(root, row.RowIndex)
require.NoError(t, err)
}

Expand Down
31 changes: 19 additions & 12 deletions share/shwap/p2p/bitswap/sample.go
Original file line number Diff line number Diff line change
Expand Up @@ -95,18 +95,25 @@ func (sid SampleID) BlockFromEDS(eds *rsmt2d.ExtendedDataSquare) (blocks.Block,
return blk, nil
}

func (sid SampleID) UnmarshalContainer(root *share.Root, data []byte) (shwap.Sample, error) {
var sampleBlk bitswappb.SampleBlock
if err := sampleBlk.Unmarshal(data); err != nil {
return shwap.Sample{}, fmt.Errorf("unmarshaling SampleBlock: %w", err)
}
type SampleBlock struct {
SampleID
Sample shwap.Sample
}

sample := shwap.SampleFromProto(sampleBlk.Sample)
if err := sample.Validate(root, sid.RowIndex, sid.ShareIndex); err != nil {
return shwap.Sample{}, fmt.Errorf("validating Sample: %w", err)
func (s *SampleBlock) Verifier(root *share.Root) verify {

Check warning on line 103 in share/shwap/p2p/bitswap/sample.go

View workflow job for this annotation

GitHub Actions / go-ci / Lint

unexported-return: exported method Verifier returns unexported type bitswap.verify, which can be annoying to use (revive)
return func(data []byte) error {
var sampleBlk bitswappb.SampleBlock
if err := sampleBlk.Unmarshal(data); err != nil {
return fmt.Errorf("unmarshaling SampleBlock: %w", err)
}

s.Sample = shwap.SampleFromProto(sampleBlk.Sample)
if err := s.Sample.Validate(root, s.RowIndex, s.ShareIndex); err != nil {
return fmt.Errorf("validating Sample: %w", err)
}
// NOTE: We don't have to validate ID in the RowBlock, as it's implicitly verified by string
// equality of globalVerifiers entry key(requesting side) and hasher accessing the entry(response
// verification)
return nil
}
// NOTE: We don't have to validate ID in the RowBlock, as it's implicitly verified by string
// equality of globalVerifiers entry key(requesting side) and hasher accessing the entry(response
// verification)
return sample, nil
}
23 changes: 11 additions & 12 deletions share/shwap/p2p/bitswap/sample_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,14 +2,12 @@ package bitswap

import (
"context"

Check failure on line 4 in share/shwap/p2p/bitswap/sample_test.go

View workflow job for this annotation

GitHub Actions / go-ci / Lint

File is not `gofumpt`-ed with `-extra` (gofumpt)
"testing"
"time"

"github.com/stretchr/testify/require"

"github.com/celestiaorg/celestia-node/share"
"github.com/celestiaorg/celestia-node/share/eds/edstest"
"github.com/celestiaorg/celestia-node/share/shwap"
"github.com/stretchr/testify/require"

Check failure on line 8 in share/shwap/p2p/bitswap/sample_test.go

View workflow job for this annotation

GitHub Actions / go-ci / Lint

File is not `goimports`-ed with -local github.com/celestiaorg/celestia-node (goimports)
"testing"

Check failure on line 9 in share/shwap/p2p/bitswap/sample_test.go

View workflow job for this annotation

GitHub Actions / go-ci / Lint

File is not `gofumpt`-ed with `-extra` (gofumpt)
"time"
)

func TestSampleRoundtrip_GetContainers(t *testing.T) {
Expand All @@ -21,23 +19,24 @@ func TestSampleRoundtrip_GetContainers(t *testing.T) {
require.NoError(t, err)
client := remoteClient(ctx, t, newTestBlockstore(eds))

var ids []ID[shwap.Sample]
width := int(eds.Width())
ids := make([]ID, 0, width*width)
samples := make([]*SampleBlock, 0, width*width)
for x := 0; x < width; x++ {
for y := 0; y < width; y++ {
sid, err := shwap.NewSampleID(1, x, y, root)
require.NoError(t, err)
ids = append(ids, SampleID(sid))
sampleBlock := &SampleBlock{SampleID: SampleID(sid)}
ids = append(ids, sampleBlock)
samples = append(samples, sampleBlock)
}
}

cntrs, err := GetContainers(ctx, client, root, ids...)
err = GetContainers(ctx, client, root, ids...)
require.NoError(t, err)
require.Len(t, cntrs, len(ids))

for i, cntr := range cntrs {
sid := ids[i].(SampleID)
err = cntr.Validate(root, sid.RowIndex, sid.ShareIndex)
for _, sample := range samples {
err = sample.Sample.Validate(root, sample.RowIndex, sample.ShareIndex)
require.NoError(t, err)
}
}

0 comments on commit b4ee8c2

Please sign in to comment.