From 5d1f696223c48031614076aee362777ac800b352 Mon Sep 17 00:00:00 2001 From: Enrique Ortiz Date: Wed, 3 May 2023 10:14:32 -0400 Subject: [PATCH 1/7] feat: add way to specify remote signing to fall back to eth_sendTransaction --- cli/src/cmd/cast/send.rs | 19 ++++++++++++++++++- cli/src/opts/wallet/mod.rs | 13 ++++++++++++- 2 files changed, 30 insertions(+), 2 deletions(-) diff --git a/cli/src/cmd/cast/send.rs b/cli/src/cmd/cast/send.rs index 5c14a90b8ebb5..374f840d18374 100644 --- a/cli/src/cmd/cast/send.rs +++ b/cli/src/cmd/cast/send.rs @@ -87,6 +87,7 @@ impl SendTxArgs { let chain = utils::get_chain(config.chain_id, &provider).await?; let api_key = config.get_etherscan_api_key(Some(chain)); let mut sig = sig.unwrap_or_default(); + let local_signer_selected = eth.wallet.local_signer_selected(); if let Ok(signer) = eth.wallet.signer(chain.id()).await { let from = signer.address(); @@ -138,7 +139,23 @@ impl SendTxArgs { to_json, ) .await - } else if config.sender != Config::DEFAULT_SENDER { + // A different sender was specified with a local signing option, but there's no signer. + // This means that there's not enough information to sign locally. + } else if config.sender != Config::DEFAULT_SENDER && local_signer_selected { + Err(eyre::eyre!( + "\ + A local signing method was provided, but Cast can't seem to find or use the signer.\n + Depending on the signing method provided, please make sure that either:\n + - You have a valid keystore or mnemonic path pointing to the keystore file, with the correct password.\n + - Your ledger is connected and unlocked with the correct app open and no other wallet apps open.\n + - Your AWS information is set up correctly. + " + )) + } + // A different sender was specified from the default and the remote flag was + // used. This means that the user is using either a local node or a remote RPC + // that has the keys necessary to sign the transaction. + else if config.sender != Config::DEFAULT_SENDER && !local_signer_selected { // Checking if signer isn't the default value // 00a329c0648769A73afAc7F9381E08FB43dBEA72. if resend { diff --git a/cli/src/opts/wallet/mod.rs b/cli/src/opts/wallet/mod.rs index edf03ea317844..2ca4477173989 100644 --- a/cli/src/opts/wallet/mod.rs +++ b/cli/src/opts/wallet/mod.rs @@ -35,6 +35,7 @@ pub mod error; /// 5. Private Key (cleartext in CLI) /// 6. Private Key (interactively via secure prompt) /// 7. AWS KMS +/// 8. Remote (Local node that holds keys or an RPC that holds keys). #[derive(Parser, Debug, Default, Clone, Serialize)] #[clap(next_help_heading = "Wallet options", about = None, long_about = None)] pub struct Wallet { @@ -133,8 +134,14 @@ pub struct Wallet { pub trezor: bool, /// Use AWS Key Management Service. - #[clap(long, help_heading = "Wallet options - remote")] + #[clap(long, help_heading = "Wallet options - AWS KMS")] pub aws: bool, + + /// Enable remote signing. + /// Useful when using a local node or an RPC that has keys able to sign the transaction. + /// This will allow the tx to fallback to eth_sendTransaction. + #[clap(long, short, help_heading = "Wallet options - Remote node signing")] + pub remote: bool, } impl Wallet { @@ -180,6 +187,10 @@ impl Wallet { } } + pub fn local_signer_selected(&self) -> bool { + return !self.remote + } + /// Tries to resolve a local wallet from the provided options. #[track_caller] fn try_resolve_local_wallet(&self) -> Result> { From a84ad353205c0bc3938eb9c1cd36a8c2c5f50469 Mon Sep 17 00:00:00 2001 From: Enrique Ortiz Date: Thu, 4 May 2023 01:34:41 -0400 Subject: [PATCH 2/7] chore: bail on walking folders, add some more message context around ledger/local failures --- cli/src/opts/wallet/mod.rs | 50 +++++++++++--------------------------- 1 file changed, 14 insertions(+), 36 deletions(-) diff --git a/cli/src/opts/wallet/mod.rs b/cli/src/opts/wallet/mod.rs index 2ca4477173989..66719182c4217 100644 --- a/cli/src/opts/wallet/mod.rs +++ b/cli/src/opts/wallet/mod.rs @@ -13,7 +13,7 @@ use ethers::{ Address, Signature, }, }; -use eyre::{bail, eyre, Result, WrapErr}; +use eyre::{bail, Result, WrapErr}; use foundry_common::fs; use serde::{Deserialize, Serialize}; use std::{ @@ -35,7 +35,6 @@ pub mod error; /// 5. Private Key (cleartext in CLI) /// 6. Private Key (interactively via secure prompt) /// 7. AWS KMS -/// 8. Remote (Local node that holds keys or an RPC that holds keys). #[derive(Parser, Debug, Default, Clone, Serialize)] #[clap(next_help_heading = "Wallet options", about = None, long_about = None)] pub struct Wallet { @@ -136,12 +135,6 @@ pub struct Wallet { /// Use AWS Key Management Service. #[clap(long, help_heading = "Wallet options - AWS KMS")] pub aws: bool, - - /// Enable remote signing. - /// Useful when using a local node or an RPC that has keys able to sign the transaction. - /// This will allow the tx to fallback to eth_sendTransaction. - #[clap(long, short, help_heading = "Wallet options - Remote node signing")] - pub remote: bool, } impl Wallet { @@ -187,10 +180,6 @@ impl Wallet { } } - pub fn local_signer_selected(&self) -> bool { - return !self.remote - } - /// Tries to resolve a local wallet from the provided options. #[track_caller] fn try_resolve_local_wallet(&self) -> Result> { @@ -212,7 +201,13 @@ impl Wallet { Some(hd_path) => LedgerHDPath::Other(hd_path.clone()), None => LedgerHDPath::LedgerLive(self.mnemonic_index as usize), }; - let ledger = Ledger::new(derivation, chain_id).await?; + let ledger = Ledger::new(derivation, chain_id).await.wrap_err_with(|| { + "\ + Could not connect to Ledger device.\n\ + Make sure it's connected and unlocked, with no other desktop wallet apps open.\ + " + .to_string() + })?; Ok(WalletSigner::Ledger(ledger)) } else if self.trezor { @@ -249,8 +244,8 @@ impl Wallet { --private-key, --mnemonic-path, --aws, --interactive, --trezor or --ledger.\n\ \n\ Alternatively, if you're using a local node with unlocked accounts,\n\ - use the --unlocked flag and set the `ETH_FROM` environment variable to the address\n\ - of the unlocked account you want to use.\ + use the --unlocked flag and either set the `ETH_FROM` environment variable to the address\n\ + of the unlocked account you want to use, or provide the --from flag with the address directly.\ "))?; Ok(WalletSigner::Local(local.with_chain_id(chain_id))) @@ -327,10 +322,10 @@ pub trait WalletTrait { Ok(builder.build()?) } - /// Attempts to find the actual path of the keystore file. + /// Ensures the path to the keystore exists. /// - /// If the path is a directory then we try to find the first keystore file with the correct - /// sender address + /// if the path is a directory, it bails and asks the user to specify the keystore file + /// directly. fn find_keystore_file(&self, path: impl AsRef) -> Result { let path = path.as_ref(); if !path.exists() { @@ -338,24 +333,7 @@ pub trait WalletTrait { } if path.is_dir() { - let sender = - self.sender().ok_or_else(|| eyre!("No sender account configured: $ETH_FROM"))?; - - let (_, file) = walkdir::WalkDir::new(path) - .max_depth(2) - .into_iter() - .filter_map(Result::ok) - .filter(|e| e.file_type().is_file()) - .filter_map(|e| { - fs::read_json_file::(e.path()) - .map(|keystore| (keystore, e.path().to_path_buf())) - .ok() - }) - .find(|(keystore, _)| keystore.address == sender) - .ok_or_else(|| { - eyre!("No matching keystore file found for {sender:?} in {path:?}") - })?; - return Ok(file) + bail!("Keystore path `{path:?}` is a directory. Please specify the keystore file directly.") } Ok(path.to_path_buf()) From 0eca4ffd7c2fb608d5693960801f639235759355 Mon Sep 17 00:00:00 2001 From: Enrique Ortiz Date: Thu, 4 May 2023 01:35:14 -0400 Subject: [PATCH 3/7] chore: light refactor on send command to add more strictness --- cli/src/cmd/cast/send.rs | 125 ++++++++++++++++----------------------- 1 file changed, 51 insertions(+), 74 deletions(-) diff --git a/cli/src/cmd/cast/send.rs b/cli/src/cmd/cast/send.rs index 374f840d18374..7f67e11c8db59 100644 --- a/cli/src/cmd/cast/send.rs +++ b/cli/src/cmd/cast/send.rs @@ -50,6 +50,10 @@ pub struct SendTxArgs { #[clap(subcommand)] command: Option, + + /// Send via `eth_sendTransaction using the `--from` argument or $ETH_FROM as sender + #[clap(long, requires = "from")] + unlocked: bool, } #[derive(Debug, Parser)] @@ -81,15 +85,59 @@ impl SendTxArgs { json: to_json, resend, command, + unlocked, } = self; let config = Config::from(ð); let provider = utils::get_provider(&config)?; let chain = utils::get_chain(config.chain_id, &provider).await?; let api_key = config.get_etherscan_api_key(Some(chain)); let mut sig = sig.unwrap_or_default(); - let local_signer_selected = eth.wallet.local_signer_selected(); - if let Ok(signer) = eth.wallet.signer(chain.id()).await { + let code = if let Some(SendTxSubcommands::Create { + code, + sig: constructor_sig, + args: constructor_args, + }) = command + { + sig = constructor_sig.unwrap_or_default(); + args = constructor_args; + Some(code) + } else { + None + }; + + // Case 1: + // Default to sending via eth_sendTransaction if the --unlocked flag is passed. + // This should be the only way this RPC method is used as it requires a local node + // or remote RPC with unlocked accounts. + if unlocked { + // Checking if signer isn't the default value + // 00a329c0648769A73afAc7F9381E08FB43dBEA72. + if resend { + tx.nonce = Some(provider.get_transaction_count(config.sender, None).await?); + } + + cast_send( + provider, + config.sender, + to, + code, + (sig, args), + tx, + chain, + api_key, + cast_async, + confirmations, + to_json, + ) + .await + // Case 2: + // An option to use a local signer was provided. + // If we cannot successfully instanciate a local signer, then we will assume we don't have + // enough information to sign and we must bail. + } else { + // Retrieve the signer, and bail if it can't be constructed. + let signer = eth.wallet.signer(chain.id()).await?; let from = signer.address(); // prevent misconfigured hwlib from sending a transaction that defies @@ -100,8 +148,7 @@ impl SendTxArgs { The specified sender via CLI/env vars does not match the sender configured via\n\ the hardware wallet's HD Path.\n\ Please use the `--hd-path ` parameter to specify the BIP32 Path which\n\ - corresponds to the sender.\n\ - This will be automatically detected in the future: https://github.com/foundry-rs/foundry/issues/2289\ + corresponds to the sender, or let foundry automatically detect it by not specifying any sender address.\n\ ") } } @@ -110,19 +157,6 @@ impl SendTxArgs { tx.nonce = Some(provider.get_transaction_count(from, None).await?); } - let code = if let Some(SendTxSubcommands::Create { - code, - sig: constructor_sig, - args: constructor_args, - }) = command - { - sig = constructor_sig.unwrap_or_default(); - args = constructor_args; - Some(code) - } else { - None - }; - let provider = provider.with_signer(signer); cast_send( @@ -139,63 +173,6 @@ impl SendTxArgs { to_json, ) .await - // A different sender was specified with a local signing option, but there's no signer. - // This means that there's not enough information to sign locally. - } else if config.sender != Config::DEFAULT_SENDER && local_signer_selected { - Err(eyre::eyre!( - "\ - A local signing method was provided, but Cast can't seem to find or use the signer.\n - Depending on the signing method provided, please make sure that either:\n - - You have a valid keystore or mnemonic path pointing to the keystore file, with the correct password.\n - - Your ledger is connected and unlocked with the correct app open and no other wallet apps open.\n - - Your AWS information is set up correctly. - " - )) - } - // A different sender was specified from the default and the remote flag was - // used. This means that the user is using either a local node or a remote RPC - // that has the keys necessary to sign the transaction. - else if config.sender != Config::DEFAULT_SENDER && !local_signer_selected { - // Checking if signer isn't the default value - // 00a329c0648769A73afAc7F9381E08FB43dBEA72. - if resend { - tx.nonce = Some(provider.get_transaction_count(config.sender, None).await?); - } - - let code = if let Some(SendTxSubcommands::Create { - code, - sig: constructor_sig, - args: constructor_args, - }) = command - { - sig = constructor_sig.unwrap_or_default(); - args = constructor_args; - Some(code) - } else { - None - }; - - cast_send( - provider, - config.sender, - to, - code, - (sig, args), - tx, - chain, - api_key, - cast_async, - confirmations, - to_json, - ) - .await - } else { - Err(eyre::eyre!( - "\ - No wallet or sender address provided. Consider passing it via the --from flag or\n\ - setting the ETH_FROM env variable or setting in foundry.toml\ - " - )) } } } From c511f56ebb310fc5adaeb0f2d6a06d2f12eb162a Mon Sep 17 00:00:00 2001 From: Enrique Ortiz Date: Thu, 4 May 2023 09:59:42 -0400 Subject: [PATCH 4/7] chore: change error messages --- cli/src/cmd/cast/send.rs | 16 ++++++++-------- cli/src/opts/wallet/mod.rs | 34 ++++++++++++++++++---------------- 2 files changed, 26 insertions(+), 24 deletions(-) diff --git a/cli/src/cmd/cast/send.rs b/cli/src/cmd/cast/send.rs index 7f67e11c8db59..09639ebda8e69 100644 --- a/cli/src/cmd/cast/send.rs +++ b/cli/src/cmd/cast/send.rs @@ -111,8 +111,6 @@ impl SendTxArgs { // This should be the only way this RPC method is used as it requires a local node // or remote RPC with unlocked accounts. if unlocked { - // Checking if signer isn't the default value - // 00a329c0648769A73afAc7F9381E08FB43dBEA72. if resend { tx.nonce = Some(provider.get_transaction_count(config.sender, None).await?); } @@ -144,12 +142,14 @@ impl SendTxArgs { // user-specified --from if let Some(specified_from) = eth.wallet.from { if specified_from != from { - eyre::bail!("\ - The specified sender via CLI/env vars does not match the sender configured via\n\ - the hardware wallet's HD Path.\n\ - Please use the `--hd-path ` parameter to specify the BIP32 Path which\n\ - corresponds to the sender, or let foundry automatically detect it by not specifying any sender address.\n\ - ") + eyre::bail!( +r#" +The specified sender via CLI/env vars does not match the sender configured via +the hardware wallet's HD Path. +Please use the `--hd-path ` parameter to specify the BIP32 Path which +corresponds to the sender, or let foundry automatically detect it by not specifying any sender address. +"# + ) } } diff --git a/cli/src/opts/wallet/mod.rs b/cli/src/opts/wallet/mod.rs index 66719182c4217..e75be327ab96a 100644 --- a/cli/src/opts/wallet/mod.rs +++ b/cli/src/opts/wallet/mod.rs @@ -202,11 +202,10 @@ impl Wallet { None => LedgerHDPath::LedgerLive(self.mnemonic_index as usize), }; let ledger = Ledger::new(derivation, chain_id).await.wrap_err_with(|| { - "\ - Could not connect to Ledger device.\n\ - Make sure it's connected and unlocked, with no other desktop wallet apps open.\ - " - .to_string() +r#" +Could not connect to Ledger device. +Make sure it's connected and unlocked, with no other desktop wallet apps open. +"# })?; Ok(WalletSigner::Ledger(ledger)) @@ -217,7 +216,11 @@ impl Wallet { }; // cached to ~/.ethers-rs/trezor/cache/trezor.session - let trezor = Trezor::new(derivation, chain_id, None).await?; + let trezor = Trezor::new(derivation, chain_id, None).await.wrap_err_with(|| { +r#" +Could not connect to Trezor device. +Make sure it's connected and unlocked, with no other conflicting desktop wallet apps open."# + })?; Ok(WalletSigner::Trezor(trezor)) } else if self.aws { @@ -237,16 +240,15 @@ impl Wallet { let maybe_local = self.try_resolve_local_wallet()?; let local = maybe_local - .ok_or_else(|| eyre::eyre!("\ - Error accessing local wallet. Did you set a private key, mnemonic or keystore?\n\ - Run `cast send --help` or `forge create --help` and use the corresponding CLI\n\ - flag to set your key via:\n\ - --private-key, --mnemonic-path, --aws, --interactive, --trezor or --ledger.\n\ - \n\ - Alternatively, if you're using a local node with unlocked accounts,\n\ - use the --unlocked flag and either set the `ETH_FROM` environment variable to the address\n\ - of the unlocked account you want to use, or provide the --from flag with the address directly.\ - "))?; + .ok_or_else(|| eyre::eyre!(r#" +Error accessing local wallet. Did you set a private key, mnemonic or keystore? +Run `cast send --help` or `forge create --help` and use the corresponding CLI +flag to set your key via: +--private-key, --mnemonic-path, --aws, --interactive, --trezor or --ledger. +Alternatively, if you're using a local node with unlocked accounts, +use the --unlocked flag and either set the `ETH_FROM` environment variable to the address +of the unlocked account you want to use, or provide the --from flag with the address directly. + "#))?; Ok(WalletSigner::Local(local.with_chain_id(chain_id))) } From 10fab16a78f120d9735ede6a9e44556ce4601486 Mon Sep 17 00:00:00 2001 From: Enrique Ortiz Date: Thu, 4 May 2023 11:16:02 -0400 Subject: [PATCH 5/7] chore: lint --- cli/src/cmd/cast/send.rs | 2 +- cli/src/opts/wallet/mod.rs | 13 ++++++++----- 2 files changed, 9 insertions(+), 6 deletions(-) diff --git a/cli/src/cmd/cast/send.rs b/cli/src/cmd/cast/send.rs index 09639ebda8e69..a709accf60037 100644 --- a/cli/src/cmd/cast/send.rs +++ b/cli/src/cmd/cast/send.rs @@ -143,7 +143,7 @@ impl SendTxArgs { if let Some(specified_from) = eth.wallet.from { if specified_from != from { eyre::bail!( -r#" + r#" The specified sender via CLI/env vars does not match the sender configured via the hardware wallet's HD Path. Please use the `--hd-path ` parameter to specify the BIP32 Path which diff --git a/cli/src/opts/wallet/mod.rs b/cli/src/opts/wallet/mod.rs index e75be327ab96a..9fe0baa392184 100644 --- a/cli/src/opts/wallet/mod.rs +++ b/cli/src/opts/wallet/mod.rs @@ -202,7 +202,7 @@ impl Wallet { None => LedgerHDPath::LedgerLive(self.mnemonic_index as usize), }; let ledger = Ledger::new(derivation, chain_id).await.wrap_err_with(|| { -r#" + r#" Could not connect to Ledger device. Make sure it's connected and unlocked, with no other desktop wallet apps open. "# @@ -217,7 +217,7 @@ Make sure it's connected and unlocked, with no other desktop wallet apps open. // cached to ~/.ethers-rs/trezor/cache/trezor.session let trezor = Trezor::new(derivation, chain_id, None).await.wrap_err_with(|| { -r#" + r#" Could not connect to Trezor device. Make sure it's connected and unlocked, with no other conflicting desktop wallet apps open."# })?; @@ -239,8 +239,9 @@ Make sure it's connected and unlocked, with no other conflicting desktop wallet let maybe_local = self.try_resolve_local_wallet()?; - let local = maybe_local - .ok_or_else(|| eyre::eyre!(r#" + let local = maybe_local.ok_or_else(|| { + eyre::eyre!( + r#" Error accessing local wallet. Did you set a private key, mnemonic or keystore? Run `cast send --help` or `forge create --help` and use the corresponding CLI flag to set your key via: @@ -248,7 +249,9 @@ flag to set your key via: Alternatively, if you're using a local node with unlocked accounts, use the --unlocked flag and either set the `ETH_FROM` environment variable to the address of the unlocked account you want to use, or provide the --from flag with the address directly. - "#))?; + "# + ) + })?; Ok(WalletSigner::Local(local.with_chain_id(chain_id))) } From 327d7b29be085e06f1108758400e42ee67c6b87d Mon Sep 17 00:00:00 2001 From: Enrique Ortiz Date: Thu, 4 May 2023 11:20:52 -0400 Subject: [PATCH 6/7] chore: remove dir check from test --- cli/src/opts/wallet/mod.rs | 3 --- 1 file changed, 3 deletions(-) diff --git a/cli/src/opts/wallet/mod.rs b/cli/src/opts/wallet/mod.rs index 9fe0baa392184..11c145ac7291e 100644 --- a/cli/src/opts/wallet/mod.rs +++ b/cli/src/opts/wallet/mod.rs @@ -542,9 +542,6 @@ mod tests { "--from", "560d246fcddc9ea98a8b032c9a2f474efb493c28", ]); - let file = wallet.find_keystore_file(&keystore).unwrap(); - assert_eq!(file, keystore_file); - let file = wallet.find_keystore_file(&keystore_file).unwrap(); assert_eq!(file, keystore_file); } From 7da515ee9756a8c46b6656bbec910b25980e9a0b Mon Sep 17 00:00:00 2001 From: Enrique Ortiz Date: Sat, 6 May 2023 10:18:50 -0400 Subject: [PATCH 7/7] chore: escape trailing newlines --- cli/src/cmd/cast/send.rs | 5 ++--- cli/src/opts/wallet/mod.rs | 14 ++++++-------- 2 files changed, 8 insertions(+), 11 deletions(-) diff --git a/cli/src/cmd/cast/send.rs b/cli/src/cmd/cast/send.rs index a709accf60037..6ba5eea9b3cdd 100644 --- a/cli/src/cmd/cast/send.rs +++ b/cli/src/cmd/cast/send.rs @@ -143,12 +143,11 @@ impl SendTxArgs { if let Some(specified_from) = eth.wallet.from { if specified_from != from { eyre::bail!( - r#" + "\ The specified sender via CLI/env vars does not match the sender configured via the hardware wallet's HD Path. Please use the `--hd-path ` parameter to specify the BIP32 Path which -corresponds to the sender, or let foundry automatically detect it by not specifying any sender address. -"# +corresponds to the sender, or let foundry automatically detect it by not specifying any sender address." ) } } diff --git a/cli/src/opts/wallet/mod.rs b/cli/src/opts/wallet/mod.rs index 11c145ac7291e..e74a2cf6330c1 100644 --- a/cli/src/opts/wallet/mod.rs +++ b/cli/src/opts/wallet/mod.rs @@ -202,10 +202,9 @@ impl Wallet { None => LedgerHDPath::LedgerLive(self.mnemonic_index as usize), }; let ledger = Ledger::new(derivation, chain_id).await.wrap_err_with(|| { - r#" + "\ Could not connect to Ledger device. -Make sure it's connected and unlocked, with no other desktop wallet apps open. -"# +Make sure it's connected and unlocked, with no other desktop wallet apps open." })?; Ok(WalletSigner::Ledger(ledger)) @@ -217,9 +216,9 @@ Make sure it's connected and unlocked, with no other desktop wallet apps open. // cached to ~/.ethers-rs/trezor/cache/trezor.session let trezor = Trezor::new(derivation, chain_id, None).await.wrap_err_with(|| { - r#" + "\ Could not connect to Trezor device. -Make sure it's connected and unlocked, with no other conflicting desktop wallet apps open."# +Make sure it's connected and unlocked, with no other conflicting desktop wallet apps open." })?; Ok(WalletSigner::Trezor(trezor)) @@ -241,15 +240,14 @@ Make sure it's connected and unlocked, with no other conflicting desktop wallet let local = maybe_local.ok_or_else(|| { eyre::eyre!( - r#" + "\ Error accessing local wallet. Did you set a private key, mnemonic or keystore? Run `cast send --help` or `forge create --help` and use the corresponding CLI flag to set your key via: --private-key, --mnemonic-path, --aws, --interactive, --trezor or --ledger. Alternatively, if you're using a local node with unlocked accounts, use the --unlocked flag and either set the `ETH_FROM` environment variable to the address -of the unlocked account you want to use, or provide the --from flag with the address directly. - "# +of the unlocked account you want to use, or provide the --from flag with the address directly." ) })?;