From 58a5ac50988a909bff4871f4ef36e1b53eb8dab4 Mon Sep 17 00:00:00 2001 From: Calin Martinconi Date: Fri, 17 Jul 2026 10:34:30 +0300 Subject: [PATCH 01/17] feat(pullsync): add chunk checksum for divergent SOC sync (SWIP-101) --- pkg/pullsync/pb/pullsync.pb.go | 79 +++----------- pkg/pullsync/pb/pullsync.proto | 3 +- pkg/pullsync/pullsync.go | 66 ++++++------ pkg/pullsync/pullsync_test.go | 11 +- pkg/storage/storage.go | 32 ++++++ pkg/storage/storage_test.go | 108 +++++++++++++++++++ pkg/storer/compact_test.go | 9 +- pkg/storer/internal/reserve/items.go | 55 +++++++++- pkg/storer/internal/reserve/items_test.go | 1 + pkg/storer/internal/reserve/reserve.go | 47 +++++++-- pkg/storer/internal/reserve/reserve_test.go | 2 +- pkg/storer/migration/all_steps.go | 1 + pkg/storer/migration/step_08.go | 110 ++++++++++++++++++++ pkg/storer/mock/mockreserve.go | 22 +++- pkg/storer/reserve.go | 9 +- pkg/storer/reserve_test.go | 8 +- pkg/storer/storer.go | 2 +- 17 files changed, 438 insertions(+), 127 deletions(-) create mode 100644 pkg/storer/migration/step_08.go diff --git a/pkg/pullsync/pb/pullsync.pb.go b/pkg/pullsync/pb/pullsync.pb.go index 6c3bec8c85a..89dc6ea44bb 100644 --- a/pkg/pullsync/pb/pullsync.pb.go +++ b/pkg/pullsync/pb/pullsync.pb.go @@ -163,9 +163,8 @@ func (m *Get) GetStart() uint64 { } type Chunk struct { - Address []byte `protobuf:"bytes,1,opt,name=Address,proto3" json:"Address,omitempty"` - BatchID []byte `protobuf:"bytes,2,opt,name=BatchID,proto3" json:"BatchID,omitempty"` - StampHash []byte `protobuf:"bytes,3,opt,name=StampHash,proto3" json:"StampHash,omitempty"` + Address []byte `protobuf:"bytes,1,opt,name=Address,proto3" json:"Address,omitempty"` + Sum []byte `protobuf:"bytes,2,opt,name=Sum,proto3" json:"Sum,omitempty"` } func (m *Chunk) Reset() { *m = Chunk{} } @@ -208,16 +207,9 @@ func (m *Chunk) GetAddress() []byte { return nil } -func (m *Chunk) GetBatchID() []byte { - if m != nil { - return m.BatchID - } - return nil -} - -func (m *Chunk) GetStampHash() []byte { +func (m *Chunk) GetSum() []byte { if m != nil { - return m.StampHash + return m.Sum } return nil } @@ -536,17 +528,10 @@ func (m *Chunk) MarshalToSizedBuffer(dAtA []byte) (int, error) { _ = i var l int _ = l - if len(m.StampHash) > 0 { - i -= len(m.StampHash) - copy(dAtA[i:], m.StampHash) - i = encodeVarintPullsync(dAtA, i, uint64(len(m.StampHash))) - i-- - dAtA[i] = 0x1a - } - if len(m.BatchID) > 0 { - i -= len(m.BatchID) - copy(dAtA[i:], m.BatchID) - i = encodeVarintPullsync(dAtA, i, uint64(len(m.BatchID))) + if len(m.Sum) > 0 { + i -= len(m.Sum) + copy(dAtA[i:], m.Sum) + i = encodeVarintPullsync(dAtA, i, uint64(len(m.Sum))) i-- dAtA[i] = 0x12 } @@ -740,11 +725,7 @@ func (m *Chunk) Size() (n int) { if l > 0 { n += 1 + l + sovPullsync(uint64(l)) } - l = len(m.BatchID) - if l > 0 { - n += 1 + l + sovPullsync(uint64(l)) - } - l = len(m.StampHash) + l = len(m.Sum) if l > 0 { n += 1 + l + sovPullsync(uint64(l)) } @@ -1157,41 +1138,7 @@ func (m *Chunk) Unmarshal(dAtA []byte) error { iNdEx = postIndex case 2: if wireType != 2 { - return fmt.Errorf("proto: wrong wireType = %d for field BatchID", wireType) - } - var byteLen int - for shift := uint(0); ; shift += 7 { - if shift >= 64 { - return ErrIntOverflowPullsync - } - if iNdEx >= l { - return io.ErrUnexpectedEOF - } - b := dAtA[iNdEx] - iNdEx++ - byteLen |= int(b&0x7F) << shift - if b < 0x80 { - break - } - } - if byteLen < 0 { - return ErrInvalidLengthPullsync - } - postIndex := iNdEx + byteLen - if postIndex < 0 { - return ErrInvalidLengthPullsync - } - if postIndex > l { - return io.ErrUnexpectedEOF - } - m.BatchID = append(m.BatchID[:0], dAtA[iNdEx:postIndex]...) - if m.BatchID == nil { - m.BatchID = []byte{} - } - iNdEx = postIndex - case 3: - if wireType != 2 { - return fmt.Errorf("proto: wrong wireType = %d for field StampHash", wireType) + return fmt.Errorf("proto: wrong wireType = %d for field Sum", wireType) } var byteLen int for shift := uint(0); ; shift += 7 { @@ -1218,9 +1165,9 @@ func (m *Chunk) Unmarshal(dAtA []byte) error { if postIndex > l { return io.ErrUnexpectedEOF } - m.StampHash = append(m.StampHash[:0], dAtA[iNdEx:postIndex]...) - if m.StampHash == nil { - m.StampHash = []byte{} + m.Sum = append(m.Sum[:0], dAtA[iNdEx:postIndex]...) + if m.Sum == nil { + m.Sum = []byte{} } iNdEx = postIndex default: diff --git a/pkg/pullsync/pb/pullsync.proto b/pkg/pullsync/pb/pullsync.proto index fd5783905e4..fb34ce535bc 100644 --- a/pkg/pullsync/pb/pullsync.proto +++ b/pkg/pullsync/pb/pullsync.proto @@ -22,8 +22,7 @@ message Get { message Chunk { bytes Address = 1; - bytes BatchID = 2; - bytes StampHash = 3; + bytes Sum = 2; } message Offer { diff --git a/pkg/pullsync/pullsync.go b/pkg/pullsync/pullsync.go index eb385f6f60d..92886bfd896 100644 --- a/pkg/pullsync/pullsync.go +++ b/pkg/pullsync/pullsync.go @@ -36,7 +36,7 @@ const loggerName = "pullsync" const ( protocolName = "pullsync" - protocolVersion = "1.4.0" + protocolVersion = "2.0.0" streamName = "pullsync" cursorStreamName = "cursors" ) @@ -164,8 +164,10 @@ func (s *Syncer) handler(streamCtx context.Context, p p2p.Peer, stream p2p.Strea // while makeOffer is executing (waiting for the new chunks) w, r := protobuf.NewWriterAndReader(stream) - // make an offer to the upstream peer in return for the requested range - offer, err := s.makeOffer(ctx, rn) + // make an offer to the upstream peer in return for the requested range. + // the bincs slice is retained so processWant can resolve wanted chunks by + // their batch ID and stamp hash, which are no longer carried on the wire. + offer, bincs, err := s.makeOffer(ctx, rn) if err != nil { return fmt.Errorf("make offer: %w", err) } @@ -187,7 +189,7 @@ func (s *Syncer) handler(streamCtx context.Context, p p2p.Peer, stream p2p.Strea return fmt.Errorf("read want: %w", err) } - chs, err := s.processWant(ctx, offer, &want) + chs, err := s.processWant(ctx, bincs, &want) if err != nil { return fmt.Errorf("process want: %w", err) } @@ -274,8 +276,7 @@ func (s *Syncer) Sync(ctx context.Context, peer swarm.Address, bin uint8, start } addr := offer.Chunks[i].Address - batchID := offer.Chunks[i].BatchID - stampHash := offer.Chunks[i].StampHash + sum := offer.Chunks[i].Sum if len(addr) != swarm.HashSize { return 0, 0, fmt.Errorf("inconsistent hash length") } @@ -288,14 +289,14 @@ func (s *Syncer) Sync(ctx context.Context, peer swarm.Address, bin uint8, start } s.metrics.Offered.Inc() if s.store.IsWithinStorageRadius(a) { - have, err = s.store.ReserveHas(a, batchID, stampHash) + have, err = s.store.ReserveHas(a, sum) if err != nil { s.logger.Debug("storage has", "error", err) return 0, 0, err } if !have { - wantChunks[a.ByteString()+string(batchID)+string(stampHash)] = struct{}{} + wantChunks[a.ByteString()+string(sum)] = struct{}{} ctr++ s.metrics.Wanted.Inc() bv.Set(i) @@ -331,13 +332,16 @@ func (s *Syncer) Sync(ctx context.Context, peer swarm.Address, bin uint8, start chunkErr = errors.Join(chunkErr, err) continue } - stampHash, err := stamp.Hash() + + // recompute the divergence checksum from the delivered chunk and stamp + // and match it against the outstanding (address, sum) want set. + sum, err := storage.ChunkSum(newChunk.WithStamp(stamp)) if err != nil { chunkErr = errors.Join(chunkErr, err) continue } - wantChunkID := addr.ByteString() + string(stamp.BatchID()) + string(stampHash) + wantChunkID := addr.ByteString() + string(sum) if _, ok := wantChunks[wantChunkID]; !ok { s.logger.Debug("want chunks", "error", ErrUnsolicitedChunk, "peer_address", peer, "chunk_address", addr) chunkErr = errors.Join(chunkErr, ErrUnsolicitedChunk) @@ -396,20 +400,23 @@ func (s *Syncer) Sync(ctx context.Context, peer swarm.Address, bin uint8, start return topmost, chunksPut, chunkErr } -// makeOffer tries to assemble an offer for a given requested interval. -func (s *Syncer) makeOffer(ctx context.Context, rn pb.Get) (*pb.Offer, error) { - addrs, top, err := s.collectAddrs(ctx, uint8(rn.Bin), rn.Start) +// makeOffer tries to assemble an offer for a given requested interval. It +// returns both the wire offer (carrying only address and sum per chunk) and the +// underlying BinC slice, which the caller keeps to resolve wanted chunks by +// batch ID and stamp hash when fulfilling the subsequent Want. +func (s *Syncer) makeOffer(ctx context.Context, rn pb.Get) (*pb.Offer, []*storer.BinC, error) { + bincs, top, err := s.collectAddrs(ctx, uint8(rn.Bin), rn.Start) if err != nil { - return nil, err + return nil, nil, err } o := new(pb.Offer) o.Topmost = top - o.Chunks = make([]*pb.Chunk, 0, len(addrs)) - for _, v := range addrs { - o.Chunks = append(o.Chunks, &pb.Chunk{Address: v.Address.Bytes(), BatchID: v.BatchID, StampHash: v.StampHash}) + o.Chunks = make([]*pb.Chunk, 0, len(bincs)) + for _, v := range bincs { + o.Chunks = append(o.Chunks, &pb.Chunk{Address: v.Address.Bytes(), Sum: v.Sum}) } - return o, nil + return o, bincs, nil } type collectAddrsResult struct { @@ -447,7 +454,7 @@ func (s *Syncer) collectAddrs(ctx context.Context, bin uint8, start uint64) ([]* break LOOP // The stream has been closed. } - chs = append(chs, &storer.BinC{Address: c.Address, BatchID: c.BatchID, StampHash: c.StampHash}) + chs = append(chs, &storer.BinC{Address: c.Address, BatchID: c.BatchID, StampHash: c.StampHash, Sum: c.Sum}) if c.BinID > topmost { topmost = c.BinID } @@ -479,32 +486,31 @@ func (s *Syncer) collectAddrs(ctx context.Context, bin uint8, start uint64) ([]* return v.chs, v.topmost, nil } -// processWant compares a received Want to a sent Offer and returns +// processWant compares a received Want to the offered BinC slice and returns // the appropriate chunks from the local store. -func (s *Syncer) processWant(ctx context.Context, o *pb.Offer, w *pb.Want) ([]swarm.Chunk, error) { - bv, err := bitvector.NewFromBytes(w.BitVector, len(o.Chunks)) +func (s *Syncer) processWant(ctx context.Context, bincs []*storer.BinC, w *pb.Want) ([]swarm.Chunk, error) { + bv, err := bitvector.NewFromBytes(w.BitVector, len(bincs)) if err != nil { return nil, err } - chunks := make([]swarm.Chunk, 0, len(o.Chunks)) - for i := 0; i < len(o.Chunks); i++ { - if o.Chunks[i] == nil { + chunks := make([]swarm.Chunk, 0, len(bincs)) + for i := 0; i < len(bincs); i++ { + if bincs[i] == nil { return nil, fmt.Errorf("nil chunk at index %d in offer", i) } if bv.Get(i) { - ch := o.Chunks[i] - addr := swarm.NewAddress(ch.Address) + c := bincs[i] s.metrics.SentWanted.Inc() - c, err := s.store.ReserveGet(ctx, addr, ch.BatchID, ch.StampHash) + ch, err := s.store.ReserveGet(ctx, c.Address, c.BatchID, c.StampHash) if err != nil { - s.logger.Debug("processing want: unable to find chunk", "chunk_address", addr, "batch_id", hex.EncodeToString(ch.BatchID)) + s.logger.Debug("processing want: unable to find chunk", "chunk_address", c.Address, "batch_id", hex.EncodeToString(c.BatchID)) chunks = append(chunks, swarm.NewChunk(swarm.ZeroAddress, nil)) s.metrics.MissingChunks.Inc() continue } - chunks = append(chunks, c) + chunks = append(chunks, ch) } } return chunks, nil diff --git a/pkg/pullsync/pullsync_test.go b/pkg/pullsync/pullsync_test.go index 03d6927c1da..28d0bef6ab5 100644 --- a/pkg/pullsync/pullsync_test.go +++ b/pkg/pullsync/pullsync_test.go @@ -49,11 +49,13 @@ func init() { chunks[i] = testingc.GenerateTestRandomChunk() addrs[i] = chunks[i].Address() stampHash, _ := chunks[i].Stamp().Hash() + sum, _ := storage.ChunkSum(chunks[i]) results[i] = &storer.BinC{ Address: addrs[i], BatchID: chunks[i].Stamp().BatchID(), BinID: uint64(i), StampHash: stampHash, + Sum: sum, } } } @@ -165,11 +167,16 @@ func TestIncoming_WantErrors(t *testing.T) { if err != nil { t.Fatal(err) } + sum, err := storage.ChunkSum(c) + if err != nil { + t.Fatal(err) + } tResults[i] = &storer.BinC{ Address: c.Address(), BatchID: c.Stamp().BatchID(), BinID: uint64(i + 5), // start from a higher bin id StampHash: stampHash, + Sum: sum, } } @@ -314,11 +321,11 @@ func TestGetCursorsError(t *testing.T) { func haveChunks(t *testing.T, s *mock.ReserveStore, chunks ...swarm.Chunk) { t.Helper() for _, c := range chunks { - stampHash, err := c.Stamp().Hash() + sum, err := storage.ChunkSum(c) if err != nil { t.Fatal(err) } - have, err := s.ReserveHas(c.Address(), c.Stamp().BatchID(), stampHash) + have, err := s.ReserveHas(c.Address(), sum) if err != nil { t.Fatal(err) } diff --git a/pkg/storage/storage.go b/pkg/storage/storage.go index dfa56ddf4bc..a30a24470e7 100644 --- a/pkg/storage/storage.go +++ b/pkg/storage/storage.go @@ -295,6 +295,38 @@ func ChunkType(ch swarm.Chunk) swarm.ChunkType { return swarm.ChunkTypeUnspecified } +// ChunkSumSize is the length in bytes of the pullsync divergence checksum. +const ChunkSumSize = 16 + +// ChunkSum computes the pullsync divergence checksum for a stamped chunk. +// +// CAC: keccak256(batchID ‖ stampHash)[:16] +// SOC: keccak256(batchID ‖ stampHash ‖ wrappedCacAddress)[:16] +// +// Folding the wrapped CAC address into the SOC sum makes two SOCs that share a +// SOC address but wrap different content produce distinct sums, so both can be +// pulled and reconciled. The chunk must carry a stamp. +func ChunkSum(ch swarm.Chunk) ([]byte, error) { + stampHash, err := ch.Stamp().Hash() + if err != nil { + return nil, fmt.Errorf("stamp hash: %w", err) + } + + h := swarm.NewHasher() + _, _ = h.Write(ch.Stamp().BatchID()) + _, _ = h.Write(stampHash) + + if soc.Valid(ch) { + s, err := soc.FromChunk(ch) + if err != nil { + return nil, fmt.Errorf("soc from chunk: %w", err) + } + _, _ = h.Write(s.WrappedChunk().Address().Bytes()) + } + + return h.Sum(nil)[:ChunkSumSize], nil +} + // IdentityAddress returns the internally used address for the chunk // since the single owner chunk address is not a unique identifier for the chunk, // but hashing the soc address and the wrapped chunk address is. diff --git a/pkg/storage/storage_test.go b/pkg/storage/storage_test.go index b273d26c316..d8efa089d20 100644 --- a/pkg/storage/storage_test.go +++ b/pkg/storage/storage_test.go @@ -4,13 +4,18 @@ package storage_test import ( + "bytes" "encoding/hex" "testing" "github.com/ethereum/go-ethereum/common" "github.com/ethersphere/bee/v2/pkg/cac" + "github.com/ethersphere/bee/v2/pkg/crypto" + "github.com/ethersphere/bee/v2/pkg/postage" + postagetesting "github.com/ethersphere/bee/v2/pkg/postage/testing" "github.com/ethersphere/bee/v2/pkg/soc" "github.com/ethersphere/bee/v2/pkg/storage" + testingc "github.com/ethersphere/bee/v2/pkg/storage/testing" "github.com/ethersphere/bee/v2/pkg/swarm" ) @@ -79,3 +84,106 @@ func TestIdentityAddress(t *testing.T) { } }) } + +func TestChunkSum(t *testing.T) { + t.Parallel() + + t.Run("cac is deterministic and 16 bytes", func(t *testing.T) { + t.Parallel() + ch := testingc.GenerateTestRandomChunk() + + sum, err := storage.ChunkSum(ch) + if err != nil { + t.Fatal(err) + } + if len(sum) != storage.ChunkSumSize { + t.Fatalf("expected sum length %d, got %d", storage.ChunkSumSize, len(sum)) + } + + again, err := storage.ChunkSum(ch) + if err != nil { + t.Fatal(err) + } + if !bytes.Equal(sum, again) { + t.Fatal("expected deterministic sum") + } + }) + + t.Run("divergent socs share address but produce different sums", func(t *testing.T) { + t.Parallel() + + privKey, err := crypto.GenerateSecp256k1Key() + if err != nil { + t.Fatal(err) + } + signer := crypto.NewDefaultSigner(privKey) + id := make([]byte, swarm.HashSize) + + inner1, err := cac.New([]byte("content-one")) + if err != nil { + t.Fatal(err) + } + inner2, err := cac.New([]byte("content-two")) + if err != nil { + t.Fatal(err) + } + + soc1, err := soc.New(id, inner1).Sign(signer) + if err != nil { + t.Fatal(err) + } + soc2, err := soc.New(id, inner2).Sign(signer) + if err != nil { + t.Fatal(err) + } + + // same owner and id yield the same SOC address despite different content + if !soc1.Address().Equal(soc2.Address()) { + t.Fatalf("expected equal soc addresses, got %s and %s", soc1.Address(), soc2.Address()) + } + + // stamp both with the same stamp so batchID and stampHash are identical; + // only the folded wrapped-CAC address should differ the sums. + stamp := postagetesting.MustNewStamp() + + sum1, err := storage.ChunkSum(soc1.WithStamp(stamp)) + if err != nil { + t.Fatal(err) + } + sum2, err := storage.ChunkSum(soc2.WithStamp(stamp)) + if err != nil { + t.Fatal(err) + } + + if bytes.Equal(sum1, sum2) { + t.Fatal("divergent socs must produce different sums") + } + }) + + t.Run("cac sum survives stamp marshal round-trip", func(t *testing.T) { + t.Parallel() + ch := testingc.GenerateTestRandomChunk() + + s1, err := storage.ChunkSum(ch) + if err != nil { + t.Fatal(err) + } + + raw, err := ch.Stamp().MarshalBinary() + if err != nil { + t.Fatal(err) + } + st := new(postage.Stamp) + if err := st.UnmarshalBinary(raw); err != nil { + t.Fatal(err) + } + + s2, err := storage.ChunkSum(swarm.NewChunk(ch.Address(), ch.Data()).WithStamp(st)) + if err != nil { + t.Fatal(err) + } + if !bytes.Equal(s1, s2) { + t.Fatal("sum must be stable across stamp marshal round-trip") + } + }) +} diff --git a/pkg/storer/compact_test.go b/pkg/storer/compact_test.go index 4d9a691008e..93b712ba8a3 100644 --- a/pkg/storer/compact_test.go +++ b/pkg/storer/compact_test.go @@ -13,6 +13,7 @@ import ( "github.com/ethersphere/bee/v2/pkg/postage" postagetesting "github.com/ethersphere/bee/v2/pkg/postage/testing" pullerMock "github.com/ethersphere/bee/v2/pkg/puller/mock" + "github.com/ethersphere/bee/v2/pkg/storage" chunk "github.com/ethersphere/bee/v2/pkg/storage/testing" "github.com/ethersphere/bee/v2/pkg/storer" "github.com/ethersphere/bee/v2/pkg/swarm" @@ -95,11 +96,11 @@ func TestCompact(t *testing.T) { } for _, ch := range chunks { - stampHash, err := ch.Stamp().Hash() + sum, err := storage.ChunkSum(ch) if err != nil { t.Fatal(err) } - has, err := st.ReserveHas(ch.Address(), ch.Stamp().BatchID(), stampHash) + has, err := st.ReserveHas(ch.Address(), sum) if err != nil { t.Fatal(err) } @@ -184,11 +185,11 @@ func TestCompactNoEvictions(t *testing.T) { } for _, ch := range chunks { - stampHash, err := ch.Stamp().Hash() + sum, err := storage.ChunkSum(ch) if err != nil { t.Fatal(err) } - has, err := st.ReserveHas(ch.Address(), ch.Stamp().BatchID(), stampHash) + has, err := st.ReserveHas(ch.Address(), sum) if err != nil { t.Fatal(err) } diff --git a/pkg/storer/internal/reserve/items.go b/pkg/storer/internal/reserve/items.go index e05b5c0bed4..d67291e2188 100644 --- a/pkg/storer/internal/reserve/items.go +++ b/pkg/storer/internal/reserve/items.go @@ -15,6 +15,7 @@ import ( var ( errMarshalInvalidAddress = errors.New("marshal: invalid address") + errMarshalInvalidSize = errors.New("marshal: invalid size") errUnmarshalInvalidSize = errors.New("unmarshal: invalid size") ) @@ -112,6 +113,7 @@ type ChunkBinItem struct { BatchID []byte StampHash []byte ChunkType swarm.ChunkType + Sum []byte // pullsync divergence checksum, storage.ChunkSumSize bytes } func (c *ChunkBinItem) Namespace() string { @@ -144,10 +146,11 @@ func (c *ChunkBinItem) Clone() storage.Item { BatchID: copyBytes(c.BatchID), StampHash: copyBytes(c.StampHash), ChunkType: c.ChunkType, + Sum: copyBytes(c.Sum), } } -const chunkBinItemSize = 1 + 8 + swarm.HashSize + swarm.HashSize + 1 + swarm.HashSize +const chunkBinItemSize = 1 + 8 + swarm.HashSize + swarm.HashSize + 1 + swarm.HashSize + storage.ChunkSumSize func (c *ChunkBinItem) Marshal() ([]byte, error) { if c.Address.IsZero() { @@ -173,6 +176,9 @@ func (c *ChunkBinItem) Marshal() ([]byte, error) { i += 1 copy(buf[i:i+swarm.HashSize], c.StampHash) + i += swarm.HashSize + + copy(buf[i:i+storage.ChunkSumSize], c.Sum) return buf, nil } @@ -198,6 +204,53 @@ func (c *ChunkBinItem) Unmarshal(buf []byte) error { i += 1 c.StampHash = copyBytes(buf[i : i+swarm.HashSize]) + i += swarm.HashSize + + c.Sum = copyBytes(buf[i : i+storage.ChunkSumSize]) + return nil +} + +// ChunkSumItem is an existence index keyed by chunk address and pullsync +// divergence checksum. It answers "do I already hold a chunk at this address +// with this exact content checksum?" in O(1) for the pullsync want-decision. +type ChunkSumItem struct { + Address swarm.Address + Sum []byte // storage.ChunkSumSize bytes +} + +func (c *ChunkSumItem) Namespace() string { return "chunkSum" } + +// address/sum +func (c *ChunkSumItem) ID() string { + return string(c.Address.Bytes()) + string(c.Sum) +} + +func (c *ChunkSumItem) String() string { + return path.Join(c.Namespace(), c.ID()) +} + +func (c *ChunkSumItem) Clone() storage.Item { + if c == nil { + return nil + } + return &ChunkSumItem{ + Address: c.Address.Clone(), + Sum: copyBytes(c.Sum), + } +} + +// existence-only index: the identity lives entirely in the key. +func (c *ChunkSumItem) Marshal() ([]byte, error) { + if c.Address.IsZero() { + return nil, errMarshalInvalidAddress + } + if len(c.Sum) != storage.ChunkSumSize { + return nil, errMarshalInvalidSize + } + return []byte{}, nil // existence-only +} + +func (c *ChunkSumItem) Unmarshal(_ []byte) error { return nil } diff --git a/pkg/storer/internal/reserve/items_test.go b/pkg/storer/internal/reserve/items_test.go index 19c22224d3a..ca948b0f9cb 100644 --- a/pkg/storer/internal/reserve/items_test.go +++ b/pkg/storer/internal/reserve/items_test.go @@ -43,6 +43,7 @@ func TestReserveItems(t *testing.T) { Bin: 9, BinID: 100, StampHash: storagetest.MaxAddressBytes[:], + Sum: storagetest.MaxAddressBytes[:storage.ChunkSumSize], }, Factory: func() storage.Item { return new(reserve.ChunkBinItem) }, }, diff --git a/pkg/storer/internal/reserve/reserve.go b/pkg/storer/internal/reserve/reserve.go index 5566946942e..45100ee80c5 100644 --- a/pkg/storer/internal/reserve/reserve.go +++ b/pkg/storer/internal/reserve/reserve.go @@ -119,6 +119,11 @@ func (r *Reserve) Put(ctx context.Context, chunk swarm.Chunk) error { chunkType := storage.ChunkType(chunk) + sum, err := storage.ChunkSum(chunk) + if err != nil { + return err + } + bin := swarm.Proximity(r.baseAddr.Bytes(), chunk.Address().Bytes()) // bin lock @@ -172,7 +177,7 @@ func (r *Reserve) Put(ctx context.Context, chunk swarm.Chunk) error { // delete old chunk index items err = errors.Join( s.IndexStore().Delete(oldBatchRadiusItem), - s.IndexStore().Delete(&ChunkBinItem{Bin: oldBatchRadiusItem.Bin, BinID: oldBatchRadiusItem.BinID}), + deleteChunkBinItem(s.IndexStore(), oldBatchRadiusItem.Bin, oldBatchRadiusItem.BinID), stampindex.Delete(s.IndexStore(), reserveScope, oldStamp), chunkstamp.DeleteWithStamp(s.IndexStore(), reserveScope, oldBatchRadiusItem.Address, oldStamp), ) @@ -202,7 +207,9 @@ func (r *Reserve) Put(ctx context.Context, chunk swarm.Chunk) error { BatchID: chunk.Stamp().BatchID(), ChunkType: chunkType, StampHash: stampHash, + Sum: sum, }), + s.IndexStore().Put(&ChunkSumItem{Address: chunk.Address(), Sum: sum}), ) if err != nil { return err @@ -256,7 +263,9 @@ func (r *Reserve) Put(ctx context.Context, chunk swarm.Chunk) error { BatchID: chunk.Stamp().BatchID(), ChunkType: chunkType, StampHash: stampHash, + Sum: sum, }), + s.IndexStore().Put(&ChunkSumItem{Address: chunk.Address(), Sum: sum}), ) if err != nil { return err @@ -302,6 +311,12 @@ func (r *Reserve) Has(addr swarm.Address, batchID []byte, stampHash []byte) (boo return r.st.IndexStore().Has(item) } +// HasSum reports whether the reserve holds a chunk at addr whose pullsync +// divergence checksum equals sum. Used by the pullsync want-decision. +func (r *Reserve) HasSum(addr swarm.Address, sum []byte) (bool, error) { + return r.st.IndexStore().Has(&ChunkSumItem{Address: addr, Sum: sum}) +} + func (r *Reserve) Get(ctx context.Context, addr swarm.Address, batchID []byte, stampHash []byte) (swarm.Chunk, error) { r.multx.Lock(string(batchID)) defer r.multx.Unlock(string(batchID)) @@ -448,6 +463,24 @@ func (r *Reserve) removeChunk( return RemoveChunkWithItem(ctx, trx, item) } +// deleteChunkBinItem removes the ChunkBinItem identified by (bin, binID) +// together with its companion ChunkSumItem, keeping the pullsync sum index +// consistent. It is a no-op if the ChunkBinItem does not exist. +func deleteChunkBinItem(store storage.IndexStore, bin uint8, binID uint64) error { + cbi := &ChunkBinItem{Bin: bin, BinID: binID} + err := store.Get(cbi) + if err != nil { + if errors.Is(err, storage.ErrNotFound) { + return nil + } + return err + } + return errors.Join( + store.Delete(cbi), + store.Delete(&ChunkSumItem{Address: cbi.Address, Sum: cbi.Sum}), + ) +} + func RemoveChunkWithItem( ctx context.Context, trx transaction.Store, @@ -465,7 +498,7 @@ func RemoveChunkWithItem( return errors.Join(errs, trx.IndexStore().Delete(item), - trx.IndexStore().Delete(&ChunkBinItem{Bin: item.Bin, BinID: item.BinID}), + deleteChunkBinItem(trx.IndexStore(), item.Bin, item.BinID), trx.ChunkStore().Delete(ctx, item.Address), ) } @@ -489,7 +522,7 @@ func RemoveChunkMetaData( return errors.Join(errs, trx.IndexStore().Delete(item), - trx.IndexStore().Delete(&ChunkBinItem{Bin: item.Bin, BinID: item.BinID}), + deleteChunkBinItem(trx.IndexStore(), item.Bin, item.BinID), ) } @@ -530,11 +563,11 @@ func DeleteCorruptedChunkMetadata(store storage.IndexStore, baseAddr swarm.Addre stampindex.Delete(store, reserveScope, stamp), chunkstamp.DeleteWithStamp(store, reserveScope, addr, stamp), store.Delete(batchRadiusItem), - store.Delete(&ChunkBinItem{Bin: bin, BinID: batchRadiusItem.BinID}), + deleteChunkBinItem(store, bin, batchRadiusItem.BinID), ) } -func (r *Reserve) IterateBin(bin uint8, startBinID uint64, cb func(swarm.Address, uint64, []byte, []byte) (bool, error)) error { +func (r *Reserve) IterateBin(bin uint8, startBinID uint64, cb func(swarm.Address, uint64, []byte, []byte, []byte) (bool, error)) error { err := r.st.IndexStore().Iterate(storage.Query{ Factory: func() storage.Item { return &ChunkBinItem{} }, Prefix: binIDToString(bin, startBinID), @@ -545,7 +578,7 @@ func (r *Reserve) IterateBin(bin uint8, startBinID uint64, cb func(swarm.Address return true, nil } - stop, err := cb(item.Address, item.BinID, item.BatchID, item.StampHash) + stop, err := cb(item.Address, item.BinID, item.BatchID, item.StampHash, item.Sum) if stop || err != nil { return true, err } @@ -631,7 +664,7 @@ func (r *Reserve) Reset(ctx context.Context) error { return errors.Join( s.ChunkStore().Delete(ctx, item.Address), s.IndexStore().Delete(item), - s.IndexStore().Delete(&ChunkBinItem{Bin: item.Bin, BinID: item.BinID}), + deleteChunkBinItem(s.IndexStore(), item.Bin, item.BinID), ) }) }) diff --git a/pkg/storer/internal/reserve/reserve_test.go b/pkg/storer/internal/reserve/reserve_test.go index d67ec34916e..4aced7160ad 100644 --- a/pkg/storer/internal/reserve/reserve_test.go +++ b/pkg/storer/internal/reserve/reserve_test.go @@ -768,7 +768,7 @@ func TestIterate(t *testing.T) { r := createReserve(t) var id uint64 = 1 - err := r.IterateBin(1, 0, func(ch swarm.Address, binID uint64, _, _ []byte) (bool, error) { + err := r.IterateBin(1, 0, func(ch swarm.Address, binID uint64, _, _, _ []byte) (bool, error) { if binID != id { t.Fatalf("got %d, want %d", binID, id) } diff --git a/pkg/storer/migration/all_steps.go b/pkg/storer/migration/all_steps.go index 118b6ae0d12..7198cf422ba 100644 --- a/pkg/storer/migration/all_steps.go +++ b/pkg/storer/migration/all_steps.go @@ -31,6 +31,7 @@ func AfterInitSteps( 5: legacyNoopStep, 6: legacyNoopStep, 7: legacyNoopStep, + 8: step_08(st, logger), } } diff --git a/pkg/storer/migration/step_08.go b/pkg/storer/migration/step_08.go new file mode 100644 index 00000000000..92653e4b53c --- /dev/null +++ b/pkg/storer/migration/step_08.go @@ -0,0 +1,110 @@ +// Copyright 2026 The Swarm Authors. All rights reserved. +// Use of this source code is governed by a BSD-style +// license that can be found in the LICENSE file. + +package migration + +import ( + "context" + "errors" + + "github.com/ethersphere/bee/v2/pkg/log" + "github.com/ethersphere/bee/v2/pkg/storage" + "github.com/ethersphere/bee/v2/pkg/storer/internal/chunkstamp" + "github.com/ethersphere/bee/v2/pkg/storer/internal/reserve" + "github.com/ethersphere/bee/v2/pkg/storer/internal/transaction" + "github.com/ethersphere/bee/v2/pkg/swarm" +) + +// step_08 backfills the pullsync divergence checksum (SWIP-101). The +// ChunkBinItem serialization grew by a trailing Sum field, so every existing +// ChunkBinItem is rewritten (reconstructed from the authoritative +// BatchRadiusItem, whose serialization is unchanged) with its Sum populated, +// and a companion ChunkSumItem existence row is created for the pullsync +// want-decision. Chunks missing from the chunkstore or with an invalid type are +// removed, consistent with the reserve repair procedure. +func step_08( + st transaction.Storage, + logger log.Logger, +) func() error { + return func() error { + logger.Info("starting pullsync chunk sum backfill migration; do not interrupt or kill the process...") + + var items []*reserve.BatchRadiusItem + err := st.IndexStore().Iterate(storage.Query{ + Factory: func() storage.Item { return &reserve.BatchRadiusItem{} }, + }, func(res storage.Result) (bool, error) { + items = append(items, res.Entry.(*reserve.BatchRadiusItem)) + return false, nil + }) + if err != nil { + return err + } + + logger.Info("counted reserve entries to backfill", "total_entries", len(items)) + + const batchSize = 1000 + removed := 0 + + for i := 0; i < len(items); i += batchSize { + end := min(i+batchSize, len(items)) + err := st.Run(context.Background(), func(s transaction.Store) error { + for _, item := range items[i:end] { + chunk, err := s.ChunkStore().Get(context.Background(), item.Address) + if err != nil { + if errors.Is(err, storage.ErrNotFound) { + removed++ + if err := reserve.RemoveChunkWithItem(context.Background(), s, item); err != nil { + return err + } + continue + } + return err + } + + chunkType := storage.ChunkType(chunk) + if chunkType == swarm.ChunkTypeUnspecified { + removed++ + if err := reserve.RemoveChunkWithItem(context.Background(), s, item); err != nil { + return err + } + continue + } + + stamp, err := chunkstamp.LoadWithStampHash(s.IndexStore(), "reserve", item.Address, item.StampHash) + if err != nil { + return err + } + + sum, err := storage.ChunkSum(chunk.WithStamp(stamp)) + if err != nil { + return err + } + + err = errors.Join( + s.IndexStore().Put(&reserve.ChunkBinItem{ + Bin: item.Bin, + BinID: item.BinID, + Address: item.Address, + BatchID: item.BatchID, + StampHash: item.StampHash, + ChunkType: chunkType, + Sum: sum, + }), + s.IndexStore().Put(&reserve.ChunkSumItem{Address: item.Address, Sum: sum}), + ) + if err != nil { + return err + } + } + return nil + }) + if err != nil { + return err + } + } + + logger.Info("pullsync chunk sum backfill complete", "backfilled", len(items)-removed, "removed", removed) + return nil + } +} diff --git a/pkg/storer/mock/mockreserve.go b/pkg/storer/mock/mockreserve.go index 7e8dfc11625..4ae3ba59d8e 100644 --- a/pkg/storer/mock/mockreserve.go +++ b/pkg/storer/mock/mockreserve.go @@ -5,6 +5,7 @@ package mockstorer import ( + "bytes" "context" "math/big" "sync" @@ -173,7 +174,7 @@ func (s *ReserveStore) SubscribeBin(ctx context.Context, bin uint8, start uint64 go func() { for _, c := range r.chunks { select { - case out <- &storer.BinC{Address: c.Address, BatchID: c.BatchID, BinID: c.BinID, StampHash: c.StampHash}: + case out <- &storer.BinC{Address: c.Address, BatchID: c.BatchID, BinID: c.BinID, StampHash: c.StampHash, Sum: c.Sum}: case <-ctx.Done(): select { case errC <- ctx.Err(): @@ -254,11 +255,22 @@ func (s *ReserveStore) put(_ context.Context, chs ...swarm.Chunk) error { } // Has chunks. -func (s *ReserveStore) ReserveHas(addr swarm.Address, batchID []byte, stampHash []byte) (bool, error) { - if _, ok := s.chunks[addr.String()+string(batchID)+string(stampHash)]; !ok { - return false, nil +func (s *ReserveStore) ReserveHas(addr swarm.Address, sum []byte) (bool, error) { + s.mtx.Lock() + defer s.mtx.Unlock() + for _, c := range s.chunks { + if !c.Address().Equal(addr) || c.Stamp() == nil { + continue + } + cs, err := storage.ChunkSum(c) + if err != nil { + return false, err + } + if bytes.Equal(cs, sum) { + return true, nil + } } - return true, nil + return false, nil } func (s *ReserveStore) ReserveSample(context.Context, []byte, uint8, uint64, *big.Int) (storer.Sample, error) { diff --git a/pkg/storer/reserve.go b/pkg/storer/reserve.go index b6799ea1cee..18f4cde63cc 100644 --- a/pkg/storer/reserve.go +++ b/pkg/storer/reserve.go @@ -290,7 +290,7 @@ func (db *DB) ReserveGet(ctx context.Context, addr swarm.Address, batchID []byte return db.reserve.Get(ctx, addr, batchID, stampHash) } -func (db *DB) ReserveHas(addr swarm.Address, batchID []byte, stampHash []byte) (has bool, err error) { +func (db *DB) ReserveHas(addr swarm.Address, sum []byte) (has bool, err error) { dur := captureDuration(time.Now()) defer func() { db.metrics.MethodCallsDuration.WithLabelValues("reserve", "ReserveHas").Observe(dur()) @@ -302,7 +302,7 @@ func (db *DB) ReserveHas(addr swarm.Address, batchID []byte, stampHash []byte) ( } }() - return db.reserve.Has(addr, batchID, stampHash) + return db.reserve.HasSum(addr, sum) } // ReservePutter returns a Putter for inserting chunks into the reserve. @@ -463,6 +463,7 @@ type BinC struct { BinID uint64 BatchID []byte StampHash []byte + Sum []byte } // SubscribeBin returns a channel that feeds all the chunks in the reserve from a certain bin between a start and end binIDs. @@ -478,9 +479,9 @@ func (db *DB) SubscribeBin(ctx context.Context, bin uint8, start uint64) (<-chan for { - err := db.reserve.IterateBin(bin, start, func(a swarm.Address, binID uint64, batchID, stampHash []byte) (bool, error) { + err := db.reserve.IterateBin(bin, start, func(a swarm.Address, binID uint64, batchID, stampHash, sum []byte) (bool, error) { select { - case out <- &BinC{Address: a, BinID: binID, BatchID: batchID, StampHash: stampHash}: + case out <- &BinC{Address: a, BinID: binID, BatchID: batchID, StampHash: stampHash, Sum: sum}: start = binID + 1 case <-done: return true, nil diff --git a/pkg/storer/reserve_test.go b/pkg/storer/reserve_test.go index 4c83df534dc..00e7e52cbdc 100644 --- a/pkg/storer/reserve_test.go +++ b/pkg/storer/reserve_test.go @@ -251,11 +251,11 @@ func TestEvictBatch(t *testing.T) { reserve := st.Reserve() for _, ch := range chunks { - stampHash, err := ch.Stamp().Hash() + sum, err := storage.ChunkSum(ch) if err != nil { t.Fatal(err) } - has, err := st.ReserveHas(ch.Address(), ch.Stamp().BatchID(), stampHash) + has, err := st.ReserveHas(ch.Address(), sum) if err != nil { t.Fatal(err) } @@ -347,11 +347,11 @@ func TestUnreserveCap(t *testing.T) { for po, chunks := range chunksPO { for _, ch := range chunks { - stampHash, err := ch.Stamp().Hash() + sum, err := storage.ChunkSum(ch) if err != nil { t.Fatal(err) } - has, err := storer.ReserveHas(ch.Address(), ch.Stamp().BatchID(), stampHash) + has, err := storer.ReserveHas(ch.Address(), sum) if err != nil { t.Fatal(err) } diff --git a/pkg/storer/storer.go b/pkg/storer/storer.go index 20711d91e42..a29ca38f512 100644 --- a/pkg/storer/storer.go +++ b/pkg/storer/storer.go @@ -153,7 +153,7 @@ type ReserveIterator interface { // content. It will implement all the core functionality required for the protocols. type ReserveStore interface { ReserveGet(ctx context.Context, addr swarm.Address, batchID []byte, stampHash []byte) (swarm.Chunk, error) - ReserveHas(addr swarm.Address, batchID []byte, stampHash []byte) (bool, error) + ReserveHas(addr swarm.Address, sum []byte) (bool, error) ReservePutter() storage.Putter SubscribeBin(ctx context.Context, bin uint8, start uint64) (<-chan *BinC, func(), <-chan error) ReserveLastBinIDs() ([]uint64, uint64, error) From fcc3d59cbc823d64d6969171ff616425e3acb47d Mon Sep 17 00:00:00 2001 From: Calin Martinconi Date: Mon, 20 Jul 2026 22:46:44 +0300 Subject: [PATCH 02/17] feat(storer): converge divergent SOCs in the storage layer (SWIP-101) Two valid single owner chunks can share an address, batch and stamp while wrapping different content. The stamp signs the chunk address, so both carry an identical stamp and produce an identical stamp hash. The content-blind existence check at the top of reserve.Put therefore treated the second chunk as already stored and dropped it, leaving each node holding whichever chunk reached it first. Neighborhoods never converged, and the reserve sampler computed different commitments from the same address. Make the check content-aware via the pullsync sum, and settle the divergence here rather than in the protocol: the chunk wrapping the lower CAC address wins. The rule depends only on the two payloads, so every node reaches the same answer regardless of arrival order. On a win the chunk is replaced in place, reusing the stamp index and stamp entries, which are identical for both. The bin ID is bumped so peers that already synced past the old one are offered the replacement. The reserve size is unchanged: one chunk goes in, one comes out. Pullsync treats a lost tie-break as an expected outcome rather than a sync error, since the node already holds the chunk the neighborhood converges on. Divergent chunks under different batches are not covered: they occupy different stamp indices, so no tie-break fires. --- pkg/pullsync/metrics.go | 7 + pkg/pullsync/pullsync.go | 8 + pkg/storage/storage.go | 40 +++++ pkg/storage/storage_test.go | 86 ++++++++++ pkg/storer/internal/reserve/reserve.go | 130 +++++++++++++-- pkg/storer/internal/reserve/reserve_test.go | 165 ++++++++++++++++++++ 6 files changed, 427 insertions(+), 9 deletions(-) diff --git a/pkg/pullsync/metrics.go b/pkg/pullsync/metrics.go index 57e26916c93..d20453d65ff 100644 --- a/pkg/pullsync/metrics.go +++ b/pkg/pullsync/metrics.go @@ -15,6 +15,7 @@ type metrics struct { MissingChunks prometheus.Counter // number of reserve get errs ReceivedZeroAddress prometheus.Counter // number of delivered chunks with invalid address ReceivedInvalidChunk prometheus.Counter // number of delivered chunks with invalid address + DivergentRejected prometheus.Counter // number of delivered chunks that lost the divergence tie-break Delivered prometheus.Counter // number of chunk deliveries SentOffered prometheus.Counter // number of chunks offered SentWanted prometheus.Counter // number of chunks wanted @@ -57,6 +58,12 @@ func newMetrics() metrics { Name: "received_invalid_chunks", Help: "Total invalid chunks delivered.", }), + DivergentRejected: prometheus.NewCounter(prometheus.CounterOpts{ + Namespace: m.Namespace, + Subsystem: subsystem, + Name: "divergent_rejected", + Help: "Total delivered chunks discarded for losing the divergence tie-break.", + }), Delivered: prometheus.NewCounter(prometheus.CounterOpts{ Namespace: m.Namespace, Subsystem: subsystem, diff --git a/pkg/pullsync/pullsync.go b/pkg/pullsync/pullsync.go index 92886bfd896..8c15f295d21 100644 --- a/pkg/pullsync/pullsync.go +++ b/pkg/pullsync/pullsync.go @@ -391,6 +391,14 @@ func (s *Syncer) Sync(ctx context.Context, peer swarm.Address, bin uint8, start chunkErr = errors.Join(chunkErr, err) continue } + // the chunk diverged from the one already stored and lost the + // tie-break. The neighborhood converges on the stored chunk, so + // this is an expected outcome rather than a sync error. + if errors.Is(err, storage.ErrDivergentChunkRejected) { + s.logger.Debug("divergent chunk rejected", "error", err, "peer_address", peer, "chunk", c) + s.metrics.DivergentRejected.Inc() + continue + } return 0, 0, errors.Join(chunkErr, err) } chunksPut++ diff --git a/pkg/storage/storage.go b/pkg/storage/storage.go index a30a24470e7..360999a8188 100644 --- a/pkg/storage/storage.go +++ b/pkg/storage/storage.go @@ -5,6 +5,7 @@ package storage import ( + "bytes" "context" "errors" "fmt" @@ -19,6 +20,12 @@ import ( var ( ErrOverwriteNewerChunk = errors.New("overwriting chunk with newer timestamp") ErrUnknownChunkType = errors.New("unknown chunk type") + + // ErrDivergentChunkRejected is returned when a chunk that diverges from an + // already stored one at the same address, batch and stamp loses the + // deterministic tie-break and is therefore not stored. It is not a failure: + // the node already holds the chunk the whole neighborhood converges on. + ErrDivergentChunkRejected = errors.New("divergent chunk rejected by tie-break") ) // Result represents the item returned by the read operation, which returns @@ -327,6 +334,39 @@ func ChunkSum(ch swarm.Chunk) ([]byte, error) { return h.Sum(nil)[:ChunkSumSize], nil } +// DivergentChunkWins reports whether the incoming chunk should replace the +// stored one when the two share an address, batch and stamp but wrap different +// content. Both chunks must be single owner chunks; a content addressed chunk +// cannot diverge, since its address is the hash of its own payload. +// +// The winner is the chunk wrapping the lexicographically lower CAC address. +// The rule depends on nothing but the two payloads, so every node in the +// neighborhood converges on the same chunk regardless of the order in which +// they arrive. +func DivergentChunkWins(stored, incoming swarm.Chunk) (bool, error) { + storedAddr, err := wrappedAddress(stored) + if err != nil { + return false, fmt.Errorf("stored chunk: %w", err) + } + incomingAddr, err := wrappedAddress(incoming) + if err != nil { + return false, fmt.Errorf("incoming chunk: %w", err) + } + return bytes.Compare(incomingAddr.Bytes(), storedAddr.Bytes()) < 0, nil +} + +// wrappedAddress returns the address of the CAC wrapped by a single owner chunk. +func wrappedAddress(ch swarm.Chunk) (swarm.Address, error) { + if !soc.Valid(ch) { + return swarm.ZeroAddress, fmt.Errorf("%w: not a single owner chunk", ErrUnknownChunkType) + } + s, err := soc.FromChunk(ch) + if err != nil { + return swarm.ZeroAddress, fmt.Errorf("soc from chunk: %w", err) + } + return s.WrappedChunk().Address(), nil +} + // IdentityAddress returns the internally used address for the chunk // since the single owner chunk address is not a unique identifier for the chunk, // but hashing the soc address and the wrapped chunk address is. diff --git a/pkg/storage/storage_test.go b/pkg/storage/storage_test.go index d8efa089d20..0181b84669e 100644 --- a/pkg/storage/storage_test.go +++ b/pkg/storage/storage_test.go @@ -6,6 +6,7 @@ package storage_test import ( "bytes" "encoding/hex" + "errors" "testing" "github.com/ethereum/go-ethereum/common" @@ -187,3 +188,88 @@ func TestChunkSum(t *testing.T) { } }) } + +func TestDivergentChunkWins(t *testing.T) { + t.Parallel() + + privKey, err := crypto.GenerateSecp256k1Key() + if err != nil { + t.Fatal(err) + } + signer := crypto.NewDefaultSigner(privKey) + id := make([]byte, swarm.HashSize) + + newSOC := func(data string) swarm.Chunk { + t.Helper() + inner, err := cac.New([]byte(data)) + if err != nil { + t.Fatal(err) + } + ch, err := soc.New(id, inner).Sign(signer) + if err != nil { + t.Fatal(err) + } + return ch.WithStamp(postagetesting.MustNewStamp()) + } + + lower, higher := newSOC("content-one"), newSOC("content-two") + lowerInner, err := soc.UnwrapCAC(lower) + if err != nil { + t.Fatal(err) + } + higherInner, err := soc.UnwrapCAC(higher) + if err != nil { + t.Fatal(err) + } + if bytes.Compare(lowerInner.Address().Bytes(), higherInner.Address().Bytes()) > 0 { + lower, higher = higher, lower + } + + t.Run("lower wrapped address wins", func(t *testing.T) { + t.Parallel() + + wins, err := storage.DivergentChunkWins(higher, lower) + if err != nil { + t.Fatal(err) + } + if !wins { + t.Fatal("expected the chunk wrapping the lower cac address to win") + } + }) + + t.Run("tie-break is antisymmetric", func(t *testing.T) { + t.Parallel() + + wins, err := storage.DivergentChunkWins(lower, higher) + if err != nil { + t.Fatal(err) + } + if wins { + t.Fatal("expected the chunk wrapping the higher cac address to lose") + } + }) + + t.Run("a chunk does not displace itself", func(t *testing.T) { + t.Parallel() + + wins, err := storage.DivergentChunkWins(lower, lower) + if err != nil { + t.Fatal(err) + } + if wins { + t.Fatal("expected an identical chunk not to win") + } + }) + + t.Run("content addressed chunks cannot diverge", func(t *testing.T) { + t.Parallel() + + cac := testingc.GenerateTestRandomChunk() + if _, err := storage.DivergentChunkWins(cac, lower); !errors.Is(err, storage.ErrUnknownChunkType) { + t.Fatalf("expected ErrUnknownChunkType, got %v", err) + } + if _, err := storage.DivergentChunkWins(lower, cac); !errors.Is(err, storage.ErrUnknownChunkType) { + t.Fatalf("expected ErrUnknownChunkType, got %v", err) + } + }) +} diff --git a/pkg/storer/internal/reserve/reserve.go b/pkg/storer/internal/reserve/reserve.go index 45100ee80c5..03b9556294a 100644 --- a/pkg/storer/internal/reserve/reserve.go +++ b/pkg/storer/internal/reserve/reserve.go @@ -108,15 +108,6 @@ func (r *Reserve) Put(ctx context.Context, chunk swarm.Chunk) error { return err } - // check if the chunk with the same batch, stamp timestamp and index is already stored - has, err := r.Has(chunk.Address(), chunk.Stamp().BatchID(), stampHash) - if err != nil { - return err - } - if has { - return nil - } - chunkType := storage.ChunkType(chunk) sum, err := storage.ChunkSum(chunk) @@ -126,6 +117,27 @@ func (r *Reserve) Put(ctx context.Context, chunk swarm.Chunk) error { bin := swarm.Proximity(r.baseAddr.Bytes(), chunk.Address().Bytes()) + // check if the chunk with the same batch, stamp timestamp and index is already stored + has, err := r.Has(chunk.Address(), chunk.Stamp().BatchID(), stampHash) + if err != nil { + return err + } + if has { + // Address, batch and stamp all match, but two single owner chunks can + // share those and still wrap different content. The sum tells them + // apart: if it matches we already hold this exact chunk, otherwise the + // chunks diverge and a tie-break decides which one the neighborhood + // keeps. + hasSum, err := r.HasSum(chunk.Address(), sum) + if err != nil { + return err + } + if hasSum { + return nil + } + return r.resolveDivergence(ctx, chunk, sum, stampHash, bin, chunkType) + } + // bin lock r.multx.Lock(strconv.Itoa(int(bin))) defer r.multx.Unlock(strconv.Itoa(int(bin))) @@ -306,6 +318,106 @@ func (r *Reserve) Put(ctx context.Context, chunk swarm.Chunk) error { return nil } +// resolveDivergence settles two single owner chunks that share an address, +// batch and stamp but wrap different content. Both are individually valid, so +// the protocol cannot pick between them; the choice is made here, in the +// storage layer, by a tie-break that depends only on the two payloads. Every +// node in the neighborhood therefore converges on the same chunk no matter +// which one it received first. +// +// If the incoming chunk wins it replaces the stored one in place, reusing the +// existing stamp index and stamp entries, which are identical for both. The +// bin ID is bumped so that peers which already synced past the old bin ID are +// offered the replacement, propagating the resolution outwards. +// +// The reserve size is unchanged either way: one chunk goes in, one comes out. +func (r *Reserve) resolveDivergence( + ctx context.Context, + chunk swarm.Chunk, + sum []byte, + stampHash []byte, + bin uint8, + chunkType swarm.ChunkType, +) error { + // bin lock + r.multx.Lock(strconv.Itoa(int(bin))) + defer r.multx.Unlock(strconv.Itoa(int(bin))) + + return r.st.Run(ctx, func(s transaction.Store) error { + stored, err := s.ChunkStore().Get(ctx, chunk.Address()) + if err != nil { + return fmt.Errorf("failed loading diverging chunk %s: %w", chunk.Address(), err) + } + + wins, err := storage.DivergentChunkWins(stored, chunk) + if err != nil { + return fmt.Errorf("divergence tie-break for chunk %s: %w", chunk.Address(), err) + } + + if !wins { + r.logger.Debug( + "discarding diverging chunk", + "address", chunk.Address(), + "batch_id", hex.EncodeToString(chunk.Stamp().BatchID()), + ) + return fmt.Errorf("diverging chunk %s lost tie-break: %w", chunk.Address(), storage.ErrDivergentChunkRejected) + } + + item := &BatchRadiusItem{ + Bin: bin, + Address: chunk.Address(), + BatchID: chunk.Stamp().BatchID(), + StampHash: stampHash, + } + // load item to get the binID of the chunk being replaced + if err := s.IndexStore().Get(item); err != nil { + return err + } + + // drop the bin and sum entries of the replaced chunk + if err := deleteChunkBinItem(s.IndexStore(), item.Bin, item.BinID); err != nil { + return err + } + + binID, err := r.IncBinID(s.IndexStore(), bin) + if err != nil { + return err + } + + r.logger.Debug( + "replacing diverging chunk", + "address", chunk.Address(), + "batch_id", hex.EncodeToString(chunk.Stamp().BatchID()), + "old_bin_id", item.BinID, + "new_bin_id", binID, + ) + + // the BatchRadiusItem key does not cover the binID, so putting it again + // with the new binID overwrites the existing entry. + item.BinID = binID + err = errors.Join( + s.IndexStore().Put(item), + s.IndexStore().Put(&ChunkBinItem{ + Bin: bin, + BinID: binID, + Address: chunk.Address(), + BatchID: chunk.Stamp().BatchID(), + ChunkType: chunkType, + StampHash: stampHash, + Sum: sum, + }), + s.IndexStore().Put(&ChunkSumItem{Address: chunk.Address(), Sum: sum}), + ) + if err != nil { + return err + } + + // swap the payload without touching the reference count: the chunk + // store entry is reused, only its content changes. + return s.ChunkStore().Replace(ctx, chunk, false) + }) +} + func (r *Reserve) Has(addr swarm.Address, batchID []byte, stampHash []byte) (bool, error) { item := &BatchRadiusItem{Bin: swarm.Proximity(r.baseAddr.Bytes(), addr.Bytes()), BatchID: batchID, Address: addr, StampHash: stampHash} return r.st.IndexStore().Has(item) diff --git a/pkg/storer/internal/reserve/reserve_test.go b/pkg/storer/internal/reserve/reserve_test.go index 4aced7160ad..6d193370256 100644 --- a/pkg/storer/internal/reserve/reserve_test.go +++ b/pkg/storer/internal/reserve/reserve_test.go @@ -1161,3 +1161,168 @@ func checkChunkInIndexStore(t *testing.T, s storage.Reader, bin uint8, binId uin checkStore(t, s, &reserve.BatchRadiusItem{Bin: bin, BatchID: ch.Stamp().BatchID(), Address: ch.Address(), StampHash: stampHash}, false) checkStore(t, s, &reserve.ChunkBinItem{Bin: bin, BinID: binId, StampHash: stampHash}, false) } + +// TestSOCDivergence covers two single owner chunks that share an address, batch +// and stamp while wrapping different content. Both are valid, so the storage +// layer settles which one the neighborhood keeps, and it must reach the same +// answer regardless of the order the chunks arrive in. +func TestSOCDivergence(t *testing.T) { + t.Parallel() + + ctx := context.Background() + signer := getSigner(t) + batch := postagetesting.MustNewBatch() + + // same signer and id, different payloads: same SOC address, different + // wrapped CAC. + s1 := soctesting.GenerateMockSocWithSigner(t, []byte("data"), signer) + s2 := soctesting.GenerateMockSocWithSigner(t, []byte("update"), signer) + + // the stamp signs the chunk address, which both chunks share, so a single + // stamp legitimately applies to both. + stamp := postagetesting.MustNewFields(batch.ID, 0, 1) + ch1 := s1.Chunk().WithStamp(stamp) + ch2 := s2.Chunk().WithStamp(stamp) + + if !ch1.Address().Equal(ch2.Address()) { + t.Fatal("expected the diverging chunks to share an address") + } + + // the chunk wrapping the lower CAC address is the one both nodes must keep. + winner, loser := ch1, ch2 + if bytes.Compare(s2.WrappedChunk.Address().Bytes(), s1.WrappedChunk.Address().Bytes()) < 0 { + winner, loser = ch2, ch1 + } + + for _, tc := range []struct { + name string + order []swarm.Chunk + }{ + {"winner first", []swarm.Chunk{winner, loser}}, + {"loser first", []swarm.Chunk{loser, winner}}, + } { + t.Run(tc.name, func(t *testing.T) { + t.Parallel() + + baseAddr := swarm.RandAddress(t) + ts := internal.NewInmemStorage() + r, err := reserve.New(baseAddr, ts, 0, kademlia.NewTopologyDriver(), log.Noop) + if err != nil { + t.Fatal(err) + } + + if err := r.Put(ctx, tc.order[0]); err != nil { + t.Fatal(err) + } + sizeAfterFirst := r.Size() + + err = r.Put(ctx, tc.order[1]) + // the second put only errors when the incoming chunk is the loser. + if tc.order[1] == loser { + if !errors.Is(err, storage.ErrDivergentChunkRejected) { + t.Fatalf("expected ErrDivergentChunkRejected, got %v", err) + } + } else if err != nil { + t.Fatal(err) + } + + // whichever order they arrived in, the winner is what is stored. + stored, err := ts.ChunkStore().Get(ctx, winner.Address()) + if err != nil { + t.Fatal(err) + } + if !bytes.Equal(stored.Data(), winner.Data()) { + t.Fatal("expected the tie-break winner to be stored") + } + + // divergence resolution replaces a chunk, it does not add one. + if got := r.Size(); got != sizeAfterFirst { + t.Fatalf("expected reserve size to stay %d, got %d", sizeAfterFirst, got) + } + + // the sum index tracks the stored chunk only. + winnerSum, err := storage.ChunkSum(winner) + if err != nil { + t.Fatal(err) + } + loserSum, err := storage.ChunkSum(loser) + if err != nil { + t.Fatal(err) + } + has, err := r.HasSum(winner.Address(), winnerSum) + if err != nil { + t.Fatal(err) + } + if !has { + t.Fatal("expected the winner sum to be indexed") + } + has, err = r.HasSum(loser.Address(), loserSum) + if err != nil { + t.Fatal(err) + } + if has { + t.Fatal("expected the loser sum not to be indexed") + } + }) + } +} + +// TestSOCDivergenceBumpsBinID asserts that replacing a diverging chunk moves it +// to the top of its bin, so peers that already synced past the old bin ID are +// offered the replacement. +func TestSOCDivergenceBumpsBinID(t *testing.T) { + t.Parallel() + + ctx := context.Background() + baseAddr := swarm.RandAddress(t) + ts := internal.NewInmemStorage() + r, err := reserve.New(baseAddr, ts, 0, kademlia.NewTopologyDriver(), log.Noop) + if err != nil { + t.Fatal(err) + } + + signer := getSigner(t) + batch := postagetesting.MustNewBatch() + s1 := soctesting.GenerateMockSocWithSigner(t, []byte("data"), signer) + s2 := soctesting.GenerateMockSocWithSigner(t, []byte("update"), signer) + stamp := postagetesting.MustNewFields(batch.ID, 0, 1) + ch1 := s1.Chunk().WithStamp(stamp) + ch2 := s2.Chunk().WithStamp(stamp) + + // put the losing chunk first so the second put performs the replacement. + first, second := ch1, ch2 + if bytes.Compare(s2.WrappedChunk.Address().Bytes(), s1.WrappedChunk.Address().Bytes()) > 0 { + first, second = ch2, ch1 + } + + if err := r.Put(ctx, first); err != nil { + t.Fatal(err) + } + + bin := swarm.Proximity(baseAddr.Bytes(), first.Address().Bytes()) + stampHash, err := stamp.Hash() + if err != nil { + t.Fatal(err) + } + + item := &reserve.BatchRadiusItem{Bin: bin, BatchID: batch.ID, Address: first.Address(), StampHash: stampHash} + if err := ts.IndexStore().Get(item); err != nil { + t.Fatal(err) + } + oldBinID := item.BinID + + if err := r.Put(ctx, second); err != nil { + t.Fatal(err) + } + + if err := ts.IndexStore().Get(item); err != nil { + t.Fatal(err) + } + if item.BinID <= oldBinID { + t.Fatalf("expected bin id to be bumped past %d, got %d", oldBinID, item.BinID) + } + + // the stale bin entry must be gone, leaving exactly one per bin id. + checkStore(t, ts.IndexStore(), &reserve.ChunkBinItem{Bin: bin, BinID: oldBinID}, true) + checkStore(t, ts.IndexStore(), &reserve.ChunkBinItem{Bin: bin, BinID: item.BinID}, false) +} From d2bc63997e9c1c25e45f8265a64d97c837294a56 Mon Sep 17 00:00:00 2001 From: Calin Martinconi Date: Fri, 24 Jul 2026 10:25:45 +0300 Subject: [PATCH 03/17] fix(storer): harden chunk sum migration, repair and index maintenance (SWIP-101) Review fixes for the pullsync chunk checksum change, addressing upgrade safety, repair integration and sum index consistency. The critical fix concerns deleteChunkBinItem: it read the stored record before deleting it, but during the Sum backfill migration and the sharky recovery that runs before it, on-disk ChunkBinItems still carry the pre-Sum serialization and fail to unmarshal. The migration's own removal branch for dangling entries then failed the whole migration, leaving the node unable to start, with the repair command blocked behind the same migration. An undecodable record now gets a key-only delete; it predates the sum index, so no companion entry exists. Replacing a single owner chunk's payload now refreshes the divergence checksums of co-resident entries under other stamps. The payload is stored once per address while index entries exist per stamp, so a replacement used to leave sibling sums advertising content the node no longer holds. The refresh runs after the put transaction under each sibling's batch lock, recomputing from the committed payload so concurrent replacements converge. Migration and repair hardening: step_08 pages through the reserve index in fixed windows instead of loading it whole, removes legacy entries with an unset stamp hash, derives sums from the batch ID and stamp hash already on the index item so stamps are never loaded, and sweeps orphaned pre-Sum ChunkBinItems by raw key so no old-format record survives to break later iterations. The reserve repairer now rebuilds the sum index alongside the chunk bin items, sweeps stale sum entries, and deletes chunk bin items key-only so undecodable values cannot wedge it. Also: ChunkBinItem.Marshal validates the sum length, received offers are rejected on a malformed sum length after the zero-address skip, and pullsync.pb.go is properly regenerated with gogo/protobuf v1.3.2 so the embedded descriptor matches the proto definition. The migration is covered by a new test seeding genuine pre-Sum records, including the dangling-entry case, and the sibling refresh by a test covering both replacement paths; both fail against the unfixed code. --- pkg/pullsync/pb/pullsync.pb.go | 41 ++- pkg/pullsync/pullsync.go | 3 + pkg/storage/storage.go | 11 +- pkg/storer/internal/chunkstamp/chunkstamp.go | 17 ++ pkg/storer/internal/reserve/items.go | 16 ++ pkg/storer/internal/reserve/reserve.go | 174 ++++++++++++- pkg/storer/internal/reserve/reserve_test.go | 165 +++++++++++++ pkg/storer/migration/export_test.go | 7 + pkg/storer/migration/reserveRepair.go | 85 ++++++- pkg/storer/migration/reserveRepair_test.go | 55 ++++- pkg/storer/migration/step_08.go | 86 +++++-- pkg/storer/migration/step_08_test.go | 247 +++++++++++++++++++ 12 files changed, 840 insertions(+), 67 deletions(-) create mode 100644 pkg/storer/migration/export_test.go create mode 100644 pkg/storer/migration/step_08_test.go diff --git a/pkg/pullsync/pb/pullsync.pb.go b/pkg/pullsync/pb/pullsync.pb.go index 89dc6ea44bb..b1ad4cf5059 100644 --- a/pkg/pullsync/pb/pullsync.pb.go +++ b/pkg/pullsync/pb/pullsync.pb.go @@ -383,27 +383,26 @@ func init() { func init() { proto.RegisterFile("pullsync.proto", fileDescriptor_d1dee042cf9c065c) } var fileDescriptor_d1dee042cf9c065c = []byte{ - // 319 bytes of a gzipped FileDescriptorProto - 0x1f, 0x8b, 0x08, 0x00, 0x00, 0x00, 0x00, 0x00, 0x02, 0xff, 0x7c, 0x91, 0xbb, 0x4e, 0xc3, 0x30, - 0x14, 0x86, 0xeb, 0x3a, 0x29, 0xe5, 0x50, 0x01, 0xb2, 0x18, 0x32, 0x54, 0x51, 0x64, 0x21, 0x91, - 0x85, 0x0e, 0x20, 0x1e, 0xa0, 0x69, 0x11, 0x97, 0x01, 0x24, 0x17, 0x81, 0x60, 0x73, 0xd3, 0x54, - 0xa9, 0x68, 0x63, 0xcb, 0x76, 0x91, 0xfa, 0x16, 0x3c, 0x16, 0x63, 0x47, 0x46, 0xd4, 0xbc, 0x08, - 0x8a, 0x1b, 0xd3, 0x8d, 0xed, 0x7c, 0xe7, 0xf6, 0xfb, 0x3f, 0x86, 0x43, 0xb9, 0x9c, 0xcf, 0xf5, - 0xaa, 0x48, 0x7b, 0x52, 0x09, 0x23, 0x48, 0xdb, 0x31, 0xf5, 0x01, 0x8f, 0x56, 0x05, 0xbd, 0x02, - 0xdc, 0x4f, 0xdf, 0x49, 0x00, 0x7b, 0x83, 0xa5, 0xd2, 0x42, 0xe9, 0x00, 0x45, 0x38, 0xf6, 0x98, - 0x43, 0x72, 0x02, 0xfe, 0xb5, 0x14, 0x69, 0x1e, 0x34, 0x23, 0x14, 0x7b, 0x6c, 0x0b, 0xf4, 0x1c, - 0xf0, 0x4d, 0x66, 0xc8, 0x31, 0xe0, 0x64, 0x56, 0x04, 0x28, 0x42, 0xb1, 0xcf, 0xaa, 0xb0, 0x6a, - 0x1f, 0x19, 0xae, 0x8c, 0x6b, 0xb7, 0x40, 0x5f, 0xc1, 0x1f, 0xe4, 0xcb, 0xc2, 0xea, 0xf4, 0x27, - 0x13, 0x95, 0x69, 0x6d, 0x87, 0x3a, 0xcc, 0x61, 0x55, 0x49, 0xb8, 0x49, 0xf3, 0xbb, 0xa1, 0x1d, - 0xed, 0x30, 0x87, 0xa4, 0x0b, 0xfb, 0x23, 0xc3, 0x17, 0xf2, 0x96, 0xeb, 0x3c, 0xc0, 0xb6, 0xb6, - 0x4b, 0xd0, 0x7b, 0xf0, 0x1f, 0xa7, 0xd3, 0x4c, 0x55, 0x0b, 0x9e, 0x84, 0x5c, 0x08, 0x6d, 0xec, - 0x6a, 0x8f, 0x39, 0x24, 0x67, 0xd0, 0xb2, 0xea, 0x3a, 0x68, 0x46, 0x38, 0x3e, 0xb8, 0x38, 0xea, - 0xfd, 0x5d, 0xc5, 0xe6, 0x59, 0x5d, 0xa6, 0xa7, 0xe0, 0xbd, 0xf0, 0xc2, 0x54, 0x8a, 0xc9, 0xcc, - 0x3c, 0x67, 0xa9, 0x11, 0xaa, 0x7e, 0xe7, 0x2e, 0x41, 0x1f, 0xa0, 0x3d, 0xcc, 0xe6, 0xb3, 0x8f, - 0x4c, 0xad, 0xfe, 0xf1, 0x43, 0xc0, 0x1b, 0x72, 0xc3, 0x6b, 0x33, 0x36, 0xae, 0x8f, 0xb3, 0x90, - 0xb5, 0x8b, 0x2d, 0x24, 0xdd, 0xaf, 0x4d, 0x88, 0xd6, 0x9b, 0x10, 0xfd, 0x6c, 0x42, 0xf4, 0x59, - 0x86, 0x8d, 0x75, 0x19, 0x36, 0xbe, 0xcb, 0xb0, 0xf1, 0xd6, 0x94, 0xe3, 0x71, 0xcb, 0x7e, 0xdc, - 0xe5, 0x6f, 0x00, 0x00, 0x00, 0xff, 0xff, 0xd3, 0x02, 0x2f, 0x9a, 0xca, 0x01, 0x00, 0x00, + // 304 bytes of a gzipped FileDescriptorProto + 0x1f, 0x8b, 0x08, 0x00, 0x00, 0x00, 0x00, 0x00, 0x02, 0xff, 0x7c, 0x91, 0xcf, 0x4a, 0xf3, 0x40, + 0x14, 0xc5, 0x3b, 0x9d, 0xa4, 0x5f, 0xbf, 0x6b, 0x51, 0x19, 0x5c, 0xcc, 0xa2, 0x0c, 0x61, 0x10, + 0xcc, 0xc6, 0x2e, 0x2c, 0x3e, 0x40, 0xff, 0x88, 0xe0, 0x42, 0x61, 0x2a, 0x0a, 0xee, 0xd2, 0x74, + 0x4a, 0x8b, 0x6d, 0x66, 0x98, 0x99, 0x08, 0x7d, 0x0b, 0x1f, 0xcb, 0x65, 0x97, 0x2e, 0x25, 0x79, + 0x11, 0xc9, 0x34, 0xd1, 0x9d, 0xbb, 0xf3, 0x3b, 0xb9, 0xe7, 0xe6, 0x5c, 0x06, 0x8e, 0x75, 0xbe, + 0xd9, 0xd8, 0x5d, 0x96, 0x0e, 0xb4, 0x51, 0x4e, 0x91, 0x6e, 0xc3, 0x3c, 0x04, 0x3c, 0xdb, 0x65, + 0xfc, 0x1a, 0xf0, 0x28, 0x7d, 0x25, 0x14, 0xfe, 0x4d, 0x72, 0x63, 0x95, 0xb1, 0x14, 0x45, 0x38, + 0x0e, 0x44, 0x83, 0xe4, 0x0c, 0xc2, 0x1b, 0xad, 0xd2, 0x15, 0x6d, 0x47, 0x28, 0x0e, 0xc4, 0x01, + 0xf8, 0x25, 0xe0, 0x5b, 0xe9, 0xc8, 0x29, 0xe0, 0xf1, 0x3a, 0xa3, 0x28, 0x42, 0x71, 0x28, 0x2a, + 0x59, 0x8d, 0xcf, 0x5c, 0x62, 0x5c, 0x33, 0xee, 0x81, 0x0f, 0x21, 0x9c, 0xac, 0xf2, 0xcc, 0xff, + 0x67, 0xb4, 0x58, 0x18, 0x69, 0xad, 0x0f, 0xf5, 0x44, 0x83, 0xd5, 0xaa, 0x59, 0xbe, 0xf5, 0xb1, + 0x9e, 0xa8, 0x24, 0xbf, 0x83, 0xf0, 0x61, 0xb9, 0x94, 0xa6, 0x0a, 0x3d, 0x2a, 0xbd, 0x55, 0xd6, + 0xf9, 0x50, 0x20, 0x1a, 0x24, 0x17, 0xd0, 0xf1, 0x7b, 0x2d, 0x6d, 0x47, 0x38, 0x3e, 0xba, 0x3a, + 0x19, 0xfc, 0xdc, 0xeb, 0x7d, 0x51, 0x7f, 0xe6, 0xe7, 0x10, 0x3c, 0x27, 0x99, 0x23, 0x7d, 0xf8, + 0x3f, 0x5e, 0xbb, 0x27, 0x99, 0x3a, 0x65, 0xea, 0x06, 0xbf, 0x06, 0xbf, 0x87, 0xee, 0x54, 0x6e, + 0xd6, 0x6f, 0xd2, 0xec, 0xfe, 0x68, 0x4a, 0x20, 0x98, 0x26, 0x2e, 0xa9, 0xab, 0x7a, 0x5d, 0x9f, + 0xbd, 0xd5, 0x14, 0x7b, 0xf3, 0x00, 0xe3, 0xfe, 0x47, 0xc1, 0xd0, 0xbe, 0x60, 0xe8, 0xab, 0x60, + 0xe8, 0xbd, 0x64, 0xad, 0x7d, 0xc9, 0x5a, 0x9f, 0x25, 0x6b, 0xbd, 0xb4, 0xf5, 0x7c, 0xde, 0xf1, + 0x4f, 0x32, 0xfc, 0x0e, 0x00, 0x00, 0xff, 0xff, 0xb8, 0x0e, 0xff, 0x3e, 0xa4, 0x01, 0x00, 0x00, } func (m *Syn) Marshal() (dAtA []byte, err error) { diff --git a/pkg/pullsync/pullsync.go b/pkg/pullsync/pullsync.go index 92886bfd896..53ac6d7628a 100644 --- a/pkg/pullsync/pullsync.go +++ b/pkg/pullsync/pullsync.go @@ -287,6 +287,9 @@ func (s *Syncer) Sync(ctx context.Context, peer swarm.Address, bin uint8, start s.logger.Debug("syncer got a zero address hash on offer", "peer_address", peer) continue } + if len(sum) != storage.ChunkSumSize { + return 0, 0, fmt.Errorf("inconsistent chunk sum length") + } s.metrics.Offered.Inc() if s.store.IsWithinStorageRadius(a) { have, err = s.store.ReserveHas(a, sum) diff --git a/pkg/storage/storage.go b/pkg/storage/storage.go index a30a24470e7..044cd674814 100644 --- a/pkg/storage/storage.go +++ b/pkg/storage/storage.go @@ -311,9 +311,18 @@ func ChunkSum(ch swarm.Chunk) ([]byte, error) { if err != nil { return nil, fmt.Errorf("stamp hash: %w", err) } + return ChunkSumFromParts(ch.Stamp().BatchID(), stampHash, ch) +} +// ChunkSumFromParts computes the pullsync divergence checksum from an already +// known batch ID and stamp hash. It is equivalent to ChunkSum for a chunk +// carrying a matching stamp, but spares callers that hold the parts in an +// index item (migrations, repairs) from loading and rehashing the stamp. The +// chunk is consulted only to fold in the wrapped CAC address for single owner +// chunks. +func ChunkSumFromParts(batchID, stampHash []byte, ch swarm.Chunk) ([]byte, error) { h := swarm.NewHasher() - _, _ = h.Write(ch.Stamp().BatchID()) + _, _ = h.Write(batchID) _, _ = h.Write(stampHash) if soc.Valid(ch) { diff --git a/pkg/storer/internal/chunkstamp/chunkstamp.go b/pkg/storer/internal/chunkstamp/chunkstamp.go index 16b7f011c3e..87e52acd7f9 100644 --- a/pkg/storer/internal/chunkstamp/chunkstamp.go +++ b/pkg/storer/internal/chunkstamp/chunkstamp.go @@ -147,6 +147,23 @@ func Load(s storage.Reader, scope string, addr swarm.Address) (swarm.Stamp, erro return LoadWithBatchID(s, scope, addr, nil) } +// IterateAll iterates all stamps stored for the given scope and address. +func IterateAll(s storage.Reader, scope string, addr swarm.Address, fn func(swarm.Stamp) (bool, error)) error { + return s.Iterate( + storage.Query{ + Factory: func() storage.Item { + return &Item{ + scope: []byte(scope), + address: addr, + } + }, + }, + func(res storage.Result) (bool, error) { + return fn(res.Entry.(*Item).stamp) + }, + ) +} + // LoadWithBatchID returns swarm.Stamp related to the given address and batchID. func LoadWithBatchID(s storage.Reader, scope string, addr swarm.Address, batchID []byte) (swarm.Stamp, error) { var stamp swarm.Stamp diff --git a/pkg/storer/internal/reserve/items.go b/pkg/storer/internal/reserve/items.go index d67291e2188..9ce384233cd 100644 --- a/pkg/storer/internal/reserve/items.go +++ b/pkg/storer/internal/reserve/items.go @@ -7,6 +7,7 @@ package reserve import ( "encoding/binary" "errors" + "fmt" "path" "github.com/ethersphere/bee/v2/pkg/storage" @@ -131,6 +132,18 @@ func binIDToString(bin uint8, binID uint64) string { return string(bin) + string(binIDBytes) } +const chunkBinIDLength = 1 + 8 + +// ParseChunkBinID parses a raw ChunkBinItem key ID, the inverse of +// binIDToString. It allows key-only deletion of entries whose stored value +// cannot be unmarshaled, such as records left in a pre-Sum serialization. +func ParseChunkBinID(id string) (bin uint8, binID uint64, err error) { + if len(id) != chunkBinIDLength { + return 0, 0, fmt.Errorf("reserve: invalid chunkBin key length %d", len(id)) + } + return id[0], binary.BigEndian.Uint64([]byte(id[1:])), nil +} + func (c *ChunkBinItem) String() string { return path.Join(c.Namespace(), c.ID()) } @@ -156,6 +169,9 @@ func (c *ChunkBinItem) Marshal() ([]byte, error) { if c.Address.IsZero() { return nil, errMarshalInvalidAddress } + if len(c.Sum) != storage.ChunkSumSize { + return nil, errMarshalInvalidSize + } buf := make([]byte, chunkBinItemSize) i := 0 diff --git a/pkg/storer/internal/reserve/reserve.go b/pkg/storer/internal/reserve/reserve.go index 45100ee80c5..c8ebf6862dd 100644 --- a/pkg/storer/internal/reserve/reserve.go +++ b/pkg/storer/internal/reserve/reserve.go @@ -5,6 +5,7 @@ package reserve import ( + "bytes" "context" "encoding/binary" "encoding/hex" @@ -99,29 +100,48 @@ func New( // 3. A new chunk that has the same address belonging to the same stamp index with an already stored chunk will overwrite the existing chunk // if the new chunk has a higher stamp timestamp (regardless of batch type and chunk type, eg CAC & SOC). func (r *Reserve) Put(ctx context.Context, chunk swarm.Chunk) error { + socReplaced, err := r.putChunk(ctx, chunk) + if err != nil { + return err + } + if socReplaced { + // A single owner chunk's payload is stored once per address while index + // entries exist per stamp: replacing the payload invalidates the + // divergence checksums of co-resident entries under other stamps. The + // refresh runs after the put transaction, with no locks held, because + // it takes the sibling entries' batch locks (see refreshSiblingSums). + return r.refreshSiblingSums(ctx, chunk.Address()) + } + return nil +} + +// putChunk stores the chunk and reports whether the shared payload of an +// already stored single owner chunk was replaced, in which case the sums of +// co-resident entries must be refreshed by the caller. +func (r *Reserve) putChunk(ctx context.Context, chunk swarm.Chunk) (socReplaced bool, err error) { // batchID lock, Put vs Eviction r.multx.Lock(string(chunk.Stamp().BatchID())) defer r.multx.Unlock(string(chunk.Stamp().BatchID())) stampHash, err := chunk.Stamp().Hash() if err != nil { - return err + return false, err } // check if the chunk with the same batch, stamp timestamp and index is already stored has, err := r.Has(chunk.Address(), chunk.Stamp().BatchID(), stampHash) if err != nil { - return err + return false, err } if has { - return nil + return false, nil } chunkType := storage.ChunkType(chunk) sum, err := storage.ChunkSum(chunk) if err != nil { - return err + return false, err } bin := swarm.Proximity(r.baseAddr.Bytes(), chunk.Address().Bytes()) @@ -217,6 +237,7 @@ func (r *Reserve) Put(ctx context.Context, chunk swarm.Chunk) error { if chunkType == swarm.ChunkTypeSingleOwner { r.logger.Debug("replacing soc in chunkstore", "address", chunk.Address()) + socReplaced = true return s.ChunkStore().Replace(ctx, chunk, false) } @@ -279,6 +300,7 @@ func (r *Reserve) Put(ctx context.Context, chunk swarm.Chunk) error { } if has { r.logger.Debug("replacing soc in chunkstore", "address", chunk.Address()) + socReplaced = true err = s.ChunkStore().Replace(ctx, chunk, true) } else { err = s.ChunkStore().Put(ctx, chunk) @@ -298,11 +320,101 @@ func (r *Reserve) Put(ctx context.Context, chunk swarm.Chunk) error { return nil }) if err != nil { - return err + return false, err } if shouldIncReserveSize { r.size.Add(1) } + return socReplaced, nil +} + +// refreshSiblingSums recomputes the divergence checksum of every reserve entry +// at the given address after its shared payload was replaced. Without the +// refresh, entries under other stamps keep advertising content the node no +// longer holds (peers reject the deliveries as unsolicited) and keep matching +// offers for content it cannot store. +// +// It must be called with no reserve locks held: each sibling entry is updated +// under its own batch lock, one at a time, mirroring the Put-vs-Eviction lock +// discipline. The sum is recomputed from the currently committed payload +// rather than the caller's chunk, so concurrent replacements converge on the +// content that committed last. Entries whose sum already matches, including +// the caller's own, are left untouched. +func (r *Reserve) refreshSiblingSums(ctx context.Context, addr swarm.Address) error { + bin := swarm.Proximity(r.baseAddr.Bytes(), addr.Bytes()) + + // collect first: the underlying stores do not support writes during an + // iteration. A stamp deleted between collection and its locked update is + // skipped when its index entry turns up missing. + var stamps []swarm.Stamp + err := chunkstamp.IterateAll(r.st.IndexStore(), reserveScope, addr, func(stamp swarm.Stamp) (bool, error) { + stamps = append(stamps, stamp) + return false, nil + }) + if err != nil { + return fmt.Errorf("iterate stamps for %s: %w", addr, err) + } + + for _, stamp := range stamps { + stampHash, err := stamp.Hash() + if err != nil { + return err + } + + err = func() error { + // batchID lock, refresh vs Put/Eviction + r.multx.Lock(string(stamp.BatchID())) + defer r.multx.Unlock(string(stamp.BatchID())) + + return r.st.Run(ctx, func(s transaction.Store) error { + chunk, err := s.ChunkStore().Get(ctx, addr) + if err != nil { + if errors.Is(err, storage.ErrNotFound) { + return nil + } + return err + } + + sum, err := storage.ChunkSumFromParts(stamp.BatchID(), stampHash, chunk) + if err != nil { + return err + } + + item := &BatchRadiusItem{Bin: bin, BatchID: stamp.BatchID(), Address: addr, StampHash: stampHash} + err = s.IndexStore().Get(item) + if err != nil { + if errors.Is(err, storage.ErrNotFound) { + return nil + } + return err + } + + cbi := &ChunkBinItem{Bin: bin, BinID: item.BinID} + err = s.IndexStore().Get(cbi) + if err != nil { + if errors.Is(err, storage.ErrNotFound) { + return nil + } + return err + } + + if bytes.Equal(cbi.Sum, sum) { + return nil + } + + oldSum := cbi.Sum + cbi.Sum = sum + return errors.Join( + s.IndexStore().Delete(&ChunkSumItem{Address: addr, Sum: oldSum}), + s.IndexStore().Put(cbi), + s.IndexStore().Put(&ChunkSumItem{Address: addr, Sum: sum}), + ) + }) + }() + if err != nil { + return err + } + } return nil } @@ -473,6 +585,13 @@ func deleteChunkBinItem(store storage.IndexStore, bin uint8, binID uint64) error if errors.Is(err, storage.ErrNotFound) { return nil } + // The stored value predates the Sum field (pre-migration record, seen + // during the Sum backfill migration and the sharky recovery that runs + // before it). Such a record cannot have a companion ChunkSumItem, so + // deleting by key alone is complete. + if errors.Is(err, errUnmarshalInvalidSize) { + return store.Delete(&ChunkBinItem{Bin: bin, BinID: binID}) + } return err } return errors.Join( @@ -481,6 +600,51 @@ func deleteChunkBinItem(store storage.IndexStore, bin uint8, binID uint64) error ) } +// RemoveMalformedChunkBinItems deletes chunkBin entries whose stored value +// does not match the current serialization, without unmarshaling them. Such +// entries are orphaned pre-Sum records (a ChunkBinItem without a matching +// BatchRadiusItem is never rewritten by the Sum backfill migration) and would +// otherwise fail every full iteration of the namespace: the reserve sampler, +// pullsync bin subscriptions and the reserve repairer. They have no companion +// ChunkSumItem to remove. Returns the number of entries deleted. +func RemoveMalformedChunkBinItems(ctx context.Context, st transaction.Storage) (int, error) { + var malformed []*ChunkBinItem + err := st.IndexStore().Iterate(storage.Query{ + Factory: func() storage.Item { return &ChunkBinItem{} }, + ItemProperty: storage.QueryItemSize, + }, func(res storage.Result) (bool, error) { + if res.Size == chunkBinItemSize { + return false, nil + } + bin, binID, err := ParseChunkBinID(res.ID) + if err != nil { + return false, err + } + malformed = append(malformed, &ChunkBinItem{Bin: bin, BinID: binID}) + return false, nil + }) + if err != nil { + return 0, err + } + + const batchSize = 1000 + for i := 0; i < len(malformed); i += batchSize { + end := min(i+batchSize, len(malformed)) + err := st.Run(ctx, func(s transaction.Store) error { + for _, item := range malformed[i:end] { + if err := s.IndexStore().Delete(item); err != nil { + return err + } + } + return nil + }) + if err != nil { + return 0, err + } + } + return len(malformed), nil +} + func RemoveChunkWithItem( ctx context.Context, trx transaction.Store, diff --git a/pkg/storer/internal/reserve/reserve_test.go b/pkg/storer/internal/reserve/reserve_test.go index 4aced7160ad..48c8c7c47f2 100644 --- a/pkg/storer/internal/reserve/reserve_test.go +++ b/pkg/storer/internal/reserve/reserve_test.go @@ -1161,3 +1161,168 @@ func checkChunkInIndexStore(t *testing.T, s storage.Reader, bin uint8, binId uin checkStore(t, s, &reserve.BatchRadiusItem{Bin: bin, BatchID: ch.Stamp().BatchID(), Address: ch.Address(), StampHash: stampHash}, false) checkStore(t, s, &reserve.ChunkBinItem{Bin: bin, BinID: binId, StampHash: stampHash}, false) } + +// TestChunkSumIndexLockstep asserts the invariant the pullsync want-decision +// depends on: a ChunkSumItem exists exactly as long as its chunk is in the +// reserve. A stale entry would make the node silently refuse to sync a chunk +// it no longer holds. +func TestChunkSumIndexLockstep(t *testing.T) { + t.Parallel() + + ctx := context.Background() + baseAddr := swarm.RandAddress(t) + ts := internal.NewInmemStorage() + r, err := reserve.New(baseAddr, ts, 0, kademlia.NewTopologyDriver(), log.Noop) + if err != nil { + t.Fatal(err) + } + + batch := postagetesting.MustNewBatch() + ch := chunk.GenerateTestRandomChunkAt(t, baseAddr, 0).WithStamp(postagetesting.MustNewBatchStamp(batch.ID)) + if err := r.Put(ctx, ch); err != nil { + t.Fatal(err) + } + + sum, err := storage.ChunkSum(ch) + if err != nil { + t.Fatal(err) + } + + has, err := r.HasSum(ch.Address(), sum) + if err != nil { + t.Fatal(err) + } + if !has { + t.Fatal("expected chunk sum to be indexed after put") + } + + evicted, err := r.EvictBatchBin(ctx, batch.ID, math.MaxInt, swarm.MaxBins) + if err != nil { + t.Fatal(err) + } + if evicted != 1 { + t.Fatalf("expected 1 chunk evicted, got %d", evicted) + } + + has, err = r.HasSum(ch.Address(), sum) + if err != nil { + t.Fatal(err) + } + if has { + t.Fatal("expected chunk sum index entry to be removed by eviction") + } +} + +// TestSOCSiblingSumRefresh covers a single owner chunk address stored under +// multiple batches. The payload is stored once per address, so replacing it +// through one batch's entry must refresh the divergence checksums of the other +// batches' entries; a stale sum would keep advertising content the node no +// longer holds and keep matching offers for content it cannot store. +func TestSOCSiblingSumRefresh(t *testing.T) { + t.Parallel() + + ctx := context.Background() + baseAddr := swarm.RandAddress(t) + ts := internal.NewInmemStorage() + r, err := reserve.New(baseAddr, ts, 0, kademlia.NewTopologyDriver(), log.Noop) + if err != nil { + t.Fatal(err) + } + + signer := getSigner(t) + batchA := postagetesting.MustNewBatch() + batchB := postagetesting.MustNewBatch() + + // same owner and id: all three chunks share one SOC address + s1 := soctesting.GenerateMockSocWithSigner(t, []byte("v1"), signer) + s2 := soctesting.GenerateMockSocWithSigner(t, []byte("v2"), signer) + s3 := soctesting.GenerateMockSocWithSigner(t, []byte("v3"), signer) + + stampA := postagetesting.MustNewFields(batchA.ID, 0, 1) + stampB := postagetesting.MustNewFields(batchB.ID, 0, 1) + chA := s1.Chunk().WithStamp(stampA) + chB := s2.Chunk().WithStamp(stampB) + + addr := chA.Address() + bin := swarm.Proximity(baseAddr.Bytes(), addr.Bytes()) + + stampHashA, err := stampA.Hash() + if err != nil { + t.Fatal(err) + } + stampHashB, err := stampB.Hash() + if err != nil { + t.Fatal(err) + } + + // sumOf reads the sum stored on the ChunkBinItem of the entry identified + // by the given batch and stamp hash. + sumOf := func(batchID, stampHash []byte) []byte { + t.Helper() + item := &reserve.BatchRadiusItem{Bin: bin, BatchID: batchID, Address: addr, StampHash: stampHash} + if err := ts.IndexStore().Get(item); err != nil { + t.Fatal(err) + } + cbi := &reserve.ChunkBinItem{Bin: bin, BinID: item.BinID} + if err := ts.IndexStore().Get(cbi); err != nil { + t.Fatal(err) + } + return cbi.Sum + } + + if err := r.Put(ctx, chA); err != nil { + t.Fatal(err) + } + staleSumA := sumOf(batchA.ID, stampHashA) + + // storing the same address under batch B replaces the shared payload with + // v2; batch A's entry must be re-summed against the new payload. + if err := r.Put(ctx, chB); err != nil { + t.Fatal(err) + } + + freshSumA, err := storage.ChunkSumFromParts(batchA.ID, stampHashA, chB) + if err != nil { + t.Fatal(err) + } + if !bytes.Equal(sumOf(batchA.ID, stampHashA), freshSumA) { + t.Fatal("expected batch A entry to be re-summed against the replacing payload") + } + has, err := r.HasSum(addr, staleSumA) + if err != nil { + t.Fatal(err) + } + if has { + t.Fatal("expected the stale sum of batch A to be dropped") + } + has, err = r.HasSum(addr, freshSumA) + if err != nil { + t.Fatal(err) + } + if !has { + t.Fatal("expected the refreshed sum of batch A to be indexed") + } + + // the same-batch replacement path (higher stamp timestamp) must refresh + // batch B's entry the same way. + staleSumB := sumOf(batchB.ID, stampHashB) + chA2 := s3.Chunk().WithStamp(postagetesting.MustNewFields(batchA.ID, 0, 2)) + if err := r.Put(ctx, chA2); err != nil { + t.Fatal(err) + } + + freshSumB, err := storage.ChunkSumFromParts(batchB.ID, stampHashB, chA2) + if err != nil { + t.Fatal(err) + } + if !bytes.Equal(sumOf(batchB.ID, stampHashB), freshSumB) { + t.Fatal("expected batch B entry to be re-summed against the replacing payload") + } + has, err = r.HasSum(addr, staleSumB) + if err != nil { + t.Fatal(err) + } + if has { + t.Fatal("expected the stale sum of batch B to be dropped") + } +} diff --git a/pkg/storer/migration/export_test.go b/pkg/storer/migration/export_test.go new file mode 100644 index 00000000000..0a64d64656a --- /dev/null +++ b/pkg/storer/migration/export_test.go @@ -0,0 +1,7 @@ +// Copyright 2026 The Swarm Authors. All rights reserved. +// Use of this source code is governed by a BSD-style +// license that can be found in the LICENSE file. + +package migration + +var Step08 = step_08 diff --git a/pkg/storer/migration/reserveRepair.go b/pkg/storer/migration/reserveRepair.go index 67cee2a17f8..82e9f210723 100644 --- a/pkg/storer/migration/reserveRepair.go +++ b/pkg/storer/migration/reserveRepair.go @@ -32,9 +32,9 @@ func ReserveRepairer( /* STEP 0: remove epoch item STEP 1: remove all of the BinItem entries - STEP 2: remove all of the ChunkBinItem entries + STEP 2: remove all of the ChunkBinItem and ChunkSumItem entries STEP 3: iterate BatchRadiusItem, get new binID - create new ChunkBinItem and BatchRadiusItem if the chunk exists in the chunkstore + create new ChunkBinItem, ChunkSumItem and BatchRadiusItem if the chunk exists in the chunkstore if the chunk is invalid, it is removed from the chunkstore STEP 4: save the latest binID to disk */ @@ -98,14 +98,21 @@ func ReserveRepairer( logger.Info("removed all bin index entries") // STEP 2 + // key-only iteration: the values are all deleted anyway, and a record + // with an undecodable value (e.g. a pre-Sum serialization leftover) + // must not be able to wedge the repair tool. var chunkBinItems []*reserve.ChunkBinItem err = st.IndexStore().Iterate( storage.Query{ - Factory: func() storage.Item { return &reserve.ChunkBinItem{} }, + Factory: func() storage.Item { return &reserve.ChunkBinItem{} }, + ItemProperty: storage.QueryItemID, }, func(res storage.Result) (bool, error) { - item := res.Entry.(*reserve.ChunkBinItem) - chunkBinItems = append(chunkBinItems, item) + bin, binID, err := reserve.ParseChunkBinID(res.ID) + if err != nil { + return false, err + } + chunkBinItems = append(chunkBinItems, &reserve.ChunkBinItem{Bin: bin, BinID: binID}) return false, nil }, ) @@ -133,6 +140,49 @@ func ReserveRepairer( logger.Info("removed all chunk bin items", "total_entries", len(chunkBinItems)) chunkBinItems = nil + // remove all chunk sum items; they are rebuilt in step 3 alongside the + // chunk bin items, and a full sweep also clears any orphaned entries. A + // ChunkSumItem's identity lives entirely in its key, so the entries are + // parsed from the raw key instead of unmarshaled. + var chunkSumItems []*reserve.ChunkSumItem + err = st.IndexStore().Iterate( + storage.Query{ + Factory: func() storage.Item { return &reserve.ChunkSumItem{} }, + ItemProperty: storage.QueryItemID, + }, + func(res storage.Result) (bool, error) { + if len(res.ID) != swarm.HashSize+storage.ChunkSumSize { + return false, fmt.Errorf("unexpected chunk sum key length %d", len(res.ID)) + } + chunkSumItems = append(chunkSumItems, &reserve.ChunkSumItem{ + Address: swarm.NewAddress([]byte(res.ID[:swarm.HashSize])), + Sum: []byte(res.ID[swarm.HashSize:]), + }) + return false, nil + }, + ) + if err != nil { + return err + } + + for i := 0; i < len(chunkSumItems); i += batchSize { + end := min(i+batchSize, len(chunkSumItems)) + err := st.Run(context.Background(), func(s transaction.Store) error { + for _, item := range chunkSumItems[i:end] { + err := s.IndexStore().Delete(item) + if err != nil { + return err + } + } + return nil + }) + if err != nil { + return err + } + } + logger.Info("removed all chunk sum items", "total_entries", len(chunkSumItems)) + chunkSumItems = nil + // STEP 3 var batchRadiusItems []*reserve.BatchRadiusItem err = st.IndexStore().Iterate( @@ -209,14 +259,23 @@ func ReserveRepairer( return err } - return s.IndexStore().Put(&reserve.ChunkBinItem{ - BatchID: item.BatchID, - Bin: item.Bin, - Address: item.Address, - BinID: item.BinID, - StampHash: item.StampHash, - ChunkType: chunkType, - }) + sum, err := storage.ChunkSumFromParts(item.BatchID, item.StampHash, chunk) + if err != nil { + return err + } + + return errors.Join( + s.IndexStore().Put(&reserve.ChunkBinItem{ + BatchID: item.BatchID, + Bin: item.Bin, + Address: item.Address, + BinID: item.BinID, + StampHash: item.StampHash, + ChunkType: chunkType, + Sum: sum, + }), + s.IndexStore().Put(&reserve.ChunkSumItem{Address: item.Address, Sum: sum}), + ) }) }) }(item) diff --git a/pkg/storer/migration/reserveRepair_test.go b/pkg/storer/migration/reserveRepair_test.go index 273d9555025..e6e8bd9db0d 100644 --- a/pkg/storer/migration/reserveRepair_test.go +++ b/pkg/storer/migration/reserveRepair_test.go @@ -5,6 +5,7 @@ package migration_test import ( + "bytes" "context" "errors" "testing" @@ -55,6 +56,8 @@ func TestReserveRepair(t *testing.T) { BatchID: ch.Stamp().BatchID(), ChunkType: swarm.ChunkTypeContentAddressed, StampHash: stampHash, + // zeroed sum: the repair must recompute it + Sum: make([]byte, storage.ChunkSumSize), } err = store.Run(context.Background(), func(s transaction.Store) error { return s.IndexStore().Put(cb) @@ -97,17 +100,65 @@ func TestReserveRepair(t *testing.T) { } } + // seed a stale chunk sum entry with no backing chunk; the repair sweep + // must remove it, otherwise the node would refuse to sync that chunk. + staleSum := &reserve.ChunkSumItem{ + Address: swarm.RandAddress(t), + Sum: make([]byte, storage.ChunkSumSize), + } + err := store.Run(context.Background(), func(s transaction.Store) error { + return s.IndexStore().Put(staleSum) + }) + assert.NoError(t, err) + assert.NoError(t, stepFn()) + has, err := store.IndexStore().Has(staleSum) + assert.NoError(t, err) + if has { + t.Fatal("expected stale chunk sum item to be removed by repair") + } + + // every surviving chunk must have its sum rebuilt in both indexes. + for b := 2; b < 5; b++ { + for _, ch := range chunksPO[b] { + sum, err := storage.ChunkSum(ch) + assert.NoError(t, err) + has, err := store.IndexStore().Has(&reserve.ChunkSumItem{Address: ch.Address(), Sum: sum}) + assert.NoError(t, err) + if !has { + t.Fatalf("expected chunk sum item for chunk %s after repair", ch.Address()) + } + } + } + + chunkByAddr := make(map[string]swarm.Chunk) + for b := 2; b < 5; b++ { + for _, ch := range chunksPO[b] { + chunkByAddr[ch.Address().ByteString()] = ch + } + } + binIDs := make(map[uint8][]uint64) cbCount := 0 - err := store.IndexStore().Iterate( + err = store.IndexStore().Iterate( storage.Query{Factory: func() storage.Item { return &reserve.ChunkBinItem{} }}, func(res storage.Result) (stop bool, err error) { cb := res.Entry.(*reserve.ChunkBinItem) if cb.ChunkType != swarm.ChunkTypeContentAddressed { return false, errors.New("chunk type should be content addressed") } + ch, ok := chunkByAddr[cb.Address.ByteString()] + if !ok { + return false, errors.New("chunk bin item for unknown chunk") + } + sum, err := storage.ChunkSum(ch) + if err != nil { + return false, err + } + if !bytes.Equal(cb.Sum, sum) { + return false, errors.New("chunk bin item sum not recomputed by repair") + } binIDs[cb.Bin] = append(binIDs[cb.Bin], cb.BinID) cbCount++ return false, nil @@ -148,7 +199,7 @@ func TestReserveRepair(t *testing.T) { assert.Equal(t, cbCount, brCount) - has, err := store.IndexStore().Has(&reserve.EpochItem{}) + has, err = store.IndexStore().Has(&reserve.EpochItem{}) if has { t.Fatal("epoch item should be deleted") } diff --git a/pkg/storer/migration/step_08.go b/pkg/storer/migration/step_08.go index 92653e4b53c..df8c5e44723 100644 --- a/pkg/storer/migration/step_08.go +++ b/pkg/storer/migration/step_08.go @@ -5,12 +5,12 @@ package migration import ( + "bytes" "context" "errors" "github.com/ethersphere/bee/v2/pkg/log" "github.com/ethersphere/bee/v2/pkg/storage" - "github.com/ethersphere/bee/v2/pkg/storer/internal/chunkstamp" "github.com/ethersphere/bee/v2/pkg/storer/internal/reserve" "github.com/ethersphere/bee/v2/pkg/storer/internal/transaction" "github.com/ethersphere/bee/v2/pkg/swarm" @@ -21,8 +21,18 @@ import ( // ChunkBinItem is rewritten (reconstructed from the authoritative // BatchRadiusItem, whose serialization is unchanged) with its Sum populated, // and a companion ChunkSumItem existence row is created for the pullsync -// want-decision. Chunks missing from the chunkstore or with an invalid type are -// removed, consistent with the reserve repair procedure. +// want-decision. The sum is derived from the batch ID and stamp hash already +// carried by the BatchRadiusItem, so stamps are never loaded. Chunks missing +// from the chunkstore, with an invalid type or with an unset stamp hash are +// removed, consistent with the reserve repair procedure. Finally, orphaned +// pre-migration ChunkBinItems (no matching BatchRadiusItem, hence never +// rewritten) are swept by raw key so no old-format record survives to break +// later iterations. +// +// The BatchRadiusItem namespace is paged through in fixed windows instead of +// being loaded whole: at reserve capacity the full index is millions of +// entries, and the underlying stores do not support writes during an +// iteration. func step_08( st transaction.Storage, logger log.Logger, @@ -30,26 +40,38 @@ func step_08( return func() error { logger.Info("starting pullsync chunk sum backfill migration; do not interrupt or kill the process...") - var items []*reserve.BatchRadiusItem - err := st.IndexStore().Iterate(storage.Query{ - Factory: func() storage.Item { return &reserve.BatchRadiusItem{} }, - }, func(res storage.Result) (bool, error) { - items = append(items, res.Entry.(*reserve.BatchRadiusItem)) - return false, nil - }) - if err != nil { - return err - } + const pageSize = 1000 - logger.Info("counted reserve entries to backfill", "total_entries", len(items)) + backfilled, removed := 0, 0 + lastID := "" - const batchSize = 1000 - removed := 0 + for { + var items []*reserve.BatchRadiusItem + err := st.IndexStore().Iterate(storage.Query{ + Factory: func() storage.Item { return &reserve.BatchRadiusItem{} }, + Prefix: lastID, + PrefixAtStart: true, + }, func(res storage.Result) (bool, error) { + // the resume key may have been deleted in the previous window, + // in which case iteration lands on the next entry; matching on + // the ID rather than skipping the first result unconditionally + // avoids silently dropping that entry. + if res.ID == lastID { + return false, nil + } + items = append(items, res.Entry.(*reserve.BatchRadiusItem)) + return len(items) >= pageSize, nil + }) + if err != nil { + return err + } + if len(items) == 0 { + break + } + lastID = items[len(items)-1].ID() - for i := 0; i < len(items); i += batchSize { - end := min(i+batchSize, len(items)) - err := st.Run(context.Background(), func(s transaction.Store) error { - for _, item := range items[i:end] { + err = st.Run(context.Background(), func(s transaction.Store) error { + for _, item := range items { chunk, err := s.ChunkStore().Get(context.Background(), item.Address) if err != nil { if errors.Is(err, storage.ErrNotFound) { @@ -71,12 +93,20 @@ func step_08( continue } - stamp, err := chunkstamp.LoadWithStampHash(s.IndexStore(), "reserve", item.Address, item.StampHash) - if err != nil { - return err + // a legacy entry with an unset stamp hash would get a sum no + // peer can ever compute; remove it and let sync restore the + // chunk with proper stamp data. + if bytes.Equal(item.StampHash, swarm.EmptyAddress.Bytes()) { + removed++ + if err := reserve.RemoveChunkWithItem(context.Background(), s, item); err != nil { + return err + } + continue } - sum, err := storage.ChunkSum(chunk.WithStamp(stamp)) + // the sum only needs the batch ID and stamp hash, both already + // on the item, so the stamp itself is never loaded. + sum, err := storage.ChunkSumFromParts(item.BatchID, item.StampHash, chunk) if err != nil { return err } @@ -96,6 +126,7 @@ func step_08( if err != nil { return err } + backfilled++ } return nil }) @@ -104,7 +135,12 @@ func step_08( } } - logger.Info("pullsync chunk sum backfill complete", "backfilled", len(items)-removed, "removed", removed) + swept, err := reserve.RemoveMalformedChunkBinItems(context.Background(), st) + if err != nil { + return err + } + + logger.Info("pullsync chunk sum backfill complete", "backfilled", backfilled, "removed", removed, "swept_orphans", swept) return nil } } diff --git a/pkg/storer/migration/step_08_test.go b/pkg/storer/migration/step_08_test.go new file mode 100644 index 00000000000..249ac9c9490 --- /dev/null +++ b/pkg/storer/migration/step_08_test.go @@ -0,0 +1,247 @@ +// Copyright 2026 The Swarm Authors. All rights reserved. +// Use of this source code is governed by a BSD-style +// license that can be found in the LICENSE file. + +package migration_test + +import ( + "bytes" + "context" + "encoding/binary" + "errors" + "path" + "testing" + + "github.com/ethersphere/bee/v2/pkg/cac" + "github.com/ethersphere/bee/v2/pkg/log" + postagetesting "github.com/ethersphere/bee/v2/pkg/postage/testing" + soctesting "github.com/ethersphere/bee/v2/pkg/soc/testing" + "github.com/ethersphere/bee/v2/pkg/storage" + chunktest "github.com/ethersphere/bee/v2/pkg/storage/testing" + "github.com/ethersphere/bee/v2/pkg/storer/internal" + "github.com/ethersphere/bee/v2/pkg/storer/internal/chunkstamp" + "github.com/ethersphere/bee/v2/pkg/storer/internal/reserve" + "github.com/ethersphere/bee/v2/pkg/storer/internal/transaction" + localmigration "github.com/ethersphere/bee/v2/pkg/storer/migration" + "github.com/ethersphere/bee/v2/pkg/swarm" +) + +// legacyChunkBinItem writes the pre-Sum, 106-byte ChunkBinItem serialization +// under the chunkBin namespace, byte-compatible with the on-disk format the +// migration upgrades from. +type legacyChunkBinItem struct { + bin uint8 + binID uint64 + address swarm.Address + batchID []byte + chunkType swarm.ChunkType + stampHash []byte +} + +func (l *legacyChunkBinItem) Namespace() string { return "chunkBin" } + +func (l *legacyChunkBinItem) ID() string { + binIDBytes := make([]byte, 8) + binary.BigEndian.PutUint64(binIDBytes, l.binID) + return string(l.bin) + string(binIDBytes) +} + +func (l *legacyChunkBinItem) String() string { return path.Join(l.Namespace(), l.ID()) } + +func (l *legacyChunkBinItem) Marshal() ([]byte, error) { + buf := make([]byte, 1+8+swarm.HashSize+swarm.HashSize+1+swarm.HashSize) + i := 0 + buf[i] = l.bin + i += 1 + binary.BigEndian.PutUint64(buf[i:i+8], l.binID) + i += 8 + copy(buf[i:i+swarm.HashSize], l.address.Bytes()) + i += swarm.HashSize + copy(buf[i:i+swarm.HashSize], l.batchID) + i += swarm.HashSize + buf[i] = uint8(l.chunkType) + i += 1 + copy(buf[i:i+swarm.HashSize], l.stampHash) + return buf, nil +} + +func (l *legacyChunkBinItem) Unmarshal(_ []byte) error { return errors.New("not expected") } + +func (l *legacyChunkBinItem) Clone() storage.Item { + c := *l + return &c +} + +// TestStep08 runs the Sum backfill migration against genuine pre-migration +// records: old-format ChunkBinItems, entries whose chunk is missing from the +// chunkstore, an orphaned old-format ChunkBinItem and a legacy entry with an +// unset stamp hash. The migration must upgrade the healthy entries and remove +// every other one without failing. +func TestStep08(t *testing.T) { + t.Parallel() + + store := internal.NewInmemStorage() + baseAddr := swarm.RandAddress(t) + ctx := context.Background() + + binID := uint64(0) + nextBinID := func() uint64 { binID++; return binID } + + // seed writes a full pre-migration reserve entry for the chunk. When + // inChunkstore is false the chunk data and stamp are omitted, simulating a + // dangling index entry. + seed := func(ch swarm.Chunk, stampHash []byte, inChunkstore bool) *reserve.BatchRadiusItem { + t.Helper() + bin := swarm.Proximity(baseAddr.Bytes(), ch.Address().Bytes()) + id := nextBinID() + br := &reserve.BatchRadiusItem{ + Bin: bin, + BatchID: ch.Stamp().BatchID(), + Address: ch.Address(), + BinID: id, + StampHash: stampHash, + } + err := store.Run(ctx, func(s transaction.Store) error { + err := errors.Join( + s.IndexStore().Put(br), + s.IndexStore().Put(&legacyChunkBinItem{ + bin: bin, + binID: id, + address: ch.Address(), + batchID: ch.Stamp().BatchID(), + chunkType: storage.ChunkType(ch), + stampHash: stampHash, + }), + ) + if err != nil || !inChunkstore { + return err + } + return errors.Join( + chunkstamp.Store(s.IndexStore(), "reserve", ch), + s.ChunkStore().Put(ctx, ch), + ) + }) + if err != nil { + t.Fatal(err) + } + return br + } + + stampHashOf := func(ch swarm.Chunk) []byte { + t.Helper() + h, err := ch.Stamp().Hash() + if err != nil { + t.Fatal(err) + } + return h + } + + // newValidCAC builds a genuine content addressed chunk: the migration + // classifies chunks with the real ChunkType, so random-data chunks would + // be removed as invalid. + newValidCAC := func(data string) swarm.Chunk { + t.Helper() + ch, err := cac.New([]byte(data)) + if err != nil { + t.Fatal(err) + } + return ch.WithStamp(postagetesting.MustNewStamp()) + } + + // healthy CAC entry + cacCh := newValidCAC("healthy cac") + cacBr := seed(cacCh, stampHashOf(cacCh), true) + + // healthy SOC entry: its sum must fold the wrapped CAC address + socChunk := soctesting.GenerateMockSOC(t, []byte("payload")). + Chunk().WithStamp(postagetesting.MustNewStamp()) + socBr := seed(socChunk, stampHashOf(socChunk), true) + + // dangling entry: chunk missing from the chunkstore. Its removal exercises + // deleteChunkBinItem against the old-format record. + dangling := chunktest.GenerateTestRandomChunkAt(t, baseAddr, 1) + danglingBr := seed(dangling, stampHashOf(dangling), false) + + // legacy entry with an unset stamp hash: the chunk itself is valid, so + // its removal is attributable to the stamp hash check alone + zeroHash := newValidCAC("zero stamp hash") + zeroHashBr := seed(zeroHash, swarm.EmptyAddress.Bytes(), true) + + // orphaned old-format ChunkBinItem: no BatchRadiusItem, so the backfill + // never rewrites it and only the trailing sweep can remove it. + orphan := chunktest.GenerateTestRandomChunkAt(t, baseAddr, 1) + orphanItem := &legacyChunkBinItem{ + bin: swarm.Proximity(baseAddr.Bytes(), orphan.Address().Bytes()), + binID: nextBinID(), + address: orphan.Address(), + batchID: orphan.Stamp().BatchID(), + chunkType: swarm.ChunkTypeContentAddressed, + stampHash: stampHashOf(orphan), + } + if err := store.Run(ctx, func(s transaction.Store) error { + return s.IndexStore().Put(orphanItem) + }); err != nil { + t.Fatal(err) + } + + if err := localmigration.Step08(store, log.Noop)(); err != nil { + t.Fatal(err) + } + + // the healthy entries are upgraded in place with correct sums in both + // indexes. + for _, tc := range []struct { + ch swarm.Chunk + br *reserve.BatchRadiusItem + }{ + {cacCh, cacBr}, + {socChunk, socBr}, + } { + sum, err := storage.ChunkSum(tc.ch) + if err != nil { + t.Fatal(err) + } + cbi := &reserve.ChunkBinItem{Bin: tc.br.Bin, BinID: tc.br.BinID} + if err := store.IndexStore().Get(cbi); err != nil { + t.Fatalf("expected upgraded chunk bin item for %s: %v", tc.ch.Address(), err) + } + if !bytes.Equal(cbi.Sum, sum) { + t.Fatalf("wrong sum on upgraded chunk bin item for %s", tc.ch.Address()) + } + has, err := store.IndexStore().Has(&reserve.ChunkSumItem{Address: tc.ch.Address(), Sum: sum}) + if err != nil { + t.Fatal(err) + } + if !has { + t.Fatalf("expected chunk sum item for %s", tc.ch.Address()) + } + } + + // the dangling and zero-stamp-hash entries are removed entirely. + for _, br := range []*reserve.BatchRadiusItem{danglingBr, zeroHashBr} { + has, err := store.IndexStore().Has(br) + if err != nil { + t.Fatal(err) + } + if has { + t.Fatalf("expected batch radius item for %s to be removed", br.Address) + } + } + + // no chunkBin record of any format survives except the two upgraded ones, + // proving the orphan was swept and every remaining value unmarshals. + count := 0 + err := store.IndexStore().Iterate( + storage.Query{Factory: func() storage.Item { return &reserve.ChunkBinItem{} }}, + func(res storage.Result) (bool, error) { + count++ + return false, nil + }, + ) + if err != nil { + t.Fatalf("chunkBin namespace must be fully decodable after migration: %v", err) + } + if count != 2 { + t.Fatalf("expected 2 chunk bin items after migration, got %d", count) + } +} From 11eef372d491d94521b1f654e081a02a67f5e85a Mon Sep 17 00:00:00 2001 From: Calin Martinconi Date: Fri, 24 Jul 2026 12:50:28 +0300 Subject: [PATCH 04/17] test(storer): add divergence sync test, fuzz targets and sum index invariant (SWIP-101) Extend the chunk checksum test coverage to the properties that phase 1 guarantees on its own, independent of the follow-up retention work. An end-to-end pullsync test drives two syncers over a recorded stream with two single owner chunks sharing an address, batch and stamp while wrapping different content: the divergent chunk must be wanted and delivered, while identical content must not be requested. The former is precisely the case the content-blind want-check used to skip. Fuzz targets cover the surfaces that parse untrusted or hand-encoded bytes: ChunkSum, which pullsync recomputes on delivered chunks before their validity is checked, the ChunkBinItem codec, and the raw chunkBin key parser. Writing the key parser round-trip property surfaced that bin values at or above swarm.MaxBins would not survive the rune-encoded ID construction, so ParseChunkBinID now rejects them instead of misinterpreting malformed keys. A randomized operation test (overlapping SOC puts across batches with timestamp replacements, CAC puts and batch evictions) repeatedly checks the invariant the want-decision depends on: the chunk sum index is exactly the set of live (address, sum) pairs, and every stored sum matches the payload currently held in the chunkstore. --- pkg/pullsync/pullsync_test.go | 75 ++++++++++++ pkg/storage/storage_test.go | 47 ++++++++ pkg/storer/internal/reserve/fuzz_test.go | 75 ++++++++++++ pkg/storer/internal/reserve/items.go | 6 + pkg/storer/internal/reserve/reserve_test.go | 120 ++++++++++++++++++++ 5 files changed, 323 insertions(+) create mode 100644 pkg/storer/internal/reserve/fuzz_test.go diff --git a/pkg/pullsync/pullsync_test.go b/pkg/pullsync/pullsync_test.go index 28d0bef6ab5..64c8af36e9c 100644 --- a/pkg/pullsync/pullsync_test.go +++ b/pkg/pullsync/pullsync_test.go @@ -12,6 +12,7 @@ import ( "testing/synctest" "time" + "github.com/ethersphere/bee/v2/pkg/crypto" "github.com/ethersphere/bee/v2/pkg/log" "github.com/ethersphere/bee/v2/pkg/p2p" "github.com/ethersphere/bee/v2/pkg/p2p/streamtest" @@ -19,6 +20,7 @@ import ( postagetesting "github.com/ethersphere/bee/v2/pkg/postage/testing" "github.com/ethersphere/bee/v2/pkg/pullsync" "github.com/ethersphere/bee/v2/pkg/soc" + soctesting "github.com/ethersphere/bee/v2/pkg/soc/testing" "github.com/ethersphere/bee/v2/pkg/storage" testingc "github.com/ethersphere/bee/v2/pkg/storage/testing" "github.com/ethersphere/bee/v2/pkg/storer" @@ -381,3 +383,76 @@ func newPullSyncWithStamperValidator( }) return ps, storage } + +// TestIncoming_DivergentSOC covers the core SWIP-101 property end to end: a +// single owner chunk that shares address, batch and stamp with one the client +// already holds, but wraps different content, must be wanted and delivered. +// Under the previous content-blind want-check it was silently skipped, which +// kept neighborhoods from ever converging. An identical chunk must still not +// be wanted. +func TestIncoming_DivergentSOC(t *testing.T) { + synctest.Test(t, func(t *testing.T) { + privKey, err := crypto.GenerateSecp256k1Key() + if err != nil { + t.Fatal(err) + } + signer := crypto.NewDefaultSigner(privKey) + + // same owner and id: same SOC address. The stamp signs the (shared) + // chunk address, so one stamp legitimately covers both chunks and the + // stamp hashes are identical: only the sums tell them apart. + stamp := postagetesting.MustNewStamp() + held := soctesting.GenerateMockSocWithSigner(t, []byte("held"), signer).Chunk().WithStamp(stamp) + divergent := soctesting.GenerateMockSocWithSigner(t, []byte("divergent"), signer).Chunk().WithStamp(stamp) + + stampHash, err := stamp.Hash() + if err != nil { + t.Fatal(err) + } + + offer := func(ch swarm.Chunk) []*storer.BinC { + sum, err := storage.ChunkSum(ch) + if err != nil { + t.Fatal(err) + } + return []*storer.BinC{{ + Address: ch.Address(), + BatchID: stamp.BatchID(), + BinID: 1, + StampHash: stampHash, + Sum: sum, + }} + } + + // divergent content is wanted + { + ps, _ := newPullSync(t, nil, 1, mock.WithSubscribeResp(offer(divergent), nil), mock.WithChunks(divergent)) + recorder := streamtest.New(streamtest.WithProtocols(ps.Protocol())) + psClient, clientDb := newPullSync(t, recorder, 0, mock.WithChunks(held)) + + if _, _, err := psClient.Sync(context.Background(), swarm.ZeroAddress, 0, 0); err != nil { + t.Fatal(err) + } + + if p := clientDb.PutCalls(); p != 1 { + t.Fatalf("divergent soc must be delivered: want 1 put, got %d", p) + } + haveChunks(t, clientDb, divergent) + } + + // identical content is not wanted + { + ps, _ := newPullSync(t, nil, 1, mock.WithSubscribeResp(offer(held), nil), mock.WithChunks(held)) + recorder := streamtest.New(streamtest.WithProtocols(ps.Protocol())) + psClient, clientDb := newPullSync(t, recorder, 0, mock.WithChunks(held)) + + if _, _, err := psClient.Sync(context.Background(), swarm.ZeroAddress, 0, 0); err != nil { + t.Fatal(err) + } + + if p := clientDb.PutCalls(); p != 0 { + t.Fatalf("identical soc must not be delivered: want 0 puts, got %d", p) + } + } + }) +} diff --git a/pkg/storage/storage_test.go b/pkg/storage/storage_test.go index d8efa089d20..5d95cb6fd35 100644 --- a/pkg/storage/storage_test.go +++ b/pkg/storage/storage_test.go @@ -187,3 +187,50 @@ func TestChunkSum(t *testing.T) { } }) } + +// FuzzChunkSum exercises the checksum computation with fully arbitrary chunk +// data and stamp fields. Pullsync recomputes the sum on delivered chunks +// before their validity is checked, so this path processes attacker-controlled +// bytes and must never panic. It also pins the equivalence of ChunkSum and +// ChunkSumFromParts whenever a sum is computable at all. +func FuzzChunkSum(f *testing.F) { + inner, err := cac.New([]byte("seed payload")) + if err != nil { + f.Fatal(err) + } + validSoc := testingc.GenerateTestRandomSoChunk(f, inner) + f.Add(validSoc.Data(), []byte("batch"), []byte("index"), []byte("ts"), []byte("sig")) + f.Add([]byte{}, []byte{}, []byte{}, []byte{}, []byte{}) + f.Add(make([]byte, swarm.ChunkWithSpanSize), make([]byte, 32), make([]byte, 8), make([]byte, 8), make([]byte, 65)) + + f.Fuzz(func(t *testing.T, data, batchID, index, ts, sig []byte) { + if len(data) > swarm.SocMaxChunkSize { + t.Skip() + } + + hasher := swarm.NewHasher() + _, _ = hasher.Write(data) + ch := swarm.NewChunk(swarm.NewAddress(hasher.Sum(nil)), data). + WithStamp(postage.NewStamp(batchID, index, ts, sig)) + + sum, err := storage.ChunkSum(ch) // must not panic on any input + if err != nil { + return + } + if len(sum) != storage.ChunkSumSize { + t.Fatalf("sum length %d, want %d", len(sum), storage.ChunkSumSize) + } + + stampHash, err := ch.Stamp().Hash() + if err != nil { + t.Fatalf("sum computed but stamp hash failed: %v", err) + } + fromParts, err := storage.ChunkSumFromParts(ch.Stamp().BatchID(), stampHash, ch) + if err != nil { + t.Fatalf("sum computed but parts variant failed: %v", err) + } + if !bytes.Equal(sum, fromParts) { + t.Fatal("ChunkSum and ChunkSumFromParts disagree") + } + }) +} diff --git a/pkg/storer/internal/reserve/fuzz_test.go b/pkg/storer/internal/reserve/fuzz_test.go new file mode 100644 index 00000000000..aeaef1842ee --- /dev/null +++ b/pkg/storer/internal/reserve/fuzz_test.go @@ -0,0 +1,75 @@ +// Copyright 2026 The Swarm Authors. All rights reserved. +// Use of this source code is governed by a BSD-style +// license that can be found in the LICENSE file. + +package reserve_test + +import ( + "bytes" + "testing" + + "github.com/ethersphere/bee/v2/pkg/storage" + "github.com/ethersphere/bee/v2/pkg/storer/internal/reserve" + "github.com/ethersphere/bee/v2/pkg/swarm" +) + +const chunkBinItemSize = 1 + 8 + swarm.HashSize + swarm.HashSize + 1 + swarm.HashSize + storage.ChunkSumSize + +// FuzzChunkBinItemUnmarshal feeds arbitrary bytes to the hand-rolled +// ChunkBinItem codec: it must never panic, must reject any length other than +// the canonical one, and every accepted value must survive a marshal +// round-trip byte for byte. +func FuzzChunkBinItemUnmarshal(f *testing.F) { + valid := &reserve.ChunkBinItem{ + Bin: 1, + BinID: 42, + Address: swarm.NewAddress(bytes.Repeat([]byte{0xaa}, swarm.HashSize)), + BatchID: bytes.Repeat([]byte{0xbb}, swarm.HashSize), + StampHash: bytes.Repeat([]byte{0xcc}, swarm.HashSize), + ChunkType: swarm.ChunkTypeSingleOwner, + Sum: bytes.Repeat([]byte{0xdd}, storage.ChunkSumSize), + } + buf, err := valid.Marshal() + if err != nil { + f.Fatal(err) + } + f.Add(buf) + f.Add([]byte{}) + f.Add(make([]byte, chunkBinItemSize-storage.ChunkSumSize)) // pre-Sum legacy size + + f.Fuzz(func(t *testing.T, data []byte) { + item := &reserve.ChunkBinItem{} + if err := item.Unmarshal(data); err != nil { + return + } + if len(data) != chunkBinItemSize { + t.Fatalf("accepted value of length %d, want only %d", len(data), chunkBinItemSize) + } + out, err := item.Marshal() + if err != nil { + t.Fatalf("unmarshaled value failed to marshal: %v", err) + } + if !bytes.Equal(out, data) { + t.Fatal("marshal round-trip changed the value") + } + }) +} + +// FuzzParseChunkBinID feeds arbitrary key IDs to the raw-key parser used for +// key-only deletion of undecodable records: it must never panic, and every +// accepted ID must reproduce itself through the item's ID construction. +func FuzzParseChunkBinID(f *testing.F) { + f.Add([]byte((&reserve.ChunkBinItem{Bin: 3, BinID: 7}).ID())) + f.Add([]byte{}) + f.Add(make([]byte, 9)) + + f.Fuzz(func(t *testing.T, id []byte) { + bin, binID, err := reserve.ParseChunkBinID(string(id)) + if err != nil { + return + } + if got := (&reserve.ChunkBinItem{Bin: bin, BinID: binID}).ID(); got != string(id) { + t.Fatalf("parse/construct round-trip mismatch: %x -> %x", id, got) + } + }) +} diff --git a/pkg/storer/internal/reserve/items.go b/pkg/storer/internal/reserve/items.go index 9ce384233cd..b587f4c582a 100644 --- a/pkg/storer/internal/reserve/items.go +++ b/pkg/storer/internal/reserve/items.go @@ -137,10 +137,16 @@ const chunkBinIDLength = 1 + 8 // ParseChunkBinID parses a raw ChunkBinItem key ID, the inverse of // binIDToString. It allows key-only deletion of entries whose stored value // cannot be unmarshaled, such as records left in a pre-Sum serialization. +// The bin is bounded: binIDToString rune-encodes the bin byte, so values at +// or above swarm.MaxBins (which never occur in real keys) would not survive +// the ID round-trip and are rejected instead of being misinterpreted. func ParseChunkBinID(id string) (bin uint8, binID uint64, err error) { if len(id) != chunkBinIDLength { return 0, 0, fmt.Errorf("reserve: invalid chunkBin key length %d", len(id)) } + if id[0] >= swarm.MaxBins { + return 0, 0, fmt.Errorf("reserve: invalid bin %d in chunkBin key", id[0]) + } return id[0], binary.BigEndian.Uint64([]byte(id[1:])), nil } diff --git a/pkg/storer/internal/reserve/reserve_test.go b/pkg/storer/internal/reserve/reserve_test.go index 48c8c7c47f2..c5c8532df4d 100644 --- a/pkg/storer/internal/reserve/reserve_test.go +++ b/pkg/storer/internal/reserve/reserve_test.go @@ -8,11 +8,13 @@ import ( "bytes" "context" "errors" + "fmt" "math" "math/rand" "testing" "testing/synctest" + "github.com/ethersphere/bee/v2/pkg/cac" "github.com/ethersphere/bee/v2/pkg/crypto" "github.com/ethersphere/bee/v2/pkg/log" "github.com/ethersphere/bee/v2/pkg/postage" @@ -1326,3 +1328,121 @@ func TestSOCSiblingSumRefresh(t *testing.T) { t.Fatal("expected the stale sum of batch B to be dropped") } } + +// TestChunkSumIndexRandomOps drives the reserve with a randomized sequence of +// overlapping SOC puts (shared addresses across batches, timestamp +// replacements), CAC puts and batch evictions, and repeatedly asserts the +// invariant the pullsync want-decision depends on: the ChunkSumItem index is +// exactly the set of (address, sum) pairs of the live ChunkBinItems, and every +// stored sum matches the payload actually held in the chunkstore. +func TestChunkSumIndexRandomOps(t *testing.T) { + t.Parallel() + + ctx := context.Background() + baseAddr := swarm.RandAddress(t) + st := internal.NewInmemStorage() + r, err := reserve.New(baseAddr, st, 0, kademlia.NewTopologyDriver(), log.Noop) + if err != nil { + t.Fatal(err) + } + + rng := rand.New(rand.NewSource(42)) + + // small pools force address sharing, stamp collisions and replacements + signers := []crypto.Signer{getSigner(t), getSigner(t)} + batches := [][]byte{ + postagetesting.MustNewBatch().ID, + postagetesting.MustNewBatch().ID, + postagetesting.MustNewBatch().ID, + } + + checkInvariant := func(op int) { + t.Helper() + + // live (address, sum) pairs according to the chunk bin index; sums + // must match the payload currently in the chunkstore. + live := make(map[string]int) + err := st.IndexStore().Iterate( + storage.Query{Factory: func() storage.Item { return &reserve.ChunkBinItem{} }}, + func(res storage.Result) (bool, error) { + cbi := res.Entry.(*reserve.ChunkBinItem) + ch, err := st.ChunkStore().Get(ctx, cbi.Address) + if err != nil { + return false, fmt.Errorf("op %d: chunk missing for live index entry %s: %w", op, cbi.Address, err) + } + want, err := storage.ChunkSumFromParts(cbi.BatchID, cbi.StampHash, ch) + if err != nil { + return false, err + } + if !bytes.Equal(cbi.Sum, want) { + return false, fmt.Errorf("op %d: stale sum on entry %s", op, cbi.Address) + } + live[cbi.Address.ByteString()+string(cbi.Sum)]++ + return false, nil + }, + ) + if err != nil { + t.Fatal(err) + } + + // the sum index must be exactly the live set, in both directions + indexed := make(map[string]int) + err = st.IndexStore().Iterate( + storage.Query{ + Factory: func() storage.Item { return &reserve.ChunkSumItem{} }, + ItemProperty: storage.QueryItemID, + }, + func(res storage.Result) (bool, error) { + if len(res.ID) != swarm.HashSize+storage.ChunkSumSize { + return false, fmt.Errorf("op %d: malformed chunk sum key length %d", op, len(res.ID)) + } + indexed[res.ID]++ + return false, nil + }, + ) + if err != nil { + t.Fatal(err) + } + + for k := range indexed { + if live[k] == 0 { + t.Fatalf("op %d: orphaned chunk sum entry (no live chunk bin item)", op) + } + } + for k, n := range live { + if n > 1 { + t.Fatalf("op %d: %d chunk bin items share one (address, sum) pair", op, n) + } + if indexed[k] == 0 { + t.Fatalf("op %d: live chunk bin item without chunk sum entry", op) + } + } + } + + ts := uint64(0) + for op := range 200 { + switch v := rng.Intn(10); { + case v < 6: // SOC put: shared addresses, random payload, random batch + ts++ + s := soctesting.GenerateMockSocWithSigner(t, fmt.Appendf(nil, "payload-%d", rng.Intn(4)), signers[rng.Intn(len(signers))]) + stamp := postagetesting.MustNewFields(batches[rng.Intn(len(batches))], 0, ts) + err = r.Put(ctx, s.Chunk().WithStamp(stamp)) + case v < 8: // CAC put + ch, cerr := cac.New(fmt.Appendf(nil, "cac-%d", op)) + if cerr != nil { + t.Fatal(cerr) + } + err = r.Put(ctx, ch.WithStamp(postagetesting.MustNewBatchStamp(batches[rng.Intn(len(batches))]))) + default: // evict a whole batch + _, err = r.EvictBatchBin(ctx, batches[rng.Intn(len(batches))], math.MaxInt, swarm.MaxBins) + } + if err != nil && !errors.Is(err, storage.ErrOverwriteNewerChunk) { + t.Fatalf("op %d: %v", op, err) + } + + if op%20 == 19 { + checkInvariant(op) + } + } + checkInvariant(200) +} From b624e5f9294c4c86600759f1a9515e18de1d9949 Mon Sep 17 00:00:00 2001 From: sbackend Date: Sun, 26 Jul 2026 23:02:52 +0200 Subject: [PATCH 05/17] fix: cac divergence --- pkg/storer/internal/reserve/reserve.go | 52 +++++- pkg/storer/internal/reserve/reserve_test.go | 169 ++++++++++++++++++++ 2 files changed, 213 insertions(+), 8 deletions(-) diff --git a/pkg/storer/internal/reserve/reserve.go b/pkg/storer/internal/reserve/reserve.go index 03b9556294a..974479cc1d0 100644 --- a/pkg/storer/internal/reserve/reserve.go +++ b/pkg/storer/internal/reserve/reserve.go @@ -5,6 +5,7 @@ package reserve import ( + "bytes" "context" "encoding/binary" "encoding/hex" @@ -98,6 +99,9 @@ func New( // the existing chunk if the new chunk has a higher stamp timestamp (regardless of batch type). // 3. A new chunk that has the same address belonging to the same stamp index with an already stored chunk will overwrite the existing chunk // if the new chunk has a higher stamp timestamp (regardless of batch type and chunk type, eg CAC & SOC). +// 4. Two different chunk addresses that share the same batch stamp index and timestamp are settled by a tie-break: +// the lexicographically lower chunk address wins. The loser is rejected; the winner replaces the stored chunk +// through the usual remove-and-store path (including a fresh bin ID for pullsync). func (r *Reserve) Put(ctx context.Context, chunk swarm.Chunk) error { // batchID lock, Put vs Eviction r.multx.Lock(string(chunk.Stamp().BatchID())) @@ -152,19 +156,51 @@ func (r *Reserve) Put(ctx context.Context, chunk swarm.Chunk) error { // index collision if loadedStampIndex { - prev := binary.BigEndian.Uint64(oldStampIndex.StampTimestamp) curr := binary.BigEndian.Uint64(chunk.Stamp().Timestamp()) - if prev >= curr { + if prev > curr { return fmt.Errorf("overwrite same chunk. prev %d cur %d batch %s: %w", prev, curr, hex.EncodeToString(chunk.Stamp().BatchID()), storage.ErrOverwriteNewerChunk) } - r.logger.Debug( - "replacing chunk stamp index", - "old_chunk", oldStampIndex.ChunkAddress, - "new_chunk", chunk.Address(), - "batch_id", hex.EncodeToString(chunk.Stamp().BatchID()), - ) + // Same stamp index and timestamp, different chunk addresses: both + // claims are otherwise valid, so settle on the lower address. + if prev == curr && chunkType == swarm.ChunkTypeContentAddressed && !oldStampIndex.ChunkAddress.Equal(chunk.Address()) { + if bytes.Compare(chunk.Address().Bytes(), oldStampIndex.ChunkAddress.Bytes()) >= 0 { + r.logger.Debug( + "discarding stamp index collision", + "old_chunk", oldStampIndex.ChunkAddress, + "new_chunk", chunk.Address(), + "batch_id", hex.EncodeToString(chunk.Stamp().BatchID()), + "stamp_index", hex.EncodeToString(chunk.Stamp().Index()), + "stamp_timestamp", binary.BigEndian.Uint64(chunk.Stamp().Timestamp()), + "incoming_stamp_hash", hex.EncodeToString(stampHash), + "stored_stamp_hash", hex.EncodeToString(oldStampIndex.StampHash), + ) + return fmt.Errorf( + "stamp index collision chunk %s lost tie-break: %w", + chunk.Address(), + storage.ErrDivergentChunkRejected, + ) + } + r.logger.Debug( + "replacing stamp index collision", + "old_chunk", oldStampIndex.ChunkAddress, + "new_chunk", chunk.Address(), + "batch_id", hex.EncodeToString(chunk.Stamp().BatchID()), + "stamp_index", hex.EncodeToString(chunk.Stamp().Index()), + "stamp_timestamp", binary.BigEndian.Uint64(chunk.Stamp().Timestamp()), + "incoming_stamp_hash", hex.EncodeToString(stampHash), + "stored_stamp_hash", hex.EncodeToString(oldStampIndex.StampHash), + ) + // Incoming wins: fall through to removeChunk + store below. + } else { + r.logger.Debug( + "replacing chunk stamp index", + "old_chunk", oldStampIndex.ChunkAddress, + "new_chunk", chunk.Address(), + "batch_id", hex.EncodeToString(chunk.Stamp().BatchID()), + ) + } // same chunk address if oldStampIndex.ChunkAddress.Equal(chunk.Address()) { diff --git a/pkg/storer/internal/reserve/reserve_test.go b/pkg/storer/internal/reserve/reserve_test.go index 6d193370256..8a4929825ab 100644 --- a/pkg/storer/internal/reserve/reserve_test.go +++ b/pkg/storer/internal/reserve/reserve_test.go @@ -1326,3 +1326,172 @@ func TestSOCDivergenceBumpsBinID(t *testing.T) { checkStore(t, ts.IndexStore(), &reserve.ChunkBinItem{Bin: bin, BinID: oldBinID}, true) checkStore(t, ts.IndexStore(), &reserve.ChunkBinItem{Bin: bin, BinID: item.BinID}, false) } + +// TestCACStampIndexCollision covers two content-addressed chunks that share a +// batch stamp index and timestamp but have different addresses. The reserve +// keeps the lexicographically lower address regardless of arrival order. +func TestCACStampIndexCollision(t *testing.T) { + t.Parallel() + + ctx := context.Background() + batch := postagetesting.MustNewBatch() + stamp := postagetesting.MustNewFields(batch.ID, 0, 1) + + baseAddr := swarm.RandAddress(t) + ch1 := chunk.GenerateTestRandomChunkAt(t, baseAddr, 0).WithStamp(stamp) + ch2 := chunk.GenerateTestRandomChunkAt(t, baseAddr, 0).WithStamp(stamp.Clone()) + if ch1.Address().Equal(ch2.Address()) { + t.Fatal("expected different CAC addresses") + } + + winner, loser := ch1, ch2 + if bytes.Compare(ch2.Address().Bytes(), ch1.Address().Bytes()) < 0 { + winner, loser = ch2, ch1 + } + + for _, tc := range []struct { + name string + order []swarm.Chunk + }{ + {"winner first", []swarm.Chunk{winner, loser}}, + {"loser first", []swarm.Chunk{loser, winner}}, + } { + t.Run(tc.name, func(t *testing.T) { + t.Parallel() + + ts := internal.NewInmemStorage() + r, err := reserve.New(baseAddr, ts, 0, kademlia.NewTopologyDriver(), log.Noop) + if err != nil { + t.Fatal(err) + } + + if err := r.Put(ctx, tc.order[0]); err != nil { + t.Fatal(err) + } + sizeAfterFirst := r.Size() + + err = r.Put(ctx, tc.order[1]) + if tc.order[1].Address().Equal(loser.Address()) { + if !errors.Is(err, storage.ErrDivergentChunkRejected) { + t.Fatalf("expected ErrDivergentChunkRejected, got %v", err) + } + } else if err != nil { + t.Fatal(err) + } + + if _, err := ts.ChunkStore().Get(ctx, winner.Address()); err != nil { + t.Fatalf("expected winner stored: %v", err) + } + if _, err := ts.ChunkStore().Get(ctx, loser.Address()); !errors.Is(err, storage.ErrNotFound) { + t.Fatalf("expected loser absent, got %v", err) + } + + item, err := stampindex.Load(ts.IndexStore(), "reserve", winner.Stamp()) + if err != nil { + t.Fatal(err) + } + if !item.ChunkAddress.Equal(winner.Address()) { + t.Fatalf("stamp index points to %s, want %s", item.ChunkAddress, winner.Address()) + } + + if got := r.Size(); got != sizeAfterFirst { + t.Fatalf("expected reserve size to stay %d, got %d", sizeAfterFirst, got) + } + + winnerSum, err := storage.ChunkSum(winner) + if err != nil { + t.Fatal(err) + } + loserSum, err := storage.ChunkSum(loser) + if err != nil { + t.Fatal(err) + } + has, err := r.HasSum(winner.Address(), winnerSum) + if err != nil { + t.Fatal(err) + } + if !has { + t.Fatal("expected the winner sum to be indexed") + } + has, err = r.HasSum(loser.Address(), loserSum) + if err != nil { + t.Fatal(err) + } + if has { + t.Fatal("expected the loser sum not to be indexed") + } + }) + } +} + +// TestCACStampIndexCollisionBumpsBinID asserts that accepting a lower-address +// CAC over a stamp-index collision writes it at a fresh bin ID for pullsync. +func TestCACStampIndexCollisionBumpsBinID(t *testing.T) { + t.Parallel() + + ctx := context.Background() + baseAddr := swarm.RandAddress(t) + ts := internal.NewInmemStorage() + r, err := reserve.New(baseAddr, ts, 0, kademlia.NewTopologyDriver(), log.Noop) + if err != nil { + t.Fatal(err) + } + + batch := postagetesting.MustNewBatch() + stamp := postagetesting.MustNewFields(batch.ID, 0, 1) + ch1 := chunk.GenerateTestRandomChunkAt(t, baseAddr, 0).WithStamp(stamp) + ch2 := chunk.GenerateTestRandomChunkAt(t, baseAddr, 0).WithStamp(stamp.Clone()) + + first, second := ch1, ch2 + if bytes.Compare(ch2.Address().Bytes(), ch1.Address().Bytes()) > 0 { + first, second = ch2, ch1 + } + + if err := r.Put(ctx, first); err != nil { + t.Fatal(err) + } + + firstStampHash, err := first.Stamp().Hash() + if err != nil { + t.Fatal(err) + } + firstBin := swarm.Proximity(baseAddr.Bytes(), first.Address().Bytes()) + oldItem := &reserve.BatchRadiusItem{ + Bin: firstBin, + BatchID: batch.ID, + Address: first.Address(), + StampHash: firstStampHash, + } + if err := ts.IndexStore().Get(oldItem); err != nil { + t.Fatal(err) + } + oldBinID := oldItem.BinID + + if err := r.Put(ctx, second); err != nil { + t.Fatal(err) + } + + secondStampHash, err := second.Stamp().Hash() + if err != nil { + t.Fatal(err) + } + secondBin := swarm.Proximity(baseAddr.Bytes(), second.Address().Bytes()) + newItem := &reserve.BatchRadiusItem{ + Bin: secondBin, + BatchID: batch.ID, + Address: second.Address(), + StampHash: secondStampHash, + } + if err := ts.IndexStore().Get(newItem); err != nil { + t.Fatal(err) + } + if secondBin == firstBin && newItem.BinID <= oldBinID { + t.Fatalf("expected bin id to be bumped past %d, got %d", oldBinID, newItem.BinID) + } + + checkStore(t, ts.IndexStore(), &reserve.BatchRadiusItem{ + Bin: firstBin, BatchID: batch.ID, Address: first.Address(), StampHash: firstStampHash, + }, true) + checkStore(t, ts.IndexStore(), &reserve.ChunkBinItem{Bin: firstBin, BinID: oldBinID}, true) + checkStore(t, ts.IndexStore(), &reserve.ChunkBinItem{Bin: secondBin, BinID: newItem.BinID}, false) +} From f9749a9576d5111af3a640e3e130c358c6d79e0b Mon Sep 17 00:00:00 2001 From: Calin Martinconi Date: Mon, 20 Jul 2026 22:46:44 +0300 Subject: [PATCH 06/17] feat(storer): converge divergent SOCs in the storage layer (SWIP-101) Two valid single owner chunks can share an address, batch and stamp while wrapping different content. The stamp signs the chunk address, so both carry an identical stamp and produce an identical stamp hash. The content-blind existence check at the top of reserve.Put therefore treated the second chunk as already stored and dropped it, leaving each node holding whichever chunk reached it first. Neighborhoods never converged, and the reserve sampler computed different commitments from the same address. Make the check content-aware via the pullsync sum, and settle the divergence here rather than in the protocol: the chunk wrapping the lower CAC address wins. The rule depends only on the two payloads, so every node reaches the same answer regardless of arrival order. On a win the chunk is replaced in place, reusing the stamp index and stamp entries, which are identical for both. The bin ID is bumped so peers that already synced past the old one are offered the replacement. The reserve size is unchanged: one chunk goes in, one comes out. Pullsync treats a lost tie-break as an expected outcome rather than a sync error, since the node already holds the chunk the neighborhood converges on. Divergent chunks under different batches are not covered: they occupy different stamp indices, so no tie-break fires. --- pkg/pullsync/metrics.go | 7 + pkg/pullsync/pullsync.go | 8 + pkg/storage/storage.go | 40 +++++ pkg/storage/storage_test.go | 86 ++++++++++ pkg/storer/internal/reserve/reserve.go | 136 ++++++++++++++-- pkg/storer/internal/reserve/reserve_test.go | 165 ++++++++++++++++++++ 6 files changed, 433 insertions(+), 9 deletions(-) diff --git a/pkg/pullsync/metrics.go b/pkg/pullsync/metrics.go index 57e26916c93..d20453d65ff 100644 --- a/pkg/pullsync/metrics.go +++ b/pkg/pullsync/metrics.go @@ -15,6 +15,7 @@ type metrics struct { MissingChunks prometheus.Counter // number of reserve get errs ReceivedZeroAddress prometheus.Counter // number of delivered chunks with invalid address ReceivedInvalidChunk prometheus.Counter // number of delivered chunks with invalid address + DivergentRejected prometheus.Counter // number of delivered chunks that lost the divergence tie-break Delivered prometheus.Counter // number of chunk deliveries SentOffered prometheus.Counter // number of chunks offered SentWanted prometheus.Counter // number of chunks wanted @@ -57,6 +58,12 @@ func newMetrics() metrics { Name: "received_invalid_chunks", Help: "Total invalid chunks delivered.", }), + DivergentRejected: prometheus.NewCounter(prometheus.CounterOpts{ + Namespace: m.Namespace, + Subsystem: subsystem, + Name: "divergent_rejected", + Help: "Total delivered chunks discarded for losing the divergence tie-break.", + }), Delivered: prometheus.NewCounter(prometheus.CounterOpts{ Namespace: m.Namespace, Subsystem: subsystem, diff --git a/pkg/pullsync/pullsync.go b/pkg/pullsync/pullsync.go index 53ac6d7628a..32a6dba4705 100644 --- a/pkg/pullsync/pullsync.go +++ b/pkg/pullsync/pullsync.go @@ -394,6 +394,14 @@ func (s *Syncer) Sync(ctx context.Context, peer swarm.Address, bin uint8, start chunkErr = errors.Join(chunkErr, err) continue } + // the chunk diverged from the one already stored and lost the + // tie-break. The neighborhood converges on the stored chunk, so + // this is an expected outcome rather than a sync error. + if errors.Is(err, storage.ErrDivergentChunkRejected) { + s.logger.Debug("divergent chunk rejected", "error", err, "peer_address", peer, "chunk", c) + s.metrics.DivergentRejected.Inc() + continue + } return 0, 0, errors.Join(chunkErr, err) } chunksPut++ diff --git a/pkg/storage/storage.go b/pkg/storage/storage.go index 044cd674814..0e910bdc7db 100644 --- a/pkg/storage/storage.go +++ b/pkg/storage/storage.go @@ -5,6 +5,7 @@ package storage import ( + "bytes" "context" "errors" "fmt" @@ -19,6 +20,12 @@ import ( var ( ErrOverwriteNewerChunk = errors.New("overwriting chunk with newer timestamp") ErrUnknownChunkType = errors.New("unknown chunk type") + + // ErrDivergentChunkRejected is returned when a chunk that diverges from an + // already stored one at the same address, batch and stamp loses the + // deterministic tie-break and is therefore not stored. It is not a failure: + // the node already holds the chunk the whole neighborhood converges on. + ErrDivergentChunkRejected = errors.New("divergent chunk rejected by tie-break") ) // Result represents the item returned by the read operation, which returns @@ -336,6 +343,39 @@ func ChunkSumFromParts(batchID, stampHash []byte, ch swarm.Chunk) ([]byte, error return h.Sum(nil)[:ChunkSumSize], nil } +// DivergentChunkWins reports whether the incoming chunk should replace the +// stored one when the two share an address, batch and stamp but wrap different +// content. Both chunks must be single owner chunks; a content addressed chunk +// cannot diverge, since its address is the hash of its own payload. +// +// The winner is the chunk wrapping the lexicographically lower CAC address. +// The rule depends on nothing but the two payloads, so every node in the +// neighborhood converges on the same chunk regardless of the order in which +// they arrive. +func DivergentChunkWins(stored, incoming swarm.Chunk) (bool, error) { + storedAddr, err := wrappedAddress(stored) + if err != nil { + return false, fmt.Errorf("stored chunk: %w", err) + } + incomingAddr, err := wrappedAddress(incoming) + if err != nil { + return false, fmt.Errorf("incoming chunk: %w", err) + } + return bytes.Compare(incomingAddr.Bytes(), storedAddr.Bytes()) < 0, nil +} + +// wrappedAddress returns the address of the CAC wrapped by a single owner chunk. +func wrappedAddress(ch swarm.Chunk) (swarm.Address, error) { + if !soc.Valid(ch) { + return swarm.ZeroAddress, fmt.Errorf("%w: not a single owner chunk", ErrUnknownChunkType) + } + s, err := soc.FromChunk(ch) + if err != nil { + return swarm.ZeroAddress, fmt.Errorf("soc from chunk: %w", err) + } + return s.WrappedChunk().Address(), nil +} + // IdentityAddress returns the internally used address for the chunk // since the single owner chunk address is not a unique identifier for the chunk, // but hashing the soc address and the wrapped chunk address is. diff --git a/pkg/storage/storage_test.go b/pkg/storage/storage_test.go index 5d95cb6fd35..3a480a5c8dc 100644 --- a/pkg/storage/storage_test.go +++ b/pkg/storage/storage_test.go @@ -6,6 +6,7 @@ package storage_test import ( "bytes" "encoding/hex" + "errors" "testing" "github.com/ethereum/go-ethereum/common" @@ -234,3 +235,88 @@ func FuzzChunkSum(f *testing.F) { } }) } + +func TestDivergentChunkWins(t *testing.T) { + t.Parallel() + + privKey, err := crypto.GenerateSecp256k1Key() + if err != nil { + t.Fatal(err) + } + signer := crypto.NewDefaultSigner(privKey) + id := make([]byte, swarm.HashSize) + + newSOC := func(data string) swarm.Chunk { + t.Helper() + inner, err := cac.New([]byte(data)) + if err != nil { + t.Fatal(err) + } + ch, err := soc.New(id, inner).Sign(signer) + if err != nil { + t.Fatal(err) + } + return ch.WithStamp(postagetesting.MustNewStamp()) + } + + lower, higher := newSOC("content-one"), newSOC("content-two") + lowerInner, err := soc.UnwrapCAC(lower) + if err != nil { + t.Fatal(err) + } + higherInner, err := soc.UnwrapCAC(higher) + if err != nil { + t.Fatal(err) + } + if bytes.Compare(lowerInner.Address().Bytes(), higherInner.Address().Bytes()) > 0 { + lower, higher = higher, lower + } + + t.Run("lower wrapped address wins", func(t *testing.T) { + t.Parallel() + + wins, err := storage.DivergentChunkWins(higher, lower) + if err != nil { + t.Fatal(err) + } + if !wins { + t.Fatal("expected the chunk wrapping the lower cac address to win") + } + }) + + t.Run("tie-break is antisymmetric", func(t *testing.T) { + t.Parallel() + + wins, err := storage.DivergentChunkWins(lower, higher) + if err != nil { + t.Fatal(err) + } + if wins { + t.Fatal("expected the chunk wrapping the higher cac address to lose") + } + }) + + t.Run("a chunk does not displace itself", func(t *testing.T) { + t.Parallel() + + wins, err := storage.DivergentChunkWins(lower, lower) + if err != nil { + t.Fatal(err) + } + if wins { + t.Fatal("expected an identical chunk not to win") + } + }) + + t.Run("content addressed chunks cannot diverge", func(t *testing.T) { + t.Parallel() + + cac := testingc.GenerateTestRandomChunk() + if _, err := storage.DivergentChunkWins(cac, lower); !errors.Is(err, storage.ErrUnknownChunkType) { + t.Fatalf("expected ErrUnknownChunkType, got %v", err) + } + if _, err := storage.DivergentChunkWins(lower, cac); !errors.Is(err, storage.ErrUnknownChunkType) { + t.Fatalf("expected ErrUnknownChunkType, got %v", err) + } + }) +} diff --git a/pkg/storer/internal/reserve/reserve.go b/pkg/storer/internal/reserve/reserve.go index c8ebf6862dd..8a67d22731a 100644 --- a/pkg/storer/internal/reserve/reserve.go +++ b/pkg/storer/internal/reserve/reserve.go @@ -128,15 +128,6 @@ func (r *Reserve) putChunk(ctx context.Context, chunk swarm.Chunk) (socReplaced return false, err } - // check if the chunk with the same batch, stamp timestamp and index is already stored - has, err := r.Has(chunk.Address(), chunk.Stamp().BatchID(), stampHash) - if err != nil { - return false, err - } - if has { - return false, nil - } - chunkType := storage.ChunkType(chunk) sum, err := storage.ChunkSum(chunk) @@ -146,6 +137,33 @@ func (r *Reserve) putChunk(ctx context.Context, chunk swarm.Chunk) (socReplaced bin := swarm.Proximity(r.baseAddr.Bytes(), chunk.Address().Bytes()) + // check if the chunk with the same batch, stamp timestamp and index is already stored + has, err := r.Has(chunk.Address(), chunk.Stamp().BatchID(), stampHash) + if err != nil { + return false, err + } + if has { + // Address, batch and stamp all match, but two single owner chunks can + // share those and still wrap different content. The sum tells them + // apart: if it matches we already hold this exact chunk, otherwise the + // chunks diverge and a tie-break decides which one the neighborhood + // keeps. + hasSum, err := r.HasSum(chunk.Address(), sum) + if err != nil { + return false, err + } + if hasSum { + return false, nil + } + if err := r.resolveDivergence(ctx, chunk, sum, stampHash, bin, chunkType); err != nil { + return false, err + } + // the tie-break winner replaced the shared payload, so co-resident + // entries under other stamps need their sums refreshed like on any + // other single owner chunk replacement. + return true, nil + } + // bin lock r.multx.Lock(strconv.Itoa(int(bin))) defer r.multx.Unlock(strconv.Itoa(int(bin))) @@ -418,6 +436,106 @@ func (r *Reserve) refreshSiblingSums(ctx context.Context, addr swarm.Address) er return nil } +// resolveDivergence settles two single owner chunks that share an address, +// batch and stamp but wrap different content. Both are individually valid, so +// the protocol cannot pick between them; the choice is made here, in the +// storage layer, by a tie-break that depends only on the two payloads. Every +// node in the neighborhood therefore converges on the same chunk no matter +// which one it received first. +// +// If the incoming chunk wins it replaces the stored one in place, reusing the +// existing stamp index and stamp entries, which are identical for both. The +// bin ID is bumped so that peers which already synced past the old bin ID are +// offered the replacement, propagating the resolution outwards. +// +// The reserve size is unchanged either way: one chunk goes in, one comes out. +func (r *Reserve) resolveDivergence( + ctx context.Context, + chunk swarm.Chunk, + sum []byte, + stampHash []byte, + bin uint8, + chunkType swarm.ChunkType, +) error { + // bin lock + r.multx.Lock(strconv.Itoa(int(bin))) + defer r.multx.Unlock(strconv.Itoa(int(bin))) + + return r.st.Run(ctx, func(s transaction.Store) error { + stored, err := s.ChunkStore().Get(ctx, chunk.Address()) + if err != nil { + return fmt.Errorf("failed loading diverging chunk %s: %w", chunk.Address(), err) + } + + wins, err := storage.DivergentChunkWins(stored, chunk) + if err != nil { + return fmt.Errorf("divergence tie-break for chunk %s: %w", chunk.Address(), err) + } + + if !wins { + r.logger.Debug( + "discarding diverging chunk", + "address", chunk.Address(), + "batch_id", hex.EncodeToString(chunk.Stamp().BatchID()), + ) + return fmt.Errorf("diverging chunk %s lost tie-break: %w", chunk.Address(), storage.ErrDivergentChunkRejected) + } + + item := &BatchRadiusItem{ + Bin: bin, + Address: chunk.Address(), + BatchID: chunk.Stamp().BatchID(), + StampHash: stampHash, + } + // load item to get the binID of the chunk being replaced + if err := s.IndexStore().Get(item); err != nil { + return err + } + + // drop the bin and sum entries of the replaced chunk + if err := deleteChunkBinItem(s.IndexStore(), item.Bin, item.BinID); err != nil { + return err + } + + binID, err := r.IncBinID(s.IndexStore(), bin) + if err != nil { + return err + } + + r.logger.Debug( + "replacing diverging chunk", + "address", chunk.Address(), + "batch_id", hex.EncodeToString(chunk.Stamp().BatchID()), + "old_bin_id", item.BinID, + "new_bin_id", binID, + ) + + // the BatchRadiusItem key does not cover the binID, so putting it again + // with the new binID overwrites the existing entry. + item.BinID = binID + err = errors.Join( + s.IndexStore().Put(item), + s.IndexStore().Put(&ChunkBinItem{ + Bin: bin, + BinID: binID, + Address: chunk.Address(), + BatchID: chunk.Stamp().BatchID(), + ChunkType: chunkType, + StampHash: stampHash, + Sum: sum, + }), + s.IndexStore().Put(&ChunkSumItem{Address: chunk.Address(), Sum: sum}), + ) + if err != nil { + return err + } + + // swap the payload without touching the reference count: the chunk + // store entry is reused, only its content changes. + return s.ChunkStore().Replace(ctx, chunk, false) + }) +} + func (r *Reserve) Has(addr swarm.Address, batchID []byte, stampHash []byte) (bool, error) { item := &BatchRadiusItem{Bin: swarm.Proximity(r.baseAddr.Bytes(), addr.Bytes()), BatchID: batchID, Address: addr, StampHash: stampHash} return r.st.IndexStore().Has(item) diff --git a/pkg/storer/internal/reserve/reserve_test.go b/pkg/storer/internal/reserve/reserve_test.go index c5c8532df4d..f8e1686a376 100644 --- a/pkg/storer/internal/reserve/reserve_test.go +++ b/pkg/storer/internal/reserve/reserve_test.go @@ -1446,3 +1446,168 @@ func TestChunkSumIndexRandomOps(t *testing.T) { } checkInvariant(200) } + +// TestSOCDivergence covers two single owner chunks that share an address, batch +// and stamp while wrapping different content. Both are valid, so the storage +// layer settles which one the neighborhood keeps, and it must reach the same +// answer regardless of the order the chunks arrive in. +func TestSOCDivergence(t *testing.T) { + t.Parallel() + + ctx := context.Background() + signer := getSigner(t) + batch := postagetesting.MustNewBatch() + + // same signer and id, different payloads: same SOC address, different + // wrapped CAC. + s1 := soctesting.GenerateMockSocWithSigner(t, []byte("data"), signer) + s2 := soctesting.GenerateMockSocWithSigner(t, []byte("update"), signer) + + // the stamp signs the chunk address, which both chunks share, so a single + // stamp legitimately applies to both. + stamp := postagetesting.MustNewFields(batch.ID, 0, 1) + ch1 := s1.Chunk().WithStamp(stamp) + ch2 := s2.Chunk().WithStamp(stamp) + + if !ch1.Address().Equal(ch2.Address()) { + t.Fatal("expected the diverging chunks to share an address") + } + + // the chunk wrapping the lower CAC address is the one both nodes must keep. + winner, loser := ch1, ch2 + if bytes.Compare(s2.WrappedChunk.Address().Bytes(), s1.WrappedChunk.Address().Bytes()) < 0 { + winner, loser = ch2, ch1 + } + + for _, tc := range []struct { + name string + order []swarm.Chunk + }{ + {"winner first", []swarm.Chunk{winner, loser}}, + {"loser first", []swarm.Chunk{loser, winner}}, + } { + t.Run(tc.name, func(t *testing.T) { + t.Parallel() + + baseAddr := swarm.RandAddress(t) + ts := internal.NewInmemStorage() + r, err := reserve.New(baseAddr, ts, 0, kademlia.NewTopologyDriver(), log.Noop) + if err != nil { + t.Fatal(err) + } + + if err := r.Put(ctx, tc.order[0]); err != nil { + t.Fatal(err) + } + sizeAfterFirst := r.Size() + + err = r.Put(ctx, tc.order[1]) + // the second put only errors when the incoming chunk is the loser. + if tc.order[1] == loser { + if !errors.Is(err, storage.ErrDivergentChunkRejected) { + t.Fatalf("expected ErrDivergentChunkRejected, got %v", err) + } + } else if err != nil { + t.Fatal(err) + } + + // whichever order they arrived in, the winner is what is stored. + stored, err := ts.ChunkStore().Get(ctx, winner.Address()) + if err != nil { + t.Fatal(err) + } + if !bytes.Equal(stored.Data(), winner.Data()) { + t.Fatal("expected the tie-break winner to be stored") + } + + // divergence resolution replaces a chunk, it does not add one. + if got := r.Size(); got != sizeAfterFirst { + t.Fatalf("expected reserve size to stay %d, got %d", sizeAfterFirst, got) + } + + // the sum index tracks the stored chunk only. + winnerSum, err := storage.ChunkSum(winner) + if err != nil { + t.Fatal(err) + } + loserSum, err := storage.ChunkSum(loser) + if err != nil { + t.Fatal(err) + } + has, err := r.HasSum(winner.Address(), winnerSum) + if err != nil { + t.Fatal(err) + } + if !has { + t.Fatal("expected the winner sum to be indexed") + } + has, err = r.HasSum(loser.Address(), loserSum) + if err != nil { + t.Fatal(err) + } + if has { + t.Fatal("expected the loser sum not to be indexed") + } + }) + } +} + +// TestSOCDivergenceBumpsBinID asserts that replacing a diverging chunk moves it +// to the top of its bin, so peers that already synced past the old bin ID are +// offered the replacement. +func TestSOCDivergenceBumpsBinID(t *testing.T) { + t.Parallel() + + ctx := context.Background() + baseAddr := swarm.RandAddress(t) + ts := internal.NewInmemStorage() + r, err := reserve.New(baseAddr, ts, 0, kademlia.NewTopologyDriver(), log.Noop) + if err != nil { + t.Fatal(err) + } + + signer := getSigner(t) + batch := postagetesting.MustNewBatch() + s1 := soctesting.GenerateMockSocWithSigner(t, []byte("data"), signer) + s2 := soctesting.GenerateMockSocWithSigner(t, []byte("update"), signer) + stamp := postagetesting.MustNewFields(batch.ID, 0, 1) + ch1 := s1.Chunk().WithStamp(stamp) + ch2 := s2.Chunk().WithStamp(stamp) + + // put the losing chunk first so the second put performs the replacement. + first, second := ch1, ch2 + if bytes.Compare(s2.WrappedChunk.Address().Bytes(), s1.WrappedChunk.Address().Bytes()) > 0 { + first, second = ch2, ch1 + } + + if err := r.Put(ctx, first); err != nil { + t.Fatal(err) + } + + bin := swarm.Proximity(baseAddr.Bytes(), first.Address().Bytes()) + stampHash, err := stamp.Hash() + if err != nil { + t.Fatal(err) + } + + item := &reserve.BatchRadiusItem{Bin: bin, BatchID: batch.ID, Address: first.Address(), StampHash: stampHash} + if err := ts.IndexStore().Get(item); err != nil { + t.Fatal(err) + } + oldBinID := item.BinID + + if err := r.Put(ctx, second); err != nil { + t.Fatal(err) + } + + if err := ts.IndexStore().Get(item); err != nil { + t.Fatal(err) + } + if item.BinID <= oldBinID { + t.Fatalf("expected bin id to be bumped past %d, got %d", oldBinID, item.BinID) + } + + // the stale bin entry must be gone, leaving exactly one per bin id. + checkStore(t, ts.IndexStore(), &reserve.ChunkBinItem{Bin: bin, BinID: oldBinID}, true) + checkStore(t, ts.IndexStore(), &reserve.ChunkBinItem{Bin: bin, BinID: item.BinID}, false) +} From acb48b23b2d6cb4ef62d7edf8083489a4b1f9808 Mon Sep 17 00:00:00 2001 From: sbackend Date: Sun, 26 Jul 2026 23:02:52 +0200 Subject: [PATCH 07/17] fix: cac divergence --- pkg/storer/internal/reserve/reserve.go | 51 +++++- pkg/storer/internal/reserve/reserve_test.go | 169 ++++++++++++++++++++ 2 files changed, 212 insertions(+), 8 deletions(-) diff --git a/pkg/storer/internal/reserve/reserve.go b/pkg/storer/internal/reserve/reserve.go index 8a67d22731a..1574ba28794 100644 --- a/pkg/storer/internal/reserve/reserve.go +++ b/pkg/storer/internal/reserve/reserve.go @@ -99,6 +99,9 @@ func New( // the existing chunk if the new chunk has a higher stamp timestamp (regardless of batch type). // 3. A new chunk that has the same address belonging to the same stamp index with an already stored chunk will overwrite the existing chunk // if the new chunk has a higher stamp timestamp (regardless of batch type and chunk type, eg CAC & SOC). +// 4. Two different chunk addresses that share the same batch stamp index and timestamp are settled by a tie-break: +// the lexicographically lower chunk address wins. The loser is rejected; the winner replaces the stored chunk +// through the usual remove-and-store path (including a fresh bin ID for pullsync). func (r *Reserve) Put(ctx context.Context, chunk swarm.Chunk) error { socReplaced, err := r.putChunk(ctx, chunk) if err != nil { @@ -178,19 +181,51 @@ func (r *Reserve) putChunk(ctx context.Context, chunk swarm.Chunk) (socReplaced // index collision if loadedStampIndex { - prev := binary.BigEndian.Uint64(oldStampIndex.StampTimestamp) curr := binary.BigEndian.Uint64(chunk.Stamp().Timestamp()) - if prev >= curr { + if prev > curr { return fmt.Errorf("overwrite same chunk. prev %d cur %d batch %s: %w", prev, curr, hex.EncodeToString(chunk.Stamp().BatchID()), storage.ErrOverwriteNewerChunk) } - r.logger.Debug( - "replacing chunk stamp index", - "old_chunk", oldStampIndex.ChunkAddress, - "new_chunk", chunk.Address(), - "batch_id", hex.EncodeToString(chunk.Stamp().BatchID()), - ) + // Same stamp index and timestamp, different chunk addresses: both + // claims are otherwise valid, so settle on the lower address. + if prev == curr && chunkType == swarm.ChunkTypeContentAddressed && !oldStampIndex.ChunkAddress.Equal(chunk.Address()) { + if bytes.Compare(chunk.Address().Bytes(), oldStampIndex.ChunkAddress.Bytes()) >= 0 { + r.logger.Debug( + "discarding stamp index collision", + "old_chunk", oldStampIndex.ChunkAddress, + "new_chunk", chunk.Address(), + "batch_id", hex.EncodeToString(chunk.Stamp().BatchID()), + "stamp_index", hex.EncodeToString(chunk.Stamp().Index()), + "stamp_timestamp", binary.BigEndian.Uint64(chunk.Stamp().Timestamp()), + "incoming_stamp_hash", hex.EncodeToString(stampHash), + "stored_stamp_hash", hex.EncodeToString(oldStampIndex.StampHash), + ) + return fmt.Errorf( + "stamp index collision chunk %s lost tie-break: %w", + chunk.Address(), + storage.ErrDivergentChunkRejected, + ) + } + r.logger.Debug( + "replacing stamp index collision", + "old_chunk", oldStampIndex.ChunkAddress, + "new_chunk", chunk.Address(), + "batch_id", hex.EncodeToString(chunk.Stamp().BatchID()), + "stamp_index", hex.EncodeToString(chunk.Stamp().Index()), + "stamp_timestamp", binary.BigEndian.Uint64(chunk.Stamp().Timestamp()), + "incoming_stamp_hash", hex.EncodeToString(stampHash), + "stored_stamp_hash", hex.EncodeToString(oldStampIndex.StampHash), + ) + // Incoming wins: fall through to removeChunk + store below. + } else { + r.logger.Debug( + "replacing chunk stamp index", + "old_chunk", oldStampIndex.ChunkAddress, + "new_chunk", chunk.Address(), + "batch_id", hex.EncodeToString(chunk.Stamp().BatchID()), + ) + } // same chunk address if oldStampIndex.ChunkAddress.Equal(chunk.Address()) { diff --git a/pkg/storer/internal/reserve/reserve_test.go b/pkg/storer/internal/reserve/reserve_test.go index f8e1686a376..7577ebe5be7 100644 --- a/pkg/storer/internal/reserve/reserve_test.go +++ b/pkg/storer/internal/reserve/reserve_test.go @@ -1611,3 +1611,172 @@ func TestSOCDivergenceBumpsBinID(t *testing.T) { checkStore(t, ts.IndexStore(), &reserve.ChunkBinItem{Bin: bin, BinID: oldBinID}, true) checkStore(t, ts.IndexStore(), &reserve.ChunkBinItem{Bin: bin, BinID: item.BinID}, false) } + +// TestCACStampIndexCollision covers two content-addressed chunks that share a +// batch stamp index and timestamp but have different addresses. The reserve +// keeps the lexicographically lower address regardless of arrival order. +func TestCACStampIndexCollision(t *testing.T) { + t.Parallel() + + ctx := context.Background() + batch := postagetesting.MustNewBatch() + stamp := postagetesting.MustNewFields(batch.ID, 0, 1) + + baseAddr := swarm.RandAddress(t) + ch1 := chunk.GenerateTestRandomChunkAt(t, baseAddr, 0).WithStamp(stamp) + ch2 := chunk.GenerateTestRandomChunkAt(t, baseAddr, 0).WithStamp(stamp.Clone()) + if ch1.Address().Equal(ch2.Address()) { + t.Fatal("expected different CAC addresses") + } + + winner, loser := ch1, ch2 + if bytes.Compare(ch2.Address().Bytes(), ch1.Address().Bytes()) < 0 { + winner, loser = ch2, ch1 + } + + for _, tc := range []struct { + name string + order []swarm.Chunk + }{ + {"winner first", []swarm.Chunk{winner, loser}}, + {"loser first", []swarm.Chunk{loser, winner}}, + } { + t.Run(tc.name, func(t *testing.T) { + t.Parallel() + + ts := internal.NewInmemStorage() + r, err := reserve.New(baseAddr, ts, 0, kademlia.NewTopologyDriver(), log.Noop) + if err != nil { + t.Fatal(err) + } + + if err := r.Put(ctx, tc.order[0]); err != nil { + t.Fatal(err) + } + sizeAfterFirst := r.Size() + + err = r.Put(ctx, tc.order[1]) + if tc.order[1].Address().Equal(loser.Address()) { + if !errors.Is(err, storage.ErrDivergentChunkRejected) { + t.Fatalf("expected ErrDivergentChunkRejected, got %v", err) + } + } else if err != nil { + t.Fatal(err) + } + + if _, err := ts.ChunkStore().Get(ctx, winner.Address()); err != nil { + t.Fatalf("expected winner stored: %v", err) + } + if _, err := ts.ChunkStore().Get(ctx, loser.Address()); !errors.Is(err, storage.ErrNotFound) { + t.Fatalf("expected loser absent, got %v", err) + } + + item, err := stampindex.Load(ts.IndexStore(), "reserve", winner.Stamp()) + if err != nil { + t.Fatal(err) + } + if !item.ChunkAddress.Equal(winner.Address()) { + t.Fatalf("stamp index points to %s, want %s", item.ChunkAddress, winner.Address()) + } + + if got := r.Size(); got != sizeAfterFirst { + t.Fatalf("expected reserve size to stay %d, got %d", sizeAfterFirst, got) + } + + winnerSum, err := storage.ChunkSum(winner) + if err != nil { + t.Fatal(err) + } + loserSum, err := storage.ChunkSum(loser) + if err != nil { + t.Fatal(err) + } + has, err := r.HasSum(winner.Address(), winnerSum) + if err != nil { + t.Fatal(err) + } + if !has { + t.Fatal("expected the winner sum to be indexed") + } + has, err = r.HasSum(loser.Address(), loserSum) + if err != nil { + t.Fatal(err) + } + if has { + t.Fatal("expected the loser sum not to be indexed") + } + }) + } +} + +// TestCACStampIndexCollisionBumpsBinID asserts that accepting a lower-address +// CAC over a stamp-index collision writes it at a fresh bin ID for pullsync. +func TestCACStampIndexCollisionBumpsBinID(t *testing.T) { + t.Parallel() + + ctx := context.Background() + baseAddr := swarm.RandAddress(t) + ts := internal.NewInmemStorage() + r, err := reserve.New(baseAddr, ts, 0, kademlia.NewTopologyDriver(), log.Noop) + if err != nil { + t.Fatal(err) + } + + batch := postagetesting.MustNewBatch() + stamp := postagetesting.MustNewFields(batch.ID, 0, 1) + ch1 := chunk.GenerateTestRandomChunkAt(t, baseAddr, 0).WithStamp(stamp) + ch2 := chunk.GenerateTestRandomChunkAt(t, baseAddr, 0).WithStamp(stamp.Clone()) + + first, second := ch1, ch2 + if bytes.Compare(ch2.Address().Bytes(), ch1.Address().Bytes()) > 0 { + first, second = ch2, ch1 + } + + if err := r.Put(ctx, first); err != nil { + t.Fatal(err) + } + + firstStampHash, err := first.Stamp().Hash() + if err != nil { + t.Fatal(err) + } + firstBin := swarm.Proximity(baseAddr.Bytes(), first.Address().Bytes()) + oldItem := &reserve.BatchRadiusItem{ + Bin: firstBin, + BatchID: batch.ID, + Address: first.Address(), + StampHash: firstStampHash, + } + if err := ts.IndexStore().Get(oldItem); err != nil { + t.Fatal(err) + } + oldBinID := oldItem.BinID + + if err := r.Put(ctx, second); err != nil { + t.Fatal(err) + } + + secondStampHash, err := second.Stamp().Hash() + if err != nil { + t.Fatal(err) + } + secondBin := swarm.Proximity(baseAddr.Bytes(), second.Address().Bytes()) + newItem := &reserve.BatchRadiusItem{ + Bin: secondBin, + BatchID: batch.ID, + Address: second.Address(), + StampHash: secondStampHash, + } + if err := ts.IndexStore().Get(newItem); err != nil { + t.Fatal(err) + } + if secondBin == firstBin && newItem.BinID <= oldBinID { + t.Fatalf("expected bin id to be bumped past %d, got %d", oldBinID, newItem.BinID) + } + + checkStore(t, ts.IndexStore(), &reserve.BatchRadiusItem{ + Bin: firstBin, BatchID: batch.ID, Address: first.Address(), StampHash: firstStampHash, + }, true) + checkStore(t, ts.IndexStore(), &reserve.ChunkBinItem{Bin: firstBin, BinID: oldBinID}, true) + checkStore(t, ts.IndexStore(), &reserve.ChunkBinItem{Bin: secondBin, BinID: newItem.BinID}, false) +} From 269320475a6ca2c983a4558495c9902e617d64f6 Mon Sep 17 00:00:00 2001 From: sbackend Date: Mon, 27 Jul 2026 10:22:23 +0200 Subject: [PATCH 08/17] fix: update branch --- pkg/storer/internal/reserve/reserve_test.go | 3 --- 1 file changed, 3 deletions(-) diff --git a/pkg/storer/internal/reserve/reserve_test.go b/pkg/storer/internal/reserve/reserve_test.go index b19705f9238..7577ebe5be7 100644 --- a/pkg/storer/internal/reserve/reserve_test.go +++ b/pkg/storer/internal/reserve/reserve_test.go @@ -1164,8 +1164,6 @@ func checkChunkInIndexStore(t *testing.T, s storage.Reader, bin uint8, binId uin checkStore(t, s, &reserve.ChunkBinItem{Bin: bin, BinID: binId, StampHash: stampHash}, false) } -<<<<<<< HEAD -======= // TestChunkSumIndexLockstep asserts the invariant the pullsync want-decision // depends on: a ChunkSumItem exists exactly as long as its chunk is in the // reserve. A stale entry would make the node silently refuse to sync a chunk @@ -1449,7 +1447,6 @@ func TestChunkSumIndexRandomOps(t *testing.T) { checkInvariant(200) } ->>>>>>> origin/feat/pullsync-soc-convergence // TestSOCDivergence covers two single owner chunks that share an address, batch // and stamp while wrapping different content. Both are valid, so the storage // layer settles which one the neighborhood keeps, and it must reach the same From a5c800fe82d27375bf756e4b9ad3b84337692d39 Mon Sep 17 00:00:00 2001 From: sbackend Date: Mon, 27 Jul 2026 10:37:26 +0200 Subject: [PATCH 09/17] fix: unsuccessful merge fix + update test --- pkg/storer/internal/reserve/reserve.go | 9 -- pkg/storer/internal/reserve/reserve_test.go | 139 ++++++-------------- 2 files changed, 40 insertions(+), 108 deletions(-) diff --git a/pkg/storer/internal/reserve/reserve.go b/pkg/storer/internal/reserve/reserve.go index 7b8b1e130a0..1574ba28794 100644 --- a/pkg/storer/internal/reserve/reserve.go +++ b/pkg/storer/internal/reserve/reserve.go @@ -140,15 +140,6 @@ func (r *Reserve) putChunk(ctx context.Context, chunk swarm.Chunk) (socReplaced bin := swarm.Proximity(r.baseAddr.Bytes(), chunk.Address().Bytes()) - chunkType := storage.ChunkType(chunk) - - sum, err := storage.ChunkSum(chunk) - if err != nil { - return err - } - - bin := swarm.Proximity(r.baseAddr.Bytes(), chunk.Address().Bytes()) - // check if the chunk with the same batch, stamp timestamp and index is already stored has, err := r.Has(chunk.Address(), chunk.Stamp().BatchID(), stampHash) if err != nil { diff --git a/pkg/storer/internal/reserve/reserve_test.go b/pkg/storer/internal/reserve/reserve_test.go index 7577ebe5be7..e69e6320b4a 100644 --- a/pkg/storer/internal/reserve/reserve_test.go +++ b/pkg/storer/internal/reserve/reserve_test.go @@ -1612,108 +1612,12 @@ func TestSOCDivergenceBumpsBinID(t *testing.T) { checkStore(t, ts.IndexStore(), &reserve.ChunkBinItem{Bin: bin, BinID: item.BinID}, false) } -// TestCACStampIndexCollision covers two content-addressed chunks that share a -// batch stamp index and timestamp but have different addresses. The reserve -// keeps the lexicographically lower address regardless of arrival order. +// TestCACStampIndexCollisionBumps asserts that accepting a lower-address +// CAC over a stamp-index collision writes it at a fresh bin ID for pullsync, +// while the postage stamp index slot still points at the winning chunk. func TestCACStampIndexCollision(t *testing.T) { t.Parallel() - ctx := context.Background() - batch := postagetesting.MustNewBatch() - stamp := postagetesting.MustNewFields(batch.ID, 0, 1) - - baseAddr := swarm.RandAddress(t) - ch1 := chunk.GenerateTestRandomChunkAt(t, baseAddr, 0).WithStamp(stamp) - ch2 := chunk.GenerateTestRandomChunkAt(t, baseAddr, 0).WithStamp(stamp.Clone()) - if ch1.Address().Equal(ch2.Address()) { - t.Fatal("expected different CAC addresses") - } - - winner, loser := ch1, ch2 - if bytes.Compare(ch2.Address().Bytes(), ch1.Address().Bytes()) < 0 { - winner, loser = ch2, ch1 - } - - for _, tc := range []struct { - name string - order []swarm.Chunk - }{ - {"winner first", []swarm.Chunk{winner, loser}}, - {"loser first", []swarm.Chunk{loser, winner}}, - } { - t.Run(tc.name, func(t *testing.T) { - t.Parallel() - - ts := internal.NewInmemStorage() - r, err := reserve.New(baseAddr, ts, 0, kademlia.NewTopologyDriver(), log.Noop) - if err != nil { - t.Fatal(err) - } - - if err := r.Put(ctx, tc.order[0]); err != nil { - t.Fatal(err) - } - sizeAfterFirst := r.Size() - - err = r.Put(ctx, tc.order[1]) - if tc.order[1].Address().Equal(loser.Address()) { - if !errors.Is(err, storage.ErrDivergentChunkRejected) { - t.Fatalf("expected ErrDivergentChunkRejected, got %v", err) - } - } else if err != nil { - t.Fatal(err) - } - - if _, err := ts.ChunkStore().Get(ctx, winner.Address()); err != nil { - t.Fatalf("expected winner stored: %v", err) - } - if _, err := ts.ChunkStore().Get(ctx, loser.Address()); !errors.Is(err, storage.ErrNotFound) { - t.Fatalf("expected loser absent, got %v", err) - } - - item, err := stampindex.Load(ts.IndexStore(), "reserve", winner.Stamp()) - if err != nil { - t.Fatal(err) - } - if !item.ChunkAddress.Equal(winner.Address()) { - t.Fatalf("stamp index points to %s, want %s", item.ChunkAddress, winner.Address()) - } - - if got := r.Size(); got != sizeAfterFirst { - t.Fatalf("expected reserve size to stay %d, got %d", sizeAfterFirst, got) - } - - winnerSum, err := storage.ChunkSum(winner) - if err != nil { - t.Fatal(err) - } - loserSum, err := storage.ChunkSum(loser) - if err != nil { - t.Fatal(err) - } - has, err := r.HasSum(winner.Address(), winnerSum) - if err != nil { - t.Fatal(err) - } - if !has { - t.Fatal("expected the winner sum to be indexed") - } - has, err = r.HasSum(loser.Address(), loserSum) - if err != nil { - t.Fatal(err) - } - if has { - t.Fatal("expected the loser sum not to be indexed") - } - }) - } -} - -// TestCACStampIndexCollisionBumpsBinID asserts that accepting a lower-address -// CAC over a stamp-index collision writes it at a fresh bin ID for pullsync. -func TestCACStampIndexCollisionBumpsBinID(t *testing.T) { - t.Parallel() - ctx := context.Background() baseAddr := swarm.RandAddress(t) ts := internal.NewInmemStorage() @@ -1752,6 +1656,15 @@ func TestCACStampIndexCollisionBumpsBinID(t *testing.T) { } oldBinID := oldItem.BinID + oldStampIndex, err := stampindex.Load(ts.IndexStore(), "reserve", first.Stamp()) + if err != nil { + t.Fatal(err) + } + if !oldStampIndex.ChunkAddress.Equal(first.Address()) { + t.Fatalf("stamp index points to %s, want %s", oldStampIndex.ChunkAddress, first.Address()) + } + wantStampIndex := append([]byte(nil), oldStampIndex.StampIndex...) + if err := r.Put(ctx, second); err != nil { t.Fatal(err) } @@ -1774,6 +1687,34 @@ func TestCACStampIndexCollisionBumpsBinID(t *testing.T) { t.Fatalf("expected bin id to be bumped past %d, got %d", oldBinID, newItem.BinID) } + newStampIndex, err := stampindex.Load(ts.IndexStore(), "reserve", second.Stamp()) + if err != nil { + t.Fatal(err) + } + if !bytes.Equal(newStampIndex.StampIndex, wantStampIndex) { + t.Fatalf("stamp index slot changed: got %x, want %x", newStampIndex.StampIndex, wantStampIndex) + } + if !newStampIndex.ChunkAddress.Equal(second.Address()) { + t.Fatalf("stamp index points to %s, want %s", newStampIndex.ChunkAddress, second.Address()) + } + if !bytes.Equal(newStampIndex.StampHash, secondStampHash) { + t.Fatalf("stamp index hash %x, want %x", newStampIndex.StampHash, secondStampHash) + } + + storedStamp, err := chunkstamp.LoadWithStampHash(ts.IndexStore(), "reserve", second.Address(), secondStampHash) + if err != nil { + t.Fatalf("expected chunkstamp for winning chunk: %v", err) + } + if !bytes.Equal(storedStamp.Index(), wantStampIndex) { + t.Fatalf("stored stamp index %x, want %x", storedStamp.Index(), wantStampIndex) + } + if !bytes.Equal(storedStamp.BatchID(), batch.ID) { + t.Fatalf("stored stamp batch %x, want %x", storedStamp.BatchID(), batch.ID) + } + if _, err := chunkstamp.LoadWithStampHash(ts.IndexStore(), "reserve", first.Address(), firstStampHash); !errors.Is(err, storage.ErrNotFound) { + t.Fatalf("expected old chunkstamp gone, got %v", err) + } + checkStore(t, ts.IndexStore(), &reserve.BatchRadiusItem{ Bin: firstBin, BatchID: batch.ID, Address: first.Address(), StampHash: firstStampHash, }, true) From 91e51c8166cda5171488fa19edcd41e4ed3a2501 Mon Sep 17 00:00:00 2001 From: Calin Martinconi Date: Mon, 27 Jul 2026 12:48:45 +0300 Subject: [PATCH 10/17] test(storer): add arrival-order convergence harness for reserve.Put (SWIP-101) Different nodes receive the same chunks in different orders, so for any set of conflicting chunks every arrival order must leave the reserve in the same final state; an order-dependent outcome means neighborhoods that can never agree. The harness drives conflict sets through every permutation against a fresh reserve and compares canonical state fingerprints (bin IDs excluded: they are order-dependent by design), asserting the sum index invariants on every run as a side effect. Converging on this branch: the equal-timestamp CAC tie-break, the identical-stamp divergent SOC resolution and timestamp ordering. Five constellations are order-dependent, all falling through the same gap: at equal stamp timestamps the tie-break fires only for a content addressed incoming chunk with a different address, and every other case drops into an unconditional replace. These are marked unresolved in the case table: they log the divergence without failing the suite, and setting RESERVE_STRICT_CONVERGENCE=1 turns them into failures, which gives the reserve.Put refactor a concrete target. A case marked unresolved that starts converging fails loudly so the marker is removed and the table stays honest. Testing methodology and findings are documented alongside the refactor notes for reuse. --- .../internal/reserve/convergence_test.go | 363 ++++++++++++++++++ 1 file changed, 363 insertions(+) create mode 100644 pkg/storer/internal/reserve/convergence_test.go diff --git a/pkg/storer/internal/reserve/convergence_test.go b/pkg/storer/internal/reserve/convergence_test.go new file mode 100644 index 00000000000..94cfe0cdfc7 --- /dev/null +++ b/pkg/storer/internal/reserve/convergence_test.go @@ -0,0 +1,363 @@ +// Copyright 2026 The Swarm Authors. All rights reserved. +// Use of this source code is governed by a BSD-style +// license that can be found in the LICENSE file. + +package reserve_test + +import ( + "bytes" + "context" + "errors" + "fmt" + "os" + "sort" + "strings" + "testing" + + "github.com/ethersphere/bee/v2/pkg/cac" + "github.com/ethersphere/bee/v2/pkg/crypto" + "github.com/ethersphere/bee/v2/pkg/log" + postagetesting "github.com/ethersphere/bee/v2/pkg/postage/testing" + "github.com/ethersphere/bee/v2/pkg/soc" + "github.com/ethersphere/bee/v2/pkg/storage" + "github.com/ethersphere/bee/v2/pkg/storer/internal" + "github.com/ethersphere/bee/v2/pkg/storer/internal/reserve" + "github.com/ethersphere/bee/v2/pkg/storer/internal/transaction" + "github.com/ethersphere/bee/v2/pkg/swarm" + kademlia "github.com/ethersphere/bee/v2/pkg/topology/mock" +) + +// This file is a reusable conflict-testing harness for reserve.Put. +// +// The property under test is ARRIVAL-ORDER CONVERGENCE: for any set of +// conflicting chunks, every permutation of arrivals must leave the reserve in +// the same final state. Different nodes receive the same chunks in different +// orders; any order-dependent outcome means neighborhoods that can never +// agree, which breaks the redistribution game. The fingerprint deliberately +// ignores bin IDs (they are order-dependent by design) and compares which +// entries exist and which content each entry serves. Index integrity (the +// chunk sum index in lockstep with the chunk bin index and the chunkstore +// payloads) is asserted on every permutation as a side effect. +// +// To reuse against a reserve.Put refactor: keep the corner-case table, run +// `go test ./pkg/storer/internal/reserve/ -run TestPutOrderConvergence -v`. +// Cases documenting currently unresolved outcomes are marked in the table. + +// benignPutErr reports errors that are legitimate per-chunk outcomes of Put +// rather than failures: losing a tie-break or carrying an older timestamp. +func benignPutErr(err error) bool { + return errors.Is(err, storage.ErrOverwriteNewerChunk) || + errors.Is(err, storage.ErrDivergentChunkRejected) +} + +// reserveFingerprint canonicalizes the reserve state: one line per reserve +// entry (batch, bin, address, stamp hash and the keccak of the payload it +// currently serves) plus the full chunk sum index. While fingerprinting it +// asserts the cross-index invariants. +func reserveFingerprint(t *testing.T, st transaction.Storage) string { + t.Helper() + ctx := context.Background() + + var lines []string + + err := st.IndexStore().Iterate( + storage.Query{Factory: func() storage.Item { return &reserve.BatchRadiusItem{} }}, + func(res storage.Result) (bool, error) { + item := res.Entry.(*reserve.BatchRadiusItem) + ch, err := st.ChunkStore().Get(ctx, item.Address) + if err != nil { + return false, fmt.Errorf("entry %s has no payload: %w", item.Address, err) + } + h := swarm.NewHasher() + _, _ = h.Write(ch.Data()) + lines = append(lines, fmt.Sprintf("entry batch=%x addr=%x stamphash=%x content=%x", + item.BatchID[:8], item.Address.Bytes()[:8], item.StampHash[:8], h.Sum(nil)[:8])) + return false, nil + }, + ) + if err != nil { + t.Fatal(err) + } + + // chunk bin index: sums must match the payload actually served + binSums := make(map[string]struct{}) + err = st.IndexStore().Iterate( + storage.Query{Factory: func() storage.Item { return &reserve.ChunkBinItem{} }}, + func(res storage.Result) (bool, error) { + cbi := res.Entry.(*reserve.ChunkBinItem) + ch, err := st.ChunkStore().Get(ctx, cbi.Address) + if err != nil { + return false, fmt.Errorf("bin entry %s has no payload: %w", cbi.Address, err) + } + want, err := storage.ChunkSumFromParts(cbi.BatchID, cbi.StampHash, ch) + if err != nil { + return false, err + } + if !bytes.Equal(cbi.Sum, want) { + return false, fmt.Errorf("stale sum on entry %s", cbi.Address) + } + binSums[cbi.Address.ByteString()+string(cbi.Sum)] = struct{}{} + lines = append(lines, fmt.Sprintf("sum addr=%x sum=%x", cbi.Address.Bytes()[:8], cbi.Sum[:8])) + return false, nil + }, + ) + if err != nil { + t.Fatal(err) + } + + // chunk sum index: exactly the live (address, sum) set + err = st.IndexStore().Iterate( + storage.Query{ + Factory: func() storage.Item { return &reserve.ChunkSumItem{} }, + ItemProperty: storage.QueryItemID, + }, + func(res storage.Result) (bool, error) { + if _, ok := binSums[res.ID]; !ok { + return false, errors.New("orphaned chunk sum entry") + } + delete(binSums, res.ID) + return false, nil + }, + ) + if err != nil { + t.Fatal(err) + } + if len(binSums) != 0 { + t.Fatal("live entries missing from the chunk sum index") + } + + sort.Strings(lines) + return strings.Join(lines, "\n") +} + +func permutations(n int) [][]int { + if n == 1 { + return [][]int{{0}} + } + var out [][]int + for _, sub := range permutations(n - 1) { + for pos := 0; pos <= len(sub); pos++ { + p := make([]int, 0, n) + p = append(p, sub[:pos]...) + p = append(p, n-1) + p = append(p, sub[pos:]...) + out = append(out, p) + } + } + return out +} + +// runOrder applies the chunks to a fresh reserve in the given order and +// returns the state fingerprint. +func runOrder(t *testing.T, chunks []swarm.Chunk, order []int) string { + t.Helper() + + baseAddr := swarm.NewAddress(make([]byte, swarm.HashSize)) // fixed base: bins irrelevant here + st := internal.NewInmemStorage() + r, err := reserve.New(baseAddr, st, 0, kademlia.NewTopologyDriver(), log.Noop) + if err != nil { + t.Fatal(err) + } + for _, i := range order { + if err := r.Put(context.Background(), chunks[i]); err != nil && !benignPutErr(err) { + t.Fatalf("order %v chunk %d: %v", order, i, err) + } + } + return reserveFingerprint(t, st) +} + +// assertOrderConvergence checks every permutation. For cases marked +// unresolved the divergence is reported without failing, so the harness +// documents the open holes while keeping the suite green; set +// RESERVE_STRICT_CONVERGENCE=1 to turn them into failures (useful while +// refactoring reserve.Put toward full order independence). An unresolved case +// that starts converging fails loudly so the marker gets removed. +func assertOrderConvergence(t *testing.T, chunks []swarm.Chunk, unresolved bool) { + t.Helper() + + strict := os.Getenv("RESERVE_STRICT_CONVERGENCE") != "" + perms := permutations(len(chunks)) + first := runOrder(t, chunks, perms[0]) + for _, p := range perms[1:] { + fp := runOrder(t, chunks, p) + if fp != first { + msg := fmt.Sprintf("order-dependent outcome:\norder %v ends with:\n%s\n\norder %v ends with:\n%s", + perms[0], first, p, fp) + if unresolved && !strict { + t.Logf("KNOWN UNRESOLVED (not failing, set RESERVE_STRICT_CONVERGENCE=1 to enforce):\n%s", msg) + } else { + t.Error(msg) + } + return + } + } + if unresolved { + t.Error("case marked unresolved now converges: remove the unresolved marker") + } +} + +func newTestSOC(t *testing.T, signer crypto.Signer, id, payload []byte) swarm.Chunk { + t.Helper() + inner, err := cac.New(payload) + if err != nil { + t.Fatal(err) + } + ch, err := soc.New(id, inner).Sign(signer) + if err != nil { + t.Fatal(err) + } + return ch +} + +func newTestCAC(t *testing.T, payload []byte) swarm.Chunk { + t.Helper() + ch, err := cac.New(payload) + if err != nil { + t.Fatal(err) + } + return ch +} + +// TestPutOrderConvergence drives conflicting chunk sets through every arrival +// order and requires an identical final state. +func TestPutOrderConvergence(t *testing.T) { + t.Parallel() + + signer := getSigner(t) + batchA := postagetesting.MustNewBatch() + batchB := postagetesting.MustNewBatch() + id1 := make([]byte, swarm.HashSize) + id2 := bytes.Repeat([]byte{1}, swarm.HashSize) + + for _, tc := range []struct { + name string + unresolved bool + chunks func(t *testing.T) []swarm.Chunk + }{ + { + // Sofia's rule 4 core case: the tie-break must make this converge. + name: "cac vs cac, same slot, equal timestamp", + chunks: func(t *testing.T) []swarm.Chunk { + t.Helper() + return []swarm.Chunk{ + newTestCAC(t, []byte("cac payload one")).WithStamp(postagetesting.MustNewFields(batchA.ID, 0, 7)), + newTestCAC(t, []byte("cac payload two")).WithStamp(postagetesting.MustNewFields(batchA.ID, 0, 7)), + } + }, + }, + { + // Phase 2 core case: same SOC address, byte-identical stamp, + // different payloads; resolveDivergence must make this converge. + name: "divergent socs, identical stamp", + chunks: func(t *testing.T) []swarm.Chunk { + t.Helper() + stamp := postagetesting.MustNewFields(batchA.ID, 0, 7) + return []swarm.Chunk{ + newTestSOC(t, signer, id1, []byte("soc payload one")).WithStamp(stamp), + newTestSOC(t, signer, id1, []byte("soc payload two")).WithStamp(stamp), + } + }, + }, + { + // Newer timestamps must win regardless of order or type. + name: "soc update, increasing timestamps", + chunks: func(t *testing.T) []swarm.Chunk { + t.Helper() + return []swarm.Chunk{ + newTestSOC(t, signer, id1, []byte("soc v1")).WithStamp(postagetesting.MustNewFields(batchA.ID, 0, 1)), + newTestSOC(t, signer, id1, []byte("soc v2")).WithStamp(postagetesting.MustNewFields(batchA.ID, 0, 2)), + } + }, + }, + { + // UNRESOLVED on this branch: same SOC address, same slot and + // timestamp, but separately stamped (distinct signatures, hence + // distinct stamp hashes). Bypasses resolveDivergence (stamp hashes + // differ) and the CAC tie-break (wrong type): last write wins. + name: "divergent socs, equal timestamp, distinct stamps", + unresolved: true, + chunks: func(t *testing.T) []swarm.Chunk { + t.Helper() + return []swarm.Chunk{ + newTestSOC(t, signer, id1, []byte("soc payload one")).WithStamp(postagetesting.MustNewFields(batchA.ID, 0, 7)), + newTestSOC(t, signer, id1, []byte("soc payload two")).WithStamp(postagetesting.MustNewFields(batchA.ID, 0, 7)), + } + }, + }, + { + // UNRESOLVED on this branch: different SOC addresses in the same + // slot at the same timestamp. Not content addressed, so the + // tie-break is skipped: last write wins. + name: "soc vs soc, different addresses, same slot, equal timestamp", + unresolved: true, + chunks: func(t *testing.T) []swarm.Chunk { + t.Helper() + return []swarm.Chunk{ + newTestSOC(t, signer, id1, []byte("soc payload one")).WithStamp(postagetesting.MustNewFields(batchA.ID, 0, 7)), + newTestSOC(t, signer, id2, []byte("soc payload two")).WithStamp(postagetesting.MustNewFields(batchA.ID, 0, 7)), + } + }, + }, + { + // UNRESOLVED on this branch: mixed types in the same slot at the + // same timestamp. The tie-break fires only when the INCOMING chunk + // is content addressed, so the two directions disagree whenever + // the CAC has the lower address. + name: "cac vs soc, same slot, equal timestamp, cac address lower", + unresolved: true, + chunks: func(t *testing.T) []swarm.Chunk { + t.Helper() + socCh := newTestSOC(t, signer, id1, []byte("soc payload")) + // search a payload whose CAC address sorts below the SOC's + for i := range 64 { + cacCh := newTestCAC(t, fmt.Appendf(nil, "cac payload %d", i)) + if bytes.Compare(cacCh.Address().Bytes(), socCh.Address().Bytes()) < 0 { + return []swarm.Chunk{ + socCh.WithStamp(postagetesting.MustNewFields(batchA.ID, 0, 7)), + cacCh.WithStamp(postagetesting.MustNewFields(batchA.ID, 0, 7)), + } + } + } + t.Fatal("no lower cac address found") + return nil + }, + }, + { + // UNRESOLVED on this branch: byte-identical CAC re-stamped in the + // same slot at the same timestamp with a different signature. The + // entries swap stamp hashes depending on order, so peers holding + // different stampings keep exchanging and replacing forever. + name: "identical cac, same slot, equal timestamp, distinct stamps", + unresolved: true, + chunks: func(t *testing.T) []swarm.Chunk { + t.Helper() + payload := []byte("identical cac payload") + return []swarm.Chunk{ + newTestCAC(t, payload).WithStamp(postagetesting.MustNewFields(batchA.ID, 0, 7)), + newTestCAC(t, payload).WithStamp(postagetesting.MustNewFields(batchA.ID, 0, 7)), + } + }, + }, + { + // Three-way conflict across batches: the batch B entry must end + // serving whatever payload the batch A conflict settles on, with + // its sum refreshed accordingly. + name: "divergent socs with a sibling entry under another batch", + unresolved: true, + chunks: func(t *testing.T) []swarm.Chunk { + t.Helper() + stamp := postagetesting.MustNewFields(batchA.ID, 0, 7) + return []swarm.Chunk{ + newTestSOC(t, signer, id1, []byte("soc payload one")).WithStamp(stamp), + newTestSOC(t, signer, id1, []byte("soc payload two")).WithStamp(stamp), + newTestSOC(t, signer, id1, []byte("soc payload one")).WithStamp(postagetesting.MustNewFields(batchB.ID, 0, 7)), + } + }, + }, + } { + t.Run(tc.name, func(t *testing.T) { + t.Parallel() + assertOrderConvergence(t, tc.chunks(t), tc.unresolved) + }) + } +} From a21f9b0c6526899255bda92f7b83ee65f544986e Mon Sep 17 00:00:00 2001 From: sbackend Date: Mon, 27 Jul 2026 20:02:06 +0200 Subject: [PATCH 11/17] fix: update SOC chunk only with newer ts --- pkg/storer/internal/reserve/reserve.go | 41 +++++ pkg/storer/internal/reserve/reserve_test.go | 177 +++++++++++++++++++- 2 files changed, 210 insertions(+), 8 deletions(-) diff --git a/pkg/storer/internal/reserve/reserve.go b/pkg/storer/internal/reserve/reserve.go index 1574ba28794..e12824b13b1 100644 --- a/pkg/storer/internal/reserve/reserve.go +++ b/pkg/storer/internal/reserve/reserve.go @@ -102,6 +102,10 @@ func New( // 4. Two different chunk addresses that share the same batch stamp index and timestamp are settled by a tie-break: // the lexicographically lower chunk address wins. The loser is rejected; the winner replaces the stored chunk // through the usual remove-and-store path (including a fresh bin ID for pullsync). +// 5. Two single owner chunks that share an address under different stamps (any batch or stamp +// index) settle on one shared payload: a strictly higher stamp timestamp replaces it; equal +// timestamps are settled by the lexicographically lower stamp hash. An older stamp is +// rejected. Same-stamp divergence remains handled by resolveDivergence above. func (r *Reserve) Put(ctx context.Context, chunk swarm.Chunk) error { socReplaced, err := r.putChunk(ctx, chunk) if err != nil { @@ -174,6 +178,12 @@ func (r *Reserve) putChunk(ctx context.Context, chunk swarm.Chunk) (socReplaced var shouldIncReserveSize bool err = r.st.Run(ctx, func(s transaction.Store) error { + if chunkType == swarm.ChunkTypeSingleOwner { + if err := checkSOCStampOverwrite(ctx, s, chunk, stampHash); err != nil { + return err + } + } + oldStampIndex, loadedStampIndex, err := stampindex.LoadOrStore(s.IndexStore(), reserveScope, chunk) if err != nil { return fmt.Errorf("load or store stamp index for chunk %v has fail: %w", chunk, err) @@ -381,6 +391,37 @@ func (r *Reserve) putChunk(ctx context.Context, chunk swarm.Chunk) (socReplaced return socReplaced, nil } +// checkSOCStampOverwrite rejects an incoming single owner chunk when the +// address already holds a payload under a stamp that should keep winning: +// a strictly higher timestamp, or an equal timestamp with a lower or equal +// stamp hash. Must run before LoadOrStore, which writes immediately. +func checkSOCStampOverwrite(ctx context.Context, s transaction.Store, chunk swarm.Chunk, stampHash []byte) error { + hasPayload, err := s.ChunkStore().Has(ctx, chunk.Address()) + if err != nil || !hasPayload { + return err + } + + curr := binary.BigEndian.Uint64(chunk.Stamp().Timestamp()) + return chunkstamp.IterateAll(s.IndexStore(), reserveScope, chunk.Address(), func(st swarm.Stamp) (bool, error) { + prev := binary.BigEndian.Uint64(st.Timestamp()) + if prev > curr { + return true, fmt.Errorf("overwrite same chunk. prev %d cur %d batch %s: %w", + prev, curr, hex.EncodeToString(chunk.Stamp().BatchID()), storage.ErrOverwriteNewerChunk) + } + if prev == curr { + prevHash, err := st.Hash() + if err != nil { + return true, err + } + if bytes.Compare(prevHash, stampHash) <= 0 { + return true, fmt.Errorf("overwrite same chunk. prev %d cur %d batch %s: %w", + prev, curr, hex.EncodeToString(chunk.Stamp().BatchID()), storage.ErrOverwriteNewerChunk) + } + } + return false, nil + }) +} + // refreshSiblingSums recomputes the divergence checksum of every reserve entry // at the given address after its shared payload was replaced. Without the // refresh, entries under other stamps keep advertising content the node no diff --git a/pkg/storer/internal/reserve/reserve_test.go b/pkg/storer/internal/reserve/reserve_test.go index e69e6320b4a..172827998e9 100644 --- a/pkg/storer/internal/reserve/reserve_test.go +++ b/pkg/storer/internal/reserve/reserve_test.go @@ -201,14 +201,12 @@ func TestSameChunkAddress(t *testing.T) { bin := swarm.Proximity(baseAddr.Bytes(), ch1.Address().Bytes()) binBinIDs[bin] += 1 err = r.Put(ctx, ch2) - if err != nil { - t.Fatal(err) + if !errors.Is(err, storage.ErrOverwriteNewerChunk) { + t.Fatal("expected error") } - bin2 := swarm.Proximity(baseAddr.Bytes(), ch2.Address().Bytes()) - binBinIDs[bin2] += 1 size2 := r.Size() - if size2-size1 != 2 { - t.Fatalf("expected reserve size to increase by 2, got %d", size2-size1) + if size2-size1 != 1 { + t.Fatalf("expected reserve size to increase by 1, got %d", size2-size1) } }) @@ -1241,7 +1239,7 @@ func TestSOCSiblingSumRefresh(t *testing.T) { s3 := soctesting.GenerateMockSocWithSigner(t, []byte("v3"), signer) stampA := postagetesting.MustNewFields(batchA.ID, 0, 1) - stampB := postagetesting.MustNewFields(batchB.ID, 0, 1) + stampB := postagetesting.MustNewFields(batchB.ID, 0, 2) chA := s1.Chunk().WithStamp(stampA) chB := s2.Chunk().WithStamp(stampB) @@ -1308,7 +1306,7 @@ func TestSOCSiblingSumRefresh(t *testing.T) { // the same-batch replacement path (higher stamp timestamp) must refresh // batch B's entry the same way. staleSumB := sumOf(batchB.ID, stampHashB) - chA2 := s3.Chunk().WithStamp(postagetesting.MustNewFields(batchA.ID, 0, 2)) + chA2 := s3.Chunk().WithStamp(postagetesting.MustNewFields(batchA.ID, 0, 3)) if err := r.Put(ctx, chA2); err != nil { t.Fatal(err) } @@ -1447,6 +1445,169 @@ func TestChunkSumIndexRandomOps(t *testing.T) { checkInvariant(200) } +// TestSOCCrossBatchTimestamp covers two single owner chunks that share an +// address but are stamped under different batches. The shared chunkstore +// payload is replaced when the incoming stamp timestamp is strictly higher, +// or when timestamps are equal and the incoming stamp hash is lower. An older +// stamp is rejected so neighborhoods converge. +func TestSOCCrossBatchTimestamp(t *testing.T) { + t.Parallel() + + ctx := context.Background() + signer := getSigner(t) + batchA := postagetesting.MustNewBatch() + batchB := postagetesting.MustNewBatch() + + sOlder := soctesting.GenerateMockSocWithSigner(t, []byte("older"), signer) + sNewer := soctesting.GenerateMockSocWithSigner(t, []byte("newer"), signer) + if !sOlder.Chunk().Address().Equal(sNewer.Chunk().Address()) { + t.Fatal("expected shared SOC address") + } + + t.Run("higher timestamp replaces", func(t *testing.T) { + t.Parallel() + + baseAddr := swarm.RandAddress(t) + ts := internal.NewInmemStorage() + r, err := reserve.New(baseAddr, ts, 0, kademlia.NewTopologyDriver(), log.Noop) + if err != nil { + t.Fatal(err) + } + + older := sOlder.Chunk().WithStamp(postagetesting.MustNewFields(batchA.ID, 0, 1)) + newer := sNewer.Chunk().WithStamp(postagetesting.MustNewFields(batchB.ID, 0, 2)) + + if err := r.Put(ctx, older); err != nil { + t.Fatal(err) + } + if err := r.Put(ctx, newer); err != nil { + t.Fatal(err) + } + + got, err := ts.ChunkStore().Get(ctx, newer.Address()) + if err != nil { + t.Fatal(err) + } + if !bytes.Equal(got.Data(), newer.Data()) { + t.Fatal("expected payload from the higher-timestamp stamp") + } + }) + + t.Run("equal timestamp stamp hash tie-break", func(t *testing.T) { + t.Parallel() + + chA := sOlder.Chunk().WithStamp(postagetesting.MustNewFields(batchA.ID, 0, 5)) + chB := sNewer.Chunk().WithStamp(postagetesting.MustNewFields(batchB.ID, 0, 5)) + hashA, err := chA.Stamp().Hash() + if err != nil { + t.Fatal(err) + } + hashB, err := chB.Stamp().Hash() + if err != nil { + t.Fatal(err) + } + var winner, loser swarm.Chunk + if bytes.Compare(hashA, hashB) < 0 { + winner, loser = chA, chB + } else { + winner, loser = chB, chA + } + + for _, order := range [][]swarm.Chunk{{winner, loser}, {loser, winner}} { + baseAddr := swarm.RandAddress(t) + ts := internal.NewInmemStorage() + r, err := reserve.New(baseAddr, ts, 0, kademlia.NewTopologyDriver(), log.Noop) + if err != nil { + t.Fatal(err) + } + if err := r.Put(ctx, order[0]); err != nil { + t.Fatal(err) + } + _ = r.Put(ctx, order[1]) // may reject when winner is already stored + + got, err := ts.ChunkStore().Get(ctx, winner.Address()) + if err != nil { + t.Fatal(err) + } + if !bytes.Equal(got.Data(), winner.Data()) { + t.Fatal("expected payload from the lower stamp-hash claim") + } + } + }) + + t.Run("lower timestamp rejected", func(t *testing.T) { + t.Parallel() + + baseAddr := swarm.RandAddress(t) + ts := internal.NewInmemStorage() + r, err := reserve.New(baseAddr, ts, 0, kademlia.NewTopologyDriver(), log.Noop) + if err != nil { + t.Fatal(err) + } + + newer := sNewer.Chunk().WithStamp(postagetesting.MustNewFields(batchA.ID, 0, 9)) + older := sOlder.Chunk().WithStamp(postagetesting.MustNewFields(batchB.ID, 0, 3)) + + if err := r.Put(ctx, newer); err != nil { + t.Fatal(err) + } + err = r.Put(ctx, older) + if !errors.Is(err, storage.ErrOverwriteNewerChunk) { + t.Fatalf("expected ErrOverwriteNewerChunk, got %v", err) + } + + got, err := ts.ChunkStore().Get(ctx, newer.Address()) + if err != nil { + t.Fatal(err) + } + if !bytes.Equal(got.Data(), newer.Data()) { + t.Fatal("expected newer payload to remain") + } + }) + + t.Run("same batch different stamp index", func(t *testing.T) { + t.Parallel() + + chLow := sOlder.Chunk().WithStamp(postagetesting.MustNewFields(batchA.ID, 0, 5)) + chHigh := sNewer.Chunk().WithStamp(postagetesting.MustNewFields(batchA.ID, 1, 5)) + hashLow, err := chLow.Stamp().Hash() + if err != nil { + t.Fatal(err) + } + hashHigh, err := chHigh.Stamp().Hash() + if err != nil { + t.Fatal(err) + } + var winner, loser swarm.Chunk + if bytes.Compare(hashLow, hashHigh) < 0 { + winner, loser = chLow, chHigh + } else { + winner, loser = chHigh, chLow + } + + for _, order := range [][]swarm.Chunk{{winner, loser}, {loser, winner}} { + baseAddr := swarm.RandAddress(t) + ts := internal.NewInmemStorage() + r, err := reserve.New(baseAddr, ts, 0, kademlia.NewTopologyDriver(), log.Noop) + if err != nil { + t.Fatal(err) + } + if err := r.Put(ctx, order[0]); err != nil { + t.Fatal(err) + } + _ = r.Put(ctx, order[1]) + + got, err := ts.ChunkStore().Get(ctx, winner.Address()) + if err != nil { + t.Fatal(err) + } + if !bytes.Equal(got.Data(), winner.Data()) { + t.Fatal("expected payload from the lower stamp-hash claim") + } + } + }) +} + // TestSOCDivergence covers two single owner chunks that share an address, batch // and stamp while wrapping different content. Both are valid, so the storage // layer settles which one the neighborhood keeps, and it must reach the same From ad8e65033f62c62e06ffd27429de2f5a162aac7c Mon Sep 17 00:00:00 2001 From: sbackend Date: Mon, 27 Jul 2026 20:36:37 +0200 Subject: [PATCH 12/17] fix: another convergence issues --- .../internal/reserve/convergence_test.go | 38 ++++++++----------- pkg/storer/internal/reserve/reserve.go | 10 ++++- 2 files changed, 24 insertions(+), 24 deletions(-) diff --git a/pkg/storer/internal/reserve/convergence_test.go b/pkg/storer/internal/reserve/convergence_test.go index 94cfe0cdfc7..805366cb9a4 100644 --- a/pkg/storer/internal/reserve/convergence_test.go +++ b/pkg/storer/internal/reserve/convergence_test.go @@ -270,12 +270,11 @@ func TestPutOrderConvergence(t *testing.T) { }, }, { - // UNRESOLVED on this branch: same SOC address, same slot and - // timestamp, but separately stamped (distinct signatures, hence - // distinct stamp hashes). Bypasses resolveDivergence (stamp hashes - // differ) and the CAC tie-break (wrong type): last write wins. - name: "divergent socs, equal timestamp, distinct stamps", - unresolved: true, + // Same SOC address, same slot and timestamp, but separately stamped + // (distinct signatures, hence distinct stamp hashes). The SOC + // stamp-overwrite guard settles on the lower stamp hash, so the + // shared payload converges regardless of order. + name: "divergent socs, equal timestamp, distinct stamps", chunks: func(t *testing.T) []swarm.Chunk { t.Helper() return []swarm.Chunk{ @@ -285,11 +284,9 @@ func TestPutOrderConvergence(t *testing.T) { }, }, { - // UNRESOLVED on this branch: different SOC addresses in the same - // slot at the same timestamp. Not content addressed, so the - // tie-break is skipped: last write wins. - name: "soc vs soc, different addresses, same slot, equal timestamp", - unresolved: true, + // Different SOC addresses in the same slot at the same timestamp. + // The lower-address tie-break now applies to all chunk types. + name: "soc vs soc, different addresses, same slot, equal timestamp", chunks: func(t *testing.T) []swarm.Chunk { t.Helper() return []swarm.Chunk{ @@ -299,12 +296,9 @@ func TestPutOrderConvergence(t *testing.T) { }, }, { - // UNRESOLVED on this branch: mixed types in the same slot at the - // same timestamp. The tie-break fires only when the INCOMING chunk - // is content addressed, so the two directions disagree whenever - // the CAC has the lower address. - name: "cac vs soc, same slot, equal timestamp, cac address lower", - unresolved: true, + // Mixed types in the same slot at the same timestamp. The + // lower-address tie-break is type-agnostic, so convergence holds. + name: "cac vs soc, same slot, equal timestamp, cac address lower", chunks: func(t *testing.T) []swarm.Chunk { t.Helper() socCh := newTestSOC(t, signer, id1, []byte("soc payload")) @@ -323,12 +317,10 @@ func TestPutOrderConvergence(t *testing.T) { }, }, { - // UNRESOLVED on this branch: byte-identical CAC re-stamped in the - // same slot at the same timestamp with a different signature. The - // entries swap stamp hashes depending on order, so peers holding - // different stampings keep exchanging and replacing forever. - name: "identical cac, same slot, equal timestamp, distinct stamps", - unresolved: true, + // Byte-identical CAC re-stamped in the same slot at the same + // timestamp with a different signature. The stamp-hash tie-break + // settles on one stamping deterministically. + name: "identical cac, same slot, equal timestamp, distinct stamps", chunks: func(t *testing.T) []swarm.Chunk { t.Helper() payload := []byte("identical cac payload") diff --git a/pkg/storer/internal/reserve/reserve.go b/pkg/storer/internal/reserve/reserve.go index e12824b13b1..4f909c73237 100644 --- a/pkg/storer/internal/reserve/reserve.go +++ b/pkg/storer/internal/reserve/reserve.go @@ -199,7 +199,7 @@ func (r *Reserve) putChunk(ctx context.Context, chunk swarm.Chunk) (socReplaced // Same stamp index and timestamp, different chunk addresses: both // claims are otherwise valid, so settle on the lower address. - if prev == curr && chunkType == swarm.ChunkTypeContentAddressed && !oldStampIndex.ChunkAddress.Equal(chunk.Address()) { + if prev == curr && !oldStampIndex.ChunkAddress.Equal(chunk.Address()) { if bytes.Compare(chunk.Address().Bytes(), oldStampIndex.ChunkAddress.Bytes()) >= 0 { r.logger.Debug( "discarding stamp index collision", @@ -239,6 +239,14 @@ func (r *Reserve) putChunk(ctx context.Context, chunk swarm.Chunk) (socReplaced // same chunk address if oldStampIndex.ChunkAddress.Equal(chunk.Address()) { + // Same address, same timestamp: settle on the lower stamp hash. + if prev == curr && bytes.Compare(oldStampIndex.StampHash, stampHash) <= 0 { + return fmt.Errorf( + "stamp index collision chunk %s lost stamp-hash tie-break: %w", + chunk.Address(), + storage.ErrOverwriteNewerChunk, + ) + } oldStamp, err := chunkstamp.LoadWithStampHash(s.IndexStore(), reserveScope, oldStampIndex.ChunkAddress, oldStampIndex.StampHash) if err != nil { From 2f164bb8b941e658a974e102b4582d4000479309 Mon Sep 17 00:00:00 2001 From: sbackend Date: Tue, 28 Jul 2026 09:24:23 +0200 Subject: [PATCH 13/17] chore: refactoring --- pkg/storer/internal/reserve/reserve.go | 407 ++++++++++++++----------- 1 file changed, 224 insertions(+), 183 deletions(-) diff --git a/pkg/storer/internal/reserve/reserve.go b/pkg/storer/internal/reserve/reserve.go index 4f909c73237..d2e0b4ef917 100644 --- a/pkg/storer/internal/reserve/reserve.go +++ b/pkg/storer/internal/reserve/reserve.go @@ -176,233 +176,273 @@ func (r *Reserve) putChunk(ctx context.Context, chunk swarm.Chunk) (socReplaced defer r.multx.Unlock(strconv.Itoa(int(bin))) var shouldIncReserveSize bool + if chunkType == swarm.ChunkTypeSingleOwner { + socReplaced, shouldIncReserveSize, err = r.putSOC(ctx, chunk, sum, stampHash, bin) + } else { + shouldIncReserveSize, err = r.putCAC(ctx, chunk, sum, stampHash, bin) + } + if err != nil { + return false, err + } + if shouldIncReserveSize { + r.size.Add(1) + } + return socReplaced, nil +} +func (r *Reserve) putSOC(ctx context.Context, chunk swarm.Chunk, sum, stampHash []byte, bin uint8) (socReplaced, shouldInc bool, err error) { err = r.st.Run(ctx, func(s transaction.Store) error { - if chunkType == swarm.ChunkTypeSingleOwner { - if err := checkSOCStampOverwrite(ctx, s, chunk, stampHash); err != nil { - return err - } + if err := checkSOCStampOverwrite(ctx, s, chunk, stampHash); err != nil { + return err } - oldStampIndex, loadedStampIndex, err := stampindex.LoadOrStore(s.IndexStore(), reserveScope, chunk) + oldStampIndex, loaded, err := stampindex.LoadOrStore(s.IndexStore(), reserveScope, chunk) if err != nil { return fmt.Errorf("load or store stamp index for chunk %v has fail: %w", chunk, err) } - // index collision - if loadedStampIndex { - prev := binary.BigEndian.Uint64(oldStampIndex.StampTimestamp) - curr := binary.BigEndian.Uint64(chunk.Stamp().Timestamp()) - if prev > curr { - return fmt.Errorf("overwrite same chunk. prev %d cur %d batch %s: %w", prev, curr, hex.EncodeToString(chunk.Stamp().BatchID()), storage.ErrOverwriteNewerChunk) + if loaded { + sameAddr, err := r.resolveStampIndexCollision(ctx, s, chunk, oldStampIndex, sum, stampHash, bin, swarm.ChunkTypeSingleOwner) + if err != nil { + return err } - - // Same stamp index and timestamp, different chunk addresses: both - // claims are otherwise valid, so settle on the lower address. - if prev == curr && !oldStampIndex.ChunkAddress.Equal(chunk.Address()) { - if bytes.Compare(chunk.Address().Bytes(), oldStampIndex.ChunkAddress.Bytes()) >= 0 { - r.logger.Debug( - "discarding stamp index collision", - "old_chunk", oldStampIndex.ChunkAddress, - "new_chunk", chunk.Address(), - "batch_id", hex.EncodeToString(chunk.Stamp().BatchID()), - "stamp_index", hex.EncodeToString(chunk.Stamp().Index()), - "stamp_timestamp", binary.BigEndian.Uint64(chunk.Stamp().Timestamp()), - "incoming_stamp_hash", hex.EncodeToString(stampHash), - "stored_stamp_hash", hex.EncodeToString(oldStampIndex.StampHash), - ) - return fmt.Errorf( - "stamp index collision chunk %s lost tie-break: %w", - chunk.Address(), - storage.ErrDivergentChunkRejected, - ) - } - r.logger.Debug( - "replacing stamp index collision", - "old_chunk", oldStampIndex.ChunkAddress, - "new_chunk", chunk.Address(), - "batch_id", hex.EncodeToString(chunk.Stamp().BatchID()), - "stamp_index", hex.EncodeToString(chunk.Stamp().Index()), - "stamp_timestamp", binary.BigEndian.Uint64(chunk.Stamp().Timestamp()), - "incoming_stamp_hash", hex.EncodeToString(stampHash), - "stored_stamp_hash", hex.EncodeToString(oldStampIndex.StampHash), - ) - // Incoming wins: fall through to removeChunk + store below. - } else { - r.logger.Debug( - "replacing chunk stamp index", - "old_chunk", oldStampIndex.ChunkAddress, - "new_chunk", chunk.Address(), - "batch_id", hex.EncodeToString(chunk.Stamp().BatchID()), - ) + if sameAddr { + r.logger.Debug("replacing soc in chunkstore", "address", chunk.Address()) + socReplaced = true + return s.ChunkStore().Replace(ctx, chunk, false) } + } - // same chunk address - if oldStampIndex.ChunkAddress.Equal(chunk.Address()) { - // Same address, same timestamp: settle on the lower stamp hash. - if prev == curr && bytes.Compare(oldStampIndex.StampHash, stampHash) <= 0 { - return fmt.Errorf( - "stamp index collision chunk %s lost stamp-hash tie-break: %w", - chunk.Address(), - storage.ErrOverwriteNewerChunk, - ) - } - - oldStamp, err := chunkstamp.LoadWithStampHash(s.IndexStore(), reserveScope, oldStampIndex.ChunkAddress, oldStampIndex.StampHash) - if err != nil { - return err - } - - oldBatchRadiusItem := &BatchRadiusItem{ - Bin: bin, - Address: oldStampIndex.ChunkAddress, - BatchID: oldStampIndex.BatchID, - StampHash: oldStampIndex.StampHash, - } - // load item to get the binID - err = s.IndexStore().Get(oldBatchRadiusItem) - if err != nil { - return err - } - - // delete old chunk index items - err = errors.Join( - s.IndexStore().Delete(oldBatchRadiusItem), - deleteChunkBinItem(s.IndexStore(), oldBatchRadiusItem.Bin, oldBatchRadiusItem.BinID), - stampindex.Delete(s.IndexStore(), reserveScope, oldStamp), - chunkstamp.DeleteWithStamp(s.IndexStore(), reserveScope, oldBatchRadiusItem.Address, oldStamp), - ) - if err != nil { - return err - } - - binID, err := r.IncBinID(s.IndexStore(), bin) - if err != nil { - return err - } + if err := r.storeReserveEntries(s, chunk, sum, stampHash, bin); err != nil { + return err + } - err = errors.Join( - stampindex.Store(s.IndexStore(), reserveScope, chunk), - chunkstamp.Store(s.IndexStore(), reserveScope, chunk), - s.IndexStore().Put(&BatchRadiusItem{ - Bin: bin, - BinID: binID, - Address: chunk.Address(), - BatchID: chunk.Stamp().BatchID(), - StampHash: stampHash, - }), - s.IndexStore().Put(&ChunkBinItem{ - Bin: bin, - BinID: binID, - Address: chunk.Address(), - BatchID: chunk.Stamp().BatchID(), - ChunkType: chunkType, - StampHash: stampHash, - Sum: sum, - }), - s.IndexStore().Put(&ChunkSumItem{Address: chunk.Address(), Sum: sum}), - ) - if err != nil { - return err - } + has, err := s.ChunkStore().Has(ctx, chunk.Address()) + if err != nil { + return err + } + if has { + r.logger.Debug("replacing soc in chunkstore", "address", chunk.Address()) + socReplaced = true + err = s.ChunkStore().Replace(ctx, chunk, true) + } else { + err = s.ChunkStore().Put(ctx, chunk) + } + if err != nil { + return err + } - if chunkType == swarm.ChunkTypeSingleOwner { - r.logger.Debug("replacing soc in chunkstore", "address", chunk.Address()) - socReplaced = true - return s.ChunkStore().Replace(ctx, chunk, false) - } + shouldInc = !loaded + return nil + }) + return +} - return nil - } +func (r *Reserve) putCAC(ctx context.Context, chunk swarm.Chunk, sum, stampHash []byte, bin uint8) (shouldInc bool, err error) { + chunkType := storage.ChunkType(chunk) - // An older and different chunk with the same batchID and stamp index has been previously - // saved to the reserve. We must do the below before saving the new chunk: - // 1. Delete the old chunk from the chunkstore. - // 2. Delete the old chunk's stamp data. - // 3. Delete ALL old chunk related items from the reserve. - // 4. Update the stamp index. + err = r.st.Run(ctx, func(s transaction.Store) error { + oldStampIndex, loaded, err := stampindex.LoadOrStore(s.IndexStore(), reserveScope, chunk) + if err != nil { + return fmt.Errorf("load or store stamp index for chunk %v has fail: %w", chunk, err) + } - err = r.removeChunk(ctx, s, oldStampIndex.ChunkAddress, oldStampIndex.BatchID, oldStampIndex.StampHash) + if loaded { + sameAddr, err := r.resolveStampIndexCollision(ctx, s, chunk, oldStampIndex, sum, stampHash, bin, chunkType) if err != nil { - return fmt.Errorf("failed removing older chunk %s: %w", oldStampIndex.ChunkAddress, err) + return err } - - // replace old stamp index. - err = stampindex.Store(s.IndexStore(), reserveScope, chunk) - if err != nil { - return fmt.Errorf("failed updating stamp index: %w", err) + if sameAddr { + return nil } } - binID, err := r.IncBinID(s.IndexStore(), bin) - if err != nil { + if err := r.storeReserveEntries(s, chunk, sum, stampHash, bin); err != nil { return err } - err = errors.Join( - chunkstamp.Store(s.IndexStore(), reserveScope, chunk), - s.IndexStore().Put(&BatchRadiusItem{ - Bin: bin, - BinID: binID, - Address: chunk.Address(), - BatchID: chunk.Stamp().BatchID(), - StampHash: stampHash, - }), - s.IndexStore().Put(&ChunkBinItem{ - Bin: bin, - BinID: binID, - Address: chunk.Address(), - BatchID: chunk.Stamp().BatchID(), - ChunkType: chunkType, - StampHash: stampHash, - Sum: sum, - }), - s.IndexStore().Put(&ChunkSumItem{Address: chunk.Address(), Sum: sum}), + if err := s.ChunkStore().Put(ctx, chunk); err != nil { + return err + } + + shouldInc = !loaded + return nil + }) + return +} + +// resolveStampIndexCollision settles a stamp-index slot collision found by +// LoadOrStore (same batchID and stamp index already occupied). On success it +// returns sameAddr to tell the caller what remains: +// +// 1. sameAddr=true: the stored entry has the same chunk address. Old reserve +// index entries for that stamp are replaced in place. The caller only +// performs the type-specific chunkstore action (SOC Replace, CAC no-op). +// 2. sameAddr=false: the stored entry points at a different address. That +// chunk and its reserve metadata are removed and the stamp index is +// rewritten. The caller must still call storeReserveEntries and write the +// new chunk to the chunkstore. +func (r *Reserve) resolveStampIndexCollision( + ctx context.Context, s transaction.Store, + chunk swarm.Chunk, oldStampIndex *stampindex.Item, + sum, stampHash []byte, bin uint8, chunkType swarm.ChunkType, +) (sameAddr bool, err error) { + prev := binary.BigEndian.Uint64(oldStampIndex.StampTimestamp) + curr := binary.BigEndian.Uint64(chunk.Stamp().Timestamp()) + if prev > curr { + return false, fmt.Errorf("overwrite same chunk. prev %d cur %d batch %s: %w", prev, curr, hex.EncodeToString(chunk.Stamp().BatchID()), storage.ErrOverwriteNewerChunk) + } + + // Same stamp index and timestamp, different chunk addresses: both + // claims are otherwise valid, so settle on the lower address. + if prev == curr && !oldStampIndex.ChunkAddress.Equal(chunk.Address()) { + if bytes.Compare(chunk.Address().Bytes(), oldStampIndex.ChunkAddress.Bytes()) >= 0 { + r.logger.Debug( + "discarding stamp index collision", + "old_chunk", oldStampIndex.ChunkAddress, + "new_chunk", chunk.Address(), + "batch_id", hex.EncodeToString(chunk.Stamp().BatchID()), + "stamp_index", hex.EncodeToString(chunk.Stamp().Index()), + "stamp_timestamp", binary.BigEndian.Uint64(chunk.Stamp().Timestamp()), + "incoming_stamp_hash", hex.EncodeToString(stampHash), + "stored_stamp_hash", hex.EncodeToString(oldStampIndex.StampHash), + ) + return false, fmt.Errorf( + "stamp index collision chunk %s lost tie-break: %w", + chunk.Address(), + storage.ErrDivergentChunkRejected, + ) + } + r.logger.Debug( + "replacing stamp index collision", + "old_chunk", oldStampIndex.ChunkAddress, + "new_chunk", chunk.Address(), + "batch_id", hex.EncodeToString(chunk.Stamp().BatchID()), + "stamp_index", hex.EncodeToString(chunk.Stamp().Index()), + "stamp_timestamp", binary.BigEndian.Uint64(chunk.Stamp().Timestamp()), + "incoming_stamp_hash", hex.EncodeToString(stampHash), + "stored_stamp_hash", hex.EncodeToString(oldStampIndex.StampHash), ) + } else { + r.logger.Debug( + "replacing chunk stamp index", + "old_chunk", oldStampIndex.ChunkAddress, + "new_chunk", chunk.Address(), + "batch_id", hex.EncodeToString(chunk.Stamp().BatchID()), + ) + } + + if oldStampIndex.ChunkAddress.Equal(chunk.Address()) { + // Same address, same timestamp, same batch id: settle on the lower stamp hash. + // Paranoid check: normally such stamps are invalid (invalid signature) and rejected earlier. + if prev == curr && bytes.Compare(oldStampIndex.StampHash, stampHash) <= 0 { + return false, fmt.Errorf( + "stamp index collision chunk %s lost stamp-hash tie-break: %w", + chunk.Address(), + storage.ErrOverwriteNewerChunk, + ) + } + + oldStamp, err := chunkstamp.LoadWithStampHash(s.IndexStore(), reserveScope, oldStampIndex.ChunkAddress, oldStampIndex.StampHash) if err != nil { - return err + return false, err } - var has bool - if chunkType == swarm.ChunkTypeSingleOwner { - has, err = s.ChunkStore().Has(ctx, chunk.Address()) - if err != nil { - return err - } - if has { - r.logger.Debug("replacing soc in chunkstore", "address", chunk.Address()) - socReplaced = true - err = s.ChunkStore().Replace(ctx, chunk, true) - } else { - err = s.ChunkStore().Put(ctx, chunk) - } - } else { - err = s.ChunkStore().Put(ctx, chunk) + oldBatchRadiusItem := &BatchRadiusItem{ + Bin: bin, + Address: oldStampIndex.ChunkAddress, + BatchID: oldStampIndex.BatchID, + StampHash: oldStampIndex.StampHash, + } + err = s.IndexStore().Get(oldBatchRadiusItem) + if err != nil { + return false, err } + err = errors.Join( + s.IndexStore().Delete(oldBatchRadiusItem), + deleteChunkBinItem(s.IndexStore(), oldBatchRadiusItem.Bin, oldBatchRadiusItem.BinID), + stampindex.Delete(s.IndexStore(), reserveScope, oldStamp), + chunkstamp.DeleteWithStamp(s.IndexStore(), reserveScope, oldBatchRadiusItem.Address, oldStamp), + ) if err != nil { - return err + return false, err } - if !loadedStampIndex { - shouldIncReserveSize = true + err = errors.Join( + stampindex.Store(s.IndexStore(), reserveScope, chunk), + r.storeReserveEntries(s, chunk, sum, stampHash, bin), + ) + if err != nil { + return false, err } - return nil - }) + return true, nil + } + + // An older and different chunk with the same batchID and stamp index has been previously + // saved to the reserve. We must do the below before saving the new chunk: + // 1. Delete the old chunk from the chunkstore. + // 2. Delete the old chunk's stamp data. + // 3. Delete ALL old chunk related items from the reserve. + // 4. Update the stamp index. + + err = r.removeChunk(ctx, s, oldStampIndex.ChunkAddress, oldStampIndex.BatchID, oldStampIndex.StampHash) if err != nil { - return false, err + return false, fmt.Errorf("failed removing older chunk %s: %w", oldStampIndex.ChunkAddress, err) } - if shouldIncReserveSize { - r.size.Add(1) + + err = stampindex.Store(s.IndexStore(), reserveScope, chunk) + if err != nil { + return false, fmt.Errorf("failed updating stamp index: %w", err) } - return socReplaced, nil + + return false, nil +} + +// storeReserveEntries writes the common set of reserve index entries for a +// chunk: chunkstamp, BatchRadiusItem, ChunkBinItem and ChunkSumItem and allocates a fresh bin ID via IncBinID. +// The stamp index is NOT written here because its lifecycle differs across call sites (LoadOrStore vs explicit Store after collision cleanup). +func (r *Reserve) storeReserveEntries(s transaction.Store, chunk swarm.Chunk, sum, stampHash []byte, bin uint8) error { + chunkType := storage.ChunkType(chunk) + binID, err := r.IncBinID(s.IndexStore(), bin) + if err != nil { + return err + } + + return errors.Join( + chunkstamp.Store(s.IndexStore(), reserveScope, chunk), + s.IndexStore().Put(&BatchRadiusItem{ + Bin: bin, + BinID: binID, + Address: chunk.Address(), + BatchID: chunk.Stamp().BatchID(), + StampHash: stampHash, + }), + s.IndexStore().Put(&ChunkBinItem{ + Bin: bin, + BinID: binID, + Address: chunk.Address(), + BatchID: chunk.Stamp().BatchID(), + ChunkType: chunkType, + StampHash: stampHash, + Sum: sum, + }), + s.IndexStore().Put(&ChunkSumItem{Address: chunk.Address(), Sum: sum}), + ) } // checkSOCStampOverwrite rejects an incoming single owner chunk when the // address already holds a payload under a stamp that should keep winning: // a strictly higher timestamp, or an equal timestamp with a lower or equal // stamp hash. Must run before LoadOrStore, which writes immediately. + +// Possible scenario without this check: +// - SOC at address X arrives with batch A, stampIndex 0, timestamp 10, payload "hello" → stored +// - Same SOC address X arrives with batch B, stampIndex 0, timestamp 5, payload "world" +// - LoadOrStore looks up (reserve, batchB, stampIndex=0) → not found (different batch) → no collision +// - Proceed to storeReserveEntries → new index entries written +// - ChunkStore.Has(X) → true → Replace(chunk, true) → newer payload "hello" silently overwritten with "world" func checkSOCStampOverwrite(ctx context.Context, s transaction.Store, chunk swarm.Chunk, stampHash []byte) error { hasPayload, err := s.ChunkStore().Has(ctx, chunk.Address()) if err != nil || !hasPayload { @@ -411,6 +451,7 @@ func checkSOCStampOverwrite(ctx context.Context, s transaction.Store, chunk swar curr := binary.BigEndian.Uint64(chunk.Stamp().Timestamp()) return chunkstamp.IterateAll(s.IndexStore(), reserveScope, chunk.Address(), func(st swarm.Stamp) (bool, error) { + // check all stamps regardless batch: we must be sure there no "newer" stamp in store prev := binary.BigEndian.Uint64(st.Timestamp()) if prev > curr { return true, fmt.Errorf("overwrite same chunk. prev %d cur %d batch %s: %w", From e4f05a94f57de98e5ee96103a1bb32d32e04fcc1 Mon Sep 17 00:00:00 2001 From: Calin Martinconi Date: Wed, 29 Jul 2026 09:10:37 +0300 Subject: [PATCH 14/17] test(storer): add reserve corner case tests for multi-stamp eviction and SOC convergence --- pkg/storer/internal/reserve/reserve_test.go | 441 ++++++++++++++++++++ 1 file changed, 441 insertions(+) diff --git a/pkg/storer/internal/reserve/reserve_test.go b/pkg/storer/internal/reserve/reserve_test.go index 172827998e9..5ce9cf90f9b 100644 --- a/pkg/storer/internal/reserve/reserve_test.go +++ b/pkg/storer/internal/reserve/reserve_test.go @@ -11,6 +11,7 @@ import ( "fmt" "math" "math/rand" + "slices" "testing" "testing/synctest" @@ -19,7 +20,9 @@ import ( "github.com/ethersphere/bee/v2/pkg/log" "github.com/ethersphere/bee/v2/pkg/postage" postagetesting "github.com/ethersphere/bee/v2/pkg/postage/testing" + "github.com/ethersphere/bee/v2/pkg/soc" soctesting "github.com/ethersphere/bee/v2/pkg/soc/testing" + "github.com/ethersphere/bee/v2/pkg/storage" chunk "github.com/ethersphere/bee/v2/pkg/storage/testing" "github.com/ethersphere/bee/v2/pkg/storer/internal" @@ -1882,3 +1885,441 @@ func TestCACStampIndexCollision(t *testing.T) { checkStore(t, ts.IndexStore(), &reserve.ChunkBinItem{Bin: firstBin, BinID: oldBinID}, true) checkStore(t, ts.IndexStore(), &reserve.ChunkBinItem{Bin: secondBin, BinID: newItem.BinID}, false) } + +func TestResolveStampIndexCollisionCornerCases(t *testing.T) { + t.Parallel() + + ctx := context.Background() + baseAddr := swarm.RandAddress(t) + + newTestReserve := func(t *testing.T) (*reserve.Reserve, transaction.Storage) { + t.Helper() + ts := internal.NewInmemStorage() + r, err := reserve.New(baseAddr, ts, 0, kademlia.NewTopologyDriver(), log.Noop) + if err != nil { + t.Fatal(err) + } + return r, ts + } + + mustCAC := func(t *testing.T, data []byte) swarm.Chunk { + t.Helper() + ch, err := cac.New(data) + if err != nil { + t.Fatal(err) + } + return ch + } + + t.Run("collision with older timestamp rejected", func(t *testing.T) { + t.Parallel() + r, ts := newTestReserve(t) + batch := postagetesting.MustNewBatch() + + ch1 := mustCAC(t, []byte("chunk 1 payload")).WithStamp(postagetesting.MustNewFields(batch.ID, 0, 10)) + ch2 := mustCAC(t, []byte("chunk 2 payload")).WithStamp(postagetesting.MustNewFields(batch.ID, 0, 5)) + + if err := r.Put(ctx, ch1); err != nil { + t.Fatal(err) + } + + err := r.Put(ctx, ch2) + if !errors.Is(err, storage.ErrOverwriteNewerChunk) { + t.Fatalf("expected ErrOverwriteNewerChunk, got %v", err) + } + + ch1StampHash, _ := ch1.Stamp().Hash() + ch2StampHash, _ := ch2.Stamp().Hash() + + has1, err := r.Has(ch1.Address(), batch.ID, ch1StampHash) + if err != nil || !has1 { + t.Fatalf("expected ch1 in reserve") + } + has2, err := r.Has(ch2.Address(), batch.ID, ch2StampHash) + if err != nil || has2 { + t.Fatalf("expected ch2 NOT in reserve") + } + _ = ts + }) + + t.Run("collision equal timestamp higher address lost tie-break", func(t *testing.T) { + t.Parallel() + r, _ := newTestReserve(t) + batch := postagetesting.MustNewBatch() + + var chLower, chHigher swarm.Chunk + for i := range 100 { + c := mustCAC(t, fmt.Appendf(nil, "payload %d", i)) + if chLower == nil { + chLower = c + continue + } + if bytes.Compare(c.Address().Bytes(), chLower.Address().Bytes()) < 0 { + chHigher = chLower + chLower = c + } else if chHigher == nil { + chHigher = c + } + if chLower != nil && chHigher != nil { + break + } + } + + chLower = chLower.WithStamp(postagetesting.MustNewFields(batch.ID, 0, 10)) + chHigher = chHigher.WithStamp(postagetesting.MustNewFields(batch.ID, 0, 10)) + + if err := r.Put(ctx, chLower); err != nil { + t.Fatal(err) + } + + err := r.Put(ctx, chHigher) + if !errors.Is(err, storage.ErrDivergentChunkRejected) { + t.Fatalf("expected ErrDivergentChunkRejected, got %v", err) + } + + chLowerStampHash, _ := chLower.Stamp().Hash() + chHigherStampHash, _ := chHigher.Stamp().Hash() + + hasLower, _ := r.Has(chLower.Address(), batch.ID, chLowerStampHash) + hasHigher, _ := r.Has(chHigher.Address(), batch.ID, chHigherStampHash) + if !hasLower || hasHigher { + t.Fatalf("expected lower address chunk kept, higher rejected") + } + }) + + t.Run("collision equal timestamp lower address wins tie-break", func(t *testing.T) { + t.Parallel() + r, _ := newTestReserve(t) + batch := postagetesting.MustNewBatch() + + var chLower, chHigher swarm.Chunk + for i := range 100 { + c := mustCAC(t, fmt.Appendf(nil, "payload %d", i)) + if chLower == nil { + chLower = c + continue + } + if bytes.Compare(c.Address().Bytes(), chLower.Address().Bytes()) < 0 { + chHigher = chLower + chLower = c + } else if chHigher == nil { + chHigher = c + } + if chLower != nil && chHigher != nil { + break + } + } + + chLower = chLower.WithStamp(postagetesting.MustNewFields(batch.ID, 0, 10)) + chHigher = chHigher.WithStamp(postagetesting.MustNewFields(batch.ID, 0, 10)) + + if err := r.Put(ctx, chHigher); err != nil { + t.Fatal(err) + } + + if err := r.Put(ctx, chLower); err != nil { + t.Fatalf("expected lower address chunk to replace higher address: %v", err) + } + + chLowerStampHash, _ := chLower.Stamp().Hash() + chHigherStampHash, _ := chHigher.Stamp().Hash() + + hasLower, _ := r.Has(chLower.Address(), batch.ID, chLowerStampHash) + hasHigher, _ := r.Has(chHigher.Address(), batch.ID, chHigherStampHash) + if !hasLower || hasHigher { + t.Fatalf("expected lower address chunk to replace higher address") + } + }) + + t.Run("collision newer timestamp replaces old chunk", func(t *testing.T) { + t.Parallel() + r, _ := newTestReserve(t) + batch := postagetesting.MustNewBatch() + + ch1 := mustCAC(t, []byte("chunk 1 payload")).WithStamp(postagetesting.MustNewFields(batch.ID, 0, 10)) + ch2 := mustCAC(t, []byte("chunk 2 payload")).WithStamp(postagetesting.MustNewFields(batch.ID, 0, 20)) + + if err := r.Put(ctx, ch1); err != nil { + t.Fatal(err) + } + + if err := r.Put(ctx, ch2); err != nil { + t.Fatalf("expected ch2 to replace ch1: %v", err) + } + + ch1StampHash, _ := ch1.Stamp().Hash() + ch2StampHash, _ := ch2.Stamp().Hash() + + has1, _ := r.Has(ch1.Address(), batch.ID, ch1StampHash) + has2, _ := r.Has(ch2.Address(), batch.ID, ch2StampHash) + if has1 || !has2 { + t.Fatalf("expected ch2 in reserve, ch1 removed") + } + }) + + t.Run("collision on multi-stamped chunk keeps chunk when other stamp remains", func(t *testing.T) { + t.Parallel() + r, ts := newTestReserve(t) + batch1 := postagetesting.MustNewBatch() + batch2 := postagetesting.MustNewBatch() + + ch1 := mustCAC(t, []byte("chunk 1 payload")) + ch1Stamp1 := ch1.WithStamp(postagetesting.MustNewFields(batch1.ID, 0, 10)) + + chOther := mustCAC(t, []byte("other chunk payload")).WithStamp(postagetesting.MustNewFields(batch2.ID, 0, 10)) + + ch2 := mustCAC(t, []byte("chunk 2 payload")).WithStamp(postagetesting.MustNewFields(batch1.ID, 0, 20)) + + if err := r.Put(ctx, ch1Stamp1); err != nil { + t.Fatal(err) + } + if err := r.Put(ctx, chOther); err != nil { + t.Fatal(err) + } + + sizeBefore := r.Size() + if sizeBefore != 2 { + t.Fatalf("expected reserve size 2, got %d", sizeBefore) + } + + // Now put ch2 with batch1 stamp index 0 and newer timestamp + if err := r.Put(ctx, ch2); err != nil { + t.Fatalf("expected ch2 to succeed: %v", err) + } + + // Size should be unchanged: batch1 index 0 transferred from ch1 to ch2 + if got := r.Size(); got != 2 { + t.Fatalf("expected reserve size to remain 2, got %d", got) + } + + ch1Stamp1Hash, _ := ch1Stamp1.Stamp().Hash() + chOtherStampHash, _ := chOther.Stamp().Hash() + ch2StampHash, _ := ch2.Stamp().Hash() + + // ch1 with batch1 should be gone + has1, _ := r.Has(ch1.Address(), batch1.ID, ch1Stamp1Hash) + if has1 { + t.Fatalf("expected ch1 with batch1 stamp removed from reserve") + } + + // chOther with batch2 should STILL be in reserve + hasOther, _ := r.Has(chOther.Address(), batch2.ID, chOtherStampHash) + if !hasOther { + t.Fatalf("expected chOther STILL in reserve") + } + + // ch2 with batch1 should be in reserve + has2, _ := r.Has(ch2.Address(), batch1.ID, ch2StampHash) + if !has2 { + t.Fatalf("expected ch2 with batch1 stamp in reserve") + } + + // ch1 payload should be removed from chunkstore + _, err := ts.ChunkStore().Get(ctx, ch1.Address()) + if !errors.Is(err, storage.ErrNotFound) { + t.Fatalf("expected ch1 payload removed from chunkstore, got error: %v", err) + } + }) +} + +func TestAdvancedReserveCornerCases(t *testing.T) { + t.Parallel() + + ctx := context.Background() + baseAddr := swarm.RandAddress(t) + + newTestReserve := func(t *testing.T) (*reserve.Reserve, transaction.Storage) { + t.Helper() + ts := internal.NewInmemStorage() + r, err := reserve.New(baseAddr, ts, 0, kademlia.NewTopologyDriver(), log.Noop) + if err != nil { + t.Fatal(err) + } + return r, ts + } + + mustCAC := func(t *testing.T, data []byte) swarm.Chunk { + t.Helper() + ch, err := cac.New(data) + if err != nil { + t.Fatal(err) + } + return ch + } + + t.Run("multi-stamp eviction chain cleans up chunkstore when final stamp evicted", func(t *testing.T) { + t.Parallel() + r, ts := newTestReserve(t) + + batch1 := postagetesting.MustNewBatch() + batch2 := postagetesting.MustNewBatch() + batch3 := postagetesting.MustNewBatch() + + data := []byte("chunk A payload") + chAStamp1 := mustCAC(t, data).WithStamp(postagetesting.MustNewFields(batch1.ID, 0, 10)) + chAStamp2 := mustCAC(t, data).WithStamp(postagetesting.MustNewFields(batch2.ID, 0, 10)) + chAStamp3 := mustCAC(t, data).WithStamp(postagetesting.MustNewFields(batch3.ID, 0, 10)) + + chB := mustCAC(t, []byte("chunk B payload")).WithStamp(postagetesting.MustNewFields(batch1.ID, 0, 20)) + chC := mustCAC(t, []byte("chunk C payload")).WithStamp(postagetesting.MustNewFields(batch2.ID, 0, 20)) + chD := mustCAC(t, []byte("chunk D payload")).WithStamp(postagetesting.MustNewFields(batch3.ID, 0, 20)) + + // Put chA with 3 different stamps + if err := r.Put(ctx, chAStamp1); err != nil { + t.Fatalf("put chAStamp1 failed: %v", err) + } + if err := r.Put(ctx, chAStamp2); err != nil { + t.Fatalf("put chAStamp2 failed: %v", err) + } + if err := r.Put(ctx, chAStamp3); err != nil { + t.Fatalf("put chAStamp3 failed: %v", err) + } + + if got := r.Size(); got != 3 { + t.Fatalf("expected reserve size 3, got %d", got) + } + + chAAddr := chAStamp1.Address() + + // Evict Stamp 1: Put chB (batch 1, index 0, ts 20) + if err := r.Put(ctx, chB); err != nil { + t.Fatal(err) + } + if got := r.Size(); got != 3 { + t.Fatalf("expected reserve size 3 after 1st eviction, got %d", got) + } + // chA must STILL be in chunkstore (2 stamps remain) + if _, err := ts.ChunkStore().Get(ctx, chAAddr); err != nil { + t.Fatalf("chA should still exist after 1st eviction: %v", err) + } + + // Evict Stamp 2: Put chC (batch 2, index 0, ts 20) + if err := r.Put(ctx, chC); err != nil { + t.Fatal(err) + } + if got := r.Size(); got != 3 { + t.Fatalf("expected reserve size 3 after 2nd eviction, got %d", got) + } + // chA must STILL be in chunkstore (1 stamp remains) + if _, err := ts.ChunkStore().Get(ctx, chAAddr); err != nil { + t.Fatalf("chA should still exist after 2nd eviction: %v", err) + } + + // Evict Stamp 3: Put chD (batch 3, index 0, ts 20) + if err := r.Put(ctx, chD); err != nil { + t.Fatal(err) + } + if got := r.Size(); got != 3 { + t.Fatalf("expected reserve size 3 after 3rd eviction, got %d", got) + } + // NOW chA must be COMPLETELY GONE from chunkstore (0 stamps remain) + if _, err := ts.ChunkStore().Get(ctx, chAAddr); !errors.Is(err, storage.ErrNotFound) { + t.Fatalf("expected chA to be deleted from chunkstore after 3rd eviction, got %v", err) + } + + // Check index cleanliness: total BatchRadiusItems must equal 3 + countBR, err := ts.IndexStore().Count(&reserve.BatchRadiusItem{}) + if err != nil || countBR != 3 { + t.Fatalf("expected 3 BatchRadiusItems, got count %d, err %v", countBR, err) + } + countCB, err := ts.IndexStore().Count(&reserve.ChunkBinItem{}) + if err != nil || countCB != 3 { + t.Fatalf("expected 3 ChunkBinItems, got count %d, err %v", countCB, err) + } + countCS, err := ts.IndexStore().Count(&reserve.ChunkSumItem{}) + if err != nil || countCS != 3 { + t.Fatalf("expected 3 ChunkSumItems, got count %d, err %v", countCS, err) + } + }) + + t.Run("sequential divergent SOC chain tie-breaks converge on lowest wrapped address", func(t *testing.T) { + t.Parallel() + r, ts := newTestReserve(t) + + privKey, err := crypto.GenerateSecp256k1Key() + if err != nil { + t.Fatal(err) + } + signer := crypto.NewDefaultSigner(privKey) + batch := postagetesting.MustNewBatch() + stamp := postagetesting.MustNewFields(batch.ID, 0, 10) + + // Generate 4 SOCs with different payloads and sort them by wrapped CAC address + var socChunks []swarm.Chunk + for i := range 100 { + c := soctesting.GenerateMockSocWithSigner(t, fmt.Appendf(nil, "payload %d", i), signer).Chunk().WithStamp(stamp) + socChunks = append(socChunks, c) + } + + // Sort by wrapped CAC address + wrappedAddr := func(ch swarm.Chunk) swarm.Address { + s, _ := soc.FromChunk(ch) + return s.WrappedChunk().Address() + } + + slices.SortFunc(socChunks, func(a, b swarm.Chunk) int { + return bytes.Compare(wrappedAddr(a).Bytes(), wrappedAddr(b).Bytes()) + }) + + // Take C0 (lowest), C1, C2, C3 (highest) + c0 := socChunks[0] + c1 := socChunks[1] + c2 := socChunks[2] + c3 := socChunks[3] + + socAddress := c0.Address() + + // 1. Put C2 (3rd lowest) first + if err := r.Put(ctx, c2); err != nil { + t.Fatal(err) + } + stored, err := ts.ChunkStore().Get(ctx, socAddress) + if err != nil || !bytes.Equal(stored.Data(), c2.Data()) { + t.Fatalf("expected C2 stored first") + } + + // 2. Put C1 (2nd lowest): C1 < C2 so C1 wins tie-break and replaces C2 + if err := r.Put(ctx, c1); err != nil { + t.Fatal(err) + } + stored, err = ts.ChunkStore().Get(ctx, socAddress) + if err != nil || !bytes.Equal(stored.Data(), c1.Data()) { + t.Fatalf("expected C1 to replace C2") + } + + // 3. Put C3 (highest): C3 > C1 so C3 loses tie-break + err = r.Put(ctx, c3) + if !errors.Is(err, storage.ErrDivergentChunkRejected) { + t.Fatalf("expected ErrDivergentChunkRejected for C3, got %v", err) + } + stored, err = ts.ChunkStore().Get(ctx, socAddress) + if err != nil || !bytes.Equal(stored.Data(), c1.Data()) { + t.Fatalf("expected C1 retained after C3 rejection") + } + + // 4. Put C0 (lowest): C0 < C1 so C0 wins tie-break and replaces C1 + if err := r.Put(ctx, c0); err != nil { + t.Fatal(err) + } + stored, err = ts.ChunkStore().Get(ctx, socAddress) + if err != nil || !bytes.Equal(stored.Data(), c0.Data()) { + t.Fatalf("expected C0 to replace C1 as overall winner") + } + + // Verify ChunkSumItem matches C0's sum + c0Sum, err := storage.ChunkSum(c0) + if err != nil { + t.Fatal(err) + } + hasSum, err := r.HasSum(socAddress, c0Sum) + if err != nil || !hasSum { + t.Fatalf("expected reserve to have C0 sum") + } + + c1Sum, _ := storage.ChunkSum(c1) + hasC1Sum, _ := r.HasSum(socAddress, c1Sum) + if hasC1Sum { + t.Fatalf("expected reserve NOT to have C1 sum") + } + }) +} From 75843bb50afa859ee95b4d21341adf5a2df9e9f8 Mon Sep 17 00:00:00 2001 From: Calin Martinconi Date: Wed, 29 Jul 2026 11:58:12 +0300 Subject: [PATCH 15/17] test(storer): preallocate socChunks slice in reserve test --- pkg/storer/internal/reserve/reserve_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/storer/internal/reserve/reserve_test.go b/pkg/storer/internal/reserve/reserve_test.go index 5ce9cf90f9b..d067d9c03ba 100644 --- a/pkg/storer/internal/reserve/reserve_test.go +++ b/pkg/storer/internal/reserve/reserve_test.go @@ -2245,7 +2245,7 @@ func TestAdvancedReserveCornerCases(t *testing.T) { stamp := postagetesting.MustNewFields(batch.ID, 0, 10) // Generate 4 SOCs with different payloads and sort them by wrapped CAC address - var socChunks []swarm.Chunk + socChunks := make([]swarm.Chunk, 0, 100) for i := range 100 { c := soctesting.GenerateMockSocWithSigner(t, fmt.Appendf(nil, "payload %d", i), signer).Chunk().WithStamp(stamp) socChunks = append(socChunks, c) From 22cd4ff9685bc8a16946ca72f558f6e34b2ec676 Mon Sep 17 00:00:00 2001 From: Calin Martinconi Date: Wed, 29 Jul 2026 14:11:55 +0300 Subject: [PATCH 16/17] ci(workflow): set BEEKEEPER_BRANCH to feat/pullsync-chuns-convergence --- .github/workflows/beekeeper.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/beekeeper.yml b/.github/workflows/beekeeper.yml index b907093bad0..26968034148 100644 --- a/.github/workflows/beekeeper.yml +++ b/.github/workflows/beekeeper.yml @@ -19,7 +19,7 @@ env: SETUP_CONTRACT_IMAGE: "ethersphere/bee-localchain" SETUP_CONTRACT_IMAGE_TAG: "0.9.4" BEELOCAL_BRANCH: "main" - BEEKEEPER_BRANCH: "master" + BEEKEEPER_BRANCH: "feat/pullsync-chuns-convergence" BEEKEEPER_METRICS_ENABLED: false REACHABILITY_OVERRIDE_PUBLIC: true BATCHFACTOR_OVERRIDE_PUBLIC: 2 From 38d5daa55e7b878f6d5246b534aa53e39668bfd9 Mon Sep 17 00:00:00 2001 From: Calin Martinconi Date: Wed, 29 Jul 2026 15:21:39 +0300 Subject: [PATCH 17/17] test(storer): order chunk addresses in TestIndexCollision for deterministic tie-break --- pkg/storer/reserve_test.go | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/pkg/storer/reserve_test.go b/pkg/storer/reserve_test.go index 00e7e52cbdc..6ff87bd3105 100644 --- a/pkg/storer/reserve_test.go +++ b/pkg/storer/reserve_test.go @@ -36,12 +36,16 @@ func TestIndexCollision(t *testing.T) { putter := storer.ReservePutter() ch1 := chunk.GenerateTestRandomChunkAt(t, baseAddr, 0).WithStamp(stamp) + ch2 := chunk.GenerateTestRandomChunkAt(t, baseAddr, 0).WithStamp(stamp) + if bytes.Compare(ch1.Address().Bytes(), ch2.Address().Bytes()) > 0 { + ch1, ch2 = ch2, ch1 + } + err := putter.Put(context.Background(), ch1) if err != nil { t.Fatal(err) } - ch2 := chunk.GenerateTestRandomChunkAt(t, baseAddr, 0).WithStamp(stamp) err = putter.Put(context.Background(), ch2) if err == nil { t.Fatal("expected index collision error")