diff --git a/src/net.cpp b/src/net.cpp index e7e49d2f65e7..85d27c5f220b 100644 --- a/src/net.cpp +++ b/src/net.cpp @@ -2552,69 +2552,74 @@ int CConnman::GetExtraBlockRelayCount() const return std::max(block_relay_peers - m_max_outbound_block_relay, 0); } -bool CConnman::DemoteToStaleOutbound(CNode& node, unsigned int max_stale) +CConnman::StaleOutboundRoom CConnman::CheckStaleOutboundRoom(ConnectionType conn_type, unsigned int max_stale, const CNode* exclude) const { - // The version handler rejects a redundant VERSION before the stale gate, so - // a peer is never demoted twice; assert that rather than guarding for it. - Assert(!node.m_is_stale_outbound); - // m_nodes_mutex guards grantOutbound and m_network_conn_counts, and lets us - // count peers without racing the socket handler. The stale count is derived - // from the flag here rather than kept in a separate counter, so it can never - // drift out of sync with the connections it describes. num_stale is unsigned - // to match max_stale (-maxstaleoutbound); inbound_equiv is int to match - // m_max_inbound, so neither comparison trips -Wsign-compare. - LOCK(m_nodes_mutex); - if (node.fDisconnect) return false; - const ConnectionType conn_type{node.m_conn_type}; - unsigned int num_stale{0}; + AssertLockHeld(m_nodes_mutex); + // The stale count is derived from the flag rather than kept in a separate + // counter, so it can never drift out of sync with the connections it + // describes. num_stale is unsigned to match max_stale (-maxstaleoutbound); + // inbound_equiv is int to match m_max_inbound, so neither comparison trips + // -Wsign-compare. + StaleOutboundRoom room; int inbound_equiv{0}; int same_target{0}; for (const CNode* pnode : m_nodes) { - if (pnode->fDisconnect) continue; + if (pnode == exclude || pnode->fDisconnect) continue; // A demoted stale peer draws on the inbound budget, like a real inbound. if (pnode->m_is_stale_outbound) { - ++num_stale; + ++room.num_stale; ++inbound_equiv; } else if (pnode->IsInboundConn()) { ++inbound_equiv; } - // Peers filling this outbound target, preferred or stale alike. A demoted - // peer keeps its connection type, so this counts both, and node itself - // is still in m_nodes here, so it counts towards its own target too. + // Peers filling this outbound target, preferred or stale alike: a demoted + // peer keeps its connection type. if (pnode->m_conn_type == conn_type) ++same_target; } - if (num_stale >= max_stale) { - LogDebug(BCLog::NET, "peer lacks NODE_BLAKE2B and already have %u stale outbound peers (limit %u), %s\n", - num_stale, max_stale, node.DisconnectMsg(fLogIPs)); - node.fDisconnect = true; - return false; + Assume(conn_type == ConnectionType::OUTBOUND_FULL_RELAY || conn_type == ConnectionType::BLOCK_RELAY); + const int max_same_target{conn_type == ConnectionType::OUTBOUND_FULL_RELAY ? m_max_outbound_full_relay : m_max_outbound_block_relay}; + if (room.num_stale >= max_stale) { + room.refusal = strprintf("already have %u stale outbound peers (limit %u)", room.num_stale, max_stale); + } else if (same_target >= max_same_target) { + // Tolerating a stale peer is only worthwhile while it fills a gap in the + // outbound target. Once that target is met, by preferred peers, already + // tolerated stale ones, or a mix, another stale peer buys us nothing. + room.refusal = strprintf("the outbound target is already full (%d/%d)", same_target, max_same_target); + } else if (inbound_equiv >= m_max_inbound) { + // Demoting releases the outbound slot, which we then refill toward the + // outbound target, so the peer must fit the inbound budget or a later + // outbound connection would push us past -maxconnections. + room.refusal = "no room within -maxconnections"; } - // Tolerating a stale peer is only worthwhile while it fills a gap in the - // outbound target. Once that target is met, by preferred peers, already - // tolerated stale ones, or a mix, another stale peer buys us nothing. - // same_target includes node, so compare with > and report the rest. - const int max_same_target{node.IsFullOutboundConn() ? m_max_outbound_full_relay : m_max_outbound_block_relay}; - if (same_target > max_same_target) { - LogDebug(BCLog::NET, "peer lacks NODE_BLAKE2B and the outbound target is already full (%d/%d), %s\n", - same_target - 1, max_same_target, node.DisconnectMsg(fLogIPs)); - node.fDisconnect = true; - return false; - } - // Demoting releases the outbound slot, which we then refill toward the - // outbound target, so the peer must fit the inbound budget or a later - // outbound connection would push us past -maxconnections. - if (inbound_equiv >= m_max_inbound) { - LogDebug(BCLog::NET, "peer lacks NODE_BLAKE2B and no room within -maxconnections, %s\n", - node.DisconnectMsg(fLogIPs)); + return room; +} + +bool CConnman::CanTolerateStaleOutbound(ConnectionType conn_type, unsigned int max_stale) const +{ + LOCK(m_nodes_mutex); + return !CheckStaleOutboundRoom(conn_type, max_stale, /*exclude=*/nullptr).refusal.has_value(); +} + +bool CConnman::DemoteToStaleOutbound(CNode& node, unsigned int max_stale) +{ + // The version handler rejects a redundant VERSION before the stale gate, so + // a peer is never demoted twice; assert that rather than guarding for it. + Assert(!node.m_is_stale_outbound); + // m_nodes_mutex guards grantOutbound and m_network_conn_counts, and lets us + // count peers without racing the socket handler. + LOCK(m_nodes_mutex); + if (node.fDisconnect) return false; + const StaleOutboundRoom room{CheckStaleOutboundRoom(node.m_conn_type, max_stale, &node)}; + if (room.refusal) { + LogDebug(BCLog::NET, "peer lacks NODE_BLAKE2B and %s, %s\n", *room.refusal, node.DisconnectMsg(fLogIPs)); node.fDisconnect = true; return false; } node.m_is_stale_outbound = true; node.grantOutbound.Release(); if (node.IsManualOrFullOutboundConn()) --m_network_conn_counts[node.addr.GetNetwork()]; - ++num_stale; LogDebug(BCLog::NET, "connected to stale outbound peer (%u/%u), %s\n", - num_stale, max_stale, node.ConnectionTypeAsString()); + room.num_stale + 1, max_stale, node.ConnectionTypeAsString()); return true; } @@ -2898,6 +2903,18 @@ void CConnman::ThreadOpenConnections(const std::vector connect, Spa continue; } + // Only a NODE_BLAKE2B peer can fill a persistent outbound slot: any other + // is demoted to stale at the handshake and stops counting towards the + // target, so prefer one for every slot of it. The extra peers opened past + // the target are the same two types, and one without the bit is refused + // outright there because the target is full, so prefer one for those too. + const bool prefer_blake2b{conn_type == ConnectionType::OUTBOUND_FULL_RELAY || conn_type == ConnectionType::BLOCK_RELAY}; + // Falling back to a peer without it after enough tries is what bootstraps + // a node that can find none, but that only helps while the peer would be + // kept: once it would be dropped at the handshake instead, we would + // reconnect to the network twice a second for nothing. + const bool blake2b_required{prefer_blake2b && !m_msgproc->CanTolerateStaleOutbound(conn_type)}; + addrman.ResolveCollisions(); const auto current_time{NodeClock::now()}; @@ -2970,8 +2987,11 @@ void CConnman::ThreadOpenConnections(const std::vector connect, Spa continue; } - // only consider very recently tried nodes after 30 failed attempts - if (current_time - addr_last_try < 10min && nTries < 30) { + // only consider very recently tried nodes after 30 failed attempts, + // unless we need NODE_BLAKE2B: an address that claims it but never + // completes a handshake is never corrected by SetServices, so + // relaxing this would re-dial it every pass + if (current_time - addr_last_try < 10min && (nTries < 30 || blake2b_required)) { continue; } @@ -2984,15 +3004,7 @@ void CConnman::ThreadOpenConnections(const std::vector connect, Spa continue; } - // Prefer NODE_BLAKE2B peers for the first outbound-full-relay slots - // so the node quickly has peers that can serve the header chain past - // the hard fork. #368 demotes any non-BLAKE2B full-outbound peer to - // stale, so nOutboundFullRelay already counts only fork-capable ones. - // Fall back to any desirable peer after enough tries so a node that - // cannot yet find one still bootstraps. - if (conn_type == ConnectionType::OUTBOUND_FULL_RELAY && - nOutboundFullRelay < SEED_OUTBOUND_CONNECTION_THRESHOLD && - !(addr.nServices & NODE_BLAKE2B) && nTries < 30) { + if (prefer_blake2b && !(addr.nServices & NODE_BLAKE2B) && (nTries < 30 || blake2b_required)) { continue; } diff --git a/src/net.h b/src/net.h index f3d7576a3212..54d4bb53dd79 100644 --- a/src/net.h +++ b/src/net.h @@ -1069,6 +1069,12 @@ class NetEventsInterface */ virtual bool HasAllDesirableServiceFlags(ServiceFlags services) const = 0; + /** + * Callback to determine whether one more outbound peer of this type that + * lacks NODE_BLAKE2B would be kept rather than dropped at the handshake. + */ + virtual bool CanTolerateStaleOutbound(ConnectionType conn_type) const = 0; + /** * Process protocol messages received from a given node * @@ -1261,16 +1267,18 @@ class CConnman // Count the number of block-relay-only peers we have over our limit. int GetExtraBlockRelayCount() const; + /** NetEventsInterface::CanTolerateStaleOutbound, given the -maxstaleoutbound + * budget the caller enforces. */ + bool CanTolerateStaleOutbound(ConnectionType conn_type, unsigned int max_stale) const EXCLUSIVE_LOCKS_REQUIRED(!m_nodes_mutex); + /** Demote an outbound peer that did not advertise NODE_BLAKE2B to an * additional connection: give up its automatic outbound semaphore slot (so * we keep looking for a preferred peer) and stop counting it as our outbound * coverage of its network. A demoted peer draws on the inbound budget, so it - * is kept only while fewer than max_stale are already tolerated, its outbound - * target is not already filled (by preferred or stale peers alike), and the - * inbound budget has room; otherwise this sets fDisconnect and returns false - * without demoting. Does its own logging. Must not be called on an already- - * demoted peer (Assert): the version handler guarantees this by rejecting - * redundant VERSION messages. */ + * is kept only while CheckStaleOutboundRoom finds room for it; otherwise this + * sets fDisconnect and returns false without demoting. Does its own logging. + * Must not be called on an already-demoted peer (Assert): the version handler + * guarantees this by rejecting redundant VERSION messages. */ bool DemoteToStaleOutbound(CNode& node, unsigned int max_stale) EXCLUSIVE_LOCKS_REQUIRED(!m_nodes_mutex); bool AddNode(const AddedNodeParams& add) EXCLUSIVE_LOCKS_REQUIRED(!m_added_nodes_mutex); @@ -1365,6 +1373,17 @@ class CConnman //! in case of no limit, it will always return 0 std::chrono::seconds GetMaxOutboundTimeLeftInCycle_() const EXCLUSIVE_LOCKS_REQUIRED(m_total_bytes_sent_mutex); + struct StaleOutboundRoom { + //! Unset if another stale outbound peer fits, else why not, for logging. + std::optional refusal; + unsigned int num_stale{0}; + }; + + /** Counts that decide whether another stale outbound peer fits. + * `exclude`, when set, is skipped while counting, so a peer that has already + * connected is measured the same way as one that has not. */ + StaleOutboundRoom CheckStaleOutboundRoom(ConnectionType conn_type, unsigned int max_stale, const CNode* exclude) const EXCLUSIVE_LOCKS_REQUIRED(m_nodes_mutex); + bool BindListenPort(const CService& bindAddr, bilingual_str& strError, NetPermissionFlags permissions); bool Bind(const CService& addr, unsigned int flags, NetPermissionFlags permissions); bool InitBinds(const Options& options); diff --git a/src/net_processing.cpp b/src/net_processing.cpp index 53f1aa313acd..d498c21069ac 100644 --- a/src/net_processing.cpp +++ b/src/net_processing.cpp @@ -493,6 +493,7 @@ class PeerManagerImpl final : public PeerManager void InitializeNode(const CNode& node, ServiceFlags our_services) override EXCLUSIVE_LOCKS_REQUIRED(!m_peer_mutex, !m_tx_download_mutex); void FinalizeNode(const CNode& node) override EXCLUSIVE_LOCKS_REQUIRED(!m_peer_mutex, !m_headers_presync_mutex, !m_tx_download_mutex); bool HasAllDesirableServiceFlags(ServiceFlags services) const override; + bool CanTolerateStaleOutbound(ConnectionType conn_type) const override; bool ProcessMessages(CNode* pfrom, std::atomic& interrupt) override EXCLUSIVE_LOCKS_REQUIRED(!m_peer_mutex, !m_most_recent_block_mutex, !m_headers_presync_mutex, g_msgproc_mutex, !m_tx_download_mutex); bool SendMessages(CNode* pto) override @@ -1655,6 +1656,11 @@ bool PeerManagerImpl::HasAllDesirableServiceFlags(ServiceFlags services) const return !(GetDesirableServiceFlags(services) & (~services)); } +bool PeerManagerImpl::CanTolerateStaleOutbound(ConnectionType conn_type) const +{ + return m_connman.CanTolerateStaleOutbound(conn_type, m_opts.maxstaleoutbound); +} + ServiceFlags PeerManagerImpl::GetDesirableServiceFlags(ServiceFlags services) const { if (services & NODE_NETWORK_LIMITED) { diff --git a/src/test/denialofservice_tests.cpp b/src/test/denialofservice_tests.cpp index 18497c067882..a7fba611b9ec 100644 --- a/src/test/denialofservice_tests.cpp +++ b/src/test/denialofservice_tests.cpp @@ -489,6 +489,65 @@ BOOST_FIXTURE_TEST_CASE(stale_outbound_respects_maxconnections, OutboundTest) connman->ClearTestNodes(); } +//! ThreadOpenConnections asks CanTolerateStaleOutbound before opening a +//! connection precisely so it does not open one the handshake would drop, so the +//! answer has to match what DemoteToStaleOutbound then does. The candidate is in +//! m_nodes for one and not the other, which is what the exclude argument of +//! CheckStaleOutboundRoom is for; check they agree on each refusal. +BOOST_FIXTURE_TEST_CASE(stale_outbound_room_predicts_demotion, OutboundTest) +{ + NodeId id{0}; + auto connman = std::make_unique(0x1337, 0x1337, *m_node.addrman, *m_node.netgroupman, Params()); + auto peerLogic = PeerManager::make(*connman, *m_node.addrman, nullptr, *m_node.chainman, *m_node.mempool, *m_node.warnings, {}); + + CConnman::Options options; + options.m_max_automatic_connections = 20; + connman->Init(options); + std::vector vNodes; + + // Ask whether one more stale peer of this type fits, then connect one and + // demote it. A refused peer keeps its outbound slot and is disconnected, so + // it drops out of the counts either way and the two stay comparable. + const auto ask_then_demote = [&](ConnectionType conn_type, unsigned int max_stale) { + const bool predicted{connman->CanTolerateStaleOutbound(conn_type, max_stale)}; + AddRandomOutboundPeer(id, vNodes, *peerLogic, *connman, conn_type); + BOOST_CHECK_EQUAL(predicted, connman->DemoteToStaleOutbound(*vNodes.back(), max_stale)); + return predicted; + }; + + // A budget of zero tolerates nothing, which is what makes a node started with + // -maxstaleoutbound=0 dial NODE_BLAKE2B peers only. + BOOST_CHECK(!ask_then_demote(ConnectionType::OUTBOUND_FULL_RELAY, 0)); + + // With room, both agree the peer is kept, up to the block-relay target. + BOOST_CHECK(ask_then_demote(ConnectionType::BLOCK_RELAY, 1000)); + BOOST_CHECK(ask_then_demote(ConnectionType::BLOCK_RELAY, 1000)); + // Which stale peers now fill, so another buys nothing. + BOOST_CHECK(!ask_then_demote(ConnectionType::BLOCK_RELAY, 1000)); + + // Those two also spend a -maxstaleoutbound budget of 2, whatever the target. + BOOST_CHECK(!ask_then_demote(ConnectionType::OUTBOUND_FULL_RELAY, 2)); + BOOST_CHECK(ask_then_demote(ConnectionType::OUTBOUND_FULL_RELAY, 1000)); + + // Fill the inbound budget the stale peers already tolerated above draw on, + // counted rather than hardcoded so reordering the cases cannot silently turn + // the check below into one of the other two refusals. + const int tolerated{static_cast(std::count_if(vNodes.begin(), vNodes.end(), + [](const CNode* n) -> bool { return n->m_is_stale_outbound; }))}; + const int max_inbound = options.m_max_automatic_connections + - (MAX_OUTBOUND_FULL_RELAY_CONNECTIONS + MAX_BLOCK_RELAY_ONLY_CONNECTIONS + MAX_FEELER_CONNECTIONS); + BOOST_REQUIRE(tolerated < max_inbound); + for (int i = tolerated; i < max_inbound; ++i) { + AddRandomOutboundPeer(id, vNodes, *peerLogic, *connman, ConnectionType::INBOUND); + } + BOOST_CHECK(!ask_then_demote(ConnectionType::OUTBOUND_FULL_RELAY, 1000)); + + for (const CNode* node : vNodes) { + peerLogic->FinalizeNode(*node); + } + connman->ClearTestNodes(); +} + BOOST_AUTO_TEST_CASE(peer_discouragement) { LOCK(NetEventsInterface::g_msgproc_mutex); diff --git a/test/functional/p2p_blake2b_outbound_preference.py b/test/functional/p2p_blake2b_outbound_preference.py index dce8b8b2fcab..1c83a67514e4 100755 --- a/test/functional/p2p_blake2b_outbound_preference.py +++ b/test/functional/p2p_blake2b_outbound_preference.py @@ -1,13 +1,16 @@ #!/usr/bin/env python3 -"""ThreadOpenConnections prefers NODE_BLAKE2B peers for the first outbound -full-relay slots (SEED_OUTBOUND_CONNECTION_THRESHOLD), so a node quickly gains -peers that can serve the header chain past the BLAKE2b hard fork. It falls back -to any desirable peer after enough tries so a node with none yet still bootstraps. +"""ThreadOpenConnections prefers NODE_BLAKE2B peers when filling an outbound +target, so a node quickly gains peers that can serve the header chain past the +BLAKE2b hard fork. It falls back to any desirable peer after enough tries, while +one that lacks the bit would still be tolerated as a stale peer, so a node that +can find none still bootstraps. The addresses are unreachable (connections go through an unreachable proxy and -never complete), so we observe the addresses the node attempts in debug.log. A -fresh node with no anchors always opens OUTBOUND_FULL_RELAY first, so every -attempt exercises the preference. +never complete), so we observe the addresses the node attempts in debug.log and +no peer is ever demoted, which is what keeps the fallback in play here. A fresh +node with no anchors always opens OUTBOUND_FULL_RELAY first, so every attempt +exercises the preference. p2p_blake2b_outbound_slots.py covers what happens once +connections complete and the stale budget runs out. Case A: mix of NODE_BLAKE2B (250.x) and non-HF (251.x) -> the first slots are HF. Case B: only non-HF (251.x) -> attempted anyway (fallback). diff --git a/test/functional/p2p_blake2b_outbound_slots.py b/test/functional/p2p_blake2b_outbound_slots.py new file mode 100755 index 000000000000..17066be29a9f --- /dev/null +++ b/test/functional/p2p_blake2b_outbound_slots.py @@ -0,0 +1,411 @@ +#!/usr/bin/env python3 +# Copyright (c) 2026 The Bitcoin Knots developers +# Distributed under the MIT software license, see the accompanying +# file COPYING or http://www.opensource.org/licenses/mit-license.php. +"""Test how the automatic outbound slots are filled when peers lack NODE_BLAKE2B. + +Only a NODE_BLAKE2B peer can fill a persistent outbound slot: any other is +demoted to stale at the handshake and stops counting towards the target. + +A SOCKS5 proxy stands in for the network, so every address the node picks out of +addrman is dialed for real and answered by a python peer with chosen services. + +Case A: no fork-capable peer exists. The node fills the outbound target with + stale peers and then stops dialing, instead of connecting to and + dropping one peer after another. +Case B: fork-capable peers exist and no stale peer would be tolerated + (-maxstaleoutbound=0). Every outbound-full-relay and block-relay slot + goes to a NODE_BLAKE2B peer, and no peer without it takes one. +Case C: addrman claims NODE_BLAKE2B for peers that turn out not to have it, + which is what a fixed seed is: net.cpp stamps those with + SeedsServiceFlags() without knowing. The node has to dial one to find + out, so it works through them and then stops, rather than dialing them + over and over. +Case D: the same claim, but the peer never completes a handshake. addrman is + never corrected, since SetServices only runs on a received VERSION, so + the node has to fall back on the recently-tried backoff instead. +Case E: the extra peers opened past a full target, here the stale-tip one. A + peer without the bit is refused outright there, so only a fork-capable + one is dialed for it. Feelers stay ungated so addrman keeps being probed. +""" + +import os +import socket +import threading +import time + +from test_framework.messages import ( + CBlockHeader, + NODE_BLAKE2B, + NODE_NETWORK, + NODE_WITNESS, + from_hex, + msg_headers, +) +from test_framework.p2p import NetworkThread, P2PInterface +from test_framework.socks5 import Socks5Configuration, Socks5Server +from test_framework.test_framework import BitcoinTestFramework +from test_framework.util import assert_equal, p2p_port + +PRE_FORK = NODE_NETWORK | NODE_WITNESS +FORK_CAPABLE = PRE_FORK | NODE_BLAKE2B + +# src/net.h outbound targets and the src/net_processing.h -maxstaleoutbound +# default. The full-relay target and the default are equal, so case A reaches +# both limits at once. +MAX_OUTBOUND_FULL_RELAY_CONNECTIONS = 8 +MAX_BLOCK_RELAY_ONLY_CONNECTIONS = 2 +DEFAULT_MAXSTALEOUTBOUND = 8 + +# Enough peers to answer both outbound targets at once, with room to spare for +# a slot being re-armed after the node drops its peer. Case B keeps some of them +# pre-fork, so that dialing one is observable rather than silently refused by the +# proxy, which is what makes its "no peer without the bit took a slot" real. +NUM_FORK_CAPABLE_LISTENERS = 14 +NUM_PRE_FORK_LISTENERS = 6 +NUM_LISTENERS = NUM_FORK_CAPABLE_LISTENERS + NUM_PRE_FORK_LISTENERS +# Enough pre-fork addresses that the node never runs out of ones to pick, so +# picking a fork-capable one has to be a preference rather than an accident, +# and enough fork-capable ones that it can still find the last few it needs by +# drawing from addrman. They stay a small minority of it either way. +NUM_PRE_FORK = 1000 +NUM_FORK_CAPABLE = 64 +# Case C works through these one at a time, so keep it to what fits the window. +NUM_LYING = 40 +# Case D only needs enough to tell one dial each from dialing in a loop. +NUM_DEAD = 5 +# Case E ages the tip past the 30 minutes that make it stale, in steps short +# enough that no peer trips the 20 minute inactivity timeout. +STALE_TIP_STEP = 8 * 60 +STALE_TIP_STEPS = 4 +# Long enough for the two connections per second the dial loop used to make. +QUIET_SECONDS = 15 +# The last slot can fill while a connection is already on its way, so let that +# one land before taking the counts the quiet window is measured against. +SETTLE_SECONDS = 5 + +REFUSED = "peer lacks NODE_BLAKE2B and" + + +def free_port(): + s = socket.socket() + s.bind(('127.0.0.1', 0)) + port = s.getsockname()[1] + s.close() + return port + + +class Blake2bOutboundSlots(BitcoinTestFramework): + def set_test_params(self): + self.setup_clean_chain = True + self.num_nodes = 1 + # Let the node run ThreadOpenConnections and pick its own peers. + self.disable_autoconnect = False + + def setup_network(self): + self.lock = threading.Lock() + # Case C makes every listener answer without NODE_BLAKE2B, whatever the + # address it was reached at claims in addrman. + self.lying = False + # Case D has the proxy close every dial instead of serving it. + self.dead = False + self.exhausted = [] + self.dialed = [] + self.last_dial = 0.0 + self.free = {PRE_FORK: [], FORK_CAPABLE: []} + self.ports = {} + + conf = Socks5Configuration() + conf.addr = ('127.0.0.1', p2p_port(self.num_nodes)) + conf.unauth = True + conf.auth = True + conf.keep_alive = True + conf.destinations_factory = self.pick_destination + self.proxy = Socks5Server(conf) + self.proxy.start() + + self.extra_args = [[f"-proxy={conf.addr[0]}:{conf.addr[1]}", "-proxyrandomize=0", + "-dnsseed=0", "-fixedseeds=0", "-debug=net"]] + self.add_nodes(self.num_nodes, self.extra_args) + + def serving(self, port, peer): + """Wait until `peer` is the protocol the listener on `port` will hand the + next connection. NetworkThread.listeners persists once bound, so checking + that would be vacuous on a re-arm and would also race the window before + the proto is installed, in which the node's dial is dropped.""" + deadline = time.time() + 10 * self.options.timeout_factor + while time.time() < deadline: + if NetworkThread.protos.get(('127.0.0.1', port)) is peer: + return True + time.sleep(0.05) + return False + + def arm(self, idx, services): + """Have listener `idx` answer its next connection with `services`.""" + for _ in range(10): + # A port free when we look can be taken before the listener binds it, + # which under a parallel test run happens often enough to matter, so + # check the listener came up and pick another port if it did not. + port = self.ports.setdefault(idx, free_port()) + peer = P2PInterface() + # connect_id and connect_cb go unused: the node finds these peers + # through the proxy, not through the addconnection RPC. + peer.peer_accept_connection( + connect_id=1, connect_cb=lambda a, p: None, net=self.chain, + timeout_factor=self.options.timeout_factor, + supports_v2_p2p=False, reconnect=False, services=services) + NetworkThread.listen(peer, lambda a, p: None, port=port) + if self.serving(port, peer): + self.peers[idx] = (peer, services) + self.free[services].append(idx) + return + del self.ports[idx] + raise AssertionError(f"could not bind a listener for slot {idx}") + + def arm_listeners(self, counts): + """Have the listeners answer their next connection, `counts` of each + service flavor.""" + self.free = {PRE_FORK: [], FORK_CAPABLE: []} + self.peers = {} + idx = 0 + for services, count in counts.items(): + for _ in range(count): + self.arm(idx, services) + idx += 1 + assert idx <= len(self.ports) + + def wait_serving(self, predicate, timeout): + """Wait for the node, re-arming dropped listeners as we go.""" + def check(): + self.reap() + return predicate() + self.wait_until(check, timeout=timeout) + + def reap(self): + """Re-arm the listeners whose peer the node has dropped, so a slot is + never spent for good on a connection that did not become a peer.""" + with self.lock: + for idx, (peer, services) in list(self.peers.items()): + if not peer.is_connected and peer.message_count.get("version"): + self.arm(idx, services) + + def pick_destination(self, addr, port): + """Answer a dial with a listener advertising that address's services.""" + services = PRE_FORK if self.lying else ( + FORK_CAPABLE if addr.startswith("250.") else PRE_FORK) + with self.lock: + self.dialed.append(addr) + self.last_dial = time.time() + if self.dead: + return None + if not self.free[services]: + # The proxy has already accepted, so returning None here would + # leave the node holding a connection that never handshakes and + # would surface as an unexplained "kept dialing" failure. + self.exhausted.append(addr) + return None + idx = self.free[services].pop(0) + return {"actual_to_addr": "127.0.0.1", "actual_to_port": self.ports[idx]} + + def seed_addrman(self, node, fork_capable): + """Fill addrman, every address in its own /16 so it is the service flags + and not the netgroup rule that decides which ones the node picks.""" + for n in range(fork_capable): + node.addpeeraddress(f"250.{n}.0.1", 8333, False, FORK_CAPABLE) + for n in range(NUM_PRE_FORK): + node.addpeeraddress(f"{251 + n // 256}.{n % 256}.0.1", 8333, False, PRE_FORK) + + def dialing_stopped(self): + """Nothing dialed for a while. The count of addresses worked through is + not a reliable wait: addrman drops the occasional one to a bucket + collision, so an exact total may never arrive.""" + with self.lock: + return bool(self.dialed) and time.time() - self.last_dial > SETTLE_SECONDS + + def log_since(self, node, offset): + with open(node.debug_log_path, encoding="utf8") as f: + f.seek(offset) + return f.read() + + def log_count(self, node, needle): + with open(node.debug_log_path, encoding="utf8") as f: + return f.read().count(needle) + + def outbound(self, node, conn_type, fork_capable=False): + peers = [p for p in node.getpeerinfo() if p["connection_type"] == conn_type] + if fork_capable: + peers = [p for p in peers if int(p["services"], 16) & NODE_BLAKE2B] + return peers + + def run_test(self): + try: + self.check_outbound_slots() + finally: + self.proxy.stop() + + def check_outbound_slots(self): + node = self.nodes[0] + target = MAX_OUTBOUND_FULL_RELAY_CONNECTIONS + + self.log.info("Case A: with no fork-capable peer, fill the target and stop dialing") + self.arm_listeners({PRE_FORK: NUM_LISTENERS}) + self.start_node(0) + self.seed_addrman(node, fork_capable=0) + self.wait_serving(lambda: self.log_count( + node, f"connected to stale outbound peer ({DEFAULT_MAXSTALEOUTBOUND}/") >= 1, timeout=60) + assert_equal(len(self.outbound(node, "outbound-full-relay")), target) + + time.sleep(SETTLE_SECONDS) + with self.lock: + dialed = len(self.dialed) + refused = self.log_count(node, REFUSED) + time.sleep(QUIET_SECONDS) + with self.lock: + assert_equal(len(self.dialed), dialed) + assert_equal(self.log_count(node, REFUSED), refused) + assert_equal(self.exhausted, []) + self.log.info(f"stopped after {dialed} dials, nothing further in {QUIET_SECONDS}s") + + self.log.info("Case B: with -maxstaleoutbound=0, every slot goes to a fork-capable peer") + self.stop_node(0) + self.dialed.clear() + self.arm_listeners({FORK_CAPABLE: NUM_FORK_CAPABLE_LISTENERS, + PRE_FORK: NUM_PRE_FORK_LISTENERS}) + self.start_node(0, extra_args=self.extra_args[0] + ["-maxstaleoutbound=0"]) + # debug.log spans both cases, and case A can leave a refusal in it from a + # connection that was already on its way when the last slot filled, so + # count from here rather than from the start of the file. + refused = self.log_count(node, REFUSED) + self.seed_addrman(node, fork_capable=NUM_FORK_CAPABLE) + self.wait_serving(lambda: len( + self.outbound(node, "outbound-full-relay", fork_capable=True)) == target, timeout=120) + # The block-relay slots are filled through the same preference, once the + # full-relay target no longer claims every connection attempt. + self.wait_serving(lambda: len(self.outbound(node, "block-relay-only", fork_capable=True)) + == MAX_BLOCK_RELAY_ONLY_CONNECTIONS, timeout=120) + # Some of the listeners answer without NODE_BLAKE2B, so had the node + # dialed one it would have been dropped at the handshake and logged. + assert_equal(self.log_count(node, REFUSED), refused) + assert_equal(self.exhausted, []) + self.log.info(f"all {target} outbound-full-relay and " + f"{MAX_BLOCK_RELAY_ONLY_CONNECTIONS} block-relay slots are NODE_BLAKE2B peers") + + self.log.info("Case C: addresses that claim NODE_BLAKE2B without having it") + self.stop_node(0) + # Start from an empty addrman so these are the only addresses to pick. + for stale in ("peers.dat", "anchors.dat"): + (node.chain_path / stale).unlink(missing_ok=True) + self.lying = True + self.dialed.clear() + self.arm_listeners({PRE_FORK: NUM_LISTENERS}) + self.start_node(0) + for n in range(NUM_LYING): + node.addpeeraddress(f"250.{n}.0.1", 8333, False, FORK_CAPABLE) + # Each one costs a connection to disprove, after which the handshake has + # corrected addrman and it is not picked again. + self.wait_serving(lambda: self.dialing_stopped(), timeout=240) + with self.lock: + dialed = len(self.dialed) + assert dialed <= NUM_LYING + 2, f"{dialed} dials for {NUM_LYING} addresses looks like a loop" + time.sleep(QUIET_SECONDS) + with self.lock: + assert_equal(len(self.dialed), dialed) + assert_equal(self.exhausted, []) + self.log.info(f"worked through {dialed} of them and stopped, " + f"nothing further in {QUIET_SECONDS}s") + + self.log.info("Case D: the claim is never disproven because the peer never answers") + self.stop_node(0) + for stale in ("peers.dat", "anchors.dat"): + (node.chain_path / stale).unlink(missing_ok=True) + self.dialed.clear() + # Every dial now goes to a port nothing listens on, so no VERSION ever + # arrives and addrman keeps the claim. -maxstaleoutbound=0 means no peer + # without NODE_BLAKE2B would be kept, so these are the only candidates. + # The proxy has to close too: a half-open connection would keep the + # address's netgroup occupied and hide the re-dialing this checks for. + self.proxy.keep_alive = False + self.dead = True + self.start_node(0, extra_args=self.extra_args[0] + ["-maxstaleoutbound=0"]) + for n in range(NUM_DEAD): + node.addpeeraddress(f"250.{n}.0.1", 8333, False, FORK_CAPABLE) + self.wait_serving(lambda: self.dialing_stopped(), timeout=240) + with self.lock: + dialed = len(self.dialed) + assert dialed <= NUM_DEAD + 2, f"{dialed} dials for {NUM_DEAD} addresses looks like a loop" + time.sleep(QUIET_SECONDS) + with self.lock: + assert_equal(len(self.dialed), dialed) + self.log.info(f"tried each of {NUM_DEAD} once ({dialed} dials) and backed off, " + f"nothing further in {QUIET_SECONDS}s") + + self.log.info("Case E: the extra peer opened once the target is already full") + self.stop_node(0) + for stale in ("peers.dat", "anchors.dat"): + (node.chain_path / stale).unlink(missing_ok=True) + # Cases C and D left every listener answering without the bit; case E + # needs the fork-capable ones to answer with it again. + self.lying = False + self.dead = False + self.proxy.keep_alive = True + self.dialed.clear() + self.arm_listeners({FORK_CAPABLE: NUM_FORK_CAPABLE_LISTENERS, + PRE_FORK: NUM_PRE_FORK_LISTENERS}) + self.start_node(0, extra_args=self.extra_args[0] + ["-maxstaleoutbound=0"]) + self.seed_addrman(node, fork_capable=NUM_FORK_CAPABLE) + self.wait_serving(lambda: len( + self.outbound(node, "outbound-full-relay", fork_capable=True)) == target, timeout=120) + self.wait_serving(lambda: len(self.outbound(node, "block-relay-only", fork_capable=True)) + == MAX_BLOCK_RELAY_ONLY_CONNECTIONS, timeout=120) + + now = int(time.time()) + node.setmocktime(now) + self.generate(node, 1, sync_fun=self.no_op) + self.wait_until(lambda: not node.getblockchaininfo()["initialblockdownload"], timeout=30) + # Each peer announces the tip, or ConsiderEviction drops them all once it + # is 22 minutes old for never having sent a header. + tip = from_hex(CBlockHeader(), node.getblockheader(node.getbestblockhash(), False)) + with self.lock: + for peer, _ in self.peers.values(): + if peer.is_connected: + peer.send_message(msg_headers(headers=[tip])) + + def all_fresh(t): + peers = [p for p in node.getpeerinfo() if not p["inbound"]] + return len(peers) >= target and all(p["lastrecv"] >= t - 60 for p in peers) + + for step in range(STALE_TIP_STEPS): + if step == STALE_TIP_STEPS - 1: + # offset first: a feeler logged between here and the clear would + # otherwise reach the proxy unaccounted for and read as wasted + mark = node.debug_log_path.stat().st_size + with self.lock: + self.dialed.clear() + now += STALE_TIP_STEP + node.setmocktime(now) + # the ping this triggers refreshes lastrecv, so the next step cannot + # trip the inactivity timeout + self.wait_until(lambda: all_fresh(now), timeout=60) + self.reap() + + self.wait_until(lambda: "Potential stale tip detected" in self.log_since(node, mark), + timeout=120) + # Wait for the extra peer to actually land rather than sampling a fixed + # window: without the preference the node works through pre-fork peers + # to get here, so the refusals below are then certain rather than likely. + self.wait_serving(lambda: len(self.outbound(node, "outbound-full-relay")) > target, + timeout=180) + + window = self.log_since(node, mark) + feelers = {line.split("Making feeler connection to ")[1].split(":")[0] + for line in window.splitlines() if "Making feeler connection to " in line} + with self.lock: + wasted = [a for a in self.dialed if not a.startswith("250.") and a not in feelers] + assert_equal(window.count(REFUSED), 0) + assert_equal(wasted, []) + self.log.info(f"stale-tip peer dialed only fork-capable addresses, " + f"{len(feelers)} feeler(s) unaffected") + + +if __name__ == '__main__': + Blake2bOutboundSlots(__file__).main() diff --git a/test/functional/test_runner.py b/test/functional/test_runner.py index 65c900b2b7de..80b47cc7ad89 100755 --- a/test/functional/test_runner.py +++ b/test/functional/test_runner.py @@ -142,6 +142,7 @@ 'p2p_dns_seeds.py', 'p2p_blake2b_dns_immediate.py', 'p2p_blake2b_outbound_preference.py', + 'p2p_blake2b_outbound_slots.py', 'wallet_groups.py --legacy-wallet', 'wallet_groups.py --descriptors', 'p2p_blockfilters.py',