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 new - enable default keystore #9201

Open
wants to merge 6 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 1 commit
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
1 change: 1 addition & 0 deletions Cargo.lock

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

1 change: 1 addition & 0 deletions crates/cast/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -70,6 +70,7 @@ foundry-cli.workspace = true
clap = { version = "4", features = ["derive", "env", "unicode", "wrap_help"] }
clap_complete = "4"
clap_complete_fig = "4"
dirs = "5"
zerosnacks marked this conversation as resolved.
Show resolved Hide resolved
comfy-table.workspace = true
dunce.workspace = true
indicatif = "0.17"
Expand Down
42 changes: 40 additions & 2 deletions crates/cast/bin/cmd/wallet/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ pub enum WalletSubcommands {
/// Password for the JSON keystore in cleartext.
///
/// This is UNSAFE to use and we recommend using the --password.
#[arg(long, requires = "path", env = "CAST_PASSWORD", value_name = "PASSWORD")]
#[arg(long, env = "CAST_PASSWORD", value_name = "PASSWORD")]
unsafe_password: Option<String>,

/// Number of wallets to generate.
Expand All @@ -53,6 +53,10 @@ pub enum WalletSubcommands {
/// Output generated wallets as JSON.
#[arg(long, short, default_value = "false")]
json: bool,

/// Use default keystore location (~/.foundry/keystores)
zerosnacks marked this conversation as resolved.
Show resolved Hide resolved
#[arg(long, short, conflicts_with = "path", default_value = "false")]
Copy link
Collaborator

Choose a reason for hiding this comment

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

If I get it right, the default keystore is used if no path provided but only with unsafe_password option (any reason why this is not available for password too?).
If so, do we want to make this explicit by adding the new default_keystore arg or can we just use default keystore if path is None?
That is the

} else if default_keystore {

below to be

} else if unsafe_password {

@zerosnacks wdyt?

Copy link
Member

Choose a reason for hiding this comment

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

the removal of requires = "path should allow it to use the default path, not place a conditional on it

I would prefer making the default path implicit as well over having a new flag be introduced

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@zerosnacks @grandizzy tks for reviewing

so as my understanding, we should use default_key_store when path is none + unsafe password or password is provided.

if password fields not provided, we just printout the keys for user?

Copy link
Member

Choose a reason for hiding this comment

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

Correct, a keystore implies having a password whereas a keypair does not

default_keystore: bool,
},

/// Generates a random BIP39 mnemonic phrase
Expand Down Expand Up @@ -219,7 +223,7 @@ pub enum WalletSubcommands {
impl WalletSubcommands {
pub async fn run(self) -> Result<()> {
match self {
Self::New { path, unsafe_password, number, json, .. } => {
Self::New { path, unsafe_password, number, json, default_keystore, .. } => {
let mut rng = thread_rng();

let mut json_values = if json { Some(vec![]) } else { None };
Expand Down Expand Up @@ -270,6 +274,40 @@ impl WalletSubcommands {
if let Some(json) = json_values.as_ref() {
sh_println!("{}", serde_json::to_string_pretty(json)?)?;
}
} else if default_keystore {
let path = Config::foundry_keystores_dir().ok_or_else(|| {
eyre::eyre!("Could not find the default keystore directory.")
})?;
fs::create_dir_all(&path)?;

let password = if let Some(password) = unsafe_password {
password
} else {
// if no --unsafe-password was provided read via stdin
rpassword::prompt_password("Enter secret: ")?
};

for _ in 0..number {
let (wallet, uuid) = PrivateKeySigner::new_keystore(
&path,
&mut rng,
password.clone(),
None,
)?;

if let Some(json) = json_values.as_mut() {
json.push(json!({
"address": wallet.address().to_checksum(None),
"path": format!("{}", path.join(uuid).display()),
}));
} else {
sh_println!(
"Created new encrypted keystore file: {}",
path.join(uuid).display()
)?;
sh_println!("Address: {}", wallet.address().to_checksum(None))?;
zerosnacks marked this conversation as resolved.
Show resolved Hide resolved
}
}
} else {
for _ in 0..number {
let wallet = PrivateKeySigner::random_with(&mut rng);
Expand Down
29 changes: 29 additions & 0 deletions crates/cast/tests/cli/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -125,6 +125,35 @@ Created new encrypted keystore file: [..]
]);
});

// tests that we can create a new wallet with default keystore location
casttest!(new_wallet_default_keystore, |_prj, cmd| {
cmd.args(["wallet", "new", "--unsafe-password", "test", "--default-keystore"])
.assert_success()
.stdout_eq(str![[r#"
Created new encrypted keystore file: [..]
[ADDRESS]

"#]]);

// Verify the default keystore directory was created
let keystore_path = dirs::home_dir().unwrap().join(".foundry").join("keystores");
assert!(keystore_path.exists());
assert!(keystore_path.is_dir());
});

// tests that we can outputting multiple keys without a keystore path
casttest!(new_wallet_multiple_keys, |_prj, cmd| {
cmd.args(["wallet", "new", "-n", "2"]).assert_success().stdout_eq(str![[r#"
Successfully created new keypair.
Address: [..]
Private key: [..]
Successfully created new keypair.
Address: [..]
Private key: [..]

"#]]);
});

// tests that we can get the address of a keystore file
casttest!(wallet_address_keystore_with_password_file, |_prj, cmd| {
let keystore_dir = Path::new(env!("CARGO_MANIFEST_DIR")).join("tests/fixtures/keystore");
Expand Down
Loading