test: fix addr relay test silently passing and other improvements

stratospher commented at 5:58 am on March 6, 2026: contributor

couple of improvements in the addr relay test:

fixes the silent test pass discovered in #34717 (comment) (will remove this if that PR gets merged - the test doesn’t fail even though #34717 changes the behaviour)
correct wrong peerinfo index
prevent intermittent disconnection warnings like the one shown below by protecting outbound peer from ConsiderEviction

0  TestFramework (INFO): Check that we answer getaddr messages only once per connection
1  TestFramework.p2p (WARNING): Connection lost to 127.0.0.1:58829 due to [Errno 54] Connection reset by peer

remove a no longer applicable test comment since we don’t need to send initial GETADDR for intial self announcement anymore

test: fix addr relay test silent pass and wrong peerinfo index

the test silently passes on master because SetupAddressRelay
isn't called by default for inbound connections.

ecb5ce6e76

test: protect outbound connection from eviction in getaddr_test

since we're bumping mocktime more than CHAIN_SYNC_TIMEOUT = 20 * 60,
it's possible for disconnections like this to happen in the test:

$ test/functional/p2p_addr_relay.py --randomseed=7758649581790797022
...
TestFramework (INFO): Check that we answer getaddr messages only once per connection
TestFramework.p2p (WARNING): Connection lost to 127.0.0.1:58829 due to [Errno 54] Connection reset by peer
...

7ee8c0abc6

test: use static methods and clarify comment in addr_relay

we don't need to send GETADDR for initial self announcement
anymore + can construct addr_receivers using
AddrReceiver(send_getaddr=False).

however we would need to send an empty ADDR message to each
of the addr_receivers to initialise addr relay for inbound
connections. so current code is simpler and we can just
clarify the comment.

57bfa864fe

DrahtBot added the label Tests on Mar 6, 2026

DrahtBot commented at 5:58 am on March 6, 2026: contributor

The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.

Reviews

See the guideline for information on the review process.

Type	Reviewers
ACK	Bortlesboat, naiyoma, mzumsande

If your review is incorrectly listed, please copy-paste <!–meta-tag:bot-skip–> into the comment that the bot should ignore.

Conflicts

Reviewers, this pull request conflicts with the following ones:

#34717 (p2p: remove m_getaddr_sent by naiyoma)

If you consider this pull request important, please also help to review the conflicting pull requests. Ideally, start with the one that should be merged first.

DrahtBot added the label CI failed on Mar 6, 2026

stratospher commented at 7:12 am on March 6, 2026: contributor

(CI failure is unrelated)

fanquake added this to the milestone 31.0 on Mar 6, 2026

DrahtBot removed the label CI failed on Mar 6, 2026

Bortlesboat commented at 6:10 pm on March 6, 2026: none

utACK 57bfa864fe

The move of msg_addr() before the relay check is the key fix — without it, the num_ipv4_received == 0 assertion was passing because address relay was never initialized, not because relay was actually suppressed.

The peerinfo index fix and header announcement to prevent eviction are straightforward.

Bortlesboat commented at 7:14 pm on March 8, 2026: none

ACK 57bfa864fe69. Ran both p2p_addr_relay.py and p2p_addr_selfannouncement.py locally, both pass. Good catch on the stale peerinfo reference in inbound_blackhole_tests — that would silently check the wrong peer.

fanquake commented at 10:58 am on March 10, 2026: member

cc @Crypt-iQ @naiyoma

achow101 added the label Needs Backport (31.x) on Mar 11, 2026

fanquake requested review from mzumsande on Mar 11, 2026

naiyoma commented at 2:18 pm on March 11, 2026: contributor

ACK 57bfa864fe69ea5610399f9db60cf2299930703a

I would suggest testing this flow, as it was previously an edge case but is now the default behaviour:

