From 6fcdfda69ec5935eb405bdf8180bf327a1e022fe Mon Sep 17 00:00:00 2001 From: Kris Nuttycombe Date: Wed, 26 Jan 2022 13:24:56 -0700 Subject: [PATCH] Derive OVKs from transparent account-level key, not child keys. This also renames a number of legacy key types to better reflect their intended use. --- zcash_client_backend/src/data_api/wallet.rs | 18 +- zcash_client_backend/src/keys.rs | 29 +-- zcash_client_sqlite/src/lib.rs | 4 +- zcash_client_sqlite/src/wallet/init.rs | 9 +- zcash_primitives/src/legacy/keys.rs | 213 ++++++++++++-------- 5 files changed, 155 insertions(+), 118 deletions(-) diff --git a/zcash_client_backend/src/data_api/wallet.rs b/zcash_client_backend/src/data_api/wallet.rs index 412868915..ab9ad9e8f 100644 --- a/zcash_client_backend/src/data_api/wallet.rs +++ b/zcash_client_backend/src/data_api/wallet.rs @@ -12,7 +12,9 @@ use zcash_primitives::{ }; #[cfg(feature = "transparent-inputs")] -use zcash_primitives::{keys::OutgoingViewingKey, legacy::keys as transparent}; +use zcash_primitives::{ + keys::OutgoingViewingKey, legacy::keys as transparent, legacy::keys::IncomingViewingKey, +}; use crate::{ address::RecipientAddress, @@ -378,7 +380,7 @@ pub fn shield_transparent_funds( wallet_db: &mut D, params: &P, prover: impl TxProver, - sk: &transparent::ExternalPrivKey, + sk: &transparent::AccountPrivKey, extfvk: &ExtendedFullViewingKey, account: AccountId, memo: &MemoBytes, @@ -401,9 +403,8 @@ where .and_then(|x| x.ok_or_else(|| Error::ScanRequired.into()))?; // derive the t-address for the extpubkey at child index 0 - let t_ext_pubkey = sk.to_external_pubkey(); - let taddr = t_ext_pubkey.to_address(); - let ovk = OutgoingViewingKey(t_ext_pubkey.internal_ovk().as_bytes()); + let account_pubkey = sk.to_account_pubkey(); + let ovk = OutgoingViewingKey(account_pubkey.internal_ovk().as_bytes()); // derive own shielded address from the provided extended spending key // TODO: this should become the internal change address derived from @@ -411,6 +412,10 @@ where let z_address = extfvk.default_address().1; // get UTXOs from DB + let (taddr, child_index) = account_pubkey + .derive_external_ivk() + .unwrap() + .default_address(); let utxos = wallet_db.get_unspent_transparent_outputs(&taddr, latest_anchor)?; let total_amount = utxos .iter() @@ -427,9 +432,10 @@ where let mut builder = Builder::new(params.clone(), latest_scanned_height); + let secret_key = sk.derive_external_secret_key(child_index).unwrap(); for utxo in &utxos { builder - .add_transparent_input(*sk.secret_key(), utxo.outpoint.clone(), utxo.txout.clone()) + .add_transparent_input(secret_key, utxo.outpoint.clone(), utxo.txout.clone()) .map_err(Error::Builder)?; } diff --git a/zcash_client_backend/src/keys.rs b/zcash_client_backend/src/keys.rs index 570ac3cf8..bccff28c4 100644 --- a/zcash_client_backend/src/keys.rs +++ b/zcash_client_backend/src/keys.rs @@ -229,7 +229,7 @@ mod tests { super::transparent, crate::encoding::AddressCodec, secp256k1::key::SecretKey, - zcash_primitives::{consensus::MAIN_NETWORK, legacy}, + zcash_primitives::{consensus::MAIN_NETWORK, legacy, legacy::keys::IncomingViewingKey}, }; #[cfg(feature = "transparent-inputs")] @@ -251,7 +251,7 @@ mod tests { .unwrap() .derive_external_secret_key(0) .unwrap(); - let wif = transparent::Wif::from_secret_key(&MAIN_NETWORK, &sk.secret_key(), true).0; + let wif = transparent::Wif::from_secret_key(&MAIN_NETWORK, &sk, true).0; assert_eq!( wif, "L4BvDC33yLjMRxipZvdiUmdYeRfZmR8viziwsVwe72zJdGbiJPv2".to_string() @@ -270,30 +270,17 @@ mod tests { assert_eq!(taddr, "t1PKtYdJJHhc3Pxowmznkg7vdTwnhEsCvR4".to_string()); } - #[cfg(feature = "transparent-inputs")] - #[test] - fn pk_from_seed() { - let pk = legacy::keys::AccountPrivKey::from_seed(&MAIN_NETWORK, &seed(), AccountId(0)) - .unwrap() - .derive_external_secret_key(0) - .unwrap() - .to_external_pubkey(); - let hex_value = hex::encode(&pk.public_key().serialize()); - assert_eq!( - hex_value, - "03b1d7fb28d17c125b504d06b1530097e0a3c76ada184237e3bc0925041230a5af".to_string() - ); - } - #[cfg(feature = "transparent-inputs")] #[test] fn pk_to_taddr() { - let pk = legacy::keys::AccountPrivKey::from_seed(&MAIN_NETWORK, &seed(), AccountId(0)) + let taddr = legacy::keys::AccountPrivKey::from_seed(&MAIN_NETWORK, &seed(), AccountId(0)) .unwrap() - .derive_external_secret_key(0) + .to_account_pubkey() + .derive_external_ivk() .unwrap() - .to_external_pubkey(); - let taddr = pk.to_address().encode(&MAIN_NETWORK); + .derive_address(0) + .unwrap() + .encode(&MAIN_NETWORK); assert_eq!(taddr, "t1PKtYdJJHhc3Pxowmznkg7vdTwnhEsCvR4".to_string()); } } diff --git a/zcash_client_sqlite/src/lib.rs b/zcash_client_sqlite/src/lib.rs index 9d87d828e..73b9f657f 100644 --- a/zcash_client_sqlite/src/lib.rs +++ b/zcash_client_sqlite/src/lib.rs @@ -707,7 +707,7 @@ mod tests { }; #[cfg(feature = "transparent-inputs")] - use zcash_primitives::legacy; + use zcash_primitives::{legacy, legacy::keys::IncomingViewingKey}; use zcash_primitives::{ block::BlockHash, @@ -771,7 +771,7 @@ mod tests { init_accounts_table(db_data, &[ufvk]).unwrap(); ( extfvk, - tkey.map(|k| k.derive_external_pubkey(0).unwrap().to_address()), + tkey.map(|k| k.derive_external_ivk().unwrap().default_address().0), ) } diff --git a/zcash_client_sqlite/src/wallet/init.rs b/zcash_client_sqlite/src/wallet/init.rs index 970f58196..41714b55e 100644 --- a/zcash_client_sqlite/src/wallet/init.rs +++ b/zcash_client_sqlite/src/wallet/init.rs @@ -14,7 +14,10 @@ use zcash_client_backend::{ use crate::{address_from_extfvk, error::SqliteClientError, WalletDb}; #[cfg(feature = "transparent-inputs")] -use zcash_client_backend::encoding::AddressCodec; +use { + zcash_client_backend::encoding::AddressCodec, + zcash_primitives::legacy::keys::IncomingViewingKey, +}; /// Sets up the internal structure of the data database. /// @@ -207,9 +210,9 @@ pub fn init_accounts_table( .map(|extfvk| address_from_extfvk(&wdb.params, extfvk)); #[cfg(feature = "transparent-inputs")] let taddress_str: Option = key.transparent().and_then(|k| { - k.derive_external_pubkey(0) + k.derive_external_ivk() .ok() - .map(|k| k.to_address().encode(&wdb.params)) + .map(|k| k.default_address().0.encode(&wdb.params)) }); #[cfg(not(feature = "transparent-inputs"))] let taddress_str: Option = None; diff --git a/zcash_primitives/src/legacy/keys.rs b/zcash_primitives/src/legacy/keys.rs index 665a1a975..990952f3f 100644 --- a/zcash_primitives/src/legacy/keys.rs +++ b/zcash_primitives/src/legacy/keys.rs @@ -7,17 +7,19 @@ use crate::{consensus, keys::prf_expand_vec, zip32::AccountId}; use super::TransparentAddress; +const MAX_TRANSPARENT_CHILD_INDEX: u32 = 0x7FFFFFFF; + /// A type representing a BIP-44 private key at the account path level /// `m/44'/'/' #[derive(Clone, Debug)] pub struct AccountPrivKey(ExtendedPrivKey); impl AccountPrivKey { - /// Perform derivation of the extended private key for the BIP-44 path: - /// `m/44'/'/' + /// Performs derivation of the extended private key for the BIP-44 path: + /// `m/44'/'/'`. /// - /// This produces the extended private key for the external (non-change) - /// address at the specified index for the provided account. + /// This produces the root of the derivation tree for transparent + /// viewing keys and addresses for the for the provided account. pub fn from_seed( params: &P, seed: &[u8], @@ -42,98 +44,54 @@ impl AccountPrivKey { AccountPubKey(ExtendedPubKey::from_private_key(&self.0)) } - /// Derive BIP-44 private key at the external child path - /// `m/44'/'/'/0/ + /// Derives the BIP-44 private spending key for the external (incoming payment) child path + /// `m/44'/'/'/0/`. pub fn derive_external_secret_key( &self, child_index: u32, - ) -> Result { + ) -> Result { self.0 .derive_private_key(KeyIndex::Normal(0))? .derive_private_key(KeyIndex::Normal(child_index)) - .map(ExternalPrivKey) + .map(|k| k.private_key) + } + + /// Derives the BIP-44 private spending key for the internal (change) child path + /// `m/44'/'/'/1/`. + pub fn derive_internal_secret_key( + &self, + child_index: u32, + ) -> Result { + self.0 + .derive_private_key(KeyIndex::Normal(1))? + .derive_private_key(KeyIndex::Normal(child_index)) + .map(|k| k.private_key) } } /// A type representing a BIP-44 public key at the account path level -/// `m/44'/'/' +/// `m/44'/'/'`. +/// +/// This provides the necessary derivation capability for the for +/// the transparent component of a unified full viewing key. #[derive(Clone, Debug)] pub struct AccountPubKey(ExtendedPubKey); impl AccountPubKey { - /// Derive BIP-44 public key at the external child path - /// `m/44'/'/'/0/ - pub fn derive_external_pubkey( - &self, - child_index: u32, - ) -> Result { + /// Derives the BIP-44 public key at the external "change level" path + /// `m/44'/'/'/0`. + pub fn derive_external_ivk(&self) -> Result { self.0 - .derive_public_key(KeyIndex::Normal(0))? - .derive_public_key(KeyIndex::Normal(child_index)) - .map(ExternalPubKey) + .derive_public_key(KeyIndex::Normal(0)) + .map(ExternalIvk) } - pub fn from_extended_pubkey(extpubkey: ExtendedPubKey) -> Self { - AccountPubKey(extpubkey) - } - - pub fn extended_pubkey(&self) -> &ExtendedPubKey { - &self.0 - } -} - -/// A type representing a private key at the BIP-44 external child -/// level `m/44'/'/'/0/ -#[derive(Clone, Debug)] -pub struct ExternalPrivKey(ExtendedPrivKey); - -impl ExternalPrivKey { - /// Returns the external public key corresponding to this private key - pub fn to_external_pubkey(&self) -> ExternalPubKey { - ExternalPubKey(ExtendedPubKey::from_private_key(&self.0)) - } - - /// Extracts the secp256k1 secret key component - pub fn secret_key(&self) -> &secp256k1::key::SecretKey { - &self.0.private_key - } -} - -pub fn pubkey_to_address(pubkey: &secp256k1::key::PublicKey) -> TransparentAddress { - let mut hash160 = ripemd160::Ripemd160::new(); - hash160.update(Sha256::digest(&pubkey.serialize())); - TransparentAddress::PublicKey(*hash160.finalize().as_ref()) -} - -/// A type representing a public key at the BIP-44 external child -/// level `m/44'/'/'/0/ -#[derive(Clone, Debug)] -pub struct ExternalPubKey(ExtendedPubKey); - -impl std::convert::TryFrom<&[u8; 65]> for ExternalPubKey { - type Error = hdwallet::error::Error; - - fn try_from(data: &[u8; 65]) -> Result { - ExternalPubKey::deserialize(data) - } -} - -impl ExternalPubKey { - /// Returns the transparent address corresponding to - /// this public key. - pub fn to_address(&self) -> TransparentAddress { - pubkey_to_address(&self.0.public_key) - } - - /// Returns the secp256k1::key::PublicKey component of - /// this public key. - pub fn public_key(&self) -> &secp256k1::key::PublicKey { - &self.0.public_key - } - - /// Returns the chain code component of this public key. - pub fn chain_code(&self) -> &[u8] { - &self.0.chain_code + /// Derives the BIP-44 public key at the internal "change level" path + /// `m/44'/'/'/1`. + pub fn derive_internal_ivk(&self) -> Result { + self.0 + .derive_public_key(KeyIndex::Normal(1)) + .map(InternalIvk) } /// Derives the internal ovk and external ovk corresponding to this @@ -142,8 +100,8 @@ impl ExternalPubKey { /// [transparent-ovk]: https://zips.z.cash/zip-0316#deriving-internal-keys pub fn ovks_for_shielding(&self) -> (InternalOvk, ExternalOvk) { let i_ovk = prf_expand_vec( - &self.chain_code(), - &[&[0xd0], &self.public_key().serialize()], + &self.0.chain_code, + &[&[0xd0], &self.0.public_key.serialize()], ); let i_ovk = i_ovk.as_bytes(); let ovk_internal = InternalOvk(i_ovk[..32].try_into().unwrap()); @@ -161,23 +119,106 @@ impl ExternalPubKey { pub fn external_ovk(&self) -> ExternalOvk { self.ovks_for_shielding().1 } +} - pub fn serialize(&self) -> Vec { - let mut buf = self.0.chain_code.clone(); - buf.extend(self.0.public_key.serialize().to_vec()); +pub fn pubkey_to_address(pubkey: &secp256k1::key::PublicKey) -> TransparentAddress { + let mut hash160 = ripemd160::Ripemd160::new(); + hash160.update(Sha256::digest(&pubkey.serialize())); + TransparentAddress::PublicKey(*hash160.finalize().as_ref()) +} + +pub(crate) mod private { + use hdwallet::ExtendedPubKey; + pub trait SealedChangeLevelKey { + fn extended_pubkey(&self) -> &ExtendedPubKey; + fn from_extended_pubkey(key: ExtendedPubKey) -> Self; + } +} + +pub trait IncomingViewingKey: private::SealedChangeLevelKey + std::marker::Sized { + /// Derives a transparent address at the provided child index. + fn derive_address( + &self, + child_index: u32, + ) -> Result { + let child_key = self + .extended_pubkey() + .derive_public_key(KeyIndex::Normal(child_index))?; + Ok(pubkey_to_address(&child_key.public_key)) + } + + /// Searches the space of child indexes for an index that will + /// generate a valid transparent address, and returns the resulting + /// address and the index at which it was generated. + fn default_address(&self) -> (TransparentAddress, u32) { + let mut child_index = 0; + while child_index <= MAX_TRANSPARENT_CHILD_INDEX { + match self.derive_address(child_index) { + Ok(addr) => { + return (addr, child_index); + } + Err(_) => { + child_index += 1; + } + } + } + panic!("Exhausted child index space attempting to find a default address."); + } + + fn serialize(&self) -> Vec { + let extpubkey = self.extended_pubkey(); + let mut buf = extpubkey.chain_code.clone(); + buf.extend(extpubkey.public_key.serialize().to_vec()); buf } - pub fn deserialize(data: &[u8; 65]) -> Result { + fn deserialize(data: &[u8; 65]) -> Result { let chain_code = data[..32].to_vec(); let public_key = PublicKey::from_slice(&data[32..])?; - Ok(ExternalPubKey(ExtendedPubKey { + Ok(Self::from_extended_pubkey(ExtendedPubKey { public_key, chain_code, })) } } +/// A type representing an incoming viewing key at the BIP-44 "external" +/// path `m/44'/'/'/0`. This allows derivation +/// of child addresses that may be provided to external parties. +#[derive(Clone, Debug)] +pub struct ExternalIvk(ExtendedPubKey); + +impl private::SealedChangeLevelKey for ExternalIvk { + fn extended_pubkey(&self) -> &ExtendedPubKey { + &self.0 + } + + fn from_extended_pubkey(key: ExtendedPubKey) -> Self { + ExternalIvk(key) + } +} + +impl IncomingViewingKey for ExternalIvk {} + +/// A type representing an incoming viewing key at the BIP-44 "internal" +/// path `m/44'/'/'/1`. This allows derivation +/// of change addresses for use within the wallet, but which should +/// not be shared with external parties. +#[derive(Clone, Debug)] +pub struct InternalIvk(ExtendedPubKey); + +impl private::SealedChangeLevelKey for InternalIvk { + fn extended_pubkey(&self) -> &ExtendedPubKey { + &self.0 + } + + fn from_extended_pubkey(key: ExtendedPubKey) -> Self { + InternalIvk(key) + } +} + +impl IncomingViewingKey for InternalIvk {} + /// Internal ovk used for autoshielding. pub struct InternalOvk([u8; 32]);