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

Fix minor wallet issues #220

Merged
merged 21 commits into from
Oct 30, 2024
Merged
Show file tree
Hide file tree
Changes from 16 commits
Commits
Show all changes
21 commits
Select commit Hold shift + click to select a range
6d63e8a
ignore vscode ide/settings
zees-dev Oct 29, 2024
657cf84
added rust toolchain
zees-dev Oct 29, 2024
3fa1b27
fix new_at_index not printing checksum address
zees-dev Oct 29, 2024
35e29d0
wallet existance also checks content; handles the case if it's not a …
zees-dev Oct 29, 2024
9792e74
removed redundant helper functions; replaced with tempfile dep instead
zees-dev Oct 29, 2024
46bfbac
updated create_wallet test helper func to take optional file content
zees-dev Oct 29, 2024
11e0b50
existing unit tests refactored to remove nesting
zees-dev Oct 29, 2024
8fadf3a
new unit tests for ensure_no_wallet_exists
zees-dev Oct 29, 2024
9632374
updated CI toolchain to match rust-toolchain.toml
zees-dev Oct 29, 2024
ab3e98d
github actions checkoutv4 and nodev4
zees-dev Oct 29, 2024
6e6af2e
ci update to use semver rust version
zees-dev Oct 29, 2024
8f9c6d0
fixed CI nightly version for code cov
zees-dev Oct 29, 2024
eaf5301
reverting CI and toolchain to use original nightly version
zees-dev Oct 29, 2024
78511f8
fixed cargo clippy errors
zees-dev Oct 29, 2024
762405c
Revert "reverting CI and toolchain to use original nightly version"
zees-dev Oct 29, 2024
3ac5970
ci: install nightly version for code-cov
kayagokalp Oct 29, 2024
6b82bc4
updated rust toolchain to stable 1.82.0; nightly to nightly-2024-10-2…
zees-dev Oct 29, 2024
0306d83
create_wallet added description
zees-dev Oct 29, 2024
515cb7a
Merge branch 'master' into fix/minor-wallet-issues
zees-dev Oct 29, 2024
f8124e8
create_wallet updated param name; content -> data; updated description
zees-dev Oct 29, 2024
9abc8ad
create_wallet -> serialize_wallet_to_file; using an enum with variant…
zees-dev Oct 29, 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
9 changes: 5 additions & 4 deletions .github/workflows/ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,8 @@ env:
CARGO_TERM_COLOR: always
RUSTFLAGS: -D warnings
REGISTRY: ghcr.io
RUST_VERSION_COV: nightly-2024-06-05
RUST_VERSION: 1.80.0
NIGHTLY_RUST_VERSION: nightly-2024-07-21
zees-dev marked this conversation as resolved.
Show resolved Hide resolved

jobs:
cancel-previous-runs:
Expand All @@ -29,17 +30,17 @@ jobs:
permissions: # Write access to push changes to pages
contents: write
steps:
- uses: actions/checkout@v3
- uses: actions/checkout@v4
- name: Install latest Rust
uses: dtolnay/rust-toolchain@master
with:
toolchain: ${{ env.RUST_VERSION_COV }}
toolchain: ${{ env.NIGHTLY_RUST_VERSION }}

- name: Install cargo-llvm-codecov
uses: taiki-e/install-action@cargo-llvm-cov

- name: Code coverage report
run: cargo +${{ env.RUST_VERSION_COV }} llvm-cov --all-features --lcov --branch --output-path lcov.info
run: cargo +${{ env.NIGHTLY_RUST_VERSION }} llvm-cov --all-features --lcov --branch --output-path lcov.info

- name: Setup LCOV
uses: hrishikesh-kadam/setup-lcov@v1
Expand Down
4 changes: 2 additions & 2 deletions .github/workflows/markdown-lint.yml
Original file line number Diff line number Diff line change
Expand Up @@ -13,8 +13,8 @@ jobs:
name: Markdown Lint
runs-on: ubuntu-latest
steps:
- uses: actions/checkout@v3
- uses: actions/setup-node@v3
- uses: actions/checkout@v4
- uses: actions/setup-node@v4
with:
node-version: 18
- run: |
Expand Down
5 changes: 5 additions & 0 deletions .gitignore
Original file line number Diff line number Diff line change
@@ -1 +1,6 @@

# IDE
.vscode

# Output
/target
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.

3 changes: 3 additions & 0 deletions Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -39,3 +39,6 @@ path = "src/lib.rs"
[[bin]]
name = "forc-wallet"
path = "src/main.rs"

