Merge bitcoin/bitcoin#27264: p2p: Improve diversification of new connections

72e8ffd p2p: Account for MANUAL conns when diversifying persistent outbound conns (Gleb Naumenko)
3faae99 p2p: Diversify connections only w.r.t *persistent* outbound peers (Gleb Naumenko)

Pull request description:

  Revives #19860.

  In order to make sure that our persistent outbound slots belong to different netgroups, distinct net groups of our peers are added to [`setConnected`](https://github.com/bitcoin/bitcoin/blob/8c4958bd4c06026dc108bc7f5f063d1f389d279b/src/net.cpp#L1716). We’d only open a persistent outbound connection to peers which have a different netgroup compared to those netgroups present in `setConnected`.

  **behaviour on master**

  we open persistent outbound connections to peers which have different netgroups compared to outbound full relay, block relay, addrfetch and feeler connection peers.

  **behaviour on PR**

  netgroup diversity is based on outbound full relay, block relay and manual connection peers.

  **rationale**

  - addrfetch and feeler connections are short lived connections and shouldn’t affect how we select outbound peers from addrman.
  - manual connections are like regular connections when viewed from addrman’s netgroup diversity point of view and should affect how we select outbound peers from addrman

ACKs for top commit:
  amitiuttarwar:
    code review ACK 72e8ffd
  vasild:
    ACK 72e8ffd
  mzumsande:
    Code Review ACK 72e8ffd
  brunoerg:
    crACK 72e8ffd

Tree-SHA512: 359451945a707b312ef6c2696a3a9d4256ab14dab9bd461cca4a52dae034db099012df6de3faef2f3fb38184b05996402ac280b681959483824419b6deb4db1a

Author: fanquake

src/net.cpp

@@ -1721,19 +1721,20 @@ void CConnman::ThreadOpenConnections(const std::vector<std::string> connect)
                 if (pnode->IsFullOutboundConn()) nOutboundFullRelay++;
                 if (pnode->IsBlockOnlyConn()) nOutboundBlockRelay++;
 
-                // Netgroups for inbound and manual peers are not excluded because our goal here
-                // is to not use multiple of our limited outbound slots on a single netgroup
-                // but inbound and manual peers do not use our outbound slots. Inbound peers
-                // also have the added issue that they could be attacker controlled and used
-                // to prevent us from connecting to particular hosts if we used them here.
+                // Make sure our persistent outbound slots belong to different netgroups.
                 switch (pnode->m_conn_type) {
+                    // We currently don't take inbound connections into account. Since they are
+                    // free to make, an attacker could make them to prevent us from connecting to
+                    // certain peers.
                     case ConnectionType::INBOUND:
-                    case ConnectionType::MANUAL:
+                    // Short-lived outbound connections should not affect how we select outbound
+                    // peers from addrman.
+                    case ConnectionType::ADDR_FETCH:
+                    case ConnectionType::FEELER:
                         break;
+                    case ConnectionType::MANUAL:
                     case ConnectionType::OUTBOUND_FULL_RELAY:
                     case ConnectionType::BLOCK_RELAY:
-                    case ConnectionType::ADDR_FETCH:
-                    case ConnectionType::FEELER:
                         setConnected.insert(m_netgroupman.GetGroup(pnode->addr));
                 } // no default case, so the compiler can warn about missing cases
             }