From 3c8ad421100a530cf780b8e7a76eda9c1bdf1bf2 Mon Sep 17 00:00:00 2001 From: Wondertan Date: Sat, 26 Oct 2024 08:32:12 +0200 Subject: [PATCH 01/26] refactor(share): GetShare -> GetSamples --- blob/service_test.go | 7 +- nodebuilder/share/mocks/api.go | 30 ++++---- nodebuilder/share/share.go | 14 +++- share/availability/light/availability.go | 70 +++++++----------- share/availability/light/availability_test.go | 72 +++++++++++++------ share/eds/accessor.go | 1 + share/eds/validation.go | 7 +- share/shwap/getter.go | 6 +- share/shwap/getters/cascade.go | 21 ++---- share/shwap/getters/cascade_test.go | 2 +- share/shwap/getters/mock/getter.go | 14 ++-- share/shwap/getters/testing.go | 46 +++++++----- share/shwap/p2p/bitswap/getter.go | 66 ++++++++--------- share/shwap/p2p/bitswap/sample_block.go | 4 +- share/shwap/p2p/bitswap/sample_block_test.go | 5 +- share/shwap/p2p/shrex/shrex_getter/shrex.go | 4 +- share/shwap/sample.go | 13 ++-- share/shwap/sample_id.go | 27 ++++++- share/shwap/sample_id_test.go | 16 ++++- store/file/ods.go | 7 +- store/file/ods_q4.go | 7 +- store/getter.go | 34 +++++---- 22 files changed, 276 insertions(+), 197 deletions(-) diff --git a/blob/service_test.go b/blob/service_test.go index 2cbfcf7b03..9b47b74cb8 100644 --- a/blob/service_test.go +++ b/blob/service_test.go @@ -902,10 +902,9 @@ func createService(ctx context.Context, t testing.TB, shares []libshare.Share) * nd, err := eds.NamespaceData(ctx, accessor, ns) return nd, err }) - shareGetter.EXPECT().GetShare(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any()).AnyTimes(). - DoAndReturn(func(ctx context.Context, h *header.ExtendedHeader, row, col int) (libshare.Share, error) { - s, err := accessor.Sample(ctx, row, col) - return s.Share, err + shareGetter.EXPECT().GetSamples(gomock.Any(), gomock.Any(), gomock.Any()).AnyTimes(). + DoAndReturn(func(ctx context.Context, h *header.ExtendedHeader, indices []shwap.SampleIndex) ([]shwap.Sample, error) { + return smpls, nil }) // create header and put it into the store diff --git a/nodebuilder/share/mocks/api.go b/nodebuilder/share/mocks/api.go index cccc81a452..84866bf784 100644 --- a/nodebuilder/share/mocks/api.go +++ b/nodebuilder/share/mocks/api.go @@ -53,6 +53,21 @@ func (mr *MockModuleMockRecorder) GetEDS(arg0, arg1 interface{}) *gomock.Call { return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GetEDS", reflect.TypeOf((*MockModule)(nil).GetEDS), arg0, arg1) } +// GetNamespaceData mocks base method. +func (m *MockModule) GetNamespaceData(arg0 context.Context, arg1 uint64, arg2 share0.Namespace) (shwap.NamespaceData, error) { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "GetNamespaceData", arg0, arg1, arg2) + ret0, _ := ret[0].(shwap.NamespaceData) + ret1, _ := ret[1].(error) + return ret0, ret1 +} + +// GetNamespaceData indicates an expected call of GetNamespaceData. +func (mr *MockModuleMockRecorder) GetNamespaceData(arg0, arg1, arg2 interface{}) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GetNamespaceData", reflect.TypeOf((*MockModule)(nil).GetNamespaceData), arg0, arg1, arg2) +} + // GetRange mocks base method. func (m *MockModule) GetRange(arg0 context.Context, arg1 uint64, arg2, arg3 int) (*share.GetRangeResult, error) { m.ctrl.T.Helper() @@ -83,21 +98,6 @@ func (mr *MockModuleMockRecorder) GetShare(arg0, arg1, arg2, arg3 interface{}) * return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GetShare", reflect.TypeOf((*MockModule)(nil).GetShare), arg0, arg1, arg2, arg3) } -// GetSharesByNamespace mocks base method. -func (m *MockModule) GetSharesByNamespace(arg0 context.Context, arg1 uint64, arg2 share0.Namespace) (shwap.NamespaceData, error) { - m.ctrl.T.Helper() - ret := m.ctrl.Call(m, "GetSharesByNamespace", arg0, arg1, arg2) - ret0, _ := ret[0].(shwap.NamespaceData) - ret1, _ := ret[1].(error) - return ret0, ret1 -} - -// GetSharesByNamespace indicates an expected call of GetSharesByNamespace. -func (mr *MockModuleMockRecorder) GetSharesByNamespace(arg0, arg1, arg2 interface{}) *gomock.Call { - mr.mock.ctrl.T.Helper() - return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GetSharesByNamespace", reflect.TypeOf((*MockModule)(nil).GetSharesByNamespace), arg0, arg1, arg2) -} - // SharesAvailable mocks base method. func (m *MockModule) SharesAvailable(arg0 context.Context, arg1 uint64) error { m.ctrl.T.Helper() diff --git a/nodebuilder/share/share.go b/nodebuilder/share/share.go index 8a3efcc757..9a904b48fd 100644 --- a/nodebuilder/share/share.go +++ b/nodebuilder/share/share.go @@ -112,12 +112,24 @@ type module struct { hs headerServ.Module } +// TODO(@Wondertan): break 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) + + idx, err := shwap.SampleIndexFromCoordinates(row, col, len(header.DAH.RowRoots)) + if err != nil { + return libshare.Share{}, err + } + + smpls, err := m.getter.GetSamples(ctx, header, []shwap.SampleIndex{idx}) + if err != nil { + return libshare.Share{}, err + } + + return smpls[0].Share, nil } func (m module) GetEDS(ctx context.Context, height uint64) (*rsmt2d.ExtendedDataSquare, error) { diff --git a/share/availability/light/availability.go b/share/availability/light/availability.go index 6a23abe3d9..a9e60d486a 100644 --- a/share/availability/light/availability.go +++ b/share/availability/light/availability.go @@ -6,7 +6,6 @@ import ( "errors" "fmt" "sync" - "time" "github.com/ipfs/boxo/blockstore" "github.com/ipfs/go-datastore" @@ -114,59 +113,34 @@ func (la *ShareAvailability) SharesAvailable(ctx context.Context, header *header return nil } - var ( - mutex sync.Mutex - failedSamples []Sample - wg sync.WaitGroup - ) - - log.Debugw("starting sampling session", "height", header.Height()) - - // remove one second from the deadline to ensure we have enough time to process the results - samplingCtx, cancel := context.WithCancel(ctx) - if deadline, ok := ctx.Deadline(); ok { - samplingCtx, cancel = context.WithDeadline(ctx, deadline.Add(-time.Second)) - } - defer cancel() - - // Concurrently sample shares - for _, s := range samples.Remaining { - wg.Add(1) - go func(s Sample) { - defer wg.Done() - _, err := la.getter.GetShare(samplingCtx, header, s.Row, s.Col) - mutex.Lock() - defer mutex.Unlock() - if err != nil { - log.Debugw("error fetching share", "height", header.Height(), "row", s.Row, "col", s.Col) - failedSamples = append(failedSamples, s) - } else { - samples.Available = append(samples.Available, s) - } - }(s) - } - wg.Wait() + log.Debugw("starting sampling session", "root", dah.String()) - // Update remaining samples with failed ones - samples.Remaining = failedSamples + idxs := make([]shwap.SampleIndex, len(samples.Available)) + for i, s := range samples.Available { + idx, err := shwap.SampleIndexFromCoordinates(int(s.Row), int(s.Col), len(dah.RowRoots)) + if err != nil { + return err + } - // Store the updated sampling result - updatedData, err := json.Marshal(samples) - if err != nil { - return err - } - la.dsLk.Lock() - err = la.ds.Put(ctx, key, updatedData) - la.dsLk.Unlock() - if err != nil { - return fmt.Errorf("store sampling result: %w", err) + idxs[i] = idx } + smpls, err := la.getter.GetSamples(ctx, header, idxs) if errors.Is(ctx.Err(), context.Canceled) { // Availability did not complete due to context cancellation, return context error instead of // share.ErrNotAvailable return ctx.Err() } + if len(smpls) == 0 { + return share.ErrNotAvailable + } + + var failedSamples []Sample + for i, smpl := range smpls { + if smpl.IsEmpty() { + failedSamples = append(failedSamples, samples.Available[i]) + } + } // if any of the samples failed, return an error if len(failedSamples) > 0 { @@ -210,7 +184,11 @@ func (la *ShareAvailability) Prune(ctx context.Context, h *header.ExtendedHeader // delete stored samples for _, sample := range result.Available { - blk, err := bitswap.NewEmptySampleBlock(h.Height(), sample.Row, sample.Col, len(h.DAH.RowRoots)) + idx, err := shwap.SampleIndexFromCoordinates(sample.Row, sample.Col, len(h.DAH.RowRoots)) + if err != nil { + return err + } + blk, err := bitswap.NewEmptySampleBlock(h.Height(), idx, len(h.DAH.RowRoots)) if err != nil { return fmt.Errorf("marshal sample ID: %w", err) } diff --git a/share/availability/light/availability_test.go b/share/availability/light/availability_test.go index 64834d46b5..6974c3dc8d 100644 --- a/share/availability/light/availability_test.go +++ b/share/availability/light/availability_test.go @@ -21,11 +21,13 @@ import ( "github.com/stretchr/testify/require" libshare "github.com/celestiaorg/go-square/v2/share" + "github.com/celestiaorg/nmt" "github.com/celestiaorg/rsmt2d" "github.com/celestiaorg/celestia-node/header" "github.com/celestiaorg/celestia-node/header/headertest" "github.com/celestiaorg/celestia-node/share" + "github.com/celestiaorg/celestia-node/share/eds" "github.com/celestiaorg/celestia-node/share/eds/edstest" "github.com/celestiaorg/celestia-node/share/shwap" "github.com/celestiaorg/celestia-node/share/shwap/getters/mock" @@ -38,22 +40,32 @@ func TestSharesAvailableSuccess(t *testing.T) { ctx, cancel := context.WithCancel(context.Background()) defer cancel() - eds := edstest.RandEDS(t, 16) - roots, err := share.NewAxisRoots(eds) + square := edstest.RandEDS(t, 16) + roots, err := share.NewAxisRoots(square) require.NoError(t, err) eh := headertest.RandExtendedHeaderWithRoot(t, roots) getter := mock.NewMockGetter(gomock.NewController(t)) getter.EXPECT(). - GetShare(gomock.Any(), eh, gomock.Any(), gomock.Any()). + GetSamples(gomock.Any(), eh, gomock.Any()). DoAndReturn( - func(_ context.Context, _ *header.ExtendedHeader, row, col int) (libshare.Share, error) { - rawSh := eds.GetCell(uint(row), uint(col)) - sh, err := libshare.NewShare(rawSh) - if err != nil { - return libshare.Share{}, err + func(_ context.Context, hdr *header.ExtendedHeader, indices []shwap.SampleIndex) ([]shwap.Sample, error) { + acc := eds.Rsmt2D{ExtendedDataSquare: square} + smpls := make([]shwap.Sample, len(indices)) + for i, idx := range indices { + rowIdx, colIdx, err := idx.Coordinates(len(hdr.DAH.RowRoots)) + if err != nil { + return nil, err + } + + smpl, err := acc.Sample(ctx, rowIdx, colIdx) + if err != nil { + return nil, err + } + + smpls[i] = smpl } - return *sh, nil + return smpls, nil }). AnyTimes() @@ -87,8 +99,8 @@ func TestSharesAvailableSkipSampled(t *testing.T) { // Create a getter that always returns ErrNotFound getter := mock.NewMockGetter(gomock.NewController(t)) getter.EXPECT(). - GetShare(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any()). - Return(libshare.Share{}, shrex.ErrNotFound). + GetSamples(gomock.Any(), gomock.Any(), gomock.Any()). + Return(nil, shrex.ErrNotFound). AnyTimes() ds := datastore.NewMapDatastore() @@ -137,9 +149,12 @@ func TestSharesAvailableFailed(t *testing.T) { failGetter := mock.NewMockGetter(gomock.NewController(t)) // Getter doesn't have the eds, so it should fail for all samples + // failGetter.EXPECT(). + // GetShare(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any()). + // Return(libshare.Share{}, shrex.ErrNotFound). failGetter.EXPECT(). - GetShare(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any()). - Return(libshare.Share{}, shrex.ErrNotFound). + GetSamples(gomock.Any(), gomock.Any(), gomock.Any()). + Return(make([]shwap.Sample, DefaultSampleAmount), shrex.ErrNotFound). AnyTimes() ds := datastore.NewMapDatastore() @@ -209,8 +224,11 @@ func TestSharesAvailableFailed(t *testing.T) { // onceGetter should have no more samples stored after the call successfulGetter.checkOnce(t) - require.ElementsMatch(t, failed.Remaining, successfulGetter.sampledList()) + // require.ElementsMatch(t, failed.Remaining, successfulGetter.sampledList()) } + // // onceGetter should have no more samples stored after the call + // successfulGetter.checkOnce(t) + // require.ElementsMatch(t, failed.Remaining, len(successfulGetter.sampled)) } func TestParallelAvailability(t *testing.T) { @@ -238,7 +256,7 @@ func TestParallelAvailability(t *testing.T) { }() } wg.Wait() - require.Len(t, successfulGetter.sampledList(), int(avail.params.SampleAmount)) + require.Len(t, len(successfulGetter.sampled), int(avail.params.SampleAmount)) // Verify that the sampling result is stored with all samples marked as available resultData, err := avail.ds.Get(ctx, datastoreKeyForRoot(roots)) @@ -274,14 +292,24 @@ func (g onceGetter) checkOnce(t *testing.T) { } } -func (g onceGetter) sampledList() []Sample { - g.Lock() - defer g.Unlock() - samples := make([]Sample, 0, len(g.sampled)) - for s := range g.sampled { - samples = append(samples, s) +func (m onceGetter) GetSamples(_ context.Context, hdr *header.ExtendedHeader, indices []shwap.SampleIndex) ([]shwap.Sample, error) { + m.Lock() + defer m.Unlock() + + smpls := make([]shwap.Sample, 0, len(indices)) + for _, idx := range indices { + rowIdx, colIdx, err := idx.Coordinates(len(hdr.DAH.RowRoots)) + if err != nil { + return nil, err + } + + s := Sample{Row: rowIdx, Col: colIdx} + if _, ok := m.sampled[s]; ok { + delete(m.sampled, s) + smpls = append(smpls, shwap.Sample{Proof: &nmt.Proof{}}) + } } - return samples + return smpls, nil } func (g onceGetter) GetShare(_ context.Context, _ *header.ExtendedHeader, row, col int) (libshare.Share, error) { diff --git a/share/eds/accessor.go b/share/eds/accessor.go index 07eb6db542..81f55fcf64 100644 --- a/share/eds/accessor.go +++ b/share/eds/accessor.go @@ -25,6 +25,7 @@ type Accessor interface { // Sample returns share and corresponding proof for row and column indices. Implementation can // choose which axis to use for proof. Chosen axis for proof should be indicated in the returned // Sample. + // TODO(@Wondertan): change to SampleIndex Sample(ctx context.Context, rowIdx, colIdx int) (shwap.Sample, error) // AxisHalf returns half of shares axis of the given type and index. Side is determined by // implementation. Implementations should indicate the side in the returned AxisHalf. diff --git a/share/eds/validation.go b/share/eds/validation.go index 845a5bac77..0d513911d9 100644 --- a/share/eds/validation.go +++ b/share/eds/validation.go @@ -35,7 +35,12 @@ func (f validation) Size(ctx context.Context) int { } func (f validation) Sample(ctx context.Context, rowIdx, colIdx int) (shwap.Sample, error) { - _, err := shwap.NewSampleID(1, rowIdx, colIdx, f.Size(ctx)) + idx, err := shwap.SampleIndexFromCoordinates(rowIdx, colIdx, f.Size(ctx)) + if err != nil { + return shwap.Sample{}, err + } + + _, err = shwap.NewSampleID(1, idx, f.Size(ctx)) if err != nil { return shwap.Sample{}, fmt.Errorf("sample validation: %w", err) } diff --git a/share/shwap/getter.go b/share/shwap/getter.go index 21ac2ec49f..f88a10debe 100644 --- a/share/shwap/getter.go +++ b/share/shwap/getter.go @@ -29,8 +29,10 @@ var ( // //go:generate mockgen -destination=getters/mock/getter.go -package=mock . Getter type Getter interface { - // GetShare gets a Share by coordinates in EDS. - GetShare(ctx context.Context, header *header.ExtendedHeader, row, col int) (libshare.Share, error) + // GetSamples gets samples by their indices. + // Returns Sample slice with requested number of samples in the requested order. + // May return partial response with some samples being empty if they weren't found. + GetSamples(ctx context.Context, header *header.ExtendedHeader, indices []SampleIndex) ([]Sample, error) // GetEDS gets the full EDS identified by the given extended header. GetEDS(context.Context, *header.ExtendedHeader) (*rsmt2d.ExtendedDataSquare, error) diff --git a/share/shwap/getters/cascade.go b/share/shwap/getters/cascade.go index 962adffd3c..3518ce8eea 100644 --- a/share/shwap/getters/cascade.go +++ b/share/shwap/getters/cascade.go @@ -41,24 +41,15 @@ func NewCascadeGetter(getters []shwap.Getter) *CascadeGetter { } } -// GetShare gets a share from any of registered shwap.Getters in cascading order. -func (cg *CascadeGetter) GetShare( - ctx context.Context, header *header.ExtendedHeader, row, col int, -) (libshare.Share, error) { - ctx, span := tracer.Start(ctx, "cascade/get-share", trace.WithAttributes( - attribute.Int("row", row), - attribute.Int("col", col), +// GetSamples gets samples from any of registered shwap.Getters in cascading order. +func (cg *CascadeGetter) GetSamples(ctx context.Context, hdr *header.ExtendedHeader, indices []shwap.SampleIndex) ([]shwap.Sample, error) { + ctx, span := tracer.Start(ctx, "cascade/get-samples", trace.WithAttributes( + attribute.Int("amount", len(indices)), )) defer span.End() - upperBound := len(header.DAH.RowRoots) - if row >= upperBound || col >= upperBound { - err := shwap.ErrOutOfBounds - span.RecordError(err) - return libshare.Share{}, err - } - get := func(ctx context.Context, get shwap.Getter) (libshare.Share, error) { - return get.GetShare(ctx, header, row, col) + get := func(ctx context.Context, get shwap.Getter) ([]shwap.Sample, error) { + return get.GetSamples(ctx, hdr, indices) } return cascadeGetters(ctx, cg.getters, get) diff --git a/share/shwap/getters/cascade_test.go b/share/shwap/getters/cascade_test.go index a23568006f..39e9999fa1 100644 --- a/share/shwap/getters/cascade_test.go +++ b/share/shwap/getters/cascade_test.go @@ -30,7 +30,7 @@ func TestCascadeGetter(t *testing.T) { getter := NewCascadeGetter(getters) t.Run("GetShare", func(t *testing.T) { for _, eh := range headers { - sh, err := getter.GetShare(ctx, eh, 0, 0) + sh, err := getter.GetSamples(ctx, eh, []shwap.SampleIndex{0}) assert.NoError(t, err) assert.NotEmpty(t, sh) } diff --git a/share/shwap/getters/mock/getter.go b/share/shwap/getters/mock/getter.go index 856802d75d..235b5a1fc7 100644 --- a/share/shwap/getters/mock/getter.go +++ b/share/shwap/getters/mock/getter.go @@ -68,17 +68,17 @@ func (mr *MockGetterMockRecorder) GetNamespaceData(arg0, arg1, arg2 interface{}) return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GetNamespaceData", reflect.TypeOf((*MockGetter)(nil).GetNamespaceData), arg0, arg1, arg2) } -// GetShare mocks base method. -func (m *MockGetter) GetShare(arg0 context.Context, arg1 *header.ExtendedHeader, arg2, arg3 int) (share.Share, error) { +// GetSamples mocks base method. +func (m *MockGetter) GetSamples(arg0 context.Context, arg1 *header.ExtendedHeader, arg2 []shwap.SampleIndex) ([]shwap.Sample, error) { m.ctrl.T.Helper() - ret := m.ctrl.Call(m, "GetShare", arg0, arg1, arg2, arg3) - ret0, _ := ret[0].(share.Share) + ret := m.ctrl.Call(m, "GetSamples", arg0, arg1, arg2) + ret0, _ := ret[0].([]shwap.Sample) ret1, _ := ret[1].(error) return ret0, ret1 } -// GetShare indicates an expected call of GetShare. -func (mr *MockGetterMockRecorder) GetShare(arg0, arg1, arg2, arg3 interface{}) *gomock.Call { +// GetSamples indicates an expected call of GetSamples. +func (mr *MockGetterMockRecorder) GetSamples(arg0, arg1, arg2 interface{}) *gomock.Call { mr.mock.ctrl.T.Helper() - return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GetShare", reflect.TypeOf((*MockGetter)(nil).GetShare), arg0, arg1, arg2, arg3) + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GetSamples", reflect.TypeOf((*MockGetter)(nil).GetSamples), arg0, arg1, arg2) } diff --git a/share/shwap/getters/testing.go b/share/shwap/getters/testing.go index a8fdd53ee6..3d7338a2fb 100644 --- a/share/shwap/getters/testing.go +++ b/share/shwap/getters/testing.go @@ -14,43 +14,51 @@ import ( "github.com/celestiaorg/celestia-node/header" "github.com/celestiaorg/celestia-node/header/headertest" "github.com/celestiaorg/celestia-node/share" + "github.com/celestiaorg/celestia-node/share/eds" "github.com/celestiaorg/celestia-node/share/eds/edstest" "github.com/celestiaorg/celestia-node/share/shwap" ) // TestGetter provides a testing SingleEDSGetter and the root of the EDS it holds. func TestGetter(t *testing.T) (shwap.Getter, *header.ExtendedHeader) { - eds := edstest.RandEDS(t, 8) - roots, err := share.NewAxisRoots(eds) + square := edstest.RandEDS(t, 8) + roots, err := share.NewAxisRoots(square) eh := headertest.RandExtendedHeaderWithRoot(t, roots) require.NoError(t, err) return &SingleEDSGetter{ - EDS: eds, + EDS: eds.Rsmt2D{ExtendedDataSquare: square}, }, eh } // SingleEDSGetter contains a single EDS where data is retrieved from. // Its primary use is testing, and GetNamespaceData is not supported. type SingleEDSGetter struct { - EDS *rsmt2d.ExtendedDataSquare + EDS eds.Rsmt2D } -// GetShare gets a share from a kept EDS if exist and if the correct root is given. -func (seg *SingleEDSGetter) GetShare( - _ context.Context, - header *header.ExtendedHeader, - row, col int, -) (libshare.Share, error) { - err := seg.checkRoots(header.DAH) +// GetSamples get samples from a kept EDS if exist and if the correct root is given. +func (seg *SingleEDSGetter) GetSamples(ctx context.Context, hdr *header.ExtendedHeader, indices []shwap.SampleIndex) ([]shwap.Sample, error) { + err := seg.checkRoots(hdr.DAH) if err != nil { - return libshare.Share{}, err + return nil, err } - rawSh := seg.EDS.GetCell(uint(row), uint(col)) - sh, err := libshare.NewShare(rawSh) - if err != nil { - return libshare.Share{}, err + + smpls := make([]shwap.Sample, len(indices)) + for i, idx := range indices { + rowIdx, colIdx, err := idx.Coordinates(len(hdr.DAH.RowRoots)) + if err != nil { + return nil, err + } + + smpl, err := seg.EDS.Sample(ctx, rowIdx, colIdx) + if err != nil { + return nil, err + } + + smpls[i] = smpl } - return *sh, nil + + return smpls, nil } // GetEDS returns a kept EDS if the correct root is given. @@ -62,7 +70,7 @@ func (seg *SingleEDSGetter) GetEDS( if err != nil { return nil, err } - return seg.EDS, nil + return seg.EDS.ExtendedDataSquare, nil } // GetNamespaceData returns NamespacedShares from a kept EDS if the correct root is given. @@ -72,7 +80,7 @@ func (seg *SingleEDSGetter) GetNamespaceData(context.Context, *header.ExtendedHe } func (seg *SingleEDSGetter) checkRoots(roots *share.AxisRoots) error { - dah, err := da.NewDataAvailabilityHeader(seg.EDS) + dah, err := da.NewDataAvailabilityHeader(seg.EDS.ExtendedDataSquare) if err != nil { return err } diff --git a/share/shwap/p2p/bitswap/getter.go b/share/shwap/p2p/bitswap/getter.go index 308185a36c..058f84158e 100644 --- a/share/shwap/p2p/bitswap/getter.go +++ b/share/shwap/p2p/bitswap/getter.go @@ -11,6 +11,7 @@ import ( "go.opentelemetry.io/otel" "go.opentelemetry.io/otel/attribute" "go.opentelemetry.io/otel/codes" + "go.opentelemetry.io/otel/trace" "github.com/celestiaorg/celestia-app/v3/pkg/wrapper" libshare "github.com/celestiaorg/go-square/v2/share" @@ -74,27 +75,24 @@ func (g *Getter) Stop() { g.cancel() } -// GetShares uses [SampleBlock] and [Fetch] to get and verify samples for given coordinates. -// TODO(@Wondertan): Rework API to get coordinates as a single param to make it ergonomic. -func (g *Getter) GetShares( +// GetSamples uses [SampleBlock] and [Fetch] to get and verify samples for given coordinates. +func (g *Getter) GetSamples( ctx context.Context, hdr *header.ExtendedHeader, - rowIdxs, colIdxs []int, -) ([]libshare.Share, error) { - if len(rowIdxs) != len(colIdxs) { - return nil, fmt.Errorf("row indecies and col indices must be same length") + indices []shwap.SampleIndex, +) ([]shwap.Sample, error) { + if len(indices) == 0 { + return nil, fmt.Errorf("no sample indicies to fetch") } - if len(rowIdxs) == 0 { - return nil, fmt.Errorf("empty coordinates") - } - - ctx, span := tracer.Start(ctx, "get-shares") + ctx, span := tracer.Start(ctx, "get-samples", trace.WithAttributes( + attribute.Int("amount", len(indices)), + )) defer span.End() - blks := make([]Block, len(rowIdxs)) - for i, rowIdx := range rowIdxs { - sid, err := NewEmptySampleBlock(hdr.Height(), rowIdx, colIdxs[i], len(hdr.DAH.RowRoots)) + blks := make([]Block, len(indices)) + for i, idx := range indices { + sid, err := NewEmptySampleBlock(hdr.Height(), idx, len(hdr.DAH.RowRoots)) if err != nil { span.RecordError(err) span.SetStatus(codes.Error, "NewEmptySampleBlock") @@ -112,36 +110,32 @@ func (g *Getter) GetShares( err := Fetch(ctx, g.exchange, hdr.DAH, blks, WithStore(g.bstore), WithFetcher(ses)) if err != nil { + // handle partial fetches + var fetched int + smpls := make([]shwap.Sample, len(blks)) + for i, blk := range blks { + if smpl := blk.(*SampleBlock).Container; !smpl.IsEmpty() { + smpls[i] = smpl + fetched++ + } + } + span.RecordError(err) span.SetStatus(codes.Error, "Fetch") + if fetched > 0 { + span.SetAttributes(attribute.Int("fetched", fetched)) + return smpls, err + } return nil, err } - shares := make([]libshare.Share, len(blks)) + smpls := make([]shwap.Sample, len(blks)) for i, blk := range blks { - shares[i] = blk.(*SampleBlock).Container.Share + smpls[i] = blk.(*SampleBlock).Container } span.SetStatus(codes.Ok, "") - return shares, nil -} - -// GetShare uses [GetShare] to fetch and verify single share by the given coordinates. -func (g *Getter) GetShare( - ctx context.Context, - hdr *header.ExtendedHeader, - row, col int, -) (libshare.Share, error) { - shrs, err := g.GetShares(ctx, hdr, []int{row}, []int{col}) - if err != nil { - return libshare.Share{}, err - } - - if len(shrs) != 1 { - return libshare.Share{}, fmt.Errorf("expected 1 share row, got %d", len(shrs)) - } - - return shrs[0], nil + return smpls, nil } // GetEDS uses [RowBlock] and [Fetch] to get half of the first EDS quadrant(ODS) and diff --git a/share/shwap/p2p/bitswap/sample_block.go b/share/shwap/p2p/bitswap/sample_block.go index 0f25ccb454..bf4ca43774 100644 --- a/share/shwap/p2p/bitswap/sample_block.go +++ b/share/shwap/p2p/bitswap/sample_block.go @@ -47,8 +47,8 @@ type SampleBlock struct { } // NewEmptySampleBlock constructs a new empty SampleBlock. -func NewEmptySampleBlock(height uint64, rowIdx, colIdx, edsSize int) (*SampleBlock, error) { - id, err := shwap.NewSampleID(height, rowIdx, colIdx, edsSize) +func NewEmptySampleBlock(height uint64, idx shwap.SampleIndex, edsSize int) (*SampleBlock, error) { + id, err := shwap.NewSampleID(height, idx, edsSize) if err != nil { return nil, err } diff --git a/share/shwap/p2p/bitswap/sample_block_test.go b/share/shwap/p2p/bitswap/sample_block_test.go index 2a28e7e4c9..6c12edcb8d 100644 --- a/share/shwap/p2p/bitswap/sample_block_test.go +++ b/share/shwap/p2p/bitswap/sample_block_test.go @@ -9,6 +9,7 @@ import ( "github.com/celestiaorg/celestia-node/share" "github.com/celestiaorg/celestia-node/share/eds/edstest" + "github.com/celestiaorg/celestia-node/share/shwap" ) func TestSample_FetchRoundtrip(t *testing.T) { @@ -24,7 +25,9 @@ func TestSample_FetchRoundtrip(t *testing.T) { blks := make([]Block, 0, width*width) for x := 0; x < width; x++ { for y := 0; y < width; y++ { - blk, err := NewEmptySampleBlock(1, x, y, len(root.RowRoots)) + idx, err := shwap.SampleIndexFromCoordinates(x, y, width) + require.NoError(t, err) + blk, err := NewEmptySampleBlock(1, idx, len(root.RowRoots)) require.NoError(t, err) blks = append(blks, blk) } diff --git a/share/shwap/p2p/shrex/shrex_getter/shrex.go b/share/shwap/p2p/shrex/shrex_getter/shrex.go index f51328136c..6247223622 100644 --- a/share/shwap/p2p/shrex/shrex_getter/shrex.go +++ b/share/shwap/p2p/shrex/shrex_getter/shrex.go @@ -146,8 +146,8 @@ func (sg *Getter) Stop(ctx context.Context) error { return sg.archivalPeerManager.Stop(ctx) } -func (sg *Getter) GetShare(context.Context, *header.ExtendedHeader, int, int) (libshare.Share, error) { - return libshare.Share{}, fmt.Errorf("getter/shrex: GetShare %w", shwap.ErrOperationNotSupported) +func (sg *Getter) GetSamples(context.Context, *header.ExtendedHeader, []shwap.SampleIndex) ([]shwap.Sample, error) { + return nil, fmt.Errorf("getter/shrex: GetShare %w", shwap.ErrOperationNotSupported) } func (sg *Getter) GetEDS(ctx context.Context, header *header.ExtendedHeader) (*rsmt2d.ExtendedDataSquare, error) { diff --git a/share/shwap/sample.go b/share/shwap/sample.go index ab07d2f5de..06fa84ab04 100644 --- a/share/shwap/sample.go +++ b/share/shwap/sample.go @@ -31,8 +31,13 @@ type Sample struct { // SampleFromShares creates a Sample from a list of shares, using the specified proof type and // the share index to be included in the sample. -func SampleFromShares(shares []libshare.Share, proofType rsmt2d.Axis, axisIdx, shrIdx int) (Sample, error) { - tree := wrapper.NewErasuredNamespacedMerkleTree(uint64(len(shares)/2), uint(axisIdx)) +func SampleFromShares(shares []libshare.Share, proofType rsmt2d.Axis, idx SampleIndex) (Sample, error) { + rowIdx, colIdx, err := idx.Coordinates(len(shares)) + if err != nil { + return Sample{}, err + } + + tree := wrapper.NewErasuredNamespacedMerkleTree(uint64(len(shares)/2), uint(rowIdx)) for _, shr := range shares { err := tree.Push(shr.ToBytes()) if err != nil { @@ -40,13 +45,13 @@ func SampleFromShares(shares []libshare.Share, proofType rsmt2d.Axis, axisIdx, s } } - proof, err := tree.ProveRange(shrIdx, shrIdx+1) + proof, err := tree.ProveRange(colIdx, colIdx+1) if err != nil { return Sample{}, err } return Sample{ - Share: shares[shrIdx], + Share: shares[colIdx], Proof: &proof, ProofType: proofType, }, nil diff --git a/share/shwap/sample_id.go b/share/shwap/sample_id.go index b03bbacfda..f2f3c9c2f4 100644 --- a/share/shwap/sample_id.go +++ b/share/shwap/sample_id.go @@ -10,6 +10,26 @@ import ( // bytes for the ShareIndex. const SampleIDSize = RowIDSize + 2 +type SampleIndex int + +func SampleIndexFromCoordinates(rowIdx, colIdx, edsSize int) (SampleIndex, error) { + if rowIdx < 0 || colIdx < 0 { + return 0, fmt.Errorf("negative row or col index: %w", ErrInvalidID) + } + if rowIdx >= edsSize || colIdx >= edsSize { + return 0, fmt.Errorf("SampleIndex %d || %d > %d: %w", rowIdx, colIdx, edsSize, ErrOutOfBounds) + } + return SampleIndex(rowIdx*edsSize + colIdx), nil +} + +func (s SampleIndex) Coordinates(squareSize int) (int, int, error) { + if int(s) > squareSize*squareSize { + return 0, 0, fmt.Errorf("SampleIndex %d > %d: %w", s, squareSize*squareSize, ErrOutOfBounds) + } + + return int(s) / squareSize, int(s) % squareSize, nil +} + // SampleID uniquely identifies a specific sample within a row of an Extended Data Square (EDS). type SampleID struct { RowID // Embeds RowID to incorporate block height and row index. @@ -18,7 +38,12 @@ type SampleID struct { // NewSampleID constructs a new SampleID using the provided block height, sample index, and EDS // size. It calculates the row and share index based on the sample index and EDS size. -func NewSampleID(height uint64, rowIdx, colIdx, edsSize int) (SampleID, error) { +func NewSampleID(height uint64, idx SampleIndex, edsSize int) (SampleID, error) { + rowIdx, colIdx, err := idx.Coordinates(edsSize) + if err != nil { + return SampleID{}, err + } + sid := SampleID{ RowID: RowID{ EdsID: EdsID{ diff --git a/share/shwap/sample_id_test.go b/share/shwap/sample_id_test.go index 125d536854..2344c932ea 100644 --- a/share/shwap/sample_id_test.go +++ b/share/shwap/sample_id_test.go @@ -11,7 +11,7 @@ import ( func TestSampleID(t *testing.T) { edsSize := 4 - id, err := NewSampleID(1, 1, 1, edsSize) + id, err := NewSampleID(1, 1, edsSize) require.NoError(t, err) data, err := id.MarshalBinary() @@ -29,7 +29,7 @@ func TestSampleID(t *testing.T) { func TestSampleIDReaderWriter(t *testing.T) { edsSize := 4 - id, err := NewSampleID(1, 1, 1, edsSize) + id, err := NewSampleID(1, 1, edsSize) require.NoError(t, err) buf := bytes.NewBuffer(nil) @@ -44,3 +44,15 @@ func TestSampleIDReaderWriter(t *testing.T) { require.EqualValues(t, id, sidOut) } + +func TestSampleIndex(t *testing.T) { + edsSize := 16 + + idxIn := SampleIndex(13 * 16) + row, col, err := idxIn.Coordinates(edsSize) + require.NoError(t, err) + + idxOut, err := SampleIndexFromCoordinates(row, col, edsSize) + require.NoError(t, err) + assert.Equal(t, idxIn, idxOut) +} diff --git a/store/file/ods.go b/store/file/ods.go index 3ec5082b83..a4f1313a6e 100644 --- a/store/file/ods.go +++ b/store/file/ods.go @@ -246,7 +246,12 @@ func (o *ODS) Sample(ctx context.Context, rowIdx, colIdx int) (shwap.Sample, err return shwap.Sample{}, fmt.Errorf("reading axis: %w", err) } - return shwap.SampleFromShares(axis, axisType, axisIdx, shrIdx) + idx, err := shwap.SampleIndexFromCoordinates(rowIdx, shrIdx, o.Size(ctx)) + if err != nil { + return shwap.Sample{}, err + } + + return shwap.SampleFromShares(axis, axisType, idx) } // AxisHalf returns half of shares axis of the given type and index. Side is determined by diff --git a/store/file/ods_q4.go b/store/file/ods_q4.go index 06b255cae9..cea6492e84 100644 --- a/store/file/ods_q4.go +++ b/store/file/ods_q4.go @@ -132,7 +132,12 @@ func (odsq4 *ODSQ4) Sample(ctx context.Context, rowIdx, colIdx int) (shwap.Sampl if err != nil { return shwap.Sample{}, fmt.Errorf("extending shares: %w", err) } - return shwap.SampleFromShares(shares, rsmt2d.Row, rowIdx, colIdx) + + idx, err := shwap.SampleIndexFromCoordinates(rowIdx, colIdx, odsq4.Size(ctx)) + if err != nil { + return shwap.Sample{}, err + } + return shwap.SampleFromShares(shares, rsmt2d.Row, idx) } func (odsq4 *ODSQ4) AxisHalf(ctx context.Context, axisType rsmt2d.Axis, axisIdx int) (eds.AxisHalf, error) { diff --git a/store/getter.go b/store/getter.go index 54ce70e4ae..cfacede23d 100644 --- a/store/getter.go +++ b/store/getter.go @@ -24,26 +24,32 @@ func NewGetter(store *Store) *Getter { return &Getter{store: store} } -func (g *Getter) GetShare(ctx context.Context, h *header.ExtendedHeader, row, col int) (libshare.Share, error) { - acc, err := g.store.GetByHeight(ctx, h.Height()) +func (g *Getter) GetSamples(ctx context.Context, hdr *header.ExtendedHeader, indices []shwap.SampleIndex) ([]shwap.Sample, error) { + acc, err := g.store.GetByHeight(ctx, hdr.Height()) if err != nil { if errors.Is(err, ErrNotFound) { - return libshare.Share{}, shwap.ErrNotFound + return nil, shwap.ErrNotFound } - return libshare.Share{}, fmt.Errorf("get accessor from store:%w", err) + return nil, fmt.Errorf("get accessor from store:%w", err) } - logger := log.With( - "height", h.Height(), - "row", row, - "col", col, - ) - defer utils.CloseAndLog(logger, "getter/sample", acc) + defer utils.CloseAndLog(log.With("height", hdr.Height()), "getter/sample", acc) - sample, err := acc.Sample(ctx, row, col) - if err != nil { - return libshare.Share{}, fmt.Errorf("get sample from accessor:%w", err) + smpls := make([]shwap.Sample, len(indices)) + for i, idx := range indices { + rowIdx, colIdx, err := idx.Coordinates(len(hdr.DAH.RowRoots)) + if err != nil { + return nil, err + } + + smpl, err := acc.Sample(ctx, rowIdx, colIdx) + if err != nil { + return nil, fmt.Errorf("get sample from accessor:%w", err) + } + + smpls[i] = smpl } - return sample.Share, nil + + return smpls, nil } func (g *Getter) GetEDS(ctx context.Context, h *header.ExtendedHeader) (*rsmt2d.ExtendedDataSquare, error) { From 5be2291055df5fb254428c2043a17a35a6fa0a0a Mon Sep 17 00:00:00 2001 From: Oleg Kovalov Date: Wed, 6 Nov 2024 15:45:45 +0100 Subject: [PATCH 02/26] refactor(share): GetShare -> GetSamples (#3905) --- blob/service_test.go | 30 +++++++++++++------ nodebuilder/share/share.go | 20 +++++++++++++ share/availability/light/availability.go | 4 +-- share/availability/light/availability_test.go | 11 +++---- share/eds/accessor.go | 3 +- share/eds/close_once.go | 4 +-- share/eds/close_once_test.go | 6 ++-- share/eds/proofs_cache.go | 7 ++++- share/eds/rsmt2d.go | 11 +++++-- share/eds/rsmt2d_test.go | 5 +++- share/eds/testing.go | 30 ++++++++++++------- share/eds/validation.go | 11 ++----- share/eds/validation_test.go | 9 +++++- share/shwap/getters/cascade.go | 4 ++- share/shwap/getters/testing.go | 11 +++---- share/shwap/p2p/bitswap/sample_block.go | 7 ++++- share/shwap/sample_test.go | 21 ++++++++----- store/cache/accessor_cache_test.go | 2 +- store/cache/noop.go | 2 +- store/file/ods.go | 9 ++++-- store/file/ods_q4.go | 12 ++++---- store/getter.go | 11 +++---- store/getter_test.go | 8 +++-- 23 files changed, 153 insertions(+), 85 deletions(-) diff --git a/blob/service_test.go b/blob/service_test.go index 9b47b74cb8..d7e44e2aa7 100644 --- a/blob/service_test.go +++ b/blob/service_test.go @@ -145,9 +145,11 @@ func TestBlobService_Get(t *testing.T) { shareOffset := 0 for i := range blobs { row, col := calculateIndex(len(h.DAH.RowRoots), blobs[i].index) - sh, err := service.shareGetter.GetShare(ctx, h, row, col) + idx, err := shwap.SampleIndexFromCoordinates(row, col, len(h.DAH.RowRoots)) require.NoError(t, err) - require.True(t, bytes.Equal(sh.ToBytes(), resultShares[shareOffset].ToBytes()), + smpls, err := service.shareGetter.GetSamples(ctx, h, []shwap.SampleIndex{idx}) + require.NoError(t, err) + require.True(t, bytes.Equal(smpls[0].Share.ToBytes(), resultShares[shareOffset].ToBytes()), fmt.Sprintf("issue on %d attempt. ROW:%d, COL: %d, blobIndex:%d", i, row, col, blobs[i].index), ) shareOffset += libshare.SparseSharesNeeded(uint32(len(blobs[i].Data()))) @@ -487,10 +489,13 @@ func TestService_GetSingleBlobWithoutPadding(t *testing.T) { h, err := service.headerGetter(ctx, 1) require.NoError(t, err) row, col := calculateIndex(len(h.DAH.RowRoots), newBlob.index) - sh, err := service.shareGetter.GetShare(ctx, h, row, col) + idx, err := shwap.SampleIndexFromCoordinates(row, col, len(h.DAH.RowRoots)) + require.NoError(t, err) + + smpls, err := service.shareGetter.GetSamples(ctx, h, []shwap.SampleIndex{idx}) require.NoError(t, err) - assert.Equal(t, sh, resultShares[0]) + assert.Equal(t, smpls[0].Share, resultShares[0]) } func TestService_Get(t *testing.T) { @@ -521,10 +526,13 @@ func TestService_Get(t *testing.T) { assert.Equal(t, b.Commitment, blob.Commitment) row, col := calculateIndex(len(h.DAH.RowRoots), b.index) - sh, err := service.shareGetter.GetShare(ctx, h, row, col) + idx, err := shwap.SampleIndexFromCoordinates(row, col, len(h.DAH.RowRoots)) require.NoError(t, err) - assert.Equal(t, sh, resultShares[shareOffset], fmt.Sprintf("issue on %d attempt", i)) + smpls, err := service.shareGetter.GetSamples(ctx, h, []shwap.SampleIndex{idx}) + require.NoError(t, err) + + assert.Equal(t, smpls[0].Share, resultShares[shareOffset], fmt.Sprintf("issue on %d attempt", i)) shareOffset += libshare.SparseSharesNeeded(uint32(len(blob.Data()))) } } @@ -580,10 +588,13 @@ func TestService_GetAllWithoutPadding(t *testing.T) { require.True(t, blobs[i].compareCommitments(blob.Commitment)) row, col := calculateIndex(len(h.DAH.RowRoots), blob.index) - sh, err := service.shareGetter.GetShare(ctx, h, row, col) + idx, err := shwap.SampleIndexFromCoordinates(row, col, len(h.DAH.RowRoots)) + require.NoError(t, err) + + smpls, err := service.shareGetter.GetSamples(ctx, h, []shwap.SampleIndex{idx}) require.NoError(t, err) - assert.Equal(t, sh, resultShares[shareOffset]) + assert.Equal(t, smpls[0].Share, resultShares[shareOffset]) shareOffset += libshare.SparseSharesNeeded(uint32(len(blob.Data()))) } } @@ -904,7 +915,8 @@ func createService(ctx context.Context, t testing.TB, shares []libshare.Share) * }) shareGetter.EXPECT().GetSamples(gomock.Any(), gomock.Any(), gomock.Any()).AnyTimes(). DoAndReturn(func(ctx context.Context, h *header.ExtendedHeader, indices []shwap.SampleIndex) ([]shwap.Sample, error) { - return smpls, nil + smpl, err := accessor.Sample(ctx, indices[0]) + return []shwap.Sample{smpl}, err }) // create header and put it into the store diff --git a/nodebuilder/share/share.go b/nodebuilder/share/share.go index 9a904b48fd..887e6f27a0 100644 --- a/nodebuilder/share/share.go +++ b/nodebuilder/share/share.go @@ -8,6 +8,7 @@ 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" @@ -45,6 +46,8 @@ type Module interface { SharesAvailable(ctx context.Context, height uint64) error // GetShare gets a Share by coordinates in EDS. GetShare(ctx context.Context, height uint64, row, col int) (libshare.Share, error) + // GetSamples gets sample for given indices. + GetSamples(ctx context.Context, header *header.ExtendedHeader, indices []shwap.SampleIndex) ([]shwap.Sample, error) // GetEDS gets the full EDS identified by the given extended header. GetEDS(ctx context.Context, height uint64) (*rsmt2d.ExtendedDataSquare, error) // GetNamespaceData gets all shares from an EDS within the given namespace. @@ -65,6 +68,11 @@ type API struct { height uint64, row, col int, ) (libshare.Share, error) `perm:"read"` + GetSamples func( + ctx context.Context, + header *header.ExtendedHeader, + indices []shwap.SampleIndex, + ) ([]shwap.Sample, error) `perm:"read"` GetEDS func( ctx context.Context, height uint64, @@ -90,6 +98,12 @@ func (api *API) GetShare(ctx context.Context, height uint64, row, col int) (libs return api.Internal.GetShare(ctx, height, row, col) } +func (api *API) GetSamples(ctx context.Context, header *header.ExtendedHeader, + indices []shwap.SampleIndex, +) ([]shwap.Sample, error) { + return api.Internal.GetSamples(ctx, header, indices) +} + func (api *API) GetEDS(ctx context.Context, height uint64) (*rsmt2d.ExtendedDataSquare, error) { return api.Internal.GetEDS(ctx, height) } @@ -132,6 +146,12 @@ func (m module) GetShare(ctx context.Context, height uint64, row, col int) (libs return smpls[0].Share, nil } +func (m module) GetSamples(ctx context.Context, header *header.ExtendedHeader, + indices []shwap.SampleIndex, +) ([]shwap.Sample, error) { + return m.getter.GetSamples(ctx, header, indices) +} + func (m module) GetEDS(ctx context.Context, height uint64) (*rsmt2d.ExtendedDataSquare, error) { header, err := m.hs.GetByHeight(ctx, height) if err != nil { diff --git a/share/availability/light/availability.go b/share/availability/light/availability.go index a9e60d486a..f31bada53a 100644 --- a/share/availability/light/availability.go +++ b/share/availability/light/availability.go @@ -126,10 +126,10 @@ func (la *ShareAvailability) SharesAvailable(ctx context.Context, header *header } smpls, err := la.getter.GetSamples(ctx, header, idxs) - if errors.Is(ctx.Err(), context.Canceled) { + if errors.Is(err, context.Canceled) { // Availability did not complete due to context cancellation, return context error instead of // share.ErrNotAvailable - return ctx.Err() + return err } if len(smpls) == 0 { return share.ErrNotAvailable diff --git a/share/availability/light/availability_test.go b/share/availability/light/availability_test.go index 6974c3dc8d..d40c987383 100644 --- a/share/availability/light/availability_test.go +++ b/share/availability/light/availability_test.go @@ -53,12 +53,7 @@ func TestSharesAvailableSuccess(t *testing.T) { acc := eds.Rsmt2D{ExtendedDataSquare: square} smpls := make([]shwap.Sample, len(indices)) for i, idx := range indices { - rowIdx, colIdx, err := idx.Coordinates(len(hdr.DAH.RowRoots)) - if err != nil { - return nil, err - } - - smpl, err := acc.Sample(ctx, rowIdx, colIdx) + smpl, err := acc.Sample(ctx, idx) if err != nil { return nil, err } @@ -292,7 +287,9 @@ func (g onceGetter) checkOnce(t *testing.T) { } } -func (m onceGetter) GetSamples(_ context.Context, hdr *header.ExtendedHeader, indices []shwap.SampleIndex) ([]shwap.Sample, error) { +func (m onceGetter) GetSamples(_ context.Context, hdr *header.ExtendedHeader, + indices []shwap.SampleIndex, +) ([]shwap.Sample, error) { m.Lock() defer m.Unlock() diff --git a/share/eds/accessor.go b/share/eds/accessor.go index 81f55fcf64..e262382790 100644 --- a/share/eds/accessor.go +++ b/share/eds/accessor.go @@ -25,8 +25,7 @@ type Accessor interface { // Sample returns share and corresponding proof for row and column indices. Implementation can // choose which axis to use for proof. Chosen axis for proof should be indicated in the returned // Sample. - // TODO(@Wondertan): change to SampleIndex - Sample(ctx context.Context, rowIdx, colIdx int) (shwap.Sample, error) + Sample(ctx context.Context, idx shwap.SampleIndex) (shwap.Sample, error) // AxisHalf returns half of shares axis of the given type and index. Side is determined by // implementation. Implementations should indicate the side in the returned AxisHalf. AxisHalf(ctx context.Context, axisType rsmt2d.Axis, axisIdx int) (AxisHalf, error) diff --git a/share/eds/close_once.go b/share/eds/close_once.go index cc217710ce..6de62419fa 100644 --- a/share/eds/close_once.go +++ b/share/eds/close_once.go @@ -57,11 +57,11 @@ func (c *closeOnce) AxisRoots(ctx context.Context) (*share.AxisRoots, error) { return c.f.AxisRoots(ctx) } -func (c *closeOnce) Sample(ctx context.Context, rowIdx, colIdx int) (shwap.Sample, error) { +func (c *closeOnce) Sample(ctx context.Context, idx shwap.SampleIndex) (shwap.Sample, error) { if c.closed.Load() { return shwap.Sample{}, errAccessorClosed } - return c.f.Sample(ctx, rowIdx, colIdx) + return c.f.Sample(ctx, idx) } func (c *closeOnce) AxisHalf( diff --git a/share/eds/close_once_test.go b/share/eds/close_once_test.go index d515ac7bda..59b4452174 100644 --- a/share/eds/close_once_test.go +++ b/share/eds/close_once_test.go @@ -20,7 +20,7 @@ func TestWithClosedOnce(t *testing.T) { stub := &stubEdsAccessorCloser{} closedOnce := WithClosedOnce(stub) - _, err := closedOnce.Sample(ctx, 0, 0) + _, err := closedOnce.Sample(ctx, 0) require.NoError(t, err) _, err = closedOnce.AxisHalf(ctx, rsmt2d.Row, 0) require.NoError(t, err) @@ -33,7 +33,7 @@ func TestWithClosedOnce(t *testing.T) { require.True(t, stub.closed) // Ensure that the underlying file is not accessible after closing - _, err = closedOnce.Sample(ctx, 0, 0) + _, err = closedOnce.Sample(ctx, 0) require.ErrorIs(t, err, errAccessorClosed) _, err = closedOnce.AxisHalf(ctx, rsmt2d.Row, 0) require.ErrorIs(t, err, errAccessorClosed) @@ -59,7 +59,7 @@ func (s *stubEdsAccessorCloser) AxisRoots(context.Context) (*share.AxisRoots, er return &share.AxisRoots{}, nil } -func (s *stubEdsAccessorCloser) Sample(context.Context, int, int) (shwap.Sample, error) { +func (s *stubEdsAccessorCloser) Sample(context.Context, shwap.SampleIndex) (shwap.Sample, error) { return shwap.Sample{}, nil } diff --git a/share/eds/proofs_cache.go b/share/eds/proofs_cache.go index e777b82962..006d1e46ac 100644 --- a/share/eds/proofs_cache.go +++ b/share/eds/proofs_cache.go @@ -112,7 +112,12 @@ func (c *proofsCache) AxisRoots(ctx context.Context) (*share.AxisRoots, error) { return roots, nil } -func (c *proofsCache) Sample(ctx context.Context, rowIdx, colIdx int) (shwap.Sample, error) { +func (c *proofsCache) Sample(ctx context.Context, idx shwap.SampleIndex) (shwap.Sample, error) { + rowIdx, colIdx, err := idx.Coordinates(c.Size(ctx)) + if err != nil { + return shwap.Sample{}, err + } + axisType, axisIdx, shrIdx := rsmt2d.Row, rowIdx, colIdx ax, err := c.axisWithProofs(ctx, axisType, axisIdx) if err != nil { diff --git a/share/eds/rsmt2d.go b/share/eds/rsmt2d.go index e0e945fccb..d7ee5376a5 100644 --- a/share/eds/rsmt2d.go +++ b/share/eds/rsmt2d.go @@ -46,17 +46,22 @@ func (eds *Rsmt2D) AxisRoots(context.Context) (*share.AxisRoots, error) { // Sample returns share and corresponding proof for row and column indices. func (eds *Rsmt2D) Sample( _ context.Context, - rowIdx, colIdx int, + idx shwap.SampleIndex, ) (shwap.Sample, error) { - return eds.SampleForProofAxis(rowIdx, colIdx, rsmt2d.Row) + return eds.SampleForProofAxis(idx, rsmt2d.Row) } // SampleForProofAxis samples a share from an Extended Data Square based on the provided // row and column indices and proof axis. It returns a sample with the share and proof. func (eds *Rsmt2D) SampleForProofAxis( - rowIdx, colIdx int, + idx shwap.SampleIndex, proofType rsmt2d.Axis, ) (shwap.Sample, error) { + rowIdx, colIdx, err := idx.Coordinates(int(eds.Width())) + if err != nil { + return shwap.Sample{}, err + } + axisIdx, shrIdx := relativeIndexes(rowIdx, colIdx, proofType) shares, err := getAxis(eds.ExtendedDataSquare, proofType, axisIdx) if err != nil { diff --git a/share/eds/rsmt2d_test.go b/share/eds/rsmt2d_test.go index 96bde8c2ab..4ae001704f 100644 --- a/share/eds/rsmt2d_test.go +++ b/share/eds/rsmt2d_test.go @@ -56,7 +56,10 @@ func TestRsmt2dSampleForProofAxis(t *testing.T) { for _, proofType := range []rsmt2d.Axis{rsmt2d.Row, rsmt2d.Col} { for rowIdx := 0; rowIdx < odsSize*2; rowIdx++ { for colIdx := 0; colIdx < odsSize*2; colIdx++ { - sample, err := accessor.SampleForProofAxis(rowIdx, colIdx, proofType) + idx, err := shwap.SampleIndexFromCoordinates(rowIdx, colIdx, accessor.Size(context.Background())) + require.NoError(t, err) + + sample, err := accessor.SampleForProofAxis(idx, proofType) require.NoError(t, err) want := eds.GetCell(uint(rowIdx), uint(colIdx)) diff --git a/share/eds/testing.go b/share/eds/testing.go index 388544bc00..c4373780fb 100644 --- a/share/eds/testing.go +++ b/share/eds/testing.go @@ -148,7 +148,9 @@ func testAccessorSample( // t.Parallel() this fails the test for some reason for rowIdx := 0; rowIdx < width; rowIdx++ { for colIdx := 0; colIdx < width; colIdx++ { - testSample(ctx, t, acc, roots, colIdx, rowIdx) + idx, err := shwap.SampleIndexFromCoordinates(rowIdx, colIdx, acc.Size(ctx)) + require.NoError(t, err) + testSample(ctx, t, acc, roots, idx) } } }) @@ -162,10 +164,12 @@ func testAccessorSample( for rowIdx := 0; rowIdx < width; rowIdx++ { for colIdx := 0; colIdx < width; colIdx++ { wg.Add(1) - go func(rowIdx, colIdx int) { + idx, err := shwap.SampleIndexFromCoordinates(rowIdx, colIdx, acc.Size(ctx)) + require.NoError(t, err) + go func(idx shwap.SampleIndex) { defer wg.Done() - testSample(ctx, t, acc, roots, rowIdx, colIdx) - }(rowIdx, colIdx) + testSample(ctx, t, acc, roots, idx) + }(idx) } } wg.Wait() @@ -182,8 +186,8 @@ func testAccessorSample( wg.Add(1) go func() { defer wg.Done() - rowIdx, colIdx := rand.IntN(width), rand.IntN(width) //nolint:gosec - testSample(ctx, t, acc, roots, rowIdx, colIdx) + idx := rand.IntN(int(eds.Width())) //nolint:gosec + testSample(ctx, t, acc, roots, shwap.SampleIndex(idx)) }() } wg.Wait() @@ -195,9 +199,12 @@ func testSample( t *testing.T, acc Accessor, roots *share.AxisRoots, - rowIdx, colIdx int, + idx shwap.SampleIndex, ) { - shr, err := acc.Sample(ctx, rowIdx, colIdx) + shr, err := acc.Sample(ctx, idx) + require.NoError(t, err) + + rowIdx, colIdx, err := idx.Coordinates(acc.Size(ctx)) require.NoError(t, err) err = shr.Verify(roots, rowIdx, colIdx) @@ -444,13 +451,16 @@ func BenchGetSampleFromAccessor( name := fmt.Sprintf("Size:%v/quadrant:%s", size, q) b.Run(name, func(b *testing.B) { rowIdx, colIdx := q.coordinates(acc.Size(ctx)) + idx, err := shwap.SampleIndexFromCoordinates(rowIdx, colIdx, acc.Size(ctx)) + require.NoError(b, err) + // warm up cache - _, err := acc.Sample(ctx, rowIdx, colIdx) + _, err = acc.Sample(ctx, idx) require.NoError(b, err, q.String()) b.ResetTimer() for i := 0; i < b.N; i++ { - _, err := acc.Sample(ctx, rowIdx, colIdx) + _, err := acc.Sample(ctx, idx) require.NoError(b, err) } }) diff --git a/share/eds/validation.go b/share/eds/validation.go index 0d513911d9..d3323f87ac 100644 --- a/share/eds/validation.go +++ b/share/eds/validation.go @@ -34,17 +34,12 @@ func (f validation) Size(ctx context.Context) int { return int(size) } -func (f validation) Sample(ctx context.Context, rowIdx, colIdx int) (shwap.Sample, error) { - idx, err := shwap.SampleIndexFromCoordinates(rowIdx, colIdx, f.Size(ctx)) - if err != nil { - return shwap.Sample{}, err - } - - _, err = shwap.NewSampleID(1, idx, f.Size(ctx)) +func (f validation) Sample(ctx context.Context, idx shwap.SampleIndex) (shwap.Sample, error) { + _, err := shwap.NewSampleID(1, idx, f.Size(ctx)) if err != nil { return shwap.Sample{}, fmt.Errorf("sample validation: %w", err) } - return f.Accessor.Sample(ctx, rowIdx, colIdx) + return f.Accessor.Sample(ctx, idx) } func (f validation) AxisHalf(ctx context.Context, axisType rsmt2d.Axis, axisIdx int) (AxisHalf, error) { diff --git a/share/eds/validation_test.go b/share/eds/validation_test.go index 3e645cbfb3..fbe17868a9 100644 --- a/share/eds/validation_test.go +++ b/share/eds/validation_test.go @@ -34,7 +34,14 @@ func TestValidation_Sample(t *testing.T) { accessor := &Rsmt2D{ExtendedDataSquare: randEDS} validation := WithValidation(AccessorAndStreamer(accessor, nil)) - _, err := validation.Sample(context.Background(), tt.rowIdx, tt.colIdx) + idx, err := shwap.SampleIndexFromCoordinates(tt.rowIdx, tt.colIdx, accessor.Size(context.Background())) + if tt.expectFail { + require.ErrorIs(t, err, shwap.ErrInvalidID, tt.name) + return + } + require.NoError(t, err, tt.name) + + _, err = validation.Sample(context.Background(), idx) if tt.expectFail { require.ErrorIs(t, err, shwap.ErrInvalidID) } else { diff --git a/share/shwap/getters/cascade.go b/share/shwap/getters/cascade.go index 3518ce8eea..8b0ce5349c 100644 --- a/share/shwap/getters/cascade.go +++ b/share/shwap/getters/cascade.go @@ -42,7 +42,9 @@ func NewCascadeGetter(getters []shwap.Getter) *CascadeGetter { } // GetSamples gets samples from any of registered shwap.Getters in cascading order. -func (cg *CascadeGetter) GetSamples(ctx context.Context, hdr *header.ExtendedHeader, indices []shwap.SampleIndex) ([]shwap.Sample, error) { +func (cg *CascadeGetter) GetSamples(ctx context.Context, hdr *header.ExtendedHeader, + indices []shwap.SampleIndex, +) ([]shwap.Sample, error) { ctx, span := tracer.Start(ctx, "cascade/get-samples", trace.WithAttributes( attribute.Int("amount", len(indices)), )) diff --git a/share/shwap/getters/testing.go b/share/shwap/getters/testing.go index 3d7338a2fb..3b1f4bb8c9 100644 --- a/share/shwap/getters/testing.go +++ b/share/shwap/getters/testing.go @@ -37,7 +37,9 @@ type SingleEDSGetter struct { } // GetSamples get samples from a kept EDS if exist and if the correct root is given. -func (seg *SingleEDSGetter) GetSamples(ctx context.Context, hdr *header.ExtendedHeader, indices []shwap.SampleIndex) ([]shwap.Sample, error) { +func (seg *SingleEDSGetter) GetSamples(ctx context.Context, hdr *header.ExtendedHeader, + indices []shwap.SampleIndex, +) ([]shwap.Sample, error) { err := seg.checkRoots(hdr.DAH) if err != nil { return nil, err @@ -45,12 +47,7 @@ func (seg *SingleEDSGetter) GetSamples(ctx context.Context, hdr *header.Extended smpls := make([]shwap.Sample, len(indices)) for i, idx := range indices { - rowIdx, colIdx, err := idx.Coordinates(len(hdr.DAH.RowRoots)) - if err != nil { - return nil, err - } - - smpl, err := seg.EDS.Sample(ctx, rowIdx, colIdx) + smpl, err := seg.EDS.Sample(ctx, idx) if err != nil { return nil, err } diff --git a/share/shwap/p2p/bitswap/sample_block.go b/share/shwap/p2p/bitswap/sample_block.go index bf4ca43774..f7aa01b642 100644 --- a/share/shwap/p2p/bitswap/sample_block.go +++ b/share/shwap/p2p/bitswap/sample_block.go @@ -94,7 +94,12 @@ func (sb *SampleBlock) Marshal() ([]byte, error) { } func (sb *SampleBlock) Populate(ctx context.Context, eds eds.Accessor) error { - smpl, err := eds.Sample(ctx, sb.ID.RowIndex, sb.ID.ShareIndex) + idx, err := shwap.SampleIndexFromCoordinates(sb.ID.RowIndex, sb.ID.ShareIndex, eds.Size(ctx)) + if err != nil { + return err + } + + smpl, err := eds.Sample(ctx, idx) if err != nil { return fmt.Errorf("accessing Sample: %w", err) } diff --git a/share/shwap/sample_test.go b/share/shwap/sample_test.go index 030eeb4677..2a25d369d8 100644 --- a/share/shwap/sample_test.go +++ b/share/shwap/sample_test.go @@ -25,10 +25,13 @@ func TestSampleValidate(t *testing.T) { for _, proofType := range []rsmt2d.Axis{rsmt2d.Row, rsmt2d.Col} { for rowIdx := 0; rowIdx < odsSize*2; rowIdx++ { for colIdx := 0; colIdx < odsSize*2; colIdx++ { - sample, err := inMem.SampleForProofAxis(rowIdx, colIdx, proofType) + idx, err := shwap.SampleIndexFromCoordinates(rowIdx, colIdx, int(randEDS.Width())) require.NoError(t, err) - require.NoError(t, sample.Verify(root, rowIdx, colIdx)) + sample, err := inMem.SampleForProofAxis(idx, proofType) + require.NoError(t, err) + + require.NoError(t, sample.Verify(root, rowIdx, colIdx), "row: %d col: %d", rowIdx, colIdx) } } } @@ -42,7 +45,7 @@ func TestSampleNegativeVerifyInclusion(t *testing.T) { require.NoError(t, err) inMem := eds.Rsmt2D{ExtendedDataSquare: randEDS} - sample, err := inMem.Sample(context.Background(), 0, 0) + sample, err := inMem.Sample(context.Background(), 0) require.NoError(t, err) err = sample.Verify(root, 0, 0) require.NoError(t, err) @@ -61,14 +64,14 @@ func TestSampleNegativeVerifyInclusion(t *testing.T) { require.ErrorIs(t, err, shwap.ErrFailedVerification) // incorrect proofType - sample, err = inMem.Sample(context.Background(), 0, 0) + sample, err = inMem.Sample(context.Background(), 0) require.NoError(t, err) sample.ProofType = rsmt2d.Col err = sample.Verify(root, 0, 0) require.ErrorIs(t, err, shwap.ErrFailedVerification) // Corrupt the last root hash byte - sample, err = inMem.Sample(context.Background(), 0, 0) + sample, err = inMem.Sample(context.Background(), 0) require.NoError(t, err) root.RowRoots[0][len(root.RowRoots[0])-1] ^= 0xFF err = sample.Verify(root, 0, 0) @@ -83,7 +86,10 @@ func TestSampleProtoEncoding(t *testing.T) { for _, proofType := range []rsmt2d.Axis{rsmt2d.Row, rsmt2d.Col} { for rowIdx := 0; rowIdx < odsSize*2; rowIdx++ { for colIdx := 0; colIdx < odsSize*2; colIdx++ { - sample, err := inMem.SampleForProofAxis(rowIdx, colIdx, proofType) + idx, err := shwap.SampleIndexFromCoordinates(rowIdx, colIdx, int(randEDS.Width())) + require.NoError(t, err) + + sample, err := inMem.SampleForProofAxis(idx, proofType) require.NoError(t, err) pb := sample.ToProto() @@ -103,7 +109,8 @@ func BenchmarkSampleValidate(b *testing.B) { root, err := share.NewAxisRoots(randEDS) require.NoError(b, err) inMem := eds.Rsmt2D{ExtendedDataSquare: randEDS} - sample, err := inMem.SampleForProofAxis(0, 0, rsmt2d.Row) + + sample, err := inMem.SampleForProofAxis(0, rsmt2d.Row) require.NoError(b, err) b.ResetTimer() diff --git a/store/cache/accessor_cache_test.go b/store/cache/accessor_cache_test.go index 9c7104fbe7..82073a722c 100644 --- a/store/cache/accessor_cache_test.go +++ b/store/cache/accessor_cache_test.go @@ -315,7 +315,7 @@ func (m *mockAccessor) AxisRoots(context.Context) (*share.AxisRoots, error) { panic("implement me") } -func (m *mockAccessor) Sample(context.Context, int, int) (shwap.Sample, error) { +func (m *mockAccessor) Sample(context.Context, shwap.SampleIndex) (shwap.Sample, error) { panic("implement me") } diff --git a/store/cache/noop.go b/store/cache/noop.go index d777fdb2e4..2ccc87f387 100644 --- a/store/cache/noop.go +++ b/store/cache/noop.go @@ -59,7 +59,7 @@ func (n NoopFile) AxisRoots(context.Context) (*share.AxisRoots, error) { return &share.AxisRoots{}, nil } -func (n NoopFile) Sample(context.Context, int, int) (shwap.Sample, error) { +func (n NoopFile) Sample(context.Context, shwap.SampleIndex) (shwap.Sample, error) { return shwap.Sample{}, nil } diff --git a/store/file/ods.go b/store/file/ods.go index a4f1313a6e..0e7efd88bc 100644 --- a/store/file/ods.go +++ b/store/file/ods.go @@ -228,7 +228,7 @@ func (o *ODS) Close() error { // Sample returns share and corresponding proof for row and column indices. Implementation can // choose which axis to use for proof. Chosen axis for proof should be indicated in the returned // Sample. -func (o *ODS) Sample(ctx context.Context, rowIdx, colIdx int) (shwap.Sample, error) { +func (o *ODS) Sample(ctx context.Context, idx shwap.SampleIndex) (shwap.Sample, error) { // Sample proof axis is selected to optimize read performance. // - For the first and second quadrants, we read the row axis because it is more efficient to read // single row than reading full ODS to calculate single column @@ -236,6 +236,11 @@ func (o *ODS) Sample(ctx context.Context, rowIdx, colIdx int) (shwap.Sample, err // column than reading full ODS to calculate single row // - For the fourth quadrant, it does not matter which axis we read because we need to read full ODS // to calculate the sample + rowIdx, colIdx, err := idx.Coordinates(o.Size(ctx)) + if err != nil { + return shwap.Sample{}, err + } + axisType, axisIdx, shrIdx := rsmt2d.Row, rowIdx, colIdx if colIdx < o.size()/2 && rowIdx >= o.size()/2 { axisType, axisIdx, shrIdx = rsmt2d.Col, colIdx, rowIdx @@ -246,7 +251,7 @@ func (o *ODS) Sample(ctx context.Context, rowIdx, colIdx int) (shwap.Sample, err return shwap.Sample{}, fmt.Errorf("reading axis: %w", err) } - idx, err := shwap.SampleIndexFromCoordinates(rowIdx, shrIdx, o.Size(ctx)) + idx, err = shwap.SampleIndexFromCoordinates(axisIdx, shrIdx, o.Size(ctx)) if err != nil { return shwap.Sample{}, err } diff --git a/store/file/ods_q4.go b/store/file/ods_q4.go index cea6492e84..799a87e48f 100644 --- a/store/file/ods_q4.go +++ b/store/file/ods_q4.go @@ -122,9 +122,13 @@ func (odsq4 *ODSQ4) AxisRoots(ctx context.Context) (*share.AxisRoots, error) { return odsq4.ods.AxisRoots(ctx) } -func (odsq4 *ODSQ4) Sample(ctx context.Context, rowIdx, colIdx int) (shwap.Sample, error) { +func (odsq4 *ODSQ4) Sample(ctx context.Context, idx shwap.SampleIndex) (shwap.Sample, error) { + rowIdw, _, err := idx.Coordinates(odsq4.Size(ctx)) + if err != nil { + return shwap.Sample{}, err + } // use native AxisHalf implementation, to read axis from q4 quadrant when possible - half, err := odsq4.AxisHalf(ctx, rsmt2d.Row, rowIdx) + half, err := odsq4.AxisHalf(ctx, rsmt2d.Row, rowIdw) if err != nil { return shwap.Sample{}, fmt.Errorf("reading axis: %w", err) } @@ -133,10 +137,6 @@ func (odsq4 *ODSQ4) Sample(ctx context.Context, rowIdx, colIdx int) (shwap.Sampl return shwap.Sample{}, fmt.Errorf("extending shares: %w", err) } - idx, err := shwap.SampleIndexFromCoordinates(rowIdx, colIdx, odsq4.Size(ctx)) - if err != nil { - return shwap.Sample{}, err - } return shwap.SampleFromShares(shares, rsmt2d.Row, idx) } diff --git a/store/getter.go b/store/getter.go index cfacede23d..f0515b7fb0 100644 --- a/store/getter.go +++ b/store/getter.go @@ -24,7 +24,9 @@ func NewGetter(store *Store) *Getter { return &Getter{store: store} } -func (g *Getter) GetSamples(ctx context.Context, hdr *header.ExtendedHeader, indices []shwap.SampleIndex) ([]shwap.Sample, error) { +func (g *Getter) GetSamples(ctx context.Context, hdr *header.ExtendedHeader, + indices []shwap.SampleIndex, +) ([]shwap.Sample, error) { acc, err := g.store.GetByHeight(ctx, hdr.Height()) if err != nil { if errors.Is(err, ErrNotFound) { @@ -36,12 +38,7 @@ func (g *Getter) GetSamples(ctx context.Context, hdr *header.ExtendedHeader, ind smpls := make([]shwap.Sample, len(indices)) for i, idx := range indices { - rowIdx, colIdx, err := idx.Coordinates(len(hdr.DAH.RowRoots)) - if err != nil { - return nil, err - } - - smpl, err := acc.Sample(ctx, rowIdx, colIdx) + smpl, err := acc.Sample(ctx, idx) if err != nil { return nil, fmt.Errorf("get sample from accessor:%w", err) } diff --git a/store/getter_test.go b/store/getter_test.go index 47c0ce82a4..d80221c9f5 100644 --- a/store/getter_test.go +++ b/store/getter_test.go @@ -36,14 +36,16 @@ func TestStoreGetter(t *testing.T) { squareSize := int(eds.Width()) for i := 0; i < squareSize; i++ { for j := 0; j < squareSize; j++ { - share, err := sg.GetShare(ctx, eh, i, j) + idx, err := shwap.SampleIndexFromCoordinates(i, j, len(eh.DAH.RowRoots)) require.NoError(t, err) - require.Equal(t, eds.GetCell(uint(i), uint(j)), share.ToBytes()) + smpls, err := sg.GetSamples(ctx, eh, []shwap.SampleIndex{idx}) + require.NoError(t, err) + require.Equal(t, eds.GetCell(uint(i), uint(j)), smpls[0].Share.ToBytes()) } } // doesn't panic on indexes too high - _, err = sg.GetShare(ctx, eh, squareSize, squareSize) + _, err = sg.GetSamples(ctx, eh, []shwap.SampleIndex{shwap.SampleIndex(squareSize * squareSize)}) require.ErrorIs(t, err, shwap.ErrOutOfBounds) }) From cb4c1e363c8fed6f6f02bfc0ca0a2e76c90ed9e1 Mon Sep 17 00:00:00 2001 From: Oleg Kovalov Date: Fri, 8 Nov 2024 15:06:09 +0100 Subject: [PATCH 03/26] fix --- share/availability/light/availability.go | 15 +++++++++------ share/availability/light/availability_test.go | 11 ++++++----- 2 files changed, 15 insertions(+), 11 deletions(-) diff --git a/share/availability/light/availability.go b/share/availability/light/availability.go index f31bada53a..27cb4f69eb 100644 --- a/share/availability/light/availability.go +++ b/share/availability/light/availability.go @@ -115,9 +115,9 @@ func (la *ShareAvailability) SharesAvailable(ctx context.Context, header *header log.Debugw("starting sampling session", "root", dah.String()) - idxs := make([]shwap.SampleIndex, len(samples.Available)) - for i, s := range samples.Available { - idx, err := shwap.SampleIndexFromCoordinates(int(s.Row), int(s.Col), len(dah.RowRoots)) + idxs := make([]shwap.SampleIndex, len(samples.Remaining)) + for i, s := range samples.Remaining { + idx, err := shwap.SampleIndexFromCoordinates(s.Row, s.Col, len(dah.RowRoots)) if err != nil { return err } @@ -126,9 +126,12 @@ func (la *ShareAvailability) SharesAvailable(ctx context.Context, header *header } smpls, err := la.getter.GetSamples(ctx, header, idxs) - if errors.Is(err, context.Canceled) { - // Availability did not complete due to context cancellation, return context error instead of - // share.ErrNotAvailable + if err != nil { + if errors.Is(err, context.Canceled) { + // Availability did not complete due to context cancellation, return context error instead of + // share.ErrNotAvailable + return context.Canceled + } return err } if len(smpls) == 0 { diff --git a/share/availability/light/availability_test.go b/share/availability/light/availability_test.go index d40c987383..72bad83d85 100644 --- a/share/availability/light/availability_test.go +++ b/share/availability/light/availability_test.go @@ -37,6 +37,7 @@ import ( ) func TestSharesAvailableSuccess(t *testing.T) { + t.Skip("TODO") ctx, cancel := context.WithCancel(context.Background()) defer cancel() @@ -287,11 +288,11 @@ func (g onceGetter) checkOnce(t *testing.T) { } } -func (m onceGetter) GetSamples(_ context.Context, hdr *header.ExtendedHeader, +func (g onceGetter) GetSamples(_ context.Context, hdr *header.ExtendedHeader, indices []shwap.SampleIndex, ) ([]shwap.Sample, error) { - m.Lock() - defer m.Unlock() + g.Lock() + defer g.Unlock() smpls := make([]shwap.Sample, 0, len(indices)) for _, idx := range indices { @@ -301,8 +302,8 @@ func (m onceGetter) GetSamples(_ context.Context, hdr *header.ExtendedHeader, } s := Sample{Row: rowIdx, Col: colIdx} - if _, ok := m.sampled[s]; ok { - delete(m.sampled, s) + if _, ok := g.sampled[s]; ok { + delete(g.sampled, s) smpls = append(smpls, shwap.Sample{Proof: &nmt.Proof{}}) } } From da7e39de3b70534b10f7d847d7cc392ae17174d5 Mon Sep 17 00:00:00 2001 From: Oleg Kovalov Date: Fri, 8 Nov 2024 15:11:48 +0100 Subject: [PATCH 04/26] whatever --- share/availability/light/availability.go | 11 ++++------- 1 file changed, 4 insertions(+), 7 deletions(-) diff --git a/share/availability/light/availability.go b/share/availability/light/availability.go index 27cb4f69eb..8c5a724c29 100644 --- a/share/availability/light/availability.go +++ b/share/availability/light/availability.go @@ -126,13 +126,10 @@ func (la *ShareAvailability) SharesAvailable(ctx context.Context, header *header } smpls, err := la.getter.GetSamples(ctx, header, idxs) - if err != nil { - if errors.Is(err, context.Canceled) { - // Availability did not complete due to context cancellation, return context error instead of - // share.ErrNotAvailable - return context.Canceled - } - return err + if errors.Is(err, context.Canceled) { + // Availability did not complete due to context cancellation, return context error instead of + // share.ErrNotAvailable + return context.Canceled } if len(smpls) == 0 { return share.ErrNotAvailable From c3a482dc32773a511ccdf3c924bb42d96b3b7ea6 Mon Sep 17 00:00:00 2001 From: Oleg Kovalov Date: Tue, 12 Nov 2024 13:24:46 +0100 Subject: [PATCH 05/26] todo tests --- share/availability/light/availability.go | 6 +++++- share/availability/light/availability_test.go | 4 ++++ 2 files changed, 9 insertions(+), 1 deletion(-) diff --git a/share/availability/light/availability.go b/share/availability/light/availability.go index 8c5a724c29..e8bc7ecc14 100644 --- a/share/availability/light/availability.go +++ b/share/availability/light/availability.go @@ -138,7 +138,11 @@ func (la *ShareAvailability) SharesAvailable(ctx context.Context, header *header var failedSamples []Sample for i, smpl := range smpls { if smpl.IsEmpty() { - failedSamples = append(failedSamples, samples.Available[i]) + row, col, err := idxs[i].Coordinates(len(dah.RowRoots)) + if err != nil { + return err + } + failedSamples = append(failedSamples, Sample{Row: row, Col: col}) } } diff --git a/share/availability/light/availability_test.go b/share/availability/light/availability_test.go index 72bad83d85..136b5618c6 100644 --- a/share/availability/light/availability_test.go +++ b/share/availability/light/availability_test.go @@ -140,6 +140,7 @@ func TestSharesAvailableEmptyEDS(t *testing.T) { } func TestSharesAvailableFailed(t *testing.T) { + t.Skip("TODO") ctx, cancel := context.WithCancel(context.Background()) defer cancel() @@ -228,6 +229,7 @@ func TestSharesAvailableFailed(t *testing.T) { } func TestParallelAvailability(t *testing.T) { + t.Skip("TODO") ctx, cancel := context.WithCancel(context.Background()) defer cancel() @@ -331,6 +333,7 @@ func (g onceGetter) GetNamespaceData( } func TestPruneAll(t *testing.T) { + t.Skip("TODO") const size = 8 ctx, cancel := context.WithTimeout(context.Background(), time.Second*2) t.Cleanup(cancel) @@ -378,6 +381,7 @@ func TestPruneAll(t *testing.T) { } func TestPrunePartialFailed(t *testing.T) { + t.Skip("TODO") const size = 8 ctx, cancel := context.WithTimeout(context.Background(), time.Second*2) t.Cleanup(cancel) From 646346ae8e32f6021360c55438edd19d729c25fb Mon Sep 17 00:00:00 2001 From: Oleg Kovalov Date: Mon, 18 Nov 2024 10:58:42 +0100 Subject: [PATCH 06/26] partial fix --- share/availability/light/availability.go | 31 ++++++++++++-- share/availability/light/availability_test.go | 40 ++++++++++++++----- 2 files changed, 56 insertions(+), 15 deletions(-) diff --git a/share/availability/light/availability.go b/share/availability/light/availability.go index e8bc7ecc14..ecfb1ee2d1 100644 --- a/share/availability/light/availability.go +++ b/share/availability/light/availability.go @@ -135,21 +135,44 @@ func (la *ShareAvailability) SharesAvailable(ctx context.Context, header *header return share.ErrNotAvailable } + var fetchedSamples []Sample var failedSamples []Sample + for i, smpl := range smpls { + row, col, err := idxs[i].Coordinates(len(dah.RowRoots)) + if err != nil { + return err + } + if smpl.IsEmpty() { - row, col, err := idxs[i].Coordinates(len(dah.RowRoots)) - if err != nil { - return err - } failedSamples = append(failedSamples, Sample{Row: row, Col: col}) + } else { + fetchedSamples = append(fetchedSamples, Sample{Row: row, Col: col}) } } + log.Error(len(failedSamples)) + + samples.Available = fetchedSamples + samples.Remaining = failedSamples + + // Store the updated sampling result + updatedData, err := json.Marshal(samples) + if err != nil { + return err + } + la.dsLk.Lock() + err = la.ds.Put(ctx, key, updatedData) + la.dsLk.Unlock() + if err != nil { + return fmt.Errorf("store sampling result: %w", err) + } + // if any of the samples failed, return an error if len(failedSamples) > 0 { return share.ErrNotAvailable } + return nil } diff --git a/share/availability/light/availability_test.go b/share/availability/light/availability_test.go index 136b5618c6..0d42b64824 100644 --- a/share/availability/light/availability_test.go +++ b/share/availability/light/availability_test.go @@ -37,7 +37,6 @@ import ( ) func TestSharesAvailableSuccess(t *testing.T) { - t.Skip("TODO") ctx, cancel := context.WithCancel(context.Background()) defer cancel() @@ -140,7 +139,6 @@ func TestSharesAvailableEmptyEDS(t *testing.T) { } func TestSharesAvailableFailed(t *testing.T) { - t.Skip("TODO") ctx, cancel := context.WithCancel(context.Background()) defer cancel() @@ -171,6 +169,10 @@ func TestSharesAvailableFailed(t *testing.T) { roots, err := share.NewAxisRoots(eds) require.NoError(t, err) eh := headertest.RandExtendedHeaderWithRoot(t, roots) + // // Simulate a getter that now returns shares successfully + // successfulGetter := newOnceGetter() + // successfulGetter.addSamples(failed.Remaining) + // avail.getter = successfulGetter tests = append(tests, test{eh: eh, roots: roots, eds: eds}) } @@ -226,10 +228,12 @@ func TestSharesAvailableFailed(t *testing.T) { // // onceGetter should have no more samples stored after the call // successfulGetter.checkOnce(t) // require.ElementsMatch(t, failed.Remaining, len(successfulGetter.sampled)) + // onceGetter should have no more samples stored after the call + // successfulGetter.checkOnce(t) + // require.Empty(t, successfulGetter.sampledList()) } func TestParallelAvailability(t *testing.T) { - t.Skip("TODO") ctx, cancel := context.WithCancel(context.Background()) defer cancel() @@ -245,8 +249,8 @@ func TestParallelAvailability(t *testing.T) { eh := headertest.RandExtendedHeaderWithRoot(t, roots) var wg sync.WaitGroup + wg.Add(100) for i := 0; i < 100; i++ { - wg.Add(1) go func() { defer wg.Done() err := avail.SharesAvailable(ctx, eh) @@ -280,6 +284,26 @@ func newOnceGetter() onceGetter { } } +func (g onceGetter) addSamples(remaining []Sample) { + g.Lock() + defer g.Unlock() + + for _, r := range remaining { + s := Sample{Row: r.Row, Col: r.Col} + g.sampled[s]++ + } +} + +func (g onceGetter) sampledList() []Sample { + g.Lock() + defer g.Unlock() + samples := make([]Sample, 0, len(g.sampled)) + for s := range g.sampled { + samples = append(samples, s) + } + return samples +} + func (g onceGetter) checkOnce(t *testing.T) { g.Lock() defer g.Unlock() @@ -313,11 +337,7 @@ func (g onceGetter) GetSamples(_ context.Context, hdr *header.ExtendedHeader, } func (g onceGetter) GetShare(_ context.Context, _ *header.ExtendedHeader, row, col int) (libshare.Share, error) { - g.Lock() - defer g.Unlock() - s := Sample{Row: row, Col: col} - g.sampled[s]++ - return libshare.Share{}, nil + panic("not implemented") } func (g onceGetter) GetEDS(_ context.Context, _ *header.ExtendedHeader) (*rsmt2d.ExtendedDataSquare, error) { @@ -333,7 +353,6 @@ func (g onceGetter) GetNamespaceData( } func TestPruneAll(t *testing.T) { - t.Skip("TODO") const size = 8 ctx, cancel := context.WithTimeout(context.Background(), time.Second*2) t.Cleanup(cancel) @@ -381,7 +400,6 @@ func TestPruneAll(t *testing.T) { } func TestPrunePartialFailed(t *testing.T) { - t.Skip("TODO") const size = 8 ctx, cancel := context.WithTimeout(context.Background(), time.Second*2) t.Cleanup(cancel) From 97b03a31a6acf6581ef5d1f6608ff5581bb40ef6 Mon Sep 17 00:00:00 2001 From: Oleg Kovalov Date: Mon, 18 Nov 2024 13:38:25 +0100 Subject: [PATCH 07/26] upd --- share/availability/light/availability.go | 2 -- share/availability/light/availability_test.go | 6 +++--- 2 files changed, 3 insertions(+), 5 deletions(-) diff --git a/share/availability/light/availability.go b/share/availability/light/availability.go index ecfb1ee2d1..e060bdd00f 100644 --- a/share/availability/light/availability.go +++ b/share/availability/light/availability.go @@ -151,8 +151,6 @@ func (la *ShareAvailability) SharesAvailable(ctx context.Context, header *header } } - log.Error(len(failedSamples)) - samples.Available = fetchedSamples samples.Remaining = failedSamples diff --git a/share/availability/light/availability_test.go b/share/availability/light/availability_test.go index 0d42b64824..e2018b0ae0 100644 --- a/share/availability/light/availability_test.go +++ b/share/availability/light/availability_test.go @@ -284,11 +284,11 @@ func newOnceGetter() onceGetter { } } -func (g onceGetter) addSamples(remaining []Sample) { +func (g onceGetter) addSamples(samples []Sample) { g.Lock() defer g.Unlock() - for _, r := range remaining { + for _, r := range samples { s := Sample{Row: r.Row, Col: r.Col} g.sampled[s]++ } @@ -336,7 +336,7 @@ func (g onceGetter) GetSamples(_ context.Context, hdr *header.ExtendedHeader, return smpls, nil } -func (g onceGetter) GetShare(_ context.Context, _ *header.ExtendedHeader, row, col int) (libshare.Share, error) { +func (g onceGetter) GetShare(_ context.Context, _ *header.ExtendedHeader, _, _ int) (libshare.Share, error) { panic("not implemented") } From e2cb3ae521914cfe145c477badc10733b8997269 Mon Sep 17 00:00:00 2001 From: Oleg Kovalov Date: Tue, 19 Nov 2024 10:17:16 +0100 Subject: [PATCH 08/26] fix --- share/availability/light/availability.go | 4 +- share/availability/light/availability_test.go | 66 +++++++++++++++++-- 2 files changed, 60 insertions(+), 10 deletions(-) diff --git a/share/availability/light/availability.go b/share/availability/light/availability.go index e060bdd00f..0e7a8fd2d7 100644 --- a/share/availability/light/availability.go +++ b/share/availability/light/availability.go @@ -135,7 +135,6 @@ func (la *ShareAvailability) SharesAvailable(ctx context.Context, header *header return share.ErrNotAvailable } - var fetchedSamples []Sample var failedSamples []Sample for i, smpl := range smpls { @@ -147,11 +146,10 @@ func (la *ShareAvailability) SharesAvailable(ctx context.Context, header *header if smpl.IsEmpty() { failedSamples = append(failedSamples, Sample{Row: row, Col: col}) } else { - fetchedSamples = append(fetchedSamples, Sample{Row: row, Col: col}) + samples.Available = append(samples.Available, Sample{Row: row, Col: col}) } } - samples.Available = fetchedSamples samples.Remaining = failedSamples // Store the updated sampling result diff --git a/share/availability/light/availability_test.go b/share/availability/light/availability_test.go index e2018b0ae0..21827bd95a 100644 --- a/share/availability/light/availability_test.go +++ b/share/availability/light/availability_test.go @@ -5,6 +5,8 @@ import ( _ "embed" "encoding/json" "errors" + "maps" + "slices" "sync" "sync/atomic" "testing" @@ -239,7 +241,7 @@ func TestParallelAvailability(t *testing.T) { ds := datastore.NewMapDatastore() // Simulate a getter that returns shares successfully - successfulGetter := newOnceGetter() + successfulGetter := newSuccessGetter() avail := NewShareAvailability(successfulGetter, ds, nil) // create new eds, that is not available by getter @@ -249,8 +251,9 @@ func TestParallelAvailability(t *testing.T) { eh := headertest.RandExtendedHeaderWithRoot(t, roots) var wg sync.WaitGroup - wg.Add(100) - for i := 0; i < 100; i++ { + const iters = 100 + wg.Add(iters) + for i := 0; i < iters; i++ { go func() { defer wg.Done() err := avail.SharesAvailable(ctx, eh) @@ -258,7 +261,7 @@ func TestParallelAvailability(t *testing.T) { }() } wg.Wait() - require.Len(t, len(successfulGetter.sampled), int(avail.params.SampleAmount)) + require.Equal(t, len(successfulGetter.sampledList()), int(avail.params.SampleAmount)) // Verify that the sampling result is stored with all samples marked as available resultData, err := avail.ds.Get(ctx, datastoreKeyForRoot(roots)) @@ -336,15 +339,62 @@ func (g onceGetter) GetSamples(_ context.Context, hdr *header.ExtendedHeader, return smpls, nil } -func (g onceGetter) GetShare(_ context.Context, _ *header.ExtendedHeader, _, _ int) (libshare.Share, error) { +func (g onceGetter) GetEDS(_ context.Context, _ *header.ExtendedHeader) (*rsmt2d.ExtendedDataSquare, error) { panic("not implemented") } -func (g onceGetter) GetEDS(_ context.Context, _ *header.ExtendedHeader) (*rsmt2d.ExtendedDataSquare, error) { +func (g onceGetter) GetNamespaceData( + _ context.Context, + _ *header.ExtendedHeader, + _ libshare.Namespace, +) (shwap.NamespaceData, error) { panic("not implemented") } -func (g onceGetter) GetNamespaceData( +type successGetter struct { + *sync.Mutex + sampled map[Sample]int +} + +func newSuccessGetter() successGetter { + return successGetter{ + Mutex: &sync.Mutex{}, + sampled: make(map[Sample]int), + } +} + +func (g successGetter) sampledList() []Sample { + g.Lock() + defer g.Unlock() + return slices.Collect(maps.Keys(g.sampled)) +} + +func (g successGetter) GetSamples(_ context.Context, hdr *header.ExtendedHeader, + indices []shwap.SampleIndex, +) ([]shwap.Sample, error) { + g.Lock() + defer g.Unlock() + + smpls := make([]shwap.Sample, 0, len(indices)) + for _, idx := range indices { + rowIdx, colIdx, err := idx.Coordinates(len(hdr.DAH.RowRoots)) + if err != nil { + return nil, err + } + + s := Sample{Row: rowIdx, Col: colIdx} + g.sampled[s]++ + smpls = append(smpls, shwap.Sample{Proof: &nmt.Proof{}}) + + } + return smpls, nil +} + +func (g successGetter) GetEDS(_ context.Context, _ *header.ExtendedHeader) (*rsmt2d.ExtendedDataSquare, error) { + panic("not implemented") +} + +func (g successGetter) GetNamespaceData( _ context.Context, _ *header.ExtendedHeader, _ libshare.Namespace, @@ -353,6 +403,7 @@ func (g onceGetter) GetNamespaceData( } func TestPruneAll(t *testing.T) { + t.Skip("TODO") const size = 8 ctx, cancel := context.WithTimeout(context.Background(), time.Second*2) t.Cleanup(cancel) @@ -400,6 +451,7 @@ func TestPruneAll(t *testing.T) { } func TestPrunePartialFailed(t *testing.T) { + t.Skip("TODO") const size = 8 ctx, cancel := context.WithTimeout(context.Background(), time.Second*2) t.Cleanup(cancel) From 86962374daef11d42f89fa64e4970eed1332833a Mon Sep 17 00:00:00 2001 From: Oleg Kovalov Date: Tue, 19 Nov 2024 15:53:05 +0100 Subject: [PATCH 09/26] almost a fix --- share/availability/light/availability_test.go | 41 ++++++------------- share/shwap/p2p/bitswap/getter.go | 5 ++- 2 files changed, 16 insertions(+), 30 deletions(-) diff --git a/share/availability/light/availability_test.go b/share/availability/light/availability_test.go index 21827bd95a..a442206e07 100644 --- a/share/availability/light/availability_test.go +++ b/share/availability/light/availability_test.go @@ -144,19 +144,6 @@ func TestSharesAvailableFailed(t *testing.T) { ctx, cancel := context.WithCancel(context.Background()) defer cancel() - failGetter := mock.NewMockGetter(gomock.NewController(t)) - // Getter doesn't have the eds, so it should fail for all samples - // failGetter.EXPECT(). - // GetShare(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any()). - // Return(libshare.Share{}, shrex.ErrNotFound). - failGetter.EXPECT(). - GetSamples(gomock.Any(), gomock.Any(), gomock.Any()). - Return(make([]shwap.Sample, DefaultSampleAmount), shrex.ErrNotFound). - AnyTimes() - - ds := datastore.NewMapDatastore() - avail := NewShareAvailability(failGetter, ds, nil) - type test struct { eh *header.ExtendedHeader roots *share.AxisRoots @@ -171,16 +158,21 @@ func TestSharesAvailableFailed(t *testing.T) { roots, err := share.NewAxisRoots(eds) require.NoError(t, err) eh := headertest.RandExtendedHeaderWithRoot(t, roots) - // // Simulate a getter that now returns shares successfully - // successfulGetter := newOnceGetter() - // successfulGetter.addSamples(failed.Remaining) - // avail.getter = successfulGetter tests = append(tests, test{eh: eh, roots: roots, eds: eds}) } for _, tt := range tests { - avail.getter = failGetter + failGetter := mock.NewMockGetter(gomock.NewController(t)) + ds := datastore.NewMapDatastore() + avail := NewShareAvailability(failGetter, ds, nil) + + // Getter doesn't have the eds, so it should fail for all samples + mockSamples := min(int(avail.params.SampleAmount), 2*len(tt.eh.DAH.RowRoots)) + failGetter.EXPECT(). + GetSamples(gomock.Any(), gomock.Any(), gomock.Any()). + Return(make([]shwap.Sample, mockSamples), shrex.ErrNotFound). + AnyTimes() err := avail.SharesAvailable(ctx, tt.eh) require.ErrorIs(t, err, share.ErrNotAvailable) @@ -201,7 +193,7 @@ func TestSharesAvailableFailed(t *testing.T) { } // Simulate a getter that now returns shares successfully - successfulGetter := newOnceGetter() + successfulGetter := newSuccessGetter() avail.getter = successfulGetter // should be able to retrieve all the failed samples now @@ -224,15 +216,8 @@ func TestSharesAvailableFailed(t *testing.T) { } // onceGetter should have no more samples stored after the call - successfulGetter.checkOnce(t) - // require.ElementsMatch(t, failed.Remaining, successfulGetter.sampledList()) + require.ElementsMatch(t, failed.Remaining, successfulGetter.sampledList()) } - // // onceGetter should have no more samples stored after the call - // successfulGetter.checkOnce(t) - // require.ElementsMatch(t, failed.Remaining, len(successfulGetter.sampled)) - // onceGetter should have no more samples stored after the call - // successfulGetter.checkOnce(t) - // require.Empty(t, successfulGetter.sampledList()) } func TestParallelAvailability(t *testing.T) { @@ -403,7 +388,6 @@ func (g successGetter) GetNamespaceData( } func TestPruneAll(t *testing.T) { - t.Skip("TODO") const size = 8 ctx, cancel := context.WithTimeout(context.Background(), time.Second*2) t.Cleanup(cancel) @@ -451,7 +435,6 @@ func TestPruneAll(t *testing.T) { } func TestPrunePartialFailed(t *testing.T) { - t.Skip("TODO") const size = 8 ctx, cancel := context.WithTimeout(context.Background(), time.Second*2) t.Cleanup(cancel) diff --git a/share/shwap/p2p/bitswap/getter.go b/share/shwap/p2p/bitswap/getter.go index 058f84158e..11b3d2846a 100644 --- a/share/shwap/p2p/bitswap/getter.go +++ b/share/shwap/p2p/bitswap/getter.go @@ -15,6 +15,7 @@ import ( "github.com/celestiaorg/celestia-app/v3/pkg/wrapper" libshare "github.com/celestiaorg/go-square/v2/share" + "github.com/celestiaorg/nmt" "github.com/celestiaorg/rsmt2d" "github.com/celestiaorg/celestia-node/header" @@ -131,7 +132,9 @@ func (g *Getter) GetSamples( smpls := make([]shwap.Sample, len(blks)) for i, blk := range blks { - smpls[i] = blk.(*SampleBlock).Container + c := blk.(*SampleBlock).Container + c.Proof = &nmt.Proof{} // TODO + smpls[i] = c } span.SetStatus(codes.Ok, "") From 560087ae0622b5d7ef0a4a13b6d3a8eab46bbbe5 Mon Sep 17 00:00:00 2001 From: Oleg Kovalov Date: Tue, 19 Nov 2024 15:56:58 +0100 Subject: [PATCH 10/26] one more fix --- share/availability/light/availability_test.go | 77 ------------------- share/availability/light/sample.go | 4 +- 2 files changed, 2 insertions(+), 79 deletions(-) diff --git a/share/availability/light/availability_test.go b/share/availability/light/availability_test.go index a442206e07..3d129c2625 100644 --- a/share/availability/light/availability_test.go +++ b/share/availability/light/availability_test.go @@ -260,82 +260,6 @@ func TestParallelAvailability(t *testing.T) { require.Len(t, samplingResult.Available, int(avail.params.SampleAmount)) } -type onceGetter struct { - *sync.Mutex - sampled map[Sample]int -} - -func newOnceGetter() onceGetter { - return onceGetter{ - Mutex: &sync.Mutex{}, - sampled: make(map[Sample]int), - } -} - -func (g onceGetter) addSamples(samples []Sample) { - g.Lock() - defer g.Unlock() - - for _, r := range samples { - s := Sample{Row: r.Row, Col: r.Col} - g.sampled[s]++ - } -} - -func (g onceGetter) sampledList() []Sample { - g.Lock() - defer g.Unlock() - samples := make([]Sample, 0, len(g.sampled)) - for s := range g.sampled { - samples = append(samples, s) - } - return samples -} - -func (g onceGetter) checkOnce(t *testing.T) { - g.Lock() - defer g.Unlock() - for s, count := range g.sampled { - if count > 1 { - t.Errorf("sample %v was called more than once", s) - } - } -} - -func (g onceGetter) GetSamples(_ context.Context, hdr *header.ExtendedHeader, - indices []shwap.SampleIndex, -) ([]shwap.Sample, error) { - g.Lock() - defer g.Unlock() - - smpls := make([]shwap.Sample, 0, len(indices)) - for _, idx := range indices { - rowIdx, colIdx, err := idx.Coordinates(len(hdr.DAH.RowRoots)) - if err != nil { - return nil, err - } - - s := Sample{Row: rowIdx, Col: colIdx} - if _, ok := g.sampled[s]; ok { - delete(g.sampled, s) - smpls = append(smpls, shwap.Sample{Proof: &nmt.Proof{}}) - } - } - return smpls, nil -} - -func (g onceGetter) GetEDS(_ context.Context, _ *header.ExtendedHeader) (*rsmt2d.ExtendedDataSquare, error) { - panic("not implemented") -} - -func (g onceGetter) GetNamespaceData( - _ context.Context, - _ *header.ExtendedHeader, - _ libshare.Namespace, -) (shwap.NamespaceData, error) { - panic("not implemented") -} - type successGetter struct { *sync.Mutex sampled map[Sample]int @@ -370,7 +294,6 @@ func (g successGetter) GetSamples(_ context.Context, hdr *header.ExtendedHeader, s := Sample{Row: rowIdx, Col: colIdx} g.sampled[s]++ smpls = append(smpls, shwap.Sample{Proof: &nmt.Proof{}}) - } return smpls, nil } diff --git a/share/availability/light/sample.go b/share/availability/light/sample.go index 6857ab5365..0641e11a99 100644 --- a/share/availability/light/sample.go +++ b/share/availability/light/sample.go @@ -50,8 +50,8 @@ func selectRandomSamples(squareSize, sampleCount int) []Sample { return maps.Keys(samples) } -func randInt(max int) int { - n, err := crand.Int(crand.Reader, big.NewInt(int64(max))) +func randInt(m int) int { + n, err := crand.Int(crand.Reader, big.NewInt(int64(m))) if err != nil { panic(err) // won't panic as rand.Reader is endless } From e13f2c24c994131439a84a81ba04ece7c5d30258 Mon Sep 17 00:00:00 2001 From: Wondertan Date: Wed, 20 Nov 2024 09:16:14 +0100 Subject: [PATCH 11/26] fix prunning tests --- share/availability/light/availability_test.go | 135 +++++++++++------- share/shwap/p2p/bitswap/getter.go | 2 - 2 files changed, 85 insertions(+), 52 deletions(-) diff --git a/share/availability/light/availability_test.go b/share/availability/light/availability_test.go index 3d129c2625..ceb21621f4 100644 --- a/share/availability/light/availability_test.go +++ b/share/availability/light/availability_test.go @@ -4,7 +4,9 @@ import ( "context" _ "embed" "encoding/json" - "errors" + "github.com/ipfs/boxo/bitswap/client" + "github.com/libp2p/go-libp2p/core/host" + mocknet "github.com/libp2p/go-libp2p/p2p/net/mock" "maps" "slices" "sync" @@ -15,7 +17,6 @@ import ( "github.com/golang/mock/gomock" "github.com/ipfs/boxo/blockstore" "github.com/ipfs/boxo/exchange" - "github.com/ipfs/boxo/exchange/offline" blocks "github.com/ipfs/go-block-format" "github.com/ipfs/go-cid" "github.com/ipfs/go-datastore" @@ -35,7 +36,6 @@ import ( "github.com/celestiaorg/celestia-node/share/shwap/getters/mock" "github.com/celestiaorg/celestia-node/share/shwap/p2p/bitswap" "github.com/celestiaorg/celestia-node/share/shwap/p2p/shrex" - "github.com/celestiaorg/celestia-node/store" ) func TestSharesAvailableSuccess(t *testing.T) { @@ -315,19 +315,11 @@ func TestPruneAll(t *testing.T) { ctx, cancel := context.WithTimeout(context.Background(), time.Second*2) t.Cleanup(cancel) - dir := t.TempDir() - store, err := store.NewStore(store.DefaultParameters(), dir) - require.NoError(t, err) - defer require.NoError(t, store.Stop(ctx)) eds, h := randEdsAndHeader(t, size) - err = store.PutODSQ4(ctx, h.DAH, h.Height(), eds) - require.NoError(t, err) - - // Create a new bitswap getter ds := ds_sync.MutexWrap(datastore.NewMapDatastore()) clientBs := blockstore.NewBlockstore(ds) - serverBS := &bitswap.Blockstore{Getter: store} - ex := newFakeExchange(serverBS) + + ex := newExchangeOverEDS(ctx, t, eds) getter := bitswap.NewGetter(ex, clientBs, 0) getter.Start() defer getter.Stop() @@ -335,7 +327,7 @@ func TestPruneAll(t *testing.T) { // Create a new ShareAvailability instance and sample the shares sampleAmount := uint(20) avail := NewShareAvailability(getter, ds, clientBs, WithSampleAmount(sampleAmount)) - err = avail.SharesAvailable(ctx, h) + err := avail.SharesAvailable(ctx, h) require.NoError(t, err) // close ShareAvailability to force flush of batched writes avail.Close(ctx) @@ -359,22 +351,14 @@ func TestPruneAll(t *testing.T) { func TestPrunePartialFailed(t *testing.T) { const size = 8 - ctx, cancel := context.WithTimeout(context.Background(), time.Second*2) + ctx, cancel := context.WithTimeout(context.Background(), time.Second*200) t.Cleanup(cancel) - dir := t.TempDir() - store, err := store.NewStore(store.DefaultParameters(), dir) - require.NoError(t, err) - defer require.NoError(t, store.Stop(ctx)) eds, h := randEdsAndHeader(t, size) - err = store.PutODSQ4(ctx, h.DAH, h.Height(), eds) - require.NoError(t, err) - - // Create a new bitswap getter ds := ds_sync.MutexWrap(datastore.NewMapDatastore()) clientBs := blockstore.NewBlockstore(ds) - serverBS := newHalfFailBlockstore(&bitswap.Blockstore{Getter: store}) - ex := newFakeExchange(serverBS) + + ex := newHalfSessionExchange(newExchangeOverEDS(ctx, t, eds)) getter := bitswap.NewGetter(ex, clientBs, 0) getter.Start() defer getter.Stop() @@ -382,8 +366,8 @@ func TestPrunePartialFailed(t *testing.T) { // Create a new ShareAvailability instance and sample the shares sampleAmount := uint(20) avail := NewShareAvailability(getter, ds, clientBs, WithSampleAmount(sampleAmount)) - err = avail.SharesAvailable(ctx, h) - require.NoError(t, err) + err := avail.SharesAvailable(ctx, h) + require.Error(t, err) // close ShareAvailability to force flush of batched writes avail.Close(ctx) @@ -405,38 +389,37 @@ func TestPrunePartialFailed(t *testing.T) { require.False(t, exist) } -var _ exchange.SessionExchange = (*fakeSessionExchange)(nil) - -func newFakeExchange(bs blockstore.Blockstore) *fakeSessionExchange { - return &fakeSessionExchange{ - Interface: offline.Exchange(bs), - session: offline.Exchange(bs), - } +type halfSessionExchange struct { + exchange.SessionExchange + attempt atomic.Int32 } -type fakeSessionExchange struct { - exchange.Interface - session exchange.Fetcher +func newHalfSessionExchange(ex exchange.SessionExchange) *halfSessionExchange { + return &halfSessionExchange{SessionExchange: ex} } -func (fe *fakeSessionExchange) NewSession(context.Context) exchange.Fetcher { - return fe.session +func (hse *halfSessionExchange) NewSession(context.Context) exchange.Fetcher { + return hse } -type halfFailBlockstore struct { - blockstore.Blockstore - attempt atomic.Int32 -} +func (hse *halfSessionExchange) GetBlocks(ctx context.Context, cids []cid.Cid) (<-chan blocks.Block, error) { + out := make(chan blocks.Block, len(cids)) + defer close(out) -func newHalfFailBlockstore(bs blockstore.Blockstore) *halfFailBlockstore { - return &halfFailBlockstore{Blockstore: bs} -} + for _, cid := range cids { + if hse.attempt.Add(1)%2 == 0 { + continue + } -func (hfb *halfFailBlockstore) Get(ctx context.Context, c cid.Cid) (blocks.Block, error) { - if hfb.attempt.Add(1)%2 == 0 { - return nil, errors.New("fail") + blk, err := hse.SessionExchange.GetBlock(ctx, cid) + if err != nil { + return nil, err + } + + out <- blk } - return hfb.Blockstore.Get(ctx, c) + + return out, nil } func randEdsAndHeader(t *testing.T, size int) (*rsmt2d.ExtendedDataSquare, *header.ExtendedHeader) { @@ -463,3 +446,55 @@ func countKeys(ctx context.Context, t *testing.T, bs blockstore.Blockstore) int } return count } + +func newExchangeOverEDS(ctx context.Context, t *testing.T, rsmt2d *rsmt2d.ExtendedDataSquare) exchange.SessionExchange { + bstore := &bitswap.Blockstore{ + Getter: testAccessorGetter{ + AccessorStreamer: &eds.Rsmt2D{ExtendedDataSquare: rsmt2d}, + }, + } + return newExchange(ctx, t, bstore) +} + +func newExchange(ctx context.Context, t *testing.T, bstore blockstore.Blockstore) exchange.SessionExchange { + net, err := mocknet.FullMeshLinked(3) + require.NoError(t, err) + + newServer(ctx, net.Hosts()[0], bstore) + newServer(ctx, net.Hosts()[1], bstore) + + client := newClient(ctx, net.Hosts()[2], bstore) + + err = net.ConnectAllButSelf() + require.NoError(t, err) + return client +} + +func newServer(ctx context.Context, host host.Host, store blockstore.Blockstore) { + net := bitswap.NewNetwork(host, "test") + server := bitswap.NewServer( + ctx, + net, + store, + ) + net.Start(server) +} + +func newClient(ctx context.Context, host host.Host, store blockstore.Blockstore) *client.Client { + net := bitswap.NewNetwork(host, "test") + client := bitswap.NewClient(ctx, net, store) + net.Start(client) + return client +} + +type testAccessorGetter struct { + eds.AccessorStreamer +} + +func (t testAccessorGetter) GetByHeight(context.Context, uint64) (eds.AccessorStreamer, error) { + return t.AccessorStreamer, nil +} + +func (t testAccessorGetter) HasByHeight(context.Context, uint64) (bool, error) { + return true, nil +} diff --git a/share/shwap/p2p/bitswap/getter.go b/share/shwap/p2p/bitswap/getter.go index 11b3d2846a..e46f0d24c6 100644 --- a/share/shwap/p2p/bitswap/getter.go +++ b/share/shwap/p2p/bitswap/getter.go @@ -15,7 +15,6 @@ import ( "github.com/celestiaorg/celestia-app/v3/pkg/wrapper" libshare "github.com/celestiaorg/go-square/v2/share" - "github.com/celestiaorg/nmt" "github.com/celestiaorg/rsmt2d" "github.com/celestiaorg/celestia-node/header" @@ -133,7 +132,6 @@ func (g *Getter) GetSamples( smpls := make([]shwap.Sample, len(blks)) for i, blk := range blks { c := blk.(*SampleBlock).Container - c.Proof = &nmt.Proof{} // TODO smpls[i] = c } From cfe040989e988dce04e744388f45d992f54e4b1d Mon Sep 17 00:00:00 2001 From: Wondertan Date: Wed, 20 Nov 2024 09:24:58 +0100 Subject: [PATCH 12/26] lint --- share/availability/light/availability_test.go | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/share/availability/light/availability_test.go b/share/availability/light/availability_test.go index ceb21621f4..ba59161ca8 100644 --- a/share/availability/light/availability_test.go +++ b/share/availability/light/availability_test.go @@ -4,9 +4,6 @@ import ( "context" _ "embed" "encoding/json" - "github.com/ipfs/boxo/bitswap/client" - "github.com/libp2p/go-libp2p/core/host" - mocknet "github.com/libp2p/go-libp2p/p2p/net/mock" "maps" "slices" "sync" @@ -15,12 +12,15 @@ import ( "time" "github.com/golang/mock/gomock" + "github.com/ipfs/boxo/bitswap/client" "github.com/ipfs/boxo/blockstore" "github.com/ipfs/boxo/exchange" blocks "github.com/ipfs/go-block-format" "github.com/ipfs/go-cid" "github.com/ipfs/go-datastore" ds_sync "github.com/ipfs/go-datastore/sync" + "github.com/libp2p/go-libp2p/core/host" + mocknet "github.com/libp2p/go-libp2p/p2p/net/mock" "github.com/stretchr/testify/require" libshare "github.com/celestiaorg/go-square/v2/share" @@ -351,7 +351,7 @@ func TestPruneAll(t *testing.T) { func TestPrunePartialFailed(t *testing.T) { const size = 8 - ctx, cancel := context.WithTimeout(context.Background(), time.Second*200) + ctx, cancel := context.WithTimeout(context.Background(), time.Second*2) t.Cleanup(cancel) eds, h := randEdsAndHeader(t, size) From f10c0696b9a978ec9aab1acece6c43b9cc84f5a2 Mon Sep 17 00:00:00 2001 From: Oleg Kovalov Date: Wed, 20 Nov 2024 13:32:04 +0100 Subject: [PATCH 13/26] use coord tuple --- blob/service_test.go | 8 ++--- nodebuilder/share/share.go | 6 +--- share/availability/light/availability.go | 22 ++++---------- share/availability/light/availability_test.go | 7 +---- share/eds/close_once_test.go | 4 +-- share/eds/proofs_cache.go | 7 +---- share/eds/rsmt2d.go | 7 +---- share/eds/rsmt2d_test.go | 3 +- share/eds/testing.go | 21 +++++--------- share/eds/validation_test.go | 9 ++---- share/shwap/getters/cascade_test.go | 2 +- share/shwap/p2p/bitswap/sample_block.go | 5 +--- share/shwap/p2p/bitswap/sample_block_test.go | 4 +-- share/shwap/sample.go | 11 ++----- share/shwap/sample_id.go | 29 ++++--------------- share/shwap/sample_id_test.go | 16 ++-------- share/shwap/sample_test.go | 14 ++++----- store/file/ods.go | 12 ++------ store/file/ods_q4.go | 6 +--- store/getter_test.go | 7 +++-- 20 files changed, 54 insertions(+), 146 deletions(-) diff --git a/blob/service_test.go b/blob/service_test.go index d7e44e2aa7..37130c9696 100644 --- a/blob/service_test.go +++ b/blob/service_test.go @@ -145,7 +145,7 @@ func TestBlobService_Get(t *testing.T) { shareOffset := 0 for i := range blobs { row, col := calculateIndex(len(h.DAH.RowRoots), blobs[i].index) - idx, err := shwap.SampleIndexFromCoordinates(row, col, len(h.DAH.RowRoots)) + idx := shwap.SampleIndex{Row: row, Col: col} require.NoError(t, err) smpls, err := service.shareGetter.GetSamples(ctx, h, []shwap.SampleIndex{idx}) require.NoError(t, err) @@ -489,7 +489,7 @@ func TestService_GetSingleBlobWithoutPadding(t *testing.T) { h, err := service.headerGetter(ctx, 1) require.NoError(t, err) row, col := calculateIndex(len(h.DAH.RowRoots), newBlob.index) - idx, err := shwap.SampleIndexFromCoordinates(row, col, len(h.DAH.RowRoots)) + idx := shwap.SampleIndex{Row: row, Col: col} require.NoError(t, err) smpls, err := service.shareGetter.GetSamples(ctx, h, []shwap.SampleIndex{idx}) @@ -526,7 +526,7 @@ func TestService_Get(t *testing.T) { assert.Equal(t, b.Commitment, blob.Commitment) row, col := calculateIndex(len(h.DAH.RowRoots), b.index) - idx, err := shwap.SampleIndexFromCoordinates(row, col, len(h.DAH.RowRoots)) + idx := shwap.SampleIndex{Row: row, Col: col} require.NoError(t, err) smpls, err := service.shareGetter.GetSamples(ctx, h, []shwap.SampleIndex{idx}) @@ -588,7 +588,7 @@ func TestService_GetAllWithoutPadding(t *testing.T) { require.True(t, blobs[i].compareCommitments(blob.Commitment)) row, col := calculateIndex(len(h.DAH.RowRoots), blob.index) - idx, err := shwap.SampleIndexFromCoordinates(row, col, len(h.DAH.RowRoots)) + idx := shwap.SampleIndex{Row: row, Col: col} require.NoError(t, err) smpls, err := service.shareGetter.GetSamples(ctx, h, []shwap.SampleIndex{idx}) diff --git a/nodebuilder/share/share.go b/nodebuilder/share/share.go index 887e6f27a0..2b7e10a92a 100644 --- a/nodebuilder/share/share.go +++ b/nodebuilder/share/share.go @@ -126,17 +126,13 @@ type module struct { hs headerServ.Module } -// TODO(@Wondertan): break 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 } - idx, err := shwap.SampleIndexFromCoordinates(row, col, len(header.DAH.RowRoots)) - if err != nil { - return libshare.Share{}, err - } + idx := shwap.SampleIndex{Row: row, Col: col} smpls, err := m.getter.GetSamples(ctx, header, []shwap.SampleIndex{idx}) if err != nil { diff --git a/share/availability/light/availability.go b/share/availability/light/availability.go index 0e7a8fd2d7..220e8ec7bb 100644 --- a/share/availability/light/availability.go +++ b/share/availability/light/availability.go @@ -117,12 +117,7 @@ func (la *ShareAvailability) SharesAvailable(ctx context.Context, header *header idxs := make([]shwap.SampleIndex, len(samples.Remaining)) for i, s := range samples.Remaining { - idx, err := shwap.SampleIndexFromCoordinates(s.Row, s.Col, len(dah.RowRoots)) - if err != nil { - return err - } - - idxs[i] = idx + idxs[i] = shwap.SampleIndex{Row: s.Row, Col: s.Col} } smpls, err := la.getter.GetSamples(ctx, header, idxs) @@ -138,15 +133,10 @@ func (la *ShareAvailability) SharesAvailable(ctx context.Context, header *header var failedSamples []Sample for i, smpl := range smpls { - row, col, err := idxs[i].Coordinates(len(dah.RowRoots)) - if err != nil { - return err - } - if smpl.IsEmpty() { - failedSamples = append(failedSamples, Sample{Row: row, Col: col}) + failedSamples = append(failedSamples, Sample{Row: idxs[i].Row, Col: idxs[i].Col}) } else { - samples.Available = append(samples.Available, Sample{Row: row, Col: col}) + samples.Available = append(samples.Available, Sample{Row: idxs[i].Row, Col: idxs[i].Col}) } } @@ -207,10 +197,8 @@ func (la *ShareAvailability) Prune(ctx context.Context, h *header.ExtendedHeader // delete stored samples for _, sample := range result.Available { - idx, err := shwap.SampleIndexFromCoordinates(sample.Row, sample.Col, len(h.DAH.RowRoots)) - if err != nil { - return err - } + idx := shwap.SampleIndex{Row: sample.Row, Col: sample.Col} + blk, err := bitswap.NewEmptySampleBlock(h.Height(), idx, len(h.DAH.RowRoots)) if err != nil { return fmt.Errorf("marshal sample ID: %w", err) diff --git a/share/availability/light/availability_test.go b/share/availability/light/availability_test.go index ba59161ca8..c70c511642 100644 --- a/share/availability/light/availability_test.go +++ b/share/availability/light/availability_test.go @@ -286,12 +286,7 @@ func (g successGetter) GetSamples(_ context.Context, hdr *header.ExtendedHeader, smpls := make([]shwap.Sample, 0, len(indices)) for _, idx := range indices { - rowIdx, colIdx, err := idx.Coordinates(len(hdr.DAH.RowRoots)) - if err != nil { - return nil, err - } - - s := Sample{Row: rowIdx, Col: colIdx} + s := Sample{Row: idx.Row, Col: idx.Col} g.sampled[s]++ smpls = append(smpls, shwap.Sample{Proof: &nmt.Proof{}}) } diff --git a/share/eds/close_once_test.go b/share/eds/close_once_test.go index 59b4452174..a3c9cd96ee 100644 --- a/share/eds/close_once_test.go +++ b/share/eds/close_once_test.go @@ -20,7 +20,7 @@ func TestWithClosedOnce(t *testing.T) { stub := &stubEdsAccessorCloser{} closedOnce := WithClosedOnce(stub) - _, err := closedOnce.Sample(ctx, 0) + _, err := closedOnce.Sample(ctx, shwap.SampleIndex{}) require.NoError(t, err) _, err = closedOnce.AxisHalf(ctx, rsmt2d.Row, 0) require.NoError(t, err) @@ -33,7 +33,7 @@ func TestWithClosedOnce(t *testing.T) { require.True(t, stub.closed) // Ensure that the underlying file is not accessible after closing - _, err = closedOnce.Sample(ctx, 0) + _, err = closedOnce.Sample(ctx, shwap.SampleIndex{}) require.ErrorIs(t, err, errAccessorClosed) _, err = closedOnce.AxisHalf(ctx, rsmt2d.Row, 0) require.ErrorIs(t, err, errAccessorClosed) diff --git a/share/eds/proofs_cache.go b/share/eds/proofs_cache.go index 006d1e46ac..0b70a6006c 100644 --- a/share/eds/proofs_cache.go +++ b/share/eds/proofs_cache.go @@ -113,12 +113,7 @@ func (c *proofsCache) AxisRoots(ctx context.Context) (*share.AxisRoots, error) { } func (c *proofsCache) Sample(ctx context.Context, idx shwap.SampleIndex) (shwap.Sample, error) { - rowIdx, colIdx, err := idx.Coordinates(c.Size(ctx)) - if err != nil { - return shwap.Sample{}, err - } - - axisType, axisIdx, shrIdx := rsmt2d.Row, rowIdx, colIdx + axisType, axisIdx, shrIdx := rsmt2d.Row, idx.Row, idx.Col ax, err := c.axisWithProofs(ctx, axisType, axisIdx) if err != nil { return shwap.Sample{}, err diff --git a/share/eds/rsmt2d.go b/share/eds/rsmt2d.go index d7ee5376a5..286b9baa85 100644 --- a/share/eds/rsmt2d.go +++ b/share/eds/rsmt2d.go @@ -57,12 +57,7 @@ func (eds *Rsmt2D) SampleForProofAxis( idx shwap.SampleIndex, proofType rsmt2d.Axis, ) (shwap.Sample, error) { - rowIdx, colIdx, err := idx.Coordinates(int(eds.Width())) - if err != nil { - return shwap.Sample{}, err - } - - axisIdx, shrIdx := relativeIndexes(rowIdx, colIdx, proofType) + axisIdx, shrIdx := relativeIndexes(idx.Row, idx.Col, proofType) shares, err := getAxis(eds.ExtendedDataSquare, proofType, axisIdx) if err != nil { return shwap.Sample{}, err diff --git a/share/eds/rsmt2d_test.go b/share/eds/rsmt2d_test.go index 4ae001704f..1dd21faa58 100644 --- a/share/eds/rsmt2d_test.go +++ b/share/eds/rsmt2d_test.go @@ -56,8 +56,7 @@ func TestRsmt2dSampleForProofAxis(t *testing.T) { for _, proofType := range []rsmt2d.Axis{rsmt2d.Row, rsmt2d.Col} { for rowIdx := 0; rowIdx < odsSize*2; rowIdx++ { for colIdx := 0; colIdx < odsSize*2; colIdx++ { - idx, err := shwap.SampleIndexFromCoordinates(rowIdx, colIdx, accessor.Size(context.Background())) - require.NoError(t, err) + idx := shwap.SampleIndex{Row: rowIdx, Col: colIdx} sample, err := accessor.SampleForProofAxis(idx, proofType) require.NoError(t, err) diff --git a/share/eds/testing.go b/share/eds/testing.go index c4373780fb..08b723cc81 100644 --- a/share/eds/testing.go +++ b/share/eds/testing.go @@ -148,8 +148,7 @@ func testAccessorSample( // t.Parallel() this fails the test for some reason for rowIdx := 0; rowIdx < width; rowIdx++ { for colIdx := 0; colIdx < width; colIdx++ { - idx, err := shwap.SampleIndexFromCoordinates(rowIdx, colIdx, acc.Size(ctx)) - require.NoError(t, err) + idx := shwap.SampleIndex{Row: rowIdx, Col: colIdx} testSample(ctx, t, acc, roots, idx) } } @@ -164,8 +163,7 @@ func testAccessorSample( for rowIdx := 0; rowIdx < width; rowIdx++ { for colIdx := 0; colIdx < width; colIdx++ { wg.Add(1) - idx, err := shwap.SampleIndexFromCoordinates(rowIdx, colIdx, acc.Size(ctx)) - require.NoError(t, err) + idx := shwap.SampleIndex{Row: rowIdx, Col: colIdx} go func(idx shwap.SampleIndex) { defer wg.Done() testSample(ctx, t, acc, roots, idx) @@ -186,8 +184,9 @@ func testAccessorSample( wg.Add(1) go func() { defer wg.Done() - idx := rand.IntN(int(eds.Width())) //nolint:gosec - testSample(ctx, t, acc, roots, shwap.SampleIndex(idx)) + rowIdx := rand.IntN(int(eds.Width())) //nolint:gosec + colIdx := rand.IntN(int(eds.Width())) //nolint:gosec + testSample(ctx, t, acc, roots, shwap.SampleIndex{Row: rowIdx, Col: colIdx}) }() } wg.Wait() @@ -204,10 +203,7 @@ func testSample( shr, err := acc.Sample(ctx, idx) require.NoError(t, err) - rowIdx, colIdx, err := idx.Coordinates(acc.Size(ctx)) - require.NoError(t, err) - - err = shr.Verify(roots, rowIdx, colIdx) + err = shr.Verify(roots, idx.Row, idx.Col) require.NoError(t, err) } @@ -451,11 +447,10 @@ func BenchGetSampleFromAccessor( name := fmt.Sprintf("Size:%v/quadrant:%s", size, q) b.Run(name, func(b *testing.B) { rowIdx, colIdx := q.coordinates(acc.Size(ctx)) - idx, err := shwap.SampleIndexFromCoordinates(rowIdx, colIdx, acc.Size(ctx)) - require.NoError(b, err) + idx := shwap.SampleIndex{Row: rowIdx, Col: colIdx} // warm up cache - _, err = acc.Sample(ctx, idx) + _, err := acc.Sample(ctx, idx) require.NoError(b, err, q.String()) b.ResetTimer() diff --git a/share/eds/validation_test.go b/share/eds/validation_test.go index fbe17868a9..de550068d1 100644 --- a/share/eds/validation_test.go +++ b/share/eds/validation_test.go @@ -34,14 +34,9 @@ func TestValidation_Sample(t *testing.T) { accessor := &Rsmt2D{ExtendedDataSquare: randEDS} validation := WithValidation(AccessorAndStreamer(accessor, nil)) - idx, err := shwap.SampleIndexFromCoordinates(tt.rowIdx, tt.colIdx, accessor.Size(context.Background())) - if tt.expectFail { - require.ErrorIs(t, err, shwap.ErrInvalidID, tt.name) - return - } - require.NoError(t, err, tt.name) + idx := shwap.SampleIndex{Row: tt.rowIdx, Col: tt.colIdx} - _, err = validation.Sample(context.Background(), idx) + _, err := validation.Sample(context.Background(), idx) if tt.expectFail { require.ErrorIs(t, err, shwap.ErrInvalidID) } else { diff --git a/share/shwap/getters/cascade_test.go b/share/shwap/getters/cascade_test.go index 39e9999fa1..5d944ba9d4 100644 --- a/share/shwap/getters/cascade_test.go +++ b/share/shwap/getters/cascade_test.go @@ -30,7 +30,7 @@ func TestCascadeGetter(t *testing.T) { getter := NewCascadeGetter(getters) t.Run("GetShare", func(t *testing.T) { for _, eh := range headers { - sh, err := getter.GetSamples(ctx, eh, []shwap.SampleIndex{0}) + sh, err := getter.GetSamples(ctx, eh, []shwap.SampleIndex{{}}) assert.NoError(t, err) assert.NotEmpty(t, sh) } diff --git a/share/shwap/p2p/bitswap/sample_block.go b/share/shwap/p2p/bitswap/sample_block.go index f7aa01b642..504e5e5f63 100644 --- a/share/shwap/p2p/bitswap/sample_block.go +++ b/share/shwap/p2p/bitswap/sample_block.go @@ -94,10 +94,7 @@ func (sb *SampleBlock) Marshal() ([]byte, error) { } func (sb *SampleBlock) Populate(ctx context.Context, eds eds.Accessor) error { - idx, err := shwap.SampleIndexFromCoordinates(sb.ID.RowIndex, sb.ID.ShareIndex, eds.Size(ctx)) - if err != nil { - return err - } + idx := shwap.SampleIndex{Row: sb.ID.RowIndex, Col: sb.ID.ShareIndex} smpl, err := eds.Sample(ctx, idx) if err != nil { diff --git a/share/shwap/p2p/bitswap/sample_block_test.go b/share/shwap/p2p/bitswap/sample_block_test.go index 6c12edcb8d..23031e6d72 100644 --- a/share/shwap/p2p/bitswap/sample_block_test.go +++ b/share/shwap/p2p/bitswap/sample_block_test.go @@ -25,8 +25,8 @@ func TestSample_FetchRoundtrip(t *testing.T) { blks := make([]Block, 0, width*width) for x := 0; x < width; x++ { for y := 0; y < width; y++ { - idx, err := shwap.SampleIndexFromCoordinates(x, y, width) - require.NoError(t, err) + idx := shwap.SampleIndex{Row: x, Col: y} + blk, err := NewEmptySampleBlock(1, idx, len(root.RowRoots)) require.NoError(t, err) blks = append(blks, blk) diff --git a/share/shwap/sample.go b/share/shwap/sample.go index 06fa84ab04..afe21680f3 100644 --- a/share/shwap/sample.go +++ b/share/shwap/sample.go @@ -32,12 +32,7 @@ type Sample struct { // SampleFromShares creates a Sample from a list of shares, using the specified proof type and // the share index to be included in the sample. func SampleFromShares(shares []libshare.Share, proofType rsmt2d.Axis, idx SampleIndex) (Sample, error) { - rowIdx, colIdx, err := idx.Coordinates(len(shares)) - if err != nil { - return Sample{}, err - } - - tree := wrapper.NewErasuredNamespacedMerkleTree(uint64(len(shares)/2), uint(rowIdx)) + tree := wrapper.NewErasuredNamespacedMerkleTree(uint64(len(shares)/2), uint(idx.Row)) for _, shr := range shares { err := tree.Push(shr.ToBytes()) if err != nil { @@ -45,13 +40,13 @@ func SampleFromShares(shares []libshare.Share, proofType rsmt2d.Axis, idx Sample } } - proof, err := tree.ProveRange(colIdx, colIdx+1) + proof, err := tree.ProveRange(idx.Col, idx.Col+1) if err != nil { return Sample{}, err } return Sample{ - Share: shares[colIdx], + Share: shares[idx.Col], Proof: &proof, ProofType: proofType, }, nil diff --git a/share/shwap/sample_id.go b/share/shwap/sample_id.go index f2f3c9c2f4..8c56baf367 100644 --- a/share/shwap/sample_id.go +++ b/share/shwap/sample_id.go @@ -10,24 +10,10 @@ import ( // bytes for the ShareIndex. const SampleIDSize = RowIDSize + 2 -type SampleIndex int +type SampleIndex struct { + Row, Col int -func SampleIndexFromCoordinates(rowIdx, colIdx, edsSize int) (SampleIndex, error) { - if rowIdx < 0 || colIdx < 0 { - return 0, fmt.Errorf("negative row or col index: %w", ErrInvalidID) - } - if rowIdx >= edsSize || colIdx >= edsSize { - return 0, fmt.Errorf("SampleIndex %d || %d > %d: %w", rowIdx, colIdx, edsSize, ErrOutOfBounds) - } - return SampleIndex(rowIdx*edsSize + colIdx), nil -} - -func (s SampleIndex) Coordinates(squareSize int) (int, int, error) { - if int(s) > squareSize*squareSize { - return 0, 0, fmt.Errorf("SampleIndex %d > %d: %w", s, squareSize*squareSize, ErrOutOfBounds) - } - - return int(s) / squareSize, int(s) % squareSize, nil + _ struct{} // to prevent unkeyed literals } // SampleID uniquely identifies a specific sample within a row of an Extended Data Square (EDS). @@ -39,19 +25,14 @@ type SampleID struct { // NewSampleID constructs a new SampleID using the provided block height, sample index, and EDS // size. It calculates the row and share index based on the sample index and EDS size. func NewSampleID(height uint64, idx SampleIndex, edsSize int) (SampleID, error) { - rowIdx, colIdx, err := idx.Coordinates(edsSize) - if err != nil { - return SampleID{}, err - } - sid := SampleID{ RowID: RowID{ EdsID: EdsID{ Height: height, }, - RowIndex: rowIdx, + RowIndex: idx.Row, }, - ShareIndex: colIdx, + ShareIndex: idx.Col, } if err := sid.Verify(edsSize); err != nil { diff --git a/share/shwap/sample_id_test.go b/share/shwap/sample_id_test.go index 2344c932ea..499b9abfdb 100644 --- a/share/shwap/sample_id_test.go +++ b/share/shwap/sample_id_test.go @@ -11,7 +11,7 @@ import ( func TestSampleID(t *testing.T) { edsSize := 4 - id, err := NewSampleID(1, 1, edsSize) + id, err := NewSampleID(1, SampleIndex{Col: 1}, edsSize) require.NoError(t, err) data, err := id.MarshalBinary() @@ -29,7 +29,7 @@ func TestSampleID(t *testing.T) { func TestSampleIDReaderWriter(t *testing.T) { edsSize := 4 - id, err := NewSampleID(1, 1, edsSize) + id, err := NewSampleID(1, SampleIndex{Col: 1}, edsSize) require.NoError(t, err) buf := bytes.NewBuffer(nil) @@ -44,15 +44,3 @@ func TestSampleIDReaderWriter(t *testing.T) { require.EqualValues(t, id, sidOut) } - -func TestSampleIndex(t *testing.T) { - edsSize := 16 - - idxIn := SampleIndex(13 * 16) - row, col, err := idxIn.Coordinates(edsSize) - require.NoError(t, err) - - idxOut, err := SampleIndexFromCoordinates(row, col, edsSize) - require.NoError(t, err) - assert.Equal(t, idxIn, idxOut) -} diff --git a/share/shwap/sample_test.go b/share/shwap/sample_test.go index 2a25d369d8..43e8891d73 100644 --- a/share/shwap/sample_test.go +++ b/share/shwap/sample_test.go @@ -25,8 +25,7 @@ func TestSampleValidate(t *testing.T) { for _, proofType := range []rsmt2d.Axis{rsmt2d.Row, rsmt2d.Col} { for rowIdx := 0; rowIdx < odsSize*2; rowIdx++ { for colIdx := 0; colIdx < odsSize*2; colIdx++ { - idx, err := shwap.SampleIndexFromCoordinates(rowIdx, colIdx, int(randEDS.Width())) - require.NoError(t, err) + idx := shwap.SampleIndex{Row: rowIdx, Col: colIdx} sample, err := inMem.SampleForProofAxis(idx, proofType) require.NoError(t, err) @@ -45,7 +44,7 @@ func TestSampleNegativeVerifyInclusion(t *testing.T) { require.NoError(t, err) inMem := eds.Rsmt2D{ExtendedDataSquare: randEDS} - sample, err := inMem.Sample(context.Background(), 0) + sample, err := inMem.Sample(context.Background(), shwap.SampleIndex{}) require.NoError(t, err) err = sample.Verify(root, 0, 0) require.NoError(t, err) @@ -64,14 +63,14 @@ func TestSampleNegativeVerifyInclusion(t *testing.T) { require.ErrorIs(t, err, shwap.ErrFailedVerification) // incorrect proofType - sample, err = inMem.Sample(context.Background(), 0) + sample, err = inMem.Sample(context.Background(), shwap.SampleIndex{}) require.NoError(t, err) sample.ProofType = rsmt2d.Col err = sample.Verify(root, 0, 0) require.ErrorIs(t, err, shwap.ErrFailedVerification) // Corrupt the last root hash byte - sample, err = inMem.Sample(context.Background(), 0) + sample, err = inMem.Sample(context.Background(), shwap.SampleIndex{}) require.NoError(t, err) root.RowRoots[0][len(root.RowRoots[0])-1] ^= 0xFF err = sample.Verify(root, 0, 0) @@ -86,8 +85,7 @@ func TestSampleProtoEncoding(t *testing.T) { for _, proofType := range []rsmt2d.Axis{rsmt2d.Row, rsmt2d.Col} { for rowIdx := 0; rowIdx < odsSize*2; rowIdx++ { for colIdx := 0; colIdx < odsSize*2; colIdx++ { - idx, err := shwap.SampleIndexFromCoordinates(rowIdx, colIdx, int(randEDS.Width())) - require.NoError(t, err) + idx := shwap.SampleIndex{Row: rowIdx, Col: colIdx} sample, err := inMem.SampleForProofAxis(idx, proofType) require.NoError(t, err) @@ -110,7 +108,7 @@ func BenchmarkSampleValidate(b *testing.B) { require.NoError(b, err) inMem := eds.Rsmt2D{ExtendedDataSquare: randEDS} - sample, err := inMem.SampleForProofAxis(0, rsmt2d.Row) + sample, err := inMem.SampleForProofAxis(shwap.SampleIndex{}, rsmt2d.Row) require.NoError(b, err) b.ResetTimer() diff --git a/store/file/ods.go b/store/file/ods.go index 0e7efd88bc..7edac00c2d 100644 --- a/store/file/ods.go +++ b/store/file/ods.go @@ -236,10 +236,7 @@ func (o *ODS) Sample(ctx context.Context, idx shwap.SampleIndex) (shwap.Sample, // column than reading full ODS to calculate single row // - For the fourth quadrant, it does not matter which axis we read because we need to read full ODS // to calculate the sample - rowIdx, colIdx, err := idx.Coordinates(o.Size(ctx)) - if err != nil { - return shwap.Sample{}, err - } + rowIdx, colIdx := idx.Row, idx.Col axisType, axisIdx, shrIdx := rsmt2d.Row, rowIdx, colIdx if colIdx < o.size()/2 && rowIdx >= o.size()/2 { @@ -251,12 +248,9 @@ func (o *ODS) Sample(ctx context.Context, idx shwap.SampleIndex) (shwap.Sample, return shwap.Sample{}, fmt.Errorf("reading axis: %w", err) } - idx, err = shwap.SampleIndexFromCoordinates(axisIdx, shrIdx, o.Size(ctx)) - if err != nil { - return shwap.Sample{}, err - } + idxNew := shwap.SampleIndex{Row: axisIdx, Col: shrIdx} - return shwap.SampleFromShares(axis, axisType, idx) + return shwap.SampleFromShares(axis, axisType, idxNew) } // AxisHalf returns half of shares axis of the given type and index. Side is determined by diff --git a/store/file/ods_q4.go b/store/file/ods_q4.go index 799a87e48f..a4d6cbd681 100644 --- a/store/file/ods_q4.go +++ b/store/file/ods_q4.go @@ -123,12 +123,8 @@ func (odsq4 *ODSQ4) AxisRoots(ctx context.Context) (*share.AxisRoots, error) { } func (odsq4 *ODSQ4) Sample(ctx context.Context, idx shwap.SampleIndex) (shwap.Sample, error) { - rowIdw, _, err := idx.Coordinates(odsq4.Size(ctx)) - if err != nil { - return shwap.Sample{}, err - } // use native AxisHalf implementation, to read axis from q4 quadrant when possible - half, err := odsq4.AxisHalf(ctx, rsmt2d.Row, rowIdw) + half, err := odsq4.AxisHalf(ctx, rsmt2d.Row, idx.Row) if err != nil { return shwap.Sample{}, fmt.Errorf("reading axis: %w", err) } diff --git a/store/getter_test.go b/store/getter_test.go index d80221c9f5..b6b2f02200 100644 --- a/store/getter_test.go +++ b/store/getter_test.go @@ -36,8 +36,8 @@ func TestStoreGetter(t *testing.T) { squareSize := int(eds.Width()) for i := 0; i < squareSize; i++ { for j := 0; j < squareSize; j++ { - idx, err := shwap.SampleIndexFromCoordinates(i, j, len(eh.DAH.RowRoots)) - require.NoError(t, err) + idx := shwap.SampleIndex{Row: i, Col: j} + smpls, err := sg.GetSamples(ctx, eh, []shwap.SampleIndex{idx}) require.NoError(t, err) require.Equal(t, eds.GetCell(uint(i), uint(j)), smpls[0].Share.ToBytes()) @@ -45,7 +45,8 @@ func TestStoreGetter(t *testing.T) { } // doesn't panic on indexes too high - _, err = sg.GetSamples(ctx, eh, []shwap.SampleIndex{shwap.SampleIndex(squareSize * squareSize)}) + bigIdx := squareSize * squareSize + _, err = sg.GetSamples(ctx, eh, []shwap.SampleIndex{{Row: bigIdx, Col: bigIdx}}) require.ErrorIs(t, err, shwap.ErrOutOfBounds) }) From 7bc54ae833a444e06d540da311d36b94ff45f6f6 Mon Sep 17 00:00:00 2001 From: Oleg Kovalov Date: Wed, 20 Nov 2024 14:10:34 +0100 Subject: [PATCH 14/26] add helpers --- share/shwap/sample_id.go | 20 +++++++++++++++++++- share/shwap/sample_id_test.go | 12 ++++++++++++ 2 files changed, 31 insertions(+), 1 deletion(-) diff --git a/share/shwap/sample_id.go b/share/shwap/sample_id.go index 8c56baf367..b83ff1199f 100644 --- a/share/shwap/sample_id.go +++ b/share/shwap/sample_id.go @@ -12,8 +12,26 @@ const SampleIDSize = RowIDSize + 2 type SampleIndex struct { Row, Col int +} + +func SampleIndexAs1DIndex(idx SampleIndex, edsSize int) (int, error) { + if idx.Row < 0 || idx.Col < 0 { + return 0, fmt.Errorf("negative row or col index: %w", ErrInvalidID) + } + if idx.Row >= edsSize || idx.Col >= edsSize { + return 0, fmt.Errorf("SampleIndex %d || %d > %d: %w", idx.Row, idx.Col, edsSize, ErrOutOfBounds) + } + return idx.Row*edsSize + idx.Col, nil +} + +func SampleIndexFrom1DIndex(idx, squareSize int) (SampleIndex, error) { + if int(idx) > squareSize*squareSize { + return SampleIndex{}, fmt.Errorf("SampleIndex %d > %d: %w", idx, squareSize*squareSize, ErrOutOfBounds) + } - _ struct{} // to prevent unkeyed literals + rowIdx := int(idx) / squareSize + colIdx := int(idx) % squareSize + return SampleIndex{Row: rowIdx, Col: colIdx}, nil } // SampleID uniquely identifies a specific sample within a row of an Extended Data Square (EDS). diff --git a/share/shwap/sample_id_test.go b/share/shwap/sample_id_test.go index 499b9abfdb..2b492e0742 100644 --- a/share/shwap/sample_id_test.go +++ b/share/shwap/sample_id_test.go @@ -44,3 +44,15 @@ func TestSampleIDReaderWriter(t *testing.T) { require.EqualValues(t, id, sidOut) } + +func TestSampleIndex(t *testing.T) { + edsSize := 16 + + rawIdx := 13 * 16 + idxIn, err := SampleIndexFrom1DIndex(rawIdx, edsSize) + require.NoError(t, err) + + idxOut, err := SampleIndexAs1DIndex(idxIn, edsSize) + require.NoError(t, err) + assert.Equal(t, rawIdx, idxOut) +} From fddcf4a7feb4ac755ef57a1b82ff15ba68b4dea9 Mon Sep 17 00:00:00 2001 From: Oleg Kovalov Date: Wed, 20 Nov 2024 14:14:45 +0100 Subject: [PATCH 15/26] rename to SampleCoords --- blob/service_test.go | 18 +++++++++--------- nodebuilder/share/mocks/api.go | 16 ++++++++++++++++ nodebuilder/share/share.go | 12 ++++++------ share/availability/light/availability.go | 6 +++--- share/availability/light/availability_test.go | 4 ++-- share/eds/accessor.go | 2 +- share/eds/close_once.go | 2 +- share/eds/close_once_test.go | 6 +++--- share/eds/proofs_cache.go | 2 +- share/eds/rsmt2d.go | 4 ++-- share/eds/rsmt2d_test.go | 2 +- share/eds/testing.go | 12 ++++++------ share/eds/validation.go | 2 +- share/eds/validation_test.go | 2 +- share/shwap/getter.go | 2 +- share/shwap/getters/cascade.go | 2 +- share/shwap/getters/cascade_test.go | 2 +- share/shwap/getters/mock/getter.go | 2 +- share/shwap/getters/testing.go | 2 +- share/shwap/p2p/bitswap/getter.go | 2 +- share/shwap/p2p/bitswap/sample_block.go | 4 ++-- share/shwap/p2p/bitswap/sample_block_test.go | 2 +- share/shwap/p2p/shrex/shrex_getter/shrex.go | 2 +- share/shwap/sample.go | 2 +- share/shwap/sample_id.go | 14 +++++++------- share/shwap/sample_id_test.go | 10 +++++----- share/shwap/sample_test.go | 12 ++++++------ store/cache/accessor_cache_test.go | 2 +- store/cache/noop.go | 2 +- store/file/ods.go | 4 ++-- store/file/ods_q4.go | 2 +- store/getter.go | 2 +- store/getter_test.go | 6 +++--- 33 files changed, 91 insertions(+), 75 deletions(-) diff --git a/blob/service_test.go b/blob/service_test.go index 37130c9696..8a01e10e9c 100644 --- a/blob/service_test.go +++ b/blob/service_test.go @@ -145,9 +145,9 @@ func TestBlobService_Get(t *testing.T) { shareOffset := 0 for i := range blobs { row, col := calculateIndex(len(h.DAH.RowRoots), blobs[i].index) - idx := shwap.SampleIndex{Row: row, Col: col} + idx := shwap.SampleCoords{Row: row, Col: col} require.NoError(t, err) - smpls, err := service.shareGetter.GetSamples(ctx, h, []shwap.SampleIndex{idx}) + smpls, err := service.shareGetter.GetSamples(ctx, h, []shwap.SampleCoords{idx}) require.NoError(t, err) require.True(t, bytes.Equal(smpls[0].Share.ToBytes(), resultShares[shareOffset].ToBytes()), fmt.Sprintf("issue on %d attempt. ROW:%d, COL: %d, blobIndex:%d", i, row, col, blobs[i].index), @@ -489,10 +489,10 @@ func TestService_GetSingleBlobWithoutPadding(t *testing.T) { h, err := service.headerGetter(ctx, 1) require.NoError(t, err) row, col := calculateIndex(len(h.DAH.RowRoots), newBlob.index) - idx := shwap.SampleIndex{Row: row, Col: col} + idx := shwap.SampleCoords{Row: row, Col: col} require.NoError(t, err) - smpls, err := service.shareGetter.GetSamples(ctx, h, []shwap.SampleIndex{idx}) + smpls, err := service.shareGetter.GetSamples(ctx, h, []shwap.SampleCoords{idx}) require.NoError(t, err) assert.Equal(t, smpls[0].Share, resultShares[0]) @@ -526,10 +526,10 @@ func TestService_Get(t *testing.T) { assert.Equal(t, b.Commitment, blob.Commitment) row, col := calculateIndex(len(h.DAH.RowRoots), b.index) - idx := shwap.SampleIndex{Row: row, Col: col} + idx := shwap.SampleCoords{Row: row, Col: col} require.NoError(t, err) - smpls, err := service.shareGetter.GetSamples(ctx, h, []shwap.SampleIndex{idx}) + smpls, err := service.shareGetter.GetSamples(ctx, h, []shwap.SampleCoords{idx}) require.NoError(t, err) assert.Equal(t, smpls[0].Share, resultShares[shareOffset], fmt.Sprintf("issue on %d attempt", i)) @@ -588,10 +588,10 @@ func TestService_GetAllWithoutPadding(t *testing.T) { require.True(t, blobs[i].compareCommitments(blob.Commitment)) row, col := calculateIndex(len(h.DAH.RowRoots), blob.index) - idx := shwap.SampleIndex{Row: row, Col: col} + idx := shwap.SampleCoords{Row: row, Col: col} require.NoError(t, err) - smpls, err := service.shareGetter.GetSamples(ctx, h, []shwap.SampleIndex{idx}) + smpls, err := service.shareGetter.GetSamples(ctx, h, []shwap.SampleCoords{idx}) require.NoError(t, err) assert.Equal(t, smpls[0].Share, resultShares[shareOffset]) @@ -914,7 +914,7 @@ func createService(ctx context.Context, t testing.TB, shares []libshare.Share) * return nd, err }) shareGetter.EXPECT().GetSamples(gomock.Any(), gomock.Any(), gomock.Any()).AnyTimes(). - DoAndReturn(func(ctx context.Context, h *header.ExtendedHeader, indices []shwap.SampleIndex) ([]shwap.Sample, error) { + DoAndReturn(func(ctx context.Context, h *header.ExtendedHeader, indices []shwap.SampleCoords) ([]shwap.Sample, error) { smpl, err := accessor.Sample(ctx, indices[0]) return []shwap.Sample{smpl}, err }) diff --git a/nodebuilder/share/mocks/api.go b/nodebuilder/share/mocks/api.go index 84866bf784..7fde2338cc 100644 --- a/nodebuilder/share/mocks/api.go +++ b/nodebuilder/share/mocks/api.go @@ -8,6 +8,7 @@ import ( context "context" reflect "reflect" + header "github.com/celestiaorg/celestia-node/header" share "github.com/celestiaorg/celestia-node/nodebuilder/share" shwap "github.com/celestiaorg/celestia-node/share/shwap" share0 "github.com/celestiaorg/go-square/v2/share" @@ -83,6 +84,21 @@ func (mr *MockModuleMockRecorder) GetRange(arg0, arg1, arg2, arg3 interface{}) * return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GetRange", reflect.TypeOf((*MockModule)(nil).GetRange), arg0, arg1, arg2, arg3) } +// GetSamples mocks base method. +func (m *MockModule) GetSamples(arg0 context.Context, arg1 *header.ExtendedHeader, arg2 []shwap.SampleCoords) ([]shwap.Sample, error) { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "GetSamples", arg0, arg1, arg2) + ret0, _ := ret[0].([]shwap.Sample) + ret1, _ := ret[1].(error) + return ret0, ret1 +} + +// GetSamples indicates an expected call of GetSamples. +func (mr *MockModuleMockRecorder) GetSamples(arg0, arg1, arg2 interface{}) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GetSamples", reflect.TypeOf((*MockModule)(nil).GetSamples), arg0, arg1, arg2) +} + // GetShare mocks base method. func (m *MockModule) GetShare(arg0 context.Context, arg1 uint64, arg2, arg3 int) (share0.Share, error) { m.ctrl.T.Helper() diff --git a/nodebuilder/share/share.go b/nodebuilder/share/share.go index 2b7e10a92a..783018fe47 100644 --- a/nodebuilder/share/share.go +++ b/nodebuilder/share/share.go @@ -47,7 +47,7 @@ type Module interface { // GetShare gets a Share by coordinates in EDS. GetShare(ctx context.Context, height uint64, row, col int) (libshare.Share, error) // GetSamples gets sample for given indices. - GetSamples(ctx context.Context, header *header.ExtendedHeader, indices []shwap.SampleIndex) ([]shwap.Sample, error) + GetSamples(ctx context.Context, header *header.ExtendedHeader, indices []shwap.SampleCoords) ([]shwap.Sample, error) // GetEDS gets the full EDS identified by the given extended header. GetEDS(ctx context.Context, height uint64) (*rsmt2d.ExtendedDataSquare, error) // GetNamespaceData gets all shares from an EDS within the given namespace. @@ -71,7 +71,7 @@ type API struct { GetSamples func( ctx context.Context, header *header.ExtendedHeader, - indices []shwap.SampleIndex, + indices []shwap.SampleCoords, ) ([]shwap.Sample, error) `perm:"read"` GetEDS func( ctx context.Context, @@ -99,7 +99,7 @@ func (api *API) GetShare(ctx context.Context, height uint64, row, col int) (libs } func (api *API) GetSamples(ctx context.Context, header *header.ExtendedHeader, - indices []shwap.SampleIndex, + indices []shwap.SampleCoords, ) ([]shwap.Sample, error) { return api.Internal.GetSamples(ctx, header, indices) } @@ -132,9 +132,9 @@ func (m module) GetShare(ctx context.Context, height uint64, row, col int) (libs return libshare.Share{}, err } - idx := shwap.SampleIndex{Row: row, Col: col} + idx := shwap.SampleCoords{Row: row, Col: col} - smpls, err := m.getter.GetSamples(ctx, header, []shwap.SampleIndex{idx}) + smpls, err := m.getter.GetSamples(ctx, header, []shwap.SampleCoords{idx}) if err != nil { return libshare.Share{}, err } @@ -143,7 +143,7 @@ func (m module) GetShare(ctx context.Context, height uint64, row, col int) (libs } func (m module) GetSamples(ctx context.Context, header *header.ExtendedHeader, - indices []shwap.SampleIndex, + indices []shwap.SampleCoords, ) ([]shwap.Sample, error) { return m.getter.GetSamples(ctx, header, indices) } diff --git a/share/availability/light/availability.go b/share/availability/light/availability.go index 220e8ec7bb..287766be4c 100644 --- a/share/availability/light/availability.go +++ b/share/availability/light/availability.go @@ -115,9 +115,9 @@ func (la *ShareAvailability) SharesAvailable(ctx context.Context, header *header log.Debugw("starting sampling session", "root", dah.String()) - idxs := make([]shwap.SampleIndex, len(samples.Remaining)) + idxs := make([]shwap.SampleCoords, len(samples.Remaining)) for i, s := range samples.Remaining { - idxs[i] = shwap.SampleIndex{Row: s.Row, Col: s.Col} + idxs[i] = shwap.SampleCoords{Row: s.Row, Col: s.Col} } smpls, err := la.getter.GetSamples(ctx, header, idxs) @@ -197,7 +197,7 @@ func (la *ShareAvailability) Prune(ctx context.Context, h *header.ExtendedHeader // delete stored samples for _, sample := range result.Available { - idx := shwap.SampleIndex{Row: sample.Row, Col: sample.Col} + idx := shwap.SampleCoords{Row: sample.Row, Col: sample.Col} blk, err := bitswap.NewEmptySampleBlock(h.Height(), idx, len(h.DAH.RowRoots)) if err != nil { diff --git a/share/availability/light/availability_test.go b/share/availability/light/availability_test.go index c70c511642..dce5021abb 100644 --- a/share/availability/light/availability_test.go +++ b/share/availability/light/availability_test.go @@ -51,7 +51,7 @@ func TestSharesAvailableSuccess(t *testing.T) { getter.EXPECT(). GetSamples(gomock.Any(), eh, gomock.Any()). DoAndReturn( - func(_ context.Context, hdr *header.ExtendedHeader, indices []shwap.SampleIndex) ([]shwap.Sample, error) { + func(_ context.Context, hdr *header.ExtendedHeader, indices []shwap.SampleCoords) ([]shwap.Sample, error) { acc := eds.Rsmt2D{ExtendedDataSquare: square} smpls := make([]shwap.Sample, len(indices)) for i, idx := range indices { @@ -279,7 +279,7 @@ func (g successGetter) sampledList() []Sample { } func (g successGetter) GetSamples(_ context.Context, hdr *header.ExtendedHeader, - indices []shwap.SampleIndex, + indices []shwap.SampleCoords, ) ([]shwap.Sample, error) { g.Lock() defer g.Unlock() diff --git a/share/eds/accessor.go b/share/eds/accessor.go index e262382790..eb3fea6e41 100644 --- a/share/eds/accessor.go +++ b/share/eds/accessor.go @@ -25,7 +25,7 @@ type Accessor interface { // Sample returns share and corresponding proof for row and column indices. Implementation can // choose which axis to use for proof. Chosen axis for proof should be indicated in the returned // Sample. - Sample(ctx context.Context, idx shwap.SampleIndex) (shwap.Sample, error) + Sample(ctx context.Context, idx shwap.SampleCoords) (shwap.Sample, error) // AxisHalf returns half of shares axis of the given type and index. Side is determined by // implementation. Implementations should indicate the side in the returned AxisHalf. AxisHalf(ctx context.Context, axisType rsmt2d.Axis, axisIdx int) (AxisHalf, error) diff --git a/share/eds/close_once.go b/share/eds/close_once.go index 6de62419fa..579b4b1e20 100644 --- a/share/eds/close_once.go +++ b/share/eds/close_once.go @@ -57,7 +57,7 @@ func (c *closeOnce) AxisRoots(ctx context.Context) (*share.AxisRoots, error) { return c.f.AxisRoots(ctx) } -func (c *closeOnce) Sample(ctx context.Context, idx shwap.SampleIndex) (shwap.Sample, error) { +func (c *closeOnce) Sample(ctx context.Context, idx shwap.SampleCoords) (shwap.Sample, error) { if c.closed.Load() { return shwap.Sample{}, errAccessorClosed } diff --git a/share/eds/close_once_test.go b/share/eds/close_once_test.go index a3c9cd96ee..e34299f805 100644 --- a/share/eds/close_once_test.go +++ b/share/eds/close_once_test.go @@ -20,7 +20,7 @@ func TestWithClosedOnce(t *testing.T) { stub := &stubEdsAccessorCloser{} closedOnce := WithClosedOnce(stub) - _, err := closedOnce.Sample(ctx, shwap.SampleIndex{}) + _, err := closedOnce.Sample(ctx, shwap.SampleCoords{}) require.NoError(t, err) _, err = closedOnce.AxisHalf(ctx, rsmt2d.Row, 0) require.NoError(t, err) @@ -33,7 +33,7 @@ func TestWithClosedOnce(t *testing.T) { require.True(t, stub.closed) // Ensure that the underlying file is not accessible after closing - _, err = closedOnce.Sample(ctx, shwap.SampleIndex{}) + _, err = closedOnce.Sample(ctx, shwap.SampleCoords{}) require.ErrorIs(t, err, errAccessorClosed) _, err = closedOnce.AxisHalf(ctx, rsmt2d.Row, 0) require.ErrorIs(t, err, errAccessorClosed) @@ -59,7 +59,7 @@ func (s *stubEdsAccessorCloser) AxisRoots(context.Context) (*share.AxisRoots, er return &share.AxisRoots{}, nil } -func (s *stubEdsAccessorCloser) Sample(context.Context, shwap.SampleIndex) (shwap.Sample, error) { +func (s *stubEdsAccessorCloser) Sample(context.Context, shwap.SampleCoords) (shwap.Sample, error) { return shwap.Sample{}, nil } diff --git a/share/eds/proofs_cache.go b/share/eds/proofs_cache.go index 0b70a6006c..f73ebcdf80 100644 --- a/share/eds/proofs_cache.go +++ b/share/eds/proofs_cache.go @@ -112,7 +112,7 @@ func (c *proofsCache) AxisRoots(ctx context.Context) (*share.AxisRoots, error) { return roots, nil } -func (c *proofsCache) Sample(ctx context.Context, idx shwap.SampleIndex) (shwap.Sample, error) { +func (c *proofsCache) Sample(ctx context.Context, idx shwap.SampleCoords) (shwap.Sample, error) { axisType, axisIdx, shrIdx := rsmt2d.Row, idx.Row, idx.Col ax, err := c.axisWithProofs(ctx, axisType, axisIdx) if err != nil { diff --git a/share/eds/rsmt2d.go b/share/eds/rsmt2d.go index 286b9baa85..6c244de700 100644 --- a/share/eds/rsmt2d.go +++ b/share/eds/rsmt2d.go @@ -46,7 +46,7 @@ func (eds *Rsmt2D) AxisRoots(context.Context) (*share.AxisRoots, error) { // Sample returns share and corresponding proof for row and column indices. func (eds *Rsmt2D) Sample( _ context.Context, - idx shwap.SampleIndex, + idx shwap.SampleCoords, ) (shwap.Sample, error) { return eds.SampleForProofAxis(idx, rsmt2d.Row) } @@ -54,7 +54,7 @@ func (eds *Rsmt2D) Sample( // SampleForProofAxis samples a share from an Extended Data Square based on the provided // row and column indices and proof axis. It returns a sample with the share and proof. func (eds *Rsmt2D) SampleForProofAxis( - idx shwap.SampleIndex, + idx shwap.SampleCoords, proofType rsmt2d.Axis, ) (shwap.Sample, error) { axisIdx, shrIdx := relativeIndexes(idx.Row, idx.Col, proofType) diff --git a/share/eds/rsmt2d_test.go b/share/eds/rsmt2d_test.go index 1dd21faa58..89fd316717 100644 --- a/share/eds/rsmt2d_test.go +++ b/share/eds/rsmt2d_test.go @@ -56,7 +56,7 @@ func TestRsmt2dSampleForProofAxis(t *testing.T) { for _, proofType := range []rsmt2d.Axis{rsmt2d.Row, rsmt2d.Col} { for rowIdx := 0; rowIdx < odsSize*2; rowIdx++ { for colIdx := 0; colIdx < odsSize*2; colIdx++ { - idx := shwap.SampleIndex{Row: rowIdx, Col: colIdx} + idx := shwap.SampleCoords{Row: rowIdx, Col: colIdx} sample, err := accessor.SampleForProofAxis(idx, proofType) require.NoError(t, err) diff --git a/share/eds/testing.go b/share/eds/testing.go index 08b723cc81..c8859dca5a 100644 --- a/share/eds/testing.go +++ b/share/eds/testing.go @@ -148,7 +148,7 @@ func testAccessorSample( // t.Parallel() this fails the test for some reason for rowIdx := 0; rowIdx < width; rowIdx++ { for colIdx := 0; colIdx < width; colIdx++ { - idx := shwap.SampleIndex{Row: rowIdx, Col: colIdx} + idx := shwap.SampleCoords{Row: rowIdx, Col: colIdx} testSample(ctx, t, acc, roots, idx) } } @@ -163,8 +163,8 @@ func testAccessorSample( for rowIdx := 0; rowIdx < width; rowIdx++ { for colIdx := 0; colIdx < width; colIdx++ { wg.Add(1) - idx := shwap.SampleIndex{Row: rowIdx, Col: colIdx} - go func(idx shwap.SampleIndex) { + idx := shwap.SampleCoords{Row: rowIdx, Col: colIdx} + go func(idx shwap.SampleCoords) { defer wg.Done() testSample(ctx, t, acc, roots, idx) }(idx) @@ -186,7 +186,7 @@ func testAccessorSample( defer wg.Done() rowIdx := rand.IntN(int(eds.Width())) //nolint:gosec colIdx := rand.IntN(int(eds.Width())) //nolint:gosec - testSample(ctx, t, acc, roots, shwap.SampleIndex{Row: rowIdx, Col: colIdx}) + testSample(ctx, t, acc, roots, shwap.SampleCoords{Row: rowIdx, Col: colIdx}) }() } wg.Wait() @@ -198,7 +198,7 @@ func testSample( t *testing.T, acc Accessor, roots *share.AxisRoots, - idx shwap.SampleIndex, + idx shwap.SampleCoords, ) { shr, err := acc.Sample(ctx, idx) require.NoError(t, err) @@ -447,7 +447,7 @@ func BenchGetSampleFromAccessor( name := fmt.Sprintf("Size:%v/quadrant:%s", size, q) b.Run(name, func(b *testing.B) { rowIdx, colIdx := q.coordinates(acc.Size(ctx)) - idx := shwap.SampleIndex{Row: rowIdx, Col: colIdx} + idx := shwap.SampleCoords{Row: rowIdx, Col: colIdx} // warm up cache _, err := acc.Sample(ctx, idx) diff --git a/share/eds/validation.go b/share/eds/validation.go index d3323f87ac..4f6cf0aa85 100644 --- a/share/eds/validation.go +++ b/share/eds/validation.go @@ -34,7 +34,7 @@ func (f validation) Size(ctx context.Context) int { return int(size) } -func (f validation) Sample(ctx context.Context, idx shwap.SampleIndex) (shwap.Sample, error) { +func (f validation) Sample(ctx context.Context, idx shwap.SampleCoords) (shwap.Sample, error) { _, err := shwap.NewSampleID(1, idx, f.Size(ctx)) if err != nil { return shwap.Sample{}, fmt.Errorf("sample validation: %w", err) diff --git a/share/eds/validation_test.go b/share/eds/validation_test.go index de550068d1..9ec6b3fdb3 100644 --- a/share/eds/validation_test.go +++ b/share/eds/validation_test.go @@ -34,7 +34,7 @@ func TestValidation_Sample(t *testing.T) { accessor := &Rsmt2D{ExtendedDataSquare: randEDS} validation := WithValidation(AccessorAndStreamer(accessor, nil)) - idx := shwap.SampleIndex{Row: tt.rowIdx, Col: tt.colIdx} + idx := shwap.SampleCoords{Row: tt.rowIdx, Col: tt.colIdx} _, err := validation.Sample(context.Background(), idx) if tt.expectFail { diff --git a/share/shwap/getter.go b/share/shwap/getter.go index f88a10debe..2d52397e4c 100644 --- a/share/shwap/getter.go +++ b/share/shwap/getter.go @@ -32,7 +32,7 @@ type Getter interface { // GetSamples gets samples by their indices. // Returns Sample slice with requested number of samples in the requested order. // May return partial response with some samples being empty if they weren't found. - GetSamples(ctx context.Context, header *header.ExtendedHeader, indices []SampleIndex) ([]Sample, error) + GetSamples(ctx context.Context, header *header.ExtendedHeader, indices []SampleCoords) ([]Sample, error) // GetEDS gets the full EDS identified by the given extended header. GetEDS(context.Context, *header.ExtendedHeader) (*rsmt2d.ExtendedDataSquare, error) diff --git a/share/shwap/getters/cascade.go b/share/shwap/getters/cascade.go index 8b0ce5349c..39ceb2fdb1 100644 --- a/share/shwap/getters/cascade.go +++ b/share/shwap/getters/cascade.go @@ -43,7 +43,7 @@ func NewCascadeGetter(getters []shwap.Getter) *CascadeGetter { // GetSamples gets samples from any of registered shwap.Getters in cascading order. func (cg *CascadeGetter) GetSamples(ctx context.Context, hdr *header.ExtendedHeader, - indices []shwap.SampleIndex, + indices []shwap.SampleCoords, ) ([]shwap.Sample, error) { ctx, span := tracer.Start(ctx, "cascade/get-samples", trace.WithAttributes( attribute.Int("amount", len(indices)), diff --git a/share/shwap/getters/cascade_test.go b/share/shwap/getters/cascade_test.go index 5d944ba9d4..8237d8ca5c 100644 --- a/share/shwap/getters/cascade_test.go +++ b/share/shwap/getters/cascade_test.go @@ -30,7 +30,7 @@ func TestCascadeGetter(t *testing.T) { getter := NewCascadeGetter(getters) t.Run("GetShare", func(t *testing.T) { for _, eh := range headers { - sh, err := getter.GetSamples(ctx, eh, []shwap.SampleIndex{{}}) + sh, err := getter.GetSamples(ctx, eh, []shwap.SampleCoords{{}}) assert.NoError(t, err) assert.NotEmpty(t, sh) } diff --git a/share/shwap/getters/mock/getter.go b/share/shwap/getters/mock/getter.go index 235b5a1fc7..7e4dacb24a 100644 --- a/share/shwap/getters/mock/getter.go +++ b/share/shwap/getters/mock/getter.go @@ -69,7 +69,7 @@ func (mr *MockGetterMockRecorder) GetNamespaceData(arg0, arg1, arg2 interface{}) } // GetSamples mocks base method. -func (m *MockGetter) GetSamples(arg0 context.Context, arg1 *header.ExtendedHeader, arg2 []shwap.SampleIndex) ([]shwap.Sample, error) { +func (m *MockGetter) GetSamples(arg0 context.Context, arg1 *header.ExtendedHeader, arg2 []shwap.SampleCoords) ([]shwap.Sample, error) { m.ctrl.T.Helper() ret := m.ctrl.Call(m, "GetSamples", arg0, arg1, arg2) ret0, _ := ret[0].([]shwap.Sample) diff --git a/share/shwap/getters/testing.go b/share/shwap/getters/testing.go index 3b1f4bb8c9..a3ee53753d 100644 --- a/share/shwap/getters/testing.go +++ b/share/shwap/getters/testing.go @@ -38,7 +38,7 @@ type SingleEDSGetter struct { // GetSamples get samples from a kept EDS if exist and if the correct root is given. func (seg *SingleEDSGetter) GetSamples(ctx context.Context, hdr *header.ExtendedHeader, - indices []shwap.SampleIndex, + indices []shwap.SampleCoords, ) ([]shwap.Sample, error) { err := seg.checkRoots(hdr.DAH) if err != nil { diff --git a/share/shwap/p2p/bitswap/getter.go b/share/shwap/p2p/bitswap/getter.go index e46f0d24c6..0c7c4bbf30 100644 --- a/share/shwap/p2p/bitswap/getter.go +++ b/share/shwap/p2p/bitswap/getter.go @@ -79,7 +79,7 @@ func (g *Getter) Stop() { func (g *Getter) GetSamples( ctx context.Context, hdr *header.ExtendedHeader, - indices []shwap.SampleIndex, + indices []shwap.SampleCoords, ) ([]shwap.Sample, error) { if len(indices) == 0 { return nil, fmt.Errorf("no sample indicies to fetch") diff --git a/share/shwap/p2p/bitswap/sample_block.go b/share/shwap/p2p/bitswap/sample_block.go index 504e5e5f63..8d22047330 100644 --- a/share/shwap/p2p/bitswap/sample_block.go +++ b/share/shwap/p2p/bitswap/sample_block.go @@ -47,7 +47,7 @@ type SampleBlock struct { } // NewEmptySampleBlock constructs a new empty SampleBlock. -func NewEmptySampleBlock(height uint64, idx shwap.SampleIndex, edsSize int) (*SampleBlock, error) { +func NewEmptySampleBlock(height uint64, idx shwap.SampleCoords, edsSize int) (*SampleBlock, error) { id, err := shwap.NewSampleID(height, idx, edsSize) if err != nil { return nil, err @@ -94,7 +94,7 @@ func (sb *SampleBlock) Marshal() ([]byte, error) { } func (sb *SampleBlock) Populate(ctx context.Context, eds eds.Accessor) error { - idx := shwap.SampleIndex{Row: sb.ID.RowIndex, Col: sb.ID.ShareIndex} + idx := shwap.SampleCoords{Row: sb.ID.RowIndex, Col: sb.ID.ShareIndex} smpl, err := eds.Sample(ctx, idx) if err != nil { diff --git a/share/shwap/p2p/bitswap/sample_block_test.go b/share/shwap/p2p/bitswap/sample_block_test.go index 23031e6d72..3339f24314 100644 --- a/share/shwap/p2p/bitswap/sample_block_test.go +++ b/share/shwap/p2p/bitswap/sample_block_test.go @@ -25,7 +25,7 @@ func TestSample_FetchRoundtrip(t *testing.T) { blks := make([]Block, 0, width*width) for x := 0; x < width; x++ { for y := 0; y < width; y++ { - idx := shwap.SampleIndex{Row: x, Col: y} + idx := shwap.SampleCoords{Row: x, Col: y} blk, err := NewEmptySampleBlock(1, idx, len(root.RowRoots)) require.NoError(t, err) diff --git a/share/shwap/p2p/shrex/shrex_getter/shrex.go b/share/shwap/p2p/shrex/shrex_getter/shrex.go index 6247223622..6c91a44736 100644 --- a/share/shwap/p2p/shrex/shrex_getter/shrex.go +++ b/share/shwap/p2p/shrex/shrex_getter/shrex.go @@ -146,7 +146,7 @@ func (sg *Getter) Stop(ctx context.Context) error { return sg.archivalPeerManager.Stop(ctx) } -func (sg *Getter) GetSamples(context.Context, *header.ExtendedHeader, []shwap.SampleIndex) ([]shwap.Sample, error) { +func (sg *Getter) GetSamples(context.Context, *header.ExtendedHeader, []shwap.SampleCoords) ([]shwap.Sample, error) { return nil, fmt.Errorf("getter/shrex: GetShare %w", shwap.ErrOperationNotSupported) } diff --git a/share/shwap/sample.go b/share/shwap/sample.go index afe21680f3..9b8e16a93d 100644 --- a/share/shwap/sample.go +++ b/share/shwap/sample.go @@ -31,7 +31,7 @@ type Sample struct { // SampleFromShares creates a Sample from a list of shares, using the specified proof type and // the share index to be included in the sample. -func SampleFromShares(shares []libshare.Share, proofType rsmt2d.Axis, idx SampleIndex) (Sample, error) { +func SampleFromShares(shares []libshare.Share, proofType rsmt2d.Axis, idx SampleCoords) (Sample, error) { tree := wrapper.NewErasuredNamespacedMerkleTree(uint64(len(shares)/2), uint(idx.Row)) for _, shr := range shares { err := tree.Push(shr.ToBytes()) diff --git a/share/shwap/sample_id.go b/share/shwap/sample_id.go index b83ff1199f..f7fac6d697 100644 --- a/share/shwap/sample_id.go +++ b/share/shwap/sample_id.go @@ -10,28 +10,28 @@ import ( // bytes for the ShareIndex. const SampleIDSize = RowIDSize + 2 -type SampleIndex struct { +type SampleCoords struct { Row, Col int } -func SampleIndexAs1DIndex(idx SampleIndex, edsSize int) (int, error) { +func SampleCoordsAs1DIndex(idx SampleCoords, edsSize int) (int, error) { if idx.Row < 0 || idx.Col < 0 { return 0, fmt.Errorf("negative row or col index: %w", ErrInvalidID) } if idx.Row >= edsSize || idx.Col >= edsSize { - return 0, fmt.Errorf("SampleIndex %d || %d > %d: %w", idx.Row, idx.Col, edsSize, ErrOutOfBounds) + return 0, fmt.Errorf("SampleCoords %d || %d > %d: %w", idx.Row, idx.Col, edsSize, ErrOutOfBounds) } return idx.Row*edsSize + idx.Col, nil } -func SampleIndexFrom1DIndex(idx, squareSize int) (SampleIndex, error) { +func SampleCoordsFrom1DIndex(idx, squareSize int) (SampleCoords, error) { if int(idx) > squareSize*squareSize { - return SampleIndex{}, fmt.Errorf("SampleIndex %d > %d: %w", idx, squareSize*squareSize, ErrOutOfBounds) + return SampleCoords{}, fmt.Errorf("SampleCoords %d > %d: %w", idx, squareSize*squareSize, ErrOutOfBounds) } rowIdx := int(idx) / squareSize colIdx := int(idx) % squareSize - return SampleIndex{Row: rowIdx, Col: colIdx}, nil + return SampleCoords{Row: rowIdx, Col: colIdx}, nil } // SampleID uniquely identifies a specific sample within a row of an Extended Data Square (EDS). @@ -42,7 +42,7 @@ type SampleID struct { // NewSampleID constructs a new SampleID using the provided block height, sample index, and EDS // size. It calculates the row and share index based on the sample index and EDS size. -func NewSampleID(height uint64, idx SampleIndex, edsSize int) (SampleID, error) { +func NewSampleID(height uint64, idx SampleCoords, edsSize int) (SampleID, error) { sid := SampleID{ RowID: RowID{ EdsID: EdsID{ diff --git a/share/shwap/sample_id_test.go b/share/shwap/sample_id_test.go index 2b492e0742..3df2f6ce56 100644 --- a/share/shwap/sample_id_test.go +++ b/share/shwap/sample_id_test.go @@ -11,7 +11,7 @@ import ( func TestSampleID(t *testing.T) { edsSize := 4 - id, err := NewSampleID(1, SampleIndex{Col: 1}, edsSize) + id, err := NewSampleID(1, SampleCoords{Col: 1}, edsSize) require.NoError(t, err) data, err := id.MarshalBinary() @@ -29,7 +29,7 @@ func TestSampleID(t *testing.T) { func TestSampleIDReaderWriter(t *testing.T) { edsSize := 4 - id, err := NewSampleID(1, SampleIndex{Col: 1}, edsSize) + id, err := NewSampleID(1, SampleCoords{Col: 1}, edsSize) require.NoError(t, err) buf := bytes.NewBuffer(nil) @@ -45,14 +45,14 @@ func TestSampleIDReaderWriter(t *testing.T) { require.EqualValues(t, id, sidOut) } -func TestSampleIndex(t *testing.T) { +func TestSampleCoords(t *testing.T) { edsSize := 16 rawIdx := 13 * 16 - idxIn, err := SampleIndexFrom1DIndex(rawIdx, edsSize) + idxIn, err := SampleCoordsFrom1DIndex(rawIdx, edsSize) require.NoError(t, err) - idxOut, err := SampleIndexAs1DIndex(idxIn, edsSize) + idxOut, err := SampleCoordsAs1DIndex(idxIn, edsSize) require.NoError(t, err) assert.Equal(t, rawIdx, idxOut) } diff --git a/share/shwap/sample_test.go b/share/shwap/sample_test.go index 43e8891d73..0c03eb47cc 100644 --- a/share/shwap/sample_test.go +++ b/share/shwap/sample_test.go @@ -25,7 +25,7 @@ func TestSampleValidate(t *testing.T) { for _, proofType := range []rsmt2d.Axis{rsmt2d.Row, rsmt2d.Col} { for rowIdx := 0; rowIdx < odsSize*2; rowIdx++ { for colIdx := 0; colIdx < odsSize*2; colIdx++ { - idx := shwap.SampleIndex{Row: rowIdx, Col: colIdx} + idx := shwap.SampleCoords{Row: rowIdx, Col: colIdx} sample, err := inMem.SampleForProofAxis(idx, proofType) require.NoError(t, err) @@ -44,7 +44,7 @@ func TestSampleNegativeVerifyInclusion(t *testing.T) { require.NoError(t, err) inMem := eds.Rsmt2D{ExtendedDataSquare: randEDS} - sample, err := inMem.Sample(context.Background(), shwap.SampleIndex{}) + sample, err := inMem.Sample(context.Background(), shwap.SampleCoords{}) require.NoError(t, err) err = sample.Verify(root, 0, 0) require.NoError(t, err) @@ -63,14 +63,14 @@ func TestSampleNegativeVerifyInclusion(t *testing.T) { require.ErrorIs(t, err, shwap.ErrFailedVerification) // incorrect proofType - sample, err = inMem.Sample(context.Background(), shwap.SampleIndex{}) + sample, err = inMem.Sample(context.Background(), shwap.SampleCoords{}) require.NoError(t, err) sample.ProofType = rsmt2d.Col err = sample.Verify(root, 0, 0) require.ErrorIs(t, err, shwap.ErrFailedVerification) // Corrupt the last root hash byte - sample, err = inMem.Sample(context.Background(), shwap.SampleIndex{}) + sample, err = inMem.Sample(context.Background(), shwap.SampleCoords{}) require.NoError(t, err) root.RowRoots[0][len(root.RowRoots[0])-1] ^= 0xFF err = sample.Verify(root, 0, 0) @@ -85,7 +85,7 @@ func TestSampleProtoEncoding(t *testing.T) { for _, proofType := range []rsmt2d.Axis{rsmt2d.Row, rsmt2d.Col} { for rowIdx := 0; rowIdx < odsSize*2; rowIdx++ { for colIdx := 0; colIdx < odsSize*2; colIdx++ { - idx := shwap.SampleIndex{Row: rowIdx, Col: colIdx} + idx := shwap.SampleCoords{Row: rowIdx, Col: colIdx} sample, err := inMem.SampleForProofAxis(idx, proofType) require.NoError(t, err) @@ -108,7 +108,7 @@ func BenchmarkSampleValidate(b *testing.B) { require.NoError(b, err) inMem := eds.Rsmt2D{ExtendedDataSquare: randEDS} - sample, err := inMem.SampleForProofAxis(shwap.SampleIndex{}, rsmt2d.Row) + sample, err := inMem.SampleForProofAxis(shwap.SampleCoords{}, rsmt2d.Row) require.NoError(b, err) b.ResetTimer() diff --git a/store/cache/accessor_cache_test.go b/store/cache/accessor_cache_test.go index 82073a722c..8b537049e1 100644 --- a/store/cache/accessor_cache_test.go +++ b/store/cache/accessor_cache_test.go @@ -315,7 +315,7 @@ func (m *mockAccessor) AxisRoots(context.Context) (*share.AxisRoots, error) { panic("implement me") } -func (m *mockAccessor) Sample(context.Context, shwap.SampleIndex) (shwap.Sample, error) { +func (m *mockAccessor) Sample(context.Context, shwap.SampleCoords) (shwap.Sample, error) { panic("implement me") } diff --git a/store/cache/noop.go b/store/cache/noop.go index 2ccc87f387..27b33e2dd6 100644 --- a/store/cache/noop.go +++ b/store/cache/noop.go @@ -59,7 +59,7 @@ func (n NoopFile) AxisRoots(context.Context) (*share.AxisRoots, error) { return &share.AxisRoots{}, nil } -func (n NoopFile) Sample(context.Context, shwap.SampleIndex) (shwap.Sample, error) { +func (n NoopFile) Sample(context.Context, shwap.SampleCoords) (shwap.Sample, error) { return shwap.Sample{}, nil } diff --git a/store/file/ods.go b/store/file/ods.go index 7edac00c2d..e9b14e8a80 100644 --- a/store/file/ods.go +++ b/store/file/ods.go @@ -228,7 +228,7 @@ func (o *ODS) Close() error { // Sample returns share and corresponding proof for row and column indices. Implementation can // choose which axis to use for proof. Chosen axis for proof should be indicated in the returned // Sample. -func (o *ODS) Sample(ctx context.Context, idx shwap.SampleIndex) (shwap.Sample, error) { +func (o *ODS) Sample(ctx context.Context, idx shwap.SampleCoords) (shwap.Sample, error) { // Sample proof axis is selected to optimize read performance. // - For the first and second quadrants, we read the row axis because it is more efficient to read // single row than reading full ODS to calculate single column @@ -248,7 +248,7 @@ func (o *ODS) Sample(ctx context.Context, idx shwap.SampleIndex) (shwap.Sample, return shwap.Sample{}, fmt.Errorf("reading axis: %w", err) } - idxNew := shwap.SampleIndex{Row: axisIdx, Col: shrIdx} + idxNew := shwap.SampleCoords{Row: axisIdx, Col: shrIdx} return shwap.SampleFromShares(axis, axisType, idxNew) } diff --git a/store/file/ods_q4.go b/store/file/ods_q4.go index a4d6cbd681..f0ca686094 100644 --- a/store/file/ods_q4.go +++ b/store/file/ods_q4.go @@ -122,7 +122,7 @@ func (odsq4 *ODSQ4) AxisRoots(ctx context.Context) (*share.AxisRoots, error) { return odsq4.ods.AxisRoots(ctx) } -func (odsq4 *ODSQ4) Sample(ctx context.Context, idx shwap.SampleIndex) (shwap.Sample, error) { +func (odsq4 *ODSQ4) Sample(ctx context.Context, idx shwap.SampleCoords) (shwap.Sample, error) { // use native AxisHalf implementation, to read axis from q4 quadrant when possible half, err := odsq4.AxisHalf(ctx, rsmt2d.Row, idx.Row) if err != nil { diff --git a/store/getter.go b/store/getter.go index f0515b7fb0..1315561730 100644 --- a/store/getter.go +++ b/store/getter.go @@ -25,7 +25,7 @@ func NewGetter(store *Store) *Getter { } func (g *Getter) GetSamples(ctx context.Context, hdr *header.ExtendedHeader, - indices []shwap.SampleIndex, + indices []shwap.SampleCoords, ) ([]shwap.Sample, error) { acc, err := g.store.GetByHeight(ctx, hdr.Height()) if err != nil { diff --git a/store/getter_test.go b/store/getter_test.go index b6b2f02200..b0b027fc19 100644 --- a/store/getter_test.go +++ b/store/getter_test.go @@ -36,9 +36,9 @@ func TestStoreGetter(t *testing.T) { squareSize := int(eds.Width()) for i := 0; i < squareSize; i++ { for j := 0; j < squareSize; j++ { - idx := shwap.SampleIndex{Row: i, Col: j} + idx := shwap.SampleCoords{Row: i, Col: j} - smpls, err := sg.GetSamples(ctx, eh, []shwap.SampleIndex{idx}) + smpls, err := sg.GetSamples(ctx, eh, []shwap.SampleCoords{idx}) require.NoError(t, err) require.Equal(t, eds.GetCell(uint(i), uint(j)), smpls[0].Share.ToBytes()) } @@ -46,7 +46,7 @@ func TestStoreGetter(t *testing.T) { // doesn't panic on indexes too high bigIdx := squareSize * squareSize - _, err = sg.GetSamples(ctx, eh, []shwap.SampleIndex{{Row: bigIdx, Col: bigIdx}}) + _, err = sg.GetSamples(ctx, eh, []shwap.SampleCoords{{Row: bigIdx, Col: bigIdx}}) require.ErrorIs(t, err, shwap.ErrOutOfBounds) }) From 3821b4394c14b49cb29b7321523dc7ba761f56c2 Mon Sep 17 00:00:00 2001 From: Oleg Kovalov Date: Wed, 20 Nov 2024 14:16:15 +0100 Subject: [PATCH 16/26] do in 1 loop --- share/shwap/p2p/bitswap/getter.go | 25 ++++++++++--------------- 1 file changed, 10 insertions(+), 15 deletions(-) diff --git a/share/shwap/p2p/bitswap/getter.go b/share/shwap/p2p/bitswap/getter.go index 0c7c4bbf30..ea8b15367c 100644 --- a/share/shwap/p2p/bitswap/getter.go +++ b/share/shwap/p2p/bitswap/getter.go @@ -109,17 +109,18 @@ func (g *Getter) GetSamples( defer release() err := Fetch(ctx, g.exchange, hdr.DAH, blks, WithStore(g.bstore), WithFetcher(ses)) - if err != nil { - // handle partial fetches - var fetched int - smpls := make([]shwap.Sample, len(blks)) - for i, blk := range blks { - if smpl := blk.(*SampleBlock).Container; !smpl.IsEmpty() { - smpls[i] = smpl - fetched++ - } + + var fetched int + smpls := make([]shwap.Sample, len(blks)) + for i, blk := range blks { + c := blk.(*SampleBlock).Container + if !c.IsEmpty() { + fetched++ + smpls[i] = c } + } + if err != nil { span.RecordError(err) span.SetStatus(codes.Error, "Fetch") if fetched > 0 { @@ -129,12 +130,6 @@ func (g *Getter) GetSamples( return nil, err } - smpls := make([]shwap.Sample, len(blks)) - for i, blk := range blks { - c := blk.(*SampleBlock).Container - smpls[i] = c - } - span.SetStatus(codes.Ok, "") return smpls, nil } From ef894ae4650c00e65d65fc050d23fa0ce3f2f32c Mon Sep 17 00:00:00 2001 From: Oleg Kovalov Date: Wed, 20 Nov 2024 14:21:13 +0100 Subject: [PATCH 17/26] lint fixes --- blob/service_test.go | 4 +++- share/shwap/sample_id.go | 6 +++--- store/file/codec.go | 10 +++++----- 3 files changed, 11 insertions(+), 9 deletions(-) diff --git a/blob/service_test.go b/blob/service_test.go index 8a01e10e9c..8c55d8824f 100644 --- a/blob/service_test.go +++ b/blob/service_test.go @@ -914,7 +914,9 @@ func createService(ctx context.Context, t testing.TB, shares []libshare.Share) * return nd, err }) shareGetter.EXPECT().GetSamples(gomock.Any(), gomock.Any(), gomock.Any()).AnyTimes(). - DoAndReturn(func(ctx context.Context, h *header.ExtendedHeader, indices []shwap.SampleCoords) ([]shwap.Sample, error) { + DoAndReturn(func(ctx context.Context, h *header.ExtendedHeader, + indices []shwap.SampleCoords, + ) ([]shwap.Sample, error) { smpl, err := accessor.Sample(ctx, indices[0]) return []shwap.Sample{smpl}, err }) diff --git a/share/shwap/sample_id.go b/share/shwap/sample_id.go index f7fac6d697..bc9fbca7d4 100644 --- a/share/shwap/sample_id.go +++ b/share/shwap/sample_id.go @@ -25,12 +25,12 @@ func SampleCoordsAs1DIndex(idx SampleCoords, edsSize int) (int, error) { } func SampleCoordsFrom1DIndex(idx, squareSize int) (SampleCoords, error) { - if int(idx) > squareSize*squareSize { + if idx > squareSize*squareSize { return SampleCoords{}, fmt.Errorf("SampleCoords %d > %d: %w", idx, squareSize*squareSize, ErrOutOfBounds) } - rowIdx := int(idx) / squareSize - colIdx := int(idx) % squareSize + rowIdx := idx / squareSize + colIdx := idx % squareSize return SampleCoords{Row: rowIdx, Col: colIdx}, nil } diff --git a/store/file/codec.go b/store/file/codec.go index a27280be11..f8e7b91ec9 100644 --- a/store/file/codec.go +++ b/store/file/codec.go @@ -13,7 +13,7 @@ func init() { } type Codec interface { - Encoder(len int) (reedsolomon.Encoder, error) + Encoder(ln int) (reedsolomon.Encoder, error) } type codecCache struct { @@ -24,15 +24,15 @@ func NewCodec() Codec { return &codecCache{} } -func (l *codecCache) Encoder(len int) (reedsolomon.Encoder, error) { - enc, ok := l.cache.Load(len) +func (l *codecCache) Encoder(ln int) (reedsolomon.Encoder, error) { + enc, ok := l.cache.Load(ln) if !ok { var err error - enc, err = reedsolomon.New(len/2, len/2, reedsolomon.WithLeopardGF(true)) + enc, err = reedsolomon.New(ln/2, ln/2, reedsolomon.WithLeopardGF(true)) if err != nil { return nil, err } - l.cache.Store(len, enc) + l.cache.Store(ln, enc) } return enc.(reedsolomon.Encoder), nil } From 3d767d39e508b52392656939c1f1c80f2a8202fe Mon Sep 17 00:00:00 2001 From: Oleg Kovalov Date: Thu, 21 Nov 2024 09:53:34 +0100 Subject: [PATCH 18/26] fix ctx --- share/availability/light/availability.go | 21 +++++++++++++++------ 1 file changed, 15 insertions(+), 6 deletions(-) diff --git a/share/availability/light/availability.go b/share/availability/light/availability.go index 287766be4c..e65cc733f3 100644 --- a/share/availability/light/availability.go +++ b/share/availability/light/availability.go @@ -6,6 +6,7 @@ import ( "errors" "fmt" "sync" + "time" "github.com/ipfs/boxo/blockstore" "github.com/ipfs/go-datastore" @@ -115,17 +116,19 @@ func (la *ShareAvailability) SharesAvailable(ctx context.Context, header *header log.Debugw("starting sampling session", "root", dah.String()) + // remove one second from the deadline to ensure we have enough time to process the results + samplingCtx, cancel := context.WithCancel(ctx) + if deadline, ok := ctx.Deadline(); ok { + samplingCtx, cancel = context.WithDeadline(ctx, deadline.Add(-time.Second)) + } + defer cancel() + idxs := make([]shwap.SampleCoords, len(samples.Remaining)) for i, s := range samples.Remaining { idxs[i] = shwap.SampleCoords{Row: s.Row, Col: s.Col} } - smpls, err := la.getter.GetSamples(ctx, header, idxs) - if errors.Is(err, context.Canceled) { - // Availability did not complete due to context cancellation, return context error instead of - // share.ErrNotAvailable - return context.Canceled - } + smpls, err := la.getter.GetSamples(samplingCtx, header, idxs) if len(smpls) == 0 { return share.ErrNotAvailable } @@ -154,6 +157,12 @@ func (la *ShareAvailability) SharesAvailable(ctx context.Context, header *header return fmt.Errorf("store sampling result: %w", err) } + if errors.Is(err, context.Canceled) { + // Availability did not complete due to context cancellation, return context error instead of + // share.ErrNotAvailable + return context.Canceled + } + // if any of the samples failed, return an error if len(failedSamples) > 0 { return share.ErrNotAvailable From f27236cb6f782f38052825c3281d4612c2b4596c Mon Sep 17 00:00:00 2001 From: Oleg Kovalov Date: Thu, 21 Nov 2024 10:04:09 +0100 Subject: [PATCH 19/26] fix err --- share/availability/light/availability.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/share/availability/light/availability.go b/share/availability/light/availability.go index e65cc733f3..f2bc5994e2 100644 --- a/share/availability/light/availability.go +++ b/share/availability/light/availability.go @@ -128,7 +128,7 @@ func (la *ShareAvailability) SharesAvailable(ctx context.Context, header *header idxs[i] = shwap.SampleCoords{Row: s.Row, Col: s.Col} } - smpls, err := la.getter.GetSamples(samplingCtx, header, idxs) + smpls, errGetSamples := la.getter.GetSamples(samplingCtx, header, idxs) if len(smpls) == 0 { return share.ErrNotAvailable } @@ -157,7 +157,7 @@ func (la *ShareAvailability) SharesAvailable(ctx context.Context, header *header return fmt.Errorf("store sampling result: %w", err) } - if errors.Is(err, context.Canceled) { + if errors.Is(errGetSamples, context.Canceled) { // Availability did not complete due to context cancellation, return context error instead of // share.ErrNotAvailable return context.Canceled From 783c4c190caf0f39d282e81c3dcecf5dfe74594b Mon Sep 17 00:00:00 2001 From: Oleg Kovalov Date: Thu, 21 Nov 2024 10:43:00 +0100 Subject: [PATCH 20/26] add context cancel test --- share/availability/light/availability.go | 13 ++-- share/availability/light/availability_test.go | 75 +++++++++++++++++++ 2 files changed, 82 insertions(+), 6 deletions(-) diff --git a/share/availability/light/availability.go b/share/availability/light/availability.go index f2bc5994e2..b0f33435aa 100644 --- a/share/availability/light/availability.go +++ b/share/availability/light/availability.go @@ -116,6 +116,11 @@ func (la *ShareAvailability) SharesAvailable(ctx context.Context, header *header log.Debugw("starting sampling session", "root", dah.String()) + idxs := make([]shwap.SampleCoords, len(samples.Remaining)) + for i, s := range samples.Remaining { + idxs[i] = shwap.SampleCoords{Row: s.Row, Col: s.Col} + } + // remove one second from the deadline to ensure we have enough time to process the results samplingCtx, cancel := context.WithCancel(ctx) if deadline, ok := ctx.Deadline(); ok { @@ -123,11 +128,6 @@ func (la *ShareAvailability) SharesAvailable(ctx context.Context, header *header } defer cancel() - idxs := make([]shwap.SampleCoords, len(samples.Remaining)) - for i, s := range samples.Remaining { - idxs[i] = shwap.SampleCoords{Row: s.Row, Col: s.Col} - } - smpls, errGetSamples := la.getter.GetSamples(samplingCtx, header, idxs) if len(smpls) == 0 { return share.ErrNotAvailable @@ -157,7 +157,8 @@ func (la *ShareAvailability) SharesAvailable(ctx context.Context, header *header return fmt.Errorf("store sampling result: %w", err) } - if errors.Is(errGetSamples, context.Canceled) { + if errors.Is(errGetSamples, context.Canceled) || + errors.Is(errGetSamples, context.DeadlineExceeded) { // Availability did not complete due to context cancellation, return context error instead of // share.ErrNotAvailable return context.Canceled diff --git a/share/availability/light/availability_test.go b/share/availability/light/availability_test.go index dce5021abb..16c2b0efda 100644 --- a/share/availability/light/availability_test.go +++ b/share/availability/light/availability_test.go @@ -384,6 +384,49 @@ func TestPrunePartialFailed(t *testing.T) { require.False(t, exist) } +func TestPruneWithCancelledContext(t *testing.T) { + const size = 8 + ctx, cancel := context.WithTimeout(context.Background(), time.Second*2) + t.Cleanup(cancel) + + eds, h := randEdsAndHeader(t, size) + ds := ds_sync.MutexWrap(datastore.NewMapDatastore()) + clientBs := blockstore.NewBlockstore(ds) + + ex := newTimeoutExchange(newExchangeOverEDS(ctx, t, eds)) + getter := bitswap.NewGetter(ex, clientBs, 0) + getter.Start() + defer getter.Stop() + + // Create a new ShareAvailability instance and sample the shares + sampleAmount := uint(20) + avail := NewShareAvailability(getter, ds, clientBs, WithSampleAmount(sampleAmount)) + + ctx2, cancel2 := context.WithTimeout(ctx, 1500*time.Millisecond) + defer cancel2() + + err := avail.SharesAvailable(ctx2, h) + require.Error(t, err, context.Canceled) + // close ShareAvailability to force flush of batched writes + avail.Close(ctx) + + preDeleteCount := countKeys(ctx, t, clientBs) + require.EqualValues(t, sampleAmount, preDeleteCount) + + // prune the samples + err = avail.Prune(ctx, h) + require.NoError(t, err) + + // Check if samples are deleted + postDeleteCount := countKeys(ctx, t, clientBs) + require.Zero(t, postDeleteCount) + + // Check if sampling result is deleted + exist, err := avail.ds.Has(ctx, datastoreKeyForRoot(h.DAH)) + require.NoError(t, err) + require.False(t, exist) +} + type halfSessionExchange struct { exchange.SessionExchange attempt atomic.Int32 @@ -417,6 +460,38 @@ func (hse *halfSessionExchange) GetBlocks(ctx context.Context, cids []cid.Cid) ( return out, nil } +type timeoutExchange struct { + exchange.SessionExchange +} + +func newTimeoutExchange(ex exchange.SessionExchange) *timeoutExchange { + return &timeoutExchange{SessionExchange: ex} +} + +func (hse *timeoutExchange) NewSession(context.Context) exchange.Fetcher { + return hse +} + +func (hse *timeoutExchange) GetBlocks(ctx context.Context, cids []cid.Cid) (<-chan blocks.Block, error) { + out := make(chan blocks.Block, len(cids)) + defer close(out) + + for _, cid := range cids { + + blk, err := hse.SessionExchange.GetBlock(ctx, cid) + if err != nil { + return nil, err + } + + out <- blk + } + + // sleep 1 second guarantees that we will exhaust context. + time.Sleep(time.Second) + + return out, nil +} + func randEdsAndHeader(t *testing.T, size int) (*rsmt2d.ExtendedDataSquare, *header.ExtendedHeader) { height := uint64(42) eds := edstest.RandEDS(t, size) From 77c0d23a160fb017644b173c8a30e435ec5634ba Mon Sep 17 00:00:00 2001 From: Oleg Kovalov Date: Thu, 21 Nov 2024 11:49:24 +0100 Subject: [PATCH 21/26] fix lint --- share/availability/light/availability_test.go | 1 - 1 file changed, 1 deletion(-) diff --git a/share/availability/light/availability_test.go b/share/availability/light/availability_test.go index 16c2b0efda..df501f1d60 100644 --- a/share/availability/light/availability_test.go +++ b/share/availability/light/availability_test.go @@ -477,7 +477,6 @@ func (hse *timeoutExchange) GetBlocks(ctx context.Context, cids []cid.Cid) (<-ch defer close(out) for _, cid := range cids { - blk, err := hse.SessionExchange.GetBlock(ctx, cid) if err != nil { return nil, err From 1e81a30eaa842c3b6aead80fc6e44a45beefbf28 Mon Sep 17 00:00:00 2001 From: Oleg Kovalov Date: Tue, 26 Nov 2024 12:43:44 +0100 Subject: [PATCH 22/26] review comemnt --- share/availability/light/availability.go | 3 +-- share/availability/light/availability_test.go | 11 ++++++++--- share/shwap/sample_id.go | 3 ++- 3 files changed, 11 insertions(+), 6 deletions(-) diff --git a/share/availability/light/availability.go b/share/availability/light/availability.go index b0f33435aa..ac17220375 100644 --- a/share/availability/light/availability.go +++ b/share/availability/light/availability.go @@ -157,8 +157,7 @@ func (la *ShareAvailability) SharesAvailable(ctx context.Context, header *header return fmt.Errorf("store sampling result: %w", err) } - if errors.Is(errGetSamples, context.Canceled) || - errors.Is(errGetSamples, context.DeadlineExceeded) { + if errors.Is(errGetSamples, context.Canceled) { // Availability did not complete due to context cancellation, return context error instead of // share.ErrNotAvailable return context.Canceled diff --git a/share/availability/light/availability_test.go b/share/availability/light/availability_test.go index df501f1d60..e27f726577 100644 --- a/share/availability/light/availability_test.go +++ b/share/availability/light/availability_test.go @@ -246,7 +246,7 @@ func TestParallelAvailability(t *testing.T) { }() } wg.Wait() - require.Equal(t, len(successfulGetter.sampledList()), int(avail.params.SampleAmount)) + require.Len(t, successfulGetter.sampledList(), int(avail.params.SampleAmount)) // Verify that the sampling result is stored with all samples marked as available resultData, err := avail.ds.Get(ctx, datastoreKeyForRoot(roots)) @@ -404,6 +404,11 @@ func TestPruneWithCancelledContext(t *testing.T) { ctx2, cancel2 := context.WithTimeout(ctx, 1500*time.Millisecond) defer cancel2() + go func() { + // cancel context a bit later. + time.Sleep(100 * time.Millisecond) + cancel2() + }() err := avail.SharesAvailable(ctx2, h) require.Error(t, err, context.Canceled) @@ -485,8 +490,8 @@ func (hse *timeoutExchange) GetBlocks(ctx context.Context, cids []cid.Cid) (<-ch out <- blk } - // sleep 1 second guarantees that we will exhaust context. - time.Sleep(time.Second) + // sleep guarantees that we context will be cancelled in a test. + time.Sleep(200 * time.Millisecond) return out, nil } diff --git a/share/shwap/sample_id.go b/share/shwap/sample_id.go index bc9fbca7d4..a3b13279fd 100644 --- a/share/shwap/sample_id.go +++ b/share/shwap/sample_id.go @@ -11,7 +11,8 @@ import ( const SampleIDSize = RowIDSize + 2 type SampleCoords struct { - Row, Col int + Row int `json:"row"` + Col int `json:"col"` } func SampleCoordsAs1DIndex(idx SampleCoords, edsSize int) (int, error) { From a6e3c86362a97039f19000d2f6718349b3ece15d Mon Sep 17 00:00:00 2001 From: Oleg Kovalov Date: Tue, 26 Nov 2024 12:54:46 +0100 Subject: [PATCH 23/26] lint --- share/availability/light/availability_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/share/availability/light/availability_test.go b/share/availability/light/availability_test.go index e27f726577..d168b047fe 100644 --- a/share/availability/light/availability_test.go +++ b/share/availability/light/availability_test.go @@ -490,7 +490,7 @@ func (hse *timeoutExchange) GetBlocks(ctx context.Context, cids []cid.Cid) (<-ch out <- blk } - // sleep guarantees that we context will be cancelled in a test. + // sleep guarantees that we context will be canceled in a test. time.Sleep(200 * time.Millisecond) return out, nil From 0a91bf8abd330aa33ed799691d3661bdce1e4e9f Mon Sep 17 00:00:00 2001 From: Oleg Kovalov Date: Wed, 27 Nov 2024 13:36:53 +0100 Subject: [PATCH 24/26] drop Sample, use shwap.SampleCoords --- share/availability/light/availability.go | 6 ++--- share/availability/light/availability_test.go | 12 +++++----- share/availability/light/sample.go | 22 ++++++++----------- 3 files changed, 18 insertions(+), 22 deletions(-) diff --git a/share/availability/light/availability.go b/share/availability/light/availability.go index ac17220375..b33825fe4b 100644 --- a/share/availability/light/availability.go +++ b/share/availability/light/availability.go @@ -133,13 +133,13 @@ func (la *ShareAvailability) SharesAvailable(ctx context.Context, header *header return share.ErrNotAvailable } - var failedSamples []Sample + var failedSamples []shwap.SampleCoords for i, smpl := range smpls { if smpl.IsEmpty() { - failedSamples = append(failedSamples, Sample{Row: idxs[i].Row, Col: idxs[i].Col}) + failedSamples = append(failedSamples, shwap.SampleCoords{Row: idxs[i].Row, Col: idxs[i].Col}) } else { - samples.Available = append(samples.Available, Sample{Row: idxs[i].Row, Col: idxs[i].Col}) + samples.Available = append(samples.Available, shwap.SampleCoords{Row: idxs[i].Row, Col: idxs[i].Col}) } } diff --git a/share/availability/light/availability_test.go b/share/availability/light/availability_test.go index d168b047fe..0b2f7c4de9 100644 --- a/share/availability/light/availability_test.go +++ b/share/availability/light/availability_test.go @@ -113,8 +113,8 @@ func TestSharesAvailableSkipSampled(t *testing.T) { // Store a successful sampling result in the datastore samplingResult := &SamplingResult{ - Available: make([]Sample, avail.params.SampleAmount), - Remaining: []Sample{}, + Available: make([]shwap.SampleCoords, avail.params.SampleAmount), + Remaining: []shwap.SampleCoords{}, } data, err := json.Marshal(samplingResult) require.NoError(t, err) @@ -262,17 +262,17 @@ func TestParallelAvailability(t *testing.T) { type successGetter struct { *sync.Mutex - sampled map[Sample]int + sampled map[shwap.SampleCoords]int } func newSuccessGetter() successGetter { return successGetter{ Mutex: &sync.Mutex{}, - sampled: make(map[Sample]int), + sampled: make(map[shwap.SampleCoords]int), } } -func (g successGetter) sampledList() []Sample { +func (g successGetter) sampledList() []shwap.SampleCoords { g.Lock() defer g.Unlock() return slices.Collect(maps.Keys(g.sampled)) @@ -286,7 +286,7 @@ func (g successGetter) GetSamples(_ context.Context, hdr *header.ExtendedHeader, smpls := make([]shwap.Sample, 0, len(indices)) for _, idx := range indices { - s := Sample{Row: idx.Row, Col: idx.Col} + s := shwap.SampleCoords{Row: idx.Row, Col: idx.Col} g.sampled[s]++ smpls = append(smpls, shwap.Sample{Proof: &nmt.Proof{}}) } diff --git a/share/availability/light/sample.go b/share/availability/light/sample.go index 0641e11a99..fc0b41d08a 100644 --- a/share/availability/light/sample.go +++ b/share/availability/light/sample.go @@ -2,21 +2,17 @@ package light import ( crand "crypto/rand" + "maps" "math/big" + "slices" - "golang.org/x/exp/maps" + "github.com/celestiaorg/celestia-node/share/shwap" ) // SamplingResult holds the available and remaining samples. type SamplingResult struct { - Available []Sample `json:"available"` - Remaining []Sample `json:"remaining"` -} - -// Sample represents a coordinate in a 2D data square. -type Sample struct { - Row int `json:"row"` - Col int `json:"col"` + Available []shwap.SampleCoords `json:"available"` + Remaining []shwap.SampleCoords `json:"remaining"` } // NewSamplingResult creates a new SamplingResult with randomly selected samples. @@ -33,21 +29,21 @@ func NewSamplingResult(squareSize, sampleCount int) *SamplingResult { } // selectRandomSamples randomly picks unique coordinates from a square of given size. -func selectRandomSamples(squareSize, sampleCount int) []Sample { +func selectRandomSamples(squareSize, sampleCount int) []shwap.SampleCoords { total := squareSize * squareSize if sampleCount > total { sampleCount = total } - samples := make(map[Sample]struct{}, sampleCount) + samples := make(map[shwap.SampleCoords]struct{}, sampleCount) for len(samples) < sampleCount { - s := Sample{ + s := shwap.SampleCoords{ Row: randInt(squareSize), Col: randInt(squareSize), } samples[s] = struct{}{} } - return maps.Keys(samples) + return slices.Collect(maps.Keys(samples)) } func randInt(m int) int { From b4c424d9907f7abd64ec012eee0262de4d86e9c4 Mon Sep 17 00:00:00 2001 From: Oleg Kovalov Date: Wed, 27 Nov 2024 13:55:23 +0100 Subject: [PATCH 25/26] add checkOnce back --- share/availability/light/availability_test.go | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/share/availability/light/availability_test.go b/share/availability/light/availability_test.go index 0b2f7c4de9..b1a9299bea 100644 --- a/share/availability/light/availability_test.go +++ b/share/availability/light/availability_test.go @@ -217,6 +217,7 @@ func TestSharesAvailableFailed(t *testing.T) { // onceGetter should have no more samples stored after the call require.ElementsMatch(t, failed.Remaining, successfulGetter.sampledList()) + successfulGetter.checkOnce(t) } } @@ -247,6 +248,7 @@ func TestParallelAvailability(t *testing.T) { } wg.Wait() require.Len(t, successfulGetter.sampledList(), int(avail.params.SampleAmount)) + successfulGetter.checkOnce(t) // Verify that the sampling result is stored with all samples marked as available resultData, err := avail.ds.Get(ctx, datastoreKeyForRoot(roots)) @@ -278,6 +280,16 @@ func (g successGetter) sampledList() []shwap.SampleCoords { return slices.Collect(maps.Keys(g.sampled)) } +func (g successGetter) checkOnce(t *testing.T) { + g.Lock() + defer g.Unlock() + for s, count := range g.sampled { + if count > 1 { + t.Errorf("sample %v was called more than once", s) + } + } +} + func (g successGetter) GetSamples(_ context.Context, hdr *header.ExtendedHeader, indices []shwap.SampleCoords, ) ([]shwap.Sample, error) { From aed2ec618a0d56a411d3cfe7e4b95f356083b285 Mon Sep 17 00:00:00 2001 From: Oleg Kovalov Date: Wed, 27 Nov 2024 14:00:37 +0100 Subject: [PATCH 26/26] more review suggestions --- share/shwap/getter.go | 2 ++ share/shwap/p2p/bitswap/getter.go | 2 +- 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/share/shwap/getter.go b/share/shwap/getter.go index 2d52397e4c..9e0a5d3131 100644 --- a/share/shwap/getter.go +++ b/share/shwap/getter.go @@ -22,6 +22,8 @@ var ( // ErrOutOfBounds is used to indicate that a passed row or column index is out of bounds of the // square size. ErrOutOfBounds = fmt.Errorf("index out of bounds: %w", ErrInvalidID) + // ErrNoSampleIndicies is used to indicate that no indicies where given to process. + ErrNoSampleIndicies = errors.New("no sample indicies to fetch") ) // Getter interface provides a set of accessors for shares by the Root. diff --git a/share/shwap/p2p/bitswap/getter.go b/share/shwap/p2p/bitswap/getter.go index ea8b15367c..db2938663c 100644 --- a/share/shwap/p2p/bitswap/getter.go +++ b/share/shwap/p2p/bitswap/getter.go @@ -82,7 +82,7 @@ func (g *Getter) GetSamples( indices []shwap.SampleCoords, ) ([]shwap.Sample, error) { if len(indices) == 0 { - return nil, fmt.Errorf("no sample indicies to fetch") + return nil, shwap.ErrNoSampleIndicies } ctx, span := tracer.Start(ctx, "get-samples", trace.WithAttributes(