Merge 6b4c4aeaf2 into a46065e36c

src/net.cpp

@@ -2363,7 +2363,7 @@ int CConnman::GetExtraFullOutboundCount() const
             }
         }
     }
-    return std::max(full_outbound_peers - m_max_outbound_full_relay, 0);
+    return full_outbound_peers - m_max_outbound_full_relay;
 }
 
 int CConnman::GetExtraBlockRelayCount() const

src/net_processing.cpp

@@ -510,6 +510,7 @@ public:
     /** Implement PeerManager */
     void StartScheduledTasks(CScheduler& scheduler) override;
     void CheckForStaleTipAndEvictPeers() override;
+    void CheckForLimitedMempoolAndEvictPeers() override;
     std::optional<std::string> FetchBlock(NodeId peer_id, const CBlockIndex& block_index) override
         EXCLUSIVE_LOCKS_REQUIRED(!m_peer_mutex);
     bool GetNodeStateStats(NodeId nodeid, CNodeStateStats& stats) const override EXCLUSIVE_LOCKS_REQUIRED(!m_peer_mutex);
@@ -1991,6 +1992,8 @@ void PeerManagerImpl::StartScheduledTasks(CScheduler& scheduler)
     static_assert(EXTRA_PEER_CHECK_INTERVAL < STALE_CHECK_INTERVAL, "peer eviction timer should be less than stale tip check timer");
     scheduler.scheduleEvery([this] { this->CheckForStaleTipAndEvictPeers(); }, std::chrono::seconds{EXTRA_PEER_CHECK_INTERVAL});
 
+    scheduler.scheduleEvery([this] { this->CheckForLimitedMempoolAndEvictPeers(); }, std::chrono::seconds{60});
+
     // schedule next run for 10-15 minutes in the future
     const std::chrono::milliseconds delta = 10min + GetRandMillis(5min);
     scheduler.scheduleFromNow([&] { ReattemptInitialBroadcast(scheduler); }, delta);
@@ -5343,6 +5346,62 @@ void PeerManagerImpl::CheckForStaleTipAndEvictPeers()
     }
 }
 
