Skip to content

Commit

Permalink
Merge bitcoin/bitcoin#28632: test: make python p2p not send getaddr o…
Browse files Browse the repository at this point in the history
…n incoming connections

9cfc1c9 test: check that we don't send a getaddr msg to an inbound peer (Martin Zumsande)
88c33c6 test: make python p2p not send getaddr messages when it's being connected to (Martin Zumsande)

Pull request description:

  `bitcoind` nodes send `getaddr` messages only to outbound nodes (and ignore `getaddr` received by outgoing connections).
  The python p2p node should mirror this behavior by not sending a `getaddr` message when it is not the initiator of the connection.
  This is currently causing several unnecessary messages being sent and then ignored (`Ignoring "getaddr" from outbound-full-relay connection.`) in tests like `p2p_add_connections.py`.

ACKs for top commit:
  pinheadmz:
    concept ACK 9cfc1c9
  pablomartin4btc:
    re ACK 9cfc1c9
  BrandonOdiwuor:
    re ACK 9cfc1c9

Tree-SHA512: 812bec5d8a4828b4384d4cdd4362d6eec09acb2363e888f2b3e3bf8b925e0e17f15e13dc297d6b616c68b93ace9ede7245b07b405d3f5f8eada98350f74230dc
  • Loading branch information
fanquake committed Nov 1, 2023
2 parents 4733de3 + 9cfc1c9 commit eca2e43
Show file tree
Hide file tree
Showing 3 changed files with 7 additions and 3 deletions.
5 changes: 3 additions & 2 deletions test/functional/p2p_addr_relay.py
Original file line number Diff line number Diff line change
Expand Up @@ -270,15 +270,16 @@ def getaddr_tests(self):
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
Expand Down
3 changes: 2 additions & 1 deletion test/functional/test_framework/p2p.py
Original file line number Diff line number Diff line change
Expand Up @@ -456,7 +456,8 @@ def on_version(self, message):
self.send_message(msg_verack())
self.nServices = message.nServices
self.relay = message.relay
self.send_message(msg_getaddr())
if self.p2p_connected_to_node:
self.send_message(msg_getaddr())

# Connection helper methods

Expand Down
2 changes: 2 additions & 0 deletions test/functional/test_framework/test_node.py
Original file line number Diff line number Diff line change
Expand Up @@ -644,6 +644,7 @@ def add_p2p_connection(self, p2p_conn, *, wait_for_verack=True, **kwargs):
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)
Expand Down Expand Up @@ -689,6 +690,7 @@ 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":
Expand Down

0 comments on commit eca2e43

Please sign in to comment.