self-announcement
GETADDR
ADDR.

 0diff --git a/test/functional/p2p_addr_relay.py b/test/functional/p2p_addr_relay.py
 1index 65b21c0d50..af8a10e5c0 100755
 2--- a/test/functional/p2p_addr_relay.py
 3+++ b/test/functional/p2p_addr_relay.py
 4
 5+        tip_header = from_hex(CBlockHeader(), self.nodes[0].getblockheader(self.nodes[0].getbestblockhash(), False))
 6+        full_outbound_peer.send_and_ping(msg_headers([tip_header]))
 7+
 8+        # Self-announcement is not relayed (m_getaddr_sent=true, set when node sent GETADDR)
 9+        self_announcement = self.setup_addr_msg(1)
10+        self.send_addr_msg(full_outbound_peer, self_announcement, [inbound_peer])
11+        self.log.info('Check that self-announcement from outbound peer is not relayed')
12+        assert_equal(inbound_peer.num_ipv4_received, 0)
13+
14+        # GETADDR response is not relayed (vAddr.size() >= 1000)
15+        getaddr_response = self.setup_addr_msg(1000)
16+        self.send_addr_msg(full_outbound_peer, getaddr_response, [inbound_peer])
17+        self.log.info('Check that GETADDR response from outbound peer is not relayed')
18         assert_equal(inbound_peer.num_ipv4_received, 0)
19 
20-        self.log.info('Check that subsequent addr messages sent from an outbound peer are relayed')
21+
22+        self.log.info('Check that first subsequent addr messages sent from an outbound peer are relayed(m_getaddr_sent=false)')

naiyoma commented at 12:32 pm on March 12, 2026: contributor

An alternative would be to check that a small getaddr response would be relayed, but a larger response wouldn’t. (Both this and the previous suggestion might be out of scope feel free to ignore.)

 0+        self.log.info('Test that self-announcement resets m_getaddr_sent, causing small GETADDR responses to be relayed')
 1+        outbound_peer1 = self.nodes[0].add_outbound_p2p_connection(
 2+            AddrReceiver(), p2p_idx=0, connection_type="outbound-full-relay"
 3+        )
 4+        inbound_peer1 = self.nodes[0].add_p2p_connection(AddrReceiver(send_getaddr=False))
 5+        # Enable addr relay on inbound_peer1
 6+        inbound_peer1.send_and_ping(msg_addr())
 7+
 8+        # Outbound peer sends self-announcement -> resets m_getaddr_sent to False
 9+        self_announcement = self.setup_addr_msg(1)
10+        outbound_peer1.send_and_ping(self_announcement)
11+
12+        # small GETADDR response (9 addrs) gets relayed
13+        getaddr_response_small = self.setup_addr_msg(9)
14+        self.send_addr_msg(outbound_peer1, getaddr_response_small, [inbound_peer1])
15+        assert_equal(inbound_peer1.num_ipv4_received, 9)
16+
17+        # Large GETADDR response (1000 addrs) - not relayed, but only due to size check
18+        getaddr_response_large = self.setup_addr_msg(1000)
19+        self.send_addr_msg(outbound_peer1, getaddr_response_large, [inbound_peer1])
20+        assert_equal(inbound_peer1.num_ipv4_received, 9)
21+
22+        self.nodes[0].disconnect_p2ps()

mzumsande commented at 2:38 pm on March 12, 2026: contributor

Code Review ACK 57bfa864fe69ea5610399f9db60cf2299930703a

fanquake merged this on Mar 12, 2026

fanquake closed this on Mar 12, 2026

fanquake referenced this in commit a3c1eda8f2 on Mar 12, 2026

fanquake referenced this in commit 5642a2b0fe on Mar 12, 2026

fanquake referenced this in commit a28d78c44a on Mar 12, 2026

fanquake removed the label Needs Backport (31.x) on Mar 12, 2026

fanquake commented at 3:08 pm on March 12, 2026: member

Backported to 31.x in #34800.

achow101 referenced this in commit f0e2cbc5e5 on Mar 25, 2026

test: fix addr relay test silently passing and other improvements #34750

Reviews

Conflicts