From 7a5852598e13db2d21ede308e76a6c35b4d1a3bf Mon Sep 17 00:00:00 2001 From: Kris Nuttycombe Date: Wed, 14 Feb 2024 14:44:30 -0700 Subject: [PATCH] zcash_keys, zcash_client_backend: feature-flag off transparent-input WalletRead methods. This also moves the `TransparentAddressMetadata` type behind the `transparent-inputs` feature flag and performs associated cleanup. --- zcash_client_backend/CHANGELOG.md | 9 +- zcash_client_backend/src/data_api.rs | 55 +++--- zcash_client_backend/src/data_api/wallet.rs | 7 +- zcash_client_backend/src/wallet.rs | 29 +++ zcash_client_sqlite/src/lib.rs | 33 ++-- zcash_client_sqlite/src/wallet.rs | 14 +- .../init/migrations/add_transaction_views.rs | 4 +- .../init/migrations/receiving_key_scopes.rs | 2 +- zcash_keys/src/keys.rs | 4 +- zcash_primitives/CHANGELOG.md | 6 +- zcash_primitives/src/legacy.rs | 117 +----------- zcash_primitives/src/legacy/keys.rs | 169 ++++++++++++++++-- zcash_primitives/src/transaction/builder.rs | 2 +- 13 files changed, 248 insertions(+), 203 deletions(-) diff --git a/zcash_client_backend/CHANGELOG.md b/zcash_client_backend/CHANGELOG.md index 6b64228b8..cc1e39349 100644 --- a/zcash_client_backend/CHANGELOG.md +++ b/zcash_client_backend/CHANGELOG.md @@ -52,13 +52,13 @@ and this library adheres to Rust's notion of - `wallet::input_selection::ShieldingSelector` has been factored out from the `InputSelector` trait to separate out transparent functionality and move it behind the `transparent-inputs` feature flag. - - `TransparentAddressMetadata` (which replaces `zcash_keys::address::AddressMetadata`). - `zcash_client_backend::fees::{standard, sapling}` - `zcash_client_backend::fees::ChangeValue::new` - `zcash_client_backend::wallet`: - `Note` - `ReceivedNote` - `WalletSaplingOutput::recipient_key_scope` + - `wallet::TransparentAddressMetadata` (which replaces `zcash_keys::address::AddressMetadata`). - `zcash_client_backend::zip321::TransactionRequest::total` - `zcash_client_backend::zip321::parse::Param::name` - `zcash_client_backend::proto::` @@ -164,9 +164,12 @@ and this library adheres to Rust's notion of `get_unspent_transparent_outputs` have been removed; use `data_api::InputSource` instead. - Added `get_account_ids`. + - `get_transparent_receivers` and `get_transparent_balances` are now + guarded by the `transparent-inputs` feature flag, with noop default + implementations provided. - `get_transparent_receivers` now returns - `zcash_client_backend::data_api::TransparentAddressMetadata` instead of - `zcash_keys::address::AddressMetadata`. + `Option` as part of + its result where previously it returned `zcash_keys::address::AddressMetadata`. - `wallet::{propose_shielding, shield_transparent_funds}` now takes their `min_confirmations` arguments as `u32` rather than a `NonZeroU32` to permit implmentations to enable zero-conf shielding. diff --git a/zcash_client_backend/src/data_api.rs b/zcash_client_backend/src/data_api.rs index 83e8bca3c..cb32cbe05 100644 --- a/zcash_client_backend/src/data_api.rs +++ b/zcash_client_backend/src/data_api.rs @@ -14,7 +14,6 @@ use shardtree::{error::ShardTreeError, store::ShardStore, ShardTree}; use zcash_primitives::{ block::BlockHash, consensus::BlockHeight, - legacy::{NonHardenedChildIndex, TransparentAddress}, memo::{Memo, MemoBytes}, transaction::{ components::amount::{Amount, BalanceError, NonNegativeAmount}, @@ -36,7 +35,10 @@ use self::chain::CommitmentTreeRoot; use self::scanning::ScanRange; #[cfg(feature = "transparent-inputs")] -use zcash_primitives::transaction::components::OutPoint; +use { + crate::wallet::TransparentAddressMetadata, + zcash_primitives::{legacy::TransparentAddress, transaction::components::OutPoint}, +}; pub mod chain; pub mod error; @@ -56,30 +58,6 @@ pub enum NullifierQuery { All, } -/// Describes the derivation of a transparent address. -#[derive(Debug, Clone, PartialEq, Eq)] -pub struct TransparentAddressMetadata { - scope: zip32::Scope, - address_index: NonHardenedChildIndex, -} - -impl TransparentAddressMetadata { - pub fn new(scope: zip32::Scope, address_index: NonHardenedChildIndex) -> Self { - Self { - scope, - address_index, - } - } - - pub fn scope(&self) -> zip32::Scope { - self.scope - } - - pub fn address_index(&self) -> NonHardenedChildIndex { - self.address_index - } -} - /// Balance information for a value within a single pool in an account. #[derive(Debug, Clone, Copy, PartialEq, Eq)] pub struct Balance { @@ -581,18 +559,24 @@ pub trait WalletRead { /// The set contains all transparent receivers that are known to have been derived /// under this account. Wallets should scan the chain for UTXOs sent to these /// receivers. + #[cfg(feature = "transparent-inputs")] fn get_transparent_receivers( &self, - account: AccountId, - ) -> Result>, Self::Error>; + _account: AccountId, + ) -> Result>, Self::Error> { + Ok(HashMap::new()) + } /// Returns a mapping from transparent receiver to not-yet-shielded UTXO balance, /// for each address associated with a nonzero balance. + #[cfg(feature = "transparent-inputs")] fn get_transparent_balances( &self, - account: AccountId, - max_height: BlockHeight, - ) -> Result, Self::Error>; + _account: AccountId, + _max_height: BlockHeight, + ) -> Result, Self::Error> { + Ok(HashMap::new()) + } /// Returns a vector with the IDs of all accounts known to this wallet. fn get_account_ids(&self) -> Result, Self::Error>; @@ -1133,7 +1117,6 @@ pub mod testing { use zcash_primitives::{ block::BlockHash, consensus::{BlockHeight, Network}, - legacy::TransparentAddress, memo::Memo, transaction::{components::Amount, Transaction, TxId}, zip32::{AccountId, Scope}, @@ -1149,10 +1132,12 @@ pub mod testing { use super::{ chain::CommitmentTreeRoot, scanning::ScanRange, AccountBirthday, BlockMetadata, DecryptedTransaction, InputSource, NullifierQuery, ScannedBlock, SentTransaction, - TransparentAddressMetadata, WalletCommitmentTrees, WalletRead, WalletSummary, WalletWrite, - SAPLING_SHARD_HEIGHT, + WalletCommitmentTrees, WalletRead, WalletSummary, WalletWrite, SAPLING_SHARD_HEIGHT, }; + #[cfg(feature = "transparent-inputs")] + use {crate::wallet::TransparentAddressMetadata, zcash_primitives::legacy::TransparentAddress}; + pub struct MockWalletDb { pub network: Network, pub sapling_tree: ShardTree< @@ -1306,6 +1291,7 @@ pub mod testing { Ok(Vec::new()) } + #[cfg(feature = "transparent-inputs")] fn get_transparent_receivers( &self, _account: AccountId, @@ -1314,6 +1300,7 @@ pub mod testing { Ok(HashMap::new()) } + #[cfg(feature = "transparent-inputs")] fn get_transparent_balances( &self, _account: AccountId, diff --git a/zcash_client_backend/src/data_api/wallet.rs b/zcash_client_backend/src/data_api/wallet.rs index f342e94dc..09529c787 100644 --- a/zcash_client_backend/src/data_api/wallet.rs +++ b/zcash_client_backend/src/data_api/wallet.rs @@ -5,7 +5,6 @@ use sapling::{ note_encryption::{try_sapling_note_decryption, PreparedIncomingViewingKey}, prover::{OutputProver, SpendProver}, }; -use zcash_keys::encoding::AddressCodec; use zcash_primitives::{ consensus::{self, NetworkUpgrade}, memo::MemoBytes, @@ -43,7 +42,7 @@ use super::InputSource; use { crate::wallet::WalletTransparentOutput, input_selection::ShieldingSelector, sapling::keys::OutgoingViewingKey, std::convert::Infallible, - zcash_primitives::legacy::TransparentAddress, + zcash_keys::encoding::AddressCodec, zcash_primitives::legacy::TransparentAddress, }; /// Scans a [`Transaction`] for any information that can be decrypted by the accounts in @@ -644,9 +643,7 @@ where .get(utxo.recipient_address()) .ok_or_else(|| Error::AddressNotRecognized(*utxo.recipient_address()))? .clone() - .ok_or_else(|| { - Error::NoSpendingKey(utxo.recipient_address().encode(params)) - })?; + .ok_or_else(|| Error::NoSpendingKey(utxo.recipient_address().encode(params)))?; let secret_key = usk .transparent() diff --git a/zcash_client_backend/src/wallet.rs b/zcash_client_backend/src/wallet.rs index 8cfb3a48d..4ee91500b 100644 --- a/zcash_client_backend/src/wallet.rs +++ b/zcash_client_backend/src/wallet.rs @@ -22,6 +22,9 @@ use crate::{address::UnifiedAddress, fees::sapling as sapling_fees, PoolType, Sh #[cfg(feature = "orchard")] use crate::fees::orchard as orchard_fees; +#[cfg(feature = "transparent-inputs")] +use zcash_primitives::legacy::keys::{NonHardenedChildIndex, TransparentKeyScope}; + /// A unique identifier for a shielded transaction output #[derive(Debug, Copy, Clone, PartialEq, Eq, PartialOrd, Ord)] pub struct NoteId { @@ -395,3 +398,29 @@ pub enum OvkPolicy { /// recipients, but not by the sender. Discard, } + +/// Metadata related to the ZIP 32 derivation of a transparent address. +#[derive(Debug, Clone, PartialEq, Eq)] +#[cfg(feature = "transparent-inputs")] +pub struct TransparentAddressMetadata { + scope: TransparentKeyScope, + address_index: NonHardenedChildIndex, +} + +#[cfg(feature = "transparent-inputs")] +impl TransparentAddressMetadata { + pub fn new(scope: TransparentKeyScope, address_index: NonHardenedChildIndex) -> Self { + Self { + scope, + address_index, + } + } + + pub fn scope(&self) -> TransparentKeyScope { + self.scope + } + + pub fn address_index(&self) -> NonHardenedChildIndex { + self.address_index + } +} diff --git a/zcash_client_sqlite/src/lib.rs b/zcash_client_sqlite/src/lib.rs index 5524c5b81..e1445dfa1 100644 --- a/zcash_client_sqlite/src/lib.rs +++ b/zcash_client_sqlite/src/lib.rs @@ -48,7 +48,6 @@ use shardtree::{error::ShardTreeError, ShardTree}; use zcash_primitives::{ block::BlockHash, consensus::{self, BlockHeight}, - legacy::TransparentAddress, memo::{Memo, MemoBytes}, transaction::{ components::amount::{Amount, NonNegativeAmount}, @@ -64,8 +63,8 @@ use zcash_client_backend::{ chain::{BlockSource, CommitmentTreeRoot}, scanning::{ScanPriority, ScanRange}, AccountBirthday, BlockMetadata, DecryptedTransaction, InputSource, NullifierQuery, - ScannedBlock, SentTransaction, TransparentAddressMetadata, WalletCommitmentTrees, - WalletRead, WalletSummary, WalletWrite, SAPLING_SHARD_HEIGHT, + ScannedBlock, SentTransaction, WalletCommitmentTrees, WalletRead, WalletSummary, + WalletWrite, SAPLING_SHARD_HEIGHT, }, keys::{UnifiedAddressRequest, UnifiedFullViewingKey, UnifiedSpendingKey}, proto::compact_formats::CompactBlock, @@ -76,7 +75,10 @@ use zcash_client_backend::{ use crate::{error::SqliteClientError, wallet::commitment_tree::SqliteShardStore}; #[cfg(feature = "transparent-inputs")] -use zcash_primitives::transaction::components::OutPoint; +use { + zcash_client_backend::wallet::TransparentAddressMetadata, + zcash_primitives::{legacy::TransparentAddress, transaction::components::OutPoint}, +}; #[cfg(feature = "unstable")] use { @@ -343,36 +345,21 @@ impl, P: consensus::Parameters> WalletRead for W } } + #[cfg(feature = "transparent-inputs")] fn get_transparent_receivers( &self, _account: AccountId, ) -> Result>, Self::Error> { - #[cfg(feature = "transparent-inputs")] - return wallet::get_transparent_receivers(self.conn.borrow(), &self.params, _account); - - #[cfg(not(feature = "transparent-inputs"))] - panic!( - "The wallet must be compiled with the transparent-inputs feature to use this method." - ); + wallet::get_transparent_receivers(self.conn.borrow(), &self.params, _account) } + #[cfg(feature = "transparent-inputs")] fn get_transparent_balances( &self, _account: AccountId, _max_height: BlockHeight, ) -> Result, Self::Error> { - #[cfg(feature = "transparent-inputs")] - return wallet::get_transparent_balances( - self.conn.borrow(), - &self.params, - _account, - _max_height, - ); - - #[cfg(not(feature = "transparent-inputs"))] - panic!( - "The wallet must be compiled with the transparent-inputs feature to use this method." - ); + wallet::get_transparent_balances(self.conn.borrow(), &self.params, _account, _max_height) } #[cfg(feature = "orchard")] diff --git a/zcash_client_sqlite/src/wallet.rs b/zcash_client_sqlite/src/wallet.rs index cbc561dcf..34b3f8be2 100644 --- a/zcash_client_sqlite/src/wallet.rs +++ b/zcash_client_sqlite/src/wallet.rs @@ -112,9 +112,12 @@ use { crate::UtxoId, rusqlite::Row, std::collections::BTreeSet, - zcash_client_backend::{data_api::TransparentAddressMetadata, wallet::WalletTransparentOutput}, + zcash_client_backend::wallet::{TransparentAddressMetadata, WalletTransparentOutput}, zcash_primitives::{ - legacy::{keys::IncomingViewingKey, NonHardenedChildIndex, Script, TransparentAddress}, + legacy::{ + keys::{IncomingViewingKey, NonHardenedChildIndex}, + Script, TransparentAddress, + }, transaction::components::{OutPoint, TxOut}, }, }; @@ -395,7 +398,10 @@ pub(crate) fn get_transparent_receivers( ret.insert( *taddr, - Some(TransparentAddressMetadata::new(Scope::External, index)), + Some(TransparentAddressMetadata::new( + Scope::External.into(), + index, + )), ); } } @@ -404,7 +410,7 @@ pub(crate) fn get_transparent_receivers( ret.insert( taddr, Some(TransparentAddressMetadata::new( - Scope::External, + Scope::External.into(), child_index, )), ); diff --git a/zcash_client_sqlite/src/wallet/init/migrations/add_transaction_views.rs b/zcash_client_sqlite/src/wallet/init/migrations/add_transaction_views.rs index 3fb5903f1..6508ee535 100644 --- a/zcash_client_sqlite/src/wallet/init/migrations/add_transaction_views.rs +++ b/zcash_client_sqlite/src/wallet/init/migrations/add_transaction_views.rs @@ -2,7 +2,7 @@ use std::collections::HashSet; use rusqlite::{self, types::ToSql, OptionalExtension}; -use schemer::{self}; +use schemer; use schemer_rusqlite::RusqliteMigration; use uuid::Uuid; @@ -397,7 +397,7 @@ mod tests { fn migrate_from_wm2() { use zcash_client_backend::keys::UnifiedAddressRequest; use zcash_primitives::{ - legacy::NonHardenedChildIndex, transaction::components::amount::NonNegativeAmount, + legacy::keys::NonHardenedChildIndex, transaction::components::amount::NonNegativeAmount, }; use crate::UA_TRANSPARENT; diff --git a/zcash_client_sqlite/src/wallet/init/migrations/receiving_key_scopes.rs b/zcash_client_sqlite/src/wallet/init/migrations/receiving_key_scopes.rs index 7344329a4..9836bb8d5 100644 --- a/zcash_client_sqlite/src/wallet/init/migrations/receiving_key_scopes.rs +++ b/zcash_client_sqlite/src/wallet/init/migrations/receiving_key_scopes.rs @@ -292,7 +292,7 @@ mod tests { use zcash_primitives::{ block::BlockHash, consensus::{BlockHeight, Network, NetworkUpgrade, Parameters}, - legacy::{keys::IncomingViewingKey, NonHardenedChildIndex}, + legacy::keys::{IncomingViewingKey, NonHardenedChildIndex}, memo::MemoBytes, transaction::{ builder::{BuildConfig, BuildResult, Builder}, diff --git a/zcash_keys/src/keys.rs b/zcash_keys/src/keys.rs index 1fc1b9e73..95337a24c 100644 --- a/zcash_keys/src/keys.rs +++ b/zcash_keys/src/keys.rs @@ -8,7 +8,7 @@ use zcash_primitives::{ use crate::address::UnifiedAddress; #[cfg(feature = "transparent-inputs")] -use zcash_primitives::legacy::NonHardenedChildIndex; +use zcash_primitives::legacy::keys::NonHardenedChildIndex; #[cfg(feature = "transparent-inputs")] use { @@ -815,7 +815,7 @@ mod tests { #[cfg(feature = "transparent-inputs")] #[test] fn pk_to_taddr() { - use zcash_primitives::legacy::NonHardenedChildIndex; + use zcash_primitives::legacy::keys::NonHardenedChildIndex; let taddr = legacy::keys::AccountPrivKey::from_seed(&MAIN_NETWORK, &seed(), AccountId::ZERO) diff --git a/zcash_primitives/CHANGELOG.md b/zcash_primitives/CHANGELOG.md index 875259ca2..a9673aba8 100644 --- a/zcash_primitives/CHANGELOG.md +++ b/zcash_primitives/CHANGELOG.md @@ -7,7 +7,8 @@ and this library adheres to Rust's notion of ## [Unreleased] ### Added -- `zcash_primitives::legacy::keys::NonHardenedChildIndex` +- `zcash_primitives::legacy::keys::{NonHardenedChildIndex, TransparentKeyScope}` +- `zcash_primitives::legacy::keys::AccountPrivKey::derive_secret_key` - Dependency on `bellman 0.14`. - `zcash_primitives::consensus::sapling_zip212_enforcement` - `zcash_primitives::transaction`: @@ -127,7 +128,8 @@ and this library adheres to Rust's notion of - `zcash_client_backend` changes related to `local-consensus` feature: - added tests that verify `zip321` supports Payment URIs with `Local(P)` network parameters. -- `zcash_primitives::legacy::keys::derive_external_secret_key` parameter type changed from `u32` to `NonHardenedChildIndex`. +- `zcash_primitives::legacy::keys::{derive_external_secret_key, derive_internal_secret_key}` + arguments changed from `u32` to `NonHardenedChildIndex`. ### Removed - `zcash_primitives::constants`: diff --git a/zcash_primitives/src/legacy.rs b/zcash_primitives/src/legacy.rs index 5ddf0c5a3..398d9ab97 100644 --- a/zcash_primitives/src/legacy.rs +++ b/zcash_primitives/src/legacy.rs @@ -6,11 +6,6 @@ use std::fmt; use std::io::{self, Read, Write}; use std::ops::Shl; -#[cfg(feature = "transparent-inputs")] -use hdwallet::KeyIndex; - -use subtle::{Choice, ConstantTimeEq}; - use zcash_encoding::Vector; #[cfg(feature = "transparent-inputs")] @@ -407,63 +402,6 @@ impl TransparentAddress { } } -/// A child index for a derived transparent address. -/// -/// Only NON-hardened derivation is supported. -#[derive(Clone, Copy, Debug, PartialEq, Eq)] -pub struct NonHardenedChildIndex(u32); - -impl ConstantTimeEq for NonHardenedChildIndex { - fn ct_eq(&self, other: &Self) -> Choice { - self.0.ct_eq(&other.0) - } -} - -impl NonHardenedChildIndex { - pub const ZERO: NonHardenedChildIndex = NonHardenedChildIndex(0); - - /// Parses the given ZIP 32 child index. - /// - /// Returns `None` if the hardened bit is set. - pub fn from_index(i: u32) -> Option { - if i < (1 << 31) { - Some(NonHardenedChildIndex(i)) - } else { - None - } - } - - /// Returns the index as a 32-bit integer. - pub fn index(&self) -> u32 { - self.0 - } - - pub fn next(&self) -> Option { - // overflow cannot happen because self.0 is 31 bits, and the next index is at most 32 bits - // which in that case would lead from_index to return None. - Self::from_index(self.0 + 1) - } -} - -#[cfg(feature = "transparent-inputs")] -impl TryFrom for NonHardenedChildIndex { - type Error = (); - - fn try_from(value: KeyIndex) -> Result { - match value { - KeyIndex::Normal(i) => NonHardenedChildIndex::from_index(i).ok_or(()), - KeyIndex::Hardened(_) => Err(()), - } - } -} - -#[cfg(feature = "transparent-inputs")] -impl From for KeyIndex { - fn from(value: NonHardenedChildIndex) -> Self { - Self::Normal(value.index()) - } -} - #[cfg(any(test, feature = "test-dependencies"))] pub mod testing { use proptest::prelude::{any, prop_compose}; @@ -479,9 +417,7 @@ pub mod testing { #[cfg(test)] mod tests { - use super::{NonHardenedChildIndex, OpCode, Script, TransparentAddress}; - use hdwallet::KeyIndex; - use subtle::ConstantTimeEq; + use super::{OpCode, Script, TransparentAddress}; #[test] fn script_opcode() { @@ -548,55 +484,4 @@ mod tests { ); assert_eq!(addr.script().address(), Some(addr)); } - - #[test] - fn nonhardened_indexes_accepted() { - assert_eq!(0, NonHardenedChildIndex::from_index(0).unwrap().index()); - assert_eq!( - 0x7fffffff, - NonHardenedChildIndex::from_index(0x7fffffff) - .unwrap() - .index() - ); - } - - #[test] - fn hardened_indexes_rejected() { - assert!(NonHardenedChildIndex::from_index(0x80000000).is_none()); - assert!(NonHardenedChildIndex::from_index(0xffffffff).is_none()); - } - - #[test] - fn nonhardened_index_next() { - assert_eq!(1, NonHardenedChildIndex::ZERO.next().unwrap().index()); - assert!(NonHardenedChildIndex::from_index(0x7fffffff) - .unwrap() - .next() - .is_none()); - } - - #[test] - fn nonhardened_index_ct_eq() { - assert!(check( - NonHardenedChildIndex::ZERO, - NonHardenedChildIndex::ZERO - )); - assert!(!check( - NonHardenedChildIndex::ZERO, - NonHardenedChildIndex::ZERO.next().unwrap() - )); - - fn check(v1: T, v2: T) -> bool { - v1.ct_eq(&v2).into() - } - } - - #[test] - #[cfg(feature = "transparent-inputs")] - fn nonhardened_index_tryfrom_keyindex() { - let nh: NonHardenedChildIndex = KeyIndex::Normal(0).try_into().unwrap(); - assert_eq!(nh.index(), 0); - - assert!(NonHardenedChildIndex::try_from(KeyIndex::Hardened(0)).is_err()); - } } diff --git a/zcash_primitives/src/legacy/keys.rs b/zcash_primitives/src/legacy/keys.rs index 85e53f30e..ff4f7e946 100644 --- a/zcash_primitives/src/legacy/keys.rs +++ b/zcash_primitives/src/legacy/keys.rs @@ -6,11 +6,99 @@ use hdwallet::{ }; use secp256k1::PublicKey; use sha2::{Digest, Sha256}; +use subtle::{Choice, ConstantTimeEq}; use zcash_spec::PrfExpand; use crate::{consensus, zip32::AccountId}; -use super::{NonHardenedChildIndex, TransparentAddress}; +use super::TransparentAddress; + +/// The scope of a transparent key. +/// +/// This type can represent [`zip32`] internal and external scopes, as well as custom scopes that +/// may be used in non-hardened derivation at the `change` level of the BIP 44 key path. +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub struct TransparentKeyScope(u32); + +impl TransparentKeyScope { + pub fn custom(i: u32) -> Option { + if i < (1 << 31) { + Some(TransparentKeyScope(i)) + } else { + None + } + } +} + +impl From for TransparentKeyScope { + fn from(value: zip32::Scope) -> Self { + match value { + zip32::Scope::External => TransparentKeyScope(0), + zip32::Scope::Internal => TransparentKeyScope(1), + } + } +} + +impl From for KeyIndex { + fn from(value: TransparentKeyScope) -> Self { + KeyIndex::Normal(value.0) + } +} + +/// A child index for a derived transparent address. +/// +/// Only NON-hardened derivation is supported. +#[derive(Clone, Copy, Debug, PartialEq, Eq)] +pub struct NonHardenedChildIndex(u32); + +impl ConstantTimeEq for NonHardenedChildIndex { + fn ct_eq(&self, other: &Self) -> Choice { + self.0.ct_eq(&other.0) + } +} + +impl NonHardenedChildIndex { + pub const ZERO: NonHardenedChildIndex = NonHardenedChildIndex(0); + + /// Parses the given ZIP 32 child index. + /// + /// Returns `None` if the hardened bit is set. + pub fn from_index(i: u32) -> Option { + if i < (1 << 31) { + Some(NonHardenedChildIndex(i)) + } else { + None + } + } + + /// Returns the index as a 32-bit integer. + pub fn index(&self) -> u32 { + self.0 + } + + pub fn next(&self) -> Option { + // overflow cannot happen because self.0 is 31 bits, and the next index is at most 32 bits + // which in that case would lead from_index to return None. + Self::from_index(self.0 + 1) + } +} + +impl TryFrom for NonHardenedChildIndex { + type Error = (); + + fn try_from(value: KeyIndex) -> Result { + match value { + KeyIndex::Normal(i) => NonHardenedChildIndex::from_index(i).ok_or(()), + KeyIndex::Hardened(_) => Err(()), + } + } +} + +impl From for KeyIndex { + fn from(value: NonHardenedChildIndex) -> Self { + Self::Normal(value.index()) + } +} /// A [BIP44] private key at the account path level `m/44'/'/'`. /// @@ -44,28 +132,35 @@ impl AccountPrivKey { AccountPubKey(ExtendedPubKey::from_private_key(&self.0)) } + /// Derives the BIP44 private spending key for the child path + /// `m/44'/'/'//`. + pub fn derive_secret_key( + &self, + scope: TransparentKeyScope, + child_index: NonHardenedChildIndex, + ) -> Result { + self.0 + .derive_private_key(scope.into())? + .derive_private_key(child_index.into()) + .map(|k| k.private_key) + } + /// Derives the BIP44 private spending key for the external (incoming payment) child path /// `m/44'/'/'/0/`. pub fn derive_external_secret_key( &self, child_index: NonHardenedChildIndex, ) -> Result { - self.0 - .derive_private_key(KeyIndex::Normal(0))? - .derive_private_key(child_index.into()) - .map(|k| k.private_key) + self.derive_secret_key(zip32::Scope::External.into(), child_index) } /// Derives the BIP44 private spending key for the internal (change) child path /// `m/44'/'/'/1/`. pub fn derive_internal_secret_key( &self, - child_index: u32, + child_index: NonHardenedChildIndex, ) -> Result { - self.0 - .derive_private_key(KeyIndex::Normal(1))? - .derive_private_key(KeyIndex::Normal(child_index)) - .map(|k| k.private_key) + self.derive_secret_key(zip32::Scope::Internal.into(), child_index) } /// Returns the `AccountPrivKey` serialized using the encoding for a @@ -292,7 +387,11 @@ impl ExternalOvk { #[cfg(test)] mod tests { + use hdwallet::KeyIndex; + use subtle::ConstantTimeEq; + use super::AccountPubKey; + use super::NonHardenedChildIndex; #[test] fn check_ovk_test_vectors() { @@ -539,4 +638,54 @@ mod tests { assert_eq!(tv.external_ovk, external.as_bytes()); } } + + #[test] + fn nonhardened_indexes_accepted() { + assert_eq!(0, NonHardenedChildIndex::from_index(0).unwrap().index()); + assert_eq!( + 0x7fffffff, + NonHardenedChildIndex::from_index(0x7fffffff) + .unwrap() + .index() + ); + } + + #[test] + fn hardened_indexes_rejected() { + assert!(NonHardenedChildIndex::from_index(0x80000000).is_none()); + assert!(NonHardenedChildIndex::from_index(0xffffffff).is_none()); + } + + #[test] + fn nonhardened_index_next() { + assert_eq!(1, NonHardenedChildIndex::ZERO.next().unwrap().index()); + assert!(NonHardenedChildIndex::from_index(0x7fffffff) + .unwrap() + .next() + .is_none()); + } + + #[test] + fn nonhardened_index_ct_eq() { + assert!(check( + NonHardenedChildIndex::ZERO, + NonHardenedChildIndex::ZERO + )); + assert!(!check( + NonHardenedChildIndex::ZERO, + NonHardenedChildIndex::ZERO.next().unwrap() + )); + + fn check(v1: T, v2: T) -> bool { + v1.ct_eq(&v2).into() + } + } + + #[test] + fn nonhardened_index_tryfrom_keyindex() { + let nh: NonHardenedChildIndex = KeyIndex::Normal(0).try_into().unwrap(); + assert_eq!(nh.index(), 0); + + assert!(NonHardenedChildIndex::try_from(KeyIndex::Hardened(0)).is_err()); + } } diff --git a/zcash_primitives/src/transaction/builder.rs b/zcash_primitives/src/transaction/builder.rs index cfe02aea1..95e051536 100644 --- a/zcash_primitives/src/transaction/builder.rs +++ b/zcash_primitives/src/transaction/builder.rs @@ -950,7 +950,7 @@ mod tests { #[cfg(feature = "transparent-inputs")] fn binding_sig_absent_if_no_shielded_spend_or_output() { use crate::consensus::NetworkUpgrade; - use crate::legacy::NonHardenedChildIndex; + use crate::legacy::keys::NonHardenedChildIndex; use crate::transaction::builder::{self, TransparentBuilder}; let sapling_activation_height = TEST_NETWORK