Merge #6255: backport: merge bitcoin#24531, #25500, #25814, #25962, #26888, #27264, #27257, #27324, #27374, #27467, #27411, partial bitcoin#25472 (networking backports: part 8)

8320e0ca8e merge bitcoin#27411: Restrict self-advertisements with privacy networks to avoid fingerprinting (Kittywhiskers Van Gogh)
1376289b11 merge bitcoin#27467: skip netgroup diversity follow-up (Kittywhiskers Van Gogh)
a52b3a3bf0 merge bitcoin#27374: skip netgroup diversity of new connections for tor/i2p/cjdns (Kittywhiskers Van Gogh)
ab11e0f998 merge bitcoin#27324: bitcoin#27257 follow-ups (Kittywhiskers Van Gogh)
9023dd25af merge bitcoin#27257: End friendship of CNode, CConnman and ConnmanTestMsg (Kittywhiskers Van Gogh)
3465df2689 merge bitcoin#27264: Improve diversification of new connections (Kittywhiskers Van Gogh)
d3f5b3881b merge bitcoin#26888: simplify the call to vProcessMsg.splice() (Kittywhiskers Van Gogh)
d9e56f3e78 merge bitcoin#25962: Add CNodeOptions and increase constness (Kittywhiskers Van Gogh)
79e67fd96a merge bitcoin#25814: simplify GetLocalAddress() (Kittywhiskers Van Gogh)
6d4945418a partial bitcoin#25472: Increase MS Visual Studio minimum version (Kittywhiskers Van Gogh)
54bb3a438f merge bitcoin#25500: Move inbound eviction logic to its own translation unit (Kittywhiskers Van Gogh)
b50febc0f0 merge bitcoin#24531: Use designated initializers (Kittywhiskers Van Gogh)

Pull request description:

  ## Additional Information

  * Dependent on https://github.com/dashpay/dash/pull/6254
  * When backporting [bitcoin#27411](https://github.com/bitcoin/bitcoin/pull/27411), the `CNetAddr*` variant of `GetLocal()` was not removed (upstream it was replaced by the `CNode&` variant with additional checks that rely on fields in `CNode`) as `CActiveMasternodeManager` relies on `GetLocal()` to detect a valid external address.
    * While it can also rely on other nodes to determine that, removing code that tests against a well-known public address would increase the number of reported failures esp. if the checks are run _before_ the node has a chance to connect to any peers.

  ## Breaking Changes

  None observed.

  ## Checklist:

  - [x] I have performed a self-review of my own code
  - [x] I have commented my code, particularly in hard-to-understand areas **(note: N/A)**
  - [x] I have added or updated relevant unit/integration/functional/e2e tests
  - [x] I have made corresponding changes to the documentation **(note: N/A)**
  - [x] I have assigned this pull request to a milestone _(for repository code-owners and collaborators only)_

ACKs for top commit:
  UdjinM6:
    utACK 8320e0ca8e
  PastaPastaPasta:
    utACK 8320e0ca8e

Tree-SHA512: 1d02bc33c8d62c392960d4dd044edf3de08515a5e8c8794d95cd95e9654da91b20e7290436cf9c79b0ea8dbd42b27dcc61c8eb17e573902574d7b281b8874584

src/Makefile.am

@@ -271,7 +271,9 @@ BITCOIN_CORE_H = \
   node/blockstorage.h \
   node/coin.h \
   node/coinstats.h \
+  node/connection_types.h \
   node/context.h \
+  node/eviction.h \
   node/psbt.h \
   node/transaction.h \
   node/ui_interface.h \
@@ -498,7 +500,9 @@ libbitcoin_server_a_SOURCES = \
   node/blockstorage.cpp \
   node/coin.cpp \
   node/coinstats.cpp \
+  node/connection_types.cpp \
   node/context.cpp \
+  node/eviction.cpp \
   node/interfaces.cpp \
   node/psbt.cpp \
   node/transaction.cpp \

src/masternode/node.cpp

@@ -15,6 +15,42 @@
 #include <validation.h>
 #include <warnings.h>
 
+namespace {
+bool GetLocal(CService& addr, const CNetAddr* paddrPeer)
+{
+    if (!fListen)
+        return false;
+
+    int nBestScore = -1;
+    {
+        LOCK(g_maplocalhost_mutex);
+        int nBestReachability = -1;
+        for (const auto& entry : mapLocalHost)
+        {
+            // For privacy reasons, don't advertise our privacy-network address
+            // to other networks and don't advertise our other-network address
+            // to privacy networks.
+            const Network our_net{entry.first.GetNetwork()};
+            const Network peers_net{paddrPeer->GetNetwork()};
+            if (our_net != peers_net &&
+                (our_net == NET_ONION || our_net == NET_I2P ||
+                 peers_net == NET_ONION || peers_net == NET_I2P)) {
+                continue;
+            }
+            int nScore = entry.second.nScore;
+            int nReachability = entry.first.GetReachabilityFrom(*paddrPeer);
+            if (nReachability > nBestReachability || (nReachability == nBestReachability && nScore > nBestScore))
+            {
+                addr = CService(entry.first, entry.second.nPort);
+                nBestReachability = nReachability;
+                nBestScore = nScore;
+            }
+        }
+    }
+    return nBestScore >= 0;
+}
+} // anonymous namespace
+
 CActiveMasternodeManager::CActiveMasternodeManager(const CBLSSecretKey& sk, CConnman& connman, const std::unique_ptr<CDeterministicMNManager>& dmnman) :
     m_info(sk, sk.GetPublicKey()),
     m_connman{connman},
@@ -204,8 +240,7 @@ bool CActiveMasternodeManager::GetLocalAddress(CService& addrRet)
         auto service = m_info.service;
         m_connman.ForEachNodeContinueIf(CConnman::AllNodes, [&](CNode* pnode) {
             empty = false;
-            if (pnode->addr.IsIPv4())
+            if (pnode->addr.IsIPv4()) fFoundLocal = GetLocal(service, *pnode) && IsValidNetAddr(service);
-                fFoundLocal = GetLocal(service, &pnode->addr) && IsValidNetAddr(service);
             return !fFoundLocal;
         });
         // nothing and no live connections, can't do anything for now

src/net.cpp

@@ -17,6 +17,7 @@
 #include <compat.h>
 #include <consensus/consensus.h>
 #include <crypto/sha256.h>
+#include <node/eviction.h>
 #include <fs.h>
 #include <i2p.h>
 #include <memusage.h>
@@ -185,7 +186,7 @@ uint16_t GetListenPort()
 }
 
 // find 'best' local address for a particular peer
