Skip to content

Commit e0c56cc

Browse files
committed
fix(pullsync): return empty offer at gap boundary to prevent interval over-advancement
When SubscribeBin returns a first chunk whose BinID is beyond the requested start, the server has no chunks at [start, firstBinID-1]. Previously the server included the chunk in the offer with Topmost=firstBinID, causing the client to advance its interval across BinIDs it never received. collectAddrs now detects this case: when the first chunk is within the historical range (BinID <= historicalCursor) and its BinID is beyond the requested start, the server returns an empty offer with Topmost=firstBinID-1. The client advances its interval to the gap boundary only, then issues a new Get from firstBinID and receives the chunk cleanly on the next round. The check is skipped for start=0 (the BinID namespace starts at 1; the puller always uses start>=1 in practice, but test helpers may pass 0). A new test, TestSync_HistoricalGapReturnsEmptyOfferAtBoundary, covers the gap-boundary case directly.
1 parent 2c7d7b7 commit e0c56cc

2 files changed

Lines changed: 54 additions & 0 deletions

File tree

pkg/pullsync/pullsync.go

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -484,6 +484,17 @@ func (s *Syncer) collectAddrs(ctx context.Context, bin uint8, start uint64) ([]*
484484
break LOOP // The stream has been closed.
485485
}
486486

487+
// If the first chunk the server offers has a BinID beyond start
488+
// and within the historical range, the server has no chunks at
489+
// [start, c.BinID-1]. Return an empty offer with Topmost set to
490+
// the gap boundary so the client advances its interval past only
491+
// the BinIDs that genuinely do not exist on this server, then
492+
// retries from c.BinID on the next round.
493+
if len(chs) == 0 && start > 0 && c.BinID > start && c.BinID <= historicalCursor {
494+
topmost = c.BinID - 1
495+
break LOOP
496+
}
497+
487498
chs = append(chs, &storer.BinC{Address: c.Address, BatchID: c.BatchID, StampHash: c.StampHash})
488499
if c.BinID > topmost {
489500
topmost = c.BinID

pkg/pullsync/pullsync_test.go

Lines changed: 43 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -397,6 +397,49 @@ func TestSync_LiveChunkTopCappedAtCursor(t *testing.T) {
397397
})
398398
}
399399

400+
// TestSync_HistoricalGapReturnsEmptyOfferAtBoundary verifies that when the
401+
// server's first available chunk has a BinID beyond the requested start, the
402+
// server returns an empty offer with Topmost set to firstBinID-1. This lets
403+
// the client advance its interval to the gap boundary without silently marking
404+
// BinIDs that may exist on other peers as synced.
405+
func TestSync_HistoricalGapReturnsEmptyOfferAtBoundary(t *testing.T) {
406+
synctest.Test(t, func(t *testing.T) {
407+
chunk := testingc.GenerateTestRandomChunk()
408+
stampHash, err := chunk.Stamp().Hash()
409+
if err != nil {
410+
t.Fatal(err)
411+
}
412+
413+
// Server holds one chunk at BinID 5; start=2 creates a gap at [2,4].
414+
const firstBinID = uint64(5)
415+
const cursor = uint64(5)
416+
result := []*storer.BinC{{
417+
Address: chunk.Address(),
418+
BatchID: chunk.Stamp().BatchID(),
419+
BinID: firstBinID,
420+
StampHash: stampHash,
421+
}}
422+
423+
var (
424+
ps, _ = newPullSync(t, nil, 10, mock.WithSubscribeResp(result, nil), mock.WithChunks(chunk), mock.WithCursors([]uint64{cursor}, 0))
425+
recorder = streamtest.New(streamtest.WithProtocols(ps.Protocol()))
426+
psClient, _ = newPullSync(t, recorder, 0)
427+
)
428+
429+
topmost, count, err := psClient.Sync(context.Background(), swarm.ZeroAddress, 0, 2)
430+
if err != nil {
431+
t.Fatal(err)
432+
}
433+
// Empty offer: gap boundary is firstBinID-1.
434+
if topmost != firstBinID-1 {
435+
t.Fatalf("topmost: got %d, want %d (gap boundary)", topmost, firstBinID-1)
436+
}
437+
if count != 0 {
438+
t.Fatalf("count: got %d, want 0 (no chunks in gap)", count)
439+
}
440+
})
441+
}
442+
400443
// TestSync_OverwriteNewerChunkDoesNotBlockInterval verifies that
401444
// ErrOverwriteNewerChunk does not prevent interval advancement.
402445
// The chunk is already present in the reserve with a newer stamp, so it is

0 commit comments

Comments
 (0)