Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

zcash_keys: Remove HdSeedFingerprint as it duplicates zip32::fingerprint::SeedFingerprint' #1274

Merged
merged 1 commit into from
Mar 14, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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 @@
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 @@

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 @@
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 @@
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::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 @@
) -> 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 @@
// 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 @@
"#,
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 @@
}
}

/// 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 @@

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
Loading