Skip to content

feat(wire): don't fetch peers from DNS on startup if we have enough#793

Merged
Davidson-Souza merged 2 commits into
getfloresta:masterfrom
luisschwab:feat/dont-fetch-peers-from-dns
Jan 24, 2026
Merged

feat(wire): don't fetch peers from DNS on startup if we have enough#793
Davidson-Souza merged 2 commits into
getfloresta:masterfrom
luisschwab:feat/dont-fetch-peers-from-dns

Conversation

@luisschwab

@luisschwab luisschwab commented Jan 19, 2026

Copy link
Copy Markdown
Member

Closes #784.

This PR adds some logic to skip fetching peers from DNS if we have enough in the AddressMan. The actual values are still up for discussion.

I'll rebase once #790 is merged.

@luisschwab luisschwab self-assigned this Jan 19, 2026
@luisschwab luisschwab added the enhancement New feature or request label Jan 19, 2026
@luisschwab luisschwab requested review from Davidson-Souza, JoseSK999 and jaoleal and removed request for Davidson-Souza and JoseSK999 January 19, 2026 19:06
@JoseSK999

Copy link
Copy Markdown
Member

I think it also makes sense to retry DNS requests if not enough_peers. Many times in signet I get too few peers from DNS. DNS_SEED_RETRY_PERIOD should perhaps be 30s-60s.

    fn maybe_ask_for_dns_peers(&mut self) {
        if self.config.disable_dns_seeds {
            return;
        }

        if !self.peers.is_empty() { // One peer is not enough
            return;
        }
        // ...

@JoseSK999

Copy link
Copy Markdown
Member

We can do this now

@luisschwab luisschwab force-pushed the feat/dont-fetch-peers-from-dns branch from 0f5a37f to 2e2d60d Compare January 20, 2026 18:22
@luisschwab

Copy link
Copy Markdown
Member Author

@JoseSK999 done

@luisschwab luisschwab marked this pull request as ready for review January 20, 2026 18:22

@Davidson-Souza Davidson-Souza left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Where you say peers in address_man I think addresses make more sense. Peer is someone we are connected to

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

Copy link
Copy Markdown
Member Author

@Davidson-Souza those peer count values are just placeholders. Do you have any thoughts on them?

@luisschwab luisschwab force-pushed the feat/dont-fetch-peers-from-dns branch from 2e2d60d to 16b2ab6 Compare January 21, 2026 16:53
Comment thread crates/floresta-wire/src/p2p_wire/node/conn.rs Outdated
@luisschwab

Copy link
Copy Markdown
Member Author

I think it would be useful to also decrease DNS_SEED_RETRY_PERIOD, as pointed out by @JoseSK999.

@luisschwab luisschwab force-pushed the feat/dont-fetch-peers-from-dns branch 2 times, most recently from 7791c36 to e74ac48 Compare January 22, 2026 17:08
Comment thread crates/floresta-wire/src/p2p_wire/address_man.rs Outdated
@luisschwab luisschwab force-pushed the feat/dont-fetch-peers-from-dns branch from e74ac48 to 4accae1 Compare January 22, 2026 17:22
Comment thread crates/floresta-wire/src/p2p_wire/node/conn.rs Outdated
Comment thread crates/floresta-wire/src/p2p_wire/node/conn.rs Outdated
@JoseSK999

Copy link
Copy Markdown
Member

I actually think we shouldn't have MIN_PEERS_P2PV2. Currently our address manager doesn't populate that service, so the behavior now is that there aren't always enough addresses because we never find P2PV2 ones.

Comment thread crates/floresta-wire/src/p2p_wire/node/conn.rs Outdated
Comment thread crates/floresta-wire/src/p2p_wire/node/conn.rs Outdated
@luisschwab luisschwab force-pushed the feat/dont-fetch-peers-from-dns branch 2 times, most recently from 018b3b6 to f1a0ce9 Compare January 23, 2026 17:37
@luisschwab luisschwab closed this Jan 23, 2026
@luisschwab luisschwab force-pushed the feat/dont-fetch-peers-from-dns branch from f1a0ce9 to 1905cb1 Compare January 23, 2026 17:40
@luisschwab luisschwab reopened this Jan 23, 2026
@luisschwab luisschwab requested a review from JoseSK999 January 23, 2026 17:47
…`AddressMan`

- `get_addresses_by_service` will return all known addresses known by the `AddressMan`,
filtered by `ServiceFlags`.
- `enough_addresses` will return whether the `AddressMan` knows about anough addresses
for regular node operation
@luisschwab luisschwab force-pushed the feat/dont-fetch-peers-from-dns branch from 0198e73 to 06baded Compare January 23, 2026 17:48
… populated

- Skip fetching new addresses from DNS seeds if the `AddressMan` is well populated.

- Rename `maybe_ask_for_dns_peers` to `maybe_ask_dns_seed_for_addresses`.

- Reduce the interval for DNS address fetching from 5 to 2 minutes.
@luisschwab luisschwab force-pushed the feat/dont-fetch-peers-from-dns branch from 06baded to c34c500 Compare January 23, 2026 17:49

@Davidson-Souza Davidson-Souza left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ACK c34c500

@JoseSK999 JoseSK999 left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ACK c34c500

@jaoleal

jaoleal commented Jan 24, 2026

Copy link
Copy Markdown
Member

ACK c34c500

@Davidson-Souza Davidson-Souza merged commit 10faa8f into getfloresta:master Jan 24, 2026
9 of 10 checks passed
@luisschwab luisschwab deleted the feat/dont-fetch-peers-from-dns branch January 24, 2026 16:10
Davidson-Souza added a commit to Davidson-Souza/Floresta that referenced this pull request Mar 6, 2026
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.
Davidson-Souza added a commit to Davidson-Souza/Floresta that referenced this pull request Mar 6, 2026
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.
Davidson-Souza added a commit to Davidson-Souza/Floresta that referenced this pull request Mar 6, 2026
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.
Davidson-Souza added a commit to Davidson-Souza/Floresta that referenced this pull request Mar 9, 2026
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.
Davidson-Souza added a commit that referenced this pull request Mar 9, 2026
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
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

wire: don't fetch peers from DNS if we already have enough peers in peers.json

4 participants