[dev-dependencies]
tempfile = "3.13.0"
3 changes: 3 additions & 0 deletions rust-toolchain.toml
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
[toolchain]
profile = "default" # include rustfmt, clippy
channel = "1.80.0"
zees-dev marked this conversation as resolved.
Show resolved Hide resolved
11 changes: 4 additions & 7 deletions src/account.rs
Original file line number Diff line number Diff line change
Expand Up @@ -453,18 +453,15 @@ pub(crate) fn derive_account(
Ok(derive_account_unlocked(wallet_path, account_ix, password)?.lock())
}

fn new_at_index(
keystore: &EthKeystore,
wallet_path: &Path,
account_ix: usize,
) -> Result<Bech32Address> {
fn new_at_index(keystore: &EthKeystore, wallet_path: &Path, account_ix: usize) -> Result<String> {
let prompt = format!("Please enter your wallet password to derive account {account_ix}: ");
let password = rpassword::prompt_password(prompt)?;
let account = derive_account(wallet_path, account_ix, &password)?;
let account_addr = account.address();
cache_address(&keystore.crypto.ciphertext, account_ix, account_addr)?;
println!("Wallet address: {account_addr}");
Ok(account_addr.clone())
let checksum_addr = checksum_encode(&Address::from(account_addr).to_string())?;
println!("Wallet address: {checksum_addr}");
Ok(checksum_addr)
}

pub fn new_at_index_cli(wallet_path: &Path, account_ix: usize) -> Result<()> {
Expand Down
200 changes: 103 additions & 97 deletions src/utils.rs
Original file line number Diff line number Diff line change
Expand Up @@ -142,13 +142,21 @@ pub(crate) fn ensure_no_wallet_exists(
force: bool,
mut reader: impl BufRead,
) -> Result<()> {
if wallet_path.exists() {
let remove_wallet = || {
if wallet_path.is_dir() {
fs::remove_dir_all(wallet_path).unwrap();
} else {
fs::remove_file(wallet_path).unwrap();
}
};

if wallet_path.exists() && fs::metadata(wallet_path)?.len() > 0 {
if force {
println_warning(&format!(
"Because the `--force` argument was supplied, the wallet at {} will be removed.",
wallet_path.display(),
));
fs::remove_file(wallet_path).unwrap();
remove_wallet();
} else {
println_warning(&format!(
"There is an existing wallet at {}. \
Expand All @@ -158,7 +166,7 @@ pub(crate) fn ensure_no_wallet_exists(
let mut need_replace = String::new();
reader.read_line(&mut need_replace).unwrap();
if need_replace.trim() == "y" {
fs::remove_file(wallet_path).unwrap();
remove_wallet();
} else {
bail!(
"Failed to create a new wallet at {} \
Expand All @@ -174,7 +182,7 @@ pub(crate) fn ensure_no_wallet_exists(
#[cfg(test)]
mod tests {
use super::*;
use crate::utils::test_utils::{with_tmp_dir, TEST_MNEMONIC, TEST_PASSWORD};
use crate::utils::test_utils::{TEST_MNEMONIC, TEST_PASSWORD};
// simulate input
const INPUT_NOP: &[u8; 1] = b"\n";
const INPUT_YES: &[u8; 2] = b"y\n";
Expand All @@ -185,21 +193,23 @@ mod tests {
fs::remove_file(wallet_path).unwrap();
}
}
fn create_wallet(wallet_path: &Path) {
fn create_wallet(wallet_path: &Path, content: Option<&str>) {
zees-dev marked this conversation as resolved.
Show resolved Hide resolved
if !wallet_path.exists() {
fs::File::create(wallet_path).unwrap();
}
if let Some(content) = content {
fs::write(wallet_path, content).unwrap();
}
}

#[test]
fn handle_absolute_path_argument() {
with_tmp_dir(|tmp_dir| {
let tmp_dir_abs = tmp_dir.canonicalize().unwrap();
let wallet_path = tmp_dir_abs.join("wallet.json");
write_wallet_from_mnemonic_and_password(&wallet_path, TEST_MNEMONIC, TEST_PASSWORD)
.unwrap();
load_wallet(&wallet_path).unwrap();
})
let tmp_dir = tempfile::TempDir::new().unwrap();
let tmp_dir_abs = tmp_dir.path().canonicalize().unwrap();
let wallet_path = tmp_dir_abs.join("wallet.json");
write_wallet_from_mnemonic_and_password(&wallet_path, TEST_MNEMONIC, TEST_PASSWORD)
.unwrap();
load_wallet(&wallet_path).unwrap();
}

#[test]
Expand All @@ -223,88 +233,104 @@ mod tests {
}
#[test]
fn encrypt_and_save_phrase() {
with_tmp_dir(|tmp_dir| {
let wallet_path = tmp_dir.join("wallet.json");
write_wallet_from_mnemonic_and_password(&wallet_path, TEST_MNEMONIC, TEST_PASSWORD)
.unwrap();
let phrase_recovered = eth_keystore::decrypt_key(wallet_path, TEST_PASSWORD).unwrap();
let phrase = String::from_utf8(phrase_recovered).unwrap();
assert_eq!(phrase, TEST_MNEMONIC)
});
let tmp_dir = tempfile::TempDir::new().unwrap();
let wallet_path = tmp_dir.path().join("wallet.json");
write_wallet_from_mnemonic_and_password(&wallet_path, TEST_MNEMONIC, TEST_PASSWORD)
.unwrap();
let phrase_recovered = eth_keystore::decrypt_key(wallet_path, TEST_PASSWORD).unwrap();
let phrase = String::from_utf8(phrase_recovered).unwrap();
assert_eq!(phrase, TEST_MNEMONIC)
}

#[test]
fn write_wallet() {
with_tmp_dir(|tmp_dir| {
let wallet_path = tmp_dir.join("wallet.json");
write_wallet_from_mnemonic_and_password(&wallet_path, TEST_MNEMONIC, TEST_PASSWORD)
.unwrap();
load_wallet(&wallet_path).unwrap();
})
let tmp_dir = tempfile::TempDir::new().unwrap();
let wallet_path = tmp_dir.path().join("wallet.json");
write_wallet_from_mnemonic_and_password(&wallet_path, TEST_MNEMONIC, TEST_PASSWORD)
.unwrap();
load_wallet(&wallet_path).unwrap();
}

#[test]
#[should_panic]
fn write_wallet_to_existing_file_should_fail() {
with_tmp_dir(|tmp_dir| {
let wallet_path = tmp_dir.join("wallet.json");
write_wallet_from_mnemonic_and_password(&wallet_path, TEST_MNEMONIC, TEST_PASSWORD)
.unwrap();
write_wallet_from_mnemonic_and_password(&wallet_path, TEST_MNEMONIC, TEST_PASSWORD)
.unwrap();
})
let tmp_dir = tempfile::TempDir::new().unwrap();
let wallet_path = tmp_dir.path().join("wallet.json");
write_wallet_from_mnemonic_and_password(&wallet_path, TEST_MNEMONIC, TEST_PASSWORD)
.unwrap();
write_wallet_from_mnemonic_and_password(&wallet_path, TEST_MNEMONIC, TEST_PASSWORD)
.unwrap();
}

#[test]
fn write_wallet_subdir() {
with_tmp_dir(|tmp_dir| {
let wallet_path = tmp_dir.join("path").join("to").join("wallet");
write_wallet_from_mnemonic_and_password(&wallet_path, TEST_MNEMONIC, TEST_PASSWORD)
.unwrap();
load_wallet(&wallet_path).unwrap();
})
let tmp_dir = tempfile::TempDir::new().unwrap();
let wallet_path = tmp_dir.path().join("path").join("to").join("wallet.json");
write_wallet_from_mnemonic_and_password(&wallet_path, TEST_MNEMONIC, TEST_PASSWORD)
.unwrap();
load_wallet(&wallet_path).unwrap();
}

#[test]
fn test_ensure_no_wallet_exists_no_wallet() {
with_tmp_dir(|tmp_dir| {
let wallet_path = tmp_dir.join("wallet.json");
remove_wallet(&wallet_path);
ensure_no_wallet_exists(&wallet_path, false, &INPUT_NOP[..]).unwrap();
});
}

#[test]
#[should_panic]
fn test_ensure_no_wallet_exists_throws_err() {
with_tmp_dir(|tmp_dir| {
let wallet_path = tmp_dir.join("wallet.json");
create_wallet(&wallet_path);
ensure_no_wallet_exists(&wallet_path, false, &INPUT_NO[..]).unwrap();
});
let tmp_dir = tempfile::TempDir::new().unwrap();
let wallet_path = tmp_dir.path().join("wallet.json");
remove_wallet(&wallet_path);
ensure_no_wallet_exists(&wallet_path, false, &INPUT_NOP[..]).unwrap();
}

#[test]
fn test_ensure_no_wallet_exists_exists_wallet() {
// case: wallet path exist without --force and input[yes]
with_tmp_dir(|tmp_dir| {
let wallet_path = tmp_dir.join("wallet.json");
create_wallet(&wallet_path);
ensure_no_wallet_exists(&wallet_path, false, &INPUT_YES[..]).unwrap();
});
let tmp_dir = tempfile::TempDir::new().unwrap();
let wallet_path = tmp_dir.path().join("wallet.json");
create_wallet(&wallet_path, None);
ensure_no_wallet_exists(&wallet_path, false, &INPUT_YES[..]).unwrap();

// case: wallet path exist with --force
with_tmp_dir(|tmp_dir| {
let wallet_path = tmp_dir.join("wallet.json");
create_wallet(&wallet_path);
ensure_no_wallet_exists(&wallet_path, true, &INPUT_NOP[..]).unwrap();
});
// case: wallet path exist without --force and supply a different wallet path
with_tmp_dir(|tmp_dir| {
let wallet_path = tmp_dir.join("wallet.json");
create_wallet(&wallet_path);
let diff_wallet_path = tmp_dir.join("custom-wallet.json");
ensure_no_wallet_exists(&diff_wallet_path, false, &INPUT_NOP[..]).unwrap();
});
let tmp_dir = tempfile::TempDir::new().unwrap();
let wallet_path = tmp_dir.path().join("empty_wallet.json");
create_wallet(&wallet_path, None);

// Empty file should not trigger the replacement prompt
ensure_no_wallet_exists(&wallet_path, false, &INPUT_YES[..]).unwrap();
assert!(wallet_path.exists(), "Empty file should remain untouched");
}

#[test]
fn test_ensure_no_wallet_exists_nonempty_file() {
let tmp_dir = tempfile::TempDir::new().unwrap();
let wallet_path = tmp_dir.path().join("nonempty_wallet.json");

// Create non-empty file
create_wallet(&wallet_path, Some("some wallet content"));

// Test with --force flag
ensure_no_wallet_exists(&wallet_path, true, &INPUT_NO[..]).unwrap();
assert!(
!wallet_path.exists(),
"File should be removed with --force flag"
);

// Test with user confirmation (yes)
create_wallet(&wallet_path, Some("some wallet content"));
ensure_no_wallet_exists(&wallet_path, false, &INPUT_YES[..]).unwrap();
assert!(
!wallet_path.exists(),
"File should be removed after user confirmation"
);

// Test with user rejection (no)
create_wallet(&wallet_path, Some("some wallet content"));
let result = ensure_no_wallet_exists(&wallet_path, false, &INPUT_NO[..]);
assert!(
result.is_err(),
"Should error when user rejects file removal"
);
assert!(
wallet_path.exists(),
"File should remain when user rejects removal"
);
}
}

Expand All @@ -316,35 +342,15 @@ pub(crate) mod test_utils {
pub(crate) const TEST_MNEMONIC: &str = "rapid mechanic escape victory bacon switch soda math embrace frozen novel document wait motor thrive ski addict ripple bid magnet horse merge brisk exile";
pub(crate) const TEST_PASSWORD: &str = "1234";

/// Create a tmp folder and execute the given test function `f`
pub(crate) fn with_tmp_dir<F>(f: F)
where
F: FnOnce(&Path) + panic::UnwindSafe,
{
let tmp_dir_name = format!("forc-wallet-test-{:x}", rand::random::<u64>());
let tmp_dir = user_fuel_dir().join(".tmp").join(tmp_dir_name);
std::fs::create_dir_all(&tmp_dir).unwrap();
let panic = panic::catch_unwind(|| f(&tmp_dir));
std::fs::remove_dir_all(&tmp_dir).unwrap();
if let Err(e) = panic {
panic::resume_unwind(e);
}
}

/// Saves a default test mnemonic to the disk
pub(crate) fn save_dummy_wallet_file(wallet_path: &Path) {
write_wallet_from_mnemonic_and_password(wallet_path, TEST_MNEMONIC, TEST_PASSWORD).unwrap();
}

/// The same as `with_tmp_dir`, but also provides a test wallet.
/// Creates temp dir with a temp/test wallet.
pub(crate) fn with_tmp_dir_and_wallet<F>(f: F)
where
F: FnOnce(&Path, &Path) + panic::UnwindSafe,
{
with_tmp_dir(|dir| {
let wallet_path = dir.join("wallet.json");
save_dummy_wallet_file(&wallet_path);
f(dir, &wallet_path);
})
let tmp_dir = tempfile::TempDir::new().unwrap();
let wallet_path = tmp_dir.path().join("wallet.json");
write_wallet_from_mnemonic_and_password(&wallet_path, TEST_MNEMONIC, TEST_PASSWORD)
.unwrap();
f(tmp_dir.path(), &wallet_path);
}
}
Loading