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

pull 0xB10C wants to merge 1 commits into bitcoin:master from 0xB10C:2025-10-addr-token-bucket-start-5 changing 2 files +5 −4
  1. 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.

  2. 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
  3. DrahtBot added the label P2P on Oct 24, 2025
  4. 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 copy-paste <!–meta-tag:bot-skip–> into the comment that the bot should ignore.

  5. 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.

  6. 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?

  7. fanquake commented at 10:47 am on October 27, 2025: member
    cc @ajtowns
  8. 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.

  9. 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?

  10. 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.

  11. 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.

  12. 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?
  13. 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.

  14. 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.

  15. 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).

  16. 0xB10C referenced this in commit ef983335da on Dec 3, 2025
  17. 0xB10C commented at 7:05 pm on December 3, 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.

    Thanks for that digging, I found that helpful.

    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.

    I’m actually not sure yet if any addresses my nodes rate-limited are actually self-announcements. Looking at the logs, there are certainly initial addr(v2) messages I received shortly after connection open (from inbound peers) with at least one address being rate-limited by me. However, I think if the peer doesn’t self-announce (e.g. -listen=0), these could be just early normal address relay messages. I’ve set up a few nodes with this patch to log some of the rate-limitings https://github.com/0xB10C/bitcoin/commit/159b6fd80d8a31c01f3b62d9194cf926c007dc37 and hope to compare the rate-limited address to the address of the peer. Let’s see…

    Would it make sense to change the logic to be:

    I picked that up in https://github.com/0xB10C/bitcoin/tree/2025-11-self-advertise-in-separate-message - but will hold off PRing it until I’m sure we actually rate-limit self-announcements. Most of the work was the function test for the self-announcements, which is good to have regardless1.

    1. Note that the snipped above contains a subtle bug which isn’t catched by one of our tests (but would be by the mentioned functional test): MakeAndPushMessage(node, NetMsgType::ADDR, CAddress::V1_NETWORK(local_addr)); where local_addr is a CAddress compiles, but it should be a vector of CAddress with one element. This will send out the serialized address alone, which isn’t valid content for an addr message. ↩︎

  18. 0xB10C commented at 7:24 pm on December 4, 2025: contributor

    Saw one today with https://github.com/0xB10C/bitcoin/commit/159b6fd80d8a31c01f3b62d9194cf926c007dc37. Will wait for a few more.

    0Rate-limited addr 98.97.137.54:8333 SELF-ANNOUNCEMENT!? from peer=2324 addr=98.97.137.54:46196 ua=/Satoshi:22.0.0(FutureBit-Apollo-Node)/ processed=1 rate-limited=1 age=478ms type=inbound

    edit: saw more, but interestingly only from older releases e.g. /Satoshi:23.0.0/?

     0Rate-limited addr 159.26.100.23:8333 SELF-ANNOUNCEMENT!? from peer=6959 addr=159.26.100.23:44574 ua=/Satoshi:23.0.0/ processed=1 rate-limited=1 age=1396ms type=inbound
 1Rate-limited addr 86.48.14.196:8333 SELF-ANNOUNCEMENT!? from peer=3645 addr=86.48.14.196:24948 ua=/Satoshi:22.0.0/ processed=1 rate-limited=1 age=313ms type=inbound
 2Rate-limited addr 103.251.26.153:8333 SELF-ANNOUNCEMENT!? from peer=19504 addr=103.251.26.153:59551 ua=/Satoshi:22.0.0/ processed=1 rate-limited=1 age=252ms type=inbound
 3Rate-limited addr 92.167.10.252:8333 SELF-ANNOUNCEMENT!? from peer=20975 addr=92.167.10.252:51644 ua=/Satoshi:22.0.0/ processed=1 rate-limited=1 age=614ms type=inbound
 4Rate-limited addr 150.228.85.13:8333 SELF-ANNOUNCEMENT!? from peer=12338 addr=150.228.85.13:11419 ua=/Satoshi:22.0.0/ processed=1 rate-limited=1 age=477ms type=inbound
 5
 6# LinkingLion seems to heavily self announce their node IPs and seems to be getting rate limited:
 7# see https://bnoc.xyz/t/linkinglion-an-entity-linking-bitcoin-transactions-to-ips/12/7?u=b10c
 8Rate-limited addr 199.116.84.88:8333 SELF-ANNOUNCEMENT!? from peer=7620 addr=199.116.84.88:17165 ua=/btcwire:0.5.0/Satoshi:25.0.0/ processed=0 rate-limited=550 age=798ms type=inbound
 9Rate-limited addr 199.116.84.173:8333 SELF-ANNOUNCEMENT!? from peer=7624 addr=199.116.84.173:15987 ua=/btcwire:0.5.0/Satoshi:25.0.0/ processed=0 rate-limited=689 age=1223ms type=inbound
