From c1d1f5fa529f3f9bc0e96053ec6e610fcd918752 Mon Sep 17 00:00:00 2001 From: Ljubisa Gacevic Date: Mon, 14 Sep 2026 11:57:55 +0200 Subject: [PATCH 1/7] test(storer): guard sample chunk data and add read-path benchmarks Sampling is about to start reading chunks into a buffer that each worker reuses. Nothing today checks that the bytes handed back in a SampleItem are still the bytes of that chunk, so a reused buffer would silently hand the redistribution proof the contents of some later chunk. assertValidSample now checks two things for every item: that ChunkData still reproduces ChunkAddress, and that no two items share a backing array. Every existing sample test picks both up. Also add the rulers for the work that follows. BenchmarkReserveSample1k keeps its name and behaviour so the recorded baseline stays comparable; its body moves to a helper that BenchmarkReserveSample10k reuses over a ten times larger reserve. BenchmarkChunkStoreGet measures a single chunk read, split into a variant that builds the ChunkStore handle per call as the sampler does today and one that hoists it, so the cost of the handle alone is visible. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01KFUseFQ8rhp6N9X6YS7pbq --- .../internal/chunkstore/chunkstore_test.go | 67 +++++++++++++++++-- pkg/storer/sample_test.go | 64 +++++++++++++++++- 2 files changed, 123 insertions(+), 8 deletions(-) diff --git a/pkg/storer/internal/chunkstore/chunkstore_test.go b/pkg/storer/internal/chunkstore/chunkstore_test.go index 970e92df9b8..00047a16881 100644 --- a/pkg/storer/internal/chunkstore/chunkstore_test.go +++ b/pkg/storer/internal/chunkstore/chunkstore_test.go @@ -497,17 +497,72 @@ type chunkStore struct { sharky *sharky.Store } -func makeStorage(t *testing.T) *chunkStore { - t.Helper() +func makeStorage(tb testing.TB) *chunkStore { + tb.Helper() store := inmemstore.New() sharky, err := sharky.New(&memFS{Fs: afero.NewMemMapFs()}, 1, swarm.SocMaxChunkSize) - assert.NoError(t, err) + assert.NoError(tb, err) - t.Cleanup(func() { - assert.NoError(t, store.Close()) - assert.NoError(t, sharky.Close()) + tb.Cleanup(func() { + assert.NoError(tb, store.Close()) + assert.NoError(tb, sharky.Close()) }) return &chunkStore{transaction.NewStorage(sharky, store), sharky} } + +// BenchmarkChunkStoreGet measures a single chunk read: one retrieval-index +// lookup followed by one sharky read. It is the micro-benchmark for the read +// path that the reserve sampler drives once per chunk. +// +// The two variants differ only in where the ChunkStore handle comes from. +// "per_call" mirrors what the sampler does today, building a fresh handle for +// every chunk; "hoisted" builds it once. The gap between them is the cost of +// that handle alone. +func BenchmarkChunkStoreGet(b *testing.B) { + ctx := context.Background() + + setup := func(b *testing.B) (*chunkStore, swarm.Address) { + b.Helper() + + st := makeStorage(b) + ch := chunktest.GenerateTestRandomChunk() + + err := st.Run(ctx, func(s transaction.Store) error { + return s.ChunkStore().Put(ctx, ch) + }) + if err != nil { + b.Fatal(err) + } + + return st, ch.Address() + } + + b.Run("per_call", func(b *testing.B) { + st, addr := setup(b) + + b.ReportAllocs() + b.ResetTimer() + + for b.Loop() { + if _, err := st.ChunkStore().Get(ctx, addr); err != nil { + b.Fatal(err) + } + } + }) + + b.Run("hoisted", func(b *testing.B) { + st, addr := setup(b) + cs := st.ChunkStore() + + b.ReportAllocs() + b.ResetTimer() + + for b.Loop() { + if _, err := cs.Get(ctx, addr); err != nil { + b.Fatal(err) + } + } + }) +} diff --git a/pkg/storer/sample_test.go b/pkg/storer/sample_test.go index 16b65d81a09..f9976061d23 100644 --- a/pkg/storer/sample_test.go +++ b/pkg/storer/sample_test.go @@ -14,6 +14,7 @@ import ( "github.com/ethersphere/bee/v2/pkg/bmt" "github.com/ethersphere/bee/v2/pkg/cac" "github.com/ethersphere/bee/v2/pkg/postage" + "github.com/ethersphere/bee/v2/pkg/soc" postagetesting "github.com/ethersphere/bee/v2/pkg/postage/testing" chunk "github.com/ethersphere/bee/v2/pkg/storage/testing" @@ -302,6 +303,9 @@ func assertValidSample(t *testing.T, sample storer.Sample, minRadius uint8, anch assertSampleItem(item, i) } + assertSampleDataIntact(t, sample) + assertSampleDataNotShared(t, sample) + // Assert that transformed addresses are in ascending order for i := 0; i < len(sample.Items)-1; i++ { if sample.Items[i].TransformedAddress.Compare(sample.Items[i+1].TransformedAddress) != -1 { @@ -310,6 +314,44 @@ func assertValidSample(t *testing.T, sample storer.Sample, minRadius uint8, anch } } +// assertSampleDataIntact checks that every item's ChunkData still reproduces its +// ChunkAddress. +// +// The sampler currently hands out the buffer that the chunk store allocated for +// each chunk, so the data is trivially intact. That stops being true the moment +// chunks are read into a buffer that the worker reuses: unless the bytes handed +// to a SampleItem are copied out, every item except the last one a worker +// touched carries the contents of some later chunk instead. +func assertSampleDataIntact(t *testing.T, sample storer.Sample) { + t.Helper() + + for i, item := range sample.Items { + ch := swarm.NewChunk(item.ChunkAddress, item.ChunkData) + if !cac.Valid(ch) && !soc.Valid(ch) { + t.Fatalf("sample item [%d]: chunk data does not reproduce address %s", i, item.ChunkAddress) + } + } +} + +// assertSampleDataNotShared checks that no two items are backed by the same +// array. It catches a reused read buffer even in the case where the surviving +// contents happen to stay valid for one of the aliased items. +func assertSampleDataNotShared(t *testing.T, sample storer.Sample) { + t.Helper() + + seen := make(map[*byte]int, len(sample.Items)) + for i, item := range sample.Items { + if len(item.ChunkData) == 0 { + continue + } + first := &item.ChunkData[0] + if j, ok := seen[first]; ok { + t.Fatalf("sample items [%d] and [%d] share one backing array", j, i) + } + seen[first] = i + } +} + // TestSampleVectorCAC is a deterministic test vector that verifies the chunk // address and transformed address produced by MakeSampleUsingChunks for a // single hardcoded CAC chunk and anchor. It guards against regressions in the @@ -420,11 +462,27 @@ func assertSampleNoErrors(t *testing.T, sample storer.Sample) { // method, including DB iteration, chunk loading, stamp validation, and sample // assembly. func BenchmarkReserveSample1k(b *testing.B) { - const chunkCountPerPO = 100 + benchmarkReserveSample(b, 100) +} + +// BenchmarkReserveSample10k is BenchmarkReserveSample1k over a reserve ten +// times the size. The per-chunk costs of the sampler are linear in the number +// of chunks iterated, so a change that only removes a fixed overhead reads the +// same at both sizes while a change to a per-chunk allocation does not. The +// larger reserve is also where garbage collection starts to show. +func BenchmarkReserveSample10k(b *testing.B) { + benchmarkReserveSample(b, 1000) +} + +// benchmarkReserveSample fills a reserve with chunkCountPerPO chunks in each of +// the first maxPO proximity orders and then samples it repeatedly. +func benchmarkReserveSample(b *testing.B, chunkCountPerPO int) { + b.Helper() + const maxPO = 10 baseAddr := swarm.RandAddress(b) - opts := dbTestOps(baseAddr, 5000, nil, nil, time.Second) + opts := dbTestOps(baseAddr, 5*chunkCountPerPO*maxPO, nil, nil, time.Second) opts.ValidStamp = func(ch swarm.Chunk) (swarm.Chunk, error) { return ch, nil } st, err := diskStorer(b, opts)() @@ -450,6 +508,8 @@ func BenchmarkReserveSample1k(b *testing.B) { anchor = swarm.RandAddressAt(b, baseAddr, int(radius)).Bytes() ) + b.ResetTimer() + for b.Loop() { _, err := st.ReserveSample(context.TODO(), anchor, radius, timeVar, nil) if err != nil { From abc65aca5674b296fb2d0c93b5cea066fe2ed4d4 Mon Sep 17 00:00:00 2001 From: Ljubisa Gacevic Date: Mon, 14 Sep 2026 11:58:59 +0200 Subject: [PATCH 2/7] perf(storer): build the sampler's chunk store handle once per worker db.ChunkStore() builds three objects every time it is called, and the sampler called it once per chunk. On a testnet node that is 2.3 million handles per round, thrown away immediately. Hoist it to one per worker. The handle is deliberately not shared across workers: the read-only chunk store makes no thread-safety promise, and three allocations per worker is already nothing next to three per chunk. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01KFUseFQ8rhp6N9X6YS7pbq --- pkg/storer/sample.go | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/pkg/storer/sample.go b/pkg/storer/sample.go index f4b6f8139e3..de5bb86c8b4 100644 --- a/pkg/storer/sample.go +++ b/pkg/storer/sample.go @@ -127,6 +127,11 @@ func (db *DB) ReserveSample( g.Go(safe.RunFunc(db.logger, "storer-sample-worker", func() error { wstat := SampleStats{} hasher := bmt.NewPrefixHasher(anchor) + // One handle per worker rather than one per chunk: building it + // allocates, and the sampler asks for a chunk millions of times per + // round. It is not shared between workers because the read-only + // chunk store makes no thread-safety promise. + chunkStore := db.ChunkStore() defer func() { addStats(wstat) }() @@ -147,7 +152,7 @@ func (db *DB) ReserveSample( chunkLoadStart := time.Now() - chunk, err := db.ChunkStore().Get(ctx, chItem.Address) + chunk, err := chunkStore.Get(ctx, chItem.Address) chunkLoadDuration := time.Since(chunkLoadStart) if err != nil { From 16824419f59d9479884307d18ff8413984cab286 Mon Sep 17 00:00:00 2001 From: Ljubisa Gacevic Date: Mon, 14 Sep 2026 13:19:17 +0200 Subject: [PATCH 3/7] perf(storer): read into reusable worker buffer and load sample items lazily --- pkg/file/joiner/joiner_test.go | 13 ++++ pkg/storage/chunkstore.go | 9 +++ .../inmemchunkstore/inmemchunkstore.go | 17 ++++++ pkg/storer/internal/chunkstore/chunkstore.go | 22 +++++++ .../internal/chunkstore/chunkstore_test.go | 18 +++++- .../internal/transaction/transaction.go | 7 +++ pkg/storer/sample.go | 61 +++++++++++-------- pkg/storer/sample_test.go | 3 +- 8 files changed, 122 insertions(+), 28 deletions(-) diff --git a/pkg/file/joiner/joiner_test.go b/pkg/file/joiner/joiner_test.go index dd1a7e4228e..ad7c40b8819 100644 --- a/pkg/file/joiner/joiner_test.go +++ b/pkg/file/joiner/joiner_test.go @@ -1396,6 +1396,19 @@ func (c *chunkStore) Get(_ context.Context, addr swarm.Address) (swarm.Chunk, er return chunk, nil } +func (c *chunkStore) GetInto(ctx context.Context, addr swarm.Address, buf []byte) (int, error) { + ch, err := c.Get(ctx, addr) + if err != nil { + return 0, err + } + data := ch.Data() + if len(buf) < len(data) { + return 0, fmt.Errorf("chunk store: buffer too small: %d < %d", len(buf), len(data)) + } + copy(buf, data) + return len(data), nil +} + func (c *chunkStore) Put(_ context.Context, ch swarm.Chunk) error { c.mu.Lock() defer c.mu.Unlock() diff --git a/pkg/storage/chunkstore.go b/pkg/storage/chunkstore.go index 68a9d10652a..2809493e129 100644 --- a/pkg/storage/chunkstore.go +++ b/pkg/storage/chunkstore.go @@ -21,6 +21,13 @@ type Getter interface { Get(context.Context, swarm.Address) (swarm.Chunk, error) } +// GetterInto is like Getter but reads chunk data into a caller-provided buffer, +// avoiding per-call allocations. The buffer must be large enough to hold the chunk. +// Returns the number of bytes read into buf. +type GetterInto interface { + GetInto(ctx context.Context, addr swarm.Address, buf []byte) (int, error) +} + // Putter is the interface that wraps the basic Put method. type Putter interface { // Put a chunk into the store alongside with its postage stamp. @@ -73,6 +80,7 @@ type ChunkGetterDeleter interface { type ChunkStore interface { Getter + GetterInto Putter Deleter Hasser @@ -84,5 +92,6 @@ type ChunkStore interface { type ReadOnlyChunkStore interface { Getter + GetterInto Hasser } diff --git a/pkg/storage/inmemchunkstore/inmemchunkstore.go b/pkg/storage/inmemchunkstore/inmemchunkstore.go index 4f0465016af..e775a6bc884 100644 --- a/pkg/storage/inmemchunkstore/inmemchunkstore.go +++ b/pkg/storage/inmemchunkstore/inmemchunkstore.go @@ -6,6 +6,7 @@ package inmemchunkstore import ( "context" + "fmt" "sync" "github.com/ethersphere/bee/v2/pkg/storage" @@ -39,6 +40,22 @@ func (c *ChunkStore) Get(_ context.Context, addr swarm.Address) (swarm.Chunk, er return chunk.chunk, nil } +func (c *ChunkStore) GetInto(_ context.Context, addr swarm.Address, buf []byte) (int, error) { + c.mu.Lock() + defer c.mu.Unlock() + + chunk, ok := c.chunks[c.key(addr)] + if !ok { + return 0, storage.ErrNotFound + } + data := chunk.chunk.Data() + if len(buf) < len(data) { + return 0, fmt.Errorf("chunk store: buffer too small: %d < %d", len(buf), len(data)) + } + copy(buf, data) + return len(data), nil +} + func (c *ChunkStore) Put(_ context.Context, ch swarm.Chunk) error { c.mu.Lock() defer c.mu.Unlock() diff --git a/pkg/storer/internal/chunkstore/chunkstore.go b/pkg/storer/internal/chunkstore/chunkstore.go index 6d2745cc5fd..0b70134f38b 100644 --- a/pkg/storer/internal/chunkstore/chunkstore.go +++ b/pkg/storer/internal/chunkstore/chunkstore.go @@ -51,6 +51,28 @@ func Get(ctx context.Context, r storage.Reader, s storage.Sharky, addr swarm.Add return readChunk(ctx, s, rIdx) } +// GetInto reads chunk data into the provided buffer, avoiding per-call allocation. +// Returns the number of bytes read. +func GetInto(ctx context.Context, r storage.Reader, s storage.Sharky, addr swarm.Address, buf []byte) (int, error) { + rIdx := &RetrievalIndexItem{Address: addr} + err := r.Get(rIdx) + if err != nil { + return 0, fmt.Errorf("chunk store: failed reading retrievalIndex for address %s: %w", addr, err) + } + n := int(rIdx.Location.Length) + if len(buf) < n { + return 0, fmt.Errorf("chunk store: buffer too small: %d < %d", len(buf), n) + } + err = s.Read(ctx, rIdx.Location, buf[:n]) + if err != nil { + return 0, fmt.Errorf( + "chunk store: failed reading location: %v for chunk %s from sharky: %w", + rIdx.Location, rIdx.Address, err, + ) + } + return n, nil +} + // helper to read chunk from retrievalIndex. func readChunk(ctx context.Context, s storage.Sharky, rIdx *RetrievalIndexItem) (swarm.Chunk, error) { buf := make([]byte, rIdx.Location.Length) diff --git a/pkg/storer/internal/chunkstore/chunkstore_test.go b/pkg/storer/internal/chunkstore/chunkstore_test.go index 00047a16881..d07728d321d 100644 --- a/pkg/storer/internal/chunkstore/chunkstore_test.go +++ b/pkg/storer/internal/chunkstore/chunkstore_test.go @@ -17,13 +17,12 @@ import ( "github.com/ethersphere/bee/v2/pkg/crypto" "github.com/ethersphere/bee/v2/pkg/sharky" soctesting "github.com/ethersphere/bee/v2/pkg/soc/testing" - "github.com/ethersphere/bee/v2/pkg/storer/internal/transaction" - "github.com/ethersphere/bee/v2/pkg/storage" "github.com/ethersphere/bee/v2/pkg/storage/inmemstore" "github.com/ethersphere/bee/v2/pkg/storage/storagetest" chunktest "github.com/ethersphere/bee/v2/pkg/storage/testing" "github.com/ethersphere/bee/v2/pkg/storer/internal/chunkstore" + "github.com/ethersphere/bee/v2/pkg/storer/internal/transaction" "github.com/ethersphere/bee/v2/pkg/swarm" "github.com/spf13/afero" "github.com/stretchr/testify/assert" @@ -565,4 +564,19 @@ func BenchmarkChunkStoreGet(b *testing.B) { } } }) + + b.Run("get_into", func(b *testing.B) { + st, addr := setup(b) + cs := st.ChunkStore() + buf := make([]byte, swarm.SocMaxChunkSize) + + b.ReportAllocs() + b.ResetTimer() + + for b.Loop() { + if _, err := cs.GetInto(ctx, addr, buf); err != nil { + b.Fatal(err) + } + } + }) } diff --git a/pkg/storer/internal/transaction/transaction.go b/pkg/storer/internal/transaction/transaction.go index 0f403580152..2aa1072e624 100644 --- a/pkg/storer/internal/transaction/transaction.go +++ b/pkg/storer/internal/transaction/transaction.go @@ -225,6 +225,13 @@ func (c *chunkStoreTrx) Get(ctx context.Context, addr swarm.Address) (ch swarm.C return ch, err } +func (c *chunkStoreTrx) GetInto(ctx context.Context, addr swarm.Address, buf []byte) (n int, err error) { + defer handleMetric("chunkstore_get", c.metrics)(&err) + unlock := c.lock(addr) + defer unlock() + return chunkstore.GetInto(ctx, c.indexStore, c.sharkyTrx, addr, buf) +} + func (c *chunkStoreTrx) Has(ctx context.Context, addr swarm.Address) (_ bool, err error) { defer handleMetric("chunkstore_has", c.metrics)(&err) unlock := c.lock(addr) diff --git a/pkg/storer/sample.go b/pkg/storer/sample.go index de5bb86c8b4..19e680b3d6d 100644 --- a/pkg/storer/sample.go +++ b/pkg/storer/sample.go @@ -8,6 +8,7 @@ import ( "bytes" "context" "encoding/binary" + "errors" "fmt" "math/big" "runtime" @@ -65,7 +66,7 @@ func (db *DB) ReserveSample( consensusTime uint64, minBatchBalance *big.Int, ) (Sample, error) { - g, ctx := errgroup.WithContext(ctx) + g, gCtx := errgroup.WithContext(ctx) allStats := &SampleStats{} statsLock := sync.Mutex{} @@ -111,8 +112,8 @@ func (db *DB) ReserveSample( case chunkC <- ch: stats.TotalIterated++ return false, nil - case <-ctx.Done(): - return false, ctx.Err() + case <-gCtx.Done(): + return false, gCtx.Err() } }) return err @@ -132,6 +133,7 @@ func (db *DB) ReserveSample( // round. It is not shared between workers because the read-only // chunk store makes no thread-safety promise. chunkStore := db.ChunkStore() + buf := make([]byte, swarm.SocMaxChunkSize) defer func() { addStats(wstat) }() @@ -152,7 +154,7 @@ func (db *DB) ReserveSample( chunkLoadStart := time.Now() - chunk, err := chunkStore.Get(ctx, chItem.Address) + n, err := chunkStore.GetInto(gCtx, chItem.Address, buf) chunkLoadDuration := time.Since(chunkLoadStart) if err != nil { @@ -164,7 +166,7 @@ func (db *DB) ReserveSample( wstat.ChunkLoadDuration += chunkLoadDuration taddrStart := time.Now() - taddr, err := transformedAddress(hasher, chunk, chItem.ChunkType) + taddr, err := transformedAddress(hasher, chItem.Address, buf[:n], chItem.ChunkType) if err != nil { return err } @@ -173,12 +175,11 @@ func (db *DB) ReserveSample( select { case sampleItemChan <- SampleItem{ TransformedAddress: taddr, - ChunkAddress: chunk.Address(), - ChunkData: chunk.Data(), + ChunkAddress: chItem.Address, Stamp: postage.NewStamp(chItem.BatchID, nil, nil, nil), }: - case <-ctx.Done(): - return ctx.Err() + case <-gCtx.Done(): + return gCtx.Err() } } @@ -224,6 +225,7 @@ func (db *DB) ReserveSample( // Phase 3: Assemble the sample. Here we need to assemble only the first SampleSize // no of items from the results of the 2nd phase. // In this step stamps are loaded and validated only if chunk will be added to sample. + phase3ChunkStore := db.ChunkStore() stats := SampleStats{} for item := range sampleItemChan { currentMaxAddr := swarm.EmptyAddress @@ -239,7 +241,14 @@ func (db *DB) ReserveSample( continue } - ch := swarm.NewChunk(item.ChunkAddress, item.ChunkData).WithStamp(stamp) + ch, err := phase3ChunkStore.Get(ctx, item.ChunkAddress) + if err != nil { + stats.ChunkLoadFailed++ + db.logger.Debug("failed loading chunk", "chunk_address", item.ChunkAddress, "error", err) + continue + } + + ch = ch.WithStamp(stamp) // check if the timestamp on the postage stamp is not later than the consensus time. if binary.BigEndian.Uint64(ch.Stamp().Timestamp()) > consensusTime { @@ -258,6 +267,7 @@ func (db *DB) ReserveSample( stats.ValidStampDuration += stampValidDuration item.Stamp = postage.NewStamp(stamp.BatchID(), stamp.Index(), stamp.Timestamp(), stamp.Sig()) + item.ChunkData = ch.Data() insert(item) stats.SampleInserts++ @@ -300,22 +310,25 @@ func (db *DB) batchesBelowValue(until *big.Int) (map[string]struct{}, error) { return res, err } -func transformedAddress(hasher bmt.Hasher, chunk swarm.Chunk, chType swarm.ChunkType) (swarm.Address, error) { +func transformedAddress(hasher bmt.Hasher, addr swarm.Address, data []byte, chType swarm.ChunkType) (swarm.Address, error) { switch chType { case swarm.ChunkTypeContentAddressed: - return transformedAddressCAC(hasher, chunk) + return transformedAddressCAC(hasher, data) case swarm.ChunkTypeSingleOwner: - return transformedAddressSOC(hasher, chunk) + return transformedAddressSOC(hasher, addr, data) default: return swarm.ZeroAddress, fmt.Errorf("chunk type [%v] is not valid", chType) } } -func transformedAddressCAC(hasher bmt.Hasher, chunk swarm.Chunk) (swarm.Address, error) { +func transformedAddressCAC(hasher bmt.Hasher, data []byte) (swarm.Address, error) { + if len(data) < bmt.SpanSize { + return swarm.ZeroAddress, errors.New("chunk data too short for span") + } hasher.Reset() - hasher.SetHeader(chunk.Data()[:bmt.SpanSize]) + hasher.SetHeader(data[:bmt.SpanSize]) - _, err := hasher.Write(chunk.Data()[bmt.SpanSize:]) + _, err := hasher.Write(data[bmt.SpanSize:]) if err != nil { return swarm.ZeroAddress, err } @@ -323,20 +336,20 @@ func transformedAddressCAC(hasher bmt.Hasher, chunk swarm.Chunk) (swarm.Address, return swarm.NewAddress(hasher.Sum(nil)), nil } -func transformedAddressSOC(hasher bmt.Hasher, socChunk swarm.Chunk) (swarm.Address, error) { - // Calculate transformed address from wrapped chunk - cacChunk, err := soc.UnwrapCAC(socChunk) - if err != nil { - return swarm.ZeroAddress, err +func transformedAddressSOC(hasher bmt.Hasher, socAddr swarm.Address, data []byte) (swarm.Address, error) { + if len(data) < swarm.SocMinChunkSize { + return swarm.ZeroAddress, errors.New("chunk data too short for soc") } - taddrCac, err := transformedAddressCAC(hasher, cacChunk) + cursor := swarm.HashSize + swarm.SocSignatureSize + cacData := data[cursor:] + taddrCac, err := transformedAddressCAC(hasher, cacData) if err != nil { return swarm.ZeroAddress, err } // Hash address and transformed address to make transformed address for this SOC sHasher := swarm.NewHasher() - if _, err := sHasher.Write(socChunk.Address().Bytes()); err != nil { + if _, err := sHasher.Write(socAddr.Bytes()); err != nil { return swarm.ZeroAddress, err } if _, err := sHasher.Write(taddrCac.Bytes()); err != nil { @@ -405,7 +418,7 @@ func RandSample(t *testing.T, anchor []byte) Sample { func MakeSampleUsingChunks(chunks []swarm.Chunk, anchor []byte) (Sample, error) { items := make([]SampleItem, len(chunks)) for i, ch := range chunks { - tr, err := transformedAddress(bmt.NewPrefixHasher(anchor), ch, getChunkType(ch)) + tr, err := transformedAddress(bmt.NewPrefixHasher(anchor), ch.Address(), ch.Data(), getChunkType(ch)) if err != nil { return Sample{}, err } diff --git a/pkg/storer/sample_test.go b/pkg/storer/sample_test.go index f9976061d23..bd0036004be 100644 --- a/pkg/storer/sample_test.go +++ b/pkg/storer/sample_test.go @@ -14,9 +14,8 @@ import ( "github.com/ethersphere/bee/v2/pkg/bmt" "github.com/ethersphere/bee/v2/pkg/cac" "github.com/ethersphere/bee/v2/pkg/postage" - "github.com/ethersphere/bee/v2/pkg/soc" - postagetesting "github.com/ethersphere/bee/v2/pkg/postage/testing" + "github.com/ethersphere/bee/v2/pkg/soc" chunk "github.com/ethersphere/bee/v2/pkg/storage/testing" "github.com/ethersphere/bee/v2/pkg/storer" "github.com/ethersphere/bee/v2/pkg/swarm" From 841bbafef00b15bb2418eba3f4e5c4b4f51d5cb5 Mon Sep 17 00:00:00 2001 From: Ljubisa Gacevic Date: Mon, 14 Sep 2026 13:52:38 +0200 Subject: [PATCH 4/7] perf(storer): pass batch id through sampler channel to avoid stamp allocations --- pkg/storer/sample.go | 36 ++++++++++++++++++++++-------------- 1 file changed, 22 insertions(+), 14 deletions(-) diff --git a/pkg/storer/sample.go b/pkg/storer/sample.go index 19e680b3d6d..66a21032f3c 100644 --- a/pkg/storer/sample.go +++ b/pkg/storer/sample.go @@ -38,6 +38,12 @@ type SampleItem struct { Stamp *postage.Stamp } +type sampleItemInternal struct { + transformedAddress swarm.Address + chunkAddress swarm.Address + batchID []byte +} + type Sample struct { Stats SampleStats Items []SampleItem @@ -120,7 +126,7 @@ func (db *DB) ReserveSample( })) // Phase 2: Get the chunk data and calculate transformed hash - sampleItemChan := make(chan SampleItem, 3*workers) + sampleItemChan := make(chan sampleItemInternal, 3*workers) db.logger.Debug("reserve sampler workers", "count", workers) @@ -173,10 +179,10 @@ func (db *DB) ReserveSample( wstat.TaddrDuration += time.Since(taddrStart) select { - case sampleItemChan <- SampleItem{ - TransformedAddress: taddr, - ChunkAddress: chItem.Address, - Stamp: postage.NewStamp(chItem.BatchID, nil, nil, nil), + case sampleItemChan <- sampleItemInternal{ + transformedAddress: taddr, + chunkAddress: chItem.Address, + batchID: chItem.BatchID, }: case <-gCtx.Done(): return gCtx.Err() @@ -233,18 +239,18 @@ func (db *DB) ReserveSample( currentMaxAddr = sampleItems[len(sampleItems)-1].TransformedAddress } - if le(item.TransformedAddress, currentMaxAddr) || len(sampleItems) < SampleSize { - stamp, err := chunkstamp.LoadWithBatchID(db.storage.IndexStore(), "reserve", item.ChunkAddress, item.Stamp.BatchID()) + if le(item.transformedAddress, currentMaxAddr) || len(sampleItems) < SampleSize { + stamp, err := chunkstamp.LoadWithBatchID(db.storage.IndexStore(), "reserve", item.chunkAddress, item.batchID) if err != nil { stats.StampLoadFailed++ - db.logger.Debug("failed loading stamp", "chunk_address", item.ChunkAddress, "error", err) + db.logger.Debug("failed loading stamp", "chunk_address", item.chunkAddress, "error", err) continue } - ch, err := phase3ChunkStore.Get(ctx, item.ChunkAddress) + ch, err := phase3ChunkStore.Get(ctx, item.chunkAddress) if err != nil { stats.ChunkLoadFailed++ - db.logger.Debug("failed loading chunk", "chunk_address", item.ChunkAddress, "error", err) + db.logger.Debug("failed loading chunk", "chunk_address", item.chunkAddress, "error", err) continue } @@ -266,10 +272,12 @@ func (db *DB) ReserveSample( stampValidDuration := time.Since(stampValidStart) stats.ValidStampDuration += stampValidDuration - item.Stamp = postage.NewStamp(stamp.BatchID(), stamp.Index(), stamp.Timestamp(), stamp.Sig()) - item.ChunkData = ch.Data() - - insert(item) + insert(SampleItem{ + TransformedAddress: item.transformedAddress, + ChunkAddress: item.chunkAddress, + ChunkData: ch.Data(), + Stamp: postage.NewStamp(stamp.BatchID(), stamp.Index(), stamp.Timestamp(), stamp.Sig()), + }) stats.SampleInserts++ } } From 40f51d4617e713c908fedcf758177ef5f1619d2a Mon Sep 17 00:00:00 2001 From: Ljubisa Gacevic Date: Mon, 14 Sep 2026 18:27:41 +0200 Subject: [PATCH 5/7] test(storer): benchmark transformedAddress for CAC and SOC separately Both ReserveSample benchmarks fill their reserve with chunk.GenerateValidRandomChunkAt, which produces content-addressed chunks only. The SOC branch of transformedAddress is therefore never measured by them, and any work that branch does beyond hashing the wrapped CAC is invisible. Benchmark the function directly, one case per chunk type, through a shim in export_test.go. The shim takes a swarm.Chunk and is held to that signature on purpose so the same benchmark source can be run against branches whose internal transformedAddress is shaped differently; only the shim changes between them. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01KFUseFQ8rhp6N9X6YS7pbq --- pkg/storer/export_test.go | 13 +++++++++++ pkg/storer/sample_test.go | 47 +++++++++++++++++++++++++++++++++++++++ 2 files changed, 60 insertions(+) diff --git a/pkg/storer/export_test.go b/pkg/storer/export_test.go index b30b5996edd..d9b779ba97f 100644 --- a/pkg/storer/export_test.go +++ b/pkg/storer/export_test.go @@ -5,8 +5,10 @@ package storer import ( + "github.com/ethersphere/bee/v2/pkg/bmt" "github.com/ethersphere/bee/v2/pkg/storer/internal/events" "github.com/ethersphere/bee/v2/pkg/storer/internal/reserve" + "github.com/ethersphere/bee/v2/pkg/swarm" ) func (db *DB) Reserve() *reserve.Reserve { @@ -35,3 +37,14 @@ func (db *DB) WaitForBgCacheWorkers() (unblock func()) { func DefaultOptions() *Options { return defaultOptions() } + +// TransformedAddress exposes the sampler's per-chunk hashing so it can be +// benchmarked on its own. +// +// The exported signature takes a swarm.Chunk and is deliberately held stable +// even where transformedAddress itself does not, so that the same benchmark +// source can be run against branches that shape the internal function +// differently. Only this shim changes between them. +func TransformedAddress(hasher bmt.Hasher, ch swarm.Chunk, chType swarm.ChunkType) (swarm.Address, error) { + return transformedAddress(hasher, ch.Address(), ch.Data(), chType) +} diff --git a/pkg/storer/sample_test.go b/pkg/storer/sample_test.go index bd0036004be..31f44364ed6 100644 --- a/pkg/storer/sample_test.go +++ b/pkg/storer/sample_test.go @@ -517,6 +517,53 @@ func benchmarkReserveSample(b *testing.B, chunkCountPerPO int) { } } +// BenchmarkTransformedAddress measures the sampler's per-chunk hashing on its +// own, separately for a content-addressed and a single owner chunk. +// +// The SOC case is the reason this exists. Both ReserveSample benchmarks build +// their reserve with chunk.GenerateValidRandomChunkAt, which produces CAC +// chunks only, so the SOC branch of transformedAddress is never measured by +// them. Any work the SOC path does over and above hashing the wrapped CAC shows +// up here and nowhere else. +func BenchmarkTransformedAddress(b *testing.B) { + anchor := []byte("swarm-test-anchor-deterministic!") + + content := make([]byte, swarm.ChunkSize) + for i := range content { + content[i] = byte(i) + } + + cacChunk, err := cac.New(content) + if err != nil { + b.Fatal(err) + } + socChunk := chunk.GenerateTestRandomSoChunk(b, cacChunk) + + for _, tc := range []struct { + name string + ch swarm.Chunk + typ swarm.ChunkType + }{ + {"cac", cacChunk, swarm.ChunkTypeContentAddressed}, + {"soc", socChunk, swarm.ChunkTypeSingleOwner}, + } { + b.Run(tc.name, func(b *testing.B) { + // One hasher reused across iterations, as a sampler worker does. + hasher := bmt.NewPrefixHasher(anchor) + + b.ReportAllocs() + b.SetBytes(int64(len(tc.ch.Data()))) + b.ResetTimer() + + for b.Loop() { + if _, err := storer.TransformedAddress(hasher, tc.ch, tc.typ); err != nil { + b.Fatal(err) + } + } + }) + } +} + // BenchmarkSampleHashing measures the time taken by MakeSampleUsingChunks to // hash a fixed set of CAC chunks. func BenchmarkSampleHashing(b *testing.B) { From 207b420ed1eb652162734a134f001f420e15fab9 Mon Sep 17 00:00:00 2001 From: Ljubisa Gacevic Date: Tue, 15 Sep 2026 22:33:05 +0200 Subject: [PATCH 6/7] fix(storage): check buffer capacity in GetInto --- pkg/file/joiner/joiner_test.go | 7 ++- pkg/storage/chunkstore.go | 5 +- .../inmemchunkstore/inmemchunkstore.go | 7 ++- pkg/storer/internal/chunkstore/chunkstore.go | 4 +- .../internal/chunkstore/chunkstore_test.go | 51 +++++++++++++++++++ 5 files changed, 62 insertions(+), 12 deletions(-) diff --git a/pkg/file/joiner/joiner_test.go b/pkg/file/joiner/joiner_test.go index ad7c40b8819..c8125655581 100644 --- a/pkg/file/joiner/joiner_test.go +++ b/pkg/file/joiner/joiner_test.go @@ -1402,11 +1402,10 @@ func (c *chunkStore) GetInto(ctx context.Context, addr swarm.Address, buf []byte return 0, err } data := ch.Data() - if len(buf) < len(data) { - return 0, fmt.Errorf("chunk store: buffer too small: %d < %d", len(buf), len(data)) + if cap(buf) < len(data) { + return 0, fmt.Errorf("chunk store: buffer too small: %d < %d", cap(buf), len(data)) } - copy(buf, data) - return len(data), nil + return copy(buf[:len(data)], data), nil } func (c *chunkStore) Put(_ context.Context, ch swarm.Chunk) error { diff --git a/pkg/storage/chunkstore.go b/pkg/storage/chunkstore.go index 2809493e129..e2ee4d3d481 100644 --- a/pkg/storage/chunkstore.go +++ b/pkg/storage/chunkstore.go @@ -22,8 +22,9 @@ type Getter interface { } // GetterInto is like Getter but reads chunk data into a caller-provided buffer, -// avoiding per-call allocations. The buffer must be large enough to hold the chunk. -// Returns the number of bytes read into buf. +// avoiding per-call allocations. The buffer's capacity must be large enough to +// hold the chunk. Returns the number of bytes read into buf; callers reslice +// with it. type GetterInto interface { GetInto(ctx context.Context, addr swarm.Address, buf []byte) (int, error) } diff --git a/pkg/storage/inmemchunkstore/inmemchunkstore.go b/pkg/storage/inmemchunkstore/inmemchunkstore.go index e775a6bc884..0dfeb732100 100644 --- a/pkg/storage/inmemchunkstore/inmemchunkstore.go +++ b/pkg/storage/inmemchunkstore/inmemchunkstore.go @@ -49,11 +49,10 @@ func (c *ChunkStore) GetInto(_ context.Context, addr swarm.Address, buf []byte) return 0, storage.ErrNotFound } data := chunk.chunk.Data() - if len(buf) < len(data) { - return 0, fmt.Errorf("chunk store: buffer too small: %d < %d", len(buf), len(data)) + if cap(buf) < len(data) { + return 0, fmt.Errorf("chunk store: buffer too small: %d < %d", cap(buf), len(data)) } - copy(buf, data) - return len(data), nil + return copy(buf[:len(data)], data), nil } func (c *ChunkStore) Put(_ context.Context, ch swarm.Chunk) error { diff --git a/pkg/storer/internal/chunkstore/chunkstore.go b/pkg/storer/internal/chunkstore/chunkstore.go index 0b70134f38b..05968538b6e 100644 --- a/pkg/storer/internal/chunkstore/chunkstore.go +++ b/pkg/storer/internal/chunkstore/chunkstore.go @@ -60,8 +60,8 @@ func GetInto(ctx context.Context, r storage.Reader, s storage.Sharky, addr swarm return 0, fmt.Errorf("chunk store: failed reading retrievalIndex for address %s: %w", addr, err) } n := int(rIdx.Location.Length) - if len(buf) < n { - return 0, fmt.Errorf("chunk store: buffer too small: %d < %d", len(buf), n) + if cap(buf) < n { + return 0, fmt.Errorf("chunk store: buffer too small: %d < %d", cap(buf), n) } err = s.Read(ctx, rIdx.Location, buf[:n]) if err != nil { diff --git a/pkg/storer/internal/chunkstore/chunkstore_test.go b/pkg/storer/internal/chunkstore/chunkstore_test.go index d07728d321d..09000a193ee 100644 --- a/pkg/storer/internal/chunkstore/chunkstore_test.go +++ b/pkg/storer/internal/chunkstore/chunkstore_test.go @@ -580,3 +580,54 @@ func BenchmarkChunkStoreGet(b *testing.B) { } }) } + +// TestGetInto checks the buffer contract of GetInto: the size check is on +// capacity, so one reusable buffer passed as buf[:0] serves both a CAC and a +// larger SOC, and a buffer that cannot hold the chunk is rejected. +func TestGetInto(t *testing.T) { + t.Parallel() + + ctx := context.Background() + st := makeStorage(t) + + cac := chunktest.GenerateTestRandomChunk() + soc := soctesting.GenerateMockSOC(t, cac.Data()[swarm.SpanSize:]).Chunk() + if len(soc.Data()) <= len(cac.Data()) { + t.Fatalf("soc data (%d) should be larger than cac data (%d)", len(soc.Data()), len(cac.Data())) + } + + for _, ch := range []swarm.Chunk{cac, soc} { + err := st.Run(ctx, func(s transaction.Store) error { + return s.ChunkStore().Put(ctx, ch) + }) + if err != nil { + t.Fatal(err) + } + } + + cs := st.ChunkStore() + + t.Run("reused buffer with spare capacity", func(t *testing.T) { + buf := make([]byte, 0, swarm.SocMaxChunkSize) + for _, ch := range []swarm.Chunk{cac, soc, cac} { + n, err := cs.GetInto(ctx, ch.Address(), buf) + if err != nil { + t.Fatal(err) + } + if n != len(ch.Data()) || !bytes.Equal(buf[:n], ch.Data()) { + t.Fatalf("chunk %s: got %d bytes, want %d with matching data", ch.Address(), n, len(ch.Data())) + } + } + }) + + t.Run("capacity too small", func(t *testing.T) { + buf := make([]byte, 0, len(soc.Data())-1) + n, err := cs.GetInto(ctx, soc.Address(), buf) + if err == nil { + t.Fatal("expected error for buffer with insufficient capacity") + } + if n != 0 { + t.Fatalf("got %d bytes, want 0", n) + } + }) +} From b9bf7771bfaf7133291a31326321cf9896a41b04 Mon Sep 17 00:00:00 2001 From: Ljubisa Gacevic Date: Thu, 17 Sep 2026 17:11:45 +0200 Subject: [PATCH 7/7] fix(storage): check buffer length in GetInto, not capacity --- pkg/file/joiner/joiner_test.go | 6 ++-- pkg/storage/chunkstore.go | 6 ++-- .../inmemchunkstore/inmemchunkstore.go | 6 ++-- pkg/storer/internal/chunkstore/chunkstore.go | 4 +-- .../internal/chunkstore/chunkstore_test.go | 36 +++++++++++++++---- 5 files changed, 40 insertions(+), 18 deletions(-) diff --git a/pkg/file/joiner/joiner_test.go b/pkg/file/joiner/joiner_test.go index c8125655581..9fe93640c92 100644 --- a/pkg/file/joiner/joiner_test.go +++ b/pkg/file/joiner/joiner_test.go @@ -1402,10 +1402,10 @@ func (c *chunkStore) GetInto(ctx context.Context, addr swarm.Address, buf []byte return 0, err } data := ch.Data() - if cap(buf) < len(data) { - return 0, fmt.Errorf("chunk store: buffer too small: %d < %d", cap(buf), len(data)) + if len(buf) < len(data) { + return 0, fmt.Errorf("chunk store: buffer too small: %d < %d", len(buf), len(data)) } - return copy(buf[:len(data)], data), nil + return copy(buf, data), nil } func (c *chunkStore) Put(_ context.Context, ch swarm.Chunk) error { diff --git a/pkg/storage/chunkstore.go b/pkg/storage/chunkstore.go index e2ee4d3d481..faefd1f4489 100644 --- a/pkg/storage/chunkstore.go +++ b/pkg/storage/chunkstore.go @@ -22,9 +22,9 @@ type Getter interface { } // GetterInto is like Getter but reads chunk data into a caller-provided buffer, -// avoiding per-call allocations. The buffer's capacity must be large enough to -// hold the chunk. Returns the number of bytes read into buf; callers reslice -// with it. +// avoiding per-call allocations. len(buf) must be at least the chunk size; +// GetInto never writes past len(buf). Returns the number of bytes read into +// buf; callers use buf[:n]. type GetterInto interface { GetInto(ctx context.Context, addr swarm.Address, buf []byte) (int, error) } diff --git a/pkg/storage/inmemchunkstore/inmemchunkstore.go b/pkg/storage/inmemchunkstore/inmemchunkstore.go index 0dfeb732100..6e67a505476 100644 --- a/pkg/storage/inmemchunkstore/inmemchunkstore.go +++ b/pkg/storage/inmemchunkstore/inmemchunkstore.go @@ -49,10 +49,10 @@ func (c *ChunkStore) GetInto(_ context.Context, addr swarm.Address, buf []byte) return 0, storage.ErrNotFound } data := chunk.chunk.Data() - if cap(buf) < len(data) { - return 0, fmt.Errorf("chunk store: buffer too small: %d < %d", cap(buf), len(data)) + if len(buf) < len(data) { + return 0, fmt.Errorf("chunk store: buffer too small: %d < %d", len(buf), len(data)) } - return copy(buf[:len(data)], data), nil + return copy(buf, data), nil } func (c *ChunkStore) Put(_ context.Context, ch swarm.Chunk) error { diff --git a/pkg/storer/internal/chunkstore/chunkstore.go b/pkg/storer/internal/chunkstore/chunkstore.go index 05968538b6e..0b70134f38b 100644 --- a/pkg/storer/internal/chunkstore/chunkstore.go +++ b/pkg/storer/internal/chunkstore/chunkstore.go @@ -60,8 +60,8 @@ func GetInto(ctx context.Context, r storage.Reader, s storage.Sharky, addr swarm return 0, fmt.Errorf("chunk store: failed reading retrievalIndex for address %s: %w", addr, err) } n := int(rIdx.Location.Length) - if cap(buf) < n { - return 0, fmt.Errorf("chunk store: buffer too small: %d < %d", cap(buf), n) + if len(buf) < n { + return 0, fmt.Errorf("chunk store: buffer too small: %d < %d", len(buf), n) } err = s.Read(ctx, rIdx.Location, buf[:n]) if err != nil { diff --git a/pkg/storer/internal/chunkstore/chunkstore_test.go b/pkg/storer/internal/chunkstore/chunkstore_test.go index 09000a193ee..94721a3c53a 100644 --- a/pkg/storer/internal/chunkstore/chunkstore_test.go +++ b/pkg/storer/internal/chunkstore/chunkstore_test.go @@ -582,8 +582,9 @@ func BenchmarkChunkStoreGet(b *testing.B) { } // TestGetInto checks the buffer contract of GetInto: the size check is on -// capacity, so one reusable buffer passed as buf[:0] serves both a CAC and a -// larger SOC, and a buffer that cannot hold the chunk is rejected. +// len(buf), one full-length buffer serves both a CAC and a larger SOC via the +// returned count, nothing is written past len(buf), and a buffer whose length +// is too short is rejected even when its capacity would fit the chunk. func TestGetInto(t *testing.T) { t.Parallel() @@ -607,8 +608,8 @@ func TestGetInto(t *testing.T) { cs := st.ChunkStore() - t.Run("reused buffer with spare capacity", func(t *testing.T) { - buf := make([]byte, 0, swarm.SocMaxChunkSize) + t.Run("reused buffer serves CAC and SOC", func(t *testing.T) { + buf := make([]byte, swarm.SocMaxChunkSize) for _, ch := range []swarm.Chunk{cac, soc, cac} { n, err := cs.GetInto(ctx, ch.Address(), buf) if err != nil { @@ -620,11 +621,32 @@ func TestGetInto(t *testing.T) { } }) - t.Run("capacity too small", func(t *testing.T) { - buf := make([]byte, 0, len(soc.Data())-1) + t.Run("does not write past len", func(t *testing.T) { + backing := make([]byte, swarm.SocMaxChunkSize) + for i := range backing { + backing[i] = 0xff + } + buf := backing[:len(cac.Data())] + n, err := cs.GetInto(ctx, cac.Address(), buf) + if err != nil { + t.Fatal(err) + } + if n != len(cac.Data()) || !bytes.Equal(buf, cac.Data()) { + t.Fatalf("got %d bytes, want %d with matching data", n, len(cac.Data())) + } + for i := n; i < len(backing); i++ { + if backing[i] != 0xff { + t.Fatalf("GetInto wrote past len(buf) at index %d", i) + } + } + }) + + t.Run("length too small", func(t *testing.T) { + // Capacity would fit the chunk; only the length is short. + buf := make([]byte, len(soc.Data())-1, swarm.SocMaxChunkSize) n, err := cs.GetInto(ctx, soc.Address(), buf) if err == nil { - t.Fatal("expected error for buffer with insufficient capacity") + t.Fatal("expected error for buffer shorter than the chunk") } if n != 0 { t.Fatalf("got %d bytes, want 0", n)