feat(cli): accept a node sub-command for running the node

The binary has only ever run the node, so an invocation is a bare list
of node flags. An upcoming offline block-building benchmark adds a
second entry point, which means the node first needs a name of its own.

`node` is the default sub-command: `ethlambda node --genesis ...` and
the existing flat `ethlambda --genesis ...` both run the node. A leading
`node` token is stripped before parsing and the same CliOptions parser
then sees exactly the arguments it saw before, so for the flat form the
help text, error messages, exit codes and --version are unchanged by
construction rather than by convention. That form is what the
Dockerfile, lean-quickstart, the hive shim and the devnet skills all
use, and none of them has to move.

Tests pin the flat parse, the two forms agreeing field for field, a
--node-id value that is literally "node", a trailing `node` token still
being rejected, missing required flags in both forms, the bare
invocation, and --help/--version staying top-level flags.

Author: pablodeymo

bin/ethlambda/src/command.rs

@@ -0,0 +1,179 @@
+//! Sub-command dispatch.
+//!
+//! `node` is the default sub-command: it can be named explicitly
+//! (`ethlambda node --genesis ...`) or left out entirely
+//! (`ethlambda --genesis ...`). Leaving it out is what the Dockerfile,
+//! lean-quickstart, the hive shim and the devnet skills all do, so that form
+//! stays the one this module is careful about: the token is simply removed
+//! before parsing, and the very same [`CliOptions`] parser then sees the very
+//! same arguments it saw before this module existed. Help text, error
+//! messages, exit codes and `--version` are therefore unchanged for it, by
+//! construction rather than by test.
+
+use std::ffi::OsString;
+
+use clap::Parser;
+
+use crate::cli::CliOptions;
+
+/// The sub-command token accepted in first position.
+///
+/// `CliOptions` declares no positional arguments, so the first token after the
+/// program name is either a flag or this sub-command: a flag *value* never
+/// lands there and is never mistaken for it.
+const NODE: &str = "node";
+
+/// What the command line asked the binary to do.
+#[derive(Debug)]
+pub(crate) enum Invocation {
+    /// Run the consensus node.
+    Node(CliOptions),
+}
+
+/// Parse the process arguments, exiting the way clap does on a parse error,
+/// `--help` or `--version`.
+pub(crate) fn parse() -> Invocation {
+    match try_parse_from(std::env::args_os()) {
+        Ok(invocation) => invocation,
+        Err(err) => err.exit(),
+    }
+}
+
+fn try_parse_from<I>(args: I) -> Result<Invocation, clap::Error>
+where
+    I: IntoIterator,
+    I::Item: Into<OsString>,
+{
+    let mut args: Vec<OsString> = args.into_iter().map(Into::into).collect();
+    if args.get(1).is_some_and(|arg| arg == NODE) {
+        args.remove(1);
+    }
+    CliOptions::try_parse_from(args).map(Invocation::Node)
+}
+
+#[cfg(test)]
+mod tests {
+    use std::path::PathBuf;
+
+    use clap::error::ErrorKind;
+
+    use super::*;
+
+    /// The flat invocation shape used by the Dockerfile, lean-quickstart, the
+    /// hive shim and the devnet skills. It must keep parsing unchanged.
+    const FLAT: &[&str] = &[
+        "ethlambda",
+        "--genesis",
+        "config.yaml",
+        "--validators",
+        "annotated_validators.yaml",
+        "--bootnodes",
+        "nodes.yaml",
+        "--validator-config",
+        "validator-config.yaml",
+        "--hash-sig-keys-dir",
+        "hash-sig-keys/",
+        "--node-key",
+        "node.key",
+        "--node-id",
+        "ethlambda_0",
+        "--gossipsub-port",
+        "9001",
+        "--is-aggregator",
+    ];
+
+    /// `FLAT` with an explicit `node` sub-command token.
+    fn with_node_token() -> Vec<&'static str> {
+        let mut args = vec!["ethlambda", NODE];
+        args.extend_from_slice(&FLAT[1..]);
+        args
+    }
+
+    fn node_options(args: &[&str]) -> CliOptions {
+        let Invocation::Node(options) =
+            try_parse_from(args.iter().map(OsString::from)).expect("invocation parses");
+        options
+    }
+
+    #[test]
+    fn flat_invocation_parses_unchanged() {
+        let options = node_options(FLAT);
+        assert_eq!(options.genesis, PathBuf::from("config.yaml"));
+        assert_eq!(options.hash_sig_keys_dir, PathBuf::from("hash-sig-keys/"));
+        assert_eq!(options.node_id, "ethlambda_0");
+        assert_eq!(options.gossipsub_port, 9001);
+        assert!(options.is_aggregator);
+    }
+
+    #[test]
+    fn node_sub_command_accepts_the_same_flags_as_the_flat_form() {
+        let flat = node_options(FLAT);
+        let scoped = node_options(&with_node_token());
+        assert_eq!(format!("{flat:?}"), format!("{scoped:?}"));
+    }
+
+    #[test]
+    fn a_flag_value_of_node_is_not_taken_for_the_sub_command() {
+        let mut args: Vec<&str> = FLAT.to_vec();
+        let value = args
+            .iter()
+            .position(|arg| *arg == "ethlambda_0")
+            .expect("node id value present");
+        args[value] = NODE;
+        assert_eq!(node_options(&args).node_id, NODE);
+    }
+
+    #[test]
+    fn a_node_token_after_the_flags_is_still_rejected() {
+        // Only a leading token is a sub-command; anywhere else it stays the
+        // stray positional argument it has always been.
+        let mut args: Vec<&str> = FLAT.to_vec();
+        args.push(NODE);
+        let err = try_parse_from(args.iter().map(OsString::from))
+            .expect_err("a trailing token must not be swallowed");
+        assert_eq!(err.kind(), ErrorKind::UnknownArgument);
+    }
+
+    #[test]
+    fn missing_required_flag_keeps_the_clap_error_in_both_forms() {
+        // `--genesis config.yaml` dropped from the front of the flag list.
+        let flat: Vec<&str> = std::iter::once("ethlambda")
+            .chain(FLAT[3..].iter().copied())
+            .collect();
+        let mut scoped = vec!["ethlambda", NODE];
+        scoped.extend_from_slice(&flat[1..]);
+
+        for args in [flat, scoped] {
+            let err = try_parse_from(args.iter().map(OsString::from))
+                .expect_err("a missing required flag must error");
+            assert_eq!(err.kind(), ErrorKind::MissingRequiredArgument);
+        }
+    }
+
+    #[test]
+    fn bare_invocation_still_errors_on_the_required_flags() {
+        for args in [vec!["ethlambda"], vec!["ethlambda", NODE]] {
+            let err = try_parse_from(args.iter().map(OsString::from))
+                .expect_err("an argument-less invocation must not start a node");
+            assert_eq!(err.kind(), ErrorKind::MissingRequiredArgument);
+        }
+    }
+
+    #[test]
+    fn help_and_version_stay_top_level_flags() {
+        // `ethereum/hive` builds its ethlambda image by piping
+        // `ethlambda --version` into a file, with and without flags in front.
+        let mut version_after_flags: Vec<&str> = FLAT.to_vec();
+        version_after_flags.push("--version");
+
+        for (args, expected) in [
+            (vec!["ethlambda", "--help"], ErrorKind::DisplayHelp),
+            (vec!["ethlambda", "--version"], ErrorKind::DisplayVersion),
+            (version_after_flags, ErrorKind::DisplayVersion),
+        ] {
+            let err = try_parse_from(args.iter().map(OsString::from))
+                .expect_err("help and version short-circuit parsing");
+            assert_eq!(err.kind(), expected);
+        }
+    }
+}

bin/ethlambda/src/main.rs

@@ -1,5 +1,6 @@
 mod checkpoint_sync;
 mod cli;
+mod command;
 mod fd_limit;
 mod version;
 
@@ -31,8 +32,7 @@ use std::{
 };
 use tokio_util::sync::CancellationToken;
 
-use clap::Parser;
-use cli::CliOptions;
+use command::Invocation;
 use ethlambda_blockchain::MILLISECONDS_PER_SLOT;
 use ethlambda_blockchain::block_builder::ProposerConfig;
 use ethlambda_blockchain::key_manager::ValidatorKeyPair;
@@ -80,7 +80,7 @@ async fn main() -> eyre::Result<()> {
     tracing::subscriber::set_global_default(subscriber)
         .wrap_err("failed to set global tracing subscriber")?;
 
-    let options = CliOptions::parse();
+    let Invocation::Node(options) = command::parse();
     options.validate_discovery()?;
 
     #[cfg(feature = "shadow-integration")]