From 792e2edf57ab31ae5c6f98acf33af8f67506630f Mon Sep 17 00:00:00 2001 From: 0xb10c Date: Mon, 1 Dec 2025 13:57:45 +0100 Subject: [PATCH] p2p: first addr self-announcement in separate msg This makes sure the initial address self-announcement a node sends to a peer happends in a separate P2P message. This has benefits for both inbound and outbound connections: For inbound connections from a peer to us, previously, we might send the self-announcement along with our response to a GETADDR request. However, the self-announcement might replace an address from the GETADDR response. This isn't clean. For outbound connections from us to a peer, previously, it could have happend that we send the self-announcement along with other addresses. Since shortly after connection open, the peer might only have one rate-limiting token for us, and the addresses are shuffeld on arrival, it's possible that the self-announcement gets rate-limited. However, note that these rate-limitings seem to be rare in practice. This is inspired by and based on https://github.com/bitcoin/bitcoin/pull/33699#issuecomment-3462287763 Co-Authored-By: Anthony Towns --- src/net_processing.cpp | 15 ++++++++++++++- test/functional/p2p_addr_selfannouncement.py | 16 +++++++++++++--- 2 files changed, 27 insertions(+), 4 deletions(-) diff --git a/src/net_processing.cpp b/src/net_processing.cpp index 6ec55256094..e5ead349754 100644 --- a/src/net_processing.cpp +++ b/src/net_processing.cpp @@ -5328,7 +5328,20 @@ void PeerManagerImpl::MaybeSendAddr(CNode& node, Peer& peer, std::chrono::micros } if (std::optional local_service = GetLocalAddrForPeer(node)) { CAddress local_addr{*local_service, peer.m_our_services, Now()}; - PushAddress(peer, local_addr); + if (peer.m_next_local_addr_send == 0us) { + // Send the initial self-announcement in its own message. This makes sure + // rate-limiting with limited start-tokens doesn't ignore it if the first + // message ends up containing multiple addresses. + std::vector self_announcement {local_addr}; + if (peer.m_wants_addrv2) { + MakeAndPushMessage(node, NetMsgType::ADDRV2, CAddress::V2_NETWORK(self_announcement)); + } else { + MakeAndPushMessage(node, NetMsgType::ADDR, CAddress::V1_NETWORK(self_announcement)); + } + } else { + // All later self-announcements are sent together with the other addresses. + PushAddress(peer, local_addr); + } } peer.m_next_local_addr_send = current_time + m_rng.rand_exp_duration(AVG_LOCAL_ADDRESS_BROADCAST_INTERVAL); } diff --git a/test/functional/p2p_addr_selfannouncement.py b/test/functional/p2p_addr_selfannouncement.py index 1fb91e0a935..9c98e69238e 100755 --- a/test/functional/p2p_addr_selfannouncement.py +++ b/test/functional/p2p_addr_selfannouncement.py @@ -5,6 +5,9 @@ """ Test that a node sends a self-announcement with its external IP to in- and outbound peers after connection open and again sometime later. + +Additionally, this checks that the first self-announcement arrives +in its own message and that this message is the first we receive. """ import time @@ -42,6 +45,13 @@ class SelfAnnouncementReceiver(P2PInterface): self.addresses_received += 1 if addr == self.expected: self.self_announcements_received += 1 + if self.self_announcements_received == 1: + # If it's the first self-announcement, check that it is + # in the first addr message we receive, and that this message + # only contains one address. This also implies that it is + # the first address we receive. + assert_equal(self.addr_messages_received, 1) + assert_equal(len(message.addrs), 1) def on_addrv2(self, message): assert (self.addrv2_test) @@ -69,10 +79,10 @@ class AddrSelfAnnouncementTest(BitcoinTestFramework): self.self_announcement_test(outbound=True, addrv2=True) def inbound_connection_open_assertions(self, addr_receiver): - # We expect one self-announcement and multiple other addresses in - # response to a GETADDR in a single addr / addrv2 message. + # In response to a GETADDR, we expect a message with the self-announcement + # and an addr message containing the GETADDR response. assert_equal(addr_receiver.self_announcements_received, 1) - assert_equal(addr_receiver.addr_messages_received, 1) + assert_equal(addr_receiver.addr_messages_received, 2) assert_greater_than(addr_receiver.addresses_received, 1) def outbound_connection_open_assertions(self, addr_receiver):