From c709df74cc4b9f99c86bb7ac60069a923d2bac52 Mon Sep 17 00:00:00 2001 From: Kittywhiskers Van Gogh <63189531+kwvg@users.noreply.github.com> Date: Mon, 14 Oct 2024 21:18:17 +0000 Subject: [PATCH 01/12] merge bitcoin#20524: Move MIN_VERSION_SUPPORTED to p2p.py continuation of cbff29a6 (dash#6067) --- test/functional/test_framework/test_node.py | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/test/functional/test_framework/test_node.py b/test/functional/test_framework/test_node.py index 6aa1bbd844..df020736b7 100755 --- a/test/functional/test_framework/test_node.py +++ b/test/functional/test_framework/test_node.py @@ -27,6 +27,7 @@ from .descriptors import descsum_create from .p2p import P2P_SUBVERSION from .util import ( MAX_NODES, + assert_equal, append_config, delete_cookie_file, get_auth_cookie, @@ -637,6 +638,11 @@ class TestNode(): # in comparison to the upside of making tests less fragile and unexpected intermittent errors less likely. p2p_conn.sync_with_ping() + # Consistency check that the Dash Core has received our user agent string. This checks the + # node's newest peer. It could be racy if another Dash Core node has connected since we opened + # our connection, but we don't expect that to happen. + assert_equal(self.getpeerinfo()[-1]['subver'], p2p_conn.strSubVer) + return p2p_conn def add_outbound_p2p_connection(self, p2p_conn, *, p2p_idx, connection_type="outbound-full-relay", **kwargs): From b7c0030d3d8d070c4346aa6aaa3daa73578d8675 Mon Sep 17 00:00:00 2001 From: Kittywhiskers Van Gogh <63189531+kwvg@users.noreply.github.com> Date: Mon, 19 Sep 2022 17:11:32 +0300 Subject: [PATCH 02/12] partial bitcoin#23443: Erlay support signaling contains: - e56d1d2a (changes to `test_node.py`) --- test/functional/test_framework/test_node.py | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/test/functional/test_framework/test_node.py b/test/functional/test_framework/test_node.py index df020736b7..3d1140e1d9 100755 --- a/test/functional/test_framework/test_node.py +++ b/test/functional/test_framework/test_node.py @@ -645,7 +645,7 @@ class TestNode(): return p2p_conn - def add_outbound_p2p_connection(self, p2p_conn, *, p2p_idx, connection_type="outbound-full-relay", **kwargs): + def add_outbound_p2p_connection(self, p2p_conn, *, wait_for_verack=True, p2p_idx, connection_type="outbound-full-relay", **kwargs): """Add an outbound p2p connection from node. Must be an "outbound-full-relay", "block-relay-only", "addr-fetch" or "feeler" connection. @@ -667,8 +667,9 @@ class TestNode(): p2p_conn.wait_for_connect() self.p2ps.append(p2p_conn) - p2p_conn.wait_for_verack() - p2p_conn.sync_with_ping() + if wait_for_verack: + p2p_conn.wait_for_verack() + p2p_conn.sync_with_ping() return p2p_conn From 5bf245b4a0d2400f89ade6e15c7c562201e30c36 Mon Sep 17 00:00:00 2001 From: Kittywhiskers Van Gogh <63189531+kwvg@users.noreply.github.com> Date: Mon, 14 Oct 2024 18:30:40 +0000 Subject: [PATCH 03/12] partial bitcoin#26448: fix intermittent failure in p2p_sendtxrcncl.py contains: - 74d97531 (changes to `test_node.py`) --- test/functional/test_framework/test_node.py | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/test/functional/test_framework/test_node.py b/test/functional/test_framework/test_node.py index 3d1140e1d9..12f05b6ec4 100755 --- a/test/functional/test_framework/test_node.py +++ b/test/functional/test_framework/test_node.py @@ -651,6 +651,10 @@ class TestNode(): This method adds the p2p connection to the self.p2ps list and returns the connection to the caller. + + p2p_idx must be different for simultaneously connected peers. When reusing it for the next peer + after disconnecting the previous one, it is necessary to wait for the disconnect to finish to avoid + a race condition. """ def addconnection_callback(address, port): From 1bf135bbc91d24d5f3cf947148caa37980d6c9e5 Mon Sep 17 00:00:00 2001 From: Kittywhiskers Van Gogh <63189531+kwvg@users.noreply.github.com> Date: Tue, 15 Oct 2024 07:11:47 +0000 Subject: [PATCH 04/12] merge bitcoin#26553: Fix intermittent failure in rpc_net.py --- test/functional/rpc_net.py | 5 +++-- test/functional/test_framework/test_node.py | 3 ++- 2 files changed, 5 insertions(+), 3 deletions(-) diff --git a/test/functional/rpc_net.py b/test/functional/rpc_net.py index e8d2607763..d7d7d5d0c4 100755 --- a/test/functional/rpc_net.py +++ b/test/functional/rpc_net.py @@ -114,7 +114,7 @@ class NetTest(DashTestFramework): no_version_peer_id = 3 no_version_peer_conntime = self.mocktime with self.nodes[0].assert_debug_log([f"Added connection peer={no_version_peer_id}"]): - self.nodes[0].add_p2p_connection(P2PInterface(), send_version=False, wait_for_verack=False) + no_version_peer = self.nodes[0].add_p2p_connection(P2PInterface(), send_version=False, wait_for_verack=False) peer_info = self.nodes[0].getpeerinfo()[no_version_peer_id] peer_info.pop("addr") peer_info.pop("addrbind") @@ -155,7 +155,8 @@ class NetTest(DashTestFramework): "version": 0, }, ) - self.nodes[0].disconnect_p2ps() + no_version_peer.peer_disconnect() + self.wait_until(lambda: len(self.nodes[0].getpeerinfo()) == 3) def test_getnettotals(self): self.log.info("Test getnettotals") diff --git a/test/functional/test_framework/test_node.py b/test/functional/test_framework/test_node.py index 12f05b6ec4..fb59e6c206 100755 --- a/test/functional/test_framework/test_node.py +++ b/test/functional/test_framework/test_node.py @@ -682,7 +682,8 @@ class TestNode(): return len([peer for peer in self.getpeerinfo() if P2P_SUBVERSION % "" in peer['subver']]) def disconnect_p2ps(self): - """Close all p2p connections to the node.""" + """Close all p2p connections to the node. + Use only after each p2p has sent a version message to ensure the wait works.""" for p in self.p2ps: p.peer_disconnect() From fffe6e716b509e87d5dd30b27cfd2df8ecd5a0e7 Mon Sep 17 00:00:00 2001 From: Kittywhiskers Van Gogh <63189531+kwvg@users.noreply.github.com> Date: Wed, 28 Jun 2023 10:15:41 +0200 Subject: [PATCH 05/12] merge bitcoin#27986: remove race in the user-agent reception check --- test/functional/test_framework/test_node.py | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/test/functional/test_framework/test_node.py b/test/functional/test_framework/test_node.py index fb59e6c206..3da025570f 100755 --- a/test/functional/test_framework/test_node.py +++ b/test/functional/test_framework/test_node.py @@ -638,10 +638,13 @@ class TestNode(): # in comparison to the upside of making tests less fragile and unexpected intermittent errors less likely. p2p_conn.sync_with_ping() - # Consistency check that the Dash Core has received our user agent string. This checks the - # node's newest peer. It could be racy if another Dash Core node has connected since we opened - # our connection, but we don't expect that to happen. - assert_equal(self.getpeerinfo()[-1]['subver'], p2p_conn.strSubVer) + # Consistency check that the node received our user agent string. + # Find our connection in getpeerinfo by our address:port, as it is unique. + sockname = p2p_conn._transport.get_extra_info("socket").getsockname() + our_addr_and_port = f"{sockname[0]}:{sockname[1]}" + info = [peer for peer in self.getpeerinfo() if peer["addr"] == our_addr_and_port] + assert_equal(len(info), 1) + assert_equal(info[0]["subver"], p2p_conn.strSubVer) return p2p_conn From deaee147b7ef4a28a304ee16b92e3ca0712226bc Mon Sep 17 00:00:00 2001 From: Kittywhiskers Van Gogh <63189531+kwvg@users.noreply.github.com> Date: Tue, 15 Oct 2024 11:19:07 +0000 Subject: [PATCH 06/12] merge bitcoin-core/gui#754: Add BIP324-specific labels to peer details --- src/qt/forms/debugwindow.ui | 168 +++++++++++++++++++++++------------- src/qt/rpcconsole.cpp | 23 ++++- 2 files changed, 131 insertions(+), 60 deletions(-) diff --git a/src/qt/forms/debugwindow.ui b/src/qt/forms/debugwindow.ui index d6ea9fd3eb..c45f6bcaeb 100644 --- a/src/qt/forms/debugwindow.ui +++ b/src/qt/forms/debugwindow.ui @@ -977,17 +977,17 @@ - + - The network protocol this peer is connected through: IPv4, IPv6, Onion, I2P, or CJDNS. + The transport layer version: %1 - Network + Transport - + IBeamCursor @@ -1003,14 +1003,17 @@ - + + + The BIP324 session ID string in hex, if any. + - Permissions + Session ID - + IBeamCursor @@ -1026,17 +1029,17 @@ - + - The direction and type of peer connection: %1 + The network protocol this peer is connected through: IPv4, IPv6, Onion, I2P, or CJDNS. - Direction/Type + Network - + IBeamCursor @@ -1052,14 +1055,14 @@ - + - Version + Permissions - + IBeamCursor @@ -1075,13 +1078,62 @@ + + + The direction and type of peer connection: %1 + + + Direction/Type + + + + + + + IBeamCursor + + + N/A + + + Qt::PlainText + + + Qt::LinksAccessibleByMouse|Qt::TextSelectableByKeyboard|Qt::TextSelectableByMouse + + + + + + + Version + + + + + + + IBeamCursor + + + N/A + + + Qt::PlainText + + + Qt::LinksAccessibleByMouse|Qt::TextSelectableByKeyboard|Qt::TextSelectableByMouse + + + + User Agent - + IBeamCursor @@ -1100,14 +1152,14 @@ - + Services - + IBeamCursor @@ -1123,7 +1175,7 @@ - + Whether the peer requested us to relay transactions. @@ -1133,7 +1185,7 @@ - + IBeamCursor @@ -1149,7 +1201,7 @@ - + High bandwidth BIP152 compact block relay: %1 @@ -1159,7 +1211,7 @@ - + IBeamCursor @@ -1175,14 +1227,14 @@ - + Starting Block - + IBeamCursor @@ -1198,14 +1250,14 @@ - + Synced Headers - + IBeamCursor @@ -1221,14 +1273,14 @@ - + Synced Blocks - + IBeamCursor @@ -1244,14 +1296,14 @@ - + Connection Time - + IBeamCursor @@ -1267,7 +1319,7 @@ - + Elapsed time since a novel block passing initial validity checks was received from this peer. @@ -1277,7 +1329,7 @@ - + IBeamCursor @@ -1293,7 +1345,7 @@ - + Elapsed time since a novel transaction accepted into our mempool was received from this peer. @@ -1303,7 +1355,7 @@ - + IBeamCursor @@ -1319,14 +1371,14 @@ - + Last Send - + IBeamCursor @@ -1342,14 +1394,14 @@ - + Last Receive - + IBeamCursor @@ -1365,14 +1417,14 @@ - + Sent - + IBeamCursor @@ -1388,14 +1440,14 @@ - + Received - + IBeamCursor @@ -1411,14 +1463,14 @@ - + Ping Time - + IBeamCursor @@ -1434,7 +1486,7 @@ - + The duration of a currently outstanding ping. @@ -1444,7 +1496,7 @@ - + IBeamCursor @@ -1460,14 +1512,14 @@ - + Min Ping - + IBeamCursor @@ -1483,14 +1535,14 @@ - + Time Offset - + IBeamCursor @@ -1506,7 +1558,7 @@ - + The mapped Autonomous System used for diversifying peer selection. @@ -1516,7 +1568,7 @@ - + IBeamCursor @@ -1532,7 +1584,7 @@ - + Whether we relay addresses to this peer. @@ -1542,7 +1594,7 @@ - + IBeamCursor @@ -1558,7 +1610,7 @@ - + Total number of addresses processed, excluding those dropped due to rate-limiting. @@ -1568,7 +1620,7 @@ - + IBeamCursor @@ -1584,7 +1636,7 @@ - + Total number of addresses dropped due to rate-limiting. @@ -1594,7 +1646,7 @@ - + IBeamCursor @@ -1610,7 +1662,7 @@ - + Qt::Vertical diff --git a/src/qt/rpcconsole.cpp b/src/qt/rpcconsole.cpp index b76b05bced..e5c6c20466 100644 --- a/src/qt/rpcconsole.cpp +++ b/src/qt/rpcconsole.cpp @@ -15,6 +15,7 @@ #include #include #include +#include #include #include #include @@ -533,8 +534,17 @@ RPCConsole::RPCConsole(interfaces::Node& node, QWidget* parent, Qt::WindowFlags /*: Explanatory text for a short-lived outbound peer connection that is used to request addresses from a peer. */ tr("Outbound Address Fetch: short-lived, for soliciting addresses")}; - const QString list{"
  • " + Join(CONNECTION_TYPE_DOC, QString("
  • ")) + "
