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.

Sign up for GitHub

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

Jump to bottom

feat(cast): wallet new - enable default keystore #9201

Open
wants to merge 6 commits into
base: master
Choose a base branch
from
Open

feat(cast): wallet new - enable default keystore #9201

Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
f28f6af
feat: use default keystore flag
kien6034 Oct 29, 2024
678d906
bet
kien6034 Oct 31, 2024
34461b8
refactor: wallet new function
kien6034 Oct 31, 2024
feb9a5c
Merge branch 'master' into feat/cast-wallet-default-keystore-path
kien6034 Oct 31, 2024
03bab96
merge master
kien6034 Nov 6, 2024
5806c3c
feat: implicitly defer default keystore
kien6034 Nov 6, 2024
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
View file
Open in desktop

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
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -97,6 +97,7 @@ foundry-test-utils.workspace = true

async-trait.workspace = true
divan.workspace = true
dirs = "5"

[features]
default = ["rustls", "jemalloc"]
Expand Down
135 changes: 77 additions & 58 deletions crates/cast/bin/cmd/wallet/mod.rs
View file
Open in desktop
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).
#[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,78 +223,93 @@ 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 };
if let Some(path) = path {
let path = match dunce::canonicalize(path.clone()) {
Ok(path) => path,
// If the path doesn't exist, it will fail to be canonicalized,
// so we attach more context to the error message.

// Determine the path
let path = if let Some(path) = path {
match dunce::canonicalize(path.clone()) {
Ok(path) => {
if !path.is_dir() {
// we require path to be an existing directory
eyre::bail!("`{}` is not a directory", path.display());
}
Some(path)
}
Err(e) => {
eyre::bail!("If you specified a directory, please make sure it exists, or create it before running `cast wallet new <DIR>`.\n{path} is not a directory.\nError: {}", e);
}
};
if !path.is_dir() {
// we require path to be an existing directory
eyre::bail!("`{}` is not a directory", path.display());
}
} 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)?;
Some(path)
} else {
None
};

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()),
}
));
match path {
Some(path) => {
let password = if let Some(password) = unsafe_password {
password
} else {
sh_println!(
"Created new encrypted keystore file: {}",
path.join(uuid).display()
// 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,
)?;
sh_println!("Address: {}", wallet.address().to_checksum(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))?;
}
}
}

if let Some(json) = json_values.as_ref() {
sh_println!("{}", serde_json::to_string_pretty(json)?)?;
}
} else {
for _ in 0..number {
let wallet = PrivateKeySigner::random_with(&mut rng);

if let Some(json) = json_values.as_mut() {
json.push(json!({
"address": wallet.address().to_checksum(None),
"private_key": format!("0x{}", hex::encode(wallet.credential().to_bytes())),
}))
} else {
sh_println!("Successfully created new keypair.")?;
sh_println!("Address: {}", wallet.address().to_checksum(None))?;
sh_println!(
"Private key: 0x{}",
hex::encode(wallet.credential().to_bytes())
)?;
if let Some(json) = json_values.as_ref() {
sh_println!("{}", serde_json::to_string_pretty(json)?)?;
}
}
None => {
for _ in 0..number {
let wallet = PrivateKeySigner::random_with(&mut rng);

if let Some(json) = json_values.as_mut() {
json.push(json!({
"address": wallet.address().to_checksum(None),
"private_key": format!("0x{}", hex::encode(wallet.credential().to_bytes())),
}))
} else {
sh_println!("Successfully created new keypair.")?;
sh_println!("Address: {}", wallet.address().to_checksum(None))?;
sh_println!(
"Private key: 0x{}",
hex::encode(wallet.credential().to_bytes())
)?;
}
}

if let Some(json) = json_values.as_ref() {
sh_println!("{}", serde_json::to_string_pretty(json)?)?;
if let Some(json) = json_values.as_ref() {
sh_println!("{}", serde_json::to_string_pretty(json)?)?;
}
}
}
}
Expand Down
29 changes: 29 additions & 0 deletions crates/cast/tests/cli/main.rs
View file
Open in desktop
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