From 03544175d9e9cd544a178574c3e042e058e9b717 Mon Sep 17 00:00:00 2001 From: Kittywhiskers Van Gogh <63189531+kwvg@users.noreply.github.com> Date: Wed, 2 Oct 2024 08:31:15 +0000 Subject: [PATCH] merge bitcoin#22777: don't request tx relay on feeler connections --- src/net.cpp | 4 +++- src/net.h | 4 ++-- src/net_processing.cpp | 2 +- src/rpc/net.cpp | 4 +++- test/functional/p2p_add_connections.py | 17 +++++++++++++++++ test/functional/test_framework/test_node.py | 15 ++++++++++----- 6 files changed, 36 insertions(+), 10 deletions(-) diff --git a/src/net.cpp b/src/net.cpp index a49132c8df..2df0b3ecb5 100644 --- a/src/net.cpp +++ b/src/net.cpp @@ -1998,7 +1998,6 @@ bool CConnman::AddConnection(const std::string& address, ConnectionType conn_typ switch (conn_type) { case ConnectionType::INBOUND: case ConnectionType::MANUAL: - case ConnectionType::FEELER: return false; case ConnectionType::OUTBOUND_FULL_RELAY: max_connections = m_max_outbound_full_relay; @@ -2009,6 +2008,9 @@ bool CConnman::AddConnection(const std::string& address, ConnectionType conn_typ // no limit for ADDR_FETCH because -seednode has no limit either case ConnectionType::ADDR_FETCH: break; + // no limit for FEELER connections since they're short-lived + case ConnectionType::FEELER: + break; } // no default case, so the compiler can warn about missing cases // Count existing connections diff --git a/src/net.h b/src/net.h index ab17ca7758..23988dd1d2 100644 --- a/src/net.h +++ b/src/net.h @@ -1433,8 +1433,8 @@ public: * Attempts to open a connection. Currently only used from tests. * * @param[in] address Address of node to try connecting to - * @param[in] conn_type ConnectionType::OUTBOUND or ConnectionType::BLOCK_RELAY - * or ConnectionType::ADDR_FETCH + * @param[in] conn_type ConnectionType::OUTBOUND, ConnectionType::BLOCK_RELAY, + * ConnectionType::ADDR_FETCH or ConnectionType::FEELER * @return bool Returns false if there are no available * slots for this connection: * - conn_type not a supported ConnectionType diff --git a/src/net_processing.cpp b/src/net_processing.cpp index 8bfbdefa14..2005644b64 100644 --- a/src/net_processing.cpp +++ b/src/net_processing.cpp @@ -1384,7 +1384,7 @@ void PeerManagerImpl::PushNodeVersion(CNode& pnode, const Peer& peer) nProtocolVersion = gArgs.GetArg("-pushversion", PROTOCOL_VERSION); } - const bool tx_relay = !m_ignore_incoming_txs && !pnode.IsBlockOnlyConn(); + const bool tx_relay = !m_ignore_incoming_txs && !pnode.IsBlockOnlyConn() && !pnode.IsFeelerConn(); m_connman.PushMessage(&pnode, CNetMsgMaker(INIT_PROTO_VERSION).Make(NetMsgType::VERSION, nProtocolVersion, my_services, nTime, your_services, addr_you, // Together the pre-version-31402 serialization of CAddress "addrYou" (without nTime) my_services, CService(), // Together the pre-version-31402 serialization of CAddress "addrMe" (without nTime) diff --git a/src/rpc/net.cpp b/src/rpc/net.cpp index a5ae324644..da1534ecad 100644 --- a/src/rpc/net.cpp +++ b/src/rpc/net.cpp @@ -352,7 +352,7 @@ static RPCHelpMan addconnection() "\nOpen an outbound connection to a specified node. This RPC is for testing only.\n", { {"address", RPCArg::Type::STR, RPCArg::Optional::NO, "The IP address and port to attempt connecting to."}, - {"connection_type", RPCArg::Type::STR, RPCArg::Optional::NO, "Type of connection to open (\"outbound-full-relay\", \"block-relay-only\" or \"addr-fetch\")."}, + {"connection_type", RPCArg::Type::STR, RPCArg::Optional::NO, "Type of connection to open (\"outbound-full-relay\", \"block-relay-only\", \"addr-fetch\" or \"feeler\")."}, }, RPCResult{ RPCResult::Type::OBJ, "", "", @@ -380,6 +380,8 @@ static RPCHelpMan addconnection() conn_type = ConnectionType::BLOCK_RELAY; } else if (conn_type_in == "addr-fetch") { conn_type = ConnectionType::ADDR_FETCH; + } else if (conn_type_in == "feeler") { + conn_type = ConnectionType::FEELER; } else { throw JSONRPCError(RPC_INVALID_PARAMETER, self.ToString()); } diff --git a/test/functional/p2p_add_connections.py b/test/functional/p2p_add_connections.py index 8b7ea12d91..3b11b8c5f8 100755 --- a/test/functional/p2p_add_connections.py +++ b/test/functional/p2p_add_connections.py @@ -8,6 +8,13 @@ from test_framework.p2p import P2PInterface from test_framework.test_framework import BitcoinTestFramework from test_framework.util import check_node_connections +class P2PFeelerReceiver(P2PInterface): + def on_version(self, message): + # The bitcoind node closes feeler connections as soon as a version + # message is received from the test framework. Don't send any responses + # to the node's version message since the connection will already be + # closed. + pass class P2PAddConnections(BitcoinTestFramework): def set_test_params(self): @@ -86,6 +93,16 @@ class P2PAddConnections(BitcoinTestFramework): check_node_connections(node=self.nodes[1], num_in=5, num_out=10) + self.log.info("Add 1 feeler connection to node 0") + feeler_conn = self.nodes[0].add_outbound_p2p_connection(P2PFeelerReceiver(), p2p_idx=6, connection_type="feeler") + + # Feeler connection is closed + assert not feeler_conn.is_connected + + # Verify version message received + assert_equal(feeler_conn.message_count["version"], 1) + # Feeler connections do not request tx relay + assert_equal(feeler_conn.last_message["version"].relay, 0) if __name__ == '__main__': P2PAddConnections().main() diff --git a/test/functional/test_framework/test_node.py b/test/functional/test_framework/test_node.py index dbdeb255de..5a7a54eb97 100755 --- a/test/functional/test_framework/test_node.py +++ b/test/functional/test_framework/test_node.py @@ -574,7 +574,7 @@ class TestNode(): def add_outbound_p2p_connection(self, p2p_conn, *, p2p_idx, connection_type="outbound-full-relay", **kwargs): """Add an outbound p2p connection from node. Must be an - "outbound-full-relay", "block-relay-only" or "addr-fetch" connection. + "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. @@ -586,11 +586,16 @@ class TestNode(): p2p_conn.peer_accept_connection(connect_cb=addconnection_callback, connect_id=p2p_idx + 1, net=self.chain, timeout_factor=self.timeout_factor, **kwargs)() - p2p_conn.wait_for_connect() - self.p2ps.append(p2p_conn) + if connection_type == "feeler": + # feeler connections are closed as soon as the node receives a `version` message + p2p_conn.wait_until(lambda: p2p_conn.message_count["version"] == 1, check_connected=False) + p2p_conn.wait_until(lambda: not p2p_conn.is_connected, check_connected=False) + else: + p2p_conn.wait_for_connect() + self.p2ps.append(p2p_conn) - p2p_conn.wait_for_verack() - p2p_conn.sync_with_ping() + p2p_conn.wait_for_verack() + p2p_conn.sync_with_ping() return p2p_conn