From 5d035d9f8fc2963205daaebe450f9d97644568c0 Mon Sep 17 00:00:00 2001 From: jonesmarvin8 <83104039+jonesmarvin8@users.noreply.github.com> Date: Thu, 30 Apr 2026 19:02:33 -0400 Subject: [PATCH] comment fixes --- Cargo.lock | 37 +++++-- integration_tests/tests/amm.rs | 4 - integration_tests/tests/ata.rs | 3 - .../tests/auth_transfer/private.rs | 10 -- .../tests/auth_transfer/public.rs | 8 -- integration_tests/tests/indexer.rs | 3 - integration_tests/tests/keys_restoration.rs | 7 -- integration_tests/tests/pinata.rs | 6 -- integration_tests/tests/token.rs | 8 -- keycard_tests.sh | 39 ++++--- keycard_wallet/src/lib.rs | 58 +++++----- keycard_wallet/src/python_path.rs | 18 +++- python/keycard_wallet.py | 26 ++--- wallet-ffi/src/transfer.rs | 4 +- wallet/Cargo.toml | 2 + wallet/src/cli/account.rs | 18 +--- wallet/src/cli/keycard.rs | 13 +-- wallet/src/cli/mod.rs | 1 + wallet/src/cli/programs/amm.rs | 33 ++---- .../src/cli/programs/native_token_transfer.rs | 75 ++++++------- wallet/src/cli/programs/pinata.rs | 16 +-- wallet/src/cli/programs/token.rs | 26 ++--- wallet/src/helperfunctions.rs | 28 +++-- wallet/src/lib.rs | 2 + .../native_token_transfer/public.rs | 101 ++++++++---------- 25 files changed, 243 insertions(+), 303 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 85286be6..5a34b7e9 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1959,7 +1959,7 @@ source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "7ab67060fc6b8ef687992d439ca0fa36e7ed17e9a0b16b25b601e8757df720de" dependencies = [ "data-encoding", - "syn 1.0.109", + "syn 2.0.117", ] [[package]] @@ -2108,7 +2108,7 @@ dependencies = [ "libc", "option-ext", "redox_users", - "windows-sys 0.59.0", + "windows-sys 0.61.2", ] [[package]] @@ -2409,7 +2409,7 @@ source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "39cab71617ae0d63f51a36d69f866391735b51691dbda63cf6f96d042b63efeb" dependencies = [ "libc", - "windows-sys 0.52.0", + "windows-sys 0.61.2", ] [[package]] @@ -5415,7 +5415,7 @@ version = "0.50.3" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "7957b9740744892f114936ab4a57b3f487491bbeafaf8083688b16841a4240e5" dependencies = [ - "windows-sys 0.59.0", + "windows-sys 0.61.2", ] [[package]] @@ -7037,6 +7037,17 @@ version = "0.3.1" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "afab94fb28594581f62d981211a9a4d53cc8130bbcbbb89a0440d9b8e81a7746" +[[package]] +name = "rpassword" +version = "7.5.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "2501c67132bd19c3005b0111fba298907ef002c8c1cf68e25634707e38bf66fe" +dependencies = [ + "libc", + "rtoolbox", + "windows-sys 0.61.2", +] + [[package]] name = "rpds" version = "1.2.0" @@ -7111,6 +7122,16 @@ dependencies = [ "thiserror 2.0.18", ] +[[package]] +name = "rtoolbox" +version = "0.0.5" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "50a0e551c1e27e1731aba276dbeaeac73f53c7cd34d1bda485d02bd1e0f36844" +dependencies = [ + "libc", + "windows-sys 0.59.0", +] + [[package]] name = "ruint" version = "1.17.2" @@ -7164,7 +7185,7 @@ dependencies = [ "errno", "libc", "linux-raw-sys", - "windows-sys 0.52.0", + "windows-sys 0.61.2", ] [[package]] @@ -8103,7 +8124,7 @@ dependencies = [ "getrandom 0.4.2", "once_cell", "rustix", - "windows-sys 0.52.0", + "windows-sys 0.61.2", ] [[package]] @@ -9117,6 +9138,7 @@ dependencies = [ "optfield", "pyo3", "rand 0.8.5", + "rpassword", "sequencer_service_rpc", "serde", "serde_json", @@ -9126,6 +9148,7 @@ dependencies = [ "token_core", "tokio", "url", + "zeroize", ] [[package]] @@ -9394,7 +9417,7 @@ version = "0.1.11" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "c2a7b1c03c876122aa43f3020e6c3c3ee5c05081c9a00739faf7503aeba10d22" dependencies = [ - "windows-sys 0.52.0", + "windows-sys 0.61.2", ] [[package]] diff --git a/integration_tests/tests/amm.rs b/integration_tests/tests/amm.rs index 3949a495..db3a4ead 100644 --- a/integration_tests/tests/amm.rs +++ b/integration_tests/tests/amm.rs @@ -134,7 +134,6 @@ async fn amm_public() -> Result<()> { to_npk: None, to_vpk: None, amount: 7, - pin: None, from_key_path: None, }; @@ -165,7 +164,6 @@ async fn amm_public() -> Result<()> { to_npk: None, to_vpk: None, amount: 7, - pin: None, from_key_path: None, }; @@ -555,7 +553,6 @@ async fn amm_new_pool_using_labels() -> Result<()> { to_npk: None, to_vpk: None, amount: 5, - pin: None, from_key_path: None, }; wallet::cli::execute_subcommand(ctx.wallet_mut(), Command::Token(subcommand)).await?; @@ -581,7 +578,6 @@ async fn amm_new_pool_using_labels() -> Result<()> { to_npk: None, to_vpk: None, amount: 5, - pin: None, from_key_path: None, }; wallet::cli::execute_subcommand(ctx.wallet_mut(), Command::Token(subcommand)).await?; diff --git a/integration_tests/tests/ata.rs b/integration_tests/tests/ata.rs index cbde7021..ba487739 100644 --- a/integration_tests/tests/ata.rs +++ b/integration_tests/tests/ata.rs @@ -269,7 +269,6 @@ async fn transfer_and_burn_via_ata() -> Result<()> { to_npk: None, to_vpk: None, amount: fund_amount, - pin: None, from_key_path: None, }), ) @@ -503,7 +502,6 @@ async fn transfer_via_ata_private_owner() -> Result<()> { to_npk: None, to_vpk: None, amount: fund_amount, - pin: None, from_key_path: None, }), ) @@ -619,7 +617,6 @@ async fn burn_via_ata_private_owner() -> Result<()> { to_npk: None, to_vpk: None, amount: fund_amount, - pin: None, from_key_path: None, }), ) diff --git a/integration_tests/tests/auth_transfer/private.rs b/integration_tests/tests/auth_transfer/private.rs index 9e8dce2a..a7d74dec 100644 --- a/integration_tests/tests/auth_transfer/private.rs +++ b/integration_tests/tests/auth_transfer/private.rs @@ -31,7 +31,6 @@ async fn private_transfer_to_owned_account() -> Result<()> { to_npk: None, to_vpk: None, amount: 100, - pin: None, to_key_path: None, from_key_path: None, }); @@ -75,7 +74,6 @@ async fn private_transfer_to_foreign_account() -> Result<()> { to_npk: Some(to_npk_string), to_vpk: Some(hex::encode(to_vpk.0)), amount: 100, - pin: None, to_key_path: None, from_key_path: None, }); @@ -128,7 +126,6 @@ async fn deshielded_transfer_to_public_account() -> Result<()> { to_npk: None, to_vpk: None, amount: 100, - pin: None, to_key_path: None, from_key_path: None, }); @@ -196,7 +193,6 @@ async fn private_transfer_to_owned_account_using_claiming_path() -> Result<()> { to_npk: Some(hex::encode(to_keys.nullifier_public_key.0)), to_vpk: Some(hex::encode(to_keys.viewing_public_key.0)), amount: 100, - pin: None, to_key_path: None, from_key_path: None, }); @@ -249,7 +245,6 @@ async fn shielded_transfer_to_owned_private_account() -> Result<()> { to_npk: None, to_vpk: None, amount: 100, - pin: None, to_key_path: None, from_key_path: None, }); @@ -296,7 +291,6 @@ async fn shielded_transfer_to_foreign_account() -> Result<()> { to_npk: Some(to_npk_string), to_vpk: Some(hex::encode(to_vpk.0)), amount: 100, - pin: None, to_key_path: None, from_key_path: None, }); @@ -371,7 +365,6 @@ async fn private_transfer_to_owned_account_continuous_run_path() -> Result<()> { to_npk: Some(hex::encode(to_keys.nullifier_public_key.0)), to_vpk: Some(hex::encode(to_keys.viewing_public_key.0)), amount: 100, - pin: None, to_key_path: None, from_key_path: None, }); @@ -420,7 +413,6 @@ async fn initialize_private_account() -> Result<()> { let command = Command::AuthTransfer(AuthTransferSubcommand::Init { account_id: Some(format_private_account_id(account_id)), account_label: None, - pin: None, key_path: None, }); wallet::cli::execute_subcommand(ctx.wallet_mut(), command).await?; @@ -479,7 +471,6 @@ async fn private_transfer_using_from_label() -> Result<()> { to_npk: None, to_vpk: None, amount: 100, - pin: None, from_key_path: None, to_key_path: None, }); @@ -525,7 +516,6 @@ async fn initialize_private_account_using_label() -> Result<()> { let command = Command::AuthTransfer(AuthTransferSubcommand::Init { account_id: None, account_label: Some(label), - pin: None, key_path: None, }); wallet::cli::execute_subcommand(ctx.wallet_mut(), command).await?; diff --git a/integration_tests/tests/auth_transfer/public.rs b/integration_tests/tests/auth_transfer/public.rs index 5be41c09..42ad6206 100644 --- a/integration_tests/tests/auth_transfer/public.rs +++ b/integration_tests/tests/auth_transfer/public.rs @@ -24,7 +24,6 @@ async fn successful_transfer_to_existing_account() -> Result<()> { to_npk: None, to_vpk: None, amount: 100, - pin: None, from_key_path: None, to_key_path: None, }); @@ -85,7 +84,6 @@ pub async fn successful_transfer_to_new_account() -> Result<()> { to_npk: None, to_vpk: None, amount: 100, - pin: None, from_key_path: None, to_key_path: None, }); @@ -126,7 +124,6 @@ async fn failed_transfer_with_insufficient_balance() -> Result<()> { to_npk: None, to_vpk: None, amount: 1_000_000, - pin: None, from_key_path: None, to_key_path: None, }); @@ -169,7 +166,6 @@ async fn two_consecutive_successful_transfers() -> Result<()> { to_npk: None, to_vpk: None, amount: 100, - pin: None, from_key_path: None, to_key_path: None, }); @@ -206,7 +202,6 @@ async fn two_consecutive_successful_transfers() -> Result<()> { to_npk: None, to_vpk: None, amount: 100, - pin: None, from_key_path: None, to_key_path: None, }); @@ -253,7 +248,6 @@ async fn initialize_public_account() -> Result<()> { let command = Command::AuthTransfer(AuthTransferSubcommand::Init { account_id: Some(format_public_account_id(account_id)), account_label: None, - pin: None, key_path: None, }); wallet::cli::execute_subcommand(ctx.wallet_mut(), command).await?; @@ -296,7 +290,6 @@ async fn successful_transfer_using_from_label() -> Result<()> { to_npk: None, to_vpk: None, amount: 100, - pin: None, from_key_path: None, to_key_path: None, }); @@ -346,7 +339,6 @@ async fn successful_transfer_using_to_label() -> Result<()> { to_npk: None, to_vpk: None, amount: 100, - pin: None, from_key_path: None, to_key_path: None, }); diff --git a/integration_tests/tests/indexer.rs b/integration_tests/tests/indexer.rs index 5d0c5b0d..af9866f3 100644 --- a/integration_tests/tests/indexer.rs +++ b/integration_tests/tests/indexer.rs @@ -113,7 +113,6 @@ async fn indexer_state_consistency() -> Result<()> { to_npk: None, to_vpk: None, amount: 100, - pin: None, from_key_path: None, to_key_path: None, }); @@ -152,7 +151,6 @@ async fn indexer_state_consistency() -> Result<()> { to_npk: None, to_vpk: None, amount: 100, - pin: None, from_key_path: None, to_key_path: None, }); @@ -241,7 +239,6 @@ async fn indexer_state_consistency_with_labels() -> Result<()> { to_npk: None, to_vpk: None, amount: 100, - pin: None, from_key_path: None, to_key_path: None, }); diff --git a/integration_tests/tests/keys_restoration.rs b/integration_tests/tests/keys_restoration.rs index 7b165a00..38fa25bf 100644 --- a/integration_tests/tests/keys_restoration.rs +++ b/integration_tests/tests/keys_restoration.rs @@ -76,7 +76,6 @@ async fn sync_private_account_with_non_zero_chain_index() -> Result<()> { to_npk: Some(hex::encode(to_keys.nullifier_public_key.0)), to_vpk: Some(hex::encode(to_keys.viewing_public_key.0)), amount: 100, - pin: None, from_key_path: None, to_key_path: None, }); @@ -155,7 +154,6 @@ async fn restore_keys_from_seed() -> Result<()> { to_npk: None, to_vpk: None, amount: 100, - pin: None, from_key_path: None, to_key_path: None, }); @@ -170,7 +168,6 @@ async fn restore_keys_from_seed() -> Result<()> { to_npk: None, to_vpk: None, amount: 101, - pin: None, from_key_path: None, to_key_path: None, }); @@ -213,7 +210,6 @@ async fn restore_keys_from_seed() -> Result<()> { to_npk: None, to_vpk: None, amount: 102, - pin: None, from_key_path: None, to_key_path: None, }); @@ -228,7 +224,6 @@ async fn restore_keys_from_seed() -> Result<()> { to_npk: None, to_vpk: None, amount: 103, - pin: None, from_key_path: None, to_key_path: None, }); @@ -296,7 +291,6 @@ async fn restore_keys_from_seed() -> Result<()> { to_npk: None, to_vpk: None, amount: 10, - pin: None, from_key_path: None, to_key_path: None, }); @@ -310,7 +304,6 @@ async fn restore_keys_from_seed() -> Result<()> { to_npk: None, to_vpk: None, amount: 11, - pin: None, from_key_path: None, to_key_path: None, }); diff --git a/integration_tests/tests/pinata.rs b/integration_tests/tests/pinata.rs index 82873843..fcc77a76 100644 --- a/integration_tests/tests/pinata.rs +++ b/integration_tests/tests/pinata.rs @@ -54,7 +54,6 @@ async fn claim_pinata_to_uninitialized_public_account_fails_fast() -> Result<()> Command::Pinata(PinataProgramAgnosticSubcommand::Claim { to: Some(winner_account_id_formatted), to_label: None, - pin: None, key_path: None, }), ) @@ -111,7 +110,6 @@ async fn claim_pinata_to_uninitialized_private_account_fails_fast() -> Result<() Command::Pinata(PinataProgramAgnosticSubcommand::Claim { to: Some(winner_account_id_formatted), to_label: None, - pin: None, key_path: None, }), ) @@ -145,7 +143,6 @@ async fn claim_pinata_to_existing_public_account() -> Result<()> { let command = Command::Pinata(PinataProgramAgnosticSubcommand::Claim { to: Some(format_public_account_id(ctx.existing_public_accounts()[0])), to_label: None, - pin: None, key_path: None, }); @@ -188,7 +185,6 @@ async fn claim_pinata_to_existing_private_account() -> Result<()> { ctx.existing_private_accounts()[0], )), to_label: None, - pin: None, key_path: None, }); @@ -255,7 +251,6 @@ async fn claim_pinata_to_new_private_account() -> Result<()> { let command = Command::AuthTransfer(AuthTransferSubcommand::Init { account_id: Some(winner_account_id_formatted.clone()), account_label: None, - pin: None, key_path: None, }); wallet::cli::execute_subcommand(ctx.wallet_mut(), command).await?; @@ -273,7 +268,6 @@ async fn claim_pinata_to_new_private_account() -> Result<()> { let command = Command::Pinata(PinataProgramAgnosticSubcommand::Claim { to: Some(winner_account_id_formatted), to_label: None, - pin: None, key_path: None, }); diff --git a/integration_tests/tests/token.rs b/integration_tests/tests/token.rs index f226b52e..69e0d8e2 100644 --- a/integration_tests/tests/token.rs +++ b/integration_tests/tests/token.rs @@ -135,7 +135,6 @@ async fn create_and_transfer_public_token() -> Result<()> { to_npk: None, to_vpk: None, amount: transfer_amount, - pin: None, from_key_path: None, }; @@ -182,7 +181,6 @@ async fn create_and_transfer_public_token() -> Result<()> { holder: Some(format_public_account_id(recipient_account_id)), holder_label: None, amount: burn_amount, - holder_pin: None, holder_key_path: None, }; @@ -377,7 +375,6 @@ async fn create_and_transfer_token_with_private_supply() -> Result<()> { to_npk: None, to_vpk: None, amount: transfer_amount, - pin: None, from_key_path: None, }; @@ -406,7 +403,6 @@ async fn create_and_transfer_token_with_private_supply() -> Result<()> { holder: Some(format_private_account_id(recipient_account_id)), holder_label: None, amount: burn_amount, - holder_pin: None, holder_key_path: None, }; @@ -765,7 +761,6 @@ async fn create_token_with_private_definition_and_supply() -> Result<()> { to_npk: None, to_vpk: None, amount: transfer_amount, - pin: None, from_key_path: None, }; @@ -898,7 +893,6 @@ async fn shielded_token_transfer() -> Result<()> { to_npk: None, to_vpk: None, amount: transfer_amount, - pin: None, from_key_path: None, }; @@ -1026,7 +1020,6 @@ async fn deshielded_token_transfer() -> Result<()> { to_npk: None, to_vpk: None, amount: transfer_amount, - pin: None, from_key_path: None, }; @@ -1366,7 +1359,6 @@ async fn transfer_token_using_from_label() -> Result<()> { to_npk: None, to_vpk: None, amount: transfer_amount, - pin: None, from_key_path: None, }; wallet::cli::execute_subcommand(ctx.wallet_mut(), Command::Token(subcommand)).await?; diff --git a/keycard_tests.sh b/keycard_tests.sh index 6a2e065b..4cb844ec 100644 --- a/keycard_tests.sh +++ b/keycard_tests.sh @@ -2,36 +2,35 @@ source venv/bin/activate # Load the appropriate virtual environment +export KEYCARD_PIN=111111 + # Tests wallet keycard available # - Checks whether smart reader and keycard are both available. echo "Test: wallet keycard available" wallet keycard available -echo 'Test: wallet keycard load --pin 111111 --mnemonic "final empty hair duty next drastic normal miss wreck wreck strategy omit"' # Install a new mnemonic phrase to keycard -wallet keycard load --pin 111111 --mnemonic "fashion degree mountain wool question damp current pond grow dolphin chronic then" -# Commented out to avoid resetting card constantly +echo 'Test: wallet keycard load --mnemonic "fashion degree mountain wool question damp current pond grow dolphin chronic then"' +wallet keycard load --mnemonic "fashion degree mountain wool question damp current pond grow dolphin chronic then" -echo "Test: wallet auth-transfer --pin 111111 --key-path \"m/44'/60/0\'/0/0\"" -wallet auth-transfer init --pin 111111 --key-path "m/44'/60'/0'/0/0" +echo "Test: wallet auth-transfer init --key-path \"m/44'/60'/0'/0/0\"" +wallet auth-transfer init --key-path "m/44'/60'/0'/0/0" -echo "Test: wallet account get --pin 111111 --key-path \"m/44'/60'/0'/0/0\"" -wallet account get --pin 111111 --key-path "m/44'/60'/0'/0/0" +echo "Test: wallet account get --key-path \"m/44'/60'/0'/0/0\"" +wallet account get --key-path "m/44'/60'/0'/0/0" +echo "Test: wallet pinata claim --key-path \"m/44'/60'/0'/0/0\"" +wallet pinata claim --key-path "m/44'/60'/0'/0/0" -echo "Test: wallet pinata claim --pin 111111 --key-path \"m/44'/60'/0'/0/0\"" -wallet pinata claim --pin 111111 --key-path "m/44'/60'/0'/0/0" +echo "Test: wallet account get --key-path \"m/44'/60'/0'/0/0\"" +wallet account get --key-path "m/44'/60'/0'/0/0" +#echo "Initialize new account (auth-transfer init) and send" +wallet auth-transfer init --key-path "m/44'/60'/0'/0/1" +wallet auth-transfer send --amount 40 --from-key-path "m/44'/60'/0'/0/0" --to-key-path "m/44'/60'/0'/0/1" -echo "Test: wallet account get --pin 111111 --key-path \"m/44'/60'/0'/0/0\"" -wallet account get --pin 111111 --key-path "m/44'/60'/0'/0/0" +echo "Test: wallet account get --key-path \"m/44'/60'/0'/0/0\"" +wallet account get --key-path "m/44'/60'/0'/0/0" -echo "Initialize new account (auth-transfer init) and send" -wallet auth-transfer init --pin 111111 --key-path "m/44'/60'/0'/0/1" -wallet auth-transfer send --amount 40 --pin 111111 --from-key-path "m/44'/60'/0'/0/0" --to-key-path "m/44'/60'/0'/0/1" - -echo "Test: wallet account get --pin 111111 --key-path \"m/44'/60'/0'/0/0\"" -wallet account get --pin 111111 --key-path "m/44'/60'/0'/0/0" - -echo "Test: wallet account get --pin 111111 --key-path \"m/44'/60'/0'/0/1\"" -wallet account get --pin 111111 --key-path "m/44'/60'/0'/0/1" +echo "Test: wallet account get --key-path \"m/44'/60'/0'/0/1\"" +wallet account get --key-path "m/44'/60'/0'/0/1" diff --git a/keycard_wallet/src/lib.rs b/keycard_wallet/src/lib.rs index c968a682..37b010fd 100644 --- a/keycard_wallet/src/lib.rs +++ b/keycard_wallet/src/lib.rs @@ -46,21 +46,24 @@ impl KeycardWallet { .call_method1("get_public_key_for_path", (path,))? .extract()?; - let public_key: [u8; 32] = public_key.try_into().expect("Expect 32 bytes"); + let public_key: [u8; 32] = public_key.try_into().map_err(|vec: Vec| { + PyErr::new::(format!( + "expected 32-byte public key from keycard, got {} bytes", + vec.len() + )) + })?; - Ok(PublicKey::try_new(public_key).expect("Expect a valid public key1")) + PublicKey::try_new(public_key) + .map_err(|e| PyErr::new::(e.to_string())) } - #[must_use] - pub fn get_public_key_for_path_with_connect(pin: &str, path: &str) -> PublicKey { - let pub_key = Python::with_gil(|py| { - python_path::add_python_path(py).expect("keycard_wallet.py not found"); + pub fn get_public_key_for_path_with_connect(pin: &str, path: &str) -> PyResult { + Python::with_gil(|py| { + python_path::add_python_path(py)?; - let wallet = Self::new(py).expect("Expect keycard wallet"); + let wallet = Self::new(py)?; - let is_connected = wallet - .setup_communication(py, pin) - .expect("Expect a Boolean."); + let is_connected = wallet.setup_communication(py, pin)?; if is_connected { log::info!("\u{2705} Keycard is now connected to wallet."); @@ -72,8 +75,7 @@ impl KeycardWallet { drop(wallet.disconnect(py)); pub_key - }); - pub_key.expect("Expect a valid public key2") + }) } pub fn sign_message_for_path( @@ -81,7 +83,7 @@ impl KeycardWallet { py: Python, path: &str, message: &[u8; 32], - ) -> PyResult { + ) -> PyResult<(Signature, PublicKey)> { let py_signature: Vec = self .instance .bind(py) @@ -96,22 +98,27 @@ impl KeycardWallet { )) })?; - Ok(Signature { value: signature }) + let sig = Signature { value: signature }; + let pub_key = self.get_public_key_for_path(py, path)?; + if !sig.is_valid_for(message, &pub_key) { + return Err(PyErr::new::( + "keycard returned a signature that does not verify against its own public key", + )); + } + Ok((sig, pub_key)) } pub fn sign_message_for_path_with_connect( pin: &str, path: &str, message: &[u8; 32], - ) -> PyResult { + ) -> PyResult<(Signature, PublicKey)> { Python::with_gil(|py| { - python_path::add_python_path(py).expect("keycard_wallet.py not found"); + python_path::add_python_path(py)?; - let wallet = Self::new(py).expect("Expect keycard wallet"); + let wallet = Self::new(py)?; - let is_connected = wallet - .setup_communication(py, pin) - .expect("Expect a Boolean."); + let is_connected = wallet.setup_communication(py, pin)?; if is_connected { log::info!("\u{2705} Keycard is now connected to wallet."); @@ -119,11 +126,11 @@ impl KeycardWallet { log::info!("\u{274c} Keycard is not connected to wallet."); } - let signature = wallet.sign_message_for_path(py, path, message); + let result = wallet.sign_message_for_path(py, path, message); drop(wallet.disconnect(py)); - signature + result }) } @@ -134,10 +141,9 @@ impl KeycardWallet { Ok(()) } - #[must_use] - pub fn get_account_id_for_path_with_connect(pin: &str, key_path: &str) -> String { - let public_key = Self::get_public_key_for_path_with_connect(pin, key_path); + pub fn get_account_id_for_path_with_connect(pin: &str, key_path: &str) -> PyResult { + let public_key = Self::get_public_key_for_path_with_connect(pin, key_path)?; - format!("Public/{}", AccountId::from(&public_key)) + Ok(format!("Public/{}", AccountId::from(&public_key))) } } diff --git a/keycard_wallet/src/python_path.rs b/keycard_wallet/src/python_path.rs index b98061eb..c8eb049e 100644 --- a/keycard_wallet/src/python_path.rs +++ b/keycard_wallet/src/python_path.rs @@ -6,11 +6,27 @@ use pyo3::{prelude::*, types::PyList}; pub fn add_python_path(py: Python<'_>) -> PyResult<()> { let current_dir = env::current_dir().expect("Failed to get current working directory"); - let paths_to_add: Vec = vec![ + let mut paths_to_add: Vec = vec![ current_dir.join("python"), current_dir.join("python").join("keycard-py"), ]; + // If a virtualenv is active, add its site-packages so that dependencies + // installed in the venv (e.g. smartcard, ecdsa) are importable by the + // pyo3 embedded interpreter, which does not inherit sys.path from the + // shell's `python3` executable. + if let Ok(venv) = env::var("VIRTUAL_ENV") { + let lib = PathBuf::from(&venv).join("lib"); + if let Ok(entries) = std::fs::read_dir(&lib) { + for entry in entries.flatten() { + let site_packages = entry.path().join("site-packages"); + if site_packages.exists() { + paths_to_add.push(site_packages); + } + } + } + } + // Sanity check — warns early if a path doesn't exist for path in &paths_to_add { if !path.exists() { diff --git a/python/keycard_wallet.py b/python/keycard_wallet.py index 73c27fa9..7d7e5bd3 100644 --- a/python/keycard_wallet.py +++ b/python/keycard_wallet.py @@ -9,17 +9,11 @@ from keycard import constants import keycard -PIN = '123456' -PUK = '123456123456' DEFAULT_PAIRING_PASSWORD = "KeycardDefaultPairing" -DEFAULT_MNEMONIC = "fashion degree mountain wool question damp current pond grow dolphin chronic then" -DEFAULT_PASSPHRASE = "" class KeycardWallet: def __init__(self): self.card = KeyCard() - self.pairing_index = None - self.pairing_key = None def _is_smart_card_reader_detected(self) -> bool: try: @@ -35,7 +29,6 @@ class KeycardWallet: # No readers, no card, or card doesn't respond. return False - # Wrapped def is_unpaired_keycard_available(self) -> bool: if not self._is_smart_card_reader_detected(): return False @@ -43,20 +36,16 @@ class KeycardWallet: return False return True - # Wrapped - def setup_communication(self, pin = PIN, password = DEFAULT_PAIRING_PASSWORD) -> bool: + def setup_communication(self, pin: str, password = DEFAULT_PAIRING_PASSWORD) -> bool: try: self.card.select() if not self.card.is_initialized: return False - if self.pairing_index is None: - pairing_index, pairing_key = self.card.pair(password) - self.pairing_index = pairing_index - self.pairing_key = pairing_key + pairing_index, pairing_key = self.card.pair(password) + self.pairing_index = pairing_index - self.card.open_secure_channel(pairing_index, pairing_key) self.card.verify_pin(pin) @@ -65,11 +54,11 @@ class KeycardWallet: print(f"Error: {e}") return False - def load_mnemonic(self, mnemonic = DEFAULT_MNEMONIC, passphrase = DEFAULT_PASSPHRASE) -> bool: + def load_mnemonic(self, mnemonic: str) -> bool: try: # Convert mnemonic to seed mnemo = Mnemonic("english") - seed = mnemo.to_seed(mnemonic, passphrase) + seed = mnemo.to_seed(mnemonic) # Load the LEE seed onto the card result = self.card.load_key( @@ -77,6 +66,7 @@ class KeycardWallet: lee_seed = seed ) + #TODO: this appears to be the issue. return True except Exception as e: print(f"Error during disconnect: {e}") @@ -88,8 +78,6 @@ class KeycardWallet: return None self.card.unpair(self.pairing_index) - self.pairing_index = None - self.pairing_key = None return True except Exception as e: @@ -118,7 +106,7 @@ class KeycardWallet: return None - def sign_message_for_path(self, message: bytes = b"DefaultMessageTestDefaultMessage", path: str = "m/44'/60'/0'/0/0") -> bytes | None: + def sign_message_for_path(self, message: bytes, path: str = "m/44'/60'/0'/0/0") -> bytes | None: try: if not self.card.is_secure_channel_open or not self.card.is_pin_verified: return None diff --git a/wallet-ffi/src/transfer.rs b/wallet-ffi/src/transfer.rs index e65fa473..d7dc9841 100644 --- a/wallet-ffi/src/transfer.rs +++ b/wallet-ffi/src/transfer.rs @@ -72,7 +72,7 @@ pub unsafe extern "C" fn wallet_ffi_transfer_public( let transfer = NativeTokenTransfer(&wallet); - match block_on(transfer.send_public_transfer(from_id, to_id, amount, &None, &None)) { + match block_on(transfer.send_public_transfer(from_id, to_id, amount, None, None)) { Ok(tx_hash) => { let tx_hash = CString::new(tx_hash.to_string()) .map_or(ptr::null_mut(), std::ffi::CString::into_raw); @@ -566,7 +566,7 @@ pub unsafe extern "C" fn wallet_ffi_register_public_account( let transfer = NativeTokenTransfer(&wallet); - match block_on(transfer.register_account(account_id, &None, &None)) { + match block_on(transfer.register_account(account_id, None)) { Ok(tx_hash) => { let tx_hash = CString::new(tx_hash.to_string()) .map_or(ptr::null_mut(), std::ffi::CString::into_raw); diff --git a/wallet/Cargo.toml b/wallet/Cargo.toml index 4d8750a4..a4d5cf0a 100644 --- a/wallet/Cargo.toml +++ b/wallet/Cargo.toml @@ -19,6 +19,8 @@ testnet_initial_state.workspace = true ata_core.workspace = true bip39.workspace = true pyo3.workspace = true +rpassword = "7" +zeroize = "1" anyhow.workspace = true thiserror.workspace = true diff --git a/wallet/src/cli/account.rs b/wallet/src/cli/account.rs index 9bdd7132..bc63e0bb 100644 --- a/wallet/src/cli/account.rs +++ b/wallet/src/cli/account.rs @@ -28,19 +28,12 @@ pub enum AccountSubcommand { #[arg(short, long)] keys: bool, /// Valid 32 byte base58 string with privacy prefix. - #[arg(short, long, conflicts_with = "account_label", required_unless_present_any = ["account_label", "pin"])] + #[arg(short, long, conflicts_with = "account_label", required_unless_present_any = ["account_label", "key_path"])] account_id: Option, /// Account label (alternative to --account-id). #[arg(long, conflicts_with = "account_id")] account_label: Option, - #[arg( - long, - conflicts_with = "account_id", - conflicts_with = "account_label", - requires = "key_path" - )] - pin: Option, - #[arg(long)] + #[arg(long, conflicts_with = "account_id", conflicts_with = "account_label")] key_path: Option, }, /// Produce new public or private account. @@ -195,7 +188,6 @@ impl WalletSubcommand for AccountSubcommand { keys, account_id, account_label, - pin, key_path, } => { let resolved = resolve_id_or_label( @@ -203,8 +195,7 @@ impl WalletSubcommand for AccountSubcommand { account_label, &wallet_core.storage.labels, &wallet_core.storage.user_data, - &pin, - &key_path, + key_path.as_deref(), )?; let (account_id_str, addr_kind) = parse_addr_with_privacy_prefix(&resolved)?; @@ -418,8 +409,7 @@ impl WalletSubcommand for AccountSubcommand { account_label, &wallet_core.storage.labels, &wallet_core.storage.user_data, - &None, - &None, + None, )?; let (account_id_str, _) = parse_addr_with_privacy_prefix(&resolved)?; diff --git a/wallet/src/cli/keycard.rs b/wallet/src/cli/keycard.rs index 8dde98e4..3c35ebda 100644 --- a/wallet/src/cli/keycard.rs +++ b/wallet/src/cli/keycard.rs @@ -5,7 +5,7 @@ use pyo3::prelude::*; use crate::{ WalletCore, - cli::{SubcommandReturnValue, WalletSubcommand}, + cli::{SubcommandReturnValue, WalletSubcommand, read_pin}, }; /// Represents generic chain CLI subcommand. @@ -15,8 +15,6 @@ pub enum KeycardSubcommand { Load { #[arg(short, long)] mnemonic: Option, - #[arg(short, long)] - pin: Option, }, } @@ -45,7 +43,9 @@ impl WalletSubcommand for KeycardSubcommand { Ok(SubcommandReturnValue::Empty) } - Self::Load { mnemonic, pin } => { + Self::Load { mnemonic } => { + let pin = read_pin()?; + Python::with_gil(|py| { python_path::add_python_path(py).expect("keycard_wallet.py not found"); @@ -53,10 +53,7 @@ impl WalletSubcommand for KeycardSubcommand { .expect("`wallet::keycard::load`: invalid keycard wallet provided"); let is_connected = wallet - .setup_communication( - py, - &pin.expect("`wallet::keycard::load`: invalid data received for pin"), - ) + .setup_communication(py, &pin) .expect("Expect a Boolean."); if is_connected { diff --git a/wallet/src/cli/mod.rs b/wallet/src/cli/mod.rs index 1948db24..1ce0ea2c 100644 --- a/wallet/src/cli/mod.rs +++ b/wallet/src/cli/mod.rs @@ -8,6 +8,7 @@ use futures::TryFutureExt as _; use nssa::{ProgramDeploymentTransaction, program::Program}; use sequencer_service_rpc::RpcClient as _; +pub use crate::helperfunctions::read_pin; use crate::{ WalletCore, cli::{ diff --git a/wallet/src/cli/programs/amm.rs b/wallet/src/cli/programs/amm.rs index aa0c5482..1805f444 100644 --- a/wallet/src/cli/programs/amm.rs +++ b/wallet/src/cli/programs/amm.rs @@ -216,24 +216,21 @@ impl WalletSubcommand for AmmProgramAgnosticSubcommand { user_holding_a_label, &wallet_core.storage.labels, &wallet_core.storage.user_data, - &None, - &None, + None, )?; let user_holding_b = resolve_id_or_label( user_holding_b, user_holding_b_label, &wallet_core.storage.labels, &wallet_core.storage.user_data, - &None, - &None, + None, )?; let user_holding_lp = resolve_id_or_label( user_holding_lp, user_holding_lp_label, &wallet_core.storage.labels, &wallet_core.storage.user_data, - &None, - &None, + None, )?; let (user_holding_a, user_holding_a_privacy) = parse_addr_with_privacy_prefix(&user_holding_a)?; @@ -288,16 +285,14 @@ impl WalletSubcommand for AmmProgramAgnosticSubcommand { user_holding_a_label, &wallet_core.storage.labels, &wallet_core.storage.user_data, - &None, - &None, + None, )?; let user_holding_b = resolve_id_or_label( user_holding_b, user_holding_b_label, &wallet_core.storage.labels, &wallet_core.storage.user_data, - &None, - &None, + None, )?; let (user_holding_a, user_holding_a_privacy) = parse_addr_with_privacy_prefix(&user_holding_a)?; @@ -378,24 +373,21 @@ impl WalletSubcommand for AmmProgramAgnosticSubcommand { user_holding_a_label, &wallet_core.storage.labels, &wallet_core.storage.user_data, - &None, - &None, + None, )?; let user_holding_b = resolve_id_or_label( user_holding_b, user_holding_b_label, &wallet_core.storage.labels, &wallet_core.storage.user_data, - &None, - &None, + None, )?; let user_holding_lp = resolve_id_or_label( user_holding_lp, user_holding_lp_label, &wallet_core.storage.labels, &wallet_core.storage.user_data, - &None, - &None, + None, )?; let (user_holding_a, user_holding_a_privacy) = parse_addr_with_privacy_prefix(&user_holding_a)?; @@ -453,24 +445,21 @@ impl WalletSubcommand for AmmProgramAgnosticSubcommand { user_holding_a_label, &wallet_core.storage.labels, &wallet_core.storage.user_data, - &None, - &None, + None, )?; let user_holding_b = resolve_id_or_label( user_holding_b, user_holding_b_label, &wallet_core.storage.labels, &wallet_core.storage.user_data, - &None, - &None, + None, )?; let user_holding_lp = resolve_id_or_label( user_holding_lp, user_holding_lp_label, &wallet_core.storage.labels, &wallet_core.storage.user_data, - &None, - &None, + None, )?; let (user_holding_a, user_holding_a_privacy) = parse_addr_with_privacy_prefix(&user_holding_a)?; diff --git a/wallet/src/cli/programs/native_token_transfer.rs b/wallet/src/cli/programs/native_token_transfer.rs index 003682e2..8508f0f6 100644 --- a/wallet/src/cli/programs/native_token_transfer.rs +++ b/wallet/src/cli/programs/native_token_transfer.rs @@ -1,7 +1,6 @@ use anyhow::Result; use clap::Subcommand; use common::transaction::NSSATransaction; -use keycard_wallet::KeycardWallet; use nssa::AccountId; use crate::{ @@ -24,21 +23,14 @@ pub enum AuthTransferSubcommand { #[arg( long, conflicts_with = "account_label", - conflicts_with = "pin", - required_unless_present_any = ["account_label", "pin"] + required_unless_present_any = ["account_label", "key_path"] )] account_id: Option, /// Account label (alternative to --account-id). - #[arg(long, conflicts_with = "account_id", conflicts_with = "pin")] + #[arg(long, conflicts_with = "account_id")] account_label: Option, - #[arg( - long, - conflicts_with = "account_id", - conflicts_with = "account_label", - requires = "key_path" - )] - pin: Option, - #[arg(long)] + /// Key path (alternative to --account-id) is used to retrieve data from Keycard. + #[arg(long, conflicts_with = "account_id", conflicts_with = "account_label")] key_path: Option, }, /// Send native tokens from one account to another with variable privacy. @@ -49,10 +41,10 @@ pub enum AuthTransferSubcommand { /// First is used for owned accounts, second otherwise. Send { /// from - valid 32 byte base58 string with privacy prefix. - #[arg(long, conflicts_with = "from_label", conflicts_with = "pin", required_unless_present_any = ["from_label", "pin"])] + #[arg(long, conflicts_with = "from_label", required_unless_present_any = ["from_label", "from_key_path"])] from: Option, /// From account label (alternative to --from). - #[arg(long, conflicts_with = "from", conflicts_with = "pin")] + #[arg(long, conflicts_with = "from")] from_label: Option, /// to - valid 32 byte base58 string with privacy prefix. #[arg(long, conflicts_with = "to_label")] @@ -69,10 +61,10 @@ pub enum AuthTransferSubcommand { /// amount - amount of balance to move. #[arg(long)] amount: u128, - #[arg(long)] - pin: Option, + /// `from_key_path` (alternative to --from) uses Keycard. #[arg(long, conflicts_with = "from", conflicts_with = "from_label")] from_key_path: Option, + /// `to_key_path` (alternative to --to) uses Keycard. #[arg(long, conflicts_with = "to", conflicts_with = "to_label")] to_key_path: Option, }, @@ -87,7 +79,6 @@ impl WalletSubcommand for AuthTransferSubcommand { Self::Init { account_id, account_label, - pin, key_path, } => { let resolved = resolve_id_or_label( @@ -95,8 +86,7 @@ impl WalletSubcommand for AuthTransferSubcommand { account_label, &wallet_core.storage.labels, &wallet_core.storage.user_data, - &pin, - &key_path, + key_path.as_deref(), )?; let (account_id, addr_privacy) = parse_addr_with_privacy_prefix(&resolved)?; @@ -106,7 +96,7 @@ impl WalletSubcommand for AuthTransferSubcommand { let account_id = account_id.parse()?; let tx_hash = NativeTokenTransfer(wallet_core) - .register_account(account_id, &pin, &key_path) + .register_account(account_id, key_path.as_deref()) .await?; println!("Transaction hash is {tx_hash}"); @@ -151,7 +141,6 @@ impl WalletSubcommand for AuthTransferSubcommand { to_npk, to_vpk, amount, - pin, from_key_path, to_key_path, } => { @@ -160,10 +149,10 @@ impl WalletSubcommand for AuthTransferSubcommand { from_label, &wallet_core.storage.labels, &wallet_core.storage.user_data, - &pin, - &from_key_path, + from_key_path.as_deref(), )?; + let to_key_path_for_sign = to_key_path.clone(); let to = match (to, to_label, to_key_path) { (v, None, None) => v, (None, Some(label), None) => Some(resolve_account_label( @@ -171,12 +160,13 @@ impl WalletSubcommand for AuthTransferSubcommand { &wallet_core.storage.labels, &wallet_core.storage.user_data, )?), - (None, None, Some(to_key_path)) => { - Some(KeycardWallet::get_account_id_for_path_with_connect( - pin.as_ref().expect("`wallet::programs::native_token_transfer::send`: invalid data received for pin"), - &to_key_path, - )) - } + (None, None, Some(to_key_path)) => Some(resolve_id_or_label( + None, + None, + &wallet_core.storage.labels, + &wallet_core.storage.user_data, + Some(&to_key_path), + )?), _ => { anyhow::bail!("Provide only one of --to or --to-label") } @@ -206,8 +196,8 @@ impl WalletSubcommand for AuthTransferSubcommand { from, to, amount, - pin, - key_path: from_key_path, + from_key_path, + to_key_path: to_key_path_for_sign, } } (AccountPrivacyKind::Private, AccountPrivacyKind::Private) => { @@ -287,15 +277,10 @@ pub enum NativeTokenTransferProgramSubcommand { /// amount - amount of balance to move. #[arg(long)] amount: u128, - #[arg( - long, - conflicts_with = "from", - conflicts_with = "from_label", - requires = "key_path" - )] - pin: Option, #[arg(long)] - key_path: Option, + from_key_path: Option, + #[arg(skip)] + to_key_path: Option, }, /// Private execution. #[command(subcommand)] @@ -572,14 +557,20 @@ impl WalletSubcommand for NativeTokenTransferProgramSubcommand { from, to, amount, - pin, - key_path, + from_key_path, + to_key_path, } => { let from: AccountId = from.parse().unwrap(); let to: AccountId = to.parse().unwrap(); let tx_hash = NativeTokenTransfer(wallet_core) - .send_public_transfer(from, to, amount, &pin, &key_path) + .send_public_transfer( + from, + to, + amount, + from_key_path.as_deref(), + to_key_path.as_deref(), + ) .await?; println!("Transaction hash is {tx_hash}"); diff --git a/wallet/src/cli/programs/pinata.rs b/wallet/src/cli/programs/pinata.rs index 240a6c81..d8b01959 100644 --- a/wallet/src/cli/programs/pinata.rs +++ b/wallet/src/cli/programs/pinata.rs @@ -17,19 +17,13 @@ pub enum PinataProgramAgnosticSubcommand { /// Claim pinata. Claim { /// to - valid 32 byte base58 string with privacy prefix. - #[arg(long, conflicts_with = "to_label", required_unless_present_any = ["to_label", "pin"])] + #[arg(long, conflicts_with = "to_label", required_unless_present_any = ["to_label", "key_path"])] to: Option, /// To account label (alternative to --to). #[arg(long, conflicts_with = "to")] to_label: Option, - #[arg( - long, - conflicts_with = "to", - conflicts_with = "to_label", - requires = "key_path" - )] - pin: Option, - #[arg(long)] + /// To key path (alternative to --to) uses Keycard. + #[arg(long, conflicts_with = "to", conflicts_with = "to_label")] key_path: Option, }, } @@ -43,7 +37,6 @@ impl WalletSubcommand for PinataProgramAgnosticSubcommand { Self::Claim { to, to_label, - pin, key_path, } => { let to = resolve_id_or_label( @@ -51,8 +44,7 @@ impl WalletSubcommand for PinataProgramAgnosticSubcommand { to_label, &wallet_core.storage.labels, &wallet_core.storage.user_data, - &pin, - &key_path, + key_path.as_deref(), )?; let (to, to_addr_privacy) = parse_addr_with_privacy_prefix(&to)?; diff --git a/wallet/src/cli/programs/token.rs b/wallet/src/cli/programs/token.rs index cd52cf2d..83582c87 100644 --- a/wallet/src/cli/programs/token.rs +++ b/wallet/src/cli/programs/token.rs @@ -76,8 +76,6 @@ pub enum TokenProgramAgnosticSubcommand { /// amount - amount of balance to move. #[arg(long)] amount: u128, - #[arg(long)] - pin: Option, #[arg(long, conflicts_with = "from", conflicts_with = "from_label")] from_key_path: Option, }, @@ -108,8 +106,6 @@ pub enum TokenProgramAgnosticSubcommand { #[arg(long)] amount: u128, #[arg(long, conflicts_with = "holder", conflicts_with = "holder_label")] - holder_pin: Option, - #[arg(long, conflicts_with = "holder", conflicts_with = "holder_label")] holder_key_path: Option, }, /// Mint tokens on `holder`, modify `definition`. @@ -132,7 +128,7 @@ pub enum TokenProgramAgnosticSubcommand { #[arg(long, conflicts_with = "definition")] definition_label: Option, /// holder - valid 32 byte base58 string with privacy prefix. - #[arg(long, conflicts_with = "holder_label", required_unless_present_any = ["holder_label", "pin"])] + #[arg(long, conflicts_with = "holder_label", required_unless_present_any = ["holder_label", "holder_key_path"])] holder: Option, /// Holder account label (alternative to --holder). #[arg(long, conflicts_with = "holder")] @@ -168,16 +164,14 @@ impl WalletSubcommand for TokenProgramAgnosticSubcommand { definition_account_label, &wallet_core.storage.labels, &wallet_core.storage.user_data, - &None, - &None, + None, )?; let supply_account_id = resolve_id_or_label( supply_account_id, supply_account_label, &wallet_core.storage.labels, &wallet_core.storage.user_data, - &None, - &None, + None, )?; let (definition_account_id, definition_addr_privacy) = parse_addr_with_privacy_prefix(&definition_account_id)?; @@ -237,7 +231,6 @@ impl WalletSubcommand for TokenProgramAgnosticSubcommand { to_npk, to_vpk, amount, - pin, from_key_path, } => { let from = resolve_id_or_label( @@ -245,8 +238,7 @@ impl WalletSubcommand for TokenProgramAgnosticSubcommand { from_label, &wallet_core.storage.labels, &wallet_core.storage.user_data, - &pin, - &from_key_path, + from_key_path.as_deref(), )?; let to = match (to, to_label) { (v, None) => v, @@ -348,7 +340,6 @@ impl WalletSubcommand for TokenProgramAgnosticSubcommand { holder, holder_label, amount, - holder_pin, holder_key_path, } => { let definition = resolve_id_or_label( @@ -356,16 +347,14 @@ impl WalletSubcommand for TokenProgramAgnosticSubcommand { definition_label, &wallet_core.storage.labels, &wallet_core.storage.user_data, - &None, - &None, + None, )?; let holder = resolve_id_or_label( holder, holder_label, &wallet_core.storage.labels, &wallet_core.storage.user_data, - &holder_pin, - &holder_key_path, + holder_key_path.as_deref(), )?; let underlying_subcommand = { let (definition, definition_privacy) = @@ -427,8 +416,7 @@ impl WalletSubcommand for TokenProgramAgnosticSubcommand { definition_label, &wallet_core.storage.labels, &wallet_core.storage.user_data, - &None, - &None, + None, )?; let holder = match (holder, holder_label) { (v, None) => v, diff --git a/wallet/src/helperfunctions.rs b/wallet/src/helperfunctions.rs index 8e5e3f3e..b4420b02 100644 --- a/wallet/src/helperfunctions.rs +++ b/wallet/src/helperfunctions.rs @@ -51,6 +51,20 @@ impl From for HumanReadableAccount { } } +/// Read the Keycard PIN without echoing it. +/// +/// Checks `KEYCARD_PIN` first so non-interactive callers (CI, scripts) can +/// supply it via the environment. Falls back to a TTY prompt via `rpassword` +/// so the value never appears in argv, shell history, or `ps` output. +pub fn read_pin() -> Result> { + if let Ok(pin) = std::env::var("KEYCARD_PIN") { + return Ok(zeroize::Zeroizing::new(pin)); + } + rpassword::prompt_password("Keycard PIN: ") + .map(zeroize::Zeroizing::new) + .map_err(Into::into) +} + /// Resolve an account id-or-label pair to a `Privacy/id` string. /// /// Exactly one of `id` or `label` must be `Some`. If `id` is provided it is @@ -61,16 +75,16 @@ pub fn resolve_id_or_label( label: Option, labels: &HashMap, user_data: &NSSAUserData, - pin: &Option, - key_path: &Option, + key_path: Option<&str>, ) -> Result { - match (id, label, pin) { + match (id, label, key_path) { (Some(id), None, None) => Ok(id), (None, Some(label), None) => resolve_account_label(&label, labels, user_data), - (None, None, Some(pin)) => Ok(KeycardWallet::get_account_id_for_path_with_connect( - pin, - key_path.as_ref().expect("Expect a key path String."), - )), + (None, None, Some(key_path)) => { + let pin = read_pin()?; + KeycardWallet::get_account_id_for_path_with_connect(&pin, key_path) + .map_err(anyhow::Error::from) + } _ => anyhow::bail!("provide exactly one of account id, account label or keycard path"), } } diff --git a/wallet/src/lib.rs b/wallet/src/lib.rs index fdb83d94..59c7781d 100644 --- a/wallet/src/lib.rs +++ b/wallet/src/lib.rs @@ -68,6 +68,8 @@ pub enum ExecutionFailureKind { AccountDataError(AccountId), #[error("Failed to build transaction: {0}")] TransactionBuildError(#[from] nssa::error::NssaError), + #[error(transparent)] + KeycardError(#[from] pyo3::PyErr), } #[expect(clippy::partial_pub_fields, reason = "TODO: make all fields private")] diff --git a/wallet/src/program_facades/native_token_transfer/public.rs b/wallet/src/program_facades/native_token_transfer/public.rs index e7790897..b48d8a7d 100644 --- a/wallet/src/program_facades/native_token_transfer/public.rs +++ b/wallet/src/program_facades/native_token_transfer/public.rs @@ -5,6 +5,7 @@ use nssa::{ program::Program, public_transaction::{Message, WitnessSet}, }; +use pyo3::exceptions::PyRuntimeError; use sequencer_service_rpc::RpcClient as _; use super::NativeTokenTransfer; @@ -16,8 +17,8 @@ impl NativeTokenTransfer<'_> { from: AccountId, to: AccountId, balance_to_move: u128, - pin: &Option, - key_path: &Option, + from_key_path: Option<&str>, + to_key_path: Option<&str>, ) -> Result { let balance = self .0 @@ -32,46 +33,38 @@ impl NativeTokenTransfer<'_> { let account_ids = vec![from, to]; let program_id = Program::authenticated_transfer_program().id(); - // Fetch nonces for both accounts unconditionally - let mut nonces = self + let nonces = self .0 - .get_accounts_nonces(vec![from]) + .get_accounts_nonces(account_ids.clone()) .await .map_err(ExecutionFailureKind::SequencerError)?; - let to_signing_key = self.0.storage.user_data.get_pub_account_signing_key(to); - if let Some(_to_signing_key) = to_signing_key { - let to_nonces = self - .0 - .get_accounts_nonces(vec![to]) - .await - .map_err(ExecutionFailureKind::SequencerError)?; - nonces.extend(to_nonces); + + let message = Message::try_new(program_id, account_ids, nonces, balance_to_move) + .map_err(ExecutionFailureKind::TransactionBuildError)?; + + let witness_set = if let Some(from_key_path) = from_key_path { + let pin = crate::helperfunctions::read_pin().map_err(|e| { + ExecutionFailureKind::KeycardError(pyo3::PyErr::new::( + e.to_string(), + )) + })?; + let msg_hash = message.hash_message(); + let (from_sig, from_pk) = + KeycardWallet::sign_message_for_path_with_connect(&pin, from_key_path, &msg_hash)?; + if let Some(to_key_path) = to_key_path { + let (to_sig, to_pk) = KeycardWallet::sign_message_for_path_with_connect( + &pin, + to_key_path, + &msg_hash, + )?; + WitnessSet::from_list(&[from_sig, to_sig], &[from_pk, to_pk]) + } else { + WitnessSet::from_list(&[from_sig], &[from_pk]) + } } else { - println!( - "Receiver's account ({to}) private key not found in wallet. Proceeding with only sender's key." - ); - } - - let message = Message::try_new(program_id, account_ids, nonces, balance_to_move).unwrap(); - - let witness_set = pin.as_ref().map_or_else(|| { - let sign_ids = self.0.filter_owned_accounts(&[from, to]); - WalletCore::sign_public_message(self.0, &message, &sign_ids) - .expect("`WalletCore::sign_public_message() failed to produce a signature for a NativeTokenTransfer.") - }, |pin| { - let key_path = key_path.as_ref().expect("`NativeTokenTransfer::send_public_transfer() expected a String for `key_path`."); - let pub_key = KeycardWallet::get_public_key_for_path_with_connect( - pin, - key_path, - ); - let signature = KeycardWallet::sign_message_for_path_with_connect( - pin, - key_path, - &message.hash_message(), - ) - .expect("`NativeTokenTransfer::send_public_transfer() failed to produce a Signature for the given `pin` and `key_path`."); - WitnessSet::from_list(&[signature], &[pub_key]) - }); + let sign_ids = self.0.filter_owned_accounts(&[from, to]); + WalletCore::sign_public_message(self.0, &message, &sign_ids)? + }; let tx = PublicTransaction::new(message, witness_set); @@ -85,8 +78,7 @@ impl NativeTokenTransfer<'_> { pub async fn register_account( &self, from: AccountId, - pin: &Option, // Used by Keycard. - key_path: &Option, // Used by Keycard. + key_path: Option<&str>, ) -> Result { let nonces = self .0 @@ -98,9 +90,21 @@ impl NativeTokenTransfer<'_> { let account_ids = vec![from]; let program_id = Program::authenticated_transfer_program().id(); let message = Message::try_new(program_id, account_ids, nonces, instruction) - .expect("Expect a valid Message"); + .map_err(ExecutionFailureKind::TransactionBuildError)?; - let witness_set = if pin.is_none() { + let witness_set = if let Some(key_path) = key_path { + let pin = crate::helperfunctions::read_pin().map_err(|e| { + ExecutionFailureKind::KeycardError(pyo3::PyErr::new::( + e.to_string(), + )) + })?; + let (signature, pub_key) = KeycardWallet::sign_message_for_path_with_connect( + &pin, + key_path, + &message.hash_message(), + )?; + WitnessSet::from_list(&[signature], &[pub_key]) + } else { let signing_key = self.0.storage.user_data.get_pub_account_signing_key(from); let Some(signing_key) = signing_key else { @@ -108,19 +112,6 @@ impl NativeTokenTransfer<'_> { }; WitnessSet::for_message(&message, &[signing_key]) - } else { - let pub_key = KeycardWallet::get_public_key_for_path_with_connect( - pin.as_ref().expect("`wallet::program_facades::native_token_transfer::public::register_account`: invalid data received for pin for public key"), - key_path.as_ref().expect("`wallet::program_facades::native_token_transfer::public::register_account`: invalid data received for key_path for public_key"), - ); - - let signature = KeycardWallet::sign_message_for_path_with_connect( - pin.as_ref().as_ref().expect("`wallet::program_facades::native_token_transfer::public::register_account`: invalid data received for pin for signature"), - key_path.as_ref().expect("`wallet::program_facades::native_token_transfer::public::register_account`: invalid data received for key_path for public_key"), - &message.hash_message(), - ) - .expect("Expect a valid Signature."); - WitnessSet::from_list(&[signature], &[pub_key]) }; let tx = PublicTransaction::new(message, witness_set);