diff --git a/src/net_processing.cpp b/src/net_processing.cpp index 53f1aa313acd..fa96c44deadd 100644 --- a/src/net_processing.cpp +++ b/src/net_processing.cpp @@ -496,7 +496,7 @@ class PeerManagerImpl final : public PeerManager 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 - EXCLUSIVE_LOCKS_REQUIRED(!m_peer_mutex, !m_most_recent_block_mutex, g_msgproc_mutex, !m_tx_download_mutex); + EXCLUSIVE_LOCKS_REQUIRED(!m_peer_mutex, !m_most_recent_block_mutex, g_msgproc_mutex, !m_tx_download_mutex, !m_headers_presync_mutex); /** Implement PeerManager */ void StartScheduledTasks(CScheduler& scheduler) override; @@ -670,8 +670,61 @@ class PeerManagerImpl final : public PeerManager /** Request further headers from this peer with a given locator. * We don't issue a getheaders message if we have a recent one outstanding. * This returns true if a getheaders is actually sent, and false otherwise. + * + * Pass continuing_started_sync=true when this getheaders continues a header + * sync that has already been started with this peer. The sync-source policy + * (see MayHeaderSyncFrom) was applied when that sync started and its slot is + * already spent; re-applying it mid-sync would only strand the sync state + * without freeing anything. It also keeps the Assume() at that call site + * sound if the policy flips underneath an in-flight sync -- which it now + * can, because MayHeaderSyncFrom()'s bootstrap fallback turns off the moment + * a fork-aware peer connects. + * + * That last case opens a narrow, self-closing race: if a fork-aware peer + * connects in between two SendMessages ticks for this pre-fork peer, this + * bypass can let one more getheaders through here before the slot-release + * logic in SendMessages (below) tears down the sync state on the following + * tick. The extra round-trip is harmless -- the pre-fork peer's headers are + * still validated normally regardless of who sent them -- and the window + * closes itself on the very next tick, so it is accepted as-is rather than + * closed with extra synchronization. + */ + bool MaybeSendGetHeaders(CNode& pfrom, const CBlockLocator& locator, Peer& peer, bool continuing_started_sync = false) EXCLUSIVE_LOCKS_REQUIRED(g_msgproc_mutex); + + /** Whether we are willing to run header sync against this peer. + * + * A peer lacking NODE_BLAKE2B is normally excluded. That exclusion is a + * sync-slot allocation policy, not a safety or correctness mechanism: + * headers are validated against the consensus rules for their height no + * matter who sends them, so accepting a pre-fork peer's headers is not a + * risk. The point is that such a peer dead-ends at the fork height, and we + * only ever run one initial header sync at a time -- spending that slot on + * a peer that cannot follow us past the fork wastes it. + * + * Bootstrap fallback: if we currently have no fork-aware peer connected at + * all, we sync headers from whoever we do have. Most of the network is + * still pre-fork, so a node whose only peers are pre-fork must be able to + * make progress rather than sit at zero headers indefinitely waiting for a + * fork-aware peer to show up. As soon as one connects the exclusion is back + * in force. + * + * Note the asymmetry with the live counter: the fast-path check below tests + * the bare NODE_BLAKE2B bit, while m_fork_aware_header_sync_peers counts the + * stricter IsForkAwareHeaderSyncSource() (the bit plus CanServeBlocks()). + * This is intentional, not an oversight to fix on a future edit: it + * preserves the original rule-1 behavior for a peer that already has the + * bit, and the gap is closed downstream -- TryLowWorkHeadersSync(), the only + * site that actually starts a sync, additionally requires CanServeBlocks() + * before calling here, so a peer that fails that check never gets to start + * anything even though this predicate alone would allow it. + * + * All header-sync gates call this one predicate instead of re-deriving the + * condition. They must not drift apart: in particular the + * Assume(sent_getheaders) in IsContinuationOfLowWorkHeadersSync() only + * holds if the gate in TryLowWorkHeadersSync() and the gate in + * MaybeSendGetHeaders() answer identically for the same peer. */ - bool MaybeSendGetHeaders(CNode& pfrom, const CBlockLocator& locator, Peer& peer) EXCLUSIVE_LOCKS_REQUIRED(g_msgproc_mutex); + bool MayHeaderSyncFrom(const Peer& peer) const; /** Potentially fetch blocks from this peer upon receipt of a new headers tip */ void HeadersDirectFetchBlocks(CNode& pfrom, const Peer& peer, const CBlockIndex& last_header); /** Update peer state based on received headers message */ @@ -805,6 +858,22 @@ class PeerManagerImpl final : public PeerManager /** Number of preferable block download peers. */ int m_num_preferred_download_peers GUARDED_BY(cs_main){0}; + /** Number of currently connected peers that have completed the version + * handshake and are usable header-sync sources under the BLAKE2b hardfork + * rules, i.e. that satisfy IsForkAwareHeaderSyncSource(). Maintained in the + * VERSION handler (the sole assignment to Peer::m_their_services) and in + * FinalizeNode(), following the maintained-counter pattern of + * m_num_preferred_download_peers / m_wtxid_relay_peers. + * + * Deliberately atomic and NOT GUARDED_BY(cs_main): MayHeaderSyncFrom() + * reads it from MaybeSendGetHeaders(), which is reachable from + * IsContinuationOfLowWorkHeadersSync() while Peer::m_headers_sync_mutex is + * held and without cs_main. Counting connected peers on demand instead is + * not an option there either: that would mean taking m_peer_mutex while a + * Peer-internal mutex is held, which the lock-order rule documented on + * m_peer_mutex forbids. */ + std::atomic m_fork_aware_header_sync_peers{0}; + /** Stalling timeout for blocks in IBD */ std::atomic m_block_stalling_timeout{BLOCK_STALLING_TIMEOUT_DEFAULT}; @@ -1105,6 +1174,22 @@ static bool CanServeBlocks(const Peer& peer) return peer.m_their_services & (NODE_NETWORK|NODE_NETWORK_LIMITED); } +/** Whether a peer advertising these services is a usable header-sync source + * under the BLAKE2b hardfork rules. + * + * It is not enough for the peer to set NODE_BLAKE2B: it must also be able to + * serve us blocks at all (the CanServeBlocks() condition, spelled out here + * because this runs on raw service flags before a Peer exists). A peer with + * the bare fork bit and no block-serving capability would otherwise count + * towards PeerManagerImpl::m_fork_aware_header_sync_peers and switch off the + * bootstrap fallback in MayHeaderSyncFrom() without being able to take over + * the header sync the fallback was keeping alive -- i.e. exactly the stall + * that fallback exists to prevent, in a narrower form. */ +static bool IsForkAwareHeaderSyncSource(ServiceFlags services) +{ + return (services & NODE_BLAKE2B) && (services & (NODE_NETWORK | NODE_NETWORK_LIMITED)); +} + /** Whether this peer can only serve limited recent blocks (e.g. because * it prunes old blocks) */ static bool IsLimitedPeer(const Peer& peer) @@ -1594,6 +1679,14 @@ void PeerManagerImpl::FinalizeNode(const CNode& node) assert(peer != nullptr); m_wtxid_relay_peers -= peer->m_wtxid_relay; assert(m_wtxid_relay_peers >= 0); + // Mirror the VERSION handler: only peers that were counted there are + // uncounted here. Read the atomic service flags once so the increment + // and decrement cannot see different values. + const ServiceFlags their_services{peer->m_their_services}; + if (IsForkAwareHeaderSyncSource(their_services)) { + --m_fork_aware_header_sync_peers; + } + assert(m_fork_aware_header_sync_peers >= 0); } CNodeState *state = State(nodeid); assert(state != nullptr); @@ -1632,6 +1725,7 @@ void PeerManagerImpl::FinalizeNode(const CNode& node) assert(m_peers_downloading_from == 0); assert(m_outbound_peers_with_protect_from_disconnect == 0); assert(m_wtxid_relay_peers == 0); + assert(m_fork_aware_header_sync_peers == 0); WITH_LOCK(m_tx_download_mutex, m_txdownloadman.CheckIsEmpty()); } } // cs_main @@ -2600,7 +2694,11 @@ bool PeerManagerImpl::IsContinuationOfLowWorkHeadersSync(Peer& peer, CNode& pfro if (!locator.vHave.empty()) { // It should be impossible for the getheaders request to fail, // because we just cleared the last getheaders timestamp. - bool sent_getheaders = MaybeSendGetHeaders(pfrom, locator, peer); + // This sync is already under way, so the sync-source policy does + // not apply again here -- see MaybeSendGetHeaders(). Without + // that, a fork-aware peer connecting mid-sync would flip the + // policy against this peer and break the Assume() below. + bool sent_getheaders = MaybeSendGetHeaders(pfrom, locator, peer, /*continuing_started_sync=*/true); Assume(sent_getheaders); LogDebug(BCLog::NET, "more getheaders (from %s) to peer=%d\n", locator.vHave.front().ToString(), pfrom.GetId()); @@ -2682,7 +2780,18 @@ bool PeerManagerImpl::TryLowWorkHeadersSync(Peer& peer, CNode& pfrom, const CBlo // Only try to sync with this peer if their headers message was full; // otherwise they don't have more headers after this so no point in // trying to sync their too-little-work chain. - if (headers.size() == m_opts.max_headers_result) { + // + // A peer we are not willing to header-sync from is excluded: we would + // never send it the getheaders this sync depends on (see + // MaybeSendGetHeaders), so starting one would allocate presync state for + // a sync that can never progress. + // + // This must use exactly the same predicate as MaybeSendGetHeaders(), not + // a parallel copy of the condition: the Assume(sent_getheaders) in + // IsContinuationOfLowWorkHeadersSync() is only sound while the two + // answer identically for the same peer. Hence the shared + // MayHeaderSyncFrom(), including its bootstrap fallback. + if (headers.size() == m_opts.max_headers_result && MayHeaderSyncFrom(peer)) { // Note: we could advance to the last header in this set that is // known to us, rather than starting at the first header (which we // may already have); however this is unlikely to matter much since @@ -2725,8 +2834,42 @@ bool PeerManagerImpl::IsAncestorOfBestHeaderOrTip(const CBlockIndex* header) return false; } -bool PeerManagerImpl::MaybeSendGetHeaders(CNode& pfrom, const CBlockLocator& locator, Peer& peer) +bool PeerManagerImpl::MayHeaderSyncFrom(const Peer& peer) const +{ + if (peer.m_their_services & NODE_BLAKE2B) return true; + // Bootstrap fallback: no fork-aware peer is connected, so this pre-fork peer + // is the best header source we have. See the declaration for the rationale. + return m_fork_aware_header_sync_peers.load(std::memory_order_relaxed) == 0; +} + +bool PeerManagerImpl::MaybeSendGetHeaders(CNode& pfrom, const CBlockLocator& locator, Peer& peer, bool continuing_started_sync) { + // Do not request headers from a peer we are not willing to header-sync from, + // i.e. a peer lacking NODE_BLAKE2B while some fork-aware peer is connected. + // See MayHeaderSyncFrom(): this is about not wasting the single header-sync + // slot on a peer that dead-ends at the fork height. It is *not* a safety + // check -- pre-fork headers are validated against the consensus rules for + // their height regardless of who sent them -- which is why the bootstrap + // fallback below can safely relax it to nothing when we have no fork-aware + // peer at all. + // + // The gate lives here, in the single choke point every getheaders goes + // through, rather than only at the call sites that start header sync. Three + // other call sites are reachable unilaterally by any peer, in or out of our + // IBD, and would otherwise leak a getheaders to a pre-fork peer: + // - HandleUnconnectingHeaders(), from a headers message whose first header + // does not connect. CheckHeadersPoW() only validates each header's + // self-declared target, not the difficulty the chain actually requires, + // so this costs an attacker nothing. + // - the "headers message had its maximum size" continuation below. + // - the compact block handler, when the block does not connect. + // Gating the choke point makes the rule an invariant instead of a checklist + // a future caller can be added without. + // + // This is beyond the literal design spec, which named only the header-sync + // trigger; it is a self-contained hunk and can be reverted on its own. + if (!continuing_started_sync && !MayHeaderSyncFrom(peer)) return false; + const auto current_time = NodeClock::now(); // Only allow a new getheaders message to go out if we don't have a recent @@ -3549,6 +3692,13 @@ void PeerManagerImpl::ProcessMessage(CNode& pfrom, const std::string& msg_type, } } peer->m_their_services = nServices; + // Maintain the live count of fork-aware header-sync sources. This is the + // only assignment to m_their_services, and a duplicate VERSION message is + // rejected above, so this runs at most once per peer; FinalizeNode() + // uncounts it under the same predicate. + if (IsForkAwareHeaderSyncSource(nServices)) { + ++m_fork_aware_header_sync_peers; + } pfrom.SetAddrLocal(addrMe); peer->m_starting_height = starting_height; @@ -4022,7 +4172,22 @@ void PeerManagerImpl::ProcessMessage(CNode& pfrom, const std::string& msg_type, // our initial peer is unresponsive (but less bandwidth than we'd // use if we turned on sync with all peers). CNodeState& state{*Assert(State(pfrom.GetId()))}; - if (state.fSyncStarted || (!peer->m_inv_triggered_getheaders_before_sync && *best_block != m_last_block_inv_triggering_headers_sync)) { + // As above, only header-sync from a peer the sync-source policy allows + // (see MayHeaderSyncFrom) -- an unsolicited block inv must not let a + // pre-fork peer pull us into header sync it could not otherwise start. + // The policy's bootstrap fallback applies here too: with no fork-aware + // peer connected, an inv from a pre-fork peer may start header sync, + // because otherwise nothing could. + // + // Unlike the equivalent belt-and-suspenders check in SendMessages, this one + // is load-bearing: MaybeSendGetHeaders() alone would suppress the getheaders, + // but the "if (!state.fSyncStarted)" below would still run, marking + // m_inv_triggered_getheaders_before_sync and consuming this block's one + // new-peer slot in m_last_block_inv_triggering_headers_sync -- letting a + // pre-fork peer's inv silently starve a legitimate peer's turn. So this + // gate must stay; only its condition gains the fallback. + if (MayHeaderSyncFrom(*peer) + && (state.fSyncStarted || (!peer->m_inv_triggered_getheaders_before_sync && *best_block != m_last_block_inv_triggering_headers_sync))) { if (MaybeSendGetHeaders(pfrom, GetLocator(m_chainman.m_best_header), *peer)) { LogDebug(BCLog::NET, "getheaders (%d) %s to peer=%d\n", m_chainman.m_best_header->nHeight, best_block->ToString(), @@ -5529,6 +5694,39 @@ bool PeerManagerImpl::SendMessages(CNode* pto) MaybeSendSendHeaders(*pto, *peer); + // Release the header-sync slot when the bootstrap fallback ends. + // + // This peer may have been allowed to start header sync only because we had + // no fork-aware peer at the time (see MayHeaderSyncFrom). If one has since + // connected, the policy now excludes this peer -- but it is still holding + // nSyncStarted, and we only run one initial header sync at a time. Left + // alone, the fork-aware peer that can actually follow us past the fork would + // have to wait out the headers-sync timeout before it could take over. Hand + // the slot back now instead, and tear down the presync state that went with + // it. + // + // The predicate is deliberately the same MayHeaderSyncFrom() the gates use, + // so this can never fire for a peer those gates would still allow. + if (!MayHeaderSyncFrom(*peer)) { + bool released{false}; + { + LOCK(cs_main); + CNodeState& state = *Assert(State(pto->GetId())); + if (state.fSyncStarted) { + state.fSyncStarted = false; + nSyncStarted--; + released = true; + } + } + if (released) { + LogDebug(BCLog::NET, "releasing header sync slot held by pre-fork peer=%d, a fork-aware peer is now connected\n", pto->GetId()); + peer->m_headers_sync_timeout = 0us; + WITH_LOCK(peer->m_headers_sync_mutex, peer->m_headers_sync.reset(nullptr)); + LOCK(m_headers_presync_mutex); + m_headers_presync_stats.erase(pto->GetId()); + } + } + { LOCK(cs_main); @@ -5560,7 +5758,23 @@ bool PeerManagerImpl::SendMessages(CNode* pto) } } - if (!state.fSyncStarted && CanServeBlocks(*peer) && !m_chainman.m_blockman.LoadingBlocks()) { + // Only start header sync with a peer the sync-source policy allows (see + // MayHeaderSyncFrom). This is the one site that starts initial header + // sync, so it is where the policy -- and its bootstrap fallback for the + // case where we have no fork-aware peer at all -- actually decides + // whether this node makes progress. Peers excluded here remain valid + // sources of (immutable) pre-fork block data, so the block download paths + // below are deliberately left ungated. The exclusion applies both during + // and after our own IBD, and is not exempted for manually-added peers. + // + // Belt-and-suspenders: MaybeSendGetHeaders() applies the same predicate and + // would no-op the call below on its own, so this check changes no observable + // behavior -- but only for as long as the two keep sharing MayHeaderSyncFrom() + // rather than spelling the condition out separately. It is here because it + // saves computing pindexStart and re-entering MaybeSendGetHeaders on every + // SendMessages tick for a peer that can never succeed. + if (!state.fSyncStarted && CanServeBlocks(*peer) && !m_chainman.m_blockman.LoadingBlocks() + && MayHeaderSyncFrom(*peer)) { // Only actively request headers from a single peer, unless we're close to today. if ((nSyncStarted == 0 && sync_blocks_and_headers_from_peer) || m_chainman.m_best_header->Time() > NodeClock::now() - 24h) { const CBlockIndex* pindexStart = m_chainman.m_best_header; diff --git a/test/functional/p2p_blake2b_prefork_peer_policy.py b/test/functional/p2p_blake2b_prefork_peer_policy.py new file mode 100755 index 000000000000..10911f1d7c2c --- /dev/null +++ b/test/functional/p2p_blake2b_prefork_peer_policy.py @@ -0,0 +1,616 @@ +#!/usr/bin/env python3 +"""Local peer-management policy for peers that do not advertise NODE_BLAKE2B. + +Pre-fork ("Spamcoin") peers are not refused outright -- pre-fork blocks are +immutable history and such a peer is a legitimate source of that data -- but +while at least one NODE_BLAKE2B peer is connected, we send them no +`getheaders`, in or out of our own IBD. This exclusion is not exempted for +manually-configured (-addnode/-connect) peers. It is a sync-slot efficiency +policy, not a safety one -- headers are validated against the consensus rules +for their height whoever sends them -- so it has a bootstrap fallback: with no +NODE_BLAKE2B peer connected at all we do sync headers from whoever we have, +rather than not syncing. + +Disconnecting such a peer once it is no longer useful is deliberately out of +scope here; see bitcoinknots/bitcoin#412, which covers that separately and with +an earlier trigger. + +Case A: a pre-fork peer's block is still accepted while we are in IBD. +Case B: no `getheaders` ever goes to the pre-fork peer, while an otherwise + identical NODE_BLAKE2B control peer does receive one. +Case D: a manually-added pre-fork peer is excluded from header sync just the + same -- the exclusion has no manual-peer exemption -- in or out of our + own IBD. A NODE_BLAKE2B control peer is connected first so that the + exclusion is in force at all -- otherwise the bootstrap fallback would + (correctly) send the manual peer a `getheaders` and the case would be + testing the fallback rather than the exclusion. + +Cases E and F cover the `getheaders` paths a peer can trigger unilaterally, +which the header-sync trigger alone does not gate: + +Case E: a `headers` message whose first header does not connect draws no + `getheaders` for a pre-fork peer, while it does for a control peer. +Case F: an unconnecting `cmpctblock` draws no `getheaders` either. That path + only fires out of IBD, so it reuses case D's peer on case D's node, + which case D has already taken out of IBD -- and, as in case D, the + exclusion is only in force there because that node also has a + NODE_BLAKE2B peer. + +Cases G and H cover the bootstrap fallback itself, on a third node kept free of +any other peer so the "no fork-aware peer connected" premise is exact: + +Case G: with a single pre-fork peer and no NODE_BLAKE2B peer at all, the node + does send `getheaders` to that pre-fork peer. This is the regression + the fallback exists to prevent: without it such a node -- the common + case while most of the network is still pre-fork -- would get no + headers at all until a fork-aware peer happened to connect. +Case H: as soon as a NODE_BLAKE2B peer connects, the fallback turns off again + and the pre-fork peer stops drawing `getheaders`, while the same + stimulus still draws one for the NODE_BLAKE2B peer. +Case I: the reverse of case H. A NODE_BLAKE2B peer that is our active header + sync source goes away, and the fallback comes back on by itself: the + pre-fork peer we had been withholding `getheaders` from starts getting + them, with no external stimulus. Nothing kicks this -- FinalizeNode() + both uncounts the peer and hands back its `nSyncStarted` slot, and the + next SendMessages tick re-runs the header-sync trigger -- but that is + precisely what the case pins down, since a missed decrement would strand + the node with no header source at all. +Case J: the fork-aware peer count survives a mix of connection orders and + disconnect mechanisms. The count is not exposed over RPC, so each step + is checked through the behavior it drives: whether a pre-fork peer that + has never been sent a `getheaders` (so nothing but policy can be + suppressing one) draws one or not. +Case K: the low-work headers-sync entry point (TryLowWorkHeadersSync) honors + the same predicate. Reaching it needs a full 2000-header message whose + claimed work is under the anti-DoS threshold, so these two nodes run + with `-minimumchainwork` set, the way p2p_headers_sync_with_minchainwork + does, instead of regtest's default threshold of zero. +""" +from test_framework.blocktools import create_block, create_coinbase +from test_framework.messages import ( + CBlockHeader, + HeaderAndShortIDs, + NODE_BLAKE2B, + NODE_NETWORK, + NODE_WITNESS, + msg_block, + msg_cmpctblock, + msg_headers, +) +from test_framework.p2p import P2PInterface +from test_framework.test_framework import BitcoinTestFramework +from test_framework.util import assert_equal + +HF = NODE_NETWORK | NODE_WITNESS | NODE_BLAKE2B +# NODE_NETWORK is required, otherwise CanServeBlocks() is false and the +# header-sync gate under test is never reached at all. +NON_HF = NODE_NETWORK | NODE_WITNESS + +# Number of full message-handler round trips to let any pending policy action +# fire (one is enough in practice; several make the negative `getheaders` +# assertions meaningful rather than merely early). +SETTLE_ROUNDS = 5 + +# MAX_HEADERS_RESULTS. TryLowWorkHeadersSync() only starts a low-work sync for a +# headers message of exactly this size, and PeerManager::Options::max_headers_result +# is not settable from the command line, so case K has to send a real full batch. +MAX_HEADERS_RESULTS = 2000 + +# Enough required chain work that 2000 regtest headers (worth 2 units each) stay +# far below it, so GetAntiDoSWorkThreshold() is nonzero and case K's headers take +# the low-work branch. Regtest's default is 0, which makes that branch unreachable. +MIN_CHAIN_WORK = "0x1000000" + + +class Blake2bPreforkPeerPolicy(BitcoinTestFramework): + def set_test_params(self): + self.setup_clean_chain = True + # Nodes 2..6 each host one case group and must have no peer other than + # the ones that case adds, so the "no fork-aware peer connected" premise + # the bootstrap fallback keys off is exactly what the test asserts: + # 2 -> cases G and H, 3 -> case I, 4 -> case J, 5 and 6 -> case K. + self.num_nodes = 7 + low_work_args = ["-debug=net", f"-minimumchainwork={MIN_CHAIN_WORK}", "-checkblockindex=0"] + self.extra_args = [["-debug=net"]] * 5 + [low_work_args] * 2 + + def setup_network(self): + # Leave the nodes unconnected; every peer in this test is explicit. + self.setup_nodes() + + def in_ibd(self, node): + return node.getblockchaininfo()["initialblockdownload"] + + def getheaders_count(self, p2p_conn): + # wait_for_getheaders() pops the message, so for "never sent" assertions + # use the monotonic counter instead. + return p2p_conn.message_count["getheaders"] + + def add_manual_p2p_connection(self, node, p2p_conn, *, p2p_idx, services): + """Bring up a real MANUAL connection. + + The `addconnection` RPC used by add_outbound_p2p_connection() cannot + create MANUAL connections, so drive `addnode ... onetry` instead, using + the same accept-connection plumbing. + """ + def connect_cb(address, port): + self.log.debug(f"Manually connecting to {address}:{port}") + node.addnode(f"{address}:{port}", "onetry") + + p2p_conn.peer_accept_connection( + connect_cb=connect_cb, + connect_id=p2p_idx + 1, + net=node.chain, + timeout_factor=node.timeout_factor, + services=services, + # The node is the one dialling out here, so the mock peer has to + # speak whatever transport the node was started with; hardcoding v1 + # makes the BIP324 handshake fail on magic bytes under + # --v2transport. reconnect is False because we advertise exactly + # what we support, so there is no v1 fallback to wait for. + supports_v2_p2p=node.use_v2transport, + reconnect=False, + )() + p2p_conn.wait_for_connect() + node.p2ps.append(p2p_conn) + if node.use_v2transport: + p2p_conn.wait_until(lambda: p2p_conn.v2_state.tried_v2_handshake) + p2p_conn.wait_until(lambda: not p2p_conn.on_connection_send_msg) + p2p_conn.wait_for_verack() + p2p_conn.sync_with_ping() + return p2p_conn + + def newest_peer_id(self, node): + """The node's id for the peer that connected most recently.""" + return max(p["id"] for p in node.getpeerinfo()) + + def disconnect_and_finalize(self, node, p2p_conn, peer_id): + """Disconnect a peer and wait until the node has finished accounting for it. + + Waiting for the connection to drop, or even for getpeerinfo() to stop + listing the peer, is not enough: the node removes it from m_nodes in + DisconnectNodes() but only calls FinalizeNode() -- where the fork-aware + count is decremented -- once the last reference is released, on a later + socket handler pass. A negative assertion made in that window can pass + whether the count is right or wrong, which makes it worthless. The log + line below is the last statement in FinalizeNode(), so waiting for it + pins the assertions that follow to the post-decrement state. + """ + with node.assert_debug_log([f"Cleared nodestate for peer={peer_id}"], timeout=60): + p2p_conn.peer_disconnect() + p2p_conn.wait_for_disconnect() + self.wait_until(lambda: all(p["id"] != peer_id for p in node.getpeerinfo()), timeout=60) + + def settle(self, p2p_conn): + for _ in range(SETTLE_ROUNDS): + p2p_conn.sync_with_ping() + + def unconnecting_block(self, node): + """A solved block whose parent the node has never seen. + + Announcing it is what drives both the unconnecting-headers path and the + unconnecting-compact-block path. + """ + tip = node.getbestblockhash() + tip_time = node.getblockheader(tip)["time"] + height = node.getblockcount() + 1 + # Built on the real tip, but never delivered, so the child below does not + # connect to anything in the node's block index. + orphan_parent = create_block(hashprev=int(tip, 16), coinbase=create_coinbase(height=height), + ntime=tip_time + 1) + orphan_parent.solve() + child = create_block(hashprev=orphan_parent.sha256, coinbase=create_coinbase(height=height + 1), + ntime=tip_time + 2) + child.solve() + return child + + def low_work_headers(self, node, count): + """A full, valid-PoW header chain off `node`'s tip with negligible work. + + Regtest headers are worth 2 units of chain work each, so a batch of this + size lands far under a -minimumchainwork of MIN_CHAIN_WORK and takes + TryLowWorkHeadersSync()'s low-work branch. The headers are never meant to + be accepted into the block index; only the anti-DoS path is under test. + """ + tip = node.getbestblockhash() + prev_hash = int(tip, 16) + prev_time = node.getblockheader(tip)["time"] + height = node.getblockcount() + 1 + headers = [] + for _ in range(count): + block = create_block(hashprev=prev_hash, coinbase=create_coinbase(height=height), + ntime=prev_time + 1) + block.solve() + headers.append(CBlockHeader(block)) + prev_hash = block.sha256 + prev_time = block.nTime + height += 1 + return headers + + def non_continuous_headers(self, node): + """Two headers that do not connect to each other. + + CheckHeadersPoW() rejects the sequence and calls Misbehaving(), which is + how case J gets a fork-aware peer disconnected by the punishment path + rather than by a clean close. Both headers build on the node's tip, so + the first one connects to our block index and only the second one breaks + the chain -- otherwise the message would be routed to the unconnecting + headers path instead and draw no punishment. + """ + tip = node.getbestblockhash() + tip_hash = int(tip, 16) + tip_time = node.getblockheader(tip)["time"] + height = node.getblockcount() + 1 + first = create_block(hashprev=tip_hash, coinbase=create_coinbase(height=height), ntime=tip_time + 1) + first.solve() + second = create_block(hashprev=tip_hash, coinbase=create_coinbase(height=height), ntime=tip_time + 2) + second.solve() + return [CBlockHeader(first), CBlockHeader(second)] + + def run_test(self): + node = self.nodes[0] + + self.log.info("Case A: block data from a pre-fork peer is accepted during our IBD") + assert self.in_ibd(node), "node should start out in IBD" + # Connect the fork-aware control peer first, so that the exclusion is + # in force from the moment the pre-fork peer joins. With no fork-aware + # peer present the bootstrap fallback would (correctly) send the pre-fork + # peer a getheaders, and cases B and E would be testing the fallback + # instead of the exclusion. Cases G and H cover the fallback deliberately, + # on a node kept free of fork-aware peers for the purpose. + control = node.add_p2p_connection(P2PInterface(), services=HF) + control.wait_for_getheaders(timeout=60) + prefork = node.add_p2p_connection(P2PInterface(), services=NON_HF) + + tip_hash = int(node.getbestblockhash(), 16) + tip_time = node.getblockheader(node.getbestblockhash())["time"] + # Back-date the block so accepting it does not end our IBD (cases B and + # E assert we are still in it); it only has to beat median-time-past. + block = create_block(hashprev=tip_hash, coinbase=create_coinbase(height=1), ntime=tip_time + 1) + block.solve() + prefork.send_and_ping(msg_block(block)) + + assert_equal(node.getbestblockhash(), block.hash) + assert_equal(node.getblockcount(), 1) + assert self.in_ibd(node), "node left IBD unexpectedly; later cases would be invalid" + self.log.info("PASS A: tip advanced to the pre-fork peer's block, still in IBD") + + self.log.info("Case B: no getheaders to the pre-fork peer, but a control peer gets one") + self.settle(prefork) + + assert_equal(self.getheaders_count(prefork), 0) + assert self.getheaders_count(control) > 0, "control peer got no getheaders; test would be vacuous" + # The pre-fork peer is excluded from header sync but still connected: the + # policy withholds `getheaders`, it does not drop the peer. + assert self.in_ibd(node) + assert prefork.is_connected + assert_equal(len(node.getpeerinfo()), 2) + self.log.info("PASS B: pre-fork peer got 0 getheaders, control peer got " + f"{self.getheaders_count(control)}, both still connected") + + self.log.info("Case E: an unconnecting headers message from a pre-fork peer draws no getheaders") + # A second control peer. nSyncStarted is already 1 and m_best_header is + # old, so the header-sync trigger passes it over and it starts from a + # count of 0 -- which makes it a clean positive control for the + # unconnecting-headers path specifically, rather than for header sync. + # The first control peer cannot serve here: it was sent a getheaders on + # connect and is inside the HEADERS_RESPONSE_TIME window. + control2 = node.add_p2p_connection(P2PInterface(), services=HF) + self.settle(control2) + assert_equal(self.getheaders_count(control2), 0) + + orphan = self.unconnecting_block(node) + prefork.send_and_ping(msg_headers([CBlockHeader(orphan)])) + control2.send_and_ping(msg_headers([CBlockHeader(orphan)])) + self.settle(prefork) + self.settle(control2) + + assert_equal(self.getheaders_count(prefork), 0) + assert self.getheaders_count(control2) > 0, \ + "control peer got no getheaders for unconnecting headers; the case would be vacuous" + assert self.in_ibd(node) + assert prefork.is_connected + self.log.info("PASS E: unconnecting headers drew 0 getheaders for the pre-fork peer, " + f"{self.getheaders_count(control2)} for the control peer") + + self.log.info("Case D: a manual pre-fork peer is excluded from header sync too") + manual_node = self.nodes[1] + assert self.in_ibd(manual_node) + # Give this node a fork-aware peer FIRST. The exclusion only applies + # while some NODE_BLAKE2B peer is connected; with the manual pre-fork peer + # alone the bootstrap fallback would fire and it would rightly be sent a + # getheaders, so the assertions below would be testing the fallback rather + # than the manual peer's lack of exemption from the exclusion. + manual_control = manual_node.add_p2p_connection(P2PInterface(), services=HF) + manual_control.wait_for_getheaders(timeout=60) + + manual = self.add_manual_p2p_connection(manual_node, P2PInterface(), p2p_idx=0, services=NON_HF) + + peer_info = manual_node.getpeerinfo() + assert_equal(len(peer_info), 2) + manual_info = [p for p in peer_info if p["connection_type"] == "manual"] + assert_equal(len(manual_info), 1) + assert not int(manual_info[0]["services"], 16) & NODE_BLAKE2B + # The premise: a fork-aware alternative exists, so the exclusion is live. + assert any(int(p["services"], 16) & NODE_BLAKE2B for p in peer_info), \ + "no fork-aware peer on this node; case D would be testing the bootstrap fallback" + + self.settle(manual) + assert_equal(self.getheaders_count(manual), 0) + + # Leave IBD and check again, both because the exclusion is meant to hold + # in or out of IBD and because case F's compact-block path only fires out + # of IBD. + self.generate(manual_node, 1, sync_fun=self.no_op) + self.wait_until(lambda: not self.in_ibd(manual_node), timeout=60) + self.settle(manual) + + assert manual.is_connected, "manual pre-fork peer is gone; case F needs it" + assert manual_control.is_connected, "fork-aware control peer was disconnected" + assert_equal(len(manual_node.getpeerinfo()), 2) + assert_equal(self.getheaders_count(manual), 0) + self.log.info("PASS D: manual pre-fork peer never asked for headers, in or out of IBD, " + "while a fork-aware peer was available") + + self.log.info("Case F: an unconnecting compact block from a pre-fork peer draws no getheaders") + # The compact block handler only requests deeper headers when we are out + # of IBD, so this reuses case D's peer on case D's node, which case D has + # already taken out of IBD. Case D's fork-aware control peer is still + # connected, so the exclusion (rather than the fallback) is what is under + # test here too. + assert not self.in_ibd(manual_node) + assert manual_control.is_connected, \ + "fork-aware peer gone; case F would be testing the bootstrap fallback" + cmpct = HeaderAndShortIDs() + cmpct.initialize_from_block(self.unconnecting_block(manual_node)) + manual.send_and_ping(msg_cmpctblock(cmpct.to_p2p())) + self.settle(manual) + + assert manual.is_connected + assert_equal(self.getheaders_count(manual), 0) + self.log.info("PASS F: unconnecting compact block drew 0 getheaders for the pre-fork peer") + + self.log.info("Case G: with no fork-aware peer at all, a pre-fork peer does get getheaders") + # A node of its own, still on the clean chain and with no peers yet, so + # "we have zero NODE_BLAKE2B peers" is exactly true rather than incidental. + boot_node = self.nodes[2] + assert self.in_ibd(boot_node) + assert_equal(len(boot_node.getpeerinfo()), 0) + + boot_prefork = boot_node.add_p2p_connection(P2PInterface(), services=NON_HF) + boot_info = boot_node.getpeerinfo() + assert_equal(len(boot_info), 1) + assert not int(boot_info[0]["services"], 16) & NODE_BLAKE2B, \ + "the only peer must lack NODE_BLAKE2B for the fallback premise to hold" + + # The whole point of the fallback: without it this node would sit at zero + # headers indefinitely, which is what the unconditional exclusion caused. + boot_prefork.wait_for_getheaders(timeout=60) + assert self.in_ibd(boot_node) + assert boot_prefork.is_connected + self.log.info("PASS G: pre-fork peer received " + f"{self.getheaders_count(boot_prefork)} getheaders with no fork-aware peer present") + + self.log.info("Case H: the fallback turns off once a fork-aware peer connects") + prefork_baseline = self.getheaders_count(boot_prefork) + boot_control = boot_node.add_p2p_connection(P2PInterface(), services=HF) + # It must actually become the sync source, not merely be connected. This + # only happens because the pre-fork peer hands back the header-sync slot + # once the fallback stops applying to it; otherwise nSyncStarted stays 1 + # and this would block until the headers-sync timeout. + boot_control.wait_for_getheaders(timeout=60) + + # As in case E, the positive control has to be a peer that is not already + # inside its HEADERS_RESPONSE_TIME window -- boot_control just received a + # getheaders, so a fresh fork-aware peer serves instead. nSyncStarted is + # 1 and m_best_header is old, so it is passed over by the header-sync + # trigger and starts from a count of 0. + boot_control2 = boot_node.add_p2p_connection(P2PInterface(), services=HF) + self.settle(boot_control2) + assert_equal(self.getheaders_count(boot_control2), 0) + + # Actively try to draw a fresh getheaders from each. Without a stimulus + # the negative assertion would be vacuous: normal timing alone suppresses + # a repeat request regardless of policy. + boot_orphan = self.unconnecting_block(boot_node) + boot_prefork.send_and_ping(msg_headers([CBlockHeader(boot_orphan)])) + boot_control2.send_and_ping(msg_headers([CBlockHeader(boot_orphan)])) + self.settle(boot_prefork) + self.settle(boot_control2) + + assert_equal(self.getheaders_count(boot_prefork), prefork_baseline) + assert self.getheaders_count(boot_control2) > 0, \ + "the fork-aware peer drew no getheaders either; the negative assertion would be vacuous" + assert boot_prefork.is_connected, "pre-fork peer disconnected; the policy only withholds getheaders" + assert self.in_ibd(boot_node) + self.log.info("PASS H: pre-fork peer stayed at " + f"{prefork_baseline} getheaders once a fork-aware peer connected, while the " + f"fork-aware control got {self.getheaders_count(boot_control2)}") + + self.run_case_i() + self.run_case_j() + self.run_case_k() + + def run_case_i(self): + self.log.info("Case I: the fallback comes back on when the last fork-aware peer goes away") + # The mirror image of case H, on a node of its own so that "the last + # fork-aware peer" is exact. Nothing in the test pokes the node after the + # disconnect: FinalizeNode() has to both uncount the peer and give back + # the nSyncStarted slot it was holding, and the next SendMessages tick + # has to re-run the header-sync trigger on its own. If the count did not + # come back down to zero, this node would be left with a peer it refuses + # to sync from and no other header source at all -- a permanently stuck + # node, which is the failure this case exists to catch. + node = self.nodes[3] + assert self.in_ibd(node) + assert_equal(len(node.getpeerinfo()), 0) + + fork_aware = node.add_p2p_connection(P2PInterface(), services=HF) + fork_aware_id = self.newest_peer_id(node) + # It must actually be the header sync source, not merely connected, or + # its departure would not be releasing anything. + fork_aware.wait_for_getheaders(timeout=60) + + prefork = node.add_p2p_connection(P2PInterface(), services=NON_HF) + self.settle(prefork) + assert_equal(self.getheaders_count(prefork), 0) + assert_equal(len(node.getpeerinfo()), 2) + + self.disconnect_and_finalize(node, fork_aware, fork_aware_id) + assert_equal(len(node.getpeerinfo()), 1) + assert not int(node.getpeerinfo()[0]["services"], 16) & NODE_BLAKE2B + + # No stimulus, no reconnect, no second peer: the node has to resume + # header sync from the pre-fork peer by itself. The peer has never been + # sent a getheaders, so its HEADERS_RESPONSE_TIME window is not a factor + # and policy is the only thing that could still be suppressing one. + prefork.wait_for_getheaders(timeout=60) + assert prefork.is_connected + assert self.in_ibd(node) + self.log.info("PASS I: pre-fork peer went from 0 to " + f"{self.getheaders_count(prefork)} getheaders after the fork-aware peer left, " + "with no external stimulus") + + def run_case_j(self): + self.log.info("Case J: the fork-aware peer count survives mixed orders and disconnect paths") + # m_fork_aware_header_sync_peers is not exposed over RPC, so every step + # below is checked through the one thing it drives: whether a pre-fork + # peer draws a getheaders. The pre-fork peer used for that (`prefork_b`) + # is never sent one until the final step, so its HEADERS_RESPONSE_TIME + # window is never in play -- policy is the only possible suppressor, and + # a single SendMessages tick is enough for a permitted request to go out. + # + # The negative steps are not vacuous: the first and last steps show this + # same node sending a getheaders to a pre-fork peer whenever the count is + # genuinely zero. + node = self.nodes[4] + assert self.in_ibd(node) + assert_equal(len(node.getpeerinfo()), 0) + + # Step 1: pre-fork peer first, no fork-aware peer at all -> count 0, the + # fallback applies. Deliberately the reverse of the order cases G and H + # use, so the increment is exercised on a node that already had a + # pre-fork peer holding the sync slot. + prefork_a = node.add_p2p_connection(P2PInterface(), services=NON_HF) + prefork_a_id = self.newest_peer_id(node) + prefork_a.wait_for_getheaders(timeout=60) + self.log.info(" J1: count 0, pre-fork peer got getheaders") + + # Step 2: first fork-aware peer -> count 1. It takes over the sync slot, + # which is only possible because prefork_a hands it back. + fork_aware_a = node.add_p2p_connection(P2PInterface(), services=HF) + fork_aware_a_id = self.newest_peer_id(node) + fork_aware_a.wait_for_getheaders(timeout=60) + + # Step 3: a second pre-fork peer, connected while the count is 1, is + # excluded and stays at zero getheaders. + prefork_b = node.add_p2p_connection(P2PInterface(), services=NON_HF) + self.settle(prefork_b) + assert_equal(self.getheaders_count(prefork_b), 0) + self.log.info(" J2: count 1, second pre-fork peer got 0 getheaders") + + # Step 4: disconnect a peer that was never counted. A decrement here + # would take the count to 0 and wrongly re-open the fallback. + # + # Note what this step does and does not settle on its own: fork_aware_a + # still holds the single nSyncStarted slot, so even a wrongly re-opened + # fallback cannot produce a getheaders for prefork_b yet. The spurious + # decrement is therefore caught one step later, the first moment the slot + # is free -- which is exactly what step 5 provides. Steps 4 and 5 are one + # check in two parts, not two independent ones. (Verified by mutation: + # making the decrement unconditional fails this case at step 5.) + self.disconnect_and_finalize(node, prefork_a, prefork_a_id) + self.settle(prefork_b) + assert_equal(self.getheaders_count(prefork_b), 0) + self.log.info(" J3: pre-fork peer disconnected; still no getheaders for the other pre-fork peer") + + # Step 5: a second fork-aware peer -> count 2, then take one away with a + # clean disconnect -> count 1, freeing the sync slot. The count must now + # be 1 both because fork_aware_b is still connected and because step 4's + # peer was never counted; either error shows up here as prefork_b + # suddenly being allowed. + fork_aware_b = node.add_p2p_connection(P2PInterface(), services=HF) + self.settle(fork_aware_b) + self.disconnect_and_finalize(node, fork_aware_a, fork_aware_a_id) + self.settle(prefork_b) + assert_equal(self.getheaders_count(prefork_b), 0) + assert fork_aware_b.is_connected + self.log.info(" J4: with the sync slot free and one fork-aware peer left, " + "the pre-fork peer is still excluded") + + # Step 6: remove the last fork-aware peer through the punishment path + # instead of a clean close -- Misbehaving() sets m_should_discourage and + # MaybeDiscourageAndDisconnect() drops the connection. It is a different + # route into DisconnectNodes(), but it still ends at FinalizeNode(), and + # the count must reach 0 all the same. (127.0.0.1 is treated as local, so + # the peer is disconnected but not discouraged; nothing here depends on + # which of the two happens.) + fork_aware_b.send_message(msg_headers(self.non_continuous_headers(node))) + fork_aware_b.wait_for_disconnect(timeout=60) + self.wait_until(lambda: len(node.getpeerinfo()) == 1, timeout=60) + assert_equal(len(node.getpeerinfo()), 1) + assert not int(node.getpeerinfo()[0]["services"], 16) & NODE_BLAKE2B + + prefork_b.wait_for_getheaders(timeout=60) + assert prefork_b.is_connected + assert self.in_ibd(node) + self.log.info(" J5: misbehavior disconnect took count 1 -> 0, pre-fork peer got " + f"{self.getheaders_count(prefork_b)} getheaders") + self.log.info("PASS J: the count tracked every connect and every disconnect path exercised") + + def run_case_k(self): + self.log.info("Case K: TryLowWorkHeadersSync honors the same predicate") + # The low-work headers-sync entry point is a fifth place the policy has + # to hold, and the only one not reachable with a handful of headers: it + # needs a headers message of exactly MAX_HEADERS_RESULTS whose claimed + # work is below GetAntiDoSWorkThreshold(). On regtest that threshold is + # zero unless -minimumchainwork is set, which is how + # p2p_headers_sync_with_minchainwork reaches the same branch; nodes 5 and + # 6 are started that way. Both are clean-chain nodes at the same genesis + # block, so one batch of headers serves for both. + allowed_node = self.nodes[5] + excluded_node = self.nodes[6] + assert_equal(len(allowed_node.getpeerinfo()), 0) + assert_equal(len(excluded_node.getpeerinfo()), 0) + assert_equal(allowed_node.getbestblockhash(), excluded_node.getbestblockhash()) + + headers = self.low_work_headers(allowed_node, MAX_HEADERS_RESULTS) + assert_equal(len(headers), MAX_HEADERS_RESULTS) + + self.log.info(" K1: under the bootstrap fallback, a pre-fork peer may start a low-work sync") + boot_prefork = allowed_node.add_p2p_connection(P2PInterface(), services=NON_HF) + boot_prefork.wait_for_getheaders(timeout=60) + with allowed_node.assert_debug_log(expected_msgs=["Initial headers sync started with peer="], + timeout=60): + boot_prefork.send_message(msg_headers(headers)) + boot_prefork.sync_with_ping(timeout=60) + self.log.info(" K1: low-work sync started from the pre-fork peer, as the fallback requires") + + self.log.info(" K2: with a fork-aware peer connected, the same batch is refused") + control = excluded_node.add_p2p_connection(P2PInterface(), services=HF) + control.wait_for_getheaders(timeout=60) + prefork = excluded_node.add_p2p_connection(P2PInterface(), services=NON_HF) + self.settle(prefork) + assert_equal(self.getheaders_count(prefork), 0) + + with excluded_node.assert_debug_log( + expected_msgs=[f"Ignoring low-work chain (height={MAX_HEADERS_RESULTS})"], + unexpected_msgs=["Initial headers sync started with peer="], + timeout=60): + prefork.send_message(msg_headers(headers)) + prefork.sync_with_ping(timeout=60) + + # Positive control on the same node with the same headers: the batch is + # otherwise perfectly capable of starting a low-work sync, so the refusal + # above is the policy and nothing else. + with excluded_node.assert_debug_log(expected_msgs=["Initial headers sync started with peer="], + timeout=60): + control.send_message(msg_headers(headers)) + control.sync_with_ping(timeout=60) + + assert prefork.is_connected, "pre-fork peer disconnected; the policy only withholds getheaders" + self.log.info("PASS K: the low-work sync entry point allowed the pre-fork peer only under the " + "fallback, and refused it while a fork-aware peer was connected") + + +if __name__ == '__main__': + Blake2bPreforkPeerPolicy(__file__).main() diff --git a/test/functional/p2p_segwit.py b/test/functional/p2p_segwit.py index 004a2959afff..1c32e45cc4f4 100755 --- a/test/functional/p2p_segwit.py +++ b/test/functional/p2p_segwit.py @@ -28,6 +28,7 @@ MSG_WITNESS_FLAG, MSG_WITNESS_TX, MSG_WTX, + NODE_BLAKE2B, NODE_NETWORK, NODE_WITNESS, WITNESS_SCALE_FACTOR, @@ -245,8 +246,9 @@ def run_test(self): # Setup the p2p connections # self.test_node sets P2P_SERVICES, i.e. NODE_WITNESS | NODE_NETWORK self.test_node = self.nodes[0].add_p2p_connection(TestP2PConn(), services=P2P_SERVICES) - # self.old_node sets only NODE_NETWORK - self.old_node = self.nodes[0].add_p2p_connection(TestP2PConn(), services=NODE_NETWORK) + # self.old_node sets only NODE_NETWORK (plus NODE_BLAKE2B, without which it + # would be disconnected as a pre-fork peer once we are out of IBD) + self.old_node = self.nodes[0].add_p2p_connection(TestP2PConn(), services=NODE_NETWORK | NODE_BLAKE2B) # self.std_node is for testing node1 (requires standard txs) self.std_node = self.nodes[1].add_p2p_connection(TestP2PConn(), services=P2P_SERVICES) # self.std_wtx_node is for testing node1 with wtxid relay diff --git a/test/functional/p2p_sendheaders.py b/test/functional/p2p_sendheaders.py index db706556d82f..3fd745b55bc4 100755 --- a/test/functional/p2p_sendheaders.py +++ b/test/functional/p2p_sendheaders.py @@ -83,6 +83,7 @@ from test_framework.messages import CInv from test_framework.p2p import ( CBlockHeader, + NODE_BLAKE2B, NODE_WITNESS, P2PInterface, p2p_lock, @@ -226,7 +227,8 @@ def run_test(self): inv_node = self.nodes[0].add_p2p_connection(BaseNode()) # Make sure NODE_NETWORK is not set for test_node, so no block download # will occur outside of direct fetching - test_node = self.nodes[0].add_p2p_connection(BaseNode(), services=NODE_WITNESS) + # NODE_BLAKE2B is kept so the peer is not dropped as a pre-fork peer once IBD ends. + test_node = self.nodes[0].add_p2p_connection(BaseNode(), services=NODE_WITNESS | NODE_BLAKE2B) self.test_null_locators(test_node, inv_node) self.test_nonnull_locators(test_node, inv_node) diff --git a/test/functional/rpc_net.py b/test/functional/rpc_net.py index c05f3e07e76d..4d3a8c2a8781 100755 --- a/test/functional/rpc_net.py +++ b/test/functional/rpc_net.py @@ -283,11 +283,12 @@ def test_addnode_getaddednodeinfo(self): def test_service_flags(self): self.log.info("Test service flags") - self.nodes[0].add_p2p_connection(P2PInterface(), services=(1 << 4) | (1 << 63)) + # NODE_BLAKE2B is included so the peer is not disconnected as a pre-fork peer. + self.nodes[0].add_p2p_connection(P2PInterface(), services=(1 << 4) | NODE_BLAKE2B | (1 << 63)) if self.options.v2transport: - assert_equal(['UNKNOWN[2^4]', 'P2P_V2', 'UNKNOWN[2^63]'], self.nodes[0].getpeerinfo()[-1]['servicesnames']) + assert_equal(['UNKNOWN[2^4]', 'P2P_V2', 'BLAKE2B?', 'UNKNOWN[2^63]'], self.nodes[0].getpeerinfo()[-1]['servicesnames']) else: - assert_equal(['UNKNOWN[2^4]', 'UNKNOWN[2^63]'], self.nodes[0].getpeerinfo()[-1]['servicesnames']) + assert_equal(['UNKNOWN[2^4]', 'BLAKE2B?', 'UNKNOWN[2^63]'], self.nodes[0].getpeerinfo()[-1]['servicesnames']) self.nodes[0].disconnect_p2ps() def test_getnodeaddresses(self): diff --git a/test/functional/test_runner.py b/test/functional/test_runner.py index e912af0f5cfc..e4ec9c17693b 100755 --- a/test/functional/test_runner.py +++ b/test/functional/test_runner.py @@ -144,6 +144,8 @@ 'p2p_dns_seeds.py', 'p2p_blake2b_dns_immediate.py', 'p2p_blake2b_outbound_preference.py', + 'p2p_blake2b_prefork_peer_policy.py --v1transport', + 'p2p_blake2b_prefork_peer_policy.py --v2transport', 'wallet_groups.py --legacy-wallet', 'wallet_groups.py --descriptors', 'p2p_blockfilters.py',