Skip to content

feat(wallet): enhance keystore listing with address suffix #11977

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

Draft
wants to merge 1 commit into
base: master
Choose a base branch
from
+105 −17
Draft

feat(wallet): enhance keystore listing with address suffix #11977

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
75 changes: 62 additions & 13 deletions crates/cast/src/cmd/wallet/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ use eyre::{Context, Result};
use foundry_cli::{opts::RpcOpts, utils, utils::LoadConfig};
use foundry_common::{fs, sh_println, shell};
use foundry_config::Config;
use foundry_wallets::{RawWalletOpts, WalletOpts, WalletSigner};
use foundry_wallets::{RawWalletOpts, WalletOpts, WalletSigner, utils::find_keystore_by_name};
use rand_08::thread_rng;
use serde_json::json;
use std::path::Path;
Expand Down Expand Up @@ -337,25 +337,37 @@ impl WalletSubcommands {
password.clone(),
account_name_ref.as_deref(),
)?;

// Rename the keystore file to include the address
let identifier = account_name_ref.as_deref().unwrap_or(&uuid);
let old_path = path.join(identifier);
let path_with_address = path.join(format!(
"{}_{}",
identifier,
wallet.address().to_checksum(None)
));

if old_path.exists() && old_path != path_with_address {
std::fs::rename(&old_path, &path_with_address)?;
}
Copy link
Contributor Author

Choose a reason for hiding this comment

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

The renaming of the keystore file is a bit dirty due to eth_keystore lib which directly creates the file before returning the wallet necessary to derive the address. 🫤

Ideally the lib should be modified, or upstreamed to foundry to solve this in an elegant way.


if let Some(json) = json_values.as_mut() {
json.push(if shell::verbosity() > 0 {
json!({
"address": wallet.address().to_checksum(None),
"public_key": format!("0x{}", hex::encode(wallet.public_key())),
"path": format!("{}", path.join(identifier).display()),
"path": format!("{}", path_with_address.display()),
})
} else {
json!({
"address": wallet.address().to_checksum(None),
"path": format!("{}", path.join(identifier).display()),
"path": format!("{}", path_with_address.display()),
})
});
} else {
sh_println!(
"Created new encrypted keystore file: {}",
path.join(identifier).display()
path_with_address.display()
)?;
sh_println!("Address: {}", wallet.address().to_checksum(None))?;
if shell::verbosity() > 0 {
Expand Down Expand Up @@ -621,10 +633,9 @@ impl WalletSubcommands {

fs::create_dir_all(&dir)?;

// check if account exists already
let keystore_path = Path::new(&dir).join(&account_name);
if keystore_path.exists() {
eyre::bail!("Keystore file already exists at {}", keystore_path.display());
// Check if account exists already (using shared utility)
if find_keystore_by_name(&dir, &account_name).is_some() {
eyre::bail!("Keystore file already exists for account '{}'", account_name);
}

// get wallet
Expand Down Expand Up @@ -654,13 +665,23 @@ flag to set your key via:

let mut rng = thread_rng();
let (wallet, _) = PrivateKeySigner::encrypt_keystore(
dir,
dir.clone(),
&mut rng,
private_key,
password,
Some(&account_name),
)?;
let address = wallet.address();

// Rename the file to include the address in the new format
let old_path = dir.join(&account_name);
let path_with_address =
dir.join(format!("{}_{}", account_name, address.to_checksum(None)));

if old_path.exists() && old_path != path_with_address {
std::fs::rename(&old_path, &path_with_address)?;
}

let success_message = format!(
"`{}` keystore was saved successfully. Address: {:?}",
&account_name, address,
Expand All @@ -679,7 +700,13 @@ flag to set your key via:
})?
};

let keystore_path = Path::new(&dir).join(&name);
// Use shared utility for keystore lookup
let keystore_path = if let Some(found_path) = find_keystore_by_name(&dir, &name) {
found_path
} else {
dir.join(&name) // Fallback to direct path for error message
};

if !keystore_path.exists() {
eyre::bail!("Keystore file does not exist at {}", keystore_path.display());
}
Expand Down Expand Up @@ -752,7 +779,12 @@ flag to set your key via:
})?
};

