Repository: ipfs/boxo (language: go)

Issue:
bitswap: Don't send CANCEL to peer we got block from 
### Overview
Currently, we [send CANCEL to every peer in a session](https://github.com/ipfs/boxo/blob/19bcc75077891427aba3e74cec467dde4d5a1af3/bitswap/client/internal/sessionmanager/sessionmanager.go#L175-L176) once we got a block from a peer. However, the serving peer can and [cleans the client's wantlint](https://github.com/ipfs/boxo/blob/19bcc75077891427aba3e74cec467dde4d5a1af3/bitswap/server/internal/decision/engine.go#L1046-L1049) after serving the block. This suggests that sending CANCEL to the serving peer is redundant.

### Proposal
Add peer parameter to [`PeerManager.SendCancels`](https://github.com/ipfs/boxo/blob/19bcc75077891427aba3e74cec467dde4d5a1af3/bitswap/client/internal/peermanager/peermanager.go#L157) and exclude it [when broadcasting cancels](https://github.com/ipfs/boxo/blob/19bcc75077891427aba3e74cec467dde4d5a1af3/bitswap/client/internal/peermanager/peerwantmanager.go#L259-L260).



Discussion / hints:

Fix patch:
diff --git a/bitswap/client/internal/peermanager/peerwantmanager.go b/bitswap/client/internal/peermanager/peerwantmanager.go
index 765566155..9c1e52d4f 100644
--- a/bitswap/client/internal/peermanager/peerwantmanager.go
+++ b/bitswap/client/internal/peermanager/peerwantmanager.go
@@ -257,8 +257,15 @@ func (pwm *peerWantManager) sendCancels(cancelKs []cid.Cid, excludePeer peer.ID)
 
 	// Send cancels to a particular peer
 	send := func(p peer.ID, pws *peerWant) {
-		// Start from the broadcast cancels
-		toCancel := broadcastCancels
+		noSend := p == excludePeer
+
+		var toCancel []cid.Cid
+
+		// If peer is not excluded, then send broadcast cancels to this peer.
+		if !noSend {
+			// Start from the broadcast cancels
+			toCancel = broadcastCancels
+		}
 
 		// For each key to be cancelled
 		for _, c := range cancelKs {
@@ -271,9 +278,9 @@ func (pwm *peerWantManager) sendCancels(cancelKs []cid.Cid, excludePeer peer.ID)
 			pws.wantBlocks.Remove(c)
 			pws.wantHaves.Remove(c)
 
-			// If it's a broadcast want, we've already added it to
-			// the peer cancels.
-			if !pwm.broadcastWants.Has(c) {
+			// If peer is not excluded and this a broadcast want is not already
+			// added it to the peer cancels, then add the cancel.
+			if !noSend && !pwm.broadcastWants.Has(c) {
 				toCancel = append(toCancel, c)
 			}
 		}
@@ -298,7 +305,6 @@ func (pwm *peerWantManager) sendCancels(cancelKs []cid.Cid, excludePeer peer.ID)
 				cancelPeers[p] = struct{}{}
 			}
 		}
-		delete(cancelPeers, excludePeer)
 		for p := range cancelPeers {
 			pws, ok := pwm.peerWants[p]
 			if !ok {


Test patch:
diff --git a/bitswap/client/internal/peermanager/peermanager_test.go b/bitswap/client/internal/peermanager/peermanager_test.go
index 61226acda..e9825b371 100644
--- a/bitswap/client/internal/peermanager/peermanager_test.go
+++ b/bitswap/client/internal/peermanager/peermanager_test.go
@@ -281,7 +281,7 @@ func TestSendCancelsExclude(t *testing.T) {
 	// Clear messages
 	collectMessages(msgs, 2*time.Millisecond)
 
-	// Send cancels for 1 want-block and 1 want-have
+	// Send cancels for 1 want-block and 1 want-have, excluding peer1.
 	peerManager.SendCancels(ctx, []cid.Cid{cids[0], cids[2]}, peer1)
 	collected := collectMessages(msgs, 2*time.Millisecond)
 
@@ -292,15 +292,15 @@ func TestSendCancelsExclude(t *testing.T) {
 		t.Fatal("Expected no cancels to be sent to excluded peer")
 	}
 
-	// Send cancels for all cids
+	// Send cancels for all cids. Expect cancels for the 1 remaining sid that
+	// was not previously canceled.
 	peerManager.SendCancels(ctx, cids, "")
 	collected = collectMessages(msgs, 2*time.Millisecond)
-
 	if _, ok := collected[peer2]; ok {
 		t.Fatal("Expected no cancels to be sent to peer that was not sent messages")
 	}
-	if len(collected[peer1].cancels) != 3 {
-		t.Fatal("Expected cancel to be sent for want-blocks")
+	if len(collected[peer1].cancels) != 1 {
+		t.Fatalf("Expected cancel to be sent for 1 want-blocks, got %d", len(collected[peer1].cancels))
 	}
 }