Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(cast wallet list) issue #6958: Include HW wallets in cast wallet ls #7123

Merged
merged 7 commits into from
Feb 22, 2024
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
73 changes: 73 additions & 0 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

147 changes: 147 additions & 0 deletions crates/cast/bin/cmd/wallet/list.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,147 @@
use clap::Parser;
use eyre::Result;

use foundry_common::{fs, types::ToAlloy};
use foundry_config::Config;
use foundry_wallets::{multi_wallet::MultiWalletOptsBuilder, WalletSigner};

/// CLI arguments for `cast wallet list`.
#[derive(Clone, Debug, Parser)]
pub struct ListArgs {
/// List all the accounts in the keystore directory.
/// Default keystore directory is used if no path provided.
#[clap(long, default_missing_value = "", num_args(0..=1), help_heading = "List local accounts")]
dir: Option<String>,

/// List accounts from a Ledger hardware wallet.
#[clap(long, short, help_heading = "List Ledger hardware wallet accounts")]
ledger: bool,

/// List accounts from a Trezor hardware wallet.
#[clap(long, short, help_heading = "List Trezor hardware wallet accounts")]
trezor: bool,

/// List accounts from AWS KMS.
#[clap(long, help_heading = "List AWS KMS account")]
aws: bool,

/// List all configured accounts.
#[clap(long, help_heading = "List all accounts")]
Copy link
Member

@DaniPopes DaniPopes Feb 21, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Remove help_headings, these don't need sections for each option

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

removed in 38fc356

all: bool,
}

impl ListArgs {
pub async fn run(self) -> Result<()> {
// list local accounts as files in keystore dir, no need to unlock / provide password
if self.dir.is_some() || self.all || (!self.ledger && !self.trezor && !self.aws) {
self.list_local_senders().await?;
}

// Create options for multi wallet - ledger, trezor and AWS
let list_opts = MultiWalletOptsBuilder::default()
.ledger(self.ledger || self.all)
.mnemonic_indexes(Some(vec![0]))
.trezor(self.trezor || self.all)
.aws(self.aws || self.all)
.interactives(0)
.build()
.expect("build multi wallet");

// max number of senders to be shown for ledger and trezor signers
let max_senders = 3;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should this be an argument?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yep, made this a wallet list argument, grouped with hardware wallets and all option 38fc356


// List ledger accounts
match list_opts.ledgers().await {
Ok(signers) => {
self.list_senders(signers.unwrap_or_default(), max_senders, "Ledger").await?
}
Err(e) => {
if !self.all {
println!("{}", e)
}
}
}

// List Trezor accounts
match list_opts.trezors().await {
Ok(signers) => {
self.list_senders(signers.unwrap_or_default(), max_senders, "Trezor").await?
}
Err(e) => {
if !self.all {
println!("{}", e)
}
}
}

// List AWS accounts
match list_opts.aws_signers().await {
Ok(signers) => {
self.list_senders(signers.unwrap_or_default(), max_senders, "AWS").await?
}
Err(e) => {
if !self.all {
println!("{}", e)
}
}
}

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

would be nice if we could reduce duplication here via macro/fn

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yep, good point, changed in c4ce461

Ok(())
}

async fn list_local_senders(&self) -> Result<()> {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

doesn't need async

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

updated in 38fc356

let keystore_path = self.dir.clone().unwrap_or_default();
let keystore_dir = if keystore_path.is_empty() {
// Create the keystore default directory if it doesn't exist
let default_dir = Config::foundry_keystores_dir().unwrap();
fs::create_dir_all(&default_dir)?;
default_dir
} else {
dunce::canonicalize(keystore_path)?
};

match std::fs::read_dir(keystore_dir) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

remove match, use ? as it already exists

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

removed in 38fc356

Ok(entries) => {
entries.flatten().for_each(|entry| {
let path = entry.path();
if path.is_file() && path.extension().is_none() {
if let Some(file_name) = path.file_name() {
if let Some(name) = file_name.to_str() {
println!("{} (Local)", name);
}
}
}
});
}
Err(e) => {
if !self.all {
println!("{}", e)
}
}
}

Ok(())
}

async fn list_senders(
&self,
signers: Vec<WalletSigner>,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
signers: Vec<WalletSigner>,
signers: &[WalletSigner],

Copy link
Collaborator Author

@grandizzy grandizzy Feb 21, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

not sure exactly what to change here, can you pls elaborate? changed in 4ecb726 but not needed anymore as moved list_senders logic in macro 6474ca3

max_senders: usize,
label: &str,
) -> eyre::Result<()> {
for signer in signers.iter() {
match signer.available_senders(max_senders).await {
Ok(senders) => {
senders.iter().for_each(|sender| println!("{} ({})", sender.to_alloy(), label));
}
Err(e) => {
if !self.all {
println!("{}", e)
}
}
}
}

Ok(())
}
}
42 changes: 6 additions & 36 deletions crates/cast/bin/cmd/wallet/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,9 @@ use yansi::Paint;
pub mod vanity;
use vanity::VanityArgs;