let keypath = dir.join(&account_name);
// Use shared utility for keystore lookup
let keypath = if let Some(found_path) = find_keystore_by_name(&dir, &account_name) {
found_path
} else {
dir.join(&account_name) // Fallback to direct path for error message
};

if !keypath.exists() {
eyre::bail!("Keystore file does not exist at {}", keypath.display());
Expand Down Expand Up @@ -789,7 +821,12 @@ flag to set your key via:
})?
};

let keypath = dir.join(&account_name);
// Use shared utility for keystore lookup
let keypath = if let Some(found_path) = find_keystore_by_name(&dir, &account_name) {
found_path
} else {
dir.join(&account_name) // Fallback to direct path for error message
};

if !keypath.exists() {
eyre::bail!("Keystore file does not exist at {}", keypath.display());
Expand Down Expand Up @@ -821,13 +858,25 @@ flag to set your key via:
let private_key = wallet.credential().to_bytes();
let mut rng = thread_rng();
let (wallet, _) = PrivateKeySigner::encrypt_keystore(
dir,
dir.clone(),
&mut rng,
private_key,
new_password,
Some(&account_name),
)?;

// Remove the old keystore file
std::fs::remove_file(&keypath)?;

// Rename the new file to include the address
let old_path = dir.join(&account_name);
let path_with_address =
dir.join(format!("{}_{}", account_name, wallet.address().to_checksum(None)));

if old_path.exists() && old_path != path_with_address {
std::fs::rename(&old_path, &path_with_address)?;
}

let success_message = format!(
"Password for keystore `{}` was changed successfully. Address: {:?}",
&account_name,
Expand Down
47 changes: 43 additions & 4 deletions crates/wallets/src/utils.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
use crate::{PendingSigner, WalletSigner, error::PrivateKeyError};
use alloy_primitives::{B256, hex::FromHex};
use alloy_primitives::{Address, B256, hex::FromHex};
use alloy_signer_ledger::HDPath as LedgerHDPath;
use alloy_signer_local::PrivateKeySigner;
use alloy_signer_trezor::HDPath as TrezorHDPath;
Expand Down Expand Up @@ -93,9 +93,48 @@ pub fn maybe_get_keystore_path(
) -> Result<Option<PathBuf>> {
let default_keystore_dir = Config::foundry_keystores_dir()
.ok_or_else(|| eyre::eyre!("Could not find the default keystore directory."))?;
Ok(maybe_path
.map(PathBuf::from)
.or_else(|| maybe_name.map(|name| default_keystore_dir.join(name))))

if let Some(path) = maybe_path {
return Ok(Some(PathBuf::from(path)));
}

if let Some(name) = maybe_name {
if let Some(found_path) = find_keystore_by_name(&default_keystore_dir, name) {
return Ok(Some(found_path));
}

// Return the direct path even if it doesn't exist (for better error messages)
return Ok(Some(default_keystore_dir.join(name)));
}

Ok(None)
}

/// Finds a keystore file by account name, supporting both old and new filename formats
/// Returns the path if found, or None if not found
pub fn find_keystore_by_name(
keystore_dir: &std::path::Path,
account_name: &str,
) -> Option<std::path::PathBuf> {
// Try exact match first (old format or exact filename)
let direct_path = keystore_dir.join(account_name);
if direct_path.exists() {
return Some(direct_path);
}

// Try finding with address suffix (new format: account_name_0x<address>)
if let Ok(entries) = std::fs::read_dir(keystore_dir) {
let search_prefix = format!("{account_name}_");
for entry in entries.flatten() {
if let Some(file_name) = entry.file_name().to_str()
&& file_name.starts_with(&search_prefix)
&& Address::parse_checksummed(&file_name[search_prefix.len()..], None).is_ok()
{
return Some(entry.path());
}
}
}
None
}

/// Creates keystore signer from given parameters.
Expand Down
Loading