From cc990b60e00feceb4486ad6e5aef0a60f8b60c4b Mon Sep 17 00:00:00 2001 From: Kris Nuttycombe Date: Thu, 14 Mar 2024 16:59:19 -0600 Subject: [PATCH] zcash_keys: Remove HdSeedFingerprint as it duplicates `zip32::fingerprint::SeedFingerprint` --- Cargo.lock | 4 +- Cargo.toml | 2 +- zcash_client_backend/src/data_api.rs | 12 ++--- zcash_client_sqlite/src/lib.rs | 18 +++++-- zcash_client_sqlite/src/wallet.rs | 17 ++++--- .../init/migrations/full_account_ids.rs | 10 ++-- zcash_keys/CHANGELOG.md | 1 - zcash_keys/src/keys.rs | 51 +------------------ 8 files changed, 40 insertions(+), 75 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index eaa12f5a36..820c7cf971 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -3297,9 +3297,9 @@ dependencies = [ [[package]] name = "zip32" -version = "0.1.0" +version = "0.1.1" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "d724a63be4dfb50b7f3617e542984e22e4b4a5b8ca5de91f55613152885e6b22" +checksum = "4226d0aee9c9407c27064dfeec9d7b281c917de3374e1e5a2e2cfad9e09de19e" dependencies = [ "blake2b_simd", "memuse", diff --git a/Cargo.toml b/Cargo.toml index 69897715e3..405c9dd1ed 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -114,7 +114,7 @@ rand_xorshift = "0.3" # ZIP 32 aes = "0.8" fpe = "0.6" -zip32 = "0.1" +zip32 = "0.1.1" [profile.release] lto = true diff --git a/zcash_client_backend/src/data_api.rs b/zcash_client_backend/src/data_api.rs index cb28431479..6c324cf81e 100644 --- a/zcash_client_backend/src/data_api.rs +++ b/zcash_client_backend/src/data_api.rs @@ -66,7 +66,7 @@ use std::{ use incrementalmerkletree::{frontier::Frontier, Retention}; use secrecy::SecretVec; use shardtree::{error::ShardTreeError, store::ShardStore, ShardTree}; -use zcash_keys::keys::HdSeedFingerprint; +use zip32::fingerprint::SeedFingerprint; use self::{ chain::{ChainState, CommitmentTreeRoot}, @@ -320,7 +320,7 @@ impl AccountBalance { pub enum AccountKind { /// An account derived from a known seed. Derived { - seed_fingerprint: HdSeedFingerprint, + seed_fingerprint: SeedFingerprint, account_index: zip32::AccountId, }, @@ -698,11 +698,11 @@ pub trait WalletRead { account_id: Self::AccountId, ) -> Result, Self::Error>; - /// Returns the account corresponding to a given [`HdSeedFingerprint`] and + /// Returns the account corresponding to a given [`SeedFingerprint`] and /// [`zip32::AccountId`], if any. fn get_derived_account( &self, - seed: &HdSeedFingerprint, + seed: &SeedFingerprint, account_id: zip32::AccountId, ) -> Result, Self::Error>; @@ -1586,7 +1586,7 @@ pub mod testing { use secrecy::{ExposeSecret, SecretVec}; use shardtree::{error::ShardTreeError, store::memory::MemoryShardStore, ShardTree}; use std::{collections::HashMap, convert::Infallible, num::NonZeroU32}; - use zcash_keys::keys::HdSeedFingerprint; + use zip32::fingerprint::SeedFingerprint; use zcash_primitives::{ block::BlockHash, @@ -1686,7 +1686,7 @@ pub mod testing { fn get_derived_account( &self, - _seed: &HdSeedFingerprint, + _seed: &SeedFingerprint, _account_id: zip32::AccountId, ) -> Result, Self::Error> { Ok(None) diff --git a/zcash_client_sqlite/src/lib.rs b/zcash_client_sqlite/src/lib.rs index 57ca55654b..ae5a3c84d8 100644 --- a/zcash_client_sqlite/src/lib.rs +++ b/zcash_client_sqlite/src/lib.rs @@ -45,7 +45,6 @@ use std::{ path::Path, }; use subtle::ConditionallySelectable; -use zcash_keys::keys::HdSeedFingerprint; use zcash_primitives::{ block::BlockHash, consensus::{self, BlockHeight}, @@ -53,6 +52,7 @@ use zcash_primitives::{ transaction::{components::amount::NonNegativeAmount, Transaction, TxId}, zip32::{self, DiversifierIndex, Scope}, }; +use zip32::fingerprint::SeedFingerprint; use zcash_client_backend::{ address::UnifiedAddress, @@ -304,7 +304,7 @@ impl, P: consensus::Parameters> WalletRead for W fn get_derived_account( &self, - seed: &HdSeedFingerprint, + seed: &SeedFingerprint, account_id: zip32::AccountId, ) -> Result, Self::Error> { wallet::get_derived_account(self.conn.borrow(), &self.params, seed, account_id) @@ -321,7 +321,12 @@ impl, P: consensus::Parameters> WalletRead for W account_index, } = account.kind() { - let seed_fingerprint_match = HdSeedFingerprint::from_seed(seed) == seed_fingerprint; + let seed_fingerprint_match = + SeedFingerprint::from_seed(seed.expose_secret()).ok_or_else(|| { + SqliteClientError::BadAccountData( + "Seed must be between 32 and 252 bytes in length.".to_owned(), + ) + })? == seed_fingerprint; let usk = UnifiedSpendingKey::from_seed( &self.params, @@ -502,7 +507,12 @@ impl WalletWrite for WalletDb birthday: AccountBirthday, ) -> Result<(AccountId, UnifiedSpendingKey), Self::Error> { self.transactionally(|wdb| { - let seed_fingerprint = HdSeedFingerprint::from_seed(seed); + let seed_fingerprint = + SeedFingerprint::from_seed(seed.expose_secret()).ok_or_else(|| { + SqliteClientError::BadAccountData( + "Seed must be between 32 and 252 bytes in length.".to_owned(), + ) + })?; let account_index = wallet::max_zip32_account_index(wdb.conn.0, &seed_fingerprint)? .map(|a| a.next().ok_or(SqliteClientError::AccountIdOutOfRange)) .transpose()? diff --git a/zcash_client_sqlite/src/wallet.rs b/zcash_client_sqlite/src/wallet.rs index 95a7e631f7..44c44dc854 100644 --- a/zcash_client_sqlite/src/wallet.rs +++ b/zcash_client_sqlite/src/wallet.rs @@ -67,6 +67,7 @@ use incrementalmerkletree::Retention; use rusqlite::{self, named_params, params, OptionalExtension}; use shardtree::{error::ShardTreeError, store::ShardStore, ShardTree}; +use zip32::fingerprint::SeedFingerprint; use std::collections::HashMap; use std::convert::TryFrom; @@ -75,7 +76,7 @@ use std::num::NonZeroU32; use std::ops::RangeInclusive; use tracing::debug; use zcash_address::unified::{Encoding, Ivk, Uivk}; -use zcash_keys::keys::{AddressGenerationError, HdSeedFingerprint, UnifiedAddressRequest}; +use zcash_keys::keys::{AddressGenerationError, UnifiedAddressRequest}; use zcash_client_backend::{ address::{Address, UnifiedAddress}, @@ -145,7 +146,7 @@ fn parse_account_kind( ) -> Result { match (account_kind, hd_seed_fingerprint, hd_account_index) { (0, Some(seed_fp), Some(account_index)) => Ok(AccountKind::Derived { - seed_fingerprint: HdSeedFingerprint::from_bytes(seed_fp), + seed_fingerprint: SeedFingerprint::from_bytes(seed_fp), account_index: zip32::AccountId::try_from(account_index).map_err(|_| { SqliteClientError::CorruptedData( "ZIP-32 account ID from wallet DB is out of range.".to_string(), @@ -280,11 +281,11 @@ pub(crate) fn memo_repr(memo: Option<&MemoBytes>) -> Option<&[u8]> { // Returns the highest used account index for a given seed. pub(crate) fn max_zip32_account_index( conn: &rusqlite::Connection, - seed_id: &HdSeedFingerprint, + seed_id: &SeedFingerprint, ) -> Result, SqliteClientError> { conn.query_row_and_then( "SELECT MAX(hd_account_index) FROM accounts WHERE hd_seed_fingerprint = :hd_seed", - [seed_id.as_bytes()], + [seed_id.to_bytes()], |row| { let account_id: Option = row.get(0)?; account_id @@ -366,7 +367,7 @@ pub(crate) fn add_account( "#, named_params![ ":account_kind": account_kind_code(kind), - ":hd_seed_fingerprint": hd_seed_fingerprint.as_ref().map(|fp| fp.as_bytes()), + ":hd_seed_fingerprint": hd_seed_fingerprint.as_ref().map(|fp| fp.to_bytes()), ":hd_account_index": hd_account_index.map(u32::from), ":ufvk": viewing_key.ufvk().map(|ufvk| ufvk.encode(params)), ":uivk": viewing_key.uivk_str(params)?, @@ -765,12 +766,12 @@ pub(crate) fn get_account_for_ufvk( } } -/// Returns the account id corresponding to a given [`HdSeedFingerprint`] +/// Returns the account id corresponding to a given [`SeedFingerprint`] /// and [`zip32::AccountId`], if any. pub(crate) fn get_derived_account( conn: &rusqlite::Connection, params: &P, - seed: &HdSeedFingerprint, + seed: &SeedFingerprint, account_index: zip32::AccountId, ) -> Result, SqliteClientError> { let mut stmt = conn.prepare( @@ -782,7 +783,7 @@ pub(crate) fn get_derived_account( let mut accounts = stmt.query_and_then::<_, SqliteClientError, _, _>( named_params![ - ":hd_seed_fingerprint": seed.as_bytes(), + ":hd_seed_fingerprint": seed.to_bytes(), ":hd_account_index": u32::from(account_index), ], |row| { diff --git a/zcash_client_sqlite/src/wallet/init/migrations/full_account_ids.rs b/zcash_client_sqlite/src/wallet/init/migrations/full_account_ids.rs index 60f9b924a5..9de422b9b4 100644 --- a/zcash_client_sqlite/src/wallet/init/migrations/full_account_ids.rs +++ b/zcash_client_sqlite/src/wallet/init/migrations/full_account_ids.rs @@ -6,8 +6,9 @@ use schemer_rusqlite::RusqliteMigration; use secrecy::{ExposeSecret, SecretVec}; use uuid::Uuid; use zcash_client_backend::{data_api::AccountKind, keys::UnifiedSpendingKey}; -use zcash_keys::keys::{HdSeedFingerprint, UnifiedFullViewingKey}; +use zcash_keys::keys::UnifiedFullViewingKey; use zcash_primitives::consensus; +use zip32::fingerprint::SeedFingerprint; use super::{add_account_birthdays, receiving_key_scopes, v_transactions_note_uniqueness}; @@ -45,7 +46,7 @@ impl RusqliteMigration for Migration

{ fn up(&self, transaction: &Transaction) -> Result<(), WalletMigrationError> { let account_kind_derived = account_kind_code(AccountKind::Derived { - seed_fingerprint: HdSeedFingerprint::from_bytes([0; 32]), + seed_fingerprint: SeedFingerprint::from_bytes([0; 32]), account_index: zip32::AccountId::ZERO, }); let account_kind_imported = account_kind_code(AccountKind::Imported); @@ -83,7 +84,8 @@ impl RusqliteMigration for Migration

{ Ok(row.get::<_, u32>(0)? > 0) })? { if let Some(seed) = &self.seed.as_ref() { - let seed_id = HdSeedFingerprint::from_seed(seed); + let seed_id = SeedFingerprint::from_seed(seed.expose_secret()) + .expect("Seed is between 32 and 252 bytes in length."); let mut q = transaction.prepare("SELECT * FROM accounts")?; let mut rows = q.query([])?; while let Some(row) = rows.next()? { @@ -145,7 +147,7 @@ impl RusqliteMigration for Migration

{ named_params![ ":account_id": account_id, ":account_kind": account_kind_derived, - ":seed_id": seed_id.as_bytes(), + ":seed_id": seed_id.to_bytes(), ":account_index": account_index, ":ufvk": ufvk, ":uivk": uivk, diff --git a/zcash_keys/CHANGELOG.md b/zcash_keys/CHANGELOG.md index 79654d4c95..5689f98bbc 100644 --- a/zcash_keys/CHANGELOG.md +++ b/zcash_keys/CHANGELOG.md @@ -7,7 +7,6 @@ and this library adheres to Rust's notion of ## [Unreleased] ### Added -- `zcash_keys::keys::HdSeedFingerprint` - `zcash_keys::address::Address::has_receiver` - `impl Display for zcash_keys::keys::AddressGenerationError` - `impl std::error::Error for zcash_keys::keys::AddressGenerationError` diff --git a/zcash_keys/src/keys.rs b/zcash_keys/src/keys.rs index 27dfb8393b..92f5407c1e 100644 --- a/zcash_keys/src/keys.rs +++ b/zcash_keys/src/keys.rs @@ -1,6 +1,6 @@ //! Helper functions for managing light client key material. -use blake2b_simd::Params as blake2bParams; -use secrecy::{ExposeSecret, SecretVec}; + + use std::{error, fmt}; use zcash_address::unified::{self, Container, Encoding, Typecode}; @@ -73,53 +73,6 @@ pub mod sapling { } } -/// A [ZIP 32 seed fingerprint] of a seed used for an HD account. -/// -/// For wallets that use [BIP 39] mnemonic phrases, this is the fingerprint of the binary -/// seed [produced from the mnemonic]. -/// -/// [ZIP 32 seed fingerprint]: https://zips.z.cash/zip-0032#seed-fingerprints -/// [BIP 39]: https://github.com/bitcoin/bips/blob/master/bip-0039.mediawiki -/// [produced from the mnemonic]: https://github.com/bitcoin/bips/blob/master/bip-0039.mediawiki#from-mnemonic-to-seed -#[derive(Debug, Clone, Copy, PartialEq, Eq, Hash)] -pub struct HdSeedFingerprint([u8; 32]); - -impl HdSeedFingerprint { - /// Generates the fingerprint from a given seed. - /// - /// Panics if the length of the seed is not between 32 and 252 bytes inclusive. - pub fn from_seed(seed: &SecretVec) -> Self { - let len = seed.expose_secret().len(); - let len = match len { - 32..=252 => [u8::try_from(len).unwrap()], - _ => panic!("ZIP 32 seeds MUST be at least 32 bytes and at most 252 bytes"), - }; - const PERSONALIZATION: &[u8] = b"Zcash_HD_Seed_FP"; - let hash = blake2bParams::new() - .hash_length(32) - .personal(PERSONALIZATION) - .to_state() - .update(&len) - .update(seed.expose_secret()) - .finalize(); - Self( - hash.as_bytes() - .try_into() - .expect("BLAKE2b-256 hash length is 32 bytes"), - ) - } - - /// Instantiates the fingerprint from a buffer containing a previously computed fingerprint. - pub fn from_bytes(hash: [u8; 32]) -> Self { - Self(hash) - } - - /// Returns the bytes of the fingerprint. - pub fn as_bytes(&self) -> &[u8; 32] { - &self.0 - } -} - #[cfg(feature = "transparent-inputs")] fn to_transparent_child_index(j: DiversifierIndex) -> Option { let (low_4_bytes, rest) = j.as_bytes().split_at(4);