Skip to content

Commit

Permalink
zcash_keys: Remove HdSeedFingerprint as it duplicates `zip32::fingerp…
Browse files Browse the repository at this point in the history
…rint::SeedFingerprint`
  • Loading branch information
nuttycom committed Mar 14, 2024
1 parent be312f8 commit cc990b6
Show file tree
Hide file tree
Showing 8 changed files with 40 additions and 75 deletions.
4 changes: 2 additions & 2 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
12 changes: 6 additions & 6 deletions zcash_client_backend/src/data_api.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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},
Expand Down Expand Up @@ -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,
},

Expand Down Expand Up @@ -698,11 +698,11 @@ pub trait WalletRead {
account_id: Self::AccountId,
) -> Result<Option<Self::Account>, 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<Option<Self::Account>, Self::Error>;

Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -1686,7 +1686,7 @@ pub mod testing {

fn get_derived_account(
&self,
_seed: &HdSeedFingerprint,
_seed: &SeedFingerprint,
_account_id: zip32::AccountId,
) -> Result<Option<Self::Account>, Self::Error> {
Ok(None)
Expand Down
18 changes: 14 additions & 4 deletions zcash_client_sqlite/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -45,14 +45,14 @@ use std::{
path::Path,
};
use subtle::ConditionallySelectable;
use zcash_keys::keys::HdSeedFingerprint;
use zcash_primitives::{
block::BlockHash,
consensus::{self, BlockHeight},
memo::{Memo, MemoBytes},
transaction::{components::amount::NonNegativeAmount, Transaction, TxId},
zip32::{self, DiversifierIndex, Scope},
};
use zip32::fingerprint::SeedFingerprint;

use zcash_client_backend::{
address::UnifiedAddress,
Expand Down Expand Up @@ -304,7 +304,7 @@ impl<C: Borrow<rusqlite::Connection>, P: consensus::Parameters> WalletRead for W

fn get_derived_account(
&self,
seed: &HdSeedFingerprint,
seed: &SeedFingerprint,
account_id: zip32::AccountId,
) -> Result<Option<Self::Account>, Self::Error> {
wallet::get_derived_account(self.conn.borrow(), &self.params, seed, account_id)
Expand All @@ -321,7 +321,12 @@ impl<C: Borrow<rusqlite::Connection>, 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(),

Check warning on line 327 in zcash_client_sqlite/src/lib.rs

View check run for this annotation

Codecov / codecov/patch

zcash_client_sqlite/src/lib.rs#L326-L327

Added lines #L326 - L327 were not covered by tests
)
})? == seed_fingerprint;

let usk = UnifiedSpendingKey::from_seed(
&self.params,
Expand Down Expand Up @@ -502,7 +507,12 @@ impl<P: consensus::Parameters> WalletWrite for WalletDb<rusqlite::Connection, P>
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(),

Check warning on line 513 in zcash_client_sqlite/src/lib.rs

View check run for this annotation

Codecov / codecov/patch

zcash_client_sqlite/src/lib.rs#L512-L513

Added lines #L512 - L513 were not covered by tests
)
})?;
let account_index = wallet::max_zip32_account_index(wdb.conn.0, &seed_fingerprint)?
.map(|a| a.next().ok_or(SqliteClientError::AccountIdOutOfRange))
.transpose()?
Expand Down
17 changes: 9 additions & 8 deletions zcash_client_sqlite/src/wallet.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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},
Expand Down Expand Up @@ -145,7 +146,7 @@ fn parse_account_kind(
) -> Result<AccountKind, SqliteClientError> {
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(),
Expand Down Expand Up @@ -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<Option<zip32::AccountId>, 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<u32> = row.get(0)?;
account_id
Expand Down Expand Up @@ -366,7 +367,7 @@ pub(crate) fn add_account<P: consensus::Parameters>(
"#,
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)?,
Expand Down Expand Up @@ -765,12 +766,12 @@ pub(crate) fn get_account_for_ufvk<P: consensus::Parameters>(
}
}

/// 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<P: consensus::Parameters>(
conn: &rusqlite::Connection,
params: &P,
seed: &HdSeedFingerprint,
seed: &SeedFingerprint,
account_index: zip32::AccountId,
) -> Result<Option<Account>, SqliteClientError> {
let mut stmt = conn.prepare(
Expand All @@ -782,7 +783,7 @@ pub(crate) fn get_derived_account<P: consensus::Parameters>(

let mut accounts = stmt.query_and_then::<_, SqliteClientError, _, _>(
named_params![
":hd_seed_fingerprint": seed.as_bytes(),
":hd_seed_fingerprint": seed.to_bytes(),

Check warning on line 786 in zcash_client_sqlite/src/wallet.rs

View check run for this annotation

Codecov / codecov/patch

zcash_client_sqlite/src/wallet.rs#L786

Added line #L786 was not covered by tests
":hd_account_index": u32::from(account_index),
],
|row| {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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};

Expand Down Expand Up @@ -45,7 +46,7 @@ impl<P: consensus::Parameters> RusqliteMigration for Migration<P> {

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);
Expand Down Expand Up @@ -83,7 +84,8 @@ impl<P: consensus::Parameters> RusqliteMigration for Migration<P> {
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()? {
Expand Down Expand Up @@ -145,7 +147,7 @@ impl<P: consensus::Parameters> RusqliteMigration for Migration<P> {
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,
Expand Down
1 change: 0 additions & 1 deletion zcash_keys/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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`
Expand Down
51 changes: 2 additions & 49 deletions zcash_keys/src/keys.rs
Original file line number Diff line number Diff line change
@@ -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};
Expand Down Expand Up @@ -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<u8>) -> 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<NonHardenedChildIndex> {
let (low_4_bytes, rest) = j.as_bytes().split_at(4);
Expand Down

0 comments on commit cc990b6

Please sign in to comment.