Skip to content

Commit 3d216ba

Browse files
committed
Merge bitcoin/bitcoin#29279: test: p2p: check disconnect due to lack of desirable service flags
2f23987 test: p2p: check limited peers desirability (depending on best block depth) (Sebastian Falbesoner) c4a67d3 test: p2p: check disconnect due to lack of desirable service flags (Sebastian Falbesoner) 405ac81 test: p2p: support disconnect waiting for `add_outbound_p2p_connection` (Sebastian Falbesoner) Pull request description: This PR adds missing test coverage for disconnecting peers which don't offer the desirable service flags in their VERSION message: https://github.com/bitcoin/bitcoin/blob/5f3a0574c45477288bc678b15f24940486084576/src/net_processing.cpp#L3384-L3389 This check is relevant for the connection types "outbound-full-relay", "block-relay-only" and "addr-fetch" (see `CNode::ExpectServicesFromConn(...)`). Feeler connections always disconnect, which is also tested here. In lack of finding a proper file where this test would fit in, I created a new one. Happy to take suggestions there. ACKs for top commit: davidgumberg: reACK bitcoin/bitcoin@2f23987 itornaza: tested ACK 2f23987 fjahr: re-utACK 2f23987 cbergqvist: re ACK 2f23987 stratospher: tested ACK 2f23987. 🚀 Tree-SHA512: cf75d9d4379d0f34fa1e13152e6a8d93cd51b9573466ab3a2fec32dc3e1ac49b174bd1063cae558bc736b111c8a6e7058b1b57a496df56255221bf367d29eb5d
2 parents 479ecc0 + 2f23987 commit 3d216ba

File tree

4 files changed

+92
-2
lines changed

4 files changed

+92
-2
lines changed

test/functional/p2p_handshake.py

Lines changed: 87 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,87 @@
1+
#!/usr/bin/env python3
2+
# Copyright (c) 2024 The Bitcoin Core developers
3+
# Distributed under the MIT software license, see the accompanying
4+
# file COPYING or http://www.opensource.org/licenses/mit-license.php.
5+
"""
6+
Test P2P behaviour during the handshake phase (VERSION, VERACK messages).
7+
"""
8+
import itertools
9+
import time
10+
11+
from test_framework.test_framework import BitcoinTestFramework
12+
from test_framework.messages import (
13+
NODE_NETWORK,
14+
NODE_NETWORK_LIMITED,
15+
NODE_NONE,
16+
NODE_P2P_V2,
17+
NODE_WITNESS,
18+
)
19+
from test_framework.p2p import P2PInterface
20+
21+
22+
# Desirable service flags for outbound non-pruned and pruned peers. Note that
23+
# the desirable service flags for pruned peers are dynamic and only apply if
24+
# 1. the peer's service flag NODE_NETWORK_LIMITED is set *and*
25+
# 2. the local chain is close to the tip (<24h)
26+
DESIRABLE_SERVICE_FLAGS_FULL = NODE_NETWORK | NODE_WITNESS
27+
DESIRABLE_SERVICE_FLAGS_PRUNED = NODE_NETWORK_LIMITED | NODE_WITNESS
28+
29+
30+
class P2PHandshakeTest(BitcoinTestFramework):
31+
def set_test_params(self):
32+
self.num_nodes = 1
33+
34+
def add_outbound_connection(self, node, connection_type, services, wait_for_disconnect):
35+
peer = node.add_outbound_p2p_connection(
36+
P2PInterface(), p2p_idx=0, wait_for_disconnect=wait_for_disconnect,
37+
connection_type=connection_type, services=services,
38+
supports_v2_p2p=self.options.v2transport, advertise_v2_p2p=self.options.v2transport)
39+
if not wait_for_disconnect:
40+
# check that connection is alive past the version handshake and disconnect manually
41+
peer.sync_with_ping()
42+
peer.peer_disconnect()
43+
peer.wait_for_disconnect()
44+
45+
def test_desirable_service_flags(self, node, service_flag_tests, desirable_service_flags, expect_disconnect):
46+
"""Check that connecting to a peer either fails or succeeds depending on its offered
47+
service flags in the VERSION message. The test is exercised for all relevant
48+
outbound connection types where the desirable service flags check is done."""
49+
CONNECTION_TYPES = ["outbound-full-relay", "block-relay-only", "addr-fetch"]
50+
for conn_type, services in itertools.product(CONNECTION_TYPES, service_flag_tests):
51+
if self.options.v2transport:
52+
services |= NODE_P2P_V2
53+
expected_result = "disconnect" if expect_disconnect else "connect"
54+
self.log.info(f' - services 0x{services:08x}, type "{conn_type}" [{expected_result}]')
55+
if expect_disconnect:
56+
assert (services & desirable_service_flags) != desirable_service_flags
57+
expected_debug_log = f'does not offer the expected services ' \
58+
f'({services:08x} offered, {desirable_service_flags:08x} expected)'
59+
with node.assert_debug_log([expected_debug_log]):
60+
self.add_outbound_connection(node, conn_type, services, wait_for_disconnect=True)
61+
else:
62+
assert (services & desirable_service_flags) == desirable_service_flags
63+
self.add_outbound_connection(node, conn_type, services, wait_for_disconnect=False)
64+
65+
def run_test(self):
66+
node = self.nodes[0]
67+
self.log.info("Check that lacking desired service flags leads to disconnect (non-pruned peers)")
68+
self.test_desirable_service_flags(node, [NODE_NONE, NODE_NETWORK, NODE_WITNESS],
69+
DESIRABLE_SERVICE_FLAGS_FULL, expect_disconnect=True)
70+
self.test_desirable_service_flags(node, [NODE_NETWORK | NODE_WITNESS],
71+
DESIRABLE_SERVICE_FLAGS_FULL, expect_disconnect=False)
72+
73+
self.log.info("Check that limited peers are only desired if the local chain is close to the tip (<24h)")
74+
node.setmocktime(int(time.time()) + 25 * 3600) # tip outside the 24h window, should fail
75+
self.test_desirable_service_flags(node, [NODE_NETWORK_LIMITED | NODE_WITNESS],
76+
DESIRABLE_SERVICE_FLAGS_FULL, expect_disconnect=True)
77+
node.setmocktime(int(time.time()) + 23 * 3600) # tip inside the 24h window, should succeed
78+
self.test_desirable_service_flags(node, [NODE_NETWORK_LIMITED | NODE_WITNESS],
79+
DESIRABLE_SERVICE_FLAGS_PRUNED, expect_disconnect=False)
80+
81+
self.log.info("Check that feeler connections get disconnected immediately")
82+
with node.assert_debug_log([f"feeler connection completed"]):
83+
self.add_outbound_connection(node, "feeler", NODE_NONE, wait_for_disconnect=True)
84+
85+
86+
if __name__ == '__main__':
87+
P2PHandshakeTest().main()

