fix: several improvements to how AddrMan works by Davidson-Souza · Pull Request #781 · getfloresta/Floresta

Davidson-Souza · 2026-01-13T17:27:36Z

Description and Notes

Fixes #853

This PR improves our AddrMan, and also makes our node more aggressive on requesting for new addresses. With these changes:

We always ask for new addresses when we get a new connection, even with feeler peers
We now limit how many addresses can we have in total, preventing a potential memory exhaustion DoS
We also evict the peers that we didn't successfully connect in a while, to free up more space
Limit how many addresses we can receive/send in a single AddrV2 message
Add nodes added though the CLI to AddrMan
Added several tests for AddrMan
Don't store addresses that are unreachable from our node
Rate limit AddrV2 messages to one every ten seconds
Added addresses from DNS seeds to the good addresses
Fixed a small bug where connected addresses wouldn't be seen as good until they disconnect

Davidson-Souza · 2026-01-14T15:06:00Z

Pushed 8427273:

Move the 1k addresses to a constant in node.rs
Don't keep addresses for networks we can't reach - Right now we only really do ip, other networks proper handling will come in a follow-up
Don't remove peers from the good addresses table too quickly
Add an extra address range as private, as it seems to not be checked by Rust's std lib

brunoerg · 2026-01-16T12:27:44Z

Also, it would be great to implement a rate-limiting for addrv2 messages.

erickcestari · 2026-01-16T18:57:37Z

Also, it would be great to implement a rate-limiting for addrv2 messages.

I Agree. Bitcoin Core has a strict rate limit of accepting only one address every ten seconds from a peer to avoid eclipsing.

https://github.com/bitcoin/bitcoin/blob/0f7d4ee4e8281ed141a6ebb7e0edee7b864e4dcf/src/net_processing.cpp#L191

luisschwab · 2026-01-16T20:50:57Z

While you're at it, maybe also make NodeInterface::remove_peer() also disconnect the peer we remove from the AddrMan? I'm getting this error when attempting to disconnect from a peer on bonsai:

WARN floresta_wire::p2p_wire::node: PeerNotFoundAtAddress(178.250.189.42, 38333)
ERROR bdk_floresta: Failed to manually disconnect from peer 178.250.189.42:38333

It claims the peer is not found even though I'm literally connected to it.

jaoleal

Complementing with some suggestions

jaoleal · 2026-01-17T15:49:18Z

+            good_peers_by_service: HashMap::new(),
+            peers_by_service: HashMap::new(),
+            max_size: max_size.unwrap_or(MAX_ADDRESSES),
+            reachable_networks,


Isnt interesting to have a default for this and it being the addresses we support ?

Knowing this isn't really the address_man business, and wouldn't help much. So I would say no

Davidson-Souza · 2026-01-20T13:13:18Z

While you're at it, maybe also make NodeInterface::remove_peer() also disconnect the peer we remove from the AddrMan?

This method is used for the addnode RPC, and it shouldn't disconnect a peer. We need a new one for disconnectpeer

Davidson-Souza · 2026-01-20T21:37:30Z

Pushed 724469e

Moved the methods about address reachability to LocalAddress. I think it makes more sense to have them there
Also made them public, so they can be used for testing
Added a per-peer rate-limit for AddrV2 messages, one every ten seconds
rebased

This was annoying to rebase

erickcestari · 2026-01-23T15:03:18Z

I had to simulate the behaviour of the Bitcoin Core isRoutable() function in Rust-Bitcoin for a target in bitcoinfuzz. You might find it useful to see the additional verification that it has, which might be missing in Floresta, to ensure that it behaves equally to Core and does not waste time connecting to a non-routeable address.

https://github.com/bitcoinfuzz/bitcoinfuzz/pull/400/changes#diff-97d1c6ff1c09f64634ec511f5134bf731ad57fd139ac86c539a93d0dab8d182bR237

Davidson-Souza · 2026-01-28T18:11:46Z

After a little 1v1 against CI, pushed 3124891

Refactored prune_address based on @moisesPompilio's suggestion
Improved our reachability tests based on @erickcestari's code
Unified most reachability tests in one place

The BIP demands nodes to ignore any message with more than 1_000 addresses. We currently may receive and send packages bigger than that. This commit fixes it by enforcing this limit.

