From 6eede89a4329162228827bbdd1915a17619af1f0 Mon Sep 17 00:00:00 2001
From: Olaoluwa Osuntokun <laolu32@gmail.com>
Date: Fri, 12 Nov 2021 18:03:37 -0800
Subject: [PATCH] chain: fix peer selection for the pruned block dispatcher

In this commit, we fix the peer selection for the pruned block
dispatcher. Turns out that the bits are actually mutually exclusive: a
node can set the network bit _and_ the limited (pruned bit).

The prior logic would only seek out "pure" nodes, meaning older nodes
that wouldn't set the limited bit when they're a full-full node. We fix
this by ignoring the limited bit, and we'll only look at the network and
segwit bits.

A unit test has also been added to show the issue in the prior filter
function.
---
 chain/pruned_block_dispatcher.go      |  8 +++---
 chain/pruned_block_dispatcher_test.go | 37 +++++++++++++++++++++++++++
 2 files changed, 42 insertions(+), 3 deletions(-)

diff --git a/chain/pruned_block_dispatcher.go b/chain/pruned_block_dispatcher.go
index aec8d02..4a80a5f 100644
--- a/chain/pruned_block_dispatcher.go
+++ b/chain/pruned_block_dispatcher.go
@@ -417,10 +417,12 @@ func filterNodeAddrs(nodeAddrs []btcjson.GetNodeAddressesResult) []string {
 }
 
 // satisfiesRequiredServices determines whether the services signaled by a peer
-// satisfy our requirements for retrieving pruned blocks from them.
+// satisfy our requirements for retrieving pruned blocks from them. We need the
+// full chain, and witness data as well. Note that we ignore the limited
+// (pruned bit) as nodes can have the full data and set that as well. Pure
+// pruned nodes won't set the network bit.
 func satisfiesRequiredServices(services wire.ServiceFlag) bool {
-	return services&requiredServices == requiredServices &&
-		services&prunedNodeService != prunedNodeService
+	return services&requiredServices == requiredServices
 }
 
 // newQueryPeer creates a new peer instance configured to relay any received
diff --git a/chain/pruned_block_dispatcher_test.go b/chain/pruned_block_dispatcher_test.go
index 8b319d7..50af406 100644
--- a/chain/pruned_block_dispatcher_test.go
+++ b/chain/pruned_block_dispatcher_test.go
@@ -620,3 +620,40 @@ func TestPrunedBlockDispatcherInvalidBlock(t *testing.T) {
 	h.assertPeerQueried()
 	h.assertPeerReplied(blockChan, errChan, true)
 }
+
+func TestSatisfiesRequiredServices(t *testing.T) {
+	t.Parallel()
+
+	testCases := []struct {
+		name     string
+		services wire.ServiceFlag
+		ok       bool
+	}{
+		{
+			name:     "full node, segwit",
+			services: wire.SFNodeWitness | wire.SFNodeNetwork,
+			ok:       true,
+		},
+		{
+			name:     "full node segwit, signals limited",
+			services: wire.SFNodeWitness | wire.SFNodeNetwork | prunedNodeService,
+			ok:       true,
+		},
+		{
+			name:     "full node, no segwit",
+			services: wire.SFNodeNetwork,
+			ok:       false,
+		},
+		{
+			name:     "segwit, pure pruned",
+			services: wire.SFNodeWitness | prunedNodeService,
+			ok:       false,
+		},
+	}
+	for _, testCase := range testCases {
+		ok := satisfiesRequiredServices(testCase.services)
+		require.Equal(
+			t, testCase.ok, ok, fmt.Sprintf("test case: %v", testCase.name),
+		)
+	}
+}