pub mod list;
use list::ListArgs;

/// CLI arguments for `cast wallet`.
#[derive(Debug, Parser)]
pub enum WalletSubcommands {
Expand Down Expand Up @@ -137,7 +140,7 @@ pub enum WalletSubcommands {
},
/// List all the accounts in the keystore default directory
#[clap(visible_alias = "ls")]
List,
List(ListArgs),

/// Derives private key from mnemonic
#[clap(name = "derive-private-key", visible_aliases = &["--derive-private-key"])]
Expand Down Expand Up @@ -331,41 +334,8 @@ flag to set your key via:
);
println!("{}", Paint::green(success_message));
}
WalletSubcommands::List => {
let default_keystore_dir = Config::foundry_keystores_dir()
.ok_or_else(|| eyre::eyre!("Could not find the default keystore directory."))?;
// Create the keystore directory if it doesn't exist
fs::create_dir_all(&default_keystore_dir)?;
// List all files in keystore directory
let keystore_files: Result<Vec<_>, eyre::Report> =
std::fs::read_dir(&default_keystore_dir)
.wrap_err("Failed to read the directory")?
.filter_map(|entry| match entry {
Ok(entry) => {
let path = entry.path();
if path.is_file() && path.extension().is_none() {
Some(Ok(path))
} else {
None
}
}
Err(e) => Some(Err(e.into())),
})
.collect::<Result<Vec<_>, eyre::Report>>();
// Print the names of the keystore files
match keystore_files {
Ok(files) => {
// Print the names of the keystore files
for file in files {
if let Some(file_name) = file.file_name() {
if let Some(name) = file_name.to_str() {
println!("{}", name);
}
}
}
}
Err(e) => return Err(e),
}
WalletSubcommands::List(cmd) => {
cmd.run().await?;
}
WalletSubcommands::DerivePrivateKey { mnemonic, mnemonic_index } => {
let phrase = Mnemonic::<English>::new_from_phrase(mnemonic.as_str())?.to_phrase();
Expand Down
23 changes: 22 additions & 1 deletion crates/cast/tests/cli/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@

use foundry_common::rpc::{next_http_rpc_endpoint, next_ws_rpc_endpoint};
use foundry_test_utils::{casttest, util::OutputExt};
use std::{io::Write, path::Path};
use std::{fs, io::Write, path::Path};

// tests `--help` is printed to std out
casttest!(print_help, |_prj, cmd| {
Expand Down Expand Up @@ -131,6 +131,27 @@ casttest!(wallet_sign_typed_data_file, |_prj, cmd| {
assert_eq!(output.trim(), "0x06c18bdc8163219fddc9afaf5a0550e381326474bb757c86dc32317040cf384e07a2c72ce66c1a0626b6750ca9b6c035bf6f03e7ed67ae2d1134171e9085c0b51b");
});

// tests that `cast wallet list` outputs the local accounts
casttest!(wallet_list_local_accounts, |prj, cmd| {
let keystore_path = prj.root().join("keystore");
fs::create_dir_all(keystore_path).unwrap();
cmd.set_current_dir(prj.root());

// empty results
cmd.cast_fuse().args(["wallet", "list", "--dir", "keystore"]);
let list_output = cmd.stdout_lossy();
assert!(list_output.is_empty());

// create 10 wallets
cmd.cast_fuse().args(["wallet", "new", "keystore", "-n", "10", "--unsafe-password", "test"]);
cmd.stdout_lossy();

// test list new wallet
cmd.cast_fuse().args(["wallet", "list", "--dir", "keystore"]);
let list_output = cmd.stdout_lossy();
assert_eq!(list_output.matches('\n').count(), 10);
});

// tests that `cast estimate` is working correctly.
casttest!(estimate_function_gas, |_prj, cmd| {
let eth_rpc_url = next_http_rpc_endpoint();
Expand Down
1 change: 1 addition & 0 deletions crates/wallets/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ foundry-common.workspace = true

async-trait = "0.1"
clap = { version = "4", features = ["derive", "env", "unicode", "wrap_help"] }
derive_builder = "0.20.0"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm personally okay with this

any objections @DaniPopes ?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

that's fine

eyre.workspace = true
hex = { workspace = true, features = ["serde"] }
itertools.workspace = true
Expand Down
Loading
Loading