We don't currently push peers added through CLI to our AddrMan, therefore we don't learn about them to use in the future. This commit fixes it, by calling `addr_man.push` with any peer that have been added from the CLI

Some networks might not be reachable for use, usually because they require a specific proxy that we might not have. Therefore, addresses for that net are always unreachable for us. This commit adds a method to filter for unreachable addresses, so they are never added to our address manager.

We try to keep a good view of the network, knowing which peers are working a ready to connect. To accieve this, we keep track of the lastest connections we've made, so we know what peers should be online and responsive. However, after a certain time from the last connection, we assume that the information we have is too old to be counted. But our current code is too aggressive in what it considers outdated, and will drop this info for peers we've connected just a few minutes ago. This commit fixes this by preserving this information for one day (24 hours) before considering it stale and moving that peer to the unknown state.

The address manager implements some methods like `is_private` and `is_good_peer` to help with peer selection. However, they are closer to the LocalAddress struct, so I've moved them to LocalAddress and made them public.

To avoid being eclipsed with an address spam attack, we limit the rate of addrv2 messages a peer can send us to one every 10 seconds.

DNS seeds are trusted to deliver recently seen peers, that are very likely to still be alive. This commit adds those peers directly to the "good" peers table, making it easier for our node to start up peers

Before connecting to an address, we mark it as `Failed`, in case we never hear from that peer again, and only after handshake we mark as `Connected`. However, before this commit, we would remove then from the `good_peers` bucket, and only re-add after disconnecting. That meant we wouldn't send their addresses in `AddrV2` messages, and they wouldn't count towards the `enough_peers` function. This commit fixes this by adding `Connected` peers back to the good peers bucket

Before this commit we would add duplicate addresses to each one of the per-service bucket. This commit checks whether the address already existed there, and only adds if not

JoseSK999

One last nit, if it makes sense to you

JoseSK999 · 2026-02-24T13:41:19Z

 impl FromStr for LocalAddress {
    fn from_str(s: &str) -> Result<Self, Self::Err> {
        LocalAddress::try_from(s)
    }
    type Err = std::net::AddrParseError;
 }

