Merge pull request #2035 from kcalvinalvin/2023-09-25-change-is-sync-…

…candidate-behavior

wire, netsync: change isSyncCandidate behavior

netsync/manager.go

@@ -397,20 +397,55 @@ func (sm *SyncManager) isSyncCandidate(peer *peerpkg.Peer) bool {
 		if host != "127.0.0.1" && host != "localhost" {
 			return false
 		}
-	} else {
-		// The peer is not a candidate for sync if it's not a full
-		// node. Additionally, if the segwit soft-fork package has
-		// activated, then the peer must also be upgraded.
-		segwitActive, err := sm.chain.IsDeploymentActive(chaincfg.DeploymentSegwit)
-		if err != nil {
-			log.Errorf("Unable to query for segwit "+
-				"soft-fork state: %v", err)
-		}
-		nodeServices := peer.Services()
-		if nodeServices&wire.SFNodeNetwork != wire.SFNodeNetwork ||
-			(segwitActive && !peer.IsWitnessEnabled()) {
+
+		// Candidate if all checks passed.
+		return true
+	}
+
+	// If the segwit soft-fork package has activated, then the peer must
+	// also be upgraded.
+	segwitActive, err := sm.chain.IsDeploymentActive(
+		chaincfg.DeploymentSegwit,
+	)
+	if err != nil {
+		log.Errorf("Unable to query for segwit soft-fork state: %v",
+			err)
+	}
+
+	if segwitActive && !peer.IsWitnessEnabled() {
+		return false
+	}
+
+	var (
+		nodeServices = peer.Services()
+		fullNode     = nodeServices.HasFlag(wire.SFNodeNetwork)
+		prunedNode   = nodeServices.HasFlag(wire.SFNodeNetworkLimited)
+	)
+
+	switch {
+	case fullNode:
+		// Node is a sync candidate if it has all the blocks.
+
+	case prunedNode:
+		// Even if the peer is pruned, if they have the node network
+		// limited flag, they are able to serve 2 days worth of blocks
+		// from the current tip. Therefore, check if our chaintip is
+		// within that range.
+		bestHeight := sm.chain.BestSnapshot().Height
+		peerLastBlock := peer.LastBlock()
+
+		// bestHeight+1 as we need the peer to serve us the next block,
+		// not the one we already have.
+		if bestHeight+1 <=
+			peerLastBlock-wire.NodeNetworkLimitedBlockThreshold {
+
 			return false
 		}
+
+	default:
+		// If the peer isn't an archival node, and it's not signaling
+		// NODE_NETWORK_LIMITED, we can't sync off of this node.
+		return false
 	}
 
 	// Candidate if all checks passed.
@@ -428,7 +463,7 @@ func (sm *SyncManager) handleNewPeerMsg(peer *peerpkg.Peer) {
 
 	log.Infof("New valid peer %s (%s)", peer, peer.UserAgent())
 
-	// Initialize the peer state
+	// Initialize the peer state.
 	isSyncCandidate := sm.isSyncCandidate(peer)
 	sm.peerStates[peer] = &peerSyncState{
 		syncCandidate:   isSyncCandidate,

wire/protocol.go

@@ -60,6 +60,12 @@ const (
 	AddrV2Version uint32 = 70016
 )
 
+const (
+	// NodeNetworkLimitedBlockThreshold is the number of blocks that a node
+	// broadcasting SFNodeNetworkLimited MUST be able to serve from the tip.
+	NodeNetworkLimitedBlockThreshold = 288
+)
+
 // ServiceFlag identifies services supported by a bitcoin peer.
 type ServiceFlag uint64
 
@@ -126,6 +132,11 @@ var orderedSFStrings = []ServiceFlag{
 	SFNodeNetworkLimited,
 }
 
+// HasFlag returns a bool indicating if the service has the given flag.
+func (f ServiceFlag) HasFlag(s ServiceFlag) bool {
+	return f&s == s
+}
+
 // String returns the ServiceFlag in human-readable form.
 func (f ServiceFlag) String() string {
 	// No flags are set.

wire/protocol_test.go

@@ -4,7 +4,11 @@
 
 package wire
 
-import "testing"
+import (
+	"testing"
+
+	"github.com/stretchr/testify/require"
+)
 
 // TestServiceFlagStringer tests the stringized output for service flag types.
 func TestServiceFlagStringer(t *testing.T) {
@@ -59,3 +63,19 @@ func TestBitcoinNetStringer(t *testing.T) {
 		}
 	}
 }
+
+func TestHasFlag(t *testing.T) {
+	tests := []struct {
+		in    ServiceFlag
+		check ServiceFlag
+		want  bool
+	}{
+		{0, SFNodeNetwork, false},
+		{SFNodeNetwork | SFNodeNetworkLimited | SFNodeWitness, SFNodeBloom, false},
+		{SFNodeNetwork | SFNodeNetworkLimited | SFNodeWitness, SFNodeNetworkLimited, true},
+	}
+
+	for _, test := range tests {
+		require.Equal(t, test.want, test.in.HasFlag(test.check))
+	}
+}