p2p: reduce false-positives in addr rate-limiting

0xB10C commented at 5:10 pm on October 24, 2025: contributor

Start the rate-limiter with 5 tokens instead of 1 token.

On some occasions, a peer will send us addresses a few seconds after the connection is established. As peers start with token=1 and only get an additional token only every 10 seconds, some of these addresses might be rate-limited. The following log snippet contains an example:

004:12:02 [net] Added connection to [redacted] peer=1234
1...
204:12:03 [net] Received addr: 1 addresses (1 processed, 0 rate-limited) from peer=1234
304:12:34 [net] Received addr: 4 addresses (3 processed, 1 rate-limited) from peer=1234
404:13:09 [net] Received addr: 2 addresses (2 processed, 0 rate-limited) from peer=1234

The peer starts with token=1 and uses one for its self-announcement at 04:12:03. Until the next addr message arrives at 04:12:34, it re-gains 3 tokens and uses them, however, the fourth address in this addr message is rate-limited. At 04:13:09, the peer has token=3 and uses 2.

This change gives peers token=5 to start with. To choose this value, I looked at the address relay of freshly connected peers. The data was sampled from the debug.logs from different nodes. It appears, that only very few peers require more than 5 initial tokens.

By increasing the initial tokens to 5, someone could connect, send 5 addresses, disconnect, and repeat to spam the node with addresses. Previously, the same was possible but only one address was processed. I don’t think this is a problem anymore since #30568.

p2p: reduce false-positives in addr rate-limiting

On some occasions, a peer will send us addresses a few seconds after
the connection is established. As peers start with token=1 and only
get an additional token only every 10 seconds, some of these
addresses might be rate-limited. The following log snippet contains
an example:

```
04:12:02 [net] Added connection to [redacted] peer=1234
...
04:12:03 [net] Received addr: 1 addresses (1 processed, 0 rate-limited) from peer=1234
04:12:34 [net] Received addr: 4 addresses (3 processed, 1 rate-limited) from peer=1234
04:13:09 [net] Received addr: 2 addresses (2 processed, 0 rate-limited) from peer=1234
```

The peer starts with token=1 and uses one for its self-announcement at
04:12:03. Until the next addr message arrives at 04:12:34, it re-gains
3 tokens and uses them, however, the fourth address in this addr
message is rate-limited. At 04:13:09, the peer has token=3 and uses 2.

This change gives peers token=5 to start with. To choose this value, I
looked at the address relay of freshly connected peers. The data was
sampled from the debug.logs from different nodes. It appears, that only
very few peers require more than 5 initial tokens.

(see image in PR)

By increasing the initial tokens to 5, someone could connect, send 5
addresses, disconnect, and repeat to spam the node with addresses.
Previously, the same was possible but only one address was processed.
I don't think this is a problem since we have #30568.

fdf4863d66

DrahtBot added the label P2P on Oct 24, 2025

DrahtBot commented at 5:10 pm on October 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/33699.

Reviews

See the guideline for information on the review process.

Type	Reviewers
ACK	mzumsande, Crypt-iQ

If your review is incorrectly listed, please react with 👎 to this comment and the bot will ignore it on the next update.

mzumsande commented at 6:27 pm on October 26, 2025: contributor

utACK fdf4863d66ed09a6b45fb6cd0d0c48a02e0e267d

I think that while the existing rate limit is sufficient, the observed events where the rate limit kicks in are just due to statistical fluctuation (which wouldn’t lead to rate limiting later on because the node will usually have saved up some cushion) - having fewer of those could prevent a peer’s self-announcement from being ignored, and could make monitoring a bit easier.

Crypt-iQ commented at 11:24 pm on October 26, 2025: contributor

crACK fdf4863d66ed09a6b45fb6cd0d0c48a02e0e267d

The graph is helpful, I wonder what’s going on with those peers above and to the left of the staircase?

fanquake commented at 10:47 am on October 27, 2025: member

cc @ajtowns

0xB10C commented at 3:42 pm on October 27, 2025: contributor

I wonder what’s going on with those peers above and to the left of the staircase?

It can happen that a (Bitcoin Core) peer sends us an addr message with more than X entries by random chance. Observed from the logs I looked at:

 0chance >= 1 entries 	 100.00%
 1chance >= 2 entries 	 37.62%
 2chance >= 3 entries 	 19.14%
 3chance >= 4 entries 	 10.16%
 4chance >= 5 entries 	 5.66%
 5chance >= 6 entries 	 3.28%
 6chance >= 7 entries 	 1.98%
 7chance >= 8 entries 	 1.25%
 8chance >= 9 entries 	 0.81%
 9chance >= 10 entries 	 0.55%
10chance >= 11 entries 	 0.39%
11chance >= 12 entries 	 0.28%
12chance >= 13 entries 	 0.21%
13chance >= 14 entries 	 0.16%
14chance >= 15 entries 	 0.13%

So there’s a 3.28% chance that a peer sends us a addr message with more than 5 addr. If this message arrives <10s after connection open, the peer does not have enough token yet. However, it’s also unlikely that we choose to relay >5 addresses to this peer in <10s.

This is a hand-picked example from a peer (that claims to be v29.0) that sent some larger addr messages early on:

