Fix wantlist overflow handling to select newer entries.

bitswap wantlist overflow handling now selects newer entries when truncating wantlist. This fix prevents the retained portion of the wantlist from filling up with CIDs that the server does not have.

Fixes #527

CHANGELOG.md

@@ -39,6 +39,9 @@ The following emojis are used to highlight certain changes:
 
 - `routing/http`: the `FindPeer` now returns `routing.ErrNotFound` when no addresses are found
 - `routing/http`: the `FindProvidersAsync` no longer causes a goroutine buildup
+- bitswap wantlist overflow handling now selects newer entries when truncating wantlist. This fix prevents the retained portion of the wantlist from filling up with CIDs that the server does not have.
+
+### Security
 
 ## [v0.20.0]
 

bitswap/server/internal/decision/engine.go

@@ -4,7 +4,6 @@ package decision
 import (
 	"context"
 	"fmt"
-	"math/bits"
 	"sync"
 	"time"
 
@@ -717,20 +716,18 @@ func (e *Engine) MessageReceived(ctx context.Context, p peer.ID, m bsmsg.BitSwap
 		e.peerLedger.ClearPeerWantlist(p)
 	}
 
-	s := uint(e.peerLedger.WantlistSizeForPeer(p))
-	if wouldBe := s + uint(len(wants)); wouldBe > e.maxQueuedWantlistEntriesPerPeer {
-		log.Debugw("wantlist overflow", "local", e.self, "remote", p, "would be", wouldBe)
-		// truncate wantlist to avoid overflow
-		available, o := bits.Sub(e.maxQueuedWantlistEntriesPerPeer, s, 0)
-		if o != 0 {
-			available = 0
-		}
-		wants = wants[:available]
+	var skipWants int
+	s := e.peerLedger.WantlistSizeForPeer(p)
+	available := int(e.maxQueuedWantlistEntriesPerPeer) - s
+	if len(wants) > available {
+		log.Debugw("wantlist overflow", "local", e.self, "remote", p, "would be", s+len(wants))
+		// truncate-left wantlist to avoid overflow
+		skipWants = len(wants) - available
 	}
 
 	filteredWants := wants[:0] // shift inplace
 
-	for _, entry := range wants {
+	for _, entry := range wants[skipWants:] {
 		if entry.Cid.Prefix().MhType == mh.IDENTITY {
 			// This is a truely broken client, let's kill the connection.
 			e.lock.Unlock()

bitswap/server/internal/decision/engine_test.go

@@ -1733,3 +1733,36 @@ func TestKillConnectionForInlineCid(t *testing.T) {
 		t.Fatal("connection was not killed when receiving inline in cancel")
 	}
 }
+
+func TestWantlistOverflow(t *testing.T) {
+	ctx, cancel := context.WithCancel(context.Background())
+	defer cancel()
+
+	const limit = 32
+	warsaw := newTestEngine(ctx, "warsaw", WithMaxQueuedWantlistEntriesPerPeer(limit))
+	riga := newTestEngine(ctx, "riga")
+
+	m := message.New(false)
+	for i := 0; i < 2*limit; i++ {
+		m.AddEntry(blocks.NewBlock([]byte(fmt.Sprint(i))).Cid(), 0, pb.Message_Wantlist_Block, true)
+	}
+	warsaw.Engine.MessageReceived(ctx, riga.Peer, m)
+
+	if warsaw.Peer == riga.Peer {
+		t.Fatal("Sanity Check: Peers have same Key!")
+	}
+
+	wl := warsaw.Engine.WantlistForPeer(riga.Peer)
+	if len(wl) != limit {
+		t.Fatal("wantlist does not match limit", len(wl))
+	}
+
+	firstEnt := wl[0]
+
+	m.AddEntry(blocks.NewBlock([]byte(fmt.Sprint(2*limit+1))).Cid(), 0, pb.Message_Wantlist_Block, true)
+	warsaw.Engine.MessageReceived(ctx, riga.Peer, m)
+	wl = warsaw.Engine.WantlistForPeer(riga.Peer)
+	if wl[0].Cid == firstEnt.Cid {
+		t.Fatal("First entry in wantlist should be different")
+	}
+}