Merge bitcoin/bitcoin#27467: p2p: skip netgroup diversity follow-up

11bb31c p2p: "skip netgroup diversity of new connections for tor/i2p/cjdns" follow-up (Jon Atack)

Pull request description:

  In #27374 the role of the `setConnected` data structure in `CConnman::ThreadOpenConnections` changed from the set of outbound peer netgroups to those of outbound IPv4/6 peers only.

  In accordance with the changed semantics, this pull fixes a code comment regarding feeler connections and updates the naming of `setConnected` to `outbound_ipv46_peer_netgroups`.

  Addresses bitcoin/bitcoin#27374 (comment).

ACKs for top commit:
  mzumsande:
    Code Review ACK 11bb31c
  vasild:
    ACK 11bb31c
  ryanofsky:
    Code review ACK 11bb31c

Tree-SHA512: df9151a6cce53c279e549683a9f30fdc23d513dc664cfee1cf0eb8ec80b2848d32c80a92cc0a9f47d967f305864975ffb339fe0eaa80bc3bef1b28406419eb96

Author: ryanofsky

src/net.cpp

@@ -1703,7 +1703,7 @@ void CConnman::ThreadOpenConnections(const std::vector<std::string> connect)
         int nOutboundFullRelay = 0;
         int nOutboundBlockRelay = 0;
         int outbound_privacy_network_peers = 0;
-        std::set<std::vector<unsigned char>> setConnected; // netgroups of our ipv4/ipv6 outbound peers
+        std::set<std::vector<unsigned char>> outbound_ipv46_peer_netgroups;
 
         {
             LOCK(m_nodes_mutex);
@@ -1725,7 +1725,7 @@ void CConnman::ThreadOpenConnections(const std::vector<std::string> connect)
                     case ConnectionType::MANUAL:
                     case ConnectionType::OUTBOUND_FULL_RELAY:
                     case ConnectionType::BLOCK_RELAY:
-                        CAddress address{pnode->addr};
+                        const CAddress address{pnode->addr};
                         if (address.IsTor() || address.IsI2P() || address.IsCJDNS()) {
                             // Since our addrman-groups for these networks are
                             // random, without relation to the route we
@@ -1736,7 +1736,7 @@ void CConnman::ThreadOpenConnections(const std::vector<std::string> connect)
                             // these networks.
                             ++outbound_privacy_network_peers;
                         } else {
-                            setConnected.insert(m_netgroupman.GetGroup(address));
+                            outbound_ipv46_peer_netgroups.insert(m_netgroupman.GetGroup(address));
                         }
                 } // no default case, so the compiler can warn about missing cases
             }
@@ -1811,7 +1811,7 @@ void CConnman::ThreadOpenConnections(const std::vector<std::string> connect)
                 m_anchors.pop_back();
                 if (!addr.IsValid() || IsLocal(addr) || !IsReachable(addr) ||
                     !HasAllDesirableServiceFlags(addr.nServices) ||
-                    setConnected.count(m_netgroupman.GetGroup(addr))) continue;
+                    outbound_ipv46_peer_netgroups.count(m_netgroupman.GetGroup(addr))) continue;
                 addrConnect = addr;
                 LogPrint(BCLog::NET, "Trying to make an anchor connection to %s\n", addrConnect.ToStringAddrPort());
                 break;
@@ -1851,8 +1851,8 @@ void CConnman::ThreadOpenConnections(const std::vector<std::string> connect)
                 std::tie(addr, addr_last_try) = addrman.Select();
             }
 
-            // Require outbound connections, other than feelers, to be to distinct network groups
-            if (!fFeeler && setConnected.count(m_netgroupman.GetGroup(addr))) {
+            // Require outbound IPv4/IPv6 connections, other than feelers, to be to distinct network groups
+            if (!fFeeler && outbound_ipv46_peer_netgroups.count(m_netgroupman.GetGroup(addr))) {
                 break;
             }
 
@@ -1898,8 +1898,9 @@ void CConnman::ThreadOpenConnections(const std::vector<std::string> connect)
             // Record addrman failure attempts when node has at least 2 persistent outbound connections to peers with
             // different netgroups in ipv4/ipv6 networks + all peers in Tor/I2P/CJDNS networks.
             // Don't record addrman failure attempts when node is offline. This can be identified since all local
-            // network connections(if any) belong in the same netgroup and size of setConnected would only be 1.
-            OpenNetworkConnection(addrConnect, (int)setConnected.size() + outbound_privacy_network_peers >= std::min(nMaxConnections - 1, 2), &grant, nullptr, conn_type);
+            // network connections (if any) belong in the same netgroup, and the size of `outbound_ipv46_peer_netgroups` would only be 1.
+            const bool count_failures{((int)outbound_ipv46_peer_netgroups.size() + outbound_privacy_network_peers) >= std::min(nMaxConnections - 1, 2)};
+            OpenNetworkConnection(addrConnect, count_failures, &grant, /*strDest=*/nullptr, conn_type);
         }
     }
 }