From b8e67de1676ad4eded81893cfdfa6864a7e673d9 Mon Sep 17 00:00:00 2001 From: Aleksandr Karbyshev Date: Fri, 12 Jan 2024 22:33:29 +0100 Subject: [PATCH 01/20] Fix: undo erroneous changes in wallet introduced in 0.28.0 rc1 --- crates/apps/src/lib/cli/wallet.rs | 15 ++++++--------- crates/sdk/src/wallet/mod.rs | 8 ++------ 2 files changed, 8 insertions(+), 15 deletions(-) diff --git a/crates/apps/src/lib/cli/wallet.rs b/crates/apps/src/lib/cli/wallet.rs index c6de0d6c5c..49dd193789 100644 --- a/crates/apps/src/lib/cli/wallet.rs +++ b/crates/apps/src/lib/cli/wallet.rs @@ -435,15 +435,12 @@ fn transparent_key_and_address_gen( edisplay_line!(io, "{}", err); cli::safe_exit(1) }); - let (_mnemonic, seed) = Wallet::::gen_hd_seed( - None, - &mut OsRng, - unsafe_dont_encrypt, - ) - .unwrap_or_else(|err| { - edisplay_line!(io, "{}", err); - cli::safe_exit(1) - }); + let (_mnemonic, seed) = + Wallet::::gen_hd_seed(None, &mut OsRng) + .unwrap_or_else(|err| { + edisplay_line!(io, "{}", err); + cli::safe_exit(1) + }); wallet.derive_store_hd_secret_key( scheme, Some(alias), diff --git a/crates/sdk/src/wallet/mod.rs b/crates/sdk/src/wallet/mod.rs index 04d6d13796..3e9f645212 100644 --- a/crates/sdk/src/wallet/mod.rs +++ b/crates/sdk/src/wallet/mod.rs @@ -614,7 +614,6 @@ impl Wallet { pub fn gen_hd_seed( passphrase: Option>, rng: &mut U::Rng, - unsafe_dont_encrypt: bool, ) -> Result<(Mnemonic, Seed), GenRestoreKeyError> { const MNEMONIC_TYPE: MnemonicType = MnemonicType::Words24; let mnemonic = U::generate_mnemonic_code(MNEMONIC_TYPE, rng)?; @@ -624,11 +623,8 @@ impl Wallet { ); println!("{}", mnemonic.clone().into_phrase()); - let passphrase = if unsafe_dont_encrypt { - Zeroizing::new(String::new()) - } else { - passphrase.unwrap_or_else(|| U::read_mnemonic_passphrase(true)) - }; + let passphrase = + passphrase.unwrap_or_else(|| U::read_mnemonic_passphrase(true)); let seed = Seed::new(&mnemonic, &passphrase); Ok((mnemonic, seed)) } From 0222bfbabdb2a8d2943b74267d43f37353fa2e11 Mon Sep 17 00:00:00 2001 From: Aleksandr Karbyshev Date: Mon, 15 Jan 2024 12:16:44 +0100 Subject: [PATCH 02/20] Purpose constant --- crates/sdk/src/wallet/derivation_path.rs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/crates/sdk/src/wallet/derivation_path.rs b/crates/sdk/src/wallet/derivation_path.rs index c1aba983bb..66b679a30d 100644 --- a/crates/sdk/src/wallet/derivation_path.rs +++ b/crates/sdk/src/wallet/derivation_path.rs @@ -45,8 +45,9 @@ impl DerivationPath { } fn bip44_base_indexes_for_scheme(scheme: SchemeType) -> Vec { + const BIP44_PURPOSE: u32 = 44; vec![ - ChildIndex::Hardened(44), + ChildIndex::Hardened(BIP44_PURPOSE), match scheme { SchemeType::Secp256k1 => ChildIndex::Hardened(ETH_COIN_TYPE), SchemeType::Ed25519 => ChildIndex::Hardened(NAMADA_COIN_TYPE), From d438cf7801ed46f93cee0aa7a648f69e669f9bab Mon Sep 17 00:00:00 2001 From: Aleksandr Karbyshev Date: Mon, 15 Jan 2024 12:32:43 +0100 Subject: [PATCH 03/20] Refactor: rename derivation path methods --- crates/apps/src/lib/cli/wallet.rs | 31 ++++++++------- crates/sdk/src/wallet/derivation_path.rs | 50 +++++++++++++++--------- crates/sdk/src/wallet/store.rs | 30 +++++++++----- 3 files changed, 68 insertions(+), 43 deletions(-) diff --git a/crates/apps/src/lib/cli/wallet.rs b/crates/apps/src/lib/cli/wallet.rs index 49dd193789..6c878747f8 100644 --- a/crates/apps/src/lib/cli/wallet.rs +++ b/crates/apps/src/lib/cli/wallet.rs @@ -284,16 +284,17 @@ fn shielded_key_address_add( } /// Decode the derivation path from the given string unless it is "default", -/// in which case use the default derivation path for the given scheme. -pub fn decode_derivation_path( +/// in which case use the default derivation path for the given transparent +/// scheme. +pub fn decode_transparent_derivation_path( scheme: SchemeType, derivation_path: String, ) -> Result { let is_default = derivation_path.eq_ignore_ascii_case("DEFAULT"); let parsed_derivation_path = if is_default { - DerivationPath::default_for_scheme(scheme) + DerivationPath::default_for_transparent_scheme(scheme) } else { - DerivationPath::from_path_str(scheme, &derivation_path)? + DerivationPath::from_path_string_for_scheme(scheme, &derivation_path)? }; if !parsed_derivation_path.is_compatible(scheme) { println!( @@ -321,11 +322,12 @@ async fn transparent_key_and_address_derive( }: args::KeyDerive, ) { let mut wallet = load_wallet(ctx); - let derivation_path = decode_derivation_path(scheme, derivation_path) - .unwrap_or_else(|err| { - edisplay_line!(io, "{}", err); - cli::safe_exit(1) - }); + let derivation_path = + decode_transparent_derivation_path(scheme, derivation_path) + .unwrap_or_else(|err| { + edisplay_line!(io, "{}", err); + cli::safe_exit(1) + }); let alias = alias.to_lowercase(); let alias = if !use_device { let encryption_password = @@ -430,11 +432,12 @@ fn transparent_key_and_address_gen( &mut OsRng, ) } else { - let derivation_path = decode_derivation_path(scheme, derivation_path) - .unwrap_or_else(|err| { - edisplay_line!(io, "{}", err); - cli::safe_exit(1) - }); + let derivation_path = + decode_transparent_derivation_path(scheme, derivation_path) + .unwrap_or_else(|err| { + edisplay_line!(io, "{}", err); + cli::safe_exit(1) + }); let (_mnemonic, seed) = Wallet::::gen_hd_seed(None, &mut OsRng) .unwrap_or_else(|err| { diff --git a/crates/sdk/src/wallet/derivation_path.rs b/crates/sdk/src/wallet/derivation_path.rs index 66b679a30d..2aaac91968 100644 --- a/crates/sdk/src/wallet/derivation_path.rs +++ b/crates/sdk/src/wallet/derivation_path.rs @@ -81,19 +81,23 @@ impl DerivationPath { ) } - pub fn default_for_scheme(scheme: SchemeType) -> Self { + pub fn default_for_transparent_scheme(scheme: SchemeType) -> Self { let path = Self::bip44(scheme, 0, 0, 0); path.hardened(scheme) } - pub fn from_path_str( - scheme: SchemeType, - path: &str, - ) -> Result { + pub fn from_path_string(path: &str) -> Result { let inner = DerivationPathInner::from_str(path).map_err(|err| { DerivationPathError::InvalidDerivationPath(err.to_string()) })?; - Ok(Self(inner).hardened(scheme)) + Ok(Self(inner)) + } + + pub fn from_path_string_for_scheme( + scheme: SchemeType, + path: &str, + ) -> Result { + Self::from_path_string(path).map(|dp| dp.hardened(scheme)) } pub fn path(&self) -> &[ChildIndex] { @@ -144,21 +148,25 @@ mod tests { #[test] fn path_is_compatible() { - let path_empty = - DerivationPath::from_path_str(SchemeType::Secp256k1, "m") - .expect("Path construction cannot fail."); + let path_empty = DerivationPath::from_path_string_for_scheme( + SchemeType::Secp256k1, + "m", + ) + .expect("Path construction cannot fail."); assert!(path_empty.is_compatible(SchemeType::Ed25519)); assert!(path_empty.is_compatible(SchemeType::Secp256k1)); assert!(path_empty.is_compatible(SchemeType::Common)); - let path_one = - DerivationPath::from_path_str(SchemeType::Secp256k1, "m/44'") - .expect("Path construction cannot fail."); + let path_one = DerivationPath::from_path_string_for_scheme( + SchemeType::Secp256k1, + "m/44'", + ) + .expect("Path construction cannot fail."); assert!(path_one.is_compatible(SchemeType::Ed25519)); assert!(path_one.is_compatible(SchemeType::Secp256k1)); assert!(path_one.is_compatible(SchemeType::Common)); - let path_two = DerivationPath::from_path_str( + let path_two = DerivationPath::from_path_string_for_scheme( SchemeType::Secp256k1, "m/44'/99999'", ) @@ -167,16 +175,20 @@ mod tests { assert!(!path_two.is_compatible(SchemeType::Secp256k1)); assert!(path_two.is_compatible(SchemeType::Common)); - let path_eth = - DerivationPath::from_path_str(SchemeType::Secp256k1, "m/44'/60'") - .expect("Path construction cannot fail."); + let path_eth = DerivationPath::from_path_string_for_scheme( + SchemeType::Secp256k1, + "m/44'/60'", + ) + .expect("Path construction cannot fail."); assert!(!path_eth.is_compatible(SchemeType::Ed25519)); assert!(path_eth.is_compatible(SchemeType::Secp256k1)); assert!(path_eth.is_compatible(SchemeType::Common)); - let path_nam = - DerivationPath::from_path_str(SchemeType::Ed25519, "m/44'/877'") - .expect("Path construction cannot fail."); + let path_nam = DerivationPath::from_path_string_for_scheme( + SchemeType::Ed25519, + "m/44'/877'", + ) + .expect("Path construction cannot fail."); assert!(path_nam.is_compatible(SchemeType::Ed25519)); assert!(!path_nam.is_compatible(SchemeType::Secp256k1)); assert!(path_nam.is_compatible(SchemeType::Common)); diff --git a/crates/sdk/src/wallet/store.rs b/crates/sdk/src/wallet/store.rs index f8a370b347..408e7b6ea9 100644 --- a/crates/sdk/src/wallet/store.rs +++ b/crates/sdk/src/wallet/store.rs @@ -799,9 +799,11 @@ mod test_wallet { let seed = Seed::new(&mnemonic, PASSPHRASE); assert_eq!(format!("{:x}", seed), SEED_EXPECTED); - let derivation_path = - DerivationPath::from_path_str(SCHEME, DERIVATION_PATH) - .expect("Derivation path construction cannot fail"); + let derivation_path = DerivationPath::from_path_string_for_scheme( + SCHEME, + DERIVATION_PATH, + ) + .expect("Derivation path construction cannot fail"); let sk = derive_hd_secret_key(SCHEME, seed.as_bytes(), derivation_path); @@ -825,13 +827,18 @@ mod test_wallet { let seed = Seed::new(&mnemonic, PASSPHRASE); assert_eq!(format!("{:x}", seed), SEED_EXPECTED); - let derivation_path = - DerivationPath::from_path_str(SCHEME, DERIVATION_PATH) - .expect("Derivation path construction cannot fail"); + let derivation_path = DerivationPath::from_path_string_for_scheme( + SCHEME, + DERIVATION_PATH, + ) + .expect("Derivation path construction cannot fail"); let derivation_path_hardened = - DerivationPath::from_path_str(SCHEME, DERIVATION_PATH_HARDENED) - .expect("Derivation path construction cannot fail"); + DerivationPath::from_path_string_for_scheme( + SCHEME, + DERIVATION_PATH_HARDENED, + ) + .expect("Derivation path construction cannot fail"); let sk = derive_hd_secret_key(SCHEME, seed.as_bytes(), derivation_path); @@ -857,8 +864,11 @@ mod test_wallet { .decode(seed.as_bytes()) .expect("Seed parsing cannot fail.") .as_slice(), - DerivationPath::from_path_str(scheme, derivation_path) - .expect("Derivation path construction cannot fail"), + DerivationPath::from_path_string_for_scheme( + scheme, + derivation_path, + ) + .expect("Derivation path construction cannot fail"), ); let sk_expected = if priv_key.starts_with("xprv") { // this is an extended private key encoded in base58 From e492d4856de9b77cd12fb5818f62dda09f8cfa4a Mon Sep 17 00:00:00 2001 From: Aleksandr Karbyshev Date: Mon, 15 Jan 2024 12:33:01 +0100 Subject: [PATCH 04/20] Add derivation paths for zip32 --- crates/sdk/src/wallet/derivation_path.rs | 32 ++++++++++++++++++++++++ 1 file changed, 32 insertions(+) diff --git a/crates/sdk/src/wallet/derivation_path.rs b/crates/sdk/src/wallet/derivation_path.rs index 2aaac91968..8938a3b9a7 100644 --- a/crates/sdk/src/wallet/derivation_path.rs +++ b/crates/sdk/src/wallet/derivation_path.rs @@ -2,6 +2,7 @@ use core::fmt; use std::str::FromStr; use derivation_path::{ChildIndex, DerivationPath as DerivationPathInner}; +use masp_primitives::zip32; use namada_core::types::key::SchemeType; use serde::{Deserialize, Deserializer, Serialize, Serializer}; use thiserror::Error; @@ -69,6 +70,21 @@ impl DerivationPath { Self::new(indexes) } + /// Key path according to zip-0032 + /// https://zips.z.cash/zip-0032#sapling-key-path + fn zip32(account: u32, address: Option) -> Self { + const ZIP32_PURPOSE: u32 = 32; + let mut indexes = vec![ + ChildIndex::Hardened(ZIP32_PURPOSE), + ChildIndex::Hardened(NAMADA_COIN_TYPE), + ChildIndex::Hardened(account), + ]; + if let Some(address) = address { + indexes.push(ChildIndex::Normal(address)); + } + Self::new(indexes) + } + fn hardened(&self, scheme: SchemeType) -> Self { Self::new( self.0 @@ -86,6 +102,10 @@ impl DerivationPath { path.hardened(scheme) } + pub fn default_for_shielded() -> Self { + Self::zip32(0, None) + } + pub fn from_path_string(path: &str) -> Result { let inner = DerivationPathInner::from_str(path).map_err(|err| { DerivationPathError::InvalidDerivationPath(err.to_string()) @@ -140,6 +160,18 @@ impl IntoHDeriveDerivationPath for DerivationPath { } } +impl From for Vec { + fn from(path: DerivationPath) -> Vec { + path.0 + .into_iter() + .map(|idx| match idx { + ChildIndex::Normal(idx) => zip32::ChildIndex::NonHardened(*idx), + ChildIndex::Hardened(idx) => zip32::ChildIndex::Hardened(*idx), + }) + .collect() + } +} + #[cfg(test)] mod tests { use namada_core::types::key::SchemeType; From e2cd7b2175efc86a0d300c31e29a149505f55841 Mon Sep 17 00:00:00 2001 From: Aleksandr Karbyshev Date: Mon, 15 Jan 2024 12:33:15 +0100 Subject: [PATCH 05/20] Fix zip32 import --- crates/sdk/src/wallet/store.rs | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/crates/sdk/src/wallet/store.rs b/crates/sdk/src/wallet/store.rs index 408e7b6ea9..55f746895e 100644 --- a/crates/sdk/src/wallet/store.rs +++ b/crates/sdk/src/wallet/store.rs @@ -6,7 +6,7 @@ use std::str::FromStr; use bimap::BiBTreeMap; use itertools::Itertools; -use masp_primitives::zip32::ExtendedFullViewingKey; +use masp_primitives::zip32; use namada_core::types::address::{Address, ImplicitAddress}; use namada_core::types::key::*; use namada_core::types::masp::{ @@ -394,7 +394,8 @@ impl Store { StoredKeypair::new(spendkey, password); self.spend_keys.insert(alias.clone(), spendkey_to_store); // Simultaneously add the derived viewing key to ease balance viewing - let viewkey = ExtendedFullViewingKey::from(&spendkey.into()).into(); + let viewkey = + zip32::ExtendedFullViewingKey::from(&spendkey.into()).into(); self.view_keys.insert(alias.clone(), viewkey); Some(alias) } From 8f498c4a08b6106d980e49f63ce71177cc82512e Mon Sep 17 00:00:00 2001 From: Aleksandr Karbyshev Date: Mon, 15 Jan 2024 12:36:27 +0100 Subject: [PATCH 06/20] Rename key gen error struct --- crates/apps/src/lib/wallet/mod.rs | 8 ++++---- crates/sdk/src/wallet/mod.rs | 20 ++++++++++---------- 2 files changed, 14 insertions(+), 14 deletions(-) diff --git a/crates/apps/src/lib/wallet/mod.rs b/crates/apps/src/lib/wallet/mod.rs index 6696f7ee3a..2dada40035 100644 --- a/crates/apps/src/lib/wallet/mod.rs +++ b/crates/apps/src/lib/wallet/mod.rs @@ -12,7 +12,7 @@ pub use namada_sdk::wallet::alias::Alias; use namada_sdk::wallet::fs::FsWalletStorage; use namada_sdk::wallet::store::Store; use namada_sdk::wallet::{ - ConfirmationResponse, FindKeyError, GenRestoreKeyError, Wallet, WalletIo, + ConfirmationResponse, FindKeyError, GenDeriveKeyError, Wallet, WalletIo, }; pub use namada_sdk::wallet::{ValidatorData, ValidatorKeys}; use rand_core::OsRng; @@ -85,11 +85,11 @@ impl WalletIo for CliWalletUtils { alias.trim().to_owned() } - fn read_mnemonic_code() -> Result { + fn read_mnemonic_code() -> Result { let phrase = get_secure_user_input("Input mnemonic code: ") - .map_err(|_| GenRestoreKeyError::MnemonicInputError)?; + .map_err(|_| GenDeriveKeyError::MnemonicInputError)?; Mnemonic::from_phrase(phrase.as_ref(), Language::English) - .map_err(|_| GenRestoreKeyError::MnemonicInputError) + .map_err(|_| GenDeriveKeyError::MnemonicInputError) } fn read_mnemonic_passphrase(confirm: bool) -> Zeroizing { diff --git a/crates/sdk/src/wallet/mod.rs b/crates/sdk/src/wallet/mod.rs index 3e9f645212..f8179c4761 100644 --- a/crates/sdk/src/wallet/mod.rs +++ b/crates/sdk/src/wallet/mod.rs @@ -29,9 +29,9 @@ pub use self::keys::{DecryptionError, StoredKeypair}; pub use self::store::{ConfirmationResponse, ValidatorData, ValidatorKeys}; use crate::wallet::store::derive_hd_secret_key; -/// Errors of key generation / recovery +/// Errors of key generation / derivation #[derive(Error, Debug)] -pub enum GenRestoreKeyError { +pub enum GenDeriveKeyError { /// Derivation path parse error #[error("Derivation path parse error")] DerivationPathError(DerivationPathError), @@ -55,7 +55,7 @@ pub trait WalletIo: Sized + Clone { fn generate_mnemonic_code( mnemonic_type: MnemonicType, rng: &mut Self::Rng, - ) -> Result { + ) -> Result { const BITS_PER_BYTE: usize = 8; // generate random mnemonic @@ -79,7 +79,7 @@ pub trait WalletIo: Sized + Clone { } /// Read mnemonic code from the file/env/stdin. - fn read_mnemonic_code() -> Result { + fn read_mnemonic_code() -> Result { panic!("attempted to prompt for alias in non-interactive mode"); } @@ -537,7 +537,7 @@ impl Wallet { derivation_path: DerivationPath, mnemonic_passphrase: Option<(Mnemonic, Zeroizing)>, password: Option>, - ) -> Result<(String, common::SecretKey), GenRestoreKeyError> { + ) -> Result<(String, common::SecretKey), GenDeriveKeyError> { let (mnemonic, passphrase) = if let Some(mnemonic_passphrase) = mnemonic_passphrase { mnemonic_passphrase @@ -596,7 +596,7 @@ impl Wallet { alias_force: bool, password: Option>, rng: &mut (impl CryptoRng + RngCore), - ) -> Result<(String, common::SecretKey), GenRestoreKeyError> { + ) -> Result<(String, common::SecretKey), GenDeriveKeyError> { let sk = gen_secret_key(scheme, rng); self.insert_keypair( alias.unwrap_or_default(), @@ -614,7 +614,7 @@ impl Wallet { pub fn gen_hd_seed( passphrase: Option>, rng: &mut U::Rng, - ) -> Result<(Mnemonic, Seed), GenRestoreKeyError> { + ) -> Result<(Mnemonic, Seed), GenDeriveKeyError> { const MNEMONIC_TYPE: MnemonicType = MnemonicType::Words24; let mnemonic = U::generate_mnemonic_code(MNEMONIC_TYPE, rng)?; println!( @@ -646,7 +646,7 @@ impl Wallet { seed: Seed, derivation_path: DerivationPath, password: Option>, - ) -> Result<(String, common::SecretKey), GenRestoreKeyError> { + ) -> Result<(String, common::SecretKey), GenDeriveKeyError> { let sk = derive_hd_secret_key( scheme, seed.as_bytes(), @@ -894,7 +894,7 @@ impl Wallet { password: Option>, address: Option
, path: Option, - ) -> Result { + ) -> Result { self.store .insert_keypair::( alias.into(), @@ -909,7 +909,7 @@ impl Wallet { self.decrypted_key_cache.insert(alias.clone(), sk); alias.into() }) - .ok_or(GenRestoreKeyError::KeyStorageError) + .ok_or(GenDeriveKeyError::KeyStorageError) } /// Insert a new public key with the given alias. If the alias is already From 4d18b5ad0604ea52c94cb9a5ff8a2d9629ef9a08 Mon Sep 17 00:00:00 2001 From: Aleksandr Karbyshev Date: Mon, 15 Jan 2024 12:39:55 +0100 Subject: [PATCH 07/20] Add doc comment --- crates/sdk/src/wallet/mod.rs | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/crates/sdk/src/wallet/mod.rs b/crates/sdk/src/wallet/mod.rs index f8179c4761..2a623babf7 100644 --- a/crates/sdk/src/wallet/mod.rs +++ b/crates/sdk/src/wallet/mod.rs @@ -886,6 +886,11 @@ impl Wallet { .map(Into::into) } + /// Add a new keypair with the given alias. If the alias is already used, + /// will ask whether the existing alias should be replaced, a different + /// alias is desired, or the alias creation should be cancelled. Return + /// the chosen alias if the keypair has been added, otherwise return + /// nothing. pub fn insert_keypair( &mut self, alias: String, From 98228eae5c9f2666dc9581633b19c8ed999333c8 Mon Sep 17 00:00:00 2001 From: Aleksandr Karbyshev Date: Mon, 15 Jan 2024 13:11:34 +0100 Subject: [PATCH 08/20] Implement zip32 key generation in wallet --- crates/apps/src/lib/cli/wallet.rs | 52 ++++++++++++++++++++++++++++--- crates/sdk/src/wallet/mod.rs | 37 ++++++++++++++++------ crates/sdk/src/wallet/store.rs | 14 +++++++++ 3 files changed, 89 insertions(+), 14 deletions(-) diff --git a/crates/apps/src/lib/cli/wallet.rs b/crates/apps/src/lib/cli/wallet.rs index 6c878747f8..573679d968 100644 --- a/crates/apps/src/lib/cli/wallet.rs +++ b/crates/apps/src/lib/cli/wallet.rs @@ -170,21 +170,50 @@ fn payment_addresses_list( } /// Generate a spending key. -fn spending_key_gen( +fn shielded_key_gen( ctx: Context, io: &impl Io, args::KeyGen { + raw, alias, alias_force, unsafe_dont_encrypt, + derivation_path, .. }: args::KeyGen, ) { let mut wallet = load_wallet(ctx); let alias = alias.to_lowercase(); let password = read_and_confirm_encryption_password(unsafe_dont_encrypt); - let (alias, _key) = - wallet.gen_store_spending_key(alias, password, alias_force, &mut OsRng); + let alias = if raw { + wallet.gen_store_spending_key(alias, password, alias_force, &mut OsRng) + } else { + let derivation_path = decode_shielded_derivation_path(derivation_path) + .unwrap_or_else(|err| { + edisplay_line!(io, "{}", err); + cli::safe_exit(1) + }); + let (_mnemonic, seed) = + Wallet::::gen_hd_seed(None, &mut OsRng) + .unwrap_or_else(|err| { + edisplay_line!(io, "{}", err); + cli::safe_exit(1) + }); + wallet.derive_store_hd_spendind_key( + alias, + alias_force, + seed, + derivation_path, + password, + ) + } + .map(|x| x.0) + .unwrap_or_else(|| { + eprintln!("Failed to generate a key."); + println!("No changes are persisted. Exiting."); + cli::safe_exit(0); + }); + wallet .save() .unwrap_or_else(|err| edisplay_line!(io, "{}", err)); @@ -306,6 +335,21 @@ pub fn decode_transparent_derivation_path( Ok(parsed_derivation_path) } +/// Decode the derivation path from the given string unless it is "default", +/// in which case use the default derivation path for the shielded setting. +pub fn decode_shielded_derivation_path( + derivation_path: String, +) -> Result { + let is_default = derivation_path.eq_ignore_ascii_case("DEFAULT"); + let parsed_derivation_path = if is_default { + DerivationPath::default_for_shielded() + } else { + DerivationPath::from_path_string(&derivation_path)? + }; + println!("Using HD derivation path {}", parsed_derivation_path); + Ok(parsed_derivation_path) +} + /// Derives a keypair and an implicit address from the mnemonic code in the /// wallet. async fn transparent_key_and_address_derive( @@ -474,7 +518,7 @@ fn key_gen(ctx: Context, io: &impl Io, args_key_gen: args::KeyGen) { if !args_key_gen.shielded { transparent_key_and_address_gen(ctx, io, args_key_gen) } else { - spending_key_gen(ctx, io, args_key_gen) + shielded_key_gen(ctx, io, args_key_gen) } } diff --git a/crates/sdk/src/wallet/mod.rs b/crates/sdk/src/wallet/mod.rs index 2a623babf7..a783886037 100644 --- a/crates/sdk/src/wallet/mod.rs +++ b/crates/sdk/src/wallet/mod.rs @@ -27,7 +27,7 @@ use zeroize::Zeroizing; pub use self::derivation_path::{DerivationPath, DerivationPathError}; pub use self::keys::{DecryptionError, StoredKeypair}; pub use self::store::{ConfirmationResponse, ValidatorData, ValidatorKeys}; -use crate::wallet::store::derive_hd_secret_key; +use crate::wallet::store::{derive_hd_secret_key, derive_hd_spending_key}; /// Errors of key generation / derivation #[derive(Error, Debug)] @@ -569,15 +569,10 @@ impl Wallet { password: Option>, force_alias: bool, csprng: &mut (impl CryptoRng + RngCore), - ) -> (String, ExtendedSpendingKey) { - let spendkey = gen_spending_key(csprng); - if let Some(alias) = - self.insert_spending_key(alias, spendkey, password, force_alias) - { - (alias, spendkey) - } else { - panic!("Action cancelled, no changes persisted."); - } + ) -> Option<(String, ExtendedSpendingKey)> { + let spend_key = gen_spending_key(csprng); + self.insert_spending_key(alias, spend_key, password, force_alias) + .map(|alias| (alias, spend_key)) } /// Generate a new keypair, derive an implicit address from its public key @@ -663,6 +658,28 @@ impl Wallet { .map(|alias| (alias, sk)) } + /// Derive a masp shielded key from the given seed and path, and insert it + /// into the store with the provided alias, converted to lower case. If the + /// alias already exists, optionally force overwrite the key for the + /// alias. + /// If no encryption password is provided, the key will be stored raw + /// without encryption. + /// Stores the key in decrypted key cache and returns the alias of the key + /// and the key itself. + pub fn derive_store_hd_spendind_key( + &mut self, + alias: String, + force_alias: bool, + seed: Seed, + derivation_path: DerivationPath, + password: Option>, + ) -> Option<(String, ExtendedSpendingKey)> { + let spend_key = + derive_hd_spending_key(seed.as_bytes(), derivation_path); + self.insert_spending_key(alias, spend_key, password, force_alias) + .map(|alias| (alias, spend_key)) + } + /// Generate a disposable signing key for fee payment and store it under the /// precomputed alias in the wallet. This is simply a wrapper around /// `gen_key` to manage the alias diff --git a/crates/sdk/src/wallet/store.rs b/crates/sdk/src/wallet/store.rs index 55f746895e..72ef05e940 100644 --- a/crates/sdk/src/wallet/store.rs +++ b/crates/sdk/src/wallet/store.rs @@ -733,6 +733,20 @@ pub fn derive_hd_secret_key( } } +/// Generate a new spending key from the seed. +pub fn derive_hd_spending_key( + seed: &[u8], + derivation_path: DerivationPath, +) -> ExtendedSpendingKey { + let master_spend_key = zip32::sapling::ExtendedSpendingKey::master(seed); + let zip32_path: Vec = derivation_path.into(); + zip32::sapling::ExtendedSpendingKey::from_path( + &master_spend_key, + &zip32_path, + ) + .into() +} + impl Display for AddressVpType { fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { match self { From 59210313c9c24c9c33c4bb1408613f1f4062a45f Mon Sep 17 00:00:00 2001 From: Aleksandr Karbyshev Date: Tue, 16 Jan 2024 15:37:47 +0100 Subject: [PATCH 09/20] Refactor: rename function --- crates/apps/src/lib/cli/wallet.rs | 2 +- crates/sdk/src/wallet/mod.rs | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/crates/apps/src/lib/cli/wallet.rs b/crates/apps/src/lib/cli/wallet.rs index 573679d968..4dd0840c01 100644 --- a/crates/apps/src/lib/cli/wallet.rs +++ b/crates/apps/src/lib/cli/wallet.rs @@ -377,7 +377,7 @@ async fn transparent_key_and_address_derive( let encryption_password = read_and_confirm_encryption_password(unsafe_dont_encrypt); wallet - .derive_key_from_mnemonic_code( + .derive_store_key_from_mnemonic_code( scheme, Some(alias), alias_force, diff --git a/crates/sdk/src/wallet/mod.rs b/crates/sdk/src/wallet/mod.rs index a783886037..32b2ba6fe4 100644 --- a/crates/sdk/src/wallet/mod.rs +++ b/crates/sdk/src/wallet/mod.rs @@ -529,7 +529,7 @@ impl Wallet { /// provided, will prompt for password from stdin. /// Stores the key in decrypted key cache and returns the alias of the key /// and a reference-counting pointer to the key. - pub fn derive_key_from_mnemonic_code( + pub fn derive_store_key_from_mnemonic_code( &mut self, scheme: SchemeType, alias: Option, From 84c4aad940cf6cf40cca8b635c2ffa349b88f4f5 Mon Sep 17 00:00:00 2001 From: Aleksandr Karbyshev Date: Tue, 16 Jan 2024 15:38:33 +0100 Subject: [PATCH 10/20] Derive and store maps spending key in the wallet --- crates/apps/src/lib/cli/wallet.rs | 54 ++++++++++++++++++++++++++++++- crates/sdk/src/wallet/mod.rs | 32 ++++++++++++++++++ 2 files changed, 85 insertions(+), 1 deletion(-) diff --git a/crates/apps/src/lib/cli/wallet.rs b/crates/apps/src/lib/cli/wallet.rs index 4dd0840c01..d9c1c9556b 100644 --- a/crates/apps/src/lib/cli/wallet.rs +++ b/crates/apps/src/lib/cli/wallet.rs @@ -169,6 +169,58 @@ fn payment_addresses_list( } } +/// Derives a masp spending key from the mnemonic code in the wallet. +fn shielded_key_derive( + ctx: Context, + io: &impl Io, + args::KeyDerive { + alias, + alias_force, + unsafe_dont_encrypt, + derivation_path, + use_device, + .. + }: args::KeyDerive, +) { + let mut wallet = load_wallet(ctx); + let derivation_path = decode_shielded_derivation_path(derivation_path) + .unwrap_or_else(|err| { + edisplay_line!(io, "{}", err); + cli::safe_exit(1) + }); + let alias = alias.to_lowercase(); + let alias = if !use_device { + let encryption_password = + read_and_confirm_encryption_password(unsafe_dont_encrypt); + wallet + .derive_store_spending_key_from_mnemonic_code( + alias, + alias_force, + derivation_path, + None, + encryption_password, + ) + .unwrap_or_else(|| { + edisplay_line!(io, "Failed to derive a key."); + display_line!(io, "No changes are persisted. Exiting."); + cli::safe_exit(1) + }) + .0 + } else { + display_line!(io, "Not implemented."); + display_line!(io, "No changes are persisted. Exiting."); + cli::safe_exit(1) + }; + wallet + .save() + .unwrap_or_else(|err| edisplay_line!(io, "{}", err)); + display_line!( + io, + "Successfully added a key and an address with alias: \"{}\"", + alias + ); +} + /// Generate a spending key. fn shielded_key_gen( ctx: Context, @@ -531,7 +583,7 @@ async fn key_derive( if !args_key_derive.shielded { transparent_key_and_address_derive(ctx, io, args_key_derive).await } else { - todo!() + shielded_key_derive(ctx, io, args_key_derive) } } diff --git a/crates/sdk/src/wallet/mod.rs b/crates/sdk/src/wallet/mod.rs index 32b2ba6fe4..3708e2f837 100644 --- a/crates/sdk/src/wallet/mod.rs +++ b/crates/sdk/src/wallet/mod.rs @@ -520,6 +520,38 @@ impl Wallet { } impl Wallet { + /// Restore a spending key from the user mnemonic code (read from stdin) + /// using a given ZIP32 derivation path and insert it into the store with + /// the provided alias, converted to lower case. + /// The key is encrypted with the provided password. If no password + /// provided, will prompt for password from stdin. + /// Stores the key in decrypted key cache and returns the alias of the key + /// and a reference-counting pointer to the key. + pub fn derive_store_spending_key_from_mnemonic_code( + &mut self, + alias: String, + alias_force: bool, + derivation_path: DerivationPath, + mnemonic_passphrase: Option<(Mnemonic, Zeroizing)>, + password: Option>, + ) -> Option<(String, ExtendedSpendingKey)> { + let (mnemonic, passphrase) = + if let Some(mnemonic_passphrase) = mnemonic_passphrase { + mnemonic_passphrase + } else { + ( + U::read_mnemonic_code().ok()?, + U::read_mnemonic_passphrase(false), + ) + }; + let seed = Seed::new(&mnemonic, &passphrase); + let spend_key = + derive_hd_spending_key(seed.as_bytes(), derivation_path); + + self.insert_spending_key(alias, spend_key, password, alias_force) + .map(|alias| (alias, spend_key)) + } + /// Restore a keypair from the user mnemonic code (read from stdin) using /// a given BIP44 derivation path and derive an implicit address from its /// public part and insert them into the store with the provided alias, From 9a9340f94a0a72ee5bfed5ca0d79275837dc035a Mon Sep 17 00:00:00 2001 From: Aleksandr Karbyshev Date: Tue, 16 Jan 2024 15:39:49 +0100 Subject: [PATCH 11/20] Fix return code --- crates/apps/src/lib/cli/wallet.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/apps/src/lib/cli/wallet.rs b/crates/apps/src/lib/cli/wallet.rs index d9c1c9556b..8faa318621 100644 --- a/crates/apps/src/lib/cli/wallet.rs +++ b/crates/apps/src/lib/cli/wallet.rs @@ -263,7 +263,7 @@ fn shielded_key_gen( .unwrap_or_else(|| { eprintln!("Failed to generate a key."); println!("No changes are persisted. Exiting."); - cli::safe_exit(0); + cli::safe_exit(1); }); wallet From 9eff7ea8d32a692fc0275389d2d4f0596769b382 Mon Sep 17 00:00:00 2001 From: Aleksandr Karbyshev Date: Tue, 16 Jan 2024 16:07:44 +0100 Subject: [PATCH 12/20] Store derivation path when adding zip32 key --- crates/apps/src/lib/cli/wallet.rs | 8 +++++++- crates/sdk/src/wallet/mod.rs | 30 ++++++++++++++++++++++-------- crates/sdk/src/wallet/store.rs | 7 +++++-- 3 files changed, 34 insertions(+), 11 deletions(-) diff --git a/crates/apps/src/lib/cli/wallet.rs b/crates/apps/src/lib/cli/wallet.rs index 8faa318621..230347d57a 100644 --- a/crates/apps/src/lib/cli/wallet.rs +++ b/crates/apps/src/lib/cli/wallet.rs @@ -338,7 +338,13 @@ fn shielded_key_address_add( let password = read_and_confirm_encryption_password(unsafe_dont_encrypt); let alias = wallet - .insert_spending_key(alias, spending_key, password, alias_force) + .insert_spending_key( + alias, + alias_force, + spending_key, + password, + None, + ) .unwrap_or_else(|| { edisplay_line!(io, "Spending key not added"); cli::safe_exit(1); diff --git a/crates/sdk/src/wallet/mod.rs b/crates/sdk/src/wallet/mod.rs index 3708e2f837..f892b3c8f8 100644 --- a/crates/sdk/src/wallet/mod.rs +++ b/crates/sdk/src/wallet/mod.rs @@ -546,10 +546,16 @@ impl Wallet { }; let seed = Seed::new(&mnemonic, &passphrase); let spend_key = - derive_hd_spending_key(seed.as_bytes(), derivation_path); + derive_hd_spending_key(seed.as_bytes(), derivation_path.clone()); - self.insert_spending_key(alias, spend_key, password, alias_force) - .map(|alias| (alias, spend_key)) + self.insert_spending_key( + alias, + alias_force, + spend_key, + password, + Some(derivation_path), + ) + .map(|alias| (alias, spend_key)) } /// Restore a keypair from the user mnemonic code (read from stdin) using @@ -603,7 +609,7 @@ impl Wallet { csprng: &mut (impl CryptoRng + RngCore), ) -> Option<(String, ExtendedSpendingKey)> { let spend_key = gen_spending_key(csprng); - self.insert_spending_key(alias, spend_key, password, force_alias) + self.insert_spending_key(alias, force_alias, spend_key, password, None) .map(|alias| (alias, spend_key)) } @@ -707,9 +713,15 @@ impl Wallet { password: Option>, ) -> Option<(String, ExtendedSpendingKey)> { let spend_key = - derive_hd_spending_key(seed.as_bytes(), derivation_path); - self.insert_spending_key(alias, spend_key, password, force_alias) - .map(|alias| (alias, spend_key)) + derive_hd_spending_key(seed.as_bytes(), derivation_path.clone()); + self.insert_spending_key( + alias, + force_alias, + spend_key, + password, + Some(derivation_path), + ) + .map(|alias| (alias, spend_key)) } /// Generate a disposable signing key for fee payment and store it under the @@ -1003,15 +1015,17 @@ impl Wallet { pub fn insert_spending_key( &mut self, alias: String, + force_alias: bool, spend_key: ExtendedSpendingKey, password: Option>, - force_alias: bool, + path: Option, ) -> Option { self.store .insert_spending_key::( alias.into(), spend_key, password, + path, force_alias, ) .map(|alias| { diff --git a/crates/sdk/src/wallet/store.rs b/crates/sdk/src/wallet/store.rs index 72ef05e940..df650b5316 100644 --- a/crates/sdk/src/wallet/store.rs +++ b/crates/sdk/src/wallet/store.rs @@ -366,6 +366,7 @@ impl Store { alias: Alias, spendkey: ExtendedSpendingKey, password: Option>, + path: Option, force: bool, ) -> Option { // abort if the alias is reserved @@ -373,17 +374,18 @@ impl Store { println!("The alias {} is reserved", alias); return None; } - + // abort if the alias is empty if alias.is_empty() { eprintln!("Empty alias given."); return None; } + if self.contains_alias(&alias) && !force { match U::show_overwrite_confirmation(&alias, "a spending key") { ConfirmationResponse::Replace => {} ConfirmationResponse::Reselect(new_alias) => { return self.insert_spending_key::( - new_alias, spendkey, password, false, + new_alias, spendkey, password, path, false, ); } ConfirmationResponse::Skip => return None, @@ -397,6 +399,7 @@ impl Store { let viewkey = zip32::ExtendedFullViewingKey::from(&spendkey.into()).into(); self.view_keys.insert(alias.clone(), viewkey); + path.map(|p| self.derivation_paths.insert(alias.clone(), p)); Some(alias) } From 6568af0f7f130bdc65fa6c08ea81ef12d3b56339 Mon Sep 17 00:00:00 2001 From: Aleksandr Karbyshev Date: Tue, 16 Jan 2024 18:24:58 +0100 Subject: [PATCH 13/20] Simplify some SDK interfaces --- crates/apps/src/lib/cli/wallet.rs | 24 ++++++---------- crates/apps/src/lib/client/tx.rs | 4 ++- crates/apps/src/lib/wallet/mod.rs | 19 +++++++------ crates/sdk/src/wallet/mod.rs | 47 ++++++++----------------------- crates/tests/src/e2e/setup.rs | 2 +- 5 files changed, 34 insertions(+), 62 deletions(-) diff --git a/crates/apps/src/lib/cli/wallet.rs b/crates/apps/src/lib/cli/wallet.rs index 230347d57a..93111e18a1 100644 --- a/crates/apps/src/lib/cli/wallet.rs +++ b/crates/apps/src/lib/cli/wallet.rs @@ -246,11 +246,7 @@ fn shielded_key_gen( cli::safe_exit(1) }); let (_mnemonic, seed) = - Wallet::::gen_hd_seed(None, &mut OsRng) - .unwrap_or_else(|err| { - edisplay_line!(io, "{}", err); - cli::safe_exit(1) - }); + Wallet::::gen_hd_seed(None, &mut OsRng); wallet.derive_store_hd_spendind_key( alias, alias_force, @@ -443,8 +439,8 @@ async fn transparent_key_and_address_derive( None, encryption_password, ) - .unwrap_or_else(|err| { - edisplay_line!(io, "{}", err); + .unwrap_or_else(|| { + edisplay_line!(io, "Failed to derive a keypair."); display_line!(io, "No changes are persisted. Exiting."); cli::safe_exit(1) }) @@ -541,11 +537,7 @@ fn transparent_key_and_address_gen( cli::safe_exit(1) }); let (_mnemonic, seed) = - Wallet::::gen_hd_seed(None, &mut OsRng) - .unwrap_or_else(|err| { - edisplay_line!(io, "{}", err); - cli::safe_exit(1) - }); + Wallet::::gen_hd_seed(None, &mut OsRng); wallet.derive_store_hd_secret_key( scheme, Some(alias), @@ -556,8 +548,8 @@ fn transparent_key_and_address_gen( ) } .map(|x| x.0) - .unwrap_or_else(|err| { - eprintln!("{}", err); + .unwrap_or_else(|| { + edisplay_line!(io, "Failed to generate a keypair."); println!("No changes are persisted. Exiting."); cli::safe_exit(0); }); @@ -1266,8 +1258,8 @@ fn transparent_secret_key_add( read_and_confirm_encryption_password(unsafe_dont_encrypt); let alias = wallet .insert_keypair(alias, alias_force, sk, encryption_password, None, None) - .unwrap_or_else(|err| { - edisplay_line!(io, "{}", err); + .unwrap_or_else(|| { + edisplay_line!(io, "Failed to add a keypair."); display_line!(io, "No changes are persisted. Exiting."); cli::safe_exit(1); }); diff --git a/crates/apps/src/lib/client/tx.rs b/crates/apps/src/lib/client/tx.rs index 8666e27bf7..911e1b475a 100644 --- a/crates/apps/src/lib/client/tx.rs +++ b/crates/apps/src/lib/client/tx.rs @@ -689,7 +689,9 @@ pub async fn submit_become_validator( None, None, ) - .map_err(|err| error::Error::Other(err.to_string()))?; + .ok_or(error::Error::Other(String::from( + "Failed to store the keypair.", + )))?; let tx_code_hash = query_wasm_code_hash(namada, tx_code_path.to_string_lossy()) diff --git a/crates/apps/src/lib/wallet/mod.rs b/crates/apps/src/lib/wallet/mod.rs index 2dada40035..3ed2969742 100644 --- a/crates/apps/src/lib/wallet/mod.rs +++ b/crates/apps/src/lib/wallet/mod.rs @@ -12,7 +12,7 @@ pub use namada_sdk::wallet::alias::Alias; use namada_sdk::wallet::fs::FsWalletStorage; use namada_sdk::wallet::store::Store; use namada_sdk::wallet::{ - ConfirmationResponse, FindKeyError, GenDeriveKeyError, Wallet, WalletIo, + ConfirmationResponse, FindKeyError, Wallet, WalletIo, }; pub use namada_sdk::wallet::{ValidatorData, ValidatorKeys}; use rand_core::OsRng; @@ -85,11 +85,14 @@ impl WalletIo for CliWalletUtils { alias.trim().to_owned() } - fn read_mnemonic_code() -> Result { + fn read_mnemonic_code() -> Option { let phrase = get_secure_user_input("Input mnemonic code: ") - .map_err(|_| GenDeriveKeyError::MnemonicInputError)?; - Mnemonic::from_phrase(phrase.as_ref(), Language::English) - .map_err(|_| GenDeriveKeyError::MnemonicInputError) + .unwrap_or_else(|e| { + eprintln!("{}", e); + eprintln!("Action cancelled, no changes persisted."); + cli::safe_exit(1) + }); + Mnemonic::from_phrase(phrase.as_ref(), Language::English).ok() } fn read_mnemonic_passphrase(confirm: bool) -> Zeroizing { @@ -295,11 +298,9 @@ mod tests { let mut rng = rand_core::OsRng; let mnemonic1 = - CliWalletUtils::generate_mnemonic_code(MNEMONIC_TYPE, &mut rng) - .unwrap(); + CliWalletUtils::generate_mnemonic_code(MNEMONIC_TYPE, &mut rng); let mnemonic2 = - CliWalletUtils::generate_mnemonic_code(MNEMONIC_TYPE, &mut rng) - .unwrap(); + CliWalletUtils::generate_mnemonic_code(MNEMONIC_TYPE, &mut rng); assert_ne!(mnemonic1.into_phrase(), mnemonic2.into_phrase()); } } diff --git a/crates/sdk/src/wallet/mod.rs b/crates/sdk/src/wallet/mod.rs index f892b3c8f8..e47cb5b0cb 100644 --- a/crates/sdk/src/wallet/mod.rs +++ b/crates/sdk/src/wallet/mod.rs @@ -29,23 +29,6 @@ pub use self::keys::{DecryptionError, StoredKeypair}; pub use self::store::{ConfirmationResponse, ValidatorData, ValidatorKeys}; use crate::wallet::store::{derive_hd_secret_key, derive_hd_spending_key}; -/// Errors of key generation / derivation -#[derive(Error, Debug)] -pub enum GenDeriveKeyError { - /// Derivation path parse error - #[error("Derivation path parse error")] - DerivationPathError(DerivationPathError), - /// Mnemonic generation error - #[error("Mnemonic generation error")] - MnemonicGenerationError, - /// Mnemonic input error - #[error("Mnemonic input error")] - MnemonicInputError, - /// Key storage error - #[error("Key storage error")] - KeyStorageError, -} - /// Captures the interactive parts of the wallet's functioning pub trait WalletIo: Sized + Clone { /// Secure random number generator @@ -55,17 +38,15 @@ pub trait WalletIo: Sized + Clone { fn generate_mnemonic_code( mnemonic_type: MnemonicType, rng: &mut Self::Rng, - ) -> Result { + ) -> Mnemonic { const BITS_PER_BYTE: usize = 8; // generate random mnemonic let entropy_size = mnemonic_type.entropy_bits() / BITS_PER_BYTE; let mut bytes = vec![0u8; entropy_size]; rand::RngCore::fill_bytes(rng, &mut bytes); - let mnemonic = Mnemonic::from_entropy(&bytes, Language::English) - .expect("Mnemonic creation should not fail"); - - Ok(mnemonic) + Mnemonic::from_entropy(&bytes, Language::English) + .expect("Mnemonic creation should not fail") } /// Read the password for decryption from the file/env/stdin. @@ -79,7 +60,7 @@ pub trait WalletIo: Sized + Clone { } /// Read mnemonic code from the file/env/stdin. - fn read_mnemonic_code() -> Result { + fn read_mnemonic_code() -> Option { panic!("attempted to prompt for alias in non-interactive mode"); } @@ -539,10 +520,7 @@ impl Wallet { if let Some(mnemonic_passphrase) = mnemonic_passphrase { mnemonic_passphrase } else { - ( - U::read_mnemonic_code().ok()?, - U::read_mnemonic_passphrase(false), - ) + (U::read_mnemonic_code()?, U::read_mnemonic_passphrase(false)) }; let seed = Seed::new(&mnemonic, &passphrase); let spend_key = @@ -575,7 +553,7 @@ impl Wallet { derivation_path: DerivationPath, mnemonic_passphrase: Option<(Mnemonic, Zeroizing)>, password: Option>, - ) -> Result<(String, common::SecretKey), GenDeriveKeyError> { + ) -> Option<(String, common::SecretKey)> { let (mnemonic, passphrase) = if let Some(mnemonic_passphrase) = mnemonic_passphrase { mnemonic_passphrase @@ -629,7 +607,7 @@ impl Wallet { alias_force: bool, password: Option>, rng: &mut (impl CryptoRng + RngCore), - ) -> Result<(String, common::SecretKey), GenDeriveKeyError> { + ) -> Option<(String, common::SecretKey)> { let sk = gen_secret_key(scheme, rng); self.insert_keypair( alias.unwrap_or_default(), @@ -647,9 +625,9 @@ impl Wallet { pub fn gen_hd_seed( passphrase: Option>, rng: &mut U::Rng, - ) -> Result<(Mnemonic, Seed), GenDeriveKeyError> { + ) -> (Mnemonic, Seed) { const MNEMONIC_TYPE: MnemonicType = MnemonicType::Words24; - let mnemonic = U::generate_mnemonic_code(MNEMONIC_TYPE, rng)?; + let mnemonic = U::generate_mnemonic_code(MNEMONIC_TYPE, rng); println!( "Safely store your {} words mnemonic.", MNEMONIC_TYPE.word_count() @@ -659,7 +637,7 @@ impl Wallet { let passphrase = passphrase.unwrap_or_else(|| U::read_mnemonic_passphrase(true)); let seed = Seed::new(&mnemonic, &passphrase); - Ok((mnemonic, seed)) + (mnemonic, seed) } /// Derive a keypair from the given seed and path, derive an implicit @@ -679,7 +657,7 @@ impl Wallet { seed: Seed, derivation_path: DerivationPath, password: Option>, - ) -> Result<(String, common::SecretKey), GenDeriveKeyError> { + ) -> Option<(String, common::SecretKey)> { let sk = derive_hd_secret_key( scheme, seed.as_bytes(), @@ -960,7 +938,7 @@ impl Wallet { password: Option>, address: Option
, path: Option, - ) -> Result { + ) -> Option { self.store .insert_keypair::( alias.into(), @@ -975,7 +953,6 @@ impl Wallet { self.decrypted_key_cache.insert(alias.clone(), sk); alias.into() }) - .ok_or(GenDeriveKeyError::KeyStorageError) } /// Insert a new public key with the given alias. If the alias is already diff --git a/crates/tests/src/e2e/setup.rs b/crates/tests/src/e2e/setup.rs index 8720c2af65..82cb962a0d 100644 --- a/crates/tests/src/e2e/setup.rs +++ b/crates/tests/src/e2e/setup.rs @@ -171,7 +171,7 @@ where None, &mut OsRng, ) - .unwrap_or_else(|_| { + .unwrap_or_else(|| { panic!("Could not generate new key for validator-{}", val) }); println!("alias: {}, pk: {}", alias, sk.ref_to()); From 0a171d9bef12be132d1f8bb0bd0ae12a4dd30934 Mon Sep 17 00:00:00 2001 From: Aleksandr Karbyshev Date: Wed, 17 Jan 2024 12:35:16 +0100 Subject: [PATCH 14/20] Fix e2e test cases --- crates/tests/src/e2e/ledger_tests.rs | 8 +++++++- crates/tests/src/e2e/wallet_tests.rs | 12 +++++++++++- 2 files changed, 18 insertions(+), 2 deletions(-) diff --git a/crates/tests/src/e2e/ledger_tests.rs b/crates/tests/src/e2e/ledger_tests.rs index 1185828196..eaf0cea022 100644 --- a/crates/tests/src/e2e/ledger_tests.rs +++ b/crates/tests/src/e2e/ledger_tests.rs @@ -2957,7 +2957,13 @@ fn implicit_account_reveal_pk() -> Result<()> { let mut cmd = run!( test, Bin::Wallet, - &["gen", "--alias", &key_alias, "--unsafe-dont-encrypt"], + &[ + "gen", + "--alias", + &key_alias, + "--unsafe-dont-encrypt", + "--raw" + ], Some(20), )?; cmd.assert_success(); diff --git a/crates/tests/src/e2e/wallet_tests.rs b/crates/tests/src/e2e/wallet_tests.rs index 370bdac670..8b62eeb134 100644 --- a/crates/tests/src/e2e/wallet_tests.rs +++ b/crates/tests/src/e2e/wallet_tests.rs @@ -136,6 +136,10 @@ fn wallet_unencrypted_key_cmds() -> Result<()> { &["gen", "--alias", key_alias, "--unsafe-dont-encrypt"], Some(20), )?; + + cmd.exp_string("Enter BIP39 passphrase (empty for none): ")?; + cmd.send_line("")?; + cmd.exp_string(&format!( "Successfully added a key and an address with alias: \"{}\"", key_alias.to_lowercase() @@ -183,7 +187,13 @@ fn wallet_address_cmds() -> Result<()> { let mut cmd = run!( test, Bin::Wallet, - &["gen", "--alias", gen_address_alias, "--unsafe-dont-encrypt"], + &[ + "gen", + "--alias", + gen_address_alias, + "--unsafe-dont-encrypt", + "--raw" + ], Some(20), )?; cmd.exp_string(&format!( From f9361d72079aa9fc6ab20a484b178273a1d40166 Mon Sep 17 00:00:00 2001 From: Aleksandr Karbyshev Date: Wed, 17 Jan 2024 17:58:59 +0100 Subject: [PATCH 15/20] Derivation path compliancy checks --- crates/apps/src/lib/cli/wallet.rs | 37 +++- crates/sdk/src/wallet/derivation_path.rs | 258 ++++++++++++++++++----- crates/sdk/src/wallet/store.rs | 26 +-- 3 files changed, 251 insertions(+), 70 deletions(-) diff --git a/crates/apps/src/lib/cli/wallet.rs b/crates/apps/src/lib/cli/wallet.rs index 93111e18a1..cc713acfb7 100644 --- a/crates/apps/src/lib/cli/wallet.rs +++ b/crates/apps/src/lib/cli/wallet.rs @@ -188,6 +188,12 @@ fn shielded_key_derive( edisplay_line!(io, "{}", err); cli::safe_exit(1) }); + println!("Using HD derivation path {}", derivation_path); + if !derivation_path.is_namada_shielded_compliant() { + display_line!(io, "Path {} is not compliant.", derivation_path); + display_line!(io, "No changes are persisted. Exiting."); + cli::safe_exit(1) + } let alias = alias.to_lowercase(); let alias = if !use_device { let encryption_password = @@ -245,6 +251,12 @@ fn shielded_key_gen( edisplay_line!(io, "{}", err); cli::safe_exit(1) }); + println!("Using HD derivation path {}", derivation_path); + if !derivation_path.is_namada_shielded_compliant() { + display_line!(io, "Path {} is not compliant.", derivation_path); + display_line!(io, "No changes are persisted. Exiting."); + cli::safe_exit(1) + } let (_mnemonic, seed) = Wallet::::gen_hd_seed(None, &mut OsRng); wallet.derive_store_hd_spendind_key( @@ -377,15 +389,11 @@ pub fn decode_transparent_derivation_path( let parsed_derivation_path = if is_default { DerivationPath::default_for_transparent_scheme(scheme) } else { - DerivationPath::from_path_string_for_scheme(scheme, &derivation_path)? + DerivationPath::from_path_string_for_transparent_scheme( + scheme, + &derivation_path, + )? }; - if !parsed_derivation_path.is_compatible(scheme) { - println!( - "WARNING: the specified derivation path may be incompatible with \ - the chosen cryptography scheme." - ) - } - println!("Using HD derivation path {}", parsed_derivation_path); Ok(parsed_derivation_path) } @@ -400,7 +408,6 @@ pub fn decode_shielded_derivation_path( } else { DerivationPath::from_path_string(&derivation_path)? }; - println!("Using HD derivation path {}", parsed_derivation_path); Ok(parsed_derivation_path) } @@ -426,6 +433,12 @@ async fn transparent_key_and_address_derive( edisplay_line!(io, "{}", err); cli::safe_exit(1) }); + println!("Using HD derivation path {}", derivation_path); + if !derivation_path.is_namada_transparent_compliant(scheme) { + display_line!(io, "Path {} is not compliant.", derivation_path); + display_line!(io, "No changes are persisted. Exiting."); + cli::safe_exit(1) + } let alias = alias.to_lowercase(); let alias = if !use_device { let encryption_password = @@ -536,6 +549,12 @@ fn transparent_key_and_address_gen( edisplay_line!(io, "{}", err); cli::safe_exit(1) }); + println!("Using HD derivation path {}", derivation_path); + if !derivation_path.is_namada_transparent_compliant(scheme) { + display_line!(io, "Path {} is not compliant.", derivation_path); + display_line!(io, "No changes are persisted. Exiting."); + cli::safe_exit(1) + } let (_mnemonic, seed) = Wallet::::gen_hd_seed(None, &mut OsRng); wallet.derive_store_hd_secret_key( diff --git a/crates/sdk/src/wallet/derivation_path.rs b/crates/sdk/src/wallet/derivation_path.rs index 8938a3b9a7..97e52bda17 100644 --- a/crates/sdk/src/wallet/derivation_path.rs +++ b/crates/sdk/src/wallet/derivation_path.rs @@ -12,6 +12,9 @@ use tiny_hderive::bip44::{ }; use tiny_hderive::Error as HDeriveError; +const BIP44_PURPOSE: u32 = 44; +const ZIP32_PURPOSE: u32 = 32; + const ETH_COIN_TYPE: u32 = 60; const NAMADA_COIN_TYPE: u32 = 877; @@ -32,7 +35,10 @@ impl DerivationPath { Self(DerivationPathInner::new(path)) } - pub fn is_compatible(&self, scheme: SchemeType) -> bool { + pub fn has_transparent_compatible_coin_type( + &self, + scheme: SchemeType, + ) -> bool { if let Some(coin_type) = self.0.as_ref().get(1) { let coin_type = coin_type.to_u32(); match scheme { @@ -45,8 +51,100 @@ impl DerivationPath { } } + pub fn has_shielded_compatible_coin_type(&self) -> bool { + if let Some(coin_type) = self.0.as_ref().get(1) { + coin_type.to_u32() == NAMADA_COIN_TYPE + } else { + true + } + } + + /// Check if the path is BIP-0044 conform + /// https://github.com/bitcoin/bips/blob/master/bip-0044.mediawiki#path-levels + pub fn is_bip44_conform(&self, strict: bool) -> bool { + // check the path conforms the structure: + // m / purpose' / coin_type' / account' / change / address_index + let purpose = self.0.as_ref().get(0); + let coin_type = self.0.as_ref().get(1); + let account = self.0.as_ref().get(2); + let change = self.0.as_ref().get(3); + let address = self.0.as_ref().get(4); + let junk = self.0.as_ref().get(5); + if let ( + Some(purpose), + Some(coin_type), + Some(account), + Some(change), + Some(address), + None, + ) = (purpose, coin_type, account, change, address, junk) + { + purpose.to_u32() == BIP44_PURPOSE + && purpose.is_hardened() + && coin_type.is_hardened() + && account.is_hardened() + && (!strict || (change.is_normal() && address.is_normal())) + } else { + false + } + } + + /// Check if the path is SLIP-0010 conform + /// https://github.com/satoshilabs/slips/blob/master/slip-0010.md#child-key-derivation-ckd-functions + pub fn is_slip10_conform(&self, scheme: SchemeType) -> bool { + match scheme { + SchemeType::Ed25519 => { + // all indices must be hardened + self.0.as_ref().iter().all(|idx| idx.is_hardened()) + } + _ => true, + } + } + + /// Check if the path is ZIP-0032 conform + /// https://zips.z.cash/zip-0032#sapling-key-path + pub fn is_zip32_conform(&self) -> bool { + // check the path conforms one of the structure: + // m / purpose' / coin_type' / account' + // m / purpose' / coin_type' / account' / address_index + let purpose = self.0.as_ref().get(0); + let coin_type = self.0.as_ref().get(1); + let account = self.0.as_ref().get(2); + let address = self.0.as_ref().get(3); + let junk = self.0.as_ref().get(4); + if let (Some(purpose), Some(coin_type), Some(account), None) = + (purpose, coin_type, account, junk) + { + purpose.to_u32() == ZIP32_PURPOSE + && purpose.is_hardened() + && coin_type.is_hardened() + && account.is_hardened() + && (address.is_none() || address.unwrap().is_normal()) + } else { + false + } + } + + pub fn is_namada_transparent_compliant(&self, scheme: SchemeType) -> bool { + match scheme { + SchemeType::Ed25519 => { + self.is_bip44_conform(false) + && self.is_slip10_conform(scheme) + && self.has_transparent_compatible_coin_type(scheme) + } + SchemeType::Secp256k1 => { + self.is_bip44_conform(true) + && self.has_transparent_compatible_coin_type(scheme) + } + SchemeType::Common => false, + } + } + + pub fn is_namada_shielded_compliant(&self) -> bool { + self.is_zip32_conform() && self.has_shielded_compatible_coin_type() + } + fn bip44_base_indexes_for_scheme(scheme: SchemeType) -> Vec { - const BIP44_PURPOSE: u32 = 44; vec![ ChildIndex::Hardened(BIP44_PURPOSE), match scheme { @@ -73,7 +171,6 @@ impl DerivationPath { /// Key path according to zip-0032 /// https://zips.z.cash/zip-0032#sapling-key-path fn zip32(account: u32, address: Option) -> Self { - const ZIP32_PURPOSE: u32 = 32; let mut indexes = vec![ ChildIndex::Hardened(ZIP32_PURPOSE), ChildIndex::Hardened(NAMADA_COIN_TYPE), @@ -113,7 +210,7 @@ impl DerivationPath { Ok(Self(inner)) } - pub fn from_path_string_for_scheme( + pub fn from_path_string_for_transparent_scheme( scheme: SchemeType, path: &str, ) -> Result { @@ -179,50 +276,113 @@ mod tests { use super::DerivationPath; #[test] - fn path_is_compatible() { - let path_empty = DerivationPath::from_path_string_for_scheme( - SchemeType::Secp256k1, - "m", - ) - .expect("Path construction cannot fail."); - assert!(path_empty.is_compatible(SchemeType::Ed25519)); - assert!(path_empty.is_compatible(SchemeType::Secp256k1)); - assert!(path_empty.is_compatible(SchemeType::Common)); - - let path_one = DerivationPath::from_path_string_for_scheme( - SchemeType::Secp256k1, - "m/44'", - ) - .expect("Path construction cannot fail."); - assert!(path_one.is_compatible(SchemeType::Ed25519)); - assert!(path_one.is_compatible(SchemeType::Secp256k1)); - assert!(path_one.is_compatible(SchemeType::Common)); - - let path_two = DerivationPath::from_path_string_for_scheme( - SchemeType::Secp256k1, - "m/44'/99999'", - ) - .expect("Path construction cannot fail."); - assert!(!path_two.is_compatible(SchemeType::Ed25519)); - assert!(!path_two.is_compatible(SchemeType::Secp256k1)); - assert!(path_two.is_compatible(SchemeType::Common)); - - let path_eth = DerivationPath::from_path_string_for_scheme( - SchemeType::Secp256k1, - "m/44'/60'", - ) - .expect("Path construction cannot fail."); - assert!(!path_eth.is_compatible(SchemeType::Ed25519)); - assert!(path_eth.is_compatible(SchemeType::Secp256k1)); - assert!(path_eth.is_compatible(SchemeType::Common)); - - let path_nam = DerivationPath::from_path_string_for_scheme( - SchemeType::Ed25519, - "m/44'/877'", - ) - .expect("Path construction cannot fail."); - assert!(path_nam.is_compatible(SchemeType::Ed25519)); - assert!(!path_nam.is_compatible(SchemeType::Secp256k1)); - assert!(path_nam.is_compatible(SchemeType::Common)); + fn path_conformity() { + let path_empty = DerivationPath::from_path_string("m") + .expect("Path construction cannot fail."); + assert!( + path_empty + .has_transparent_compatible_coin_type(SchemeType::Ed25519) + ); + assert!( + path_empty + .has_transparent_compatible_coin_type(SchemeType::Secp256k1) + ); + assert!(path_empty.has_shielded_compatible_coin_type()); + assert!(!path_empty.is_bip44_conform(true)); + assert!(!path_empty.is_bip44_conform(false)); + assert!(path_empty.is_slip10_conform(SchemeType::Ed25519)); + assert!(path_empty.is_slip10_conform(SchemeType::Secp256k1)); + assert!(!path_empty.is_zip32_conform()); + assert!( + !path_empty.is_namada_transparent_compliant(SchemeType::Ed25519) + ); + assert!( + !path_empty.is_namada_transparent_compliant(SchemeType::Secp256k1) + ); + assert!(!path_empty.is_namada_shielded_compliant()); + + let path_eth = DerivationPath::from_path_string("m/44'/60'/0'/0/0") + .expect("Path construction cannot fail."); + assert!( + !path_eth.has_transparent_compatible_coin_type(SchemeType::Ed25519) + ); + assert!( + path_eth + .has_transparent_compatible_coin_type(SchemeType::Secp256k1) + ); + assert!(!path_eth.has_shielded_compatible_coin_type()); + assert!(path_eth.is_bip44_conform(true)); + assert!(path_eth.is_bip44_conform(false)); + assert!(!path_eth.is_slip10_conform(SchemeType::Ed25519)); + assert!(path_eth.is_slip10_conform(SchemeType::Secp256k1)); + assert!(!path_eth.is_zip32_conform()); + assert!(!path_eth.is_namada_transparent_compliant(SchemeType::Ed25519)); + assert!( + path_eth.is_namada_transparent_compliant(SchemeType::Secp256k1) + ); + assert!(!path_eth.is_namada_shielded_compliant()); + + let path_nam = DerivationPath::from_path_string("m/44'/877'/0'/0'/0'") + .expect("Path construction cannot fail."); + assert!( + path_nam.has_transparent_compatible_coin_type(SchemeType::Ed25519) + ); + assert!( + !path_nam + .has_transparent_compatible_coin_type(SchemeType::Secp256k1) + ); + assert!(path_nam.has_shielded_compatible_coin_type()); + assert!(!path_nam.is_bip44_conform(true)); + assert!(path_nam.is_bip44_conform(false)); + assert!(path_nam.is_slip10_conform(SchemeType::Ed25519)); + assert!(path_nam.is_slip10_conform(SchemeType::Secp256k1)); + assert!(!path_nam.is_zip32_conform()); + assert!(path_nam.is_namada_transparent_compliant(SchemeType::Ed25519)); + assert!( + !path_nam.is_namada_transparent_compliant(SchemeType::Secp256k1) + ); + assert!(!path_nam.is_namada_shielded_compliant()); + + let path_z_1 = DerivationPath::from_path_string("m/32'/877'/0'") + .expect("Path construction cannot fail."); + assert!( + path_z_1.has_transparent_compatible_coin_type(SchemeType::Ed25519) + ); + assert!( + !path_z_1 + .has_transparent_compatible_coin_type(SchemeType::Secp256k1) + ); + assert!(path_z_1.has_shielded_compatible_coin_type()); + assert!(!path_z_1.is_bip44_conform(true)); + assert!(!path_z_1.is_bip44_conform(false)); + assert!(path_z_1.is_slip10_conform(SchemeType::Ed25519)); + assert!(path_z_1.is_slip10_conform(SchemeType::Secp256k1)); + assert!(path_z_1.is_zip32_conform()); + assert!(!path_z_1.is_namada_transparent_compliant(SchemeType::Ed25519)); + assert!( + !path_z_1.is_namada_transparent_compliant(SchemeType::Secp256k1) + ); + assert!(path_z_1.is_namada_shielded_compliant()); + + let path_z_2 = DerivationPath::from_path_string("m/32'/877'/0'/0") + .expect("Path construction cannot fail."); + assert!( + path_z_2.has_transparent_compatible_coin_type(SchemeType::Ed25519) + ); + assert!( + !path_z_2 + .has_transparent_compatible_coin_type(SchemeType::Secp256k1) + ); + assert!(path_z_2.has_shielded_compatible_coin_type()); + assert!(!path_z_2.is_bip44_conform(true)); + assert!(!path_z_2.is_bip44_conform(false)); + assert!(!path_z_2.is_slip10_conform(SchemeType::Ed25519)); + assert!(path_z_2.is_slip10_conform(SchemeType::Secp256k1)); + assert!(path_z_2.is_zip32_conform()); + assert!(!path_z_2.is_namada_transparent_compliant(SchemeType::Ed25519)); + assert!( + !path_z_2.is_namada_transparent_compliant(SchemeType::Secp256k1) + ); + assert!(path_z_2.is_namada_shielded_compliant()); } } diff --git a/crates/sdk/src/wallet/store.rs b/crates/sdk/src/wallet/store.rs index df650b5316..533d657f8d 100644 --- a/crates/sdk/src/wallet/store.rs +++ b/crates/sdk/src/wallet/store.rs @@ -817,11 +817,12 @@ mod test_wallet { let seed = Seed::new(&mnemonic, PASSPHRASE); assert_eq!(format!("{:x}", seed), SEED_EXPECTED); - let derivation_path = DerivationPath::from_path_string_for_scheme( - SCHEME, - DERIVATION_PATH, - ) - .expect("Derivation path construction cannot fail"); + let derivation_path = + DerivationPath::from_path_string_for_transparent_scheme( + SCHEME, + DERIVATION_PATH, + ) + .expect("Derivation path construction cannot fail"); let sk = derive_hd_secret_key(SCHEME, seed.as_bytes(), derivation_path); @@ -845,14 +846,15 @@ mod test_wallet { let seed = Seed::new(&mnemonic, PASSPHRASE); assert_eq!(format!("{:x}", seed), SEED_EXPECTED); - let derivation_path = DerivationPath::from_path_string_for_scheme( - SCHEME, - DERIVATION_PATH, - ) - .expect("Derivation path construction cannot fail"); + let derivation_path = + DerivationPath::from_path_string_for_transparent_scheme( + SCHEME, + DERIVATION_PATH, + ) + .expect("Derivation path construction cannot fail"); let derivation_path_hardened = - DerivationPath::from_path_string_for_scheme( + DerivationPath::from_path_string_for_transparent_scheme( SCHEME, DERIVATION_PATH_HARDENED, ) @@ -882,7 +884,7 @@ mod test_wallet { .decode(seed.as_bytes()) .expect("Seed parsing cannot fail.") .as_slice(), - DerivationPath::from_path_string_for_scheme( + DerivationPath::from_path_string_for_transparent_scheme( scheme, derivation_path, ) From 7e1a1a0077810225241de2f2a5ab2e0f462cbd02 Mon Sep 17 00:00:00 2001 From: Aleksandr Karbyshev Date: Wed, 17 Jan 2024 18:01:41 +0100 Subject: [PATCH 16/20] Fix comment --- crates/sdk/src/args.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/sdk/src/args.rs b/crates/sdk/src/args.rs index 575ed22c86..5d7a6c2dab 100644 --- a/crates/sdk/src/args.rs +++ b/crates/sdk/src/args.rs @@ -2127,7 +2127,7 @@ pub struct KeyDerive { pub alias_force: bool, /// Don't encrypt the keypair pub unsafe_dont_encrypt: bool, - /// BIP44 derivation path + /// BIP44 / ZIP32 derivation path pub derivation_path: String, /// Use device to generate key and address pub use_device: bool, From a7440f69b63e66fc8f93e0976c76e34cb5cb3c89 Mon Sep 17 00:00:00 2001 From: Aleksandr Karbyshev Date: Wed, 17 Jan 2024 18:46:38 +0100 Subject: [PATCH 17/20] Introduce flag to allow non-compliant derivation paths --- crates/apps/src/lib/cli.rs | 70 ++++++++++++++++++++++++------- crates/apps/src/lib/cli/wallet.rs | 18 ++++++-- crates/sdk/src/args.rs | 4 ++ 3 files changed, 72 insertions(+), 20 deletions(-) diff --git a/crates/apps/src/lib/cli.rs b/crates/apps/src/lib/cli.rs index deded0a797..602351781d 100644 --- a/crates/apps/src/lib/cli.rs +++ b/crates/apps/src/lib/cli.rs @@ -2975,8 +2975,10 @@ pub mod args { arg("validator"); pub const HALT_ACTION: ArgFlag = flag("halt"); pub const HASH_LIST: Arg = arg("hash-list"); - pub const HD_WALLET_DERIVATION_PATH: ArgDefault = + pub const HD_DERIVATION_PATH: ArgDefault = arg_default("hd-path", DefaultFn(|| "default".to_string())); + pub const HD_ALLOW_NON_COMPLIANT_DERIVATION_PATH: ArgFlag = + flag("allow-non-compliant"); pub const HISTORIC: ArgFlag = flag("historic"); pub const IBC_TRANSFER_MEMO_PATH: ArgOpt = arg_opt("memo-path"); pub const INPUT_OPT: ArgOpt = arg_opt("input"); @@ -6196,7 +6198,9 @@ pub mod args { let alias_force = ALIAS_FORCE.parse(matches); let unsafe_dont_encrypt = UNSAFE_DONT_ENCRYPT.parse(matches); let use_device = USE_DEVICE.parse(matches); - let derivation_path = HD_WALLET_DERIVATION_PATH.parse(matches); + let derivation_path = HD_DERIVATION_PATH.parse(matches); + let allow_non_compliant = + HD_ALLOW_NON_COMPLIANT_DERIVATION_PATH.parse(matches); Self { scheme, shielded, @@ -6205,6 +6209,7 @@ pub mod args { unsafe_dont_encrypt, use_device, derivation_path, + allow_non_compliant, } } @@ -6234,14 +6239,29 @@ pub mod args { "Derive an address and public key from the seed stored on the \ connected hardware wallet.", )) - .arg(HD_WALLET_DERIVATION_PATH.def().help( + .arg(HD_DERIVATION_PATH.def().help( "HD key derivation path. Use keyword `default` to refer to a \ - scheme default path:\n- m/44'/60'/0'/0/0 for secp256k1 \ - scheme\n- m/44'/877'/0'/0'/0' for ed25519 scheme.\nFor \ - ed25519, all path indices will be promoted to hardened \ - indexes. If none is specified, the scheme default path is \ - used.", + scheme default path:\n- m/44'/60'/0'/0/0 for the transparent \ + secp256k1 scheme\n- m/44'/877'/0'/0'/0' for the transparent \ + ed25519 scheme\n- m/32'/877'/0' for the shielded \ + setting\nFor ed25519 scheme, all path indices will be \ + promoted to hardened indexes. If none is specified, the \ + scheme default path is used.", )) + .arg(HD_ALLOW_NON_COMPLIANT_DERIVATION_PATH.def().help( + "Allow non-compliant HD derivation path. The compliant \ + derivation path schemes include:\n- \ + m/44'/60'/account'/change/address_index for the transparent \ + secp256k1 scheme\n- \ + m/44'/877'/account'/change'/address_index' for the \ + transparent ed25519 scheme\n- m/32'/877'/account' and\n- \ + m/32'/877'/account'/address_index for the shielded setting", + )) + .group( + ArgGroup::new("requires_group") + .args([HD_ALLOW_NON_COMPLIANT_DERIVATION_PATH.name]) + .requires(HD_DERIVATION_PATH.name), + ) } } @@ -6253,7 +6273,9 @@ pub mod args { let alias = ALIAS.parse(matches); let alias_force = ALIAS_FORCE.parse(matches); let unsafe_dont_encrypt = UNSAFE_DONT_ENCRYPT.parse(matches); - let derivation_path = HD_WALLET_DERIVATION_PATH.parse(matches); + let derivation_path = HD_DERIVATION_PATH.parse(matches); + let allow_non_compliant = + HD_ALLOW_NON_COMPLIANT_DERIVATION_PATH.parse(matches); Self { scheme, shielded, @@ -6262,6 +6284,7 @@ pub mod args { alias_force, unsafe_dont_encrypt, derivation_path, + allow_non_compliant, } } @@ -6280,7 +6303,7 @@ pub mod args { .arg( RAW_KEY_GEN .def() - .conflicts_with(HD_WALLET_DERIVATION_PATH.name) + .conflicts_with(HD_DERIVATION_PATH.name) .help( "Generate a random non-HD secret / spending key. No \ mnemonic code is generated.", @@ -6294,14 +6317,29 @@ pub mod args { "UNSAFE: Do not encrypt the keypair. Do not use this for keys \ used in a live network.", )) - .arg(HD_WALLET_DERIVATION_PATH.def().help( + .arg(HD_DERIVATION_PATH.def().help( "HD key derivation path. Use keyword `default` to refer to a \ - scheme default path:\n- m/44'/60'/0'/0/0 for secp256k1 \ - scheme\n- m/44'/877'/0'/0'/0' for ed25519 scheme.\nFor \ - ed25519, all path indices will be promoted to hardened \ - indexes. If none is specified, the scheme default path is \ - used.", + scheme default path:\n- m/44'/60'/0'/0/0 for the transparent \ + secp256k1 scheme\n- m/44'/877'/0'/0'/0' for the transparent \ + ed25519 scheme\n- m/32'/877'/0' for the shielded \ + setting\nFor ed25519 scheme, all path indices will be \ + promoted to hardened indexes. If none is specified, the \ + scheme default path is used.", )) + .arg(HD_ALLOW_NON_COMPLIANT_DERIVATION_PATH.def().help( + "Allow non-compliant HD derivation path. The compliant \ + derivation path schemes include:\n- \ + m/44'/60'/account'/change/address_index for the transparent \ + secp256k1 scheme\n- \ + m/44'/877'/account'/change'/address_index' for the \ + transparent ed25519 scheme\n- m/32'/877'/account' and\n- \ + m/32'/877'/account'/address_index for the shielded setting", + )) + .group( + ArgGroup::new("requires_group") + .args([HD_ALLOW_NON_COMPLIANT_DERIVATION_PATH.name]) + .requires(HD_DERIVATION_PATH.name), + ) } } diff --git a/crates/apps/src/lib/cli/wallet.rs b/crates/apps/src/lib/cli/wallet.rs index cc713acfb7..9551152b3c 100644 --- a/crates/apps/src/lib/cli/wallet.rs +++ b/crates/apps/src/lib/cli/wallet.rs @@ -178,6 +178,7 @@ fn shielded_key_derive( alias_force, unsafe_dont_encrypt, derivation_path, + allow_non_compliant, use_device, .. }: args::KeyDerive, @@ -189,7 +190,7 @@ fn shielded_key_derive( cli::safe_exit(1) }); println!("Using HD derivation path {}", derivation_path); - if !derivation_path.is_namada_shielded_compliant() { + if !allow_non_compliant && !derivation_path.is_namada_shielded_compliant() { display_line!(io, "Path {} is not compliant.", derivation_path); display_line!(io, "No changes are persisted. Exiting."); cli::safe_exit(1) @@ -237,6 +238,7 @@ fn shielded_key_gen( alias_force, unsafe_dont_encrypt, derivation_path, + allow_non_compliant, .. }: args::KeyGen, ) { @@ -252,7 +254,9 @@ fn shielded_key_gen( cli::safe_exit(1) }); println!("Using HD derivation path {}", derivation_path); - if !derivation_path.is_namada_shielded_compliant() { + if !allow_non_compliant + && !derivation_path.is_namada_shielded_compliant() + { display_line!(io, "Path {} is not compliant.", derivation_path); display_line!(io, "No changes are persisted. Exiting."); cli::safe_exit(1) @@ -422,6 +426,7 @@ async fn transparent_key_and_address_derive( alias_force, unsafe_dont_encrypt, derivation_path, + allow_non_compliant, use_device, .. }: args::KeyDerive, @@ -434,7 +439,9 @@ async fn transparent_key_and_address_derive( cli::safe_exit(1) }); println!("Using HD derivation path {}", derivation_path); - if !derivation_path.is_namada_transparent_compliant(scheme) { + if !allow_non_compliant + && !derivation_path.is_namada_transparent_compliant(scheme) + { display_line!(io, "Path {} is not compliant.", derivation_path); display_line!(io, "No changes are persisted. Exiting."); cli::safe_exit(1) @@ -527,6 +534,7 @@ fn transparent_key_and_address_gen( alias_force, unsafe_dont_encrypt, derivation_path, + allow_non_compliant, .. }: args::KeyGen, ) { @@ -550,7 +558,9 @@ fn transparent_key_and_address_gen( cli::safe_exit(1) }); println!("Using HD derivation path {}", derivation_path); - if !derivation_path.is_namada_transparent_compliant(scheme) { + if !allow_non_compliant + && !derivation_path.is_namada_transparent_compliant(scheme) + { display_line!(io, "Path {} is not compliant.", derivation_path); display_line!(io, "No changes are persisted. Exiting."); cli::safe_exit(1) diff --git a/crates/sdk/src/args.rs b/crates/sdk/src/args.rs index 5d7a6c2dab..9b2a2bed8b 100644 --- a/crates/sdk/src/args.rs +++ b/crates/sdk/src/args.rs @@ -2112,6 +2112,8 @@ pub struct KeyGen { pub unsafe_dont_encrypt: bool, /// BIP44 / ZIP32 derivation path pub derivation_path: String, + /// Allow non-compliant derivation path + pub allow_non_compliant: bool, } /// Wallet restore key and implicit address arguments @@ -2129,6 +2131,8 @@ pub struct KeyDerive { pub unsafe_dont_encrypt: bool, /// BIP44 / ZIP32 derivation path pub derivation_path: String, + /// Allow non-compliant derivation path + pub allow_non_compliant: bool, /// Use device to generate key and address pub use_device: bool, } From 8120ab77332c792dd67324397bbbe081eb9f7d9e Mon Sep 17 00:00:00 2001 From: Aleksandr Karbyshev Date: Thu, 18 Jan 2024 01:03:35 +0100 Subject: [PATCH 18/20] Add changelog --- .changelog/unreleased/features/2417-wallet-zip32-0.30.1.md | 2 ++ 1 file changed, 2 insertions(+) create mode 100644 .changelog/unreleased/features/2417-wallet-zip32-0.30.1.md diff --git a/.changelog/unreleased/features/2417-wallet-zip32-0.30.1.md b/.changelog/unreleased/features/2417-wallet-zip32-0.30.1.md new file mode 100644 index 0000000000..7b4ed29731 --- /dev/null +++ b/.changelog/unreleased/features/2417-wallet-zip32-0.30.1.md @@ -0,0 +1,2 @@ +- Implemented ZIP32 functionality for shielded pool keys. + ([\#2417](https://github.com/anoma/namada/pull/2417)) \ No newline at end of file From 9d47d9108b727b0b7e724d2d95ce7f29ab836e1e Mon Sep 17 00:00:00 2001 From: Aleksandr Karbyshev Date: Thu, 18 Jan 2024 14:30:58 +0100 Subject: [PATCH 19/20] Minor refactoring of wallet e2e --- crates/tests/src/e2e/wallet_tests.rs | 15 +++++++++------ crates/tests/src/strings.rs | 5 +++++ 2 files changed, 14 insertions(+), 6 deletions(-) diff --git a/crates/tests/src/e2e/wallet_tests.rs b/crates/tests/src/e2e/wallet_tests.rs index 8b62eeb134..68cb663b98 100644 --- a/crates/tests/src/e2e/wallet_tests.rs +++ b/crates/tests/src/e2e/wallet_tests.rs @@ -16,6 +16,9 @@ use color_eyre::eyre::Result; use super::setup; use crate::e2e::setup::Bin; use crate::run; +use crate::strings::{ + WALLET_FOUND_TRANSPARENT_KEYS, WALLET_HD_PASSPHRASE_PROMPT, +}; /// Test wallet key commands with an encrypted key: /// 1. key gen @@ -35,7 +38,7 @@ fn wallet_encrypted_key_cmds() -> Result<()> { cmd.send_line(password)?; cmd.exp_string("Enter same passphrase again: ")?; cmd.send_line(password)?; - cmd.exp_string("Enter BIP39 passphrase (empty for none): ")?; + cmd.exp_string(WALLET_HD_PASSPHRASE_PROMPT)?; cmd.send_line("")?; cmd.exp_string(&format!( "Successfully added a key and an address with alias: \"{}\"", @@ -50,7 +53,7 @@ fn wallet_encrypted_key_cmds() -> Result<()> { Some(20), )?; - cmd.exp_string("Found transparent keys:")?; + cmd.exp_string(WALLET_FOUND_TRANSPARENT_KEYS)?; cmd.exp_string(&format!( " Alias \"{}\" (encrypted):", key_alias.to_lowercase() @@ -86,7 +89,7 @@ fn wallet_encrypted_key_cmds_env_var() -> Result<()> { let mut cmd = run!(test, Bin::Wallet, &["gen", "--alias", key_alias], Some(20),)?; - cmd.exp_string("Enter BIP39 passphrase (empty for none): ")?; + cmd.exp_string(WALLET_HD_PASSPHRASE_PROMPT)?; cmd.send_line("")?; cmd.exp_string(&format!( @@ -102,7 +105,7 @@ fn wallet_encrypted_key_cmds_env_var() -> Result<()> { Some(20), )?; - cmd.exp_string("Found transparent keys:")?; + cmd.exp_string(WALLET_FOUND_TRANSPARENT_KEYS)?; cmd.exp_string(&format!( " Alias \"{}\" (encrypted):", key_alias.to_lowercase() @@ -137,7 +140,7 @@ fn wallet_unencrypted_key_cmds() -> Result<()> { Some(20), )?; - cmd.exp_string("Enter BIP39 passphrase (empty for none): ")?; + cmd.exp_string(WALLET_HD_PASSPHRASE_PROMPT)?; cmd.send_line("")?; cmd.exp_string(&format!( @@ -153,7 +156,7 @@ fn wallet_unencrypted_key_cmds() -> Result<()> { Some(20), )?; - cmd.exp_string("Found transparent keys:")?; + cmd.exp_string(WALLET_FOUND_TRANSPARENT_KEYS)?; cmd.exp_string(&format!( " Alias \"{}\" (not encrypted):", key_alias.to_lowercase() diff --git a/crates/tests/src/strings.rs b/crates/tests/src/strings.rs index bea5ce536d..4def362e75 100644 --- a/crates/tests/src/strings.rs +++ b/crates/tests/src/strings.rs @@ -23,3 +23,8 @@ pub const TX_FAILED: &str = "Transaction failed"; /// Wrapper transaction accepted. pub const TX_ACCEPTED: &str = "Wrapper transaction accepted"; + +pub const WALLET_HD_PASSPHRASE_PROMPT: &str = + "Enter BIP39 passphrase (empty for none): "; + +pub const WALLET_FOUND_TRANSPARENT_KEYS: &str = "Found transparent keys:"; From b6bc0e976d50a5df55e56165068c41ffaa49e07a Mon Sep 17 00:00:00 2001 From: Aleksandr Karbyshev Date: Tue, 23 Jan 2024 15:06:24 +0100 Subject: [PATCH 20/20] Add comment on path compliance check --- crates/sdk/src/wallet/derivation_path.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/crates/sdk/src/wallet/derivation_path.rs b/crates/sdk/src/wallet/derivation_path.rs index 97e52bda17..3210450d26 100644 --- a/crates/sdk/src/wallet/derivation_path.rs +++ b/crates/sdk/src/wallet/derivation_path.rs @@ -97,6 +97,7 @@ impl DerivationPath { // all indices must be hardened self.0.as_ref().iter().all(|idx| idx.is_hardened()) } + // no restriction for secp256k1 scheme _ => true, } }