feat(wallet): strict address validation in forest-wallet CLI

Mirrors the `forest-cli` treatment introduced in #6011 for the
`forest-wallet` binary. Address arguments to `balance`, `export`, `has`,
`delete`, `set-default`, `sign`, `verify` and the `--from` option of
`send` are now parsed into `StrictAddress` at CLI-parse time. clap
surfaces a `ValueValidation` error on a malformed input instead of
deferring to a runtime `StrictAddress::from_str(...)?` fallback, and
each handler simply destructures the typed value via
`field: StrictAddress(inner)`.

Network detection is moved ahead of `Cli::parse_from` so that clap-time
`StrictAddress` validation accepts testnet (`t0...`) addresses when the
daemon reports a testnet chain. Client construction errors propagate
(matching `forest-cli`); an unreachable daemon leaves the global
`CurrentNetwork` at its mainnet default.

`wallet_default_address()` now returns `Option<Address>` rather than
`Option<String>`, removing a stringify-then-reparse round trip through
`StrictAddress::from_str` in both the `list` and `send` handlers.

`Send.target_address` is intentionally kept as a `String` because it
accepts ETH (`0x...`) recipients, which `StrictAddress` rejects;
`resolve_target_address` continues to handle the dispatch.
`ValidateAddress.address` also stays a `String`, since its purpose is
to validate arbitrary user input.

Two clap-level regression tests guard against an accidental revert of
the `StrictAddress` typing.

Refs #6012

Author: 0xDevNinja

CHANGELOG.md

@@ -29,6 +29,8 @@
 
 ### Added
 
+- [#6012](https://github.com/ChainSafe/forest/issues/6012): Stricter validation of address arguments in `forest-wallet` subcommands.
+
 ### Changed
 
 ### Removed

src/wallet/main.rs

@@ -14,6 +14,14 @@ pub async fn main<ArgT>(args: impl IntoIterator<Item = ArgT>) -> anyhow::Result<
 where
     ArgT: Into<OsString> + Clone,
 {
+    // Preliminary client without the token to check network. This needs to occur before parsing to ensure the `StrictAddress` validation works correctly.
+    let client = rpc::Client::default_or_from_env(None)?;
+    if let Ok(name) = StateNetworkName::call(&client, ()).await
+        && !matches!(NetworkChain::from_str(&name), Ok(NetworkChain::Mainnet))
+    {
+        CurrentNetwork::set_global(Network::Testnet);
+    }
+
     // Capture Cli inputs
     let Cli {
         opts,
@@ -24,11 +32,6 @@ where
 
     let client = rpc::Client::default_or_from_env(opts.token.as_deref())?;
 
-    let name = StateNetworkName::call(&client, ()).await?;
-    let chain = NetworkChain::from_str(&name)?;
-    if chain.is_testnet() {
-        CurrentNetwork::set_global(Network::Testnet);
-    }
     // Run command
     cmd.run(client, remote_wallet, encrypt).await
 }

src/wallet/subcommands/mod.rs

@@ -29,3 +29,29 @@ pub struct Cli {
     #[command(subcommand)]
     pub cmd: wallet_cmd::WalletCommands,
 }
+
+#[cfg(test)]
+mod tests {
+    use super::Cli;
+    use clap::Parser;
+    use clap::error::ErrorKind;
+    use rstest::rstest;
+
+    fn try_parse(args: &[&str]) -> Result<Cli, ErrorKind> {
+        let argv = std::iter::once("forest-wallet").chain(args.iter().copied());
+        Cli::try_parse_from(argv).map_err(|e| e.kind())
+    }
+
+    #[rstest]
+    #[case::balance(&["balance", "not-an-address"])]
+    #[case::export(&["export", "not-an-address"])]
+    #[case::has(&["has", "not-an-address"])]
+    #[case::set_default(&["set-default", "not-an-address"])]
+    #[case::delete(&["delete", "not-an-address"])]
+    #[case::sign(&["sign", "-m", "de", "-a", "not-an-address"])]
+    #[case::verify(&["verify", "-a", "not-an-address", "-m", "de", "-s", "de"])]
+    #[case::send_from(&["send", "--from", "not-an-address", "f01234", "1"])]
+    fn migrated_subcommands_reject_malformed_address(#[case] args: &[&str]) {
+        assert_eq!(try_parse(args).err(), Some(ErrorKind::ValueValidation));
+    }
+}

src/wallet/subcommands/wallet_cmd.rs

@@ -149,13 +149,11 @@ impl WalletBackend {
         }
     }
 
-    async fn wallet_default_address(&self) -> anyhow::Result<Option<String>> {
+    async fn wallet_default_address(&self) -> anyhow::Result<Option<Address>> {
         if let Some(keystore) = &self.local {
-            Ok(crate::key_management::get_default(keystore)?.map(|s| s.to_string()))
+            Ok(crate::key_management::get_default(keystore)?)
         } else {
-            Ok(WalletDefaultAddress::call(&self.remote, ())
-                .await?
-                .map(|it| it.to_string()))
+            Ok(WalletDefaultAddress::call(&self.remote, ()).await?)
         }
     }
 
@@ -211,7 +209,7 @@ pub enum WalletCommands {
     /// Get account balance
     Balance {
         /// The address of the account to check
-        address: String,
+        address: StrictAddress,
         /// Output is rounded to 4 significant figures by default.
         /// Do not round
         // ENHANCE(aatifsyed): add a --round/--no-round argument pair
@@ -227,12 +225,12 @@ pub enum WalletCommands {
     /// Export the wallet's keys
     Export {
         /// The address that contains the keys to export
-        address: String,
+        address: StrictAddress,
     },
     /// Check if the wallet has a key
     Has {
         /// The key to check
-        key: String,
+        key: StrictAddress,
     },
     /// Import keys from existing wallet
     Import {
@@ -254,7 +252,7 @@ pub enum WalletCommands {
     /// Set the default wallet address
     SetDefault {
         /// The given key to set to the default address
-        key: String,
+        key: StrictAddress,
     },
     /// Sign a message
     Sign {
@@ -263,7 +261,7 @@ pub enum WalletCommands {
         message: String,
         /// The address to be used to sign the message
         #[arg(short)]
-        address: String,
+        address: StrictAddress,
     },
     /// Validates whether a given string can be decoded as a well-formed address
     ValidateAddress {
@@ -275,7 +273,7 @@ pub enum WalletCommands {
     Verify {
         /// The address used to sign the message
         #[arg(short)]
-        address: String,
+        address: StrictAddress,
         /// The message to verify
         #[arg(short)]
         message: String,
@@ -286,14 +284,18 @@ pub enum WalletCommands {
     /// Deletes the wallet associated with the given address.
     Delete {
         /// The address of the wallet to delete
-        address: String,
+        address: StrictAddress,
     },
     /// Send funds between accounts
     Send {
         /// optionally specify the account to send funds from (otherwise the default
         /// one will be used)
         #[arg(long)]
-        from: Option<String>,
+        from: Option<StrictAddress>,
+        /// The recipient address. Accepts either a FIL address (e.g.
+        /// `f1.../t1...`) or an ETH address (e.g. `0x...`).
+        // Kept as `String` rather than `StrictAddress` because the latter
+        // rejects the ETH form, which `resolve_target_address` handles.
         target_address: String,
         #[arg(value_parser = humantoken::parse)]
         amount: TokenAmount,
@@ -329,9 +331,7 @@ impl WalletCommands {
                 no_round,
                 no_abbrev,
             } => {
-                let StrictAddress(address) = StrictAddress::from_str(&address)
-                    .with_context(|| format!("Invalid address: {address}"))?;
-                let balance = WalletBalance::call(&backend.remote, (address,)).await?;
+                let balance = WalletBalance::call(&backend.remote, (address.into(),)).await?;
                 println!("{}", format_balance(&balance, no_round, no_abbrev));
                 Ok(())
             }
@@ -343,27 +343,21 @@ impl WalletCommands {
                 println!("{default_addr}");
                 Ok(())
             }
-            Self::Export {
-                address: address_string,
-            } => {
-                let StrictAddress(address) = StrictAddress::from_str(&address_string)
-                    .with_context(|| format!("Invalid address: {address_string}"))?;
-                let key_info = backend.wallet_export(address).await?;
+            Self::Export { address } => {
+                let key_info = backend.wallet_export(address.into()).await?;
                 let encoded_key = key_info.into_lotus_json_string()?;
                 println!("{}", hex::encode(encoded_key));
                 Ok(())
             }
             Self::Has { key } => {
-                let StrictAddress(address) = StrictAddress::from_str(&key)
-                    .with_context(|| format!("Invalid address: {key}"))?;
-
-                println!("{response}", response = backend.wallet_has(address).await?);
+                println!(
+                    "{response}",
+                    response = backend.wallet_has(key.into()).await?
+                );
                 Ok(())
             }
             Self::Delete { address } => {
-                let StrictAddress(address) = StrictAddress::from_str(&address)
-                    .with_context(|| format!("Invalid address: {address}"))?;
-
+                let address: Address = address.into();
                 backend.wallet_delete(address).await?;
                 println!("deleted {address}.");
                 Ok(())
@@ -407,13 +401,9 @@ impl WalletCommands {
                 no_round,
                 no_abbrev,
             } => {
-                let (key_pairs, default) =
+                let (key_pairs, default_address) =
                     tokio::try_join!(backend.list_addrs(), backend.wallet_default_address(),)?;
 
-                let default_address = default
-                    .as_deref()
-                    .and_then(|s| StrictAddress::from_str(s).ok().map(Into::into));
-
                 let remote = &backend.remote;
                 let results =
                     futures::future::join_all(key_pairs.iter().copied().map(|a| async move {
@@ -455,20 +445,12 @@ impl WalletCommands {
                 println!("{list}");
                 Ok(())
             }
-            Self::SetDefault { key } => {
-                let StrictAddress(key) = StrictAddress::from_str(&key)
-                    .with_context(|| format!("Invalid address: {key}"))?;
-
-                backend.wallet_set_default(key).await
-            }
+            Self::SetDefault { key } => backend.wallet_set_default(key.into()).await,
             Self::Sign { address, message } => {
-                let StrictAddress(address) = StrictAddress::from_str(&address)
-                    .with_context(|| format!("Invalid address: {address}"))?;
-
                 let message = hex::decode(message).context("Message has to be a hex string")?;
                 let message = BASE64_STANDARD.encode(message);
 
-                let signature = backend.wallet_sign(address, message).await?;
+                let signature = backend.wallet_sign(address.into(), message).await?;
                 println!("{}", hex::encode(signature.to_bytes()));
                 Ok(())
             }
@@ -484,12 +466,12 @@ impl WalletCommands {
             } => {
                 let sig_bytes =
                     hex::decode(signature).context("Signature has to be a hex string")?;
-                let StrictAddress(address) = StrictAddress::from_str(&address)
-                    .with_context(|| format!("Invalid address: {address}"))?;
                 let msg = hex::decode(message).context("Message has to be a hex string")?;
 
                 let signature = Signature::from_bytes(sig_bytes)?;
-                let is_valid = backend.wallet_verify(address, msg, signature).await?;
+                let is_valid = backend
+                    .wallet_verify(address.into(), msg, signature)
+                    .await?;
 
                 println!("{is_valid}");
                 Ok(())
@@ -502,13 +484,11 @@ impl WalletCommands {
                 gas_limit,
                 gas_premium,
             } => {
-                let from: Address = if let Some(from) = from {
-                    StrictAddress::from_str(&from)?.into()
-                } else {
-                    StrictAddress::from_str(&backend.wallet_default_address().await?.context(
+                let from: Address = match from {
+                    Some(a) => a.into(),
+                    None => backend.wallet_default_address().await?.context(
                         "No default wallet address selected. Please set a default address.",
-                    )?)?
-                    .into()
+                    )?,
                 };
 
                 let (mut to, is_0x_recipient) = resolve_target_address(&target_address)?;