fix(relay-server): advertise configured external multiaddrs on / and /enr#428
Open
varex83 wants to merge 3 commits into
Open
fix(relay-server): advertise configured external multiaddrs on / and /enr#428varex83 wants to merge 3 commits into
varex83 wants to merge 3 commits into
Conversation
…/enr
The /multiaddr and /enr handlers read from AppState.addrs, which was
populated exclusively by SwarmEvent::NewListenAddr. Configured external
addresses (from --p2p-external-ip / --p2p-external-hostname) are pushed
into the libp2p swarm via add_external_address but never appeared in
that Vec, so:
- / returned [] in private-network deployments (filter_private_addrs
drops the libp2p listen addrs; no externals to fall back to).
- /enr short-circuited with 500 "no addresses" before apply_ip_override
could run, making both external_ip and the DNS resolver dead
code in K8s-style deployments.
Compute external_tcp_multiaddrs + external_udp_multiaddrs once at relay
startup, thread them into AppState as an immutable Vec, and union them
with the live listeners (externals first, deduped) when serving both
endpoints. Mirrors Go charon's AddrsFactory + filterAdvertisedAddrs
shape.
For /enr, extend the TCP/UDP scan loop with a DNS fallback: if the
candidate multiaddr is /dns/<host>/{tcp,udp}/<port>, substitute the
resolver-cached external_host IP. Existing /ip4/... + apply_ip_override
path is unchanged, so external_ip continues to win over external_host
when both are set.
Verified locally with PLUTO_P2P_EXTERNAL_HOSTNAME=example.com,
PLUTO_P2P_ADVERTISE_PRIVATE_ADDRESSES=false, loopback listen addrs:
/ returns /dns/example.com/{tcp,udp}/... and /enr returns a valid ENR
with example.com's resolved IP embedded.
Adds 33 unit tests across the new code paths:
- utils: extract_dns_and_{tcp,udp}_port positive/negative cases for
Dns/Dns4/Dns6, plus regression coverage for the IPv4 extractors and
is_public_addr.
- web::AppState::advertised_addrs: union/dedup ordering with externals
first, listener-vs-external duplicates collapsed, empty-state.
- web::multiaddr_handler: returns externals first with /p2p/<peer-id>
encapsulated; empty when nothing configured; external_ip and
external_host cases.
- web::enr_handler: 500 when nothing configured, external_ip baked into
ENR (with and without conflicting listener IP), external_host DNS
fallback uses resolver-cached IP, external_ip wins over external_host
when both are set, public listener used when no externals.
Adds #[derive(Debug)] on HandlerError so tests can use `expect_err`.
Spins up the real `enr_server` axum app on an ephemeral 127.0.0.1 port
and exercises both routes over a live TCP socket via reqwest. Three
scenarios:
- external_ip only: asserts `/` returns /ip4/<ip>/{tcp,udp,quic-v1}
multiaddrs with peer-id encapsulation and that /enr returns a valid
ENR with the external IP baked in.
- empty config: asserts `/` returns [] and /enr returns 500.
- external_host=localhost: asserts `/` emits /dns/localhost/...
multiaddrs verbatim and polls /enr until the resolver populates the
cache, then asserts the ENR contains a loopback IP.
The localhost scenario relies on /etc/hosts resolution rather than
public DNS, so the suite is hermetic in CI. Each test cancels the
server via CancellationToken and waits with a bounded timeout to keep
flaky runs visible instead of hanging.
To make `enr_server` callable from a `tests/` crate, re-export it from
the crate root as a `#[doc(hidden)]` item. Adds `reqwest` and
`serde_json` to [dev-dependencies].
The existing `.github/workflows/test.yml` runs
`cargo test --locked --workspace --all-features`, which picks up the
new tests on both linux/amd64 and linux/arm64 runners. `linter.yml`
runs clippy with --all-targets, which covers the test code too. No CI
workflow changes are required.
iamquang95
reviewed
May 21, 2026
| /// addresses at ingest time when `filter_private_addrs` is set. | ||
| async fn advertised_addrs(&self) -> Vec<Multiaddr> { | ||
| let listeners = self.addrs.read().await; | ||
| let mut union: Vec<Multiaddr> = self.external_addrs.clone(); |
Collaborator
There was a problem hiding this comment.
external_addrs can have duplicates. Better use HashSet here to simplify the code
iamquang95
approved these changes
May 21, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Fixes both
/and/enrreturning no useful data in private-network / K8s deployments where libp2p only sees private listen addresses andfilter_private_addrs=true.Root cause:
AppState.addrswas populated exclusively fromSwarmEvent::NewListenAddr. External addresses configured via--p2p-external-ip/--p2p-external-hostnamereach the swarm throughadd_external_address(...)but never entered that Vec, so:/returned[]once libp2p's listen addrs were filtered out as private./enrshort-circuited with500 "no addresses"beforeapply_ip_overrideran, makingexternal_ipand the DNS resolver dead code in this deployment shape.Fix: compute
external_tcp_multiaddrs+external_udp_multiaddrsfrom config at relay startup, thread them throughenr_serverintoAppStateas an immutableVec<Multiaddr>, and union them with the live listeners (externals first, deduped) when serving both endpoints. Mirrors Go charon'sAddrsFactory+filterAdvertisedAddrsshape.For
/enrspecifically, the TCP/UDP scan loop now has a DNS fallback:/dns/<host>/tcp/<port>and/dns/<host>/udp/<port>/quic-v1multiaddrs are resolved via the cachedexternal_host_ipproduced by the existing resolver loop. The/ip4/...+apply_ip_overridepath is unchanged, soexternal_ipkeeps priority overexternal_hostwhen both are set.Files
crates/p2p/src/utils.rs—external_tcp_multiaddrs/external_udp_multiaddrsvisibility bumped frompub(crate)topub.crates/relay-server/src/utils.rs— newextract_dns_and_{tcp,udp}_porthelpers.crates/relay-server/src/web.rs—AppStategainsexternal_addrs+advertised_addrs(); both handlers use the union.crates/relay-server/src/p2p.rs— computeexternal_addrsand pass toenr_server.Verification
Ran locally with the same env shape as the affected K8s deployment:
/:/enrreturned200 OKwith a valid ENR containing example.com's resolved IPv4 address.