Merge #11456: Replace relevant services logic with a function suite.

15f5d3b17 Switch DNSSeed-needed metric to any-automatic-nodes, not services (Matt Corallo)
5ee88b4bd Clarify docs for requirements/handling of addnode/connect nodes (Matt Corallo)
57edc0b0c Rename fAddnode to a more-descriptive "manual_connection" (Matt Corallo)
44407100f Replace relevant services logic with a function suite. (Matt Corallo)

Pull request description:

  This was mostly written as a way to clean things up so that the NETWORK_LIMITED PR (#10387) can be simplified a ton, but its also a nice standalone cleanup that will also require a bit of review because it tweaks a lot of stuff across net. The new functions are fine in protocol.h right now since they're straight-forward, but after NETWORK_LIMITED will really want to move elsewhere after @theuni moves the nServices-based selection to addrman from connman.

  Adds HasAllRelevantServices and GetRelevantServices, which check
  for NETWORK|WITNESS.

  This changes the following:
   * Removes nRelevantServices from CConnman, disconnecting it a bit
     more from protocol-level logic.
   * Replaces our sometimes-connect-to-!WITNESS-nodes logic with
     simply always requiring WITNESS|NETWORK for outbound non-feeler
     connections (feelers still only require NETWORK).
   * This has the added benefit of removing nServicesExpected from
     CNode - instead letting net_processing's VERSION message
     handling simply check HasAllRelevantServices.
   * This implies we believe WITNESS nodes to continue to be a
     significant majority of nodes on the network, but also because
     we cannot sync properly from !WITNESS nodes, it is strange to
     continue using our valuable outbound slots on them.
   * In order to prevent this change from preventing connection to
     -connect= nodes which have !WITNESS, -connect nodes are now
     given the "addnode" flag. This also allows outbound connections
     to !NODE_NETWORK nodes for -connect nodes (which was already true
     of addnodes).
   * Has the (somewhat unintended) consequence of changing one of the
     eviction metrics from the same
     sometimes-connect-to-!WITNESS-nodes metric to requiring
     HasRelevantServices.

  This should make NODE_NETWORK_LIMITED much simpler to implement.

Tree-SHA512: 90606896c86cc5da14c77843b16674a6a012065e7b583d76d1c47a18215358abefcbab44ff4fab3fadcd39aa9a42d4740c6dc8874a58033bdfc8ad3fb5c649fc

1 files changed, 9 insertions, 6 deletions

diff --git a/src/net_processing.cpp b/src/net_processing.cpp
index f88982a2a0..6c26cd4cee 100644
--- a/src/net_processing.cpp
+++ b/src/net_processing.cpp
@@ -1243,11 +1243,11 @@ bool static ProcessMessage(CNode* pfrom, const std::string& strCommand, CDataStr
         {
             connman->SetServices(pfrom->addr, nServices);
         }
-        if (pfrom->nServicesExpected & ~nServices)
+        if (!pfrom->fInbound && !pfrom->fFeeler && !pfrom->m_manual_connection && !HasAllDesirableServiceFlags(nServices))
         {
-            LogPrint(BCLog::NET, "peer=%d does not offer the expected services (%08x offered, %08x expected); disconnecting\n", pfrom->GetId(), nServices, pfrom->nServicesExpected);
+            LogPrint(BCLog::NET, "peer=%d does not offer the expected services (%08x offered, %08x expected); disconnecting\n", pfrom->GetId(), nServices, GetDesirableServiceFlags(nServices));
             connman->PushMessage(pfrom, CNetMsgMaker(INIT_PROTO_VERSION).Make(NetMsgType::REJECT, strCommand, REJECT_NONSTANDARD,
-                               strprintf("Expected to offer services %08x", pfrom->nServicesExpected)));
+                               strprintf("Expected to offer services %08x", GetDesirableServiceFlags(nServices))));
             pfrom->fDisconnect = true;
             return false;
         }
@@ -1466,7 +1466,10 @@ bool static ProcessMessage(CNode* pfrom, const std::string& strCommand, CDataStr
             if (interruptMsgProc)
                 return true;
 
-            if ((addr.nServices & REQUIRED_SERVICES) != REQUIRED_SERVICES)
+            // We only bother storing full nodes, though this may include
+            // things which we would not make an outbound connection to, in
+            // part because we may make feeler connections to them.
+            if (!MayHaveUsefulAddressDB(addr.nServices))
                 continue;
 
             if (addr.nTime <= 100000000 || addr.nTime > nNow + 10 * 60)
@@ -2642,8 +2645,8 @@ static bool SendRejectsAndCheckIfBanned(CNode* pnode, CConnman* connman)
         state.fShouldBan = false;
         if (pnode->fWhitelisted)
             LogPrintf("Warning: not punishing whitelisted peer %s!\n", pnode->addr.ToString());
-        else if (pnode->fAddnode)
-            LogPrintf("Warning: not punishing addnoded peer %s!\n", pnode->addr.ToString());
+        else if (pnode->m_manual_connection)
+            LogPrintf("Warning: not punishing manually-connected peer %s!\n", pnode->addr.ToString());
         else {
             pnode->fDisconnect = true;
             if (pnode->addr.IsLocal())