From 2d4df2998df64a99d5d2c43d8e6ae178f2ab4f01 Mon Sep 17 00:00:00 2001 From: gammazero <11790789+gammazero@users.noreply.github.com> Date: Wed, 22 Jan 2025 08:45:57 -1000 Subject: [PATCH] fix sending cancels when excluding peer When sending cancels for want-block and want-have requests, a peer may be sxcluded because it was the sender of a block and that peer will cancel the want itself without having to receive a cancel message. The would-be sender of tha cancel message still needs to do everything, such as adjusting want counts, as if the cancel were being sent and then not send out the cancel message. --- .../internal/peermanager/peermanager_test.go | 10 +++++----- .../internal/peermanager/peerwantmanager.go | 18 ++++++++++++------ 2 files changed, 17 insertions(+), 11 deletions(-) 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)) } } 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 {