001:08:43 [net] Added connection to X peer=9668
101:09:05 [msghand] Received addr: 8 addresses (3 processed, 5 rate-limited) from peer=9668
201:10:25 [msghand] Received addr: 11 addresses (8 processed, 3 rate-limited) from peer=9668
301:10:48 [msghand] Received addr: 3 addresses (2 processed, 1 rate-limited) from peer=9668
401:11:38 [msghand] Received addr: 9 addresses (5 processed, 4 rate-limited) from peer=9668
501:12:13 [msghand] Received addr: 3 addresses (3 processed, 0 rate-limited) from peer=9668
601:13:09 [msghand] Received addr: 8 addresses (6 processed, 2 rate-limited) from peer=9668
701:13:55 [msghand] Received addr: 2 addresses (2 processed, 0 rate-limited) from peer=9668

Here’s an overview of the number addr message entries peers send to me and were processed or rate-limited. Nearly all of the rate-limited entries were send in the first 60 seconds.

ajtowns commented at 3:27 pm on October 28, 2025: contributor

On some occasions, a peer will send us addresses a few seconds after the connection is established.

What’s the drawback of ignoring these addresses? If we’d connected a few seconds/a minute later, we’d have missed them anyway.

An advantage of starting with an (almost) empty bucket is that someone attempting an address spamming attack has to pay for that attack by maintaining a connection to us. Increasing the initial bucket for outbound/manual connections but not inbound connections might be better?

mzumsande commented at 3:50 pm on October 28, 2025: contributor

What’s the drawback of ignoring these addresses? If we’d connected a few seconds/a minute later, we’d have missed them anyway.

Ignoring the initial self-announcement of the peer if it get mixed up with other addrs would feel strange.

Crypt-iQ commented at 6:43 pm on October 28, 2025: contributor

Ignoring self-announcements seems to occur only in the time between our inbound peer (outbound from their perspective) 1) calling SetupAddressRelay when receiving version, and 2) receiving verack where it will set fSuccessfullyConnected = true and call SendMessages followed by MaybeSendAddr. The address entries are sent in order of insert, but then shuffled before processing since #22387 (see rationale). If they weren’t shuffled on receipt, I think the self-announcement would always be processed.

If we are being connected to, I think our self-announcement won’t be rate-limited because we’ll wait until receiving getaddr, call SetupAddressRelay, and then in SendMessages, our self-announcement will replace one of the existing entries in the getaddr response. There’s no time-gap in this case afaict.

glozow commented at 8:12 pm on October 28, 2025: member

If these are Bitcoin Core nodes (so our sender-side logic is seemingly incompatible with our receiver-side logic), is an alternative approach to increase the delay after which we’d send addrs?

ajtowns commented at 3:33 pm on October 29, 2025: contributor

What’s the drawback of ignoring these addresses? If we’d connected a few seconds/a minute later, we’d have missed them anyway.

Ignoring the initial self-announcement of the peer if it get mixed up with other addrs would feel strange.

Yeah, you’re right. Though as glozow points out, it’s also strange to have our sender/receiver side logic randomly mismatch depending on network behaviour (with this PR making the random mismatch much less frequent). Would it make sense to change the logic to be:

 0void PeerManagerImpl::MaybeSendAddr(CNode& node, Peer& peer, std::chrono::microseconds current_time)
 1{
 2    ...
 3    if (... && peer.m_next_local_addr_send < current_time) {
 4        ...   
 5        if (peer.m_next_local_addr_send != 0us) {
 6            peer.m_addr_known->reset();
 7        }
 8        if (std::optional<CService> local_service = GetLocalAddrForPeer(node)) {
 9            CAddress local_addr{*local_service, peer.m_our_services, Now<NodeSeconds>()};
10            if (peer.m_next_local_addr_send == 0us) {
11                // first local addr announcement, send as a single message
12                if (peer.m_wants_addrv2) {
13                    MakeAndPushMessage(node, NetMsgType::ADDRV2, CAddress::V2_NETWORK(local_addr));
14                } else {
15                    MakeAndPushMessage(node, NetMsgType::ADDR, CAddress::V1_NETWORK(local_addr));
16                }
17            } else {
18                // normally, include local addr with any others queued up
19                PushAddress(peer, local_addr);
20            }
21        }
22        peer.m_next_local_addr_send = current_time + m_rng.rand_exp_duration(AVG_LOCAL_ADDRESS_BROADCAST_INTERVAL);
23    }
24    ...

so that we push our local address as a single message first, using up the bucket preallocation, then allow other addresses to be sent and rate-limited as normal? That seems like it would automatically help existing nodes not rate-limit self announcements from future releases.

ajtowns commented at 3:35 pm on October 29, 2025: contributor

If they weren’t shuffled on receipt, I think the self-announcement would always be processed.

I think shuffling helps avoid an attacker being able to (a) prevent honest nodes’ addresses from being relayed (by filling up the buffer, so that later honest announcements get dropped) and (b) (easily?) detect how many addresses are being rate limited. So I don’t think removing the shuffle would be a good idea.

mzumsande commented at 7:15 pm on October 29, 2025: contributor

If we are being connected to, I think our self-announcement won’t be rate-limited because we’ll wait until receiving getaddr, call SetupAddressRelay, and then in SendMessages, our self-announcement will replace one of the existing entries in the getaddr response. There’s no time-gap in this case afaict.

True, I have observed this effect too recently, it isn’t really clean though - I don’t think anyone intended it to work such that a GETADDR answers would contain 999 addresses from addrman and a self announcement replacing a random one - so aj’s suggestion makes change to me.

If these are Bitcoin Core nodes (so our sender-side logic is seemingly incompatible with our receiver-side logic), is an alternative approach to increase the delay after which we’d send addrs?

That should work too, but it would also slow things down if the receiver waits for addrs (ADDRFETCH connections).

p2p: reduce false-positives in addr rate-limiting #33699

Code Coverage & Benchmarks

Reviews