+// Note that, since we can't know the network we are operating in, this code
+// can't know what's the default port. Therefore, it will only work if you give
+// a SocketAddr, i.e. <IP:PORT>
 impl TryFrom<&str> for LocalAddress {
    fn try_from(value: &str) -> Result<Self, Self::Error> {
-        let split = value.split(':').collect::<Vec<_>>();
-        let address = split[0].parse::<Ipv4Addr>()?;
-        let port = if let Some(port) = split.get(1) {
-            port.parse().unwrap_or(8333)
-        } else {
-            8333
+        let address = value.parse::<SocketAddr>()?;
+        let ip = match address {
+            SocketAddr::V4(ipv4) => AddrV2::Ipv4(*ipv4.ip()),
+            SocketAddr::V6(ipv6) => AddrV2::Ipv6(*ipv6.ip()),
        };
+
        Ok(LocalAddress::new(
-            AddrV2::Ipv4(address),
+            ip,
            SystemTime::now()
                .duration_since(UNIX_EPOCH)
                .unwrap()
                .as_secs(),


Optional nit: we are now repeating this same logic 9 times in the file. A helper would be good:

fn time_since_unix() -> u64 { SystemTime::now() .duration_since(UNIX_EPOCH) .unwrap_or_default() .as_secs() }

Davidson-Souza · 2026-02-24T14:12:40Z

Pushed db86233

Added Cjdns to the is_net_reachable match
Added NETWORK_LIMTED to dns seed addresses
implemented and used a time_since_unix method

luisschwab · 2026-02-24T14:28:22Z

Missed linting

JoseSK999 · 2026-02-24T14:33:48Z

Suggested change

.unwrap_or_default()

Azure is so annoying

Azure is so annoying

100%

This method returns the time elapsed since the unix epoch. This logic is reused all over addr_man, so it is better to just have it somewhere and use it.

JoseSK999

ACK 21e316e

moisesPompilio · 2026-02-24T17:33:48Z

ACK 21e316e

luisschwab

ACK 21e316e

luisschwab · 2026-02-24T18:00:53Z

BOOM

In getfloresta#793 we reduced the `DNS_SEED_REQUEST_INTERVAL` to two minutes. This tells how long do we wait before re-doing a request do the DNS seeds. The goal was to make sure we had enough peers to keep going. However, it doesn't seem like it worked as intended since DNS servers are currently bad at giving utreexo peers (and on signet they are also bad at giving CBS peers). This forces our node to retry DNS requests every two minutes. But since most users don't have their own recursive resolver (I don't), the previous result is almost always cached, so you get basically the same answer several times. This pollutes stdout for no good reason. Since getfloresta#781 we now are super aggressive about asking for addresses and checking whether they are alive and learning about their capabilities and getfloresta#865 will make it even better, since we can try several feeler connections at the same time. So I think DNS seeds should be only used to get us some starting peers, and then we build our local network view from the P2P net ourselves.

0ceeb59 chore(wire): increase the dns seed grace period time (Davidson Souza) Pull request description: ### Description and Notes In #793 we reduced the `DNS_SEED_REQUEST_INTERVAL` to two minutes. This tells how long do we wait before re-doing a request do the DNS seeds. The goal was to make sure we had enough peers to keep going. However, it doesn't seem like it worked as intended since DNS servers are currently bad at giving utreexo peers (and on signet they are also bad at giving CBS peers). This forces our node to retry DNS requests every two minutes. But since most users don't have their own recursive resolver (I don't), the previous result is almost always cached, so you get basically the same answer several times. This pollutes stdout for no good reason. Since #781 we now are super aggressive about asking for addresses and checking whether they are alive and learning about their capabilities and #865 will make it even better, since we can try several feeler connections at the same time. So I think DNS seeds should be only used to get us some starting peers, and then we build our local network view from the P2P net ourselves. ACKs for top commit: luisschwab: ACK 0ceeb59 JoseSK999: ACK 0ceeb59 Tree-SHA512: a3529e147ca18ba18eab7b59918b17521a70addf9de7c37750612e58b6aea9f3173869672454126f516b2e38bd3d7f597d5e3731b850d2b86201982b8e14eae7

Davidson-Souza added bug Something isn't working code quality Generally improves code readability and maintainability labels Jan 13, 2026

Davidson-Souza force-pushed the ask-peers branch 2 times, most recently from 673d7e1 to dc1cb67 Compare January 13, 2026 21:34

Davidson-Souza marked this pull request as ready for review January 13, 2026 21:34

brunoerg reviewed Jan 14, 2026

View reviewed changes

Comment thread crates/floresta-wire/src/p2p_wire/address_man.rs Outdated

brunoerg reviewed Jan 14, 2026

View reviewed changes

Comment thread crates/floresta-wire/src/p2p_wire/peer.rs Outdated

brunoerg reviewed Jan 14, 2026

View reviewed changes

Comment thread crates/floresta-wire/src/p2p_wire/peer.rs Outdated

Davidson-Souza force-pushed the ask-peers branch from dc1cb67 to 8427273 Compare January 14, 2026 15:02

brunoerg reviewed Jan 14, 2026

View reviewed changes

Comment thread crates/floresta-wire/src/p2p_wire/peer.rs

brunoerg mentioned this pull request Jan 15, 2026

addrv2: replace rust-bitcoin to Floresta bitcoinfuzz/bitcoinfuzz#384

Closed

jaoleal reviewed Jan 17, 2026

View reviewed changes

luisschwab mentioned this pull request Jan 20, 2026

wire: implement disconnect_peer #796

Closed

Davidson-Souza force-pushed the ask-peers branch 3 times, most recently from 1703ed1 to 724469e Compare January 20, 2026 21:33

moisesPompilio reviewed Jan 21, 2026

View reviewed changes

Comment thread crates/floresta-wire/src/p2p_wire/address_man.rs

Davidson-Souza force-pushed the ask-peers branch 3 times, most recently from fadc867 to 3124891 Compare January 28, 2026 17:34

Davidson-Souza requested review from brunoerg and moisesPompilio January 28, 2026 18:13

Davidson-Souza added 9 commits February 24, 2026 10:33

fix: Add a limit on how many addresses an AddrV2 message can have

3ad90d8

The BIP demands nodes to ignore any message with more than 1_000 addresses. We currently may receive and send packages bigger than that. This commit fixes it by enforcing this limit.

fix: Insert peers added through CLI to addrMan

1d0019f

We don't currently push peers added through CLI to our AddrMan, therefore we don't learn about them to use in the future. This commit fixes it, by calling `addr_man.push` with any peer that have been added from the CLI

refactor: Move address-related functions to LocalAddress

645abc3

The address manager implements some methods like `is_private` and `is_good_peer` to help with peer selection. However, they are closer to the LocalAddress struct, so I've moved them to LocalAddress and made them public.

fix: rate-limit the rate of addrv2 messages we get

ba02caa

To avoid being eclipsed with an address spam attack, we limit the rate of addrv2 messages a peer can send us to one every 10 seconds.

fix: add peers from dns seeds to the good peers tables

ce8517e

DNS seeds are trusted to deliver recently seen peers, that are very likely to still be alive. This commit adds those peers directly to the "good" peers table, making it easier for our node to start up peers

fix: don't add duplicate addresses to per-service bits bucket

9125089

Before this commit we would add duplicate addresses to each one of the per-service bucket. This commit checks whether the address already existed there, and only adds if not

JoseSK999 reviewed Feb 24, 2026

View reviewed changes

Davidson-Souza force-pushed the ask-peers branch from c5b38b6 to 9125089 Compare February 24, 2026 13:58

JoseSK999 reviewed Feb 24, 2026

View reviewed changes

refactor(addr_man): introduce a time_since_unix method

21e316e

This method returns the time elapsed since the unix epoch. This logic is reused all over addr_man, so it is better to just have it somewhere and use it.

Davidson-Souza force-pushed the ask-peers branch from db86233 to 21e316e Compare February 24, 2026 17:00

JoseSK999 approved these changes Feb 24, 2026

View reviewed changes

luisschwab approved these changes Feb 24, 2026

View reviewed changes

Davidson-Souza merged commit 9d575d3 into getfloresta:master Feb 24, 2026
11 checks passed

Davidson-Souza mentioned this pull request Mar 6, 2026

chore(wire): increase DNS_SEED_REQUEST_INTERVAL #870

Merged

csgui added this to Floresta Mar 12, 2026

github-project-automation Bot moved this to Backlog in Floresta Mar 12, 2026

github-project-automation Bot moved this from Backlog to Done in Floresta Mar 12, 2026

Uh oh!

Conversation

Davidson-Souza commented Jan 13, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description and Notes

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Davidson-Souza commented Jan 14, 2026

Uh oh!

Uh oh!

brunoerg commented Jan 16, 2026

Uh oh!

erickcestari commented Jan 16, 2026

Uh oh!

luisschwab commented Jan 16, 2026

Uh oh!

jaoleal left a comment

Choose a reason for hiding this comment

Uh oh!

jaoleal Jan 17, 2026

Choose a reason for hiding this comment

Uh oh!

Davidson-Souza Jan 21, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Davidson-Souza commented Jan 20, 2026

Uh oh!

Davidson-Souza commented Jan 20, 2026

Uh oh!

Uh oh!

erickcestari commented Jan 23, 2026

Uh oh!

Davidson-Souza commented Jan 28, 2026

Uh oh!

JoseSK999 left a comment

Choose a reason for hiding this comment

Uh oh!

JoseSK999 Feb 24, 2026

Choose a reason for hiding this comment

Uh oh!

Davidson-Souza commented Feb 24, 2026

Uh oh!

luisschwab commented Feb 24, 2026

Uh oh!

JoseSK999 Feb 24, 2026

Choose a reason for hiding this comment

Uh oh!

JoseSK999 Feb 24, 2026

Choose a reason for hiding this comment

Uh oh!

Davidson-Souza Feb 24, 2026

Choose a reason for hiding this comment

Uh oh!

JoseSK999 left a comment

Choose a reason for hiding this comment

Uh oh!

moisesPompilio commented Feb 24, 2026

Uh oh!

luisschwab left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

luisschwab commented Feb 24, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

8 participants

Davidson-Souza commented Jan 13, 2026 •

edited

Loading