10Rate-limited addr 199.116.84.7:8333 SELF-ANNOUNCEMENT!? from peer=7854 addr=199.116.84.7:27880 ua=/btcwire:0.5.0/Satoshi:25.0.0/ processed=0 rate-limited=161 age=712ms type=inbound
11Rate-limited addr 199.116.84.147:8333 SELF-ANNOUNCEMENT!? from peer=7873 addr=199.116.84.147:4150 ua=/btcwire:0.5.0/Satoshi:25.0.0/ processed=0 rate-limited=176 age=1090ms type=inbound
12Rate-limited addr 199.116.84.185:8333 SELF-ANNOUNCEMENT!? from peer=7881 addr=199.116.84.185:20012 ua=/btcwire:0.5.0/Satoshi:25.0.0/ processed=0 rate-limited=674 age=1297ms type=inbound
13Rate-limited addr 199.116.84.185:8333 SELF-ANNOUNCEMENT!? from peer=7890 addr=199.116.84.185:37238 ua=/btcwire:0.5.0/Satoshi:25.0.0/ processed=0 rate-limited=693 age=879ms type=inbound
14Rate-limited addr 199.116.84.185:8333 SELF-ANNOUNCEMENT!? from peer=7958 addr=199.116.84.185:3673 ua=/btcwire:0.5.0/Satoshi:25.0.0/ processed=0 rate-limited=61 age=707ms type=inbound
15Rate-limited addr 199.116.84.185:8333 SELF-ANNOUNCEMENT!? from peer=8159 addr=199.116.84.185:32178 ua=/btcwire:0.5.0/Satoshi:25.0.0/ processed=0 rate-limited=348 age=483ms type=inbound
16Rate-limited addr 199.116.84.185:8333 SELF-ANNOUNCEMENT!? from peer=8396 addr=199.116.84.185:10842 ua=/btcwire:0.5.0/Satoshi:25.0.0/ processed=0 rate-limited=669 age=1342ms type=inbound
17Rate-limited addr 199.116.84.185:8333 SELF-ANNOUNCEMENT!? from peer=8491 addr=199.116.84.185:34938 ua=/btcwire:0.5.0/Satoshi:25.0.0/ processed=0 rate-limited=331 age=20710ms type=inbound
18Rate-limited addr 199.116.84.185:8333 SELF-ANNOUNCEMENT!? from peer=8910 addr=199.116.84.185:54696 ua=/btcwire:0.5.0/Satoshi:25.0.0/ processed=0 rate-limited=567 age=1098ms type=inbound
19Rate-limited addr 199.116.84.185:8333 SELF-ANNOUNCEMENT!? from peer=8946 addr=199.116.84.185:6062 ua=/btcwire:0.5.0/Satoshi:25.0.0/ processed=0 rate-limited=605 age=1203ms type=inbound
20Rate-limited addr 199.116.84.203:8333 SELF-ANNOUNCEMENT!? from peer=9049 addr=199.116.84.203:19824 ua=/btcwire:0.5.0/Satoshi:25.0.0/ processed=0 rate-limited=517 age=1240ms type=inbound
  19. 0xB10C referenced this in commit c1514bf306 on Dec 10, 2025
  20. 0xB10C commented at 9:33 am on December 10, 2025: contributor

    I picked that up in https://github.com/0xB10C/bitcoin/tree/2025-11-self-advertise-in-separate-message - but will hold off PRing it until I’m sure we actually rate-limit self-announcements. Most of the work was the function test for the self-announcements, which is good to have regardless1.

    Opened a PR for the self-announcement test in #34039

    The logic change (https://github.com/bitcoin/bitcoin/commit/c1514bf3062dd4cc632a28a06c04f55b33f864c0) might be good to be explicit about our self-announcement in case we somehow start sending them along with other addresses, but doesn’t seem to be needed now. I will keep an eye on rate-limited self-announcements for a bit longer to see if anything changes my impression there.

  21. 0xB10C commented at 10:05 am on December 10, 2025: contributor

    I’m becoming less sure that this PR is actually worthwhile doing. I’ve not seen us rate-limit a self-announcement from a recent node version yet. For all other addresses (that aren’t direct self-announcements from our peer) it probably hardly matters if we randomly rate-limit a few gossiped addresses.

    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.

    Agree, generally, it might be better to keep the start token at 1 to make it more costly for an attacker to send us addresses that we will process. Especially with e.g. LinkingLion now shifting behavior to mass self-announce IPs of their new, listening nodes.

    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?

    Yet another approach: On the sender side in RelayAddress(), pick peers where we expect that the tokens the other side has for us (i.e. 1 + connection_age_in_sec/10) is more than the addresses we relayed to it. However, future changes to the start token might break this assumption for old nodes.

    Going to close this PR for now. Might reopen if there are new insights from collected data.

  22. 0xB10C closed this on Dec 10, 2025
  23. mzumsande commented at 4:09 pm on December 10, 2025: contributor
    I think the suggestion by @ajtowns (https://github.com/bitcoin/bitcoin/pull/33699#issuecomment-3462287763) to send the first self-advertisement separately would make sense on its own - currently it will usually (I think it depends a bit on timing) get mixed up with the GETADDR answer and replaces one of the 1000 addrs from it, which isn’t clean.


0xB10C DrahtBot mzumsande Crypt-iQ fanquake ajtowns glozow

Labels
P2P

github-metadata-mirror

This is a metadata mirror of the GitHub repository bitcoin/bitcoin. This site is not affiliated with GitHub. Content is generated from a GitHub metadata backup.
generated: 2025-12-19 03:13 UTC
This site is hosted by @0xB10C
More mirrored repositories can be found on mirror.b10c.me