"}; - ui->peerConnectionTypeLabel->setToolTip(ui->peerConnectionTypeLabel->toolTip().arg(list)); + const QString connection_types_list{"
  • " + Join(CONNECTION_TYPE_DOC, QString("
  • ")) + "
"}; + ui->peerConnectionTypeLabel->setToolTip(ui->peerConnectionTypeLabel->toolTip().arg(connection_types_list)); + const std::vector TRANSPORT_TYPE_DOC{ + //: Explanatory text for "detecting" transport type. + tr("detecting: peer could be v1 or v2"), + //: Explanatory text for v1 transport type. + tr("v1: unencrypted, plaintext transport protocol"), + //: Explanatory text for v2 transport type. + tr("v2: BIP324 encrypted transport protocol")}; + const QString transport_types_list{"
  • " + Join(TRANSPORT_TYPE_DOC, QString("
  • ")) + "
"}; + ui->peerTransportTypeLabel->setToolTip(ui->peerTransportTypeLabel->toolTip().arg(transport_types_list)); const QString hb_list{"
  • \"" + ts.to + "\" – " + tr("we selected the peer for high bandwidth relay") + "
  • \"" + ts.from + "\" – " + tr("the peer selected us for high bandwidth relay") + "
  • \"" @@ -1272,6 +1282,15 @@ void RPCConsole::updateDetailWidget() ui->peerVersion->setText(QString::number(stats->nodeStats.nVersion)); ui->peerSubversion->setText(QString::fromStdString(stats->nodeStats.cleanSubVer)); ui->peerConnectionType->setText(GUIUtil::ConnectionTypeToQString(stats->nodeStats.m_conn_type, /* prepend_direction */ true)); + ui->peerTransportType->setText(QString::fromStdString(TransportTypeAsString(stats->nodeStats.m_transport_type))); + if (stats->nodeStats.m_transport_type == TransportProtocolType::V2) { + ui->peerSessionIdLabel->setVisible(true); + ui->peerSessionId->setVisible(true); + ui->peerSessionId->setText(QString::fromStdString(stats->nodeStats.m_session_id)); + } else { + ui->peerSessionIdLabel->setVisible(false); + ui->peerSessionId->setVisible(false); + } ui->peerNetwork->setText(GUIUtil::NetworkToQString(stats->nodeStats.m_network)); if (stats->nodeStats.m_permission_flags == NetPermissionFlags::None) { ui->peerPermissions->setText(ts.na); From 6a4ca62fd135190ecf8126ac15269f3ad6ce07cd Mon Sep 17 00:00:00 2001 From: Kittywhiskers Van Gogh <63189531+kwvg@users.noreply.github.com> Date: Tue, 15 Oct 2024 09:43:55 +0000 Subject: [PATCH 07/12] merge bitcoin#28645: fix `assert_debug_log` call-site bugs, add type checks --- test/functional/feature_governance.py | 2 +- test/functional/p2p_v2_transport.py | 4 ++-- test/functional/test_framework/test_node.py | 3 +++ 3 files changed, 6 insertions(+), 3 deletions(-) diff --git a/test/functional/feature_governance.py b/test/functional/feature_governance.py index 4e499ffa31..43c49784d2 100755 --- a/test/functional/feature_governance.py +++ b/test/functional/feature_governance.py @@ -302,7 +302,7 @@ class DashGovernanceTest (DashTestFramework): self.log.info("Move a few block past the recent superblock height and make sure we have no new votes") for _ in range(5): - with self.nodes[1].assert_debug_log("", [f"Voting NO-FUNDING for trigger:{winning_trigger_hash} success"]): + with self.nodes[1].assert_debug_log(expected_msgs=[""], unexpected_msgs=[f"Voting NO-FUNDING for trigger:{winning_trigger_hash} success"]): self.bump_mocktime(1) self.generate(self.nodes[0], 1, sync_fun=self.sync_blocks()) # Votes on both triggers should NOT change diff --git a/test/functional/p2p_v2_transport.py b/test/functional/p2p_v2_transport.py index a7f208c858..a99d368c70 100755 --- a/test/functional/p2p_v2_transport.py +++ b/test/functional/p2p_v2_transport.py @@ -145,7 +145,7 @@ class V2TransportTest(BitcoinTestFramework): wrong_network_magic_prefix = MAGIC_BYTES["mainnet"] + V1_PREFIX[4:] with socket.socket(socket.AF_INET, socket.SOCK_STREAM) as s: s.connect(("127.0.0.1", p2p_port(0))) - with self.nodes[0].assert_debug_log("V2 transport error: V1 peer with wrong MessageStart"): + with self.nodes[0].assert_debug_log(["V2 transport error: V1 peer with wrong MessageStart"]): s.sendall(wrong_network_magic_prefix + b"somepayload") # Check detection of missing garbage terminator (hits after fixed amount of data if terminator never matches garbage) @@ -156,7 +156,7 @@ class V2TransportTest(BitcoinTestFramework): self.wait_until(lambda: len(self.nodes[0].getpeerinfo()) == num_peers + 1) s.sendall(b'\x00' * (MAX_KEY_GARB_AND_GARBTERM_LEN - 1)) self.wait_until(lambda: self.nodes[0].getpeerinfo()[-1]["bytesrecv"] == MAX_KEY_GARB_AND_GARBTERM_LEN - 1) - with self.nodes[0].assert_debug_log("V2 transport error: missing garbage terminator"): + with self.nodes[0].assert_debug_log(["V2 transport error: missing garbage terminator"]): s.sendall(b'\x00') # send out last byte # should disconnect immediately self.wait_until(lambda: len(self.nodes[0].getpeerinfo()) == num_peers) diff --git a/test/functional/test_framework/test_node.py b/test/functional/test_framework/test_node.py index 3da025570f..91671e1f1f 100755 --- a/test/functional/test_framework/test_node.py +++ b/test/functional/test_framework/test_node.py @@ -427,6 +427,9 @@ class TestNode(): def assert_debug_log(self, expected_msgs, unexpected_msgs=None, timeout=2): if unexpected_msgs is None: unexpected_msgs = [] + assert_equal(type(expected_msgs), list) + assert_equal(type(unexpected_msgs), list) + time_end = time.time() + timeout * self.timeout_factor prev_size = self.debug_log_bytes() From 35253cdd15c3b9b0e7d3e327c5b4c0474d4f2290 Mon Sep 17 00:00:00 2001 From: Kittywhiskers Van Gogh <63189531+kwvg@users.noreply.github.com> Date: Tue, 10 Oct 2023 12:11:00 -0400 Subject: [PATCH 08/12] merge bitcoin#28632: make python p2p not send getaddr on incoming connections --- test/functional/p2p_addr_relay.py | 5 +++-- test/functional/test_framework/p2p.py | 3 ++- test/functional/test_framework/test_node.py | 2 ++ 3 files changed, 7 insertions(+), 3 deletions(-) diff --git a/test/functional/p2p_addr_relay.py b/test/functional/p2p_addr_relay.py index cbc15dcbf5..e89734fcfe 100755 --- a/test/functional/p2p_addr_relay.py +++ b/test/functional/p2p_addr_relay.py @@ -257,15 +257,16 @@ class AddrTest(BitcoinTestFramework): full_outbound_peer.sync_with_ping() assert full_outbound_peer.getaddr_received() - self.log.info('Check that we do not send a getaddr message upon connecting to a block-relay-only peer') + self.log.info('Check that we do not send a getaddr message to a block-relay-only or inbound peer') block_relay_peer = self.nodes[0].add_outbound_p2p_connection(AddrReceiver(), p2p_idx=1, connection_type="block-relay-only") block_relay_peer.sync_with_ping() assert_equal(block_relay_peer.getaddr_received(), False) - self.log.info('Check that we answer getaddr messages only from inbound peers') inbound_peer = self.nodes[0].add_p2p_connection(AddrReceiver(send_getaddr=False)) inbound_peer.sync_with_ping() + assert_equal(inbound_peer.getaddr_received(), False) + self.log.info('Check that we answer getaddr messages only from inbound peers') # Add some addresses to addrman for i in range(1000): first_octet = i >> 8 diff --git a/test/functional/test_framework/p2p.py b/test/functional/test_framework/p2p.py index 43c27a4702..3bdccbd89c 100755 --- a/test/functional/test_framework/p2p.py +++ b/test/functional/test_framework/p2p.py @@ -490,7 +490,8 @@ class P2PInterface(P2PConnection): self.send_message(msg_sendaddrv2()) self.send_message(msg_verack()) self.nServices = message.nServices - self.send_message(msg_getaddr()) + if self.p2p_connected_to_node: + self.send_message(msg_getaddr()) # Connection helper methods diff --git a/test/functional/test_framework/test_node.py b/test/functional/test_framework/test_node.py index 91671e1f1f..fd8df3eec5 100755 --- a/test/functional/test_framework/test_node.py +++ b/test/functional/test_framework/test_node.py @@ -623,6 +623,7 @@ class TestNode(): if 'dstaddr' not in kwargs: kwargs['dstaddr'] = '127.0.0.1' + p2p_conn.p2p_connected_to_node = True p2p_conn.peer_connect(**kwargs, net=self.chain, timeout_factor=self.timeout_factor)() self.p2ps.append(p2p_conn) p2p_conn.wait_until(lambda: p2p_conn.is_connected, check_connected=False) @@ -667,6 +668,7 @@ class TestNode(): self.log.debug("Connecting to %s:%d %s" % (address, port, connection_type)) self.addconnection('%s:%d' % (address, port), connection_type) + p2p_conn.p2p_connected_to_node = False p2p_conn.peer_accept_connection(connect_cb=addconnection_callback, connect_id=p2p_idx + 1, net=self.chain, timeout_factor=self.timeout_factor, **kwargs)() if connection_type == "feeler": From c0b3062215f66cdf5f26daf8af268583be9bf735 Mon Sep 17 00:00:00 2001 From: Kittywhiskers Van Gogh <63189531+kwvg@users.noreply.github.com> Date: Mon, 14 Oct 2024 20:53:38 +0000 Subject: [PATCH 09/12] merge bitcoin#28782: Add missing sync on send_version in peer_connect --- test/functional/test_framework/p2p.py | 4 ++-- test/functional/test_framework/test_node.py | 6 ++++-- 2 files changed, 6 insertions(+), 4 deletions(-) diff --git a/test/functional/test_framework/p2p.py b/test/functional/test_framework/p2p.py index 3bdccbd89c..2be688ee69 100755 --- a/test/functional/test_framework/p2p.py +++ b/test/functional/test_framework/p2p.py @@ -392,8 +392,8 @@ class P2PInterface(P2PConnection): vt.strSubVer = self.strSubVer self.on_connection_send_msg = vt # Will be sent soon after connection_made - def peer_connect(self, *args, services=P2P_SERVICES, send_version=True, **kwargs): - create_conn = super().peer_connect(*args, **kwargs) + def peer_connect(self, *, services=P2P_SERVICES, send_version, **kwargs): + create_conn = super().peer_connect(**kwargs) if send_version: self.peer_connect_send_version(services) diff --git a/test/functional/test_framework/test_node.py b/test/functional/test_framework/test_node.py index fd8df3eec5..8c1a5b1717 100755 --- a/test/functional/test_framework/test_node.py +++ b/test/functional/test_framework/test_node.py @@ -613,7 +613,7 @@ class TestNode(): assert_msg += "with expected error " + expected_msg self._raise_assertion_error(assert_msg) - def add_p2p_connection(self, p2p_conn, *, wait_for_verack=True, **kwargs): + def add_p2p_connection(self, p2p_conn, *, wait_for_verack=True, send_version=True, **kwargs): """Add an inbound p2p connection to the node. This method adds the p2p connection to the self.p2ps list and also @@ -624,9 +624,11 @@ class TestNode(): kwargs['dstaddr'] = '127.0.0.1' p2p_conn.p2p_connected_to_node = True - p2p_conn.peer_connect(**kwargs, net=self.chain, timeout_factor=self.timeout_factor)() + p2p_conn.peer_connect(**kwargs, send_version=send_version, net=self.chain, timeout_factor=self.timeout_factor)() self.p2ps.append(p2p_conn) p2p_conn.wait_until(lambda: p2p_conn.is_connected, check_connected=False) + if send_version: + p2p_conn.wait_until(lambda: not p2p_conn.on_connection_send_msg) if wait_for_verack: # Wait for the node to send us the version and verack p2p_conn.wait_for_verack() From d0804d4bf08a4764905dd0f52e0448defbddaf91 Mon Sep 17 00:00:00 2001 From: Kittywhiskers Van Gogh <63189531+kwvg@users.noreply.github.com> Date: Mon, 14 Oct 2024 21:26:07 +0000 Subject: [PATCH 10/12] merge bitcoin#28822: Add missing wait for version to be sent in add_outbound_p2p_connection --- test/functional/test_framework/test_node.py | 1 + 1 file changed, 1 insertion(+) diff --git a/test/functional/test_framework/test_node.py b/test/functional/test_framework/test_node.py index 8c1a5b1717..ded022473e 100755 --- a/test/functional/test_framework/test_node.py +++ b/test/functional/test_framework/test_node.py @@ -681,6 +681,7 @@ class TestNode(): p2p_conn.wait_for_connect() self.p2ps.append(p2p_conn) + p2p_conn.wait_until(lambda: not p2p_conn.on_connection_send_msg) if wait_for_verack: p2p_conn.wait_for_verack() p2p_conn.sync_with_ping() From 1a293c7cc57fb6555806b12e5c5ffda43a214fe2 Mon Sep 17 00:00:00 2001 From: Kittywhiskers Van Gogh <63189531+kwvg@users.noreply.github.com> Date: Tue, 15 Oct 2024 10:36:47 +0000 Subject: [PATCH 11/12] merge bitcoin#29006: fix v2 transport intermittent test failure --- test/functional/p2p_v2_transport.py | 16 ++++++++-------- test/functional/test_framework/test_node.py | 18 ++++++++++++++++++ 2 files changed, 26 insertions(+), 8 deletions(-) diff --git a/test/functional/p2p_v2_transport.py b/test/functional/p2p_v2_transport.py index a99d368c70..4c566df1a9 100755 --- a/test/functional/p2p_v2_transport.py +++ b/test/functional/p2p_v2_transport.py @@ -133,9 +133,8 @@ class V2TransportTest(BitcoinTestFramework): V1_PREFIX = MAGIC_BYTES[self.chain] + b"version\x00\x00\x00\x00\x00" assert_equal(len(V1_PREFIX), 16) with socket.socket(socket.AF_INET, socket.SOCK_STREAM) as s: - num_peers = len(self.nodes[0].getpeerinfo()) - s.connect(("127.0.0.1", p2p_port(0))) - self.wait_until(lambda: len(self.nodes[0].getpeerinfo()) == num_peers + 1) + with self.nodes[0].wait_for_new_peer(): + s.connect(("127.0.0.1", p2p_port(0))) s.sendall(V1_PREFIX[:-1]) assert_equal(self.nodes[0].getpeerinfo()[-1]["transport_protocol_type"], "detecting") s.sendall(bytes([V1_PREFIX[-1]])) # send out last prefix byte @@ -144,22 +143,23 @@ class V2TransportTest(BitcoinTestFramework): # Check wrong network prefix detection (hits if the next 12 bytes correspond to a v1 version message) wrong_network_magic_prefix = MAGIC_BYTES["mainnet"] + V1_PREFIX[4:] with socket.socket(socket.AF_INET, socket.SOCK_STREAM) as s: - s.connect(("127.0.0.1", p2p_port(0))) + with self.nodes[0].wait_for_new_peer(): + s.connect(("127.0.0.1", p2p_port(0))) with self.nodes[0].assert_debug_log(["V2 transport error: V1 peer with wrong MessageStart"]): s.sendall(wrong_network_magic_prefix + b"somepayload") # Check detection of missing garbage terminator (hits after fixed amount of data if terminator never matches garbage) MAX_KEY_GARB_AND_GARBTERM_LEN = 64 + 4095 + 16 with socket.socket(socket.AF_INET, socket.SOCK_STREAM) as s: - num_peers = len(self.nodes[0].getpeerinfo()) - s.connect(("127.0.0.1", p2p_port(0))) - self.wait_until(lambda: len(self.nodes[0].getpeerinfo()) == num_peers + 1) + with self.nodes[0].wait_for_new_peer(): + s.connect(("127.0.0.1", p2p_port(0))) s.sendall(b'\x00' * (MAX_KEY_GARB_AND_GARBTERM_LEN - 1)) self.wait_until(lambda: self.nodes[0].getpeerinfo()[-1]["bytesrecv"] == MAX_KEY_GARB_AND_GARBTERM_LEN - 1) with self.nodes[0].assert_debug_log(["V2 transport error: missing garbage terminator"]): + peer_id = self.nodes[0].getpeerinfo()[-1]["id"] s.sendall(b'\x00') # send out last byte # should disconnect immediately - self.wait_until(lambda: len(self.nodes[0].getpeerinfo()) == num_peers) + self.wait_until(lambda: not peer_id in [p["id"] for p in self.nodes[0].getpeerinfo()]) if __name__ == '__main__': diff --git a/test/functional/test_framework/test_node.py b/test/functional/test_framework/test_node.py index ded022473e..7080149b5d 100755 --- a/test/functional/test_framework/test_node.py +++ b/test/functional/test_framework/test_node.py @@ -490,6 +490,24 @@ class TestNode(): 'Expected messages "{}" does not partially match log:\n\n{}\n\n'.format( str(expected_msgs), print_log)) + @contextlib.contextmanager + def wait_for_new_peer(self, timeout=5): + """ + Wait until the node is connected to at least one new peer. We detect this + by watching for an increased highest peer id, using the `getpeerinfo` RPC call. + Note that the simpler approach of only accounting for the number of peers + suffers from race conditions, as disconnects from unrelated previous peers + could happen anytime in-between. + """ + def get_highest_peer_id(): + peer_info = self.getpeerinfo() + return peer_info[-1]["id"] if peer_info else -1 + + initial_peer_id = get_highest_peer_id() + yield + wait_until_helper(lambda: get_highest_peer_id() > initial_peer_id, + timeout=timeout, timeout_factor=self.timeout_factor) + @contextlib.contextmanager def profile_with_perf(self, profile_name: str): """ From aa5311d0fc0868e9d3dcb386a13d24f79cdfa29f Mon Sep 17 00:00:00 2001 From: Kittywhiskers Van Gogh <63189531+kwvg@users.noreply.github.com> Date: Wed, 16 Oct 2024 10:32:29 +0000 Subject: [PATCH 12/12] merge bitcoin#29212: Fix -netinfo backward compat with getpeerinfo pre-v26 --- src/bitcoin-cli.cpp | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/bitcoin-cli.cpp b/src/bitcoin-cli.cpp index 11f1a144f5..cd691e151c 100644 --- a/src/bitcoin-cli.cpp +++ b/src/bitcoin-cli.cpp @@ -518,7 +518,7 @@ public: const std::string addr{peer["addr"].get_str()}; const std::string age{conn_time == 0 ? "" : ToString((m_time_now - conn_time) / 60)}; const std::string sub_version{peer["subver"].get_str()}; - const std::string transport{peer["transport_protocol_type"].get_str()}; + const std::string transport{peer["transport_protocol_type"].isNull() ? "v1" : peer["transport_protocol_type"].get_str()}; const bool is_addr_relay_enabled{peer["addr_relay_enabled"].isNull() ? false : peer["addr_relay_enabled"].get_bool()}; const bool is_bip152_hb_from{peer["bip152_hb_from"].get_bool()}; const bool is_bip152_hb_to{peer["bip152_hb_to"].get_bool()}; @@ -538,7 +538,7 @@ public: // Report detailed peer connections list sorted by direction and minimum ping time. if (DetailsRequested() && !m_peers.empty()) { std::sort(m_peers.begin(), m_peers.end()); - result += strprintf("<-> type net tp mping ping send recv txn blk hb %*s%*s%*s ", + result += strprintf("<-> type net v mping ping send recv txn blk hb %*s%*s%*s ", m_max_addr_processed_length, "addrp", m_max_addr_rate_limited_length, "addrl", m_max_age_length, "age"); @@ -551,7 +551,7 @@ public: peer.is_outbound ? "out" : "in", ConnectionTypeForNetinfo(peer.conn_type), peer.network, - peer.transport_protocol_type == "detecting" ? "*" : peer.transport_protocol_type, + peer.transport_protocol_type.rfind('v', 0) == 0 ? peer.transport_protocol_type[1] : ' ', PingTimeToString(peer.min_ping), PingTimeToString(peer.ping), peer.last_send ? ToString(m_time_now - peer.last_send) : "", @@ -660,7 +660,7 @@ public: " \"feeler\" - short-lived connection for testing addresses\n" " \"addr\" - address fetch; short-lived connection for requesting addresses\n" " net Network the peer connected through (\"ipv4\", \"ipv6\", \"onion\", \"i2p\", \"cjdns\", or \"npr\" (not publicly routable))\n" - " tp Transport protocol used for the connection (\"v1\", \"v2\" or \"*\" if detecting)\n" + " v Version of transport protocol used for the connection\n" " mping Minimum observed ping time, in milliseconds (ms)\n" " ping Last observed ping time, in milliseconds (ms)\n" " send Time since last message sent to the peer, in seconds\n"