-bool GetLocal(CService& addr, const CNetAddr *paddrPeer)
+bool GetLocal(CService& addr, const CNode& peer)
 {
     if (!fListen)
         return false;
@@ -196,8 +197,18 @@ bool GetLocal(CService& addr, const CNetAddr *paddrPeer)
         LOCK(g_maplocalhost_mutex);
         for (const auto& entry : mapLocalHost)
         {
+            // For privacy reasons, don't advertise our privacy-network address
+            // to other networks and don't advertise our other-network address
+            // to privacy networks.
+            const Network our_net{entry.first.GetNetwork()};
+            const Network peers_net{peer.ConnectedThroughNetwork()};
+            if (our_net != peers_net &&
+                (our_net == NET_ONION || our_net == NET_I2P ||
+                 peers_net == NET_ONION || peers_net == NET_I2P)) {
+                continue;
+            }
             int nScore = entry.second.nScore;
-            int nReachability = entry.first.GetReachabilityFrom(paddrPeer);
+            int nReachability = entry.first.GetReachabilityFrom(peer.addr);
             if (nReachability > nBestReachability || (nReachability == nBestReachability && nScore > nBestScore))
             {
                 addr = CService(entry.first, entry.second.nPort);
@@ -235,14 +246,13 @@ static std::vector<CAddress> ConvertSeeds(const std::vector<uint8_t> &vSeedsIn)
 // Otherwise, return the unroutable 0.0.0.0 but filled in with
 // the normal parameters, since the IP may be changed to a useful
 // one by discovery.
-CService GetLocalAddress(const CNetAddr& addrPeer)
+CService GetLocalAddress(const CNode& peer)
 {
-    CService ret{CNetAddr(), GetListenPort()};
     CService addr;
-    if (GetLocal(addr, &addrPeer)) {
+    if (GetLocal(addr, peer)) {
-        ret = CService{addr};
+        return addr;
     }
-    return ret;
+    return CService{CNetAddr(), GetListenPort()};
 }
 
 static int GetnScore(const CService& addr)
@@ -262,7 +272,7 @@ bool IsPeerAddrLocalGood(CNode *pnode)
 
 std::optional<CService> GetLocalAddrForPeer(CNode& node)
 {
-    CService addrLocal{GetLocalAddress(node.addr)};
+    CService addrLocal{GetLocalAddress(node)};
     // If discovery is enabled, sometimes give our peer the address it
     // tells us that it sees us as in case it has a better idea of our
     // address than we do.
@@ -617,7 +627,10 @@ CNode* CConnman::ConnectNode(CAddress addrConnect, const char *pszDest, bool fCo
                              pszDest ? pszDest : "",
                              conn_type,
                              /*inbound_onion=*/false,
-                             std::move(i2p_transient_session));
+                             CNodeOptions{
+                                 .i2p_sam_session = std::move(i2p_transient_session),
+                                 .recv_flood_size = nReceiveFloodSize,
+                             });
     pnode->AddRef();
     ::g_stats_client->inc("peers.connect", 1.0f);
 
@@ -679,26 +692,6 @@ bool CNode::IsBlockRelayOnly() const {
     return (ignores_incoming_txs && !HasPermission(NetPermissionFlags::Relay)) || IsBlockOnlyConn();
 }
 
-std::string ConnectionTypeAsString(ConnectionType conn_type)
-{
-    switch (conn_type) {
-    case ConnectionType::INBOUND:
-        return "inbound";
-    case ConnectionType::MANUAL:
-        return "manual";
-    case ConnectionType::FEELER:
-        return "feeler";
-    case ConnectionType::OUTBOUND_FULL_RELAY:
-        return "outbound-full-relay";
-    case ConnectionType::BLOCK_RELAY:
-        return "block-relay-only";
-    case ConnectionType::ADDR_FETCH:
-        return "addr-fetch";
-    } // no default case, so the compiler can warn about missing cases
-
-    assert(false);
-}
-
 CService CNode::GetAddrLocal() const
 {
     AssertLockNotHeld(m_addr_local_mutex);
@@ -761,7 +754,7 @@ void CNode::CopyStats(CNodeStats& stats)
         X(nRecvBytes);
     }
     X(m_legacyWhitelisted);
-    X(m_permissionFlags);
+    X(m_permission_flags);
 
     X(m_last_ping_time);
     X(m_min_ping_time);
@@ -1054,7 +1047,7 @@ std::pair<size_t, bool> CConnman::SocketSendData(CNode& node) const
             // Notify transport that bytes have been processed.
             node.m_transport->MarkBytesSent(nBytes);
             // Update statistics per message type.
-            node.mapSendBytesPerMsgType[msg_type] += nBytes;
+            node.AccountForSentBytes(msg_type, nBytes);
             nSentSize += nBytes;
             if ((size_t)nBytes != data.size()) {
                 // could not send full message; stop sending more
@@ -1084,210 +1077,6 @@ std::pair<size_t, bool> CConnman::SocketSendData(CNode& node) const
     return {nSentSize, data_left};
 }
 
-static bool ReverseCompareNodeMinPingTime(const NodeEvictionCandidate& a, const NodeEvictionCandidate& b)
-{
-    return a.m_min_ping_time > b.m_min_ping_time;
-}
-
-static bool ReverseCompareNodeTimeConnected(const NodeEvictionCandidate& a, const NodeEvictionCandidate& b)
-{
-    return a.m_connected > b.m_connected;
-}
-
-static bool CompareNetGroupKeyed(const NodeEvictionCandidate &a, const NodeEvictionCandidate &b) {
-    return a.nKeyedNetGroup < b.nKeyedNetGroup;
-}
-
-static bool CompareNodeBlockTime(const NodeEvictionCandidate &a, const NodeEvictionCandidate &b)
-{
-    // There is a fall-through here because it is common for a node to have many peers which have not yet relayed a block.
-    if (a.m_last_block_time != b.m_last_block_time) return a.m_last_block_time < b.m_last_block_time;
-    if (a.fRelevantServices != b.fRelevantServices) return b.fRelevantServices;
-    return a.m_connected > b.m_connected;
-}
-
-static bool CompareNodeTXTime(const NodeEvictionCandidate &a, const NodeEvictionCandidate &b)
-{
-    // There is a fall-through here because it is common for a node to have more than a few peers that have not yet relayed txn.
-    if (a.m_last_tx_time != b.m_last_tx_time) return a.m_last_tx_time < b.m_last_tx_time;
-    if (a.m_relay_txs != b.m_relay_txs) return b.m_relay_txs;
-    if (a.fBloomFilter != b.fBloomFilter) return a.fBloomFilter;
-    return a.m_connected > b.m_connected;
-}
-
-// Pick out the potential block-relay only peers, and sort them by last block time.
-static bool CompareNodeBlockRelayOnlyTime(const NodeEvictionCandidate &a, const NodeEvictionCandidate &b)
-{
-    if (a.m_relay_txs != b.m_relay_txs) return a.m_relay_txs;
-    if (a.m_last_block_time != b.m_last_block_time) return a.m_last_block_time < b.m_last_block_time;
-    if (a.fRelevantServices != b.fRelevantServices) return b.fRelevantServices;
-    return a.m_connected > b.m_connected;
-}
-
-/**
- * Sort eviction candidates by network/localhost and connection uptime.
- * Candidates near the beginning are more likely to be evicted, and those
- * near the end are more likely to be protected, e.g. less likely to be evicted.
- * - First, nodes that are not `is_local` and that do not belong to `network`,
- *   sorted by increasing uptime (from most recently connected to connected longer).
- * - Then, nodes that are `is_local` or belong to `network`, sorted by increasing uptime.
- */
-struct CompareNodeNetworkTime {
-    const bool m_is_local;
-    const Network m_network;
-    CompareNodeNetworkTime(bool is_local, Network network) : m_is_local(is_local), m_network(network) {}
-    bool operator()(const NodeEvictionCandidate& a, const NodeEvictionCandidate& b) const
-    {
-        if (m_is_local && a.m_is_local != b.m_is_local) return b.m_is_local;
-        if ((a.m_network == m_network) != (b.m_network == m_network)) return b.m_network == m_network;
-        return a.m_connected > b.m_connected;
-    };
-};
-
-//! Sort an array by the specified comparator, then erase the last K elements where predicate is true.
-template <typename T, typename Comparator>
-static void EraseLastKElements(
-    std::vector<T>& elements, Comparator comparator, size_t k,
-    std::function<bool(const NodeEvictionCandidate&)> predicate = [](const NodeEvictionCandidate& n) { return true; })
-{
-    std::sort(elements.begin(), elements.end(), comparator);
-    size_t eraseSize = std::min(k, elements.size());
-    elements.erase(std::remove_if(elements.end() - eraseSize, elements.end(), predicate), elements.end());
-}
-
-void ProtectEvictionCandidatesByRatio(std::vector<NodeEvictionCandidate>& eviction_candidates)
-{
-    // Protect the half of the remaining nodes which have been connected the longest.
-    // This replicates the non-eviction implicit behavior, and precludes attacks that start later.
-    // To favorise the diversity of our peer connections, reserve up to half of these protected
-    // spots for Tor/onion, localhost, I2P, and CJDNS peers, even if they're not longest uptime
-    // overall. This helps protect these higher-latency peers that tend to be otherwise
-    // disadvantaged under our eviction criteria.
-    const size_t initial_size = eviction_candidates.size();
-    const size_t total_protect_size{initial_size / 2};
-
-    // Disadvantaged networks to protect. In the case of equal counts, earlier array members
-    // have the first opportunity to recover unused slots from the previous iteration.
-    struct Net { bool is_local; Network id; size_t count; };
-    std::array<Net, 4> networks{
-        {{false, NET_CJDNS, 0}, {false, NET_I2P, 0}, {/*localhost=*/true, NET_MAX, 0}, {false, NET_ONION, 0}}};
-
-    // Count and store the number of eviction candidates per network.
-    for (Net& n : networks) {
-        n.count = std::count_if(eviction_candidates.cbegin(), eviction_candidates.cend(),
-                                [&n](const NodeEvictionCandidate& c) {
-                                    return n.is_local ? c.m_is_local : c.m_network == n.id;
-                                });
-    }
-    // Sort `networks` by ascending candidate count, to give networks having fewer candidates
-    // the first opportunity to recover unused protected slots from the previous iteration.
-    std::stable_sort(networks.begin(), networks.end(), [](Net a, Net b) { return a.count < b.count; });
-
-    // Protect up to 25% of the eviction candidates by disadvantaged network.
-    const size_t max_protect_by_network{total_protect_size / 2};
-    size_t num_protected{0};
-
-    while (num_protected < max_protect_by_network) {
-        // Count the number of disadvantaged networks from which we have peers to protect.
-        auto num_networks = std::count_if(networks.begin(), networks.end(), [](const Net& n) { return n.count; });
-        if (num_networks == 0) {
-            break;
-        }
-        const size_t disadvantaged_to_protect{max_protect_by_network - num_protected};
-        const size_t protect_per_network{std::max(disadvantaged_to_protect / num_networks, static_cast<size_t>(1))};
-        // Early exit flag if there are no remaining candidates by disadvantaged network.
-        bool protected_at_least_one{false};
-
-        for (Net& n : networks) {
-            if (n.count == 0) continue;
-            const size_t before = eviction_candidates.size();
-            EraseLastKElements(eviction_candidates, CompareNodeNetworkTime(n.is_local, n.id),
-                               protect_per_network, [&n](const NodeEvictionCandidate& c) {
-                                   return n.is_local ? c.m_is_local : c.m_network == n.id;
-                               });
-            const size_t after = eviction_candidates.size();
-            if (before > after) {
-                protected_at_least_one = true;
-                const size_t delta{before - after};
-                num_protected += delta;
-                if (num_protected >= max_protect_by_network) {
-                    break;
-                }
-                n.count -= delta;
-            }
-        }
-        if (!protected_at_least_one) {
-            break;
-        }
-    }
-
-    // Calculate how many we removed, and update our total number of peers that
-    // we want to protect based on uptime accordingly.
-    assert(num_protected == initial_size - eviction_candidates.size());
-    const size_t remaining_to_protect{total_protect_size - num_protected};
-    EraseLastKElements(eviction_candidates, ReverseCompareNodeTimeConnected, remaining_to_protect);
-}
-
-[[nodiscard]] std::optional<NodeId> SelectNodeToEvict(std::vector<NodeEvictionCandidate>&& vEvictionCandidates)
-{
-    // Protect connections with certain characteristics
-
-    // Deterministically select 4 peers to protect by netgroup.
-    // An attacker cannot predict which netgroups will be protected
-    EraseLastKElements(vEvictionCandidates, CompareNetGroupKeyed, 4);
-    // Protect the 8 nodes with the lowest minimum ping time.
-    // An attacker cannot manipulate this metric without physically moving nodes closer to the target.
-    EraseLastKElements(vEvictionCandidates, ReverseCompareNodeMinPingTime, 8);
-    // Protect 4 nodes that most recently sent us novel transactions accepted into our mempool.
-    // An attacker cannot manipulate this metric without performing useful work.
-    EraseLastKElements(vEvictionCandidates, CompareNodeTXTime, 4);
-    // Protect up to 8 non-tx-relay peers that have sent us novel blocks.
-    EraseLastKElements(vEvictionCandidates, CompareNodeBlockRelayOnlyTime, 8,
-                       [](const NodeEvictionCandidate& n) { return !n.m_relay_txs && n.fRelevantServices; });
-
-    // Protect 4 nodes that most recently sent us novel blocks.
-    // An attacker cannot manipulate this metric without performing useful work.
-    EraseLastKElements(vEvictionCandidates, CompareNodeBlockTime, 4);
-
-    // Protect some of the remaining eviction candidates by ratios of desirable
-    // or disadvantaged characteristics.
-    ProtectEvictionCandidatesByRatio(vEvictionCandidates);
-
-    if (vEvictionCandidates.empty()) return std::nullopt;
-
-    // If any remaining peers are preferred for eviction consider only them.
-    // This happens after the other preferences since if a peer is really the best by other criteria (esp relaying blocks)
-    //  then we probably don't want to evict it no matter what.
-    if (std::any_of(vEvictionCandidates.begin(),vEvictionCandidates.end(),[](NodeEvictionCandidate const &n){return n.prefer_evict;})) {
-        vEvictionCandidates.erase(std::remove_if(vEvictionCandidates.begin(),vEvictionCandidates.end(),
-                                  [](NodeEvictionCandidate const &n){return !n.prefer_evict;}),vEvictionCandidates.end());
-    }
-
-    // Identify the network group with the most connections and youngest member.
-    // (vEvictionCandidates is already sorted by reverse connect time)
-    uint64_t naMostConnections;
-    unsigned int nMostConnections = 0;
-    std::chrono::seconds nMostConnectionsTime{0};
-    std::map<uint64_t, std::vector<NodeEvictionCandidate> > mapNetGroupNodes;
-    for (const NodeEvictionCandidate &node : vEvictionCandidates) {
-        std::vector<NodeEvictionCandidate> &group = mapNetGroupNodes[node.nKeyedNetGroup];
-        group.push_back(node);
-        const auto grouptime{group[0].m_connected};
-
-        if (group.size() > nMostConnections || (group.size() == nMostConnections && grouptime > nMostConnectionsTime)) {
-            nMostConnections = group.size();
-            nMostConnectionsTime = grouptime;
-            naMostConnections = node.nKeyedNetGroup;
-        }
-    }
-
-    // Reduce to the network group with the most connections
-    vEvictionCandidates = std::move(mapNetGroupNodes[naMostConnections]);
-
-    // Disconnect from the network group with the most connections
-    return vEvictionCandidates.front().id;
-}
-
 /** Try to find a connection to evict when the node is full.
  *  Extreme care must be taken to avoid opening the node to attacker
  *   triggered network partitioning.
@@ -1303,10 +1092,6 @@ bool CConnman::AttemptToEvictConnection()
         LOCK(m_nodes_mutex);
 
         for (const CNode* node : m_nodes) {
-            if (node->HasPermission(NetPermissionFlags::NoBan))
-                continue;
-            if (!node->IsInboundConn())
-                continue;
             if (node->fDisconnect)
                 continue;
 
@@ -1329,12 +1114,22 @@ bool CConnman::AttemptToEvictConnection()
                 }
             }
 
-            NodeEvictionCandidate candidate = {node->GetId(), node->m_connected, node->m_min_ping_time,
+            NodeEvictionCandidate candidate{
-                                               node->m_last_block_time, node->m_last_tx_time,
+                .id = node->GetId(),
-                                               node->m_has_all_wanted_services,
+                .m_connected = node->m_connected,
-                                               node->m_relays_txs.load(), node->m_bloom_filter_loaded.load(),
+                .m_min_ping_time = node->m_min_ping_time,
-                                               node->nKeyedNetGroup, node->m_prefer_evict, node->addr.IsLocal(),
+                .m_last_block_time = node->m_last_block_time,
-                                               node->ConnectedThroughNetwork()};
+                .m_last_tx_time = node->m_last_tx_time,
+                .fRelevantServices = node->m_has_all_wanted_services,
+                .m_relay_txs = node->m_relays_txs.load(),
+                .fBloomFilter = node->m_bloom_filter_loaded.load(),
+                .nKeyedNetGroup = node->nKeyedNetGroup,
+                .prefer_evict = node->m_prefer_evict,
+                .m_is_local = node->addr.IsLocal(),
+                .m_network = node->ConnectedThroughNetwork(),
+                .m_noban = node->HasPermission(NetPermissionFlags::NoBan),
+                .m_conn_type = node->m_conn_type,
+            };
             vEvictionCandidates.push_back(candidate);
         }
     }
@@ -1375,14 +1170,14 @@ void CConnman::AcceptConnection(const ListenSocket& hListenSocket, CMasternodeSy
 
     const CAddress addr_bind{MaybeFlipIPv6toCJDNS(GetBindAddress(*sock)), NODE_NONE};
 
-    NetPermissionFlags permissionFlags = NetPermissionFlags::None;
+    NetPermissionFlags permission_flags = NetPermissionFlags::None;
-    hListenSocket.AddSocketPermissionFlags(permissionFlags);
+    hListenSocket.AddSocketPermissionFlags(permission_flags);
 
-    CreateNodeFromAcceptedSocket(std::move(sock), permissionFlags, addr_bind, addr, mn_sync);
+    CreateNodeFromAcceptedSocket(std::move(sock), permission_flags, addr_bind, addr, mn_sync);
 }
 
 void CConnman::CreateNodeFromAcceptedSocket(std::unique_ptr<Sock>&& sock,
-                                            NetPermissionFlags permissionFlags,
+                                            NetPermissionFlags permission_flags,
                                             const CAddress& addr_bind,
                                             const CAddress& addr,
                                             CMasternodeSync& mn_sync)
@@ -1391,14 +1186,14 @@ void CConnman::CreateNodeFromAcceptedSocket(std::unique_ptr<Sock>&& sock,
     int nVerifiedInboundMasternodes = 0;
     int nMaxInbound = nMaxConnections - m_max_outbound;
 
-    AddWhitelistPermissionFlags(permissionFlags, addr);
+    AddWhitelistPermissionFlags(permission_flags, addr);
     bool legacyWhitelisted = false;
-    if (NetPermissions::HasFlag(permissionFlags, NetPermissionFlags::Implicit)) {
+    if (NetPermissions::HasFlag(permission_flags, NetPermissionFlags::Implicit)) {
-        NetPermissions::ClearFlag(permissionFlags, NetPermissionFlags::Implicit);
+        NetPermissions::ClearFlag(permission_flags, NetPermissionFlags::Implicit);
-        if (gArgs.GetBoolArg("-whitelistforcerelay", DEFAULT_WHITELISTFORCERELAY)) NetPermissions::AddFlag(permissionFlags, NetPermissionFlags::ForceRelay);
+        if (gArgs.GetBoolArg("-whitelistforcerelay", DEFAULT_WHITELISTFORCERELAY)) NetPermissions::AddFlag(permission_flags, NetPermissionFlags::ForceRelay);
-        if (gArgs.GetBoolArg("-whitelistrelay", DEFAULT_WHITELISTRELAY)) NetPermissions::AddFlag(permissionFlags, NetPermissionFlags::Relay);
+        if (gArgs.GetBoolArg("-whitelistrelay", DEFAULT_WHITELISTRELAY)) NetPermissions::AddFlag(permission_flags, NetPermissionFlags::Relay);
-        NetPermissions::AddFlag(permissionFlags, NetPermissionFlags::Mempool);
+        NetPermissions::AddFlag(permission_flags, NetPermissionFlags::Mempool);
-        NetPermissions::AddFlag(permissionFlags, NetPermissionFlags::NoBan);
+        NetPermissions::AddFlag(permission_flags, NetPermissionFlags::NoBan);
         legacyWhitelisted = true;
     }
 
@@ -1443,7 +1238,7 @@ void CConnman::CreateNodeFromAcceptedSocket(std::unique_ptr<Sock>&& sock,
 
     // Don't accept connections from banned peers.
     bool banned = m_banman && m_banman->IsBanned(addr);
-    if (!NetPermissions::HasFlag(permissionFlags, NetPermissionFlags::NoBan) && banned)
+    if (!NetPermissions::HasFlag(permission_flags, NetPermissionFlags::NoBan) && banned)
     {
         LogPrint(BCLog::NET, "%s (banned)\n", strDropped);
         return;
@@ -1451,7 +1246,7 @@ void CConnman::CreateNodeFromAcceptedSocket(std::unique_ptr<Sock>&& sock,
 
     // Only accept connections from discouraged peers if our inbound slots aren't (almost) full.
     bool discouraged = m_banman && m_banman->IsDiscouraged(addr);
-    if (!NetPermissions::HasFlag(permissionFlags, NetPermissionFlags::NoBan) && nInbound + 1 >= nMaxInbound && discouraged)
+    if (!NetPermissions::HasFlag(permission_flags, NetPermissionFlags::NoBan) && nInbound + 1 >= nMaxInbound && discouraged)
     {
         LogPrint(BCLog::NET, "connection from %s dropped (discouraged)\n", addr.ToStringAddrPort());
         return;
@@ -1482,7 +1277,7 @@ void CConnman::CreateNodeFromAcceptedSocket(std::unique_ptr<Sock>&& sock,
     uint64_t nonce = GetDeterministicRandomizer(RANDOMIZER_ID_LOCALHOSTNONCE).Write(id).Finalize();
 
     ServiceFlags nodeServices = nLocalServices;
-    if (NetPermissions::HasFlag(permissionFlags, NetPermissionFlags::BloomFilter)) {
+    if (NetPermissions::HasFlag(permission_flags, NetPermissionFlags::BloomFilter)) {
         nodeServices = static_cast<ServiceFlags>(nodeServices | NODE_BLOOM);
     }
 
@@ -1495,12 +1290,15 @@ void CConnman::CreateNodeFromAcceptedSocket(std::unique_ptr<Sock>&& sock,
                              addr_bind,
                              /*addrNameIn=*/"",
                              ConnectionType::INBOUND,
-                             inbound_onion);
+                             inbound_onion,
+                             CNodeOptions{
+                                 .permission_flags = permission_flags,
+                                 .prefer_evict = discouraged,
+                                 .recv_flood_size = nReceiveFloodSize,
+                             });
     pnode->AddRef();
-    pnode->m_permissionFlags = permissionFlags;
     // If this flag is present, the user probably expect that RPC and QT report it as whitelisted (backward compatibility)
     pnode->m_legacyWhitelisted = legacyWhitelisted;
-    pnode->m_prefer_evict = discouraged;
     m_msgproc->InitializeNode(*pnode, nodeServices);
 
     {
@@ -1710,16 +1508,8 @@ void CConnman::CalculateNumConnectionsChangedStats()
     mapSentBytesMsgStats[NET_MESSAGE_TYPE_OTHER] = 0;
     const NodesSnapshot snap{*this, /* filter = */ CConnman::FullyConnectedOnly};
     for (auto pnode : snap.Nodes()) {
-        {
+        WITH_LOCK(pnode->cs_vRecv, pnode->UpdateRecvMapWithStats(mapRecvBytesMsgStats));
-            LOCK(pnode->cs_vRecv);
+        WITH_LOCK(pnode->cs_vSend, pnode->UpdateSentMapWithStats(mapSentBytesMsgStats));
-            for (const mapMsgTypeSize::value_type &i : pnode->mapRecvBytesPerMsgType)
-                mapRecvBytesMsgStats[i.first] += i.second;
-        }
-        {
-            LOCK(pnode->cs_vSend);
-            for (const mapMsgTypeSize::value_type &i : pnode->mapSendBytesPerMsgType)
-                mapSentBytesMsgStats[i.first] += i.second;
-        }
         if (pnode->m_bloom_filter_loaded.load()) {
             spvNodes++;
         } else {
@@ -2312,19 +2102,7 @@ size_t CConnman::SocketRecvData(CNode *pnode)
         }
         RecordBytesRecv(nBytes);
         if (notify) {
-            size_t nSizeAdded = 0;
+            pnode->MarkReceivedMsgsForProcessing();
-            auto it(pnode->vRecvMsg.begin());
-            for (; it != pnode->vRecvMsg.end(); ++it) {
-                // vRecvMsg contains only completed CNetMessage
-                // the single possible partially deserialized message are held by TransportDeserializer
-                nSizeAdded += it->m_raw_message_size;
-            }
-            {
-                LOCK(pnode->cs_vProcessMsg);
-                pnode->vProcessMsg.splice(pnode->vProcessMsg.end(), pnode->vRecvMsg, pnode->vRecvMsg.begin(), it);
-                pnode->nProcessQueueSize += nSizeAdded;
-                pnode->fPauseRecv = pnode->nProcessQueueSize > nReceiveFloodSize;
-            }
             WakeMessageHandler();
         }
     }
@@ -2686,12 +2464,14 @@ void CConnman::ThreadOpenConnections(const std::vector<std::string> connect, CDe
         //
         CAddress addrConnect;
 
-        // Only connect out to one peer per network group (/16 for IPv4).
+        // Only connect out to one peer per ipv4/ipv6 network group (/16 for IPv4).
         // This is only done for mainnet and testnet
         int nOutboundFullRelay = 0;
         int nOutboundBlockRelay = 0;
         int nOutboundOnionRelay = 0;
-        std::set<std::vector<unsigned char> > setConnected;
+        int outbound_privacy_network_peers = 0;
+        std::set<std::vector<unsigned char>> outbound_ipv46_peer_netgroups;
+
         if (!Params().AllowMultipleAddressesFromGroup()) {
             LOCK(m_nodes_mutex);
             for (const CNode* pnode : m_nodes) {
@@ -2699,20 +2479,33 @@ void CConnman::ThreadOpenConnections(const std::vector<std::string> connect, CDe
                 if (pnode->IsBlockOnlyConn()) nOutboundBlockRelay++;
                 if (pnode->IsFullOutboundConn() && pnode->ConnectedThroughNetwork() == Network::NET_ONION) nOutboundOnionRelay++;
 
-                // Netgroups for inbound and manual peers are not excluded because our goal here
+                // Make sure our persistent outbound slots to ipv4/ipv6 peers belong to different netgroups.
-                // 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.
                 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
-                        break;
+                    // peers from addrman.
-                    case ConnectionType::OUTBOUND_FULL_RELAY:
-                    case ConnectionType::BLOCK_RELAY:
                     case ConnectionType::ADDR_FETCH:
                     case ConnectionType::FEELER:
-                        setConnected.insert(m_netgroupman.GetGroup(pnode->addr));
+                        break;
+                    case ConnectionType::MANUAL:
+                    case ConnectionType::OUTBOUND_FULL_RELAY:
+                    case ConnectionType::BLOCK_RELAY:
+                        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
+                            // take to connect to these peers or to the
+                            // difficulty in obtaining addresses with diverse
+                            // groups, we don't worry about diversity with
+                            // respect to our addrman groups when connecting to
+                            // these networks.
+                            ++outbound_privacy_network_peers;
+                        } else {
+                            outbound_ipv46_peer_netgroups.insert(m_netgroupman.GetGroup(address));
+                        }
                 } // no default case, so the compiler can warn about missing cases
             }
         }
@@ -2802,7 +2595,7 @@ void CConnman::ThreadOpenConnections(const std::vector<std::string> connect, CDe
                 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;
@@ -2845,13 +2638,13 @@ void CConnman::ThreadOpenConnections(const std::vector<std::string> connect, CDe
             auto dmn = mnList.GetMNByService(addr);
             bool isMasternode = dmn != nullptr;
 
-            // Require outbound connections, other than feelers, to be to distinct network groups
+            // Require outbound IPv4/IPv6 connections, other than feelers, to be to distinct network groups
-            if (!fFeeler && setConnected.count(m_netgroupman.GetGroup(addr))) {
+            if (!fFeeler && outbound_ipv46_peer_netgroups.count(m_netgroupman.GetGroup(addr))) {
                 break;
             }
 
             // if we selected an invalid address, restart
-            if (!addr.IsValid() || setConnected.count(m_netgroupman.GetGroup(addr)))
+            if (!addr.IsValid() || outbound_ipv46_peer_netgroups.count(m_netgroupman.GetGroup(addr)))
                 break;
 
             // don't try to connect to masternodes that we already have a connection to (most likely inbound)
@@ -2906,8 +2699,12 @@ void CConnman::ThreadOpenConnections(const std::vector<std::string> connect, CDe
                     LogPrint(BCLog::NET, "Making feeler connection\n");
                 }
             }
-
+            // Record addrman failure attempts when node has at least 2 persistent outbound connections to peers with
-            OpenNetworkConnection(addrConnect, (int)setConnected.size() >= std::min(nMaxConnections - 1, 2), &grant, nullptr, conn_type);
+            // 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 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);
         }
     }
 }
@@ -4250,8 +4047,6 @@ ServiceFlags CConnman::GetLocalServices() const
     return nLocalServices;
 }
 
-unsigned int CConnman::GetReceiveFloodSize() const { return nReceiveFloodSize; }
-
 CNode::CNode(NodeId idIn,
              std::shared_ptr<Sock> sock,
              const CAddress& addrIn,
@@ -4261,19 +4056,22 @@ CNode::CNode(NodeId idIn,
              const std::string& addrNameIn,
              ConnectionType conn_type_in,
              bool inbound_onion,
-             std::unique_ptr<i2p::sam::Session>&& i2p_sam_session)
+             CNodeOptions&& node_opts)
     : m_transport{std::make_unique<V1Transport>(idIn, SER_NETWORK, INIT_PROTO_VERSION)},
+      m_permission_flags{node_opts.permission_flags},
       m_sock{sock},
       m_connected{GetTime<std::chrono::seconds>()},
       addr{addrIn},
       addrBind{addrBindIn},
       m_addr_name{addrNameIn.empty() ? addr.ToStringAddrPort() : addrNameIn},
       m_inbound_onion{inbound_onion},
+      m_prefer_evict{node_opts.prefer_evict},
       nKeyedNetGroup{nKeyedNetGroupIn},
+      m_conn_type{conn_type_in},
       id{idIn},
       nLocalHostNonce{nLocalHostNonceIn},
-      m_conn_type{conn_type_in},
+      m_recv_flood_size{node_opts.recv_flood_size},
-      m_i2p_sam_session{std::move(i2p_sam_session)}
+      m_i2p_sam_session{std::move(node_opts.i2p_sam_session)}
 {
     if (inbound_onion) assert(conn_type_in == ConnectionType::INBOUND);
 
@@ -4288,6 +4086,37 @@ CNode::CNode(NodeId idIn,
     }
 }
 
+void CNode::MarkReceivedMsgsForProcessing()
+{
+    AssertLockNotHeld(m_msg_process_queue_mutex);
+
+    size_t nSizeAdded = 0;
+    for (const auto& msg : vRecvMsg) {
+        // vRecvMsg contains only completed CNetMessage
+        // the single possible partially deserialized message are held by TransportDeserializer
+        nSizeAdded += msg.m_raw_message_size;
+    }
+
+    LOCK(m_msg_process_queue_mutex);
+    m_msg_process_queue.splice(m_msg_process_queue.end(), vRecvMsg);
+    m_msg_process_queue_size += nSizeAdded;
+    fPauseRecv = m_msg_process_queue_size > m_recv_flood_size;
+}
+
+std::optional<std::pair<CNetMessage, bool>> CNode::PollMessage()
+{
+    LOCK(m_msg_process_queue_mutex);
+    if (m_msg_process_queue.empty()) return std::nullopt;
+
+    std::list<CNetMessage> msgs;
+    // Just take one message
+    msgs.splice(msgs.begin(), m_msg_process_queue, m_msg_process_queue.begin());
+    m_msg_process_queue_size -= msgs.front().m_raw_message_size;
+    fPauseRecv = m_msg_process_queue_size > m_recv_flood_size;
+
+    return std::make_pair(std::move(msgs.front()), !m_msg_process_queue.empty());
+}
+
 bool CConnman::NodeFullyConnected(const CNode* pnode)
 {
     return pnode && pnode->fSuccessfullyConnected && !pnode->fDisconnect;

src/net.h

@@ -19,6 +19,7 @@
 #include <netaddress.h>
 #include <netbase.h>
 #include <netgroup.h>
+#include <node/connection_types.h>
 #include <policy/feerate.h>
 #include <protocol.h>
 #include <random.h>
@@ -158,78 +159,6 @@ struct CSerializedNetMsg {
     size_t GetMemoryUsage() const noexcept;
 };
 
-/** Different types of connections to a peer. This enum encapsulates the
- * information we have available at the time of opening or accepting the
- * connection. Aside from INBOUND, all types are initiated by us.
- *
- * If adding or removing types, please update CONNECTION_TYPE_DOC in
- * src/rpc/net.cpp and src/qt/rpcconsole.cpp, as well as the descriptions in
- * src/qt/guiutil.cpp and src/bitcoin-cli.cpp::NetinfoRequestHandler. */
-enum class ConnectionType {
-    /**
-     * Inbound connections are those initiated by a peer. This is the only
-     * property we know at the time of connection, until P2P messages are
-     * exchanged.
-     */
-    INBOUND,
-
-    /**
-     * These are the default connections that we use to connect with the
-     * network. There is no restriction on what is relayed; by default we relay
-     * blocks, addresses & transactions. We automatically attempt to open
-     * MAX_OUTBOUND_FULL_RELAY_CONNECTIONS using addresses from our AddrMan.
-     */
-    OUTBOUND_FULL_RELAY,
-
-
-    /**
-     * We open manual connections to addresses that users explicitly requested
-     * via the addnode RPC or the -addnode/-connect configuration options. Even if a
-     * manual connection is misbehaving, we do not automatically disconnect or
-     * add it to our discouragement filter.
-     */
-    MANUAL,
-
-    /**
-     * Feeler connections are short-lived connections made to check that a node
-     * is alive. They can be useful for:
-     * - test-before-evict: if one of the peers is considered for eviction from
-     *   our AddrMan because another peer is mapped to the same slot in the tried table,
-     *   evict only if this longer-known peer is offline.
-     * - move node addresses from New to Tried table, so that we have more
-     *   connectable addresses in our AddrMan.
-     * Note that in the literature ("Eclipse Attacks on Bitcoin’s Peer-to-Peer Network")
-     * only the latter feature is referred to as "feeler connections",
-     * although in our codebase feeler connections encompass test-before-evict as well.
-     * We make these connections approximately every FEELER_INTERVAL:
-     * first we resolve previously found collisions if they exist (test-before-evict),
-     * otherwise we connect to a node from the new table.
-     */
-    FEELER,
-
-    /**
-     * We use block-relay-only connections to help prevent against partition
-     * attacks. By not relaying transactions or addresses, these connections
-     * are harder to detect by a third party, thus helping obfuscate the
-     * network topology. We automatically attempt to open
-     * MAX_BLOCK_RELAY_ONLY_ANCHORS using addresses from our anchors.dat. Then
-     * addresses from our AddrMan if MAX_BLOCK_RELAY_ONLY_CONNECTIONS
-     * isn't reached yet.
-     */
-    BLOCK_RELAY,
-
-    /**
-     * AddrFetch connections are short lived connections used to solicit
-     * addresses from peers. These are initiated to addresses submitted via the
-     * -seednode command line argument, or under certain conditions when the
-     * AddrMan is empty.
-     */
-    ADDR_FETCH,
-};
-
-/** Convert ConnectionType enum to a string value */
-std::string ConnectionTypeAsString(ConnectionType conn_type);
-
 /**
  * Look up IP addresses from all interfaces on the machine and add them to the
  * list of local addresses to self-advertise.
@@ -270,8 +199,8 @@ bool AddLocal(const CNetAddr& addr, int nScore = LOCAL_NONE);
 void RemoveLocal(const CService& addr);
 bool SeenLocal(const CService& addr);
 bool IsLocal(const CService& addr);
-bool GetLocal(CService &addr, const CNetAddr *paddrPeer = nullptr);
+bool GetLocal(CService& addr, const CNode& peer);
-CService GetLocalAddress(const CNetAddr& addrPeer);
+CService GetLocalAddress(const CNode& peer);
 
 
 extern bool fDiscover;
@@ -313,7 +242,7 @@ public:
     mapMsgTypeSize mapSendBytesPerMsgType;
     uint64_t nRecvBytes;
     mapMsgTypeSize mapRecvBytesPerMsgType;
-    NetPermissionFlags m_permissionFlags;
+    NetPermissionFlags m_permission_flags;
     bool m_legacyWhitelisted;
     std::chrono::microseconds m_last_ping_time;
     std::chrono::microseconds m_min_ping_time;
@@ -348,6 +277,14 @@ public:
     std::string m_type;
 
     CNetMessage(CDataStream&& recv_in) : m_recv(std::move(recv_in)) {}
+    // Only one CNetMessage object will exist for the same message on either
+    // the receive or processing queue. For performance reasons we therefore
+    // delete the copy constructor and assignment operator to avoid the
+    // possibility of copying CNetMessage objects.
+    CNetMessage(CNetMessage&&) = default;
+    CNetMessage(const CNetMessage&) = delete;
+    CNetMessage& operator=(CNetMessage&&) = default;
+    CNetMessage& operator=(const CNetMessage&) = delete;
 
     void SetVersion(int nVersionIn)
     {
@@ -520,18 +457,23 @@ public:
     size_t GetSendMemoryUsage() const noexcept override EXCLUSIVE_LOCKS_REQUIRED(!m_send_mutex);
 };
 
+struct CNodeOptions
+{
+    NetPermissionFlags permission_flags = NetPermissionFlags::None;
+    std::unique_ptr<i2p::sam::Session> i2p_sam_session = nullptr;
+    bool prefer_evict = false;
+    size_t recv_flood_size{DEFAULT_MAXRECEIVEBUFFER * 1000};
+};
+
 /** Information about a peer */
 class CNode
 {
-    friend class CConnman;
-    friend struct ConnmanTestMsg;
-
 public:
     /** Transport serializer/deserializer. The receive side functions are only called under cs_vRecv, while
      * the sending side functions are only called under cs_vSend. */
     const std::unique_ptr<Transport> m_transport;
 
-    NetPermissionFlags m_permissionFlags{NetPermissionFlags::None}; // treated as const outside of fuzz tester
+    const NetPermissionFlags m_permission_flags;
 
     /**
      * Socket used for communication with the node.
@@ -554,10 +496,6 @@ public:
     Mutex m_sock_mutex;
     Mutex cs_vRecv;
 
-    RecursiveMutex cs_vProcessMsg;
-    std::list<CNetMessage> vProcessMsg GUARDED_BY(cs_vProcessMsg);
-    size_t nProcessQueueSize GUARDED_BY(cs_vProcessMsg){0};
-
     uint64_t nRecvBytes GUARDED_BY(cs_vRecv){0};
 
     std::atomic<std::chrono::seconds> m_last_send{0s};
@@ -583,9 +521,9 @@ public:
      * from the wire. This cleaned string can safely be logged or displayed.
      */
     std::string cleanSubVer GUARDED_BY(m_subver_mutex){};
-    bool m_prefer_evict{false}; // This peer is preferred for eviction. (treated as const)
+    const bool m_prefer_evict{false}; // This peer is preferred for eviction.
     bool HasPermission(NetPermissionFlags permission) const {
-        return NetPermissions::HasFlag(m_permissionFlags, permission);
+        return NetPermissions::HasFlag(m_permission_flags, permission);
     }
     // This boolean is unusued in actual processing, only present for backward compatibility at RPC/QT level
     bool m_legacyWhitelisted{false};
@@ -617,6 +555,45 @@ public:
     std::atomic_bool fHasRecvData{false};
     std::atomic_bool fCanSendData{false};
 
+    const ConnectionType m_conn_type;
+
+    /** Move all messages from the received queue to the processing queue. */
+    void MarkReceivedMsgsForProcessing()
+        EXCLUSIVE_LOCKS_REQUIRED(!m_msg_process_queue_mutex);
+
+    /** Poll the next message from the processing queue of this connection.
+     *
+     * Returns std::nullopt if the processing queue is empty, or a pair
+     * consisting of the message and a bool that indicates if the processing
+     * queue has more entries. */
+    std::optional<std::pair<CNetMessage, bool>> PollMessage()
+        EXCLUSIVE_LOCKS_REQUIRED(!m_msg_process_queue_mutex);
+
+    /** Account for the total size of a sent message in the per msg type connection stats. */
+    void AccountForSentBytes(const std::string& msg_type, size_t sent_bytes)
+        EXCLUSIVE_LOCKS_REQUIRED(cs_vSend)
+    {
+        mapSendBytesPerMsgType[msg_type] += sent_bytes;
+    }
+
+    /** Update a supplied map with bytes sent for each msg type for this node */
+    void UpdateSentMapWithStats(mapMsgTypeSize& map_sentbytes_msg)
+        EXCLUSIVE_LOCKS_REQUIRED(cs_vSend)
+    {
+        for (auto& [msg_type, bytes] : mapSendBytesPerMsgType) {
+            map_sentbytes_msg[msg_type] += bytes;
+        }
+    }
+
+    /** Update a supplied map with bytes recv for each msg type for this node */
+    void UpdateRecvMapWithStats(mapMsgTypeSize& map_recvbytes_msg)
+        EXCLUSIVE_LOCKS_REQUIRED(cs_vRecv)
+    {
+        for (auto& [msg_type, bytes] : mapRecvBytesPerMsgType) {
+            map_recvbytes_msg[msg_type] += bytes;
+        }
+    }
+
     /**
      * Get network the peer connected through.
      *
@@ -628,6 +605,7 @@ public:
      * @return network the peer connected through.
      */
     Network ConnectedThroughNetwork() const;
+
     bool IsOutboundOrBlockRelayConn() const {
         switch (m_conn_type) {
             case ConnectionType::OUTBOUND_FULL_RELAY:
@@ -740,7 +718,7 @@ public:
           const std::string &addrNameIn,
           ConnectionType conn_type_in,
           bool inbound_onion,
-          std::unique_ptr<i2p::sam::Session>&& i2p_sam_session = nullptr);
+          CNodeOptions&& node_opts = {});
     CNode(const CNode&) = delete;
     CNode& operator=(const CNode&) = delete;
 
@@ -853,11 +831,15 @@ public:
 private:
     const NodeId id;
     const uint64_t nLocalHostNonce;
-    const ConnectionType m_conn_type;
     std::atomic<int> m_greatest_common_version{INIT_PROTO_VERSION};
 
+    const size_t m_recv_flood_size;
     std::list<CNetMessage> vRecvMsg; // Used only by SocketHandler thread
 
+    Mutex m_msg_process_queue_mutex;
+    std::list<CNetMessage> m_msg_process_queue GUARDED_BY(m_msg_process_queue_mutex);
+    size_t m_msg_process_queue_size GUARDED_BY(m_msg_process_queue_mutex){0};
+
     // Our address, as reported by the peer
     CService addrLocal GUARDED_BY(m_addr_local_mutex);
     mutable Mutex m_addr_local_mutex;
@@ -1281,8 +1263,6 @@ public:
     /** Get a unique deterministic randomizer. */
     CSipHasher GetDeterministicRandomizer(uint64_t id) const;
 
-    unsigned int GetReceiveFloodSize() const;
-
     void WakeMessageHandler() EXCLUSIVE_LOCKS_REQUIRED(!mutexMsgProc);
 
     /** Return true if we should disconnect the peer for failing an inactivity check. */
@@ -1346,12 +1326,12 @@ private:
      * Create a `CNode` object from a socket that has just been accepted and add the node to
      * the `m_nodes` member.
      * @param[in] sock Connected socket to communicate with the peer.
-     * @param[in] permissionFlags The peer's permissions.
+     * @param[in] permission_flags The peer's permissions.
      * @param[in] addr_bind The address and port at our side of the connection.
      * @param[in] addr The address and port at the peer's side of the connection.
      */
     void CreateNodeFromAcceptedSocket(std::unique_ptr<Sock>&& sock,
-                                      NetPermissionFlags permissionFlags,
+                                      NetPermissionFlags permission_flags,
                                       const CAddress& addr_bind,
                                       const CAddress& addr,
                                       CMasternodeSync& mn_sync) EXCLUSIVE_LOCKS_REQUIRED(!mutexMsgProc);
@@ -1711,62 +1691,10 @@ extern std::function<void(const CAddress& addr,
                           bool is_incoming)>
     CaptureMessage;
 
-struct NodeEvictionCandidate
-{
-    NodeId id;
-    std::chrono::seconds m_connected;
-    std::chrono::microseconds m_min_ping_time;
-    std::chrono::seconds m_last_block_time;
-    std::chrono::seconds m_last_tx_time;
-    bool fRelevantServices;
-    bool m_relay_txs;
-    bool fBloomFilter;
-    uint64_t nKeyedNetGroup;
-    bool prefer_evict;
-    bool m_is_local;
-    Network m_network;
-};
-
-/**
- * Select an inbound peer to evict after filtering out (protecting) peers having
- * distinct, difficult-to-forge characteristics. The protection logic picks out
- * fixed numbers of desirable peers per various criteria, followed by (mostly)
- * ratios of desirable or disadvantaged peers. If any eviction candidates
- * remain, the selection logic chooses a peer to evict.
- */
-[[nodiscard]] std::optional<NodeId> SelectNodeToEvict(std::vector<NodeEvictionCandidate>&& vEvictionCandidates);
-
 class CExplicitNetCleanup
 {
 public:
     static void callCleanup();
 };
 
-extern RecursiveMutex cs_main;
-
-/** Protect desirable or disadvantaged inbound peers from eviction by ratio.
- *
- * This function protects half of the peers which have been connected the
- * longest, to replicate the non-eviction implicit behavior and preclude attacks
- * that start later.
- *
- * Half of these protected spots (1/4 of the total) are reserved for the
- * following categories of peers, sorted by longest uptime, even if they're not
- * longest uptime overall:
- *
- * - onion peers connected via our tor control service
- *
- * - localhost peers, as manually configured hidden services not using
- *   `-bind=addr[:port]=onion` will not be detected as inbound onion connections
- *
- * - I2P peers
- *
- * - CJDNS peers
- *
- * This helps protect these privacy network peers, which tend to be otherwise
- * disadvantaged under our eviction criteria for their higher min ping times
- * relative to IPv4/IPv6 peers, and favorise the diversity of peer connections.
- */
-void ProtectEvictionCandidatesByRatio(std::vector<NodeEvictionCandidate>& vEvictionCandidates);
-
 #endif // BITCOIN_NET_H

src/net_processing.cpp

@@ -3460,7 +3460,7 @@ void PeerManagerImpl::ProcessMessage(
             // indicate to the peer that we will participate in addr relay.
             if (fListen && !m_chainman.ActiveChainstate().IsInitialBlockDownload())
             {
-                CAddress addr{GetLocalAddress(pfrom.addr), peer->m_our_services, (uint32_t)GetAdjustedTime()};
+                CAddress addr{GetLocalAddress(pfrom), peer->m_our_services, (uint32_t)GetAdjustedTime()};
                 FastRandomContext insecure_rand;
                 if (addr.IsRoutable())
                 {
@@ -5013,8 +5013,6 @@ bool PeerManagerImpl::ProcessMessages(CNode* pfrom, std::atomic<bool>& interrupt
 {
     AssertLockHeld(g_msgproc_mutex);
 
-    bool fMoreWork = false;
-
     PeerRef peer = GetPeerRef(pfrom->GetId());
     if (peer == nullptr) return false;
 
@@ -5050,17 +5048,14 @@ bool PeerManagerImpl::ProcessMessages(CNode* pfrom, std::atomic<bool>& interrupt
     // Don't bother if send buffer is too full to respond anyway
     if (pfrom->fPauseSend) return false;
 
-    std::list<CNetMessage> msgs;
+    auto poll_result{pfrom->PollMessage()};
-    {
+    if (!poll_result) {
-        LOCK(pfrom->cs_vProcessMsg);
+        // No message to process
-        if (pfrom->vProcessMsg.empty()) return false;
+        return false;
-        // Just take one message
-        msgs.splice(msgs.begin(), pfrom->vProcessMsg, pfrom->vProcessMsg.begin());
-        pfrom->nProcessQueueSize -= msgs.front().m_raw_message_size;
-        pfrom->fPauseRecv = pfrom->nProcessQueueSize > m_connman.GetReceiveFloodSize();
-        fMoreWork = !pfrom->vProcessMsg.empty();
     }
-    CNetMessage& msg(msgs.front());
+
+    CNetMessage& msg{poll_result->first};
+    bool fMoreWork = poll_result->second;
 
     TRACE6(net, inbound_message,
         pfrom->GetId(),

src/netaddress.cpp

@@ -742,19 +742,16 @@ uint64_t CNetAddr::GetHash() const
 
 // private extensions to enum Network, only returned by GetExtNetwork,
 // and only used in GetReachabilityFrom
-static const int NET_UNKNOWN = NET_MAX + 0;
+static const int NET_TEREDO = NET_MAX;
-static const int NET_TEREDO  = NET_MAX + 1;
+int static GetExtNetwork(const CNetAddr& addr)
-int static GetExtNetwork(const CNetAddr *addr)
 {
-    if (addr == nullptr)
+    if (addr.IsRFC4380())
-        return NET_UNKNOWN;
-    if (addr->IsRFC4380())
         return NET_TEREDO;
-    return addr->GetNetwork();
+    return addr.GetNetwork();
 }
 
 /** Calculates a metric for how reachable (*this) is from a given partner */
-int CNetAddr::GetReachabilityFrom(const CNetAddr *paddrPartner) const
+int CNetAddr::GetReachabilityFrom(const CNetAddr& paddrPartner) const
 {
     enum Reachability {
         REACH_UNREACHABLE,
@@ -769,7 +766,7 @@ int CNetAddr::GetReachabilityFrom(const CNetAddr *paddrPartner) const
     if (!IsRoutable() || IsInternal())
         return REACH_UNREACHABLE;
 
-    int ourNet = GetExtNetwork(this);
+    int ourNet = GetExtNetwork(*this);
     int theirNet = GetExtNetwork(paddrPartner);
     bool fTunnel = IsRFC3964() || IsRFC6052() || IsRFC6145();
 
@@ -809,7 +806,6 @@ int CNetAddr::GetReachabilityFrom(const CNetAddr *paddrPartner) const
         case NET_IPV6:    return REACH_IPV6_WEAK;
         case NET_IPV4:    return REACH_IPV4;
         }
-    case NET_UNKNOWN:
     case NET_UNROUTABLE:
     default:
         switch(ourNet) {

src/netaddress.h

@@ -211,7 +211,7 @@ public:
     bool HasLinkedIPv4() const;
 
     std::vector<unsigned char> GetAddrBytes() const;
-    int GetReachabilityFrom(const CNetAddr *paddrPartner = nullptr) const;
+    int GetReachabilityFrom(const CNetAddr& paddrPartner) const;
 
     explicit CNetAddr(const struct in6_addr& pipv6Addr, const uint32_t scope = 0);
     bool GetIn6Addr(struct in6_addr* pipv6Addr) const;

src/node/connection_types.cpp

@@ -0,0 +1,26 @@
+// Copyright (c) 2022 The Bitcoin Core developers
+// Distributed under the MIT software license, see the accompanying
+// file COPYING or http://www.opensource.org/licenses/mit-license.php.
+
+#include <node/connection_types.h>
+#include <cassert>
+
+std::string ConnectionTypeAsString(ConnectionType conn_type)
+{
+    switch (conn_type) {
+    case ConnectionType::INBOUND:
+        return "inbound";
+    case ConnectionType::MANUAL:
+        return "manual";
+    case ConnectionType::FEELER:
+        return "feeler";
+    case ConnectionType::OUTBOUND_FULL_RELAY:
+        return "outbound-full-relay";
+    case ConnectionType::BLOCK_RELAY:
+        return "block-relay-only";
+    case ConnectionType::ADDR_FETCH:
+        return "addr-fetch";
+    } // no default case, so the compiler can warn about missing cases
+
+    assert(false);
+}

src/node/connection_types.h

@@ -0,0 +1,82 @@
+// Copyright (c) 2022 The Bitcoin Core developers
+// Distributed under the MIT software license, see the accompanying
+// file COPYING or http://www.opensource.org/licenses/mit-license.php.
+
+#ifndef BITCOIN_NODE_CONNECTION_TYPES_H
+#define BITCOIN_NODE_CONNECTION_TYPES_H
+
+#include <string>
+
+/** Different types of connections to a peer. This enum encapsulates the
+ * information we have available at the time of opening or accepting the
+ * connection. Aside from INBOUND, all types are initiated by us.
+ *
+ * If adding or removing types, please update CONNECTION_TYPE_DOC in
+ * src/rpc/net.cpp and src/qt/rpcconsole.cpp, as well as the descriptions in
+ * src/qt/guiutil.cpp and src/bitcoin-cli.cpp::NetinfoRequestHandler. */
+enum class ConnectionType {
+    /**
+     * Inbound connections are those initiated by a peer. This is the only
+     * property we know at the time of connection, until P2P messages are
+     * exchanged.
+     */
+    INBOUND,
+
+    /**
+     * These are the default connections that we use to connect with the
+     * network. There is no restriction on what is relayed; by default we relay
+     * blocks, addresses & transactions. We automatically attempt to open
+     * MAX_OUTBOUND_FULL_RELAY_CONNECTIONS using addresses from our AddrMan.
+     */
+    OUTBOUND_FULL_RELAY,
+
+
+    /**
+     * We open manual connections to addresses that users explicitly requested
+     * via the addnode RPC or the -addnode/-connect configuration options. Even if a
+     * manual connection is misbehaving, we do not automatically disconnect or
+     * add it to our discouragement filter.
+     */
+    MANUAL,
+
+    /**
+     * Feeler connections are short-lived connections made to check that a node
+     * is alive. They can be useful for:
+     * - test-before-evict: if one of the peers is considered for eviction from
+     *   our AddrMan because another peer is mapped to the same slot in the tried table,
+     *   evict only if this longer-known peer is offline.
+     * - move node addresses from New to Tried table, so that we have more
+     *   connectable addresses in our AddrMan.
+     * Note that in the literature ("Eclipse Attacks on Bitcoin’s Peer-to-Peer Network")
+     * only the latter feature is referred to as "feeler connections",
+     * although in our codebase feeler connections encompass test-before-evict as well.
+     * We make these connections approximately every FEELER_INTERVAL:
+     * first we resolve previously found collisions if they exist (test-before-evict),
+     * otherwise we connect to a node from the new table.
+     */
+    FEELER,
+
+    /**
+     * We use block-relay-only connections to help prevent against partition
+     * attacks. By not relaying transactions or addresses, these connections
+     * are harder to detect by a third party, thus helping obfuscate the
+     * network topology. We automatically attempt to open
+     * MAX_BLOCK_RELAY_ONLY_ANCHORS using addresses from our anchors.dat. Then
+     * addresses from our AddrMan if MAX_BLOCK_RELAY_ONLY_CONNECTIONS
+     * isn't reached yet.
+     */
+    BLOCK_RELAY,
+
+    /**
+     * AddrFetch connections are short lived connections used to solicit
+     * addresses from peers. These are initiated to addresses submitted via the
+     * -seednode command line argument, or under certain conditions when the
+     * AddrMan is empty.
+     */
+    ADDR_FETCH,
+};
+
+/** Convert ConnectionType enum to a string value */
+std::string ConnectionTypeAsString(ConnectionType conn_type);
+
+#endif // BITCOIN_NODE_CONNECTION_TYPES_H

src/node/eviction.cpp

@@ -0,0 +1,240 @@
+// Copyright (c) 2022 The Bitcoin Core developers
+// Distributed under the MIT software license, see the accompanying
+// file COPYING or http://www.opensource.org/licenses/mit-license.php.
+
+#include <node/eviction.h>
+
+#include <algorithm>
+#include <array>
+#include <chrono>
+#include <cstdint>
+#include <functional>
+#include <map>
+#include <vector>
+
+
+static bool ReverseCompareNodeMinPingTime(const NodeEvictionCandidate &a, const NodeEvictionCandidate &b)
+{
+    return a.m_min_ping_time > b.m_min_ping_time;
+}
+
+static bool ReverseCompareNodeTimeConnected(const NodeEvictionCandidate &a, const NodeEvictionCandidate &b)
+{
+    return a.m_connected > b.m_connected;
+}
+
+static bool CompareNetGroupKeyed(const NodeEvictionCandidate &a, const NodeEvictionCandidate &b) {
+    return a.nKeyedNetGroup < b.nKeyedNetGroup;
+}
+
+static bool CompareNodeBlockTime(const NodeEvictionCandidate &a, const NodeEvictionCandidate &b)
+{
+    // There is a fall-through here because it is common for a node to have many peers which have not yet relayed a block.
+    if (a.m_last_block_time != b.m_last_block_time) return a.m_last_block_time < b.m_last_block_time;
+    if (a.fRelevantServices != b.fRelevantServices) return b.fRelevantServices;
+    return a.m_connected > b.m_connected;
+}
+
+static bool CompareNodeTXTime(const NodeEvictionCandidate &a, const NodeEvictionCandidate &b)
+{
+    // There is a fall-through here because it is common for a node to have more than a few peers that have not yet relayed txn.
+    if (a.m_last_tx_time != b.m_last_tx_time) return a.m_last_tx_time < b.m_last_tx_time;
+    if (a.m_relay_txs != b.m_relay_txs) return b.m_relay_txs;
+    if (a.fBloomFilter != b.fBloomFilter) return a.fBloomFilter;
+    return a.m_connected > b.m_connected;
+}
+
+// Pick out the potential block-relay only peers, and sort them by last block time.
+static bool CompareNodeBlockRelayOnlyTime(const NodeEvictionCandidate &a, const NodeEvictionCandidate &b)
+{
+    if (a.m_relay_txs != b.m_relay_txs) return a.m_relay_txs;
+    if (a.m_last_block_time != b.m_last_block_time) return a.m_last_block_time < b.m_last_block_time;
+    if (a.fRelevantServices != b.fRelevantServices) return b.fRelevantServices;
+    return a.m_connected > b.m_connected;
+}
+
+/**
+ * Sort eviction candidates by network/localhost and connection uptime.
+ * Candidates near the beginning are more likely to be evicted, and those
+ * near the end are more likely to be protected, e.g. less likely to be evicted.
+ * - First, nodes that are not `is_local` and that do not belong to `network`,
+ *   sorted by increasing uptime (from most recently connected to connected longer).
+ * - Then, nodes that are `is_local` or belong to `network`, sorted by increasing uptime.
+ */
+struct CompareNodeNetworkTime {
+    const bool m_is_local;
+    const Network m_network;
+    CompareNodeNetworkTime(bool is_local, Network network) : m_is_local(is_local), m_network(network) {}
+    bool operator()(const NodeEvictionCandidate& a, const NodeEvictionCandidate& b) const
+    {
+        if (m_is_local && a.m_is_local != b.m_is_local) return b.m_is_local;
+        if ((a.m_network == m_network) != (b.m_network == m_network)) return b.m_network == m_network;
+        return a.m_connected > b.m_connected;
+    };
+};
+
+//! Sort an array by the specified comparator, then erase the last K elements where predicate is true.
+template <typename T, typename Comparator>
+static void EraseLastKElements(
+    std::vector<T>& elements, Comparator comparator, size_t k,
+    std::function<bool(const NodeEvictionCandidate&)> predicate = [](const NodeEvictionCandidate& n) { return true; })
+{
+    std::sort(elements.begin(), elements.end(), comparator);
+    size_t eraseSize = std::min(k, elements.size());
+    elements.erase(std::remove_if(elements.end() - eraseSize, elements.end(), predicate), elements.end());
+}
+
+void ProtectNoBanConnections(std::vector<NodeEvictionCandidate>& eviction_candidates)
+{
+    eviction_candidates.erase(std::remove_if(eviction_candidates.begin(), eviction_candidates.end(),
+                                             [](NodeEvictionCandidate const& n) {
+                                                 return n.m_noban;
+                                             }),
+                              eviction_candidates.end());
+}
+
+void ProtectOutboundConnections(std::vector<NodeEvictionCandidate>& eviction_candidates)
+{
+    eviction_candidates.erase(std::remove_if(eviction_candidates.begin(), eviction_candidates.end(),
+                                             [](NodeEvictionCandidate const& n) {
+                                                 return n.m_conn_type != ConnectionType::INBOUND;
+                                             }),
+                              eviction_candidates.end());
+}
+
+void ProtectEvictionCandidatesByRatio(std::vector<NodeEvictionCandidate>& eviction_candidates)
+{
+    // Protect the half of the remaining nodes which have been connected the longest.
+    // This replicates the non-eviction implicit behavior, and precludes attacks that start later.
+    // To favorise the diversity of our peer connections, reserve up to half of these protected
+    // spots for Tor/onion, localhost, I2P, and CJDNS peers, even if they're not longest uptime
+    // overall. This helps protect these higher-latency peers that tend to be otherwise
+    // disadvantaged under our eviction criteria.
+    const size_t initial_size = eviction_candidates.size();
+    const size_t total_protect_size{initial_size / 2};
+
+    // Disadvantaged networks to protect. In the case of equal counts, earlier array members
+    // have the first opportunity to recover unused slots from the previous iteration.
+    struct Net { bool is_local; Network id; size_t count; };
+    std::array<Net, 4> networks{
+        {{false, NET_CJDNS, 0}, {false, NET_I2P, 0}, {/*localhost=*/true, NET_MAX, 0}, {false, NET_ONION, 0}}};
+
+    // Count and store the number of eviction candidates per network.
+    for (Net& n : networks) {
+        n.count = std::count_if(eviction_candidates.cbegin(), eviction_candidates.cend(),
+                                [&n](const NodeEvictionCandidate& c) {
+                                    return n.is_local ? c.m_is_local : c.m_network == n.id;
+                                });
+    }
+    // Sort `networks` by ascending candidate count, to give networks having fewer candidates
+    // the first opportunity to recover unused protected slots from the previous iteration.
+    std::stable_sort(networks.begin(), networks.end(), [](Net a, Net b) { return a.count < b.count; });
+
+    // Protect up to 25% of the eviction candidates by disadvantaged network.
+    const size_t max_protect_by_network{total_protect_size / 2};
+    size_t num_protected{0};
+
+    while (num_protected < max_protect_by_network) {
+        // Count the number of disadvantaged networks from which we have peers to protect.
+        auto num_networks = std::count_if(networks.begin(), networks.end(), [](const Net& n) { return n.count; });
+        if (num_networks == 0) {
+            break;
+        }
+        const size_t disadvantaged_to_protect{max_protect_by_network - num_protected};
+        const size_t protect_per_network{std::max(disadvantaged_to_protect / num_networks, static_cast<size_t>(1))};
+        // Early exit flag if there are no remaining candidates by disadvantaged network.
+        bool protected_at_least_one{false};
+
+        for (Net& n : networks) {
+            if (n.count == 0) continue;
+            const size_t before = eviction_candidates.size();
+            EraseLastKElements(eviction_candidates, CompareNodeNetworkTime(n.is_local, n.id),
+                               protect_per_network, [&n](const NodeEvictionCandidate& c) {
+                                   return n.is_local ? c.m_is_local : c.m_network == n.id;
+                               });
+            const size_t after = eviction_candidates.size();
+            if (before > after) {
+                protected_at_least_one = true;
+                const size_t delta{before - after};
+                num_protected += delta;
+                if (num_protected >= max_protect_by_network) {
+                    break;
+                }
+                n.count -= delta;
+            }
+        }
+        if (!protected_at_least_one) {
+            break;
+        }
+    }
+
+    // Calculate how many we removed, and update our total number of peers that
+    // we want to protect based on uptime accordingly.
+    assert(num_protected == initial_size - eviction_candidates.size());
+    const size_t remaining_to_protect{total_protect_size - num_protected};
+    EraseLastKElements(eviction_candidates, ReverseCompareNodeTimeConnected, remaining_to_protect);
+}
+
+[[nodiscard]] std::optional<NodeId> SelectNodeToEvict(std::vector<NodeEvictionCandidate>&& vEvictionCandidates)
+{
+    // Protect connections with certain characteristics
+
+    ProtectNoBanConnections(vEvictionCandidates);
+
+    ProtectOutboundConnections(vEvictionCandidates);
+
+    // Deterministically select 4 peers to protect by netgroup.
+    // An attacker cannot predict which netgroups will be protected
+    EraseLastKElements(vEvictionCandidates, CompareNetGroupKeyed, 4);
+    // Protect the 8 nodes with the lowest minimum ping time.
+    // An attacker cannot manipulate this metric without physically moving nodes closer to the target.
+    EraseLastKElements(vEvictionCandidates, ReverseCompareNodeMinPingTime, 8);
+    // Protect 4 nodes that most recently sent us novel transactions accepted into our mempool.
+    // An attacker cannot manipulate this metric without performing useful work.
+    EraseLastKElements(vEvictionCandidates, CompareNodeTXTime, 4);
+    // Protect up to 8 non-tx-relay peers that have sent us novel blocks.
+    EraseLastKElements(vEvictionCandidates, CompareNodeBlockRelayOnlyTime, 8,
+                       [](const NodeEvictionCandidate& n) { return !n.m_relay_txs && n.fRelevantServices; });
+
+    // Protect 4 nodes that most recently sent us novel blocks.
+    // An attacker cannot manipulate this metric without performing useful work.
+    EraseLastKElements(vEvictionCandidates, CompareNodeBlockTime, 4);
+
+    // Protect some of the remaining eviction candidates by ratios of desirable
+    // or disadvantaged characteristics.
+    ProtectEvictionCandidatesByRatio(vEvictionCandidates);
+
+    if (vEvictionCandidates.empty()) return std::nullopt;
+
+    // If any remaining peers are preferred for eviction consider only them.
+    // This happens after the other preferences since if a peer is really the best by other criteria (esp relaying blocks)
+    //  then we probably don't want to evict it no matter what.
+    if (std::any_of(vEvictionCandidates.begin(),vEvictionCandidates.end(),[](NodeEvictionCandidate const &n){return n.prefer_evict;})) {
+        vEvictionCandidates.erase(std::remove_if(vEvictionCandidates.begin(),vEvictionCandidates.end(),
+                                  [](NodeEvictionCandidate const &n){return !n.prefer_evict;}),vEvictionCandidates.end());
+    }
+
+    // Identify the network group with the most connections and youngest member.
+    // (vEvictionCandidates is already sorted by reverse connect time)
+    uint64_t naMostConnections;
+    unsigned int nMostConnections = 0;
+    std::chrono::seconds nMostConnectionsTime{0};
+    std::map<uint64_t, std::vector<NodeEvictionCandidate> > mapNetGroupNodes;
+    for (const NodeEvictionCandidate &node : vEvictionCandidates) {
+        std::vector<NodeEvictionCandidate> &group = mapNetGroupNodes[node.nKeyedNetGroup];
+        group.push_back(node);
+        const auto grouptime{group[0].m_connected};
+
+        if (group.size() > nMostConnections || (group.size() == nMostConnections && grouptime > nMostConnectionsTime)) {
+            nMostConnections = group.size();
+            nMostConnectionsTime = grouptime;
+            naMostConnections = node.nKeyedNetGroup;
+        }
+    }
+
+    // Reduce to the network group with the most connections
+    vEvictionCandidates = std::move(mapNetGroupNodes[naMostConnections]);
+
+    // Disconnect from the network group with the most connections
+    return vEvictionCandidates.front().id;
+}

src/node/eviction.h

@@ -0,0 +1,69 @@
+// Copyright (c) 2022 The Bitcoin Core developers
+// Distributed under the MIT software license, see the accompanying
+// file COPYING or http://www.opensource.org/licenses/mit-license.php.
+
+#ifndef BITCOIN_NODE_EVICTION_H
+#define BITCOIN_NODE_EVICTION_H
+
+#include <node/connection_types.h>
+#include <net_permissions.h>
+
+#include <chrono>
+#include <cstdint>
+#include <optional>
+#include <vector>
+
+typedef int64_t NodeId;
+
+struct NodeEvictionCandidate {
+    NodeId id;
+    std::chrono::seconds m_connected;
+    std::chrono::microseconds m_min_ping_time;
+    std::chrono::seconds m_last_block_time;
+    std::chrono::seconds m_last_tx_time;
+    bool fRelevantServices;
+    bool m_relay_txs;
+    bool fBloomFilter;
+    uint64_t nKeyedNetGroup;
+    bool prefer_evict;
+    bool m_is_local;
+    Network m_network;
+    bool m_noban;
+    ConnectionType m_conn_type;
+};
+
+/**
+ * Select an inbound peer to evict after filtering out (protecting) peers having
+ * distinct, difficult-to-forge characteristics. The protection logic picks out
+ * fixed numbers of desirable peers per various criteria, followed by (mostly)
+ * ratios of desirable or disadvantaged peers. If any eviction candidates
+ * remain, the selection logic chooses a peer to evict.
+ */
+[[nodiscard]] std::optional<NodeId> SelectNodeToEvict(std::vector<NodeEvictionCandidate>&& vEvictionCandidates);
+
+/** Protect desirable or disadvantaged inbound peers from eviction by ratio.
+ *
+ * This function protects half of the peers which have been connected the
+ * longest, to replicate the non-eviction implicit behavior and preclude attacks
+ * that start later.
+ *
+ * Half of these protected spots (1/4 of the total) are reserved for the
+ * following categories of peers, sorted by longest uptime, even if they're not
+ * longest uptime overall:
+ *
+ * - onion peers connected via our tor control service
+ *
+ * - localhost peers, as manually configured hidden services not using
+ *   `-bind=addr[:port]=onion` will not be detected as inbound onion connections
+ *
+ * - I2P peers
+ *
+ * - CJDNS peers
+ *
+ * This helps protect these privacy network peers, which tend to be otherwise
+ * disadvantaged under our eviction criteria for their higher min ping times
+ * relative to IPv4/IPv6 peers, and favorise the diversity of peer connections.
+ */
+void ProtectEvictionCandidatesByRatio(std::vector<NodeEvictionCandidate>& vEvictionCandidates);
+
+#endif // BITCOIN_NODE_EVICTION_H

src/qt/rpcconsole.cpp

@@ -1234,11 +1234,11 @@ void RPCConsole::updateDetailWidget()
     ui->peerSubversion->setText(QString::fromStdString(stats->nodeStats.cleanSubVer));
     ui->peerConnectionType->setText(GUIUtil::ConnectionTypeToQString(stats->nodeStats.m_conn_type, /* prepend_direction */ true));
     ui->peerNetwork->setText(GUIUtil::NetworkToQString(stats->nodeStats.m_network));
-    if (stats->nodeStats.m_permissionFlags == NetPermissionFlags::None) {
+    if (stats->nodeStats.m_permission_flags == NetPermissionFlags::None) {
         ui->peerPermissions->setText(ts.na);
     } else {
         QStringList permissions;
-        for (const auto& permission : NetPermissions::ToStrings(stats->nodeStats.m_permissionFlags)) {
+        for (const auto& permission : NetPermissions::ToStrings(stats->nodeStats.m_permission_flags)) {
             permissions.append(QString::fromStdString(permission));
         }
         ui->peerPermissions->setText(permissions.join(" & "));

src/rpc/net.cpp

@@ -260,7 +260,7 @@ static RPCHelpMan getpeerinfo()
             obj.pushKV("whitelisted", stats.m_legacyWhitelisted);
         }
         UniValue permissions(UniValue::VARR);
-        for (const auto& permission : NetPermissions::ToStrings(stats.m_permissionFlags)) {
+        for (const auto& permission : NetPermissions::ToStrings(stats.m_permission_flags)) {
             permissions.push_back(permission);
         }
         obj.pushKV("permissions", permissions);

src/test/denialofservice_tests.cpp

@@ -71,7 +71,6 @@ BOOST_AUTO_TEST_CASE(outbound_slow_chain_eviction)
         /*successfully_connected=*/true,
         /*remote_services=*/ServiceFlags(NODE_NETWORK),
         /*local_services=*/ServiceFlags(NODE_NETWORK),
-        /*permission_flags=*/NetPermissionFlags::None,
         /*version=*/PROTOCOL_VERSION,
         /*relay_txs=*/true);
     TestOnlyResetTimeData();

src/test/fuzz/connman.cpp

@@ -126,7 +126,6 @@ FUZZ_TARGET_INIT(connman, initialize_connman)
     std::vector<CNodeStats> stats;
     connman.GetNodeStats(stats);
     (void)connman.GetOutboundTargetBytesLeft();
-    (void)connman.GetReceiveFloodSize();
     (void)connman.GetTotalBytesRecv();
     (void)connman.GetTotalBytesSent();
     (void)connman.GetTryNewOutboundPeer();

src/test/fuzz/netaddress.cpp

@@ -83,7 +83,7 @@ FUZZ_TARGET(netaddress)
     (void)service.ToStringAddrPort();
 
     const CNetAddr other_net_addr = ConsumeNetAddr(fuzzed_data_provider);
-    (void)net_addr.GetReachabilityFrom(&other_net_addr);
+    (void)net_addr.GetReachabilityFrom(other_net_addr);
     (void)sub_net.Match(other_net_addr);
 
     const CService other_service{net_addr, fuzzed_data_provider.ConsumeIntegral<uint16_t>()};

src/test/fuzz/node_eviction.cpp

@@ -32,6 +32,8 @@ FUZZ_TARGET(node_eviction)
             /* prefer_evict */ fuzzed_data_provider.ConsumeBool(),
             /* m_is_local */ fuzzed_data_provider.ConsumeBool(),
             /* m_network */ fuzzed_data_provider.PickValueInArray(ALL_NETWORKS),
+            /* m_noban */ fuzzed_data_provider.ConsumeBool(),
+            /* m_conn_type */ fuzzed_data_provider.PickValueInArray(ALL_CONNECTION_TYPES),
         });
     }
     // Make a copy since eviction_candidates may be in some valid but otherwise

src/test/fuzz/util.cpp

@@ -288,7 +288,6 @@ void FillNode(FuzzedDataProvider& fuzzed_data_provider, ConnmanTestMsg& connman,
                       /*successfully_connected=*/fuzzed_data_provider.ConsumeBool(),
                       /*remote_services=*/ConsumeWeakEnum(fuzzed_data_provider, ALL_SERVICE_FLAGS),
                       /*local_services=*/ConsumeWeakEnum(fuzzed_data_provider, ALL_SERVICE_FLAGS),
-                      /*permission_flags=*/ConsumeWeakEnum(fuzzed_data_provider, ALL_NET_PERMISSION_FLAGS),
                       /*version=*/fuzzed_data_provider.ConsumeIntegralInRange<int32_t>(MIN_PEER_PROTO_VERSION, std::numeric_limits<int32_t>::max()),
                       /*relay_txs=*/fuzzed_data_provider.ConsumeBool());
 }

src/test/fuzz/util.h

@@ -366,6 +366,7 @@ auto ConsumeNode(FuzzedDataProvider& fuzzed_data_provider, const std::optional<N
 
     const ConnectionType conn_type = fuzzed_data_provider.PickValueInArray(ALL_CONNECTION_TYPES);
     const bool inbound_onion{conn_type == ConnectionType::INBOUND ? fuzzed_data_provider.ConsumeBool() : false};
+    NetPermissionFlags permission_flags = ConsumeWeakEnum(fuzzed_data_provider, ALL_NET_PERMISSION_FLAGS);
     if constexpr (ReturnUniquePtr) {
         return std::make_unique<CNode>(node_id,
                                        sock,
@@ -375,7 +376,8 @@ auto ConsumeNode(FuzzedDataProvider& fuzzed_data_provider, const std::optional<N
                                        addr_bind,
                                        addr_name,
                                        conn_type,
-                                       inbound_onion);
+                                       inbound_onion,
+                                       CNodeOptions{ .permission_flags = permission_flags });
     } else {
         return CNode{node_id,
                      sock,
@@ -385,7 +387,8 @@ auto ConsumeNode(FuzzedDataProvider& fuzzed_data_provider, const std::optional<N
                      addr_bind,
                      addr_name,
                      conn_type,
-                     inbound_onion};
+                     inbound_onion,
+                     CNodeOptions{ .permission_flags = permission_flags }};
     }
 }
 inline std::unique_ptr<CNode> ConsumeNodeAsUniquePtr(FuzzedDataProvider& fdp, const std::optional<NodeId>& node_id_in = std::nullopt) { return ConsumeNode<true>(fdp, node_id_in); }

src/test/net_tests.cpp

@@ -908,4 +908,109 @@ BOOST_AUTO_TEST_CASE(initial_advertise_from_version_message)
     TestOnlyResetTimeData();
 }
 
+
+BOOST_AUTO_TEST_CASE(advertise_local_address)
+{
+    auto CreatePeer = [](const CAddress& addr) {
+        return std::make_unique<CNode>(/*id=*/0,
+                                       /*sock=*/nullptr,
+                                       addr,
+                                       /*nKeyedNetGroupIn=*/0,
+                                       /*nLocalHostNonceIn=*/0,
+                                       CAddress{},
+                                       /*pszDest=*/std::string{},
+                                       ConnectionType::OUTBOUND_FULL_RELAY,
+                                       /*inbound_onion=*/false);
+    };
+    SetReachable(NET_CJDNS, true);
+
+    CAddress addr_ipv4{Lookup("1.2.3.4", 8333, false).value(), NODE_NONE};
+    BOOST_REQUIRE(addr_ipv4.IsValid());
+    BOOST_REQUIRE(addr_ipv4.IsIPv4());
+
+    CAddress addr_ipv6{Lookup("1122:3344:5566:7788:9900:aabb:ccdd:eeff", 8333, false).value(), NODE_NONE};
+    BOOST_REQUIRE(addr_ipv6.IsValid());
+    BOOST_REQUIRE(addr_ipv6.IsIPv6());
+
+    CAddress addr_ipv6_tunnel{Lookup("2002:3344:5566:7788:9900:aabb:ccdd:eeff", 8333, false).value(), NODE_NONE};
+    BOOST_REQUIRE(addr_ipv6_tunnel.IsValid());
+    BOOST_REQUIRE(addr_ipv6_tunnel.IsIPv6());
+    BOOST_REQUIRE(addr_ipv6_tunnel.IsRFC3964());
+
+    CAddress addr_teredo{Lookup("2001:0000:5566:7788:9900:aabb:ccdd:eeff", 8333, false).value(), NODE_NONE};
+    BOOST_REQUIRE(addr_teredo.IsValid());
+    BOOST_REQUIRE(addr_teredo.IsIPv6());
+    BOOST_REQUIRE(addr_teredo.IsRFC4380());
+
+    CAddress addr_onion;
+    BOOST_REQUIRE(addr_onion.SetSpecial("pg6mmjiyjmcrsslvykfwnntlaru7p5svn6y2ymmju6nubxndf4pscryd.onion"));
+    BOOST_REQUIRE(addr_onion.IsValid());
+    BOOST_REQUIRE(addr_onion.IsTor());
+
+    CAddress addr_i2p;
+    BOOST_REQUIRE(addr_i2p.SetSpecial("udhdrtrcetjm5sxzskjyr5ztpeszydbh4dpl3pl4utgqqw2v4jna.b32.i2p"));
+    BOOST_REQUIRE(addr_i2p.IsValid());
+    BOOST_REQUIRE(addr_i2p.IsI2P());
+
+    CService service_cjdns{Lookup("fc00:3344:5566:7788:9900:aabb:ccdd:eeff", 8333, false).value(), NODE_NONE};
+    CAddress addr_cjdns{MaybeFlipIPv6toCJDNS(service_cjdns), NODE_NONE};
+    BOOST_REQUIRE(addr_cjdns.IsValid());
+    BOOST_REQUIRE(addr_cjdns.IsCJDNS());
+
+    const auto peer_ipv4{CreatePeer(addr_ipv4)};
+    const auto peer_ipv6{CreatePeer(addr_ipv6)};
+    const auto peer_ipv6_tunnel{CreatePeer(addr_ipv6_tunnel)};
+    const auto peer_teredo{CreatePeer(addr_teredo)};
+    const auto peer_onion{CreatePeer(addr_onion)};
+    const auto peer_i2p{CreatePeer(addr_i2p)};
+    const auto peer_cjdns{CreatePeer(addr_cjdns)};
+
+    // one local clearnet address - advertise to all but privacy peers
+    AddLocal(addr_ipv4);
+    BOOST_CHECK(GetLocalAddress(*peer_ipv4) == addr_ipv4);
+    BOOST_CHECK(GetLocalAddress(*peer_ipv6) == addr_ipv4);
+    BOOST_CHECK(GetLocalAddress(*peer_ipv6_tunnel) == addr_ipv4);
+    BOOST_CHECK(GetLocalAddress(*peer_teredo) == addr_ipv4);
+    BOOST_CHECK(GetLocalAddress(*peer_cjdns) == addr_ipv4);
+    BOOST_CHECK(!GetLocalAddress(*peer_onion).IsValid());
+    BOOST_CHECK(!GetLocalAddress(*peer_i2p).IsValid());
+    RemoveLocal(addr_ipv4);
+
+    // local privacy addresses - don't advertise to clearnet peers
+    AddLocal(addr_onion);
+    AddLocal(addr_i2p);
+    BOOST_CHECK(!GetLocalAddress(*peer_ipv4).IsValid());
+    BOOST_CHECK(!GetLocalAddress(*peer_ipv6).IsValid());
+    BOOST_CHECK(!GetLocalAddress(*peer_ipv6_tunnel).IsValid());
+    BOOST_CHECK(!GetLocalAddress(*peer_teredo).IsValid());
+    BOOST_CHECK(!GetLocalAddress(*peer_cjdns).IsValid());
+    BOOST_CHECK(GetLocalAddress(*peer_onion) == addr_onion);
+    BOOST_CHECK(GetLocalAddress(*peer_i2p) == addr_i2p);
+    RemoveLocal(addr_onion);
+    RemoveLocal(addr_i2p);
+
+    // local addresses from all networks
+    AddLocal(addr_ipv4);
+    AddLocal(addr_ipv6);
+    AddLocal(addr_ipv6_tunnel);
+    AddLocal(addr_teredo);
+    AddLocal(addr_onion);
+    AddLocal(addr_i2p);
+    AddLocal(addr_cjdns);
+    BOOST_CHECK(GetLocalAddress(*peer_ipv4) == addr_ipv4);
+    BOOST_CHECK(GetLocalAddress(*peer_ipv6) == addr_ipv6);
+    BOOST_CHECK(GetLocalAddress(*peer_ipv6_tunnel) == addr_ipv6);
+    BOOST_CHECK(GetLocalAddress(*peer_teredo) == addr_ipv4);
+    BOOST_CHECK(GetLocalAddress(*peer_onion) == addr_onion);
+    BOOST_CHECK(GetLocalAddress(*peer_i2p) == addr_i2p);
+    BOOST_CHECK(GetLocalAddress(*peer_cjdns) == addr_cjdns);
+    RemoveLocal(addr_ipv4);
+    RemoveLocal(addr_ipv6);
+    RemoveLocal(addr_ipv6_tunnel);
+    RemoveLocal(addr_teredo);
+    RemoveLocal(addr_onion);
+    RemoveLocal(addr_i2p);
+    RemoveLocal(addr_cjdns);
+}
+
 BOOST_AUTO_TEST_SUITE_END()

src/test/util/net.cpp

@@ -5,6 +5,7 @@
 #include <test/util/net.h>
 
 #include <chainparams.h>
+#include <node/eviction.h>
 #include <net.h>
 #include <net_processing.h>
 #include <netmessagemaker.h>
@@ -16,7 +17,6 @@ void ConnmanTestMsg::Handshake(CNode& node,
                                bool successfully_connected,
                                ServiceFlags remote_services,
                                ServiceFlags local_services,
-                               NetPermissionFlags permission_flags,
                                int32_t version,
                                bool relay_txs)
 {
@@ -54,7 +54,6 @@ void ConnmanTestMsg::Handshake(CNode& node,
     assert(peerman.GetNodeStateStats(node.GetId(), statestats));
     assert(statestats.m_relay_txs == (relay_txs && !node.IsBlockOnlyConn()));
     assert(statestats.their_services == remote_services);
-    node.m_permissionFlags = permission_flags;
     if (successfully_connected) {
         CSerializedNetMsg msg_verack{mm.Make(NetMsgType::VERACK)};
         (void)connman.ReceiveMsgFrom(node, std::move(msg_verack));
@@ -69,19 +68,7 @@ void ConnmanTestMsg::NodeReceiveMsgBytes(CNode& node, Span<const uint8_t> msg_by
 {
     assert(node.ReceiveMsgBytes(msg_bytes, complete));
     if (complete) {
-        size_t nSizeAdded = 0;
+        node.MarkReceivedMsgsForProcessing();
-        auto it(node.vRecvMsg.begin());
-        for (; it != node.vRecvMsg.end(); ++it) {
-            // vRecvMsg contains only completed CNetMessage
-            // the single possible partially deserialized message are held by TransportDeserializer
-            nSizeAdded += it->m_raw_message_size;
-        }
-        {
-            LOCK(node.cs_vProcessMsg);
-            node.vProcessMsg.splice(node.vProcessMsg.end(), node.vRecvMsg, node.vRecvMsg.begin(), it);
-            node.nProcessQueueSize += nSizeAdded;
-            node.fPauseRecv = node.nProcessQueueSize > nReceiveFloodSize;
-        }
     }
 }
 
@@ -128,6 +115,8 @@ std::vector<NodeEvictionCandidate> GetRandomNodeEvictionCandidates(int n_candida
             /* prefer_evict */ random_context.randbool(),
             /* m_is_local */ random_context.randbool(),
             /* m_network */ ALL_NETWORKS[random_context.randrange(ALL_NETWORKS.size())],
+            /* m_noban */ false,
+            /* m_conn_type */ConnectionType::INBOUND,
         });
     }
     return candidates;

src/test/util/net.h

@@ -6,6 +6,7 @@
 #define BITCOIN_TEST_UTIL_NET_H
 
 #include <compat.h>
+#include <node/eviction.h>
 #include <netaddress.h>
 #include <net.h>
 #include <util/sock.h>
@@ -42,7 +43,6 @@ struct ConnmanTestMsg : public CConnman {
                    bool successfully_connected,
                    ServiceFlags remote_services,
                    ServiceFlags local_services,
-                   NetPermissionFlags permission_flags,
                    int32_t version,
                    bool relay_txs)
         EXCLUSIVE_LOCKS_REQUIRED(NetEventsInterface::g_msgproc_mutex);