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" 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 cd9f2bddcd..b2be393418 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") + "
  • \"" @@ -1286,6 +1296,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); 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_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/p2p_v2_transport.py b/test/functional/p2p_v2_transport.py index a7f208c858..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].assert_debug_log("V2 transport error: V1 peer with wrong MessageStart"): + 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"): + 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/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/p2p.py b/test/functional/test_framework/p2p.py index 43c27a4702..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) @@ -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 6aa1bbd844..7080149b5d 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, @@ -426,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() @@ -486,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): """ @@ -609,7 +631,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 @@ -619,9 +641,12 @@ class TestNode(): if 'dstaddr' not in kwargs: kwargs['dstaddr'] = '127.0.0.1' - p2p_conn.peer_connect(**kwargs, net=self.chain, timeout_factor=self.timeout_factor)() + p2p_conn.p2p_connected_to_node = True + 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() @@ -637,20 +662,33 @@ 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 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 - 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. 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): 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": @@ -661,8 +699,10 @@ class TestNode(): p2p_conn.wait_for_connect() self.p2ps.append(p2p_conn) - p2p_conn.wait_for_verack() - p2p_conn.sync_with_ping() + 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() return p2p_conn @@ -671,7 +711,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()