p2p: send first addr self-announcement in separate message 🎄

0xB10C commented at 3:38 pm on December 24, 2025: contributor

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 #33699 (comment). The discussion there should be helpful for reviewers.

DrahtBot added the label P2P on Dec 24, 2025

DrahtBot commented at 3:38 pm on December 24, 2025: contributor

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

Code Coverage & Benchmarks

For details see: https://corecheck.dev/bitcoin/bitcoin/pulls/34146.

Reviews

See the guideline for information on the review process.

Type	Reviewers
ACK	fjahr, bensig

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

in test/functional/p2p_addr_selfannouncement.py:53 in 975016606a

44@@ -42,6 +45,15 @@ def handle_addr_message(self, message):
45             self.addresses_received += 1
46             if addr == self.expected:
47                 self.self_announcements_received += 1
48+                # If it's the first self-announcement:
49+                if self.self_announcements_received == 1:
50+                    # - check that it is in the first addr message we receive
51+                    assert_equal(self.addr_messages_received, 1)
52+                    # - check that it is the first address we receive
53+                    assert_equal(self.addresses_received, 1)

fjahr commented at 10:26 pm on December 24, 2025:

The check below implies this check (if it’s the only one it’s always the first), so it seems like it could be dropped.

0xB10C commented at 10:42 am on January 5, 2026:

I dropped this and reworked the comment a bit. Since it’s important for us that the self-annoucenment is the first address we receive, i’m mentioning it in the comment.

in test/functional/p2p_addr_selfannouncement.py:55 in 975016606a outdated

51+                    assert_equal(self.addr_messages_received, 1)
52+                    # - check that it is the first address we receive
53+                    assert_equal(self.addresses_received, 1)
54+                    # - check that the message only contains the self-announcement
55+                    assert_equal(len(message.addrs), 1)
56+

fjahr commented at 10:26 pm on December 24, 2025:

nit: unnecessary empty line added

in test/functional/p2p_addr_selfannouncement.py:84 in 975016606a

80@@ -69,10 +81,10 @@ def run_test(self):
81         self.self_announcement_test(outbound=True, addrv2=True)
82 
83     def inbound_connection_open_assertions(self, addr_receiver):
84-        # We expect one self-announcement and multiple other addresses in
85-        # response to a GETADDR in a single addr / addrv2 message.
86+        # We expect the self-announcement in a separate message from

fjahr commented at 10:29 pm on December 24, 2025:

Hm, the edited comment drops some information that is still relevant, e.g. “and multiple other addresses” explains the greater_than assertion below. Maybe this part could be added back above that assertion.

0xB10C commented at 10:42 am on January 5, 2026:

Going with:

In response to a GETADDR, we expect a message with the self-announcement and an addr message containing the GETADDR response.

in src/net_processing.cpp:5342 in 975016606a

5338+                    MakeAndPushMessage(node, NetMsgType::ADDRV2, CAddress::V2_NETWORK(self_announcement));
5339+                } else {
5340+                    MakeAndPushMessage(node, NetMsgType::ADDR, CAddress::V1_NETWORK(self_announcement));
5341+                }
5342+            } else {
5343+                // All later self-announcements are send together with the other addresses.

fjahr commented at 10:31 pm on December 24, 2025:

0                // All later self-announcements are sent together with the other addresses.

fjahr commented at 10:38 pm on December 24, 2025: contributor

Concept ACK

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 <aj@erisian.com.au>

792e2edf57

0xB10C force-pushed on Jan 5, 2026

in src/net_processing.cpp:5335 in 792e2edf57

5327@@ -5328,7 +5328,20 @@ void PeerManagerImpl::MaybeSendAddr(CNode& node, Peer& peer, std::chrono::micros
5328         }
5329         if (std::optional<CService> local_service = GetLocalAddrForPeer(node)) {
5330             CAddress local_addr{*local_service, peer.m_our_services, Now<NodeSeconds>()};
5331-            PushAddress(peer, local_addr);
5332+            if (peer.m_next_local_addr_send == 0us) {
5333+                // Send the initial self-announcement in its own message. This makes sure
5334+                // rate-limiting with limited start-tokens doesn't ignore it if the first
5335+                // message ends up containing multiple addresses.
5336+                std::vector<CAddress> self_announcement {local_addr};

fjahr commented at 12:06 pm on January 5, 2026:

micro-nit: could drop space

0                std::vector<CAddress> self_announcement{local_addr};

fjahr commented at 12:09 pm on January 5, 2026: contributor

Code review ACK 792e2edf57ab31ae5c6f98acf33af8f67506630f

bensig commented at 9:54 pm on January 5, 2026: contributor

ACK 792e2edf57ab31ae5c6f98acf33af8f67506630f

Built and tested.

Logic is clean - uses m_next_local_addr_send == 0us to detect first announcement and sends it separately.

Test correctly verifies it arrives alone in first message

in src/net_processing.cpp:5337 in 792e2edf57

5333+                // Send the initial self-announcement in its own message. This makes sure
5334+                // rate-limiting with limited start-tokens doesn't ignore it if the first
5335+                // message ends up containing multiple addresses.
5336+                std::vector<CAddress> self_announcement {local_addr};
5337+                if (peer.m_wants_addrv2) {
5338+                    MakeAndPushMessage(node, NetMsgType::ADDRV2, CAddress::V2_NETWORK(self_announcement));

w0xlt commented at 2:10 am on January 7, 2026:

This new direct-send path bypasses the addr.IsValid() and IsAddrCompatible(peer, addr) checks performed by PushAddress(). Could this be an issue?

p2p: send first addr self-announcement in separate message 🎄 #34146

Code Coverage & Benchmarks

Reviews