Skip to content

Commit 8bdbfc9

Browse files
authored
feat(cli): add the --discovery.* option group (#580)
## What Adds the three operator-facing flags the discv5 work needs, on their own, so the implementation PR (#579) is confined to the p2p crate. | Flag | Default | Meaning | | --- | --- | --- | | `--discovery.enable` | `false` | turn discv5 peer discovery on | | `--discovery.port` | `9000` | UDP port for the discv5 socket | | `--discovery.advertise-ip` | unset | IP to advertise in the ENR | The flags parse and validate here. **Nothing reads them yet**, which is the point of splitting them out: this is reviewable on its own and cannot change runtime behaviour of a node that does not pass them. ## Why the port validation `--discovery.port` and `--gossipsub-port` are both UDP and both default to 9000, so enabling discovery without moving one of them collides. Left unchecked, that surfaces at bind time as an opaque `EADDRINUSE` on whichever socket loses the race, pointing at neither flag. `CliOptions::validate_discovery` rejects it at startup with a message naming both flags and their values. The check only fires when discovery is enabled, so the shared default is harmless for every existing deployment. ## Why `--discovery.advertise-ip` The node binds the wildcard `0.0.0.0`, which is not dialable as published. A node whose reachable address differs from what it listens on (a devnet on `127.0.0.1`, or a host behind NAT) needs to say so explicitly. discv5's PONG-based IP voting may still learn and substitute the real external address at runtime; this only sets what the ENR carries at startup. ## Testing - `make lint` clean. - Colliding ports are rejected by name: ``` $ ethlambda ... --discovery.enable Error: --discovery.port (9000) must differ from --gossipsub-port (9000): both bind UDP and cannot share a port ``` - Distinct ports pass validation and startup proceeds: ``` $ ethlambda ... --discovery.enable --discovery.port 9010 Error: failed to load node key from /nonexistent/node.key ``` - The group renders under `--help` with its dotted prefixes intact. ## Relationship to #579 #579 carries the discv5 implementation and currently includes these same flags. If this lands first, #579 rebases onto it and drops the `cli.rs` hunk.
1 parent 7ecd85f commit 8bdbfc9

2 files changed

Lines changed: 47 additions & 0 deletions

File tree

bin/ethlambda/src/cli.rs

Lines changed: 46 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -113,13 +113,59 @@ pub(crate) struct CliOptions {
113113
/// `on_block`.
114114
#[arg(long, default_value = "3")]
115115
pub(crate) max_attestations_per_block: usize,
116+
#[command(flatten)]
117+
pub(crate) discovery: DiscoveryConfig,
116118
/// Shadow-simulator sim-cost + fake-XMSS flags (only under the
117119
/// `shadow-integration` feature).
118120
#[cfg(feature = "shadow-integration")]
119121
#[command(flatten)]
120122
pub(crate) shadow: ShadowOptions,
121123
}
122124

125+
/// discv5 peer discovery. Off by default: nothing else on the lean network
126+
/// speaks discv5 yet, so enabling it only finds other ethlambda nodes.
127+
#[derive(Debug, clap::Args)]
128+
pub(crate) struct DiscoveryConfig {
129+
/// Enable discv5 peer discovery.
130+
///
131+
/// Requires `--discovery.port` to differ from `--gossipsub-port`: both are
132+
/// UDP sockets and they cannot share one port.
133+
#[arg(long = "discovery.enable", default_value = "false")]
134+
pub(crate) enable: bool,
135+
/// UDP port for the discv5 socket.
136+
///
137+
/// Independent of `--gossipsub-port`, which carries libp2p QUIC. Both
138+
/// default to 9000, so enabling discovery means changing one of them.
139+
#[arg(long = "discovery.port", default_value = "9000")]
140+
pub(crate) port: u16,
141+
/// IP address to advertise in the ENR.
142+
///
143+
/// Defaults to the bind address, which is the wildcard `0.0.0.0` and is not
144+
/// dialable as published. Set this to the address peers should reach this
145+
/// node on: `127.0.0.1` for a local devnet, or the host's public address.
146+
/// discv5's PONG-based IP voting may still replace it at runtime.
147+
#[arg(long = "discovery.advertise-ip")]
148+
pub(crate) advertise_ip: Option<std::net::IpAddr>,
149+
}
150+
151+
impl CliOptions {
152+
/// Reject a discovery port that collides with the QUIC port.
153+
///
154+
/// Both are UDP. Without this the collision surfaces at bind time as an
155+
/// opaque `EADDRINUSE` on whichever socket loses the race.
156+
pub(crate) fn validate_discovery(&self) -> eyre::Result<()> {
157+
if self.discovery.enable && self.discovery.port == self.gossipsub_port {
158+
eyre::bail!(
159+
"--discovery.port ({}) must differ from --gossipsub-port ({}): \
160+
both bind UDP and cannot share a port",
161+
self.discovery.port,
162+
self.gossipsub_port
163+
);
164+
}
165+
Ok(())
166+
}
167+
}
168+
123169
/// Shadow-simulator sim-cost + fake-XMSS flags. Compiled only under the
124170
/// `shadow-integration` feature.
125171
#[cfg(feature = "shadow-integration")]

bin/ethlambda/src/main.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -81,6 +81,7 @@ async fn main() -> eyre::Result<()> {
8181
.wrap_err("failed to set global tracing subscriber")?;
8282

8383
let options = CliOptions::parse();
84+
options.validate_discovery()?;
8485

8586
#[cfg(feature = "shadow-integration")]
8687
init_shadow_cost(&options.shadow);

0 commit comments

Comments
 (0)