foster/bitcoin-bitcoin-core
0
0
Fork 0
mirror of https://github.com/bitcoin/bitcoin.git synced 2025-02-02 09:46:52 -05:00
Code Issues Activity

Merge bitcoin/bitcoin#22777: net processing: don't request tx relay on feeler connections

Browse source 
eaf6be0114 [net processing] Do not request transaction relay from feeler connections (John Newbery)
0220b834b1 [test] Add testing for outbound feeler connections (John Newbery)

Pull request description:

  Feelers are short-lived connections used to test the viability of peers. The bitcoind node will periodically open feeler connections to addresses in its addrman, wait for a `version` message from the peer, and then close the connection.

  Currently, we set `fRelay` to `1` in the `version` message for feeler connections, indicating that we want the peer to relay transactions to us. However, we close the connection immediately on receipt of the `version` message, and so never process any incoming transaction announcements. This PR changes that behaviour to instead set `fRelay` to `0` indicating that we do not wish to receive transaction announcements from the peer.

  This PR also extends the `addconnection` RPC to allow creating outbound feeler connections from the node to the test framework, and a test to verify that the node sets `fRelay` to `0` in the `version` message to feeler connections.

ACKs for top commit:
  naumenkogs:
    ACK eaf6be0114
  MarcoFalke:
    review ACK eaf6be0114 🏃

Tree-SHA512: 1c56837dbd0a396fe404a5e39f7459864d15f666664d6b35ad109628b13158e077e417e586bf48946a23bd5cbe63716cb4bf22cdf8781b74dfce6047b87b465a
This commit is contained in:
MarcoFalke 2021-12-14 17:57:02 +01:00
commit 9635760ce8
No known key found for this signature in database
GPG key ID: CE2B75697E69A548
6 changed files with 36 additions and 10 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

4
src/net.cpp
View file

@ -1225,7 +1225,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;
@ -1236,6 +1235,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

4
src/net.h
View file

@ -884,8 +884,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

2
src/net_processing.cpp
View file

@ -1104,7 +1104,7 @@ void PeerManagerImpl::PushNodeVersion(CNode& pnode, int64_t nTime)
CService addr_you = addr.IsRoutable() && !IsProxy(addr) && addr.IsAddrV1Compatible() ? addr : CService();
uint64_t your_services{addr.nServices};
const bool tx_relay = !m_ignore_incoming_txs && pnode.m_tx_relay != nullptr;
const bool tx_relay = !m_ignore_incoming_txs && pnode.m_tx_relay != nullptr && !pnode.IsFeelerConn();
m_connman.PushMessage(&pnode, CNetMsgMaker(INIT_PROTO_VERSION).Make(NetMsgType::VERSION, PROTOCOL_VERSION, 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)

4
src/rpc/net.cpp
View file

@ -328,7 +328,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, "", "",
@ -356,6 +356,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());
}

17
test/functional/p2p_add_connections.py
View file

@ -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):
@ -85,6 +92,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()

15
test/functional/test_framework/test_node.py
View file

@ -578,7 +578,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.
@ -590,11 +590,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