p2p: remove m_getaddr

naiyoma commented at 12:04 pm on March 3, 2026: contributor

This PR removes m_getaddr_sent, as it no longer behaves as originally intended. Initially, this flag was meant to track when a getaddr message was sent to a peer.

Now that self-announcements are sent separately from getaddr responses, the self-announcement sets m_getaddr_sent to false even though we are still waiting for the getaddr response (1000 addresses). When the getaddr response does arrive, we rely on the size of the addr message, not the flag, to decide whether it should be relayed. This makes the flag redundant.

This is the current behavior:

The initial self-announcement is not relayed but it flips m_getaddr_sent to be false
We use addr.size() to avoid relaying (1000) getaddr response.(assuming the other two flags are false)
The first addr message is relayed because the flag is false(this was not the case before #34146).

Removing this flag ensures that:

The initial self-announcement is relayed.
The getaddr response is still not relayed, using the existing size-based check.

I had initially considered an alternative approach https://github.com/naiyoma/bitcoin/pull/14, where the self-announcement would not affect this flag and would therefore retain the initial behaviour, but i decided to reattempt its removal, as this was previously attempted, see #19794. Given the changes since then, I believe revisiting it is now more appropriate.

DrahtBot added the label P2P on Mar 3, 2026

DrahtBot commented at 12:04 pm on March 3, 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
Concept NACK	taki-abedesselam
Concept ACK	w0xlt, mzumsande, danielabrozzoni
Stale ACK	Crypt-iQ, stratospher

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

Conflicts

No conflicts as of last run.

naiyoma commented at 12:06 pm on March 3, 2026: contributor

Sharing this here in case it’s helpful for review.

This PR also addresses the concern raised here → #34146 (comment). I found the comment a bit confusing, as I didn’t fully understand how the self-announcement is disadvantaged. That said, the flag being set incorrectly is true.
The issue discussed here -> #19794 (comment) Removing this flag means that we can relay a (small <= 10) getaddr reponse if an addrman is very small , maybe this is still an issue, but this flag will not prevent this from happening.
If you cherry-pick this commit 481135858a743bc28fec20a45828293eb6512a18 you can test that this assertion https://github.com/bitcoin/bitcoin/blob/master/test/functional/p2p_addr_relay.py#L189 passes even without the flag, hence the deletion
Some logs with the messages in order:

 02026-02-25T10:48:57Z [net] ADDR recv: msg [#1](/bitcoin-bitcoin/1/) from peer=15, contains 1 addresses
 12026-02-25T10:48:57Z [net] ADDR msg [#1](/bitcoin-bitcoin/1/) from peer=15: 1 received, 1 processed, 0 rate-limited, 0 relayed
 22026-02-25T10:48:57Z [net] ADDR state: peer=15 m_getaddr_sent=true
 3
 42026-02-25T10:49:05Z [net] ADDR recv: msg [#2](/bitcoin-bitcoin/2/) from peer=15, contains 999 addresses
 52026-02-25T10:49:05Z [net] ADDR msg [#2](/bitcoin-bitcoin/2/) from peer=15: 999 received, 999 processed, 0 rate-limited, 0 relayed
 62026-02-25T10:49:05Z [net] ADDR state: peer=15 m_getaddr_sent=false
 7
 82026-02-25T10:49:28Z [net] ADDR recv: msg [#3](/bitcoin-bitcoin/3/) from peer=15, contains 2 addresses
 92026-02-25T10:49:28Z [net] ADDR relay: [2a02:908:530:c1a0:bd2e:450e:8330:e0f2]:8333 from msg [#3](/bitcoin-bitcoin/3/) (peer=15)
102026-02-25T10:49:28Z [net] ADDR relay: j3og3zy5og3acpbcwl7jncmcobhfk66bmrphcev7ct6pjc765zi4t5id.onion:8333 from msg [#3](/bitcoin-bitcoin/3/) (peer=15)
112026-02-25T10:49:28Z [net] ADDR msg [#3](/bitcoin-bitcoin/3/) from peer=15: 2 received, 2 processed, 0 rate-limited, 2 relayed
122026-02-25T10:49:28Z [net] ADDR state: peer=15 m_getaddr_sent=false

msg # 1 message one is the self announcement

msg # 2 getaddr response

msg # 3 addresses to relay

w0xlt commented at 3:57 pm on March 3, 2026: contributor

Interesting. Concept ACK.

chriszeng1010 commented at 8:48 pm on March 3, 2026: none

concept ack.

in test/functional/p2p_addr_relay.py:182 in 9e34acb1db outdated

187-        # large GETADDR responses from being relayed, it now typically affects the self-announcement
188-        # of the outbound peer which is often sent before the GETADDR response.
189-        assert_equal(inbound_peer.num_ipv4_received, 0)
190-
191-        # Send an empty ADDR message to initialize address relay on this connection.
192+

stratospher commented at 9:47 am on March 5, 2026:

9e34acb: nit: I think keeping this comment “# Send an empty ADDR message to initialize address relay on this connection.” is useful.

Crypt-iQ commented at 3:28 pm on March 5, 2026:

I agree, it took me a minute to figure out what was going on here.

Also, unrelated to this PR, I noticed the outbound p2p connections are sending GETADDR that will get ignored.

naiyoma commented at 10:59 am on March 6, 2026:

comment added back b5914990464e581731f8214c0ff1c3cb6b58cb94

naiyoma commented at 3:45 pm on March 6, 2026:

Also, unrelated to this PR, I noticed the outbound p2p connections are sending GETADDR that will get ignored.

I think its because -> https://github.com/bitcoin/bitcoin/blob/master/src/net_processing.cpp#L4911

danielabrozzoni commented at 3:18 pm on March 25, 2026:

Also, unrelated to this PR, I noticed the outbound p2p connections are sending GETADDR that will get ignored.

Can you clarify? I don’t see how this can happen.

We only send a GETADDR to our outbound, non-block-relay peers:

https://github.com/bitcoin/bitcoin/blob/2fe76ed8324af44c985b96455a05c3e8bec0a03e/src/net_processing.cpp#L3749-L3767

And we only reply to GETADDR requests if they’re from inbound connections:

https://github.com/bitcoin/bitcoin/blob/2fe76ed8324af44c985b96455a05c3e8bec0a03e/src/net_processing.cpp#L4816-L4825

So, I think this happens:

(–> means the direction of the connection, so outbound for the node on the left, inbound for the node on the right)

0+------+            +------+
1| Node | ---------> | Node |
2+------+            +------+
3         -> getaddr
4         <- addr

Crypt-iQ commented at 3:28 pm on March 25, 2026:

Can you clarify? I don’t see how this can happen.

The test does it.

danielabrozzoni commented at 2:28 pm on March 26, 2026:

Got it, sorry for the confusion

stratospher commented at 10:49 am on March 5, 2026: contributor

ACK 9e34acb1. nice find!

m_getaddr_sent logic is broken after #34146 and doesn’t do what it’s supposed to do anymore.

i think removing it is a logical bug fix which would be nice to have in v31. (not sure if it’s too late for that)

there would be a few behaviour changes with this PR:

(might be debatable change) we would relay ADDR messages to peers just based on size ≤10. feel it’s ok because:
1. is it ok if someone send 5 ADDR as the GETADDR response (instead of the usual 1000 ADDR) and we relay it?
  1. we do GETADDR requests only to outbound peers ( so unlikely for it to be attacker controlled).
  2. in some remote scenario (where we got eclipsed) , suppose an attacker is chosen as our outbound peer - and it only sends us 5 ADDR as GETADDR response. we will relay it to our other peers once during the start of node’s lifecycle (wouldn’t have relayed these 5 ADDR before this PR). we also will have bigger problems to worry about than if address got relayed.
  3. there are much more effective ways for an attacker to force certain ADDR to be relayed than focus on the response to GETADDR - a normal node only sends GETADDR once during the version handshake.
(good change -fixes an inconsistent behaviour on master) currently we don’t relay the 1st ADDR message received from an outbound peer with size ≤ 10. now we relay it.
(good change) initial self announcement always gets relayed.

fanquake commented at 3:50 pm on March 5, 2026: member

cc @mzumsande

Crypt-iQ commented at 3:59 pm on March 5, 2026: contributor

crACK 9e34acb1dbff61efb1c5e4e9cfce2a4591c2832f

The first addr message is relayed because the flag is false(this was not the case before #34146).

Can you elaborate? It seems like the very first addr message was not being relayed, like you said?

fanquake commented at 10:20 am on March 6, 2026: member

cc @0xB10C

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

naiyoma force-pushed on Mar 6, 2026

naiyoma commented at 11:29 am on March 6, 2026: contributor

(good change -fixes an inconsistent behaviour on master) currently we don’t relay the 1st ADDR message received from an outbound peer with size ≤ 10. now we relay it.

Thanks for reviewing A quick clarification on this: we started relaying the first ADDR message after PR 34146, not this PR. This is because the self-announcement sets this flag to false. If you look at the logs here -> #34717 (comment) The first ADDR message, which is message 3 (msg # 3) that you are referring to, is being relayed, also, notice that by the time we receive this message, m_getaddr_sent = false, so the relay condition is true.

naiyoma commented at 11:42 am on March 6, 2026: contributor

crACK 9e34acb

The first addr message is relayed because the flag is false(this was not the case before #34146).

Can you elaborate? It seems like the very first addr message was not being relayed, like you said?

For the first regular ADDR after the GETADDR response, this condition was false because m_getaddr_sent was true https://github.com/bitcoin/bitcoin/blob/01651324f4e540f6b96cff31e89752c3f9417293/src/net_processing.cpp#L4074

mzumsande commented at 11:45 am on March 6, 2026: contributor

Concept ACK

Just making clear that while #34146 had the effect that m_get_addr_sent doesn’t match the GETADDR answer anymore, it didn’t make anything worse: the flag was already unnecessary before (due to the <10 criterion), and the first self-announcement (when it was mixed into the 1000 size GETADDR answer) did not get relayed to peers before either.

So I’d say this PR doesn’t correct a bug introduced by #34146, but improves logic that was already broken before.

ajtowns commented at 12:51 pm on March 6, 2026: contributor

I think the intended logic is (was) “we sent a getaddr, therefore the next addr message we receive will probably be old information which we shouldn’t relay further”.

So #34146 did introduce a “bug” in that it became no longer true that next next addr message contained old information – instead it would contain a self-announcement. That would then clear the flag, and we’d risk relaying the old information in the following addr message, that was in response to the getaddr request. The < 10 check and the 10 minute checks both probably made the impact of that “bug” pretty rare/trivial however. So I don’t think this is urgent for 31.0.

I think this change is fine though – the 10min and <10 check seem already sufficient: if we get a small response to our getaddr, that’s full of (honestly) recent information, then relaying that is either good or at worst harmless; and if an attacker were trying to exploit that ability, they could simply have sent a dummy addr message, and the real attack data later, avoiding the m_get_addr_sent gate entirely.

I’m not sure that improved relay of self-announcements of nodes that only just connected to you is necessarily a good thing. Currently (and prior to #34146) your node’s IP would likely only be relayed reliably once you’ve been connected to a peer for a couple of hours or so, which serves as an automatic way of avoiding cluttering up everyone’s address book with (honest) nodes that start up, sync, and shut down. So Concept ~0. Code looks fine if the behaviour is desirable, though.

mzumsande commented at 1:26 pm on March 6, 2026: contributor

I’m not sure that improved relay of self-announcements of nodes that only just connected to you is necessarily a good thing.

Does this have the direction mixed up? This is about improved relay of self-announcements originating from peers that we connected to - we, the initiating node, issue the GETADDR request, set m_get_addr_sent and process the answer / the self-announcement of that peer and maybe relay it further. So the affected self-announcements are those from reachable, usually reliable nodes (after all we picked them somehow).

The reverse direction (we, a possibly new/unreliable node, connect to a peer, and our own self-advertisment gets relayed by that peer or not) is not affected by this PR at all I think because this would involve m_getaddr_recvd instead of m_get_addr_sent.

Crypt-iQ commented at 2:34 pm on March 6, 2026: contributor

reACK b5914990464e581731f8214c0ff1c3cb6b58cb94

DrahtBot requested review from mzumsande on Mar 6, 2026

DrahtBot requested review from stratospher on Mar 6, 2026

ajtowns commented at 5:26 pm on March 6, 2026: contributor

Does this have the direction mixed up? This is about improved relay of self-announcements originating from peers that we connected to - we, the initiating node, issue the GETADDR request, set m_get_addr_sent and process the answer / the self-announcement of that peer and maybe relay it further. So the affected self-announcements are those from reachable, usually reliable nodes (after all we picked them somehow).

Yeah, it does have the direction mixed up. Still not sure how much benefit it makes to relay the initial self-announcement though. If they’re already reachable/reliable nodes, then their address is already well known, and only getting re-announced by long-term connections is probably not terrible? I guess if it weren’t useful to relay it, the peer shouldn’t bother sending it to us – we’ve already connected successfully, so we already have a working address for the peer and don’t need to be told; so why else would they send it unless they wanted it relayed so others could more easily connect to them? Delaying the self-announcement until the connection’s been alive a random amount of time would obviously be possible on the sender’s side; (eg suggested here) so why treat it specially on the receiver side?

I guess this doesn’t have privacy implications – I don’t think you can distinguish a relayed self-announcement between being one of our outbounds or one of our inbounds; and distinguishing an addr relay for a direct connection vs a random forwarded addr seems like it requires a long-term connection and probably a lot of probability analysis?

From #19794 (comment) :

(I wish the response to a getaddr was a different message type than an unsolicited address being relayed to us!)

I suppose that’s something we could consider. I suppose we could also consider limiting our GETADDR responses to addr entries that are more than 10min old, or artificially inflating the ages of the addresses we send to ensure they’re more than 10min old (and perhaps documenting that in a BIP so other node software that cares about the privacy concerns raised in that comment can implement compatible logic).

taki-abedesselam commented at 2:11 am on March 9, 2026: none

Concept NACK

I do not agree with this solution for removing m_getaddr_sent. Instead we should consider reusing it. I believe there is a significant issue in the bitcoin code: it gives a malicious neighbor node the ability to abuse our unsolicited message relaying. This could happen as follows :

We establish an outbound connection to a node X.
We increase the token counter to +1000 tokens since we expect to receive up to 1000 addresses in response from node X.
Node X sends a buffer containing only 1 address, to bypasses the initial m_getaddr_sent check (or this protection disappears entirely if the variable is removed as proposed in this PR)
Node X then starts sending unsolicited messages of size 10, since it knows it can use up to 1000 tokens (each address they will consume 1 token and if the number of addresses in the buffer is less or equal to 10 our node will relay it).
Our node become an intermediate relay, unintentionally helping hide this malicious node, since its (our node) the one who will relay the unsolicited messages.
Our node they will consume all of its tokens with other nodes by forwarding malicious addresses instead of forwarding addresses for legitimate nodes.

For these reasons, instead of removing m_getaddr_sent, we should keep it and improve the token handling logic for GETADDR messages.

Crypt-iQ commented at 12:36 pm on March 9, 2026: contributor

I do not agree with this solution for removing m_getaddr_sent. Instead we should consider reusing it. I believe there is a significant issue in the bitcoin code: it gives a malicious neighbor node the ability to abuse our unsolicited message relaying.

Can’t this be done independent of this patch? Also, the GETADDR is only sent for outbound connections so its impact is limited?

ajtowns commented at 1:20 pm on March 9, 2026: contributor

We increase the token counter to +1000 tokens since we expect to receive up to 1000 addresses in response from node X.

I think this doesn’t make sense – if we want to treat addresses received as a response to “GETADDR” differently, then m_getaddr_sent should be a bucket that’s normally 0, but is bumped to 1000 when we send “GETADDR”. Then the idea might be:

 0    bool relay{true};
 1    if (peer.m_addr_token_bucket < 1.0) {
 2        if (rate_limited) {
 3            if (m_getaddr_bucket == 0) {
 4                ++num_rate_limit; continue;
 5           } else {
 6                 --m_getaddr_bucket;
 7                relay = false;
 8           }
 9        }
10    } else {
11        peer.m_addr_token_bucket -= 1.0;
12    }
13    ...
14    if (relay && addr.nTime > current_a_time - 10min && vAddr.size() <= 10 && addr.IsRoutable()) {
15        // Relay to a limited number of other nodes
16        RelayAddress(pfrom.GetId(), addr, reachable);
17    }

naiyoma commented at 4:42 pm on March 9, 2026: contributor

This doesn’t need to be part of v31. As mentioned above, even though it is possible for us to relay a GETADDR response currently, the probability is low because the default node behaviour will almost always have around 1000 addresses (> 10 addresses, hence we don’t relay), and GETADDR responses are usually older.

fanquake removed this from the milestone 31.0 on Mar 9, 2026

naiyoma commented at 4:48 pm on March 9, 2026: contributor

I do not agree with this solution for removing m_getaddr_sent. Instead we should consider reusing it. I believe there is a significant issue in the bitcoin code: it gives a malicious neighbor node the ability to abuse our unsolicited message relaying. This could happen as follows :

This attack is still possible with or without this flag in normal address relay, not just in response to GETADDR.

stratospher commented at 5:34 pm on March 9, 2026: contributor

reACK b591499.

agree regarding it not being important for v31 - it’s just an extra address relay anyways. I thought of this as a logical couple to #34150 - we’re changing initial self announcement sending logic in that PR - so makes sense that the receiving logic code is also updated which is why I initially suggested it for v31. but it’s just an inconsistency in our code and won’t matter to people running the software.

I guess the initial self announcement being relayed + this not being done before is what’s causing concern. There’s the alternative (https://github.com/naiyoma/bitcoin/pull/14) which naiyoma proposed of making m_get_addrsent more strict - but it can’t act as a proper gate anyways and might as well remove it.

also liked aj’s alternative suggestion of making sure GETADDR responses are actually old when selecting from addrman. I collected some address relay stats of my node last month. don’t quote me on this since I haven’t cross checked with other GETADDR responses. this is the address staleness of the GETADDR response of 1 particular peer (GPT plotted a histogram from the data):

also worth noting - the previous version of PR in #19794 was made at a time when ADDR token logic wasn’t there + possibility of someone spamming us without any rate limiting.

taki-abedesselam commented at 5:17 am on March 11, 2026: none

then m_getaddr_sent should be a bucket that’s normally 0

good idea to separate the logic of the response from normal forwarding , could i also suggest the following: that during the first X seconds we are expecting that we will receive a self-announcement and a response to our GETADDR message, for self-announcement we could distinguish it by checking if the sender is the same as the address in the buffer + the time it will be fresh, so we will relay it. and for the other addresses we will use peer.m_addr_token_bucket and a timer tracker if now - get_addr_send_time <= X, otherwise we will switch to the normal tokens.

fanquake referenced this in commit 5608b8ce9e on Mar 12, 2026

DrahtBot added the label Needs rebase on Mar 12, 2026

naiyoma force-pushed on Mar 17, 2026

DrahtBot added the label CI failed on Mar 17, 2026

naiyoma force-pushed on Mar 17, 2026

DrahtBot removed the label Needs rebase on Mar 17, 2026

naiyoma force-pushed on Mar 17, 2026

DrahtBot removed the label CI failed on Mar 17, 2026

in test/functional/p2p_addr_relay.py:187 in 69907334df outdated

186@@ -187,13 +187,6 @@ def relay_tests(self):
187         full_outbound_peer = self.nodes[0].add_outbound_p2p_connection(AddrReceiver(), p2p_idx=0, connection_type="outbound-full-relay")

Bortlesboat commented at 5:45 pm on March 17, 2026:

Since this removes the suppression of the first addr message, could we flip the assertion rather than delete it? Something like:

0msg = self.setup_addr_msg(2)
1self.send_addr_msg(full_outbound_peer, msg, [inbound_peer])
2self.log.info('Check that the first addr message from an outbound peer is relayed')
3assert_equal(inbound_peer.num_ipv4_received, 2)

That way the behavioral change (first addr from outbound is now relayed) has positive test coverage.

naiyoma commented at 10:21 am on March 26, 2026:

@Bortlesboat, this is already being tested here -> https://github.com/naiyoma/bitcoin/blob/2026_1/m_getaddr_sent/test/functional/p2p_addr_relay.py#L192

naiyoma commented at 6:46 pm on March 17, 2026: contributor

rebased, re-ordered commits, and updated test commit message

DrahtBot added the label Needs rebase on Mar 20, 2026

test: delete redundant addr relay assertion

The assertion checks that the first addr message is not relayed
due to m_getaddr_sent being set to true.
But we now send a self-announcement as the first message,
which resets the flag to false, meaning subsequent addr messages are relayed.

a1ecda2609

net: remove m_getaddr_sent

The flag was intended to track whether we sent a GETADDR to a peer,
but it became redundant after the initial self-announcement sets it to false,
even though we still expect the actual getaddr response.

Co-authored-by: Martin Zumsande <mzumsande@gmail.com>

c8ee319fff

naiyoma force-pushed on Mar 26, 2026

naiyoma commented at 11:05 am on March 26, 2026: contributor

Rebased,

DrahtBot removed the label Needs rebase on Mar 26, 2026

in test/functional/p2p_addr_relay.py:189 in c8ee319fff

192-        # from a new outbound peer to be relayed to others. Originally meant to prevent
193-        # large GETADDR responses from being relayed, it now typically affects the self-announcement
194-        # of the outbound peer which is often sent before the GETADDR response.
195-        assert_equal(inbound_peer.num_ipv4_received, 0)
196 
197         self.log.info('Check that subsequent addr messages sent from an outbound peer are relayed')

danielabrozzoni commented at 2:27 pm on March 26, 2026:

nit: ‘subsequent’ here made sense when read after ‘Check that the first message is not relayed’. Since we are deleting that part of the test, this should be changed to something like ‘Check that every addr message sent from an outbound peer is relayed’.

danielabrozzoni commented at 2:55 pm on March 26, 2026: member

Concept ACK, code looks good, but I need to spend some more time thinking about this before ACKing :)

I think there’s a small source of confusion in the PR description, and here:

This is the current behavior:

The initial self-announcement is not relayed but it flips m_getaddr_sent to be false

We use addr.size() to avoid relaying (1000) getaddr response.(assuming the other two flags are false)

The first addr message is relayed because the flag is false(this was not the case before p2p: send first addr self-announcement in separate message 🎄 #34146).

What you’re calling “the first addr message” is really the second ADDR message, the first one being the initial self announcement. So it’s correct to say that in master the first ADDR message (the self announcement) is not relayed, and subsequent ADDR messages are relayed.

p2p: remove m_getaddr_sent #34717

Reviews

Conflicts