refactor(staking-cli): move skip-metadata-validation to command args (#3937)

* refactor(staking-cli): move skip-metadata-validation to command args

Move --skip-metadata-validation from global flag to MetadataUriArgs,
making it a command-specific flag that appears after the subcommand.

Changes:
- Move skip_metadata_validation field to MetadataUriArgs
- Refactor metadata_uri to Option<Url> with custom value parser
- Add Clap constraints: requires, conflicts_with, required_unless_present
- Move MetadataUriArgs to metadata.rs for better organization
- Add tests for expected error messaged.

Breaking change: --skip-metadata-validation must now appear after
the subcommand instead of before it.

Before: staking-cli --skip-metadata-validation register-validator ...
After:  staking-cli register-validator ... --skip-metadata-validation

* refactor(staking-cli): extract test helper for metadata command setup

Author: sveitser

staking-cli/README.md

@@ -169,11 +169,6 @@ Options:
 
           [env: SKIP_SIMULATION=]
 
-      --skip-metadata-validation
-          Skip metadata URI validation (fetch and schema check)
-
-          [env: SKIP_METADATA_VALIDATION=]
-
       --output <OUTPUT>
           Output file path. If not specified, outputs to stdout
 
@@ -468,7 +463,19 @@ Options for `--metadata-uri`:
 
 3. **No metadata:** `--no-metadata-uri`
 
-Use `--skip-metadata-validation` if your endpoint isn't ready yet. URL cannot exceed 2048 bytes.
+**Skipping validation:** If your metadata endpoint isn't ready yet, use `--skip-metadata-validation` after the
+`--metadata-uri` argument:
+
+```bash
+staking-cli register-validator \
+    --consensus-private-key BLS_SIGNING_KEY~... \
+    --state-private-key SCHNORR_SIGNING_KEY~... \
+    --commission 4.99 \
+    --metadata-uri https://my-validator.example.com/status/metrics \
+    --skip-metadata-validation
+```
+
+URL cannot exceed 2048 bytes.
 
 The CLI automatically detects the format (JSON or OpenMetrics) by examining the content. This works with any hosting
 service, including GitHub raw URLs.
@@ -530,6 +537,13 @@ staking-cli update-metadata-uri --metadata-uri https://my-validator.example.com/
     --consensus-public-key BLS_VER_KEY~...
 ```
 
+To skip validation (if endpoint isn't ready):
+
+```bash
+staking-cli update-metadata-uri --metadata-uri https://my-validator.example.com/status/metrics \
+    --skip-metadata-validation
+```
+
 See [Validator Metadata](#validator-metadata) for format options. Use `--no-metadata-uri` to clear.
 
 #### Metadata JSON Schema (for custom hosting)

staking-cli/src/cli.rs

@@ -480,7 +480,7 @@ pub async fn run() -> Result<()> {
 
             // Validate metadata URI if present and validation not skipped
             if let Some(url) = metadata_uri.url() {
-                if !config.skip_metadata_validation {
+                if !metadata_uri_args.skip_metadata_validation {
                     validate_metadata_uri(url, &payload.bls_vk)
                         .await
                         .context("use --skip-metadata-validation to skip")?;
@@ -534,7 +534,7 @@ pub async fn run() -> Result<()> {
 
             // Validate metadata URI if present and validation not skipped
             if let Some(url) = metadata_uri.url() {
-                if !config.skip_metadata_validation {
+                if !metadata_uri_args.skip_metadata_validation {
                     let bls_vk = consensus_public_key.ok_or_else(|| {
                         anyhow::anyhow!(
                             "--consensus-public-key is required for metadata validation (use \

staking-cli/src/lib.rs

@@ -5,7 +5,7 @@ use alloy::{
     signers::local::{coins_bip39::English, MnemonicBuilder, PrivateKeySigner},
 };
 use anyhow::{bail, Result};
-use clap::{ArgAction, Args as ClapArgs, Parser, Subcommand};
+use clap::{ArgAction, Parser, Subcommand};
 use clap_serde_derive::ClapSerde;
 use demo::DelegationConfig;
 use espresso_contract_deployer::provider::connect_ledger;
@@ -14,7 +14,7 @@ pub(crate) use hotshot_types::{
     signature_key::{BLSPrivKey, BLSPubKey},
 };
 pub(crate) use jf_signature::bls_over_bn254::KeyPair as BLSKeyPair;
-use metadata::MetadataUri;
+use metadata::MetadataUriArgs;
 use parse::Commission;
 use sequencer_utils::logging;
 use serde::{Deserialize, Serialize};
@@ -115,11 +115,6 @@ pub struct Config {
     #[serde(skip)]
     pub skip_simulation: bool,
 
-    /// Skip metadata URI validation (fetch and schema check).
-    #[clap(long, env = "SKIP_METADATA_VALIDATION", action = ArgAction::SetTrue)]
-    #[serde(skip)]
-    pub skip_metadata_validation: bool,
-
     #[clap(flatten)]
     #[serde(skip)]
     pub output: OutputArgs,
@@ -219,93 +214,6 @@ impl ValidSignerConfig {
     }
 }
 
-#[derive(ClapArgs, Debug, Clone)]
-#[group(required = true, multiple = false)]
-pub struct MetadataUriArgs {
-    /// URL where validator metadata is hosted (JSON or OpenMetrics format).
-    #[clap(long, env = "METADATA_URI")]
-    pub metadata_uri: Option<String>,
-
-    /// Register without a metadata URI.
-    #[clap(long, env = "NO_METADATA_URI")]
-    pub no_metadata_uri: bool,
-}
-
-impl MetadataUriArgs {
-    /// Parse the metadata URI arguments into an optional URL.
-    fn to_url(&self) -> Result<Option<Url>> {
-        if self.no_metadata_uri {
-            Ok(None)
-        } else if let Some(uri_str) = &self.metadata_uri {
-            Ok(Some(Url::parse(uri_str)?))
-        } else {
-            bail!("Either --metadata-uri or --no-metadata-uri must be provided")
-        }
-    }
-}
-
-impl TryFrom<MetadataUriArgs> for MetadataUri {
-    type Error = anyhow::Error;
-
-    fn try_from(args: MetadataUriArgs) -> Result<Self> {
-        match args.to_url()? {
-            Some(url) => MetadataUri::try_from(url),
-            None => Ok(MetadataUri::empty()),
-        }
-    }
-}
-
-#[cfg(test)]
-mod metadata_uri_args_tests {
-    use super::*;
-
-    #[test]
-    fn test_to_url_with_uri() {
-        let args = MetadataUriArgs {
-            metadata_uri: Some("https://example.com/metadata.json".to_string()),
-            no_metadata_uri: false,
-        };
-        let url = args.to_url().unwrap();
-        assert_eq!(
-            url.map(|u| u.as_str().to_string()),
-            Some("https://example.com/metadata.json".to_string())
-        );
-    }
-
-    #[test]
-    fn test_to_url_with_no_metadata() {
-        let args = MetadataUriArgs {
-            metadata_uri: None,
-            no_metadata_uri: true,
-        };
-        let url = args.to_url().unwrap();
-        assert!(url.is_none());
-    }
-
-    #[test]
-    fn test_metadata_uri_args_to_metadata_uri() {
-        let args = MetadataUriArgs {
-            metadata_uri: Some("https://example.com/metadata.json".to_string()),
-            no_metadata_uri: false,
-        };
-        let metadata_uri = MetadataUri::try_from(args).unwrap();
-        assert_eq!(
-            metadata_uri.url().map(|u| u.as_str()),
-            Some("https://example.com/metadata.json")
-        );
-    }
-
-    #[test]
-    fn test_metadata_uri_args_to_empty_metadata_uri() {
-        let args = MetadataUriArgs {
-            metadata_uri: None,
-            no_metadata_uri: true,
-        };
-        let metadata_uri = MetadataUri::try_from(args).unwrap();
-        assert!(metadata_uri.url().is_none());
-    }
-}
-
 impl Default for Commands {
     fn default() -> Self {
         Commands::StakeTable {

staking-cli/src/metadata.rs

@@ -3,6 +3,7 @@
 use std::{fmt, str::FromStr, time::Duration};
 
 use anyhow::{bail, Context, Result};
+use clap::{ArgAction, Args as ClapArgs};
 use hotshot_types::signature_key::BLSPubKey;
 use thiserror::Error;
 use url::Url;
@@ -148,13 +149,141 @@ impl fmt::Display for MetadataUri {
     }
 }
 
+/// Custom value parser for metadata URIs that enforces 2048 byte limit.
+///
+/// While HTTP specs don't mandate a URL length limit, most browsers and servers
+/// support at least 2048 characters. This practical limit ensures compatibility
+/// across different HTTP clients and servers.
+fn parse_metadata_url(s: &str) -> Result<Url, String> {
+    let url = Url::parse(s).map_err(|e| e.to_string())?;
+    if url.as_str().len() > 2048 {
+        return Err("metadata URI cannot exceed 2048 bytes".to_string());
+    }
+    Ok(url)
+}
+
+/// Command-line arguments for specifying validator metadata.
+#[derive(ClapArgs, Debug, Clone)]
+pub struct MetadataUriArgs {
+    /// URL where validator metadata is hosted (JSON or OpenMetrics format).
+    #[clap(
+        long,
+        env = "METADATA_URI",
+        required_unless_present = "no_metadata_uri",
+        conflicts_with = "no_metadata_uri",
+        value_parser = parse_metadata_url
+    )]
+    pub metadata_uri: Option<Url>,
+
+    /// Register without a metadata URI.
+    // Provided as separate flag to prevent accidental omission of metadata URI.
+    #[clap(long, env = "NO_METADATA_URI", conflicts_with = "metadata_uri")]
+    pub no_metadata_uri: bool,
+
+    /// Skip metadata URI validation (fetch and schema check).
+    #[clap(
+        long,
+        env = "SKIP_METADATA_VALIDATION",
+        action = ArgAction::SetTrue,
+        conflicts_with = "no_metadata_uri",
+        requires = "metadata_uri"
+    )]
+    pub skip_metadata_validation: bool,
+}
+
+impl TryFrom<MetadataUriArgs> for MetadataUri {
+    type Error = anyhow::Error;
+
+    fn try_from(args: MetadataUriArgs) -> Result<Self> {
+        match args.metadata_uri {
+            Some(url) => Self::try_from(url),
+            None => Ok(Self::empty()),
+        }
+    }
+}
+
 #[cfg(test)]
 fn generate_bls_pub_key() -> BLSPubKey {
     use jf_signature::bls_over_bn254::KeyPair;
     let keypair = KeyPair::generate(&mut rand::thread_rng());
     BLSPubKey::from(keypair.ver_key())
 }
 
+#[cfg(test)]
+mod metadata_uri_args_tests {
+    use super::*;
+
+    #[test]
+    fn test_metadata_uri_args_with_url() {
+        let args = MetadataUriArgs {
+            metadata_uri: Some(Url::parse("https://example.com/metadata.json").unwrap()),
+            no_metadata_uri: false,
+            skip_metadata_validation: false,
+        };
+        assert_eq!(
+            args.metadata_uri.as_ref().map(|u| u.as_str()),
+            Some("https://example.com/metadata.json")
+        );
+    }
+
+    #[test]
+    fn test_metadata_uri_args_with_no_metadata() {
+        let args = MetadataUriArgs {
+            metadata_uri: None,
+            no_metadata_uri: true,
+            skip_metadata_validation: false,
+        };
+        assert!(args.metadata_uri.is_none());
+        assert!(args.no_metadata_uri);
+    }
+
+    #[test]
+    fn test_metadata_uri_args_to_metadata_uri() {
+        let args = MetadataUriArgs {
+            metadata_uri: Some(Url::parse("https://example.com/metadata.json").unwrap()),
+            no_metadata_uri: false,
+            skip_metadata_validation: false,
+        };
+        let metadata_uri = MetadataUri::try_from(args).unwrap();
+        assert_eq!(
+            metadata_uri.url().map(|u| u.as_str()),
+            Some("https://example.com/metadata.json")
+        );
+    }
+
+    #[test]
+    fn test_metadata_uri_args_to_empty_metadata_uri() {
+        let args = MetadataUriArgs {
+            metadata_uri: None,
+            no_metadata_uri: true,
+            skip_metadata_validation: false,
+        };
+        let metadata_uri = MetadataUri::try_from(args).unwrap();
+        assert!(metadata_uri.url().is_none());
+    }
+
+    #[test]
+    fn test_parse_metadata_url_valid() {
+        let url = parse_metadata_url("https://example.com/metadata").unwrap();
+        assert_eq!(url.as_str(), "https://example.com/metadata");
+    }
+
+    #[test]
+    fn test_parse_metadata_url_too_long() {
+        let long_path = "a".repeat(2100);
+        let url_str = format!("https://example.com/{}", long_path);
+        let result = parse_metadata_url(&url_str);
+        assert!(result.is_err());
+        assert!(result.unwrap_err().contains("cannot exceed 2048 bytes"));
+    }
+
+    #[test]
+    fn test_parse_metadata_url_invalid() {
+        let result = parse_metadata_url("not a url");
+        assert!(result.is_err());
+    }
+}
+
 #[cfg(test)]
 mod test {
     use super::*;