test/functional/test_framework/messages.py

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -46,6 +46,7 @@
4646
MAX_HEADERS_RESULTS = 2000 # Number of headers sent in one getheaders result
4747
MAX_INV_SIZE = 50000 # Maximum number of entries in an 'inv' protocol message
4848

49+
NODE_NONE = 0
4950
NODE_NETWORK = (1 << 0)
5051
NODE_BLOOM = (1 << 2)
5152
NODE_WITNESS = (1 << 3)

test/functional/test_framework/test_node.py

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -724,7 +724,7 @@ def add_p2p_connection(self, p2p_conn, *, wait_for_verack=True, send_version=Tru
724724

725725
return p2p_conn
726726

727-
def add_outbound_p2p_connection(self, p2p_conn, *, wait_for_verack=True, p2p_idx, connection_type="outbound-full-relay", supports_v2_p2p=None, advertise_v2_p2p=None, **kwargs):
727+
def add_outbound_p2p_connection(self, p2p_conn, *, wait_for_verack=True, wait_for_disconnect=False, p2p_idx, connection_type="outbound-full-relay", supports_v2_p2p=None, advertise_v2_p2p=None, **kwargs):
728728
"""Add an outbound p2p connection from node. Must be an
729729
"outbound-full-relay", "block-relay-only", "addr-fetch" or "feeler" connection.
730730
@@ -771,7 +771,7 @@ def addconnection_callback(address, port):
771771
if reconnect:
772772
p2p_conn.wait_for_reconnect()
773773

774-
if connection_type == "feeler":
774+
if connection_type == "feeler" or wait_for_disconnect:
775775
# feeler connections are closed as soon as the node receives a `version` message
776776
p2p_conn.wait_until(lambda: p2p_conn.message_count["version"] == 1, check_connected=False)
777777
p2p_conn.wait_until(lambda: not p2p_conn.is_connected, check_connected=False)

test/functional/test_runner.py

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -395,6 +395,8 @@
395395
'rpc_getdescriptorinfo.py',
396396
'rpc_mempool_info.py',
397397
'rpc_help.py',
398+
'p2p_handshake.py',
399+
'p2p_handshake.py --v2transport',
398400
'feature_dirsymlinks.py',
399401
'feature_help.py',
400402
'feature_shutdown.py',

0 commit comments

Comments
 (0)