swarm: smart dialing classifies DNS-only multiaddrs as private addresses
Nobody has claimed this yet.
- Dominant language
- Rust
- Stars
- 5.6k
- Forks
- 1.3k
- Avg merge
- 8h 47m
- Merged PRs (30d)
- 19
Description
I was reading through the smart dialing code that landed in #6229 (swarm/src/connection/pool/dial_ranker.rs) and I think the DNS branch of is_global_addr has its condition the wrong way round. The effect is that DNS-only multiaddrs get filed under "private" and localhost gets filed under "other", i.e. the two cases are swapped, and the "Other" tier that rank_dials documents never actually runs for the addresses it names.
What the code does
rank_dials buckets each dial like this:
for dial in dials {
if dial.addr.iter().any(|p| matches!(p, Protocol::P2pCircuit)) {
relay.push(dial);
} else if !is_global_addr(&dial.addr) {
private.push(dial);
} else if dial
.addr
.iter()
.any(|p| matches!(p, Protocol::Ip4(_) | Protocol::Ip6(_)))
{
public.push(dial);
} else {
other.push(dial);
}
}
and the doc comment on the last bucket says:
Other - addresses without any IP component (no
Ip4/Ip6), e.g. DNS-only multiaddrs like/dns/example.com/tcp/443
is_global_addr handles DNS-only addresses here:
if let Some(dns) = a.iter().find_map(|p| match p {
Protocol::Dns(dns) | Protocol::Dns4(dns) | Protocol::Dns6(dns) => Some(dns),
_ => None,
}) {
return dns == "localhost" || dns.ends_with(".localhost");
}
For example.com this returns false, so !is_global_addr(..) is true and the address is pushed into private; it never reaches the else branch that was written for it. For localhost it returns true, so a localhost DNS address skips private and ends up in other (it has no Ip4/Ip6). That is the opposite of the comment directly above the function, which says link-local IPv6 and localhost DNS names are the ones that are not globally routable, i.e. everything else, including ordinary DNS names, is meant to count as routable.
/dnsaddr/... isn't matched by that find_map at all (only Dns/Dns4/Dns6 are), so it falls through to the final false and is classified as private the same way.
Why it matters
Smart dialing is meant to start the fast, likely addresses first and leave the awkward ones for later; a DNS name costs an extra resolution step, which is exactly why it gets its own tier and an extra PUBLIC_OTHER_DELAY offset. With this predicate a /dns4/... address is treated like a LAN address: it's dialed ahead of real public IP addresses and gets the short private-tier delays instead of being parked at the back. The "Other" tier effectively never runs for the addresses it was written for, so for a peer whose address book mixes DNS names and direct addresses the dial order and the pacing both come out wrong.
Evidence
The tests in this file only ever use /ip4/... and /ip6/... addresses, so the DNS path is never exercised. Ranking something like ["/dns4/bootstrap.example.com/tcp/4001", "/ip6/2606:2800:220:1:248:1893:25c8:1946/tcp/4001"] returns the DNS entry first with a zero delay, instead of last.
What I'd expect
A DNS-only multiaddr should land in the tier the doc comment describes, and a localhost DNS name should be treated as non-global (private). A small regression test with a DNS-only address next to a couple of IP addresses would keep this covered.
Possible direction
Inverting the condition (return !(dns == "localhost" || dns.ends_with(".localhost"));, plus a decision on /dnsaddr) is the smallest change, but I'm not sure that's quite the right place for it: is_global_addr is also being asked about addresses with no IP component at all, which is arguably outside what "global" means. Either way, the bucket documented in rank_dials should actually contain the DNS-only addresses it mentions. I don't have a strong view on which shape the fix should take.
Contributor guide
First steps
- Read the whole issue, then the project's contributing guide.
- Comment on the issue to say you are picking it up — it saves two people doing the same work.
- Fork the repository and make your change on a branch.
- Open a pull request that references the issue number.
Research direction
Start in swarm/src/connection/pool/dial_ranker.rs, reading is_global_addr and rank_dials alongside the existing IP-address tests. Add a regression case covering DNS-only, localhost DNS, and IP addresses, including the /dnsaddr behavior decision. Done means DNS-only addresses follow the documented Other tier, localhost is treated as non-global, and the ranking delays match those tiers.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- rust
- Domain
- networking
- Issue type
- Bug
- Difficulty
- 3/5
- Estimated time
- 1-2 days
- Activity status
- Active
- Clarity
- Mostly clear
- Newbie friendliness
- 70/100