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

CHANGELOG.md

@@ -29,6 +29,8 @@
 
 ### Added
 
+- [#6012](https://github.com/ChainSafe/forest/issues/6012): Stricter validation of address arguments in `forest-wallet` subcommands. Addresses passed to `balance`, `export`, `has`, `delete`, `set-default`, `sign`, `verify` and the `--from` option of `send` are now parsed into `StrictAddress` at CLI-parse time, producing clearer errors for malformed inputs and rejecting mismatched network prefixes up-front. The `send` subcommand's positional `target_address` is intentionally left as a raw string so that ETH (`0x...`) recipients continue to be accepted.
+
 ### Changed
 
 ### Removed

src/wallet/main.rs

@@ -14,6 +14,19 @@ pub async fn main<ArgT>(args: impl IntoIterator<Item = ArgT>) -> anyhow::Result<
 where
     ArgT: Into<OsString> + Clone,
 {
+    // Preliminary client without a token, used only to detect the target
+    // network. Must happen BEFORE `Cli::parse_from` so that clap-driven
+    // `StrictAddress` validation accepts testnet (`t0...`) addresses. Client
+    // construction errors propagate (mirroring `forest-cli` in #6011); if the
+    // daemon itself is unreachable, the global `CurrentNetwork` stays at its
+    // mainnet default and testnet addresses will be rejected at parse time.
+    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 +37,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,42 @@ pub struct Cli {
     #[command(subcommand)]
     pub cmd: wallet_cmd::WalletCommands,
 }
+
+#[cfg(test)]
+mod tests {
+    use super::Cli;
+    use clap::Parser;
+
+    /// Guards against an accidental revert of #6012 (address args ↔ `StrictAddress`).
+    /// Malformed addresses must be rejected by clap at parse time, with a
+    /// `ValueValidation` error rather than succeeding and failing later.
+    fn parse_err_kind(args: &[&str]) -> clap::error::ErrorKind {
+        match Cli::try_parse_from(args.iter().copied()) {
+            Ok(_) => panic!("expected clap parse to fail for {args:?}"),
+            Err(e) => e.kind(),
+        }
+    }
+
+    #[test]
+    fn wallet_balance_rejects_malformed_address() {
+        assert_eq!(
+            parse_err_kind(&["forest-wallet", "balance", "not-an-address"]),
+            clap::error::ErrorKind::ValueValidation,
+        );
+    }
+
+    #[test]
+    fn wallet_sign_rejects_malformed_address() {
+        assert_eq!(
+            parse_err_kind(&[
+                "forest-wallet",
+                "sign",
+                "-m",
+                "deadbeef",
+                "-a",
+                "not-an-address",
+            ]),
+            clap::error::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 a
+        /// `String` (rather than `StrictAddress`) because `StrictAddress`
+        /// rejects the ETH form, which `resolve_target_address` handles.
         target_address: String,
         #[arg(value_parser = humantoken::parse)]
         amount: TokenAmount,
@@ -325,12 +327,10 @@ impl WalletCommands {
                 Ok(())
             }
             Self::Balance {
-                address,
+                address: StrictAddress(address),
                 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?;
                 println!("{}", format_balance(&balance, no_round, no_abbrev));
                 Ok(())
@@ -344,26 +344,22 @@ impl WalletCommands {
                 Ok(())
             }
             Self::Export {
-                address: address_string,
+                address: StrictAddress(address),
             } => {
-                let StrictAddress(address) = StrictAddress::from_str(&address_string)
-                    .with_context(|| format!("Invalid address: {address_string}"))?;
                 let key_info = backend.wallet_export(address).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}"))?;
-
+            Self::Has {
+                key: StrictAddress(address),
+            } => {
                 println!("{response}", response = backend.wallet_has(address).await?);
                 Ok(())
             }
-            Self::Delete { address } => {
-                let StrictAddress(address) = StrictAddress::from_str(&address)
-                    .with_context(|| format!("Invalid address: {address}"))?;
-
+            Self::Delete {
+                address: StrictAddress(address),
+            } => {
                 backend.wallet_delete(address).await?;
                 println!("deleted {address}.");
                 Ok(())
@@ -410,9 +406,7 @@ impl WalletCommands {
                 let (key_pairs, default) =
                     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 default_address = default;
 
                 let remote = &backend.remote;
                 let results =
@@ -455,16 +449,13 @@ 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::Sign { address, message } => {
-                let StrictAddress(address) = StrictAddress::from_str(&address)
-                    .with_context(|| format!("Invalid address: {address}"))?;
-
+            Self::SetDefault {
+                key: StrictAddress(key),
+            } => backend.wallet_set_default(key).await,
+            Self::Sign {
+                address: StrictAddress(address),
+                message,
+            } => {
                 let message = hex::decode(message).context("Message has to be a hex string")?;
                 let message = BASE64_STANDARD.encode(message);
 
@@ -479,13 +470,11 @@ impl WalletCommands {
             }
             Self::Verify {
                 message,
-                address,
+                address: StrictAddress(address),
                 signature,
             } => {
                 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)?;
@@ -502,13 +491,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(StrictAddress(a)) => a,
+                    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)?;