+void PeerManagerImpl::CheckForLimitedMempoolAndEvictPeers()
+{
+    LOCK(cs_main);
+    // It only makes sense once we're out of IBD, because otherwise our mempool
+    // is not a good source for fee estimation anyway.
+    // TODO: assumeUTXO?
+    if (!CanDirectFetch()) return;
+
+    // Don't bother if we are still seeking for outbound peers: eviction
+    // instead will reduce our chances to get blocks and transactions from
+    // them.
+    // TODO this could be more nuanced.
+    if (m_connman.GetExtraFullOutboundCount() < 0) return;
+
+    // Ideally we want to compare mempool sizes, not fee filters.
+    // Otherwise we easily get confused: e.g. at empty mempools this is less
+    // critical, but that'd be impossible to account for.
+    const auto our_min_feerate = WITH_LOCK(m_mempool.cs, return m_mempool.GetMinFee());
+
+    std::vector<std::pair<CNode*, CAmount>> peer_fee_filters;
+
+    size_t around_our_percent_fee_filter = 0;
+
+    m_connman.ForEachNode([&](CNode* pnode) {
+        if (!pnode->IsFullOutboundConn() || pnode->fDisconnect) return;
+        auto peer = GetPeerRef(pnode->GetId());
+
+        if (auto tx_relay = peer->GetTxRelay(); tx_relay != nullptr) {
+            if (tx_relay->m_fee_filter_received) {
+                peer_fee_filters.push_back(std::make_pair(pnode, tx_relay->m_fee_filter_received.load()));
+            } else {
+                ++around_our_percent_fee_filter;
+            }
+        }
+    });
+
+    if (peer_fee_filters.size() == 0) return;
+
+    std::sort(peer_fee_filters.begin(), peer_fee_filters.end());
+
+    for (auto [node_id, fee_filter] : peer_fee_filters) {
+        if (fee_filter == 0) ++around_our_percent_fee_filter;
+        // Lower than our filter means they are more permissive.
+        if (CFeeRate{fee_filter} <= our_min_feerate) ++around_our_percent_fee_filter;;
+    }
+
+    if (around_our_percent_fee_filter < 4) {
+        // Drop one peer at a time to preserve a sufficient total
+        // number of connections, and to avoid network DoS.
+        // TODO add randomness chosing among low-feerate candidates.
+        peer_fee_filters.back().first->fDisconnect = true;
+    }
+
+}
+
+
 void PeerManagerImpl::MaybeSendPing(CNode& node_to, Peer& peer, std::chrono::microseconds now)
 {
     if (m_connman.ShouldRunInactivityChecks(node_to, std::chrono::duration_cast<std::chrono::seconds>(now)) &&

src/net_processing.h

@@ -104,6 +104,14 @@ public:
      */
     virtual void CheckForStaleTipAndEvictPeers() = 0;
 
+    /**
+     * We should have outbound peers with broad-enough mempools (comparable to ours) to accept
+     * transactions constructed according to our mempool. Otherwise, the lower-fee transactions
+     * will never be relayed to the network.
+     */
+    virtual void CheckForLimitedMempoolAndEvictPeers() = 0;
+
+
     /** Process a single message from a peer. Public for fuzz testing */
     virtual void ProcessMessage(CNode& pfrom, const std::string& msg_type, DataStream& vRecv,
                                 const std::chrono::microseconds time_received, const std::atomic<bool>& interruptMsgProc) EXCLUSIVE_LOCKS_REQUIRED(g_msgproc_mutex) = 0;

test/functional/p2p_out_eviction.py

@@ -0,0 +1,67 @@
+#!/usr/bin/env python3
+# Copyright (c) 2023 The Bitcoin Core developers
+# Distributed under the MIT software license, see the accompanying
+# file COPYING or http://www.opensource.org/licenses/mit-license.php.
+
+import time
+
+from test_framework.blocktools import (
+    create_block,
+    create_coinbase,
+)
+from test_framework.messages import (
+    msg_pong,
+    msg_tx,
+    msg_feefilter,
+)
+from test_framework.p2p import (
+    P2PDataStore,
+    P2PInterface,
+)
+from test_framework.test_framework import BitcoinTestFramework
+from test_framework.util import assert_equal, check_node_connections
+from test_framework.wallet import MiniWallet
+
+class P2POutEvict(BitcoinTestFramework):
+    def set_test_params(self):
+        self.num_nodes = 1
+        # Start with all the same fee filters.
+        self.extra_args = [["-minrelaytxfee=0.00000100"]]
+
+    def mock_forward(self, delta):
+        self.mock_time += delta
+        self.nodes[0].setmocktime(self.mock_time)
+
+    def run_test(self):
+        self.mock_time = int(time.time())
+        self.mock_forward(0)
+        node = self.nodes[0]
+        peers = []
+        for id in range(8):
+            peers.append(node.add_outbound_p2p_connection(P2PDataStore(), p2p_idx=id, connection_type="outbound-full-relay"))
+
+        node.mockscheduler(60)
+        check_node_connections(node=node, num_in=0, num_out=8)
+
+        # Set a limited number of fee filters. Nothing is evicted because
+        # the rest of the peers are sufficient.
+        for id in range(4):
+            peers[id].send_and_ping(msg_feefilter(10))
+        node.mockscheduler(60)
+        check_node_connections(node=node, num_in=0, num_out=8)
+
+        # Set one more filter, but make it borderline acceptable.
+        peers[4].send_and_ping(msg_feefilter(81))
+        node.mockscheduler(60)
+        check_node_connections(node=node, num_in=0, num_out=8)
+
+        # Now there is too few peers with a comparable min tx relay fee,
+        # thus one of the peers must be evicted to have a room for a new peer
+        # (hopefully with sufficient fee filter).
+        peers[4].send_and_ping(msg_feefilter(10))
+        node.mockscheduler(60)
+        self.mock_forward(60)
+        self.wait_until(lambda: len(self.nodes[0].getpeerinfo()) == 7, timeout=10)
+
+if __name__ == '__main__':
+    P2POutEvict().main()