fix: derive the advertised port from --port, not from --ip - #455
Open
samlaf wants to merge 2 commits into
Open
Conversation
--ip parsed its value as a SocketAddr, making its port a second copy of --port that silently won on one side of the node. The listen address comes from wildcard_listen_for(our_ip, flags.port), which takes only the address family from --ip and the port from --port, while the address handed to the P2P config is --ip verbatim. So --ip 203.0.113.7:18551 with --port 26000 leaves the node listening on 26000 and signing a gossiped peer record that tells the cohort to dial 18551. Peers dial a port nothing accepts on, the node looks healthy locally, and no validation anywhere compares the two. Take --ip as an IpAddr and pair it with --port, which is what the public-IP fallback in get_node_ip already does for the third resolution path. One port, from one flag, and the mismatch stops being representable. This mirrors the shape of --prom-ip/--prom-port and --rpc-ip/--rpc-port, drops the IPv6 bracketing burden on callers (2001:db8::7 with a port appended never parsed), and lets clap reject a malformed value as a usage error instead of panicking on a later parse. The flag matters most to a node joining an existing network: it has no genesis committee entry to read its address from, so without --ip it resolves its public IP by asking external IP-echo services and signs whatever they return. An operator who already knows the address — a deployment that assigns it, or one whose execution client is advertising that same address — can now supply just the host. Found while wiring Seismic's TEE node images, which are the first caller that needs --ip. A TDX node receives its public address in the boot configuration it is sent on every boot, and tdx-init hands that one address to both reth and summit: https://github.com/SeismicSystems/enclave/tree/seismic/bin/tdx-init Nothing in-tree passes --ip today (the RunFlags builders in the testnet and e2e binaries leave it None), so this was a latent footgun rather than a live defect.
The four e2e binaries that pin an advertised address each wrote the port into --ip as well, and each wrote the same one get_node_flags had already put in --port: 26600 + slot * 10, spelled 127.0.0.1:26640 for the joining node at slot 4 and computed from OBSERVER_SLOT for the observer. Pass the bare loopback address and let --port carry the port, which leaves every one of them advertising exactly the port it did before. Four hand-maintained copies of one port is the duplication --ip's SocketAddr invited, so these are the callers the parent change is for.
samlaf
added a commit
to SeismicSystems/enclave
that referenced
this pull request
Aug 18, 2026
Write summit.env with SUMMIT_ADVERTISED_ADDR, this node's external_ip at the consensus port, for summit's unit to splice into --ip. An address is not part of validator identity in summit: its genesis carries the founding cohort's addresses outside the config digest, and a validator's consensus-state record holds none at all. Each node instead signs its own address and gossips that record, so the address a node declares is what the whole cohort dials it on. Nothing was delivering that declaration, which leaves it to summit's fallbacks: a founding node reads its own genesis entry, but a node joining an existing network has no entry there and resolves its address by asking public IP-echo services, then signs and gossips whatever they return. Sourcing it from [node].external_ip is what keeps the two planes agreeing on where the machine is — the same field reth advertises via --nat extip, so no second delivered address can skew from it. It also keeps a founding node truthful across an address change, which its founding-era genesis entry cannot follow (those entries are deliberately outside the config digest, so IP churn never re-founds a network). The value is a SocketAddr rather than a textually composed string: its Display brackets IPv6, which is the form summit parses. Composing "<external_ip>:<port>" would emit 2001:db8::7:18551 for an IPv6 node, which summit rejects at startup. The consensus port is named here until summit takes --ip as a bare address and pairs it with its own --port (SeismicSystems/summit#455); the constant carries a TODO to drop it and the bracketing along with it.
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
1 participant
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.
--ip parsed its value as a SocketAddr, making its port a second copy of --port that silently won on one side of the node. The listen address comes from wildcard_listen_for(our_ip, flags.port), which takes only the address family from --ip and the port from --port, while the address handed to the P2P config is --ip verbatim. So --ip 203.0.113.7:18551 with --port 26000 leaves the node listening on 26000 and signing a gossiped peer record that tells the cohort to dial 18551. Peers dial a port nothing accepts on, the node looks healthy locally, and no validation anywhere compares the two.
Take --ip as an IpAddr and pair it with --port, which is what the public-IP fallback in get_node_ip already does for the third resolution path. One port, from one flag, and the mismatch stops being representable. This mirrors the shape of --prom-ip/--prom-port and --rpc-ip/--rpc-port, drops the IPv6 bracketing burden on callers (2001:db8::7 with a port appended never parsed), and lets clap reject a malformed value as a usage error instead of panicking on a later parse.
The flag matters most to a node joining an existing network: it has no genesis committee entry to read its address from, so without --ip it resolves its public IP by asking external IP-echo services and signs whatever they return. An operator who already knows the address — a deployment that assigns it, or one whose execution client is advertising that same address — can now supply just the host.
Found while wiring Seismic's TEE node images, which are the first caller that needs --ip. A TDX node receives its public address in the boot configuration it is sent on every boot, and tdx-init hands that one address to both reth and summit:
https://github.com/SeismicSystems/enclave/tree/seismic/bin/tdx-init Nothing in-tree passes --ip today (the RunFlags builders in the testnet and e2e binaries leave it None), so this was a latent footgun rather than a live defect.