Skip to content

feat: add DNS resolution to TrustedPeer#566

Merged
rustaceanrob merged 1 commit into2140-dev:masterfrom
0xsiddharthks:siddharth/feat/dns-peer
Apr 21, 2026
Merged

feat: add DNS resolution to TrustedPeer#566
rustaceanrob merged 1 commit into2140-dev:masterfrom
0xsiddharthks:siddharth/feat/dns-peer

Conversation

@0xsiddharthks
Copy link
Copy Markdown
Contributor

@0xsiddharthks 0xsiddharthks commented Apr 9, 2026

Summary

Extends TrustedPeer so a peer can be given as a DNS hostname instead of a pre-resolved AddrV2. Hostnames are resolved at connection time in PeerMap::next_peer via tokio::net::lookup_host, so the backing IP can change between reconnections without the caller pre-resolving.

Replaces the earlier DnsPeer + round-robin approach from this PR. The reworked design keeps kyoto free of any client-specific retry policy, matching the reviewer's ask — see this comment.

API shape

#[derive(Debug, Clone)]
pub enum PeerAddress {
    /// A pre-resolved Bitcoin P2P address.
    Addr(AddrV2),
    /// A DNS hostname, resolved at connection time.
    Hostname(String),
}

impl From<AddrV2> for PeerAddress { ... }

#[derive(Debug, Clone)]
pub struct TrustedPeer {
    pub address: PeerAddress,   // was: AddrV2
    pub port: Option<u16>,
    pub known_services: ServiceFlags,
}

impl TrustedPeer {
    pub fn from_hostname(hostname: impl Into<String>, port: u16) -> Self { ... }
    // existing constructors (`new`, `from_ip`, `from_socket_addr`) keep working
}
  • TrustedPeer::new(AddrV2::Ipv4(...), ..) and every Into<TrustedPeer> impl keep compiling because PeerAddress: From<AddrV2> and new takes impl Into<PeerAddress>.
  • New constructor TrustedPeer::from_hostname(host, port) for the hostname path.

Semantics

  • Resolution at connect time. Every call to next_peer that pops a PeerAddress::Hostname entry does a fresh lookup_host. No caching inside kyoto.
  • Consumed on use. Same as the existing IP-based TrustedPeer — popped from the whitelist, never reinstated. Retries and re-resolution across node lifetimes are the client's responsibility (rebuild the node on NoReachablePeers).
  • Resolution failure is non-fatal. If a hostname fails to resolve (or returns no addresses), the entry is logged and the next whitelist entry is tried. NoReachablePeers is only surfaced once the whole whitelist is exhausted.
  • First address wins. If the hostname resolves to multiple addresses, the first returned by the resolver is used — good enough for single-pod K8s Service / ECS targets; multi-address fallback is future work.
  • Socks5 / Tor. Hostnames are resolved locally even when a Socks5 proxy is configured. Onion addresses remain routed via AddrV2::TorV3 → SocksConnection::OnionService as today. Resolving hostnames inside the proxy (via the Socks5 0x03 domain-name address type) is out of scope for this PR.

Breaking changes

Minimal, all on the TrustedPeer surface:

  • pub address: AddrV2pub address: PeerAddress. Direct struct-literal construction needs .into() (e.g. TrustedPeer { address: AddrV2::Ipv4(ip).into(), ... }). Callers using any constructor or Into impl are unaffected.
  • fn address(&self) -> AddrV2fn address(&self) -> &PeerAddress.
  • impl From<TrustedPeer> for (AddrV2, Option<u16>)impl From<TrustedPeer> for (PeerAddress, Option<u16>).

Testing

New trusted_peer_hostname_sync integration test:

  • Starts a regtest bitcoind and mines 10 blocks.
  • Builds a TrustedPeer::from_hostname(socket_addr.ip().to_string(), socket_addr.port()) — an IP literal used as the hostname so the test is deterministic on CI (on macOS, localhost resolves to ::1 but bitcoind only listens on IPv4).
  • Syncs and asserts the tip hash matches. Debug log confirms the tokio::net::lookup_host path fires.

Real-hostname resolution (through the OS resolver) is still covered by the existing dns_works test.

Full integration suite passes locally (just test integration, 10/10 green, ~106s).

Motivation

In containerized / cloud-native environments, Bitcoin peers often sit behind service hostnames whose backing IPs rotate (Kubernetes Services, ECS service discovery, Consul, etc.). TrustedPeer today requires a resolved SocketAddr at build time; hostname support lets users defer resolution to connection time.

Used in Hashi — a decentralized Bitcoin collateralization primitive on Sui, built by Mysten Labs. Hashi's bitcoind sits behind a Kubernetes service DNS name whose backing pod IP rotates on restart; hashi runs a supervisor that rebuilds the kyoto node on NoReachablePeers, which matches the "consumed on use, rebuild to retry" model this PR settles on. See MystenLabs/hashi#402.

@0xsiddharthks 0xsiddharthks marked this pull request as ready for review April 9, 2026 22:02
@0xsiddharthks 0xsiddharthks force-pushed the siddharth/feat/dns-peer branch from 8c83bb2 to 5f7e263 Compare April 9, 2026 22:05
@0xsiddharthks 0xsiddharthks marked this pull request as draft April 9, 2026 22:07
@0xsiddharthks 0xsiddharthks marked this pull request as ready for review April 9, 2026 22:40
@0xsiddharthks 0xsiddharthks force-pushed the siddharth/feat/dns-peer branch 2 times, most recently from db181cc to 282078f Compare April 9, 2026 23:08
@rustaceanrob
Copy link
Copy Markdown
Member

Not super convinced of this one, as it adds some client-specific logic that others might want to implement differently (round robin). I think it is fair to have an add_dns_peer that automatically resolves and connects to the hostname, but retries should occur on the client side IMO

@0xsiddharthks 0xsiddharthks force-pushed the siddharth/feat/dns-peer branch from 282078f to 02004ac Compare April 19, 2026 23:40
@0xsiddharthks
Copy link
Copy Markdown
Contributor Author

Hi @rustaceanrob , thanks for taking a look. Let me just give you some context about our initial motivation.

In our usecase, we connect to a bitcoind behind a Kubernetes service DNS name (bitcoind.<ns>.svc.cluster.local).
The pod IP behind that hostname rotates on restart, so a TrustedPeer built from a resolved SocketAddr goes stale. What we actually need is "resolve this hostname every time you try to connect, so the node follows pod rotation."

I think there are two ways to land that without baking any rotation policy into kyoto:

  • Drop the round-robin index
    Keep DnsPeer persistent in PeerMap, but on each next_peer() call just iterate in insertion order and return the first hostname that resolves.
    So we don't need any index state or client specific rotation.

  • Treat DnsPeer as a hostname-flavored TrustedPeer
    Resolve at connect time, consume on use, let the client handle retries by rebuilding the node on NoReachablePeers.
    I think this is super clean and maintains the client / kyoto boundary. it works fine for our usecase as well because we already run a supervisor that rebuilds the kyoto node on disconnect (we use whitelist_only specifically for that exit behavior), and each rebuild re-resolves fresh.

I'd personally prefer the first option because it keeps the "DNS re-resolves on reconnect" property without requiring a full node rebuild each time, but the second is strictly simpler and I'm happy to go that way if you prefer.

@rustaceanrob
Copy link
Copy Markdown
Member

The latter option is the most general for all users, so I will opt for that. Rather than creating a newtype for a peer from DNS, I am going to extend TrustedPeer to also handle DNS resolution.

@0xsiddharthks 0xsiddharthks force-pushed the siddharth/feat/dns-peer branch from 02004ac to b0ce147 Compare April 20, 2026 19:23
@0xsiddharthks 0xsiddharthks changed the title feat: add DnsPeer for hostname-based peers with fresh DNS resolution feat: add DNS resolution to TrustedPeer Apr 20, 2026
@0xsiddharthks
Copy link
Copy Markdown
Contributor Author

Thanks @rustaceanrob
I updated the PR to follow the second approach.

here's the new shape:

pub enum PeerAddress {
    Addr(AddrV2),
    Hostname(String),
}
impl From<AddrV2> for PeerAddress { ... }

pub struct TrustedPeer {
    pub address: PeerAddress,   // was: AddrV2
    pub port: Option<u16>,
    pub known_services: ServiceFlags,
}

impl TrustedPeer {
    pub fn from_hostname(hostname: impl Into<String>, port: u16) -> Self;
    // existing constructors keep working — `new` now takes `impl Into<PeerAddress>`
}

Could you please have another look.

@rustaceanrob rustaceanrob force-pushed the siddharth/feat/dns-peer branch 2 times, most recently from 7b6c4bf to f28526c Compare April 21, 2026 09:03
@rustaceanrob
Copy link
Copy Markdown
Member

rustaceanrob commented Apr 21, 2026

I made some adjustments:

  • Removed the new integration test and placed the test case in whilelist_only.
  • Made the fields of TrustedPeer private and added the enum as an internal field. This allows for flexibility in how peers are sourced in the future as the internal representation may change. Also, I try to keep the number of public types limited in scope.

I notice we only take the first address resolved from the hostname. Why not add all resolved peers to the whitelist?

@rustaceanrob rustaceanrob force-pushed the siddharth/feat/dns-peer branch 2 times, most recently from 32e2130 to 7470c44 Compare April 21, 2026 09:23
@0xsiddharthks 0xsiddharthks force-pushed the siddharth/feat/dns-peer branch from 7470c44 to 2699acd Compare April 21, 2026 16:58
Introduce an  enum wrapping either a pre-resolved AddrV2 or a
DNS hostname, and a new TrustedPeer::from_hostname constructor. Hostnames
are resolved via tokio::net::lookup_host inside PeerMap::next_peer at
each attempt, so the backing IP can change between reconnections without
the caller pre-resolving.

Like IP-based trusted peers, hostname peers are consumed on use. Retries
and re-resolution across node lifetimes are the client's responsibility
(rebuild the node on NoReachablePeers). If a hostname fails to resolve,
it is logged and the next whitelist entry is tried; the node only
surfaces NoReachablePeers once the entire whitelist is exhausted.

Useful when the backing IP of a peer may change between reconnections,
e.g. a Kubernetes service whose pod IP rotates.
@0xsiddharthks 0xsiddharthks force-pushed the siddharth/feat/dns-peer branch from 2699acd to 934a7e0 Compare April 21, 2026 17:04
@0xsiddharthks
Copy link
Copy Markdown
Contributor Author

Thanks for the cleanup @rustaceanrob .

my original reasoning (for taking only the first address) was to match the "one whitelist entry = one peer attempt" contract — a hostname consumed like any other TrustedPeer, with the caller rebuilding the node on NoReachablePeers to re-resolve across lifetimes.

But yeah, i think you're right, that's needlessly strict. bootstrap_dns already fans every DNS-seed address into the address book, and tokio::net::TcpStream::connect(hostname) iterates every resolved IP.

I've also push some changes (and rebased on master):
PeerMap::next_peer now collects every address returned by lookup_host and pushes each back onto the whitelist as a resolved TrustedPeerInner::Addr, then continues the loop:

  • Consumed-on-use is preserved — each resolved IP is attempted at most once per node lifetime.
  • Addresses are pushed in reverse so the resolver's first-ranked entry is popped first. (tbh the 'reverse' doesnt really matter here, and i'm open to either. i picked the reverse so the resolver's ranking survives. but open to either.)
  • Port and known_services are inherited from the originating hostname entry.
  • NoReachablePeers only surfaces after every resolved address (plus any other whitelist entries) has been exhausted.

@rustaceanrob
Copy link
Copy Markdown
Member

LGTM ACK 934a7e0

@rustaceanrob rustaceanrob merged commit a62a66e into 2140-dev:master Apr 21, 2026
10 checks passed
@0xsiddharthks 0xsiddharthks deleted the siddharth/feat/dns-peer branch April 26, 2026 21:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants