From df1105b99688047ff0ce923b6eb3448b1c2b2a32 Mon Sep 17 00:00:00 2001 From: Jack Grigg Date: Mon, 6 Jun 2022 17:12:54 +0000 Subject: [PATCH 1/9] zcash_primitives: Add `DiversifiableFullViewingKey` --- zcash_primitives/CHANGELOG.md | 3 + zcash_primitives/src/sapling/keys.rs | 179 ++++++++++++++++++++++++++- zcash_primitives/src/zip32.rs | 2 +- 3 files changed, 178 insertions(+), 6 deletions(-) diff --git a/zcash_primitives/CHANGELOG.md b/zcash_primitives/CHANGELOG.md index dae7e392d..b24ad1c01 100644 --- a/zcash_primitives/CHANGELOG.md +++ b/zcash_primitives/CHANGELOG.md @@ -6,6 +6,9 @@ and this library adheres to Rust's notion of [Semantic Versioning](https://semver.org/spec/v2.0.0.html). ## [Unreleased] +### Added +- `zcash_primitives::sapling::keys::DiversifiableFullViewingKey` +- `zcash_primitives::sapling::keys::Scope` ## [0.6.0] - 2022-05-11 ### Added diff --git a/zcash_primitives/src/sapling/keys.rs b/zcash_primitives/src/sapling/keys.rs index 80de1379f..297ef2bad 100644 --- a/zcash_primitives/src/sapling/keys.rs +++ b/zcash_primitives/src/sapling/keys.rs @@ -4,16 +4,20 @@ //! //! [section 4.2.2]: https://zips.z.cash/protocol/protocol.pdf#saplingkeycomponents +use std::convert::TryInto; +use std::io::{self, Read, Write}; + use crate::{ constants::{PROOF_GENERATION_KEY_GENERATOR, SPENDING_KEY_GENERATOR}, keys::{prf_expand, OutgoingViewingKey}, - sapling::{ProofGenerationKey, ViewingKey}, + zip32, }; use ff::PrimeField; use group::{Group, GroupEncoding}; -use std::io::{self, Read, Write}; use subtle::CtOption; +use super::{PaymentAddress, ProofGenerationKey, SaplingIvk, ViewingKey}; + /// A Sapling expanded spending key #[derive(Clone)] pub struct ExpandedSpendingKey { @@ -22,7 +26,7 @@ pub struct ExpandedSpendingKey { pub ovk: OutgoingViewingKey, } -/// A Sapling full viewing key +/// A Sapling key that provides the capability to view incoming and outgoing transactions. #[derive(Debug)] pub struct FullViewingKey { pub vk: ViewingKey, @@ -157,6 +161,151 @@ impl FullViewingKey { } } +/// The scope of a viewing key or address. +/// +/// A "scope" narrows the visibility or usage to a level below "full". +/// +/// Consistent usage of `Scope` enables the user to provide consistent views over a wallet +/// to other people. For example, a user can give an external [`SaplingIvk`] to a merchant +/// terminal, enabling it to only detect "real" transactions from customers and not +/// internal transactions from the wallet. +#[derive(Clone, Copy, Debug, PartialEq, Eq)] +pub enum Scope { + /// A scope used for wallet-external operations, namely deriving addresses to give to + /// other users in order to receive funds. + External, + /// A scope used for wallet-internal operations, such as creating change notes, + /// auto-shielding, and note management. + Internal, +} + +/// A Sapling key that provides the capability to view incoming and outgoing transactions. +/// +/// This key is useful anywhere you need to maintain accurate balance, but do not want the +/// ability to spend funds (such as a view-only wallet). +/// +/// It comprises the subset of the ZIP 32 extended full viewing key that is used for the +/// Sapling item in a [ZIP 316 Unified Full Viewing Key][zip-0316-ufvk]. +/// +/// [zip-0316-ufvk]: https://zips.z.cash/zip-0316#encoding-of-unified-full-incoming-viewing-keys +#[derive(Clone, Debug)] +pub struct DiversifiableFullViewingKey { + fvk: FullViewingKey, + dk: zip32::DiversifierKey, +} + +impl From for DiversifiableFullViewingKey { + fn from(extfvk: zip32::ExtendedFullViewingKey) -> Self { + Self { + fvk: extfvk.fvk, + dk: extfvk.dk, + } + } +} + +impl DiversifiableFullViewingKey { + /// Parses a `DiversifiableFullViewingKey` from its raw byte encoding. + /// + /// Returns `None` if the bytes do not contain a valid encoding of a diversifiable + /// Sapling full viewing key. + pub fn from_bytes(bytes: &[u8; 128]) -> Option { + FullViewingKey::read(&bytes[..96]).ok().map(|fvk| Self { + fvk, + dk: zip32::DiversifierKey(bytes[96..].try_into().unwrap()), + }) + } + + /// Returns the raw encoding of this `DiversifiableFullViewingKey`. + pub fn to_bytes(&self) -> [u8; 128] { + let mut bytes = [0; 128]; + self.fvk + .write(&mut bytes[..96]) + .expect("slice should be the correct length"); + bytes[96..].copy_from_slice(&self.dk.0); + bytes + } + + /// Derives the internal `DiversifiableFullViewingKey` corresponding to `self` (which + /// is assumed here to be an external DFVK). + fn derive_internal(&self) -> Self { + let (fvk, dk) = zip32::sapling_derive_internal_fvk(&self.fvk, &self.dk); + Self { fvk, dk } + } + + /// Exposes the [`FullViewingKey`] component of this diversifiable full viewing key. + pub fn fvk(&self) -> &FullViewingKey { + &self.fvk + } + + /// Derives an incoming viewing key corresponding to this full viewing key. + pub fn to_ivk(&self, scope: Scope) -> SaplingIvk { + match scope { + Scope::External => self.fvk.vk.ivk(), + Scope::Internal => self.derive_internal().fvk.vk.ivk(), + } + } + + /// Derives an outgoing viewing key corresponding to this full viewing key. + pub fn to_ovk(&self, scope: Scope) -> OutgoingViewingKey { + match scope { + Scope::External => self.fvk.ovk, + Scope::Internal => self.derive_internal().fvk.ovk, + } + } + + /// Attempts to produce a valid payment address for the given diversifier index. + /// + /// Returns `None` if the diversifier index does not produce a valid diversifier for + /// this `DiversifiableFullViewingKey`. + pub fn address(&self, j: zip32::DiversifierIndex) -> Option { + zip32::sapling_address(&self.fvk, &self.dk, j) + } + + /// Finds the next valid payment address starting from the given diversifier index. + /// + /// This searches the diversifier space starting at `j` and incrementing, to find an + /// index which will produce a valid diversifier (a 50% probability for each index). + /// + /// Returns the index at which the valid diversifier was found along with the payment + /// address constructed using that diversifier, or `None` if the maximum index was + /// reached and no valid diversifier was found. + pub fn find_address( + &self, + j: zip32::DiversifierIndex, + ) -> Option<(zip32::DiversifierIndex, PaymentAddress)> { + zip32::sapling_find_address(&self.fvk, &self.dk, j) + } + + /// Attempts to decrypt the given address's diversifier with this full viewing key. + /// + /// This method extracts the diversifier from the given address and decrypts it as a + /// diversifier index, then verifies that this diversifier index produces the same + /// address. Decryption is attempted using both the internal and external parts of the + /// full viewing key. + /// + /// Returns the decrypted diversifier index and its scope, or `None` if the address + /// was not generated from this key. + pub fn decrypt_diversifier( + &self, + addr: &PaymentAddress, + ) -> Option<(zip32::DiversifierIndex, Scope)> { + let j_external = self.dk.diversifier_index(addr.diversifier()); + if self.address(j_external).as_ref() == Some(addr) { + return Some((j_external, Scope::External)); + } + + let j_internal = self + .derive_internal() + .dk + .diversifier_index(addr.diversifier()); + if self.address(j_internal).as_ref() == Some(addr) { + return Some((j_internal, Scope::Internal)); + } + + None + } +} + #[cfg(any(test, feature = "test-dependencies"))] pub mod testing { use proptest::collection::vec; @@ -185,8 +334,8 @@ pub mod testing { mod tests { use group::{Group, GroupEncoding}; - use super::FullViewingKey; - use crate::constants::SPENDING_KEY_GENERATOR; + use super::{DiversifiableFullViewingKey, FullViewingKey}; + use crate::{constants::SPENDING_KEY_GENERATOR, zip32}; #[test] fn ak_must_be_prime_order() { @@ -210,4 +359,24 @@ mod tests { // nk is allowed to be the identity. assert!(FullViewingKey::read(&buf[..]).is_ok()); } + + #[test] + fn dfvk_round_trip() { + let dfvk = { + let extsk = zip32::ExtendedSpendingKey::master(&[]); + let extfvk = zip32::ExtendedFullViewingKey::from(&extsk); + DiversifiableFullViewingKey::from(extfvk) + }; + + // Check value -> bytes -> parsed round trip. + let dfvk_bytes = dfvk.to_bytes(); + let dfvk_parsed = DiversifiableFullViewingKey::from_bytes(&dfvk_bytes).unwrap(); + assert_eq!(dfvk_parsed.fvk.vk.ak, dfvk.fvk.vk.ak); + assert_eq!(dfvk_parsed.fvk.vk.nk, dfvk.fvk.vk.nk); + assert_eq!(dfvk_parsed.fvk.ovk, dfvk.fvk.ovk); + assert_eq!(dfvk_parsed.dk, dfvk.dk); + + // Check bytes -> parsed -> bytes round trip. + assert_eq!(dfvk_parsed.to_bytes(), dfvk_bytes); + } } diff --git a/zcash_primitives/src/zip32.rs b/zcash_primitives/src/zip32.rs index 21df6e3e3..b79f3bb07 100644 --- a/zcash_primitives/src/zip32.rs +++ b/zcash_primitives/src/zip32.rs @@ -316,7 +316,7 @@ pub struct ExtendedFullViewingKey { child_index: ChildIndex, chain_code: ChainCode, pub fvk: FullViewingKey, - dk: DiversifierKey, + pub(crate) dk: DiversifierKey, } impl std::cmp::PartialEq for ExtendedSpendingKey { From e86ba927afbc8c9a51a52e537d0f3552dde654c5 Mon Sep 17 00:00:00 2001 From: Jack Grigg Date: Fri, 10 Jun 2022 22:53:52 +0000 Subject: [PATCH 2/9] zcash_client_backend: Add a fake UFVK encoding We can't use the real ZIP 316 encoding until `UnifiedFullViewingKey` has been altered to not store a Sapling `ExtendedFullViewingKey`. But making that change first requires fully migrating `zcash_client_sqlite` in the same commit (as its entire API is built around `ExtendedFullViewingKey`). Instead, we define a temporary fake encoding, to enable migrating the `zcash_client_sqlite` APIs more incrementally. --- zcash_client_backend/src/keys.rs | 102 ++++++++++++++++++++++++++++++- 1 file changed, 99 insertions(+), 3 deletions(-) diff --git a/zcash_client_backend/src/keys.rs b/zcash_client_backend/src/keys.rs index fa8e10f99..f4a599247 100644 --- a/zcash_client_backend/src/keys.rs +++ b/zcash_client_backend/src/keys.rs @@ -165,6 +165,68 @@ impl UnifiedFullViewingKey { } } + /// Attempts to decode the given string as an encoding of a `UnifiedFullViewingKey` + /// for the given network. + pub fn decode( + params: &P, + encoding: &str, + account: AccountId, + ) -> Result { + encoding + .strip_prefix("DONOTUSEUFVK") + .and_then(|data| hex::decode(data).ok()) + .as_ref() + .and_then(|data| data.split_first()) + .and_then(|(flag, data)| { + #[cfg(feature = "transparent-inputs")] + let (transparent, data) = if flag & 1 != 0 { + if data.len() < 65 { + return None; + } + let (tfvk, data) = data.split_at(65); + ( + Some(legacy::AccountPubKey::deserialize(tfvk.try_into().unwrap()).ok()?), + data, + ) + } else { + (None, data) + }; + + let sapling = if flag & 2 != 0 { + Some(sapling::ExtendedFullViewingKey::read(data).ok()?) + } else { + None + }; + + Some(Self { + account, + #[cfg(feature = "transparent-inputs")] + transparent, + sapling, + }) + }) + .ok_or("TODO Implement real UFVK encoding after fixing struct".to_string()) + } + + /// Returns the string encoding of this `UnifiedFullViewingKey` for the given network. + pub fn encode(&self, params: &P) -> String { + let flag = if self.sapling.is_some() { 2 } else { 0 }; + #[cfg(feature = "transparent-inputs")] + let flag = flag | if self.transparent.is_some() { 1 } else { 0 }; + let mut ufvk = vec![flag]; + + #[cfg(feature = "transparent-inputs")] + if let Some(transparent) = self.transparent.as_ref() { + ufvk.append(&mut transparent.serialize()); + }; + + if let Some(sapling) = self.sapling.as_ref() { + sapling.write(&mut ufvk).unwrap(); + } + + format!("DONOTUSEUFVK{}", hex::encode(&ufvk)) + } + /// Returns the ZIP32 account identifier to which all component /// keys are related. pub fn account(&self) -> AccountId { @@ -256,13 +318,16 @@ impl UnifiedFullViewingKey { #[cfg(test)] mod tests { - use super::sapling; - use zcash_primitives::zip32::AccountId; + use super::{sapling, UnifiedFullViewingKey}; + use zcash_primitives::{ + consensus::MAIN_NETWORK, + zip32::{AccountId, ExtendedFullViewingKey}, + }; #[cfg(feature = "transparent-inputs")] use { crate::encoding::AddressCodec, - zcash_primitives::{consensus::MAIN_NETWORK, legacy, legacy::keys::IncomingViewingKey}, + zcash_primitives::{legacy, legacy::keys::IncomingViewingKey}, }; #[cfg(feature = "transparent-inputs")] @@ -291,4 +356,35 @@ mod tests { .encode(&MAIN_NETWORK); assert_eq!(taddr, "t1PKtYdJJHhc3Pxowmznkg7vdTwnhEsCvR4".to_string()); } + + #[test] + fn ufvk_round_trip() { + let account = 0.into(); + + let sapling = { + let extsk = sapling::spending_key(&[0; 32], 0, account); + Some(ExtendedFullViewingKey::from(&extsk)) + }; + + #[cfg(feature = "transparent-inputs")] + let transparent = { None }; + + let ufvk = UnifiedFullViewingKey::new( + account, + #[cfg(feature = "transparent-inputs")] + transparent, + sapling, + ) + .unwrap(); + + let encoding = ufvk.encode(&MAIN_NETWORK); + let decoded = UnifiedFullViewingKey::decode(&MAIN_NETWORK, &encoding, account).unwrap(); + assert_eq!(decoded.account, ufvk.account); + #[cfg(feature = "transparent-inputs")] + assert_eq!( + decoded.transparent.map(|t| t.serialize()), + ufvk.transparent.map(|t| t.serialize()), + ); + assert_eq!(decoded.sapling, ufvk.sapling); + } } From 0d0527dbf304ecb3fa2fdc57750338ca3a046732 Mon Sep 17 00:00:00 2001 From: Jack Grigg Date: Mon, 13 Jun 2022 17:54:32 +0000 Subject: [PATCH 3/9] zcash_client_sqlite: Store UFVK/UA instead of Sapling ExtFVK/address This is a breaking change to the database format. We don't have support for migrations yet, so existing wallets won't work after this commit until zcash/librustzcash#489 is done. --- zcash_client_backend/src/address.rs | 10 +++++ zcash_client_backend/src/data_api.rs | 1 + zcash_client_sqlite/CHANGELOG.md | 15 ++++--- zcash_client_sqlite/src/lib.rs | 9 ---- zcash_client_sqlite/src/wallet.rs | 61 +++++++++++++++----------- zcash_client_sqlite/src/wallet/init.rs | 26 ++++------- 6 files changed, 66 insertions(+), 56 deletions(-) diff --git a/zcash_client_backend/src/address.rs b/zcash_client_backend/src/address.rs index cfd8960f8..126b9e589 100644 --- a/zcash_client_backend/src/address.rs +++ b/zcash_client_backend/src/address.rs @@ -108,6 +108,11 @@ impl UnifiedAddress { self.sapling.as_ref() } + /// Returns the transparent receiver within this Unified Address, if any. + pub fn transparent(&self) -> Option<&TransparentAddress> { + self.transparent.as_ref() + } + fn to_address(&self, net: Network) -> ZcashAddress { let ua = unified::Address::try_from_items( self.unknown @@ -137,6 +142,11 @@ impl UnifiedAddress { .expect("UnifiedAddress should only be constructed safely"); ZcashAddress::from_unified(net, ua) } + + /// Returns the string encoding of this `UnifiedAddress` for the given network. + pub fn encode(&self, params: &P) -> String { + self.to_address(params_to_network(params)).to_string() + } } /// An address that funds can be sent to. diff --git a/zcash_client_backend/src/data_api.rs b/zcash_client_backend/src/data_api.rs index aba7d2a91..86714048d 100644 --- a/zcash_client_backend/src/data_api.rs +++ b/zcash_client_backend/src/data_api.rs @@ -115,6 +115,7 @@ pub trait WalletRead { /// /// This will return `Ok(None)` if the account identifier does not correspond /// to a known account. + // TODO: This does not appear to be the case. fn get_address(&self, account: AccountId) -> Result, Self::Error>; /// Returns all extended full viewing keys known about by this wallet. diff --git a/zcash_client_sqlite/CHANGELOG.md b/zcash_client_sqlite/CHANGELOG.md index 57854ea32..7f660a4e8 100644 --- a/zcash_client_sqlite/CHANGELOG.md +++ b/zcash_client_sqlite/CHANGELOG.md @@ -18,6 +18,16 @@ and this library adheres to Rust's notion of rewinds exceed supported bounds. ### Changed +- Various **BREAKING CHANGES** have been made to the database tables. These will + require migrations, which may need to be performed in multiple steps. + - The `extfvk` column in the `accounts` table has been replaced by a `ufvk` + column. Values for this column should be derived from the wallet's seed and + the account number; the Sapling component of the resulting Unified Full + Viewing Key should match the old value in the `extfvk` column. + - A new non-null column, `output_pool` has been added to the `sent_notes` + table to enable distinguishing between Sapling and transparent outputs + (and in the future, outputs to other pools). Values for this column should + be assigned by inference from the address type in the stored data. - MSRV is now 1.56.1. - Bumped dependencies to `ff 0.12`, `group 0.12`, `jubjub 0.9`. - Renamed the following to use lower-case abbreviations (matching Rust @@ -38,11 +48,6 @@ and this library adheres to Rust's notion of method to be used in contexts where a transaction has just been constructed, rather than only in the case that a transaction has been decrypted after being retrieved from the network. -- A new non-null column, `output_pool` has been added to the `sent_notes` - table to enable distinguishing between Sapling and transparent outputs - (and in the future, outputs to other pools). This will require a migration, - which may need to be performed in multiple steps. Values for this column - should be assigned by inference from the address type in the stored data. ### Deprecated - A number of public API methods that are used internally to support the diff --git a/zcash_client_sqlite/src/lib.rs b/zcash_client_sqlite/src/lib.rs index 64a2752d9..a892cdcfd 100644 --- a/zcash_client_sqlite/src/lib.rs +++ b/zcash_client_sqlite/src/lib.rs @@ -53,7 +53,6 @@ use zcash_client_backend::{ data_api::{ BlockSource, DecryptedTransaction, PrunedBlock, SentTransaction, WalletRead, WalletWrite, }, - encoding::encode_payment_address, proto::compact_formats::CompactBlock, wallet::SpendableNote, }; @@ -721,14 +720,6 @@ impl BlockSource for BlockDb { } } -fn address_from_extfvk( - params: &P, - extfvk: &ExtendedFullViewingKey, -) -> String { - let addr = extfvk.default_address().1; - encode_payment_address(params.hrp_sapling_payment_address(), &addr) -} - #[cfg(test)] mod tests { use ff::PrimeField; diff --git a/zcash_client_sqlite/src/wallet.rs b/zcash_client_sqlite/src/wallet.rs index 8b17042ef..7186f4876 100644 --- a/zcash_client_sqlite/src/wallet.rs +++ b/zcash_client_sqlite/src/wallet.rs @@ -23,11 +23,10 @@ use zcash_primitives::{ }; use zcash_client_backend::{ + address::RecipientAddress, data_api::error::Error, - encoding::{ - decode_extended_full_viewing_key, decode_payment_address, encode_extended_full_viewing_key, - encode_payment_address_p, encode_transparent_address_p, - }, + encoding::{encode_payment_address_p, encode_transparent_address_p}, + keys::UnifiedFullViewingKey, wallet::{WalletShieldedOutput, WalletTx}, DecryptedOutput, }; @@ -162,8 +161,15 @@ pub fn get_address( |row| row.get(0), )?; - decode_payment_address(wdb.params.hrp_sapling_payment_address(), &addr) - .map_err(SqliteClientError::Bech32) + RecipientAddress::decode(&wdb.params, &addr) + .ok_or_else(|| { + SqliteClientError::CorruptedData("Not a valid Zcash recipient address".to_owned()) + }) + .map(|addr| match addr { + // TODO: Return the UA, not its Sapling component. + RecipientAddress::Unified(ua) => ua.sapling().cloned(), + _ => None, + }) } /// Returns the [`ExtendedFullViewingKey`]s for the wallet. @@ -178,21 +184,20 @@ pub fn get_extended_full_viewing_keys( // Fetch the ExtendedFullViewingKeys we are tracking let mut stmt_fetch_accounts = wdb .conn - .prepare("SELECT account, extfvk FROM accounts ORDER BY account ASC")?; + .prepare("SELECT account, ufvk FROM accounts ORDER BY account ASC")?; let rows = stmt_fetch_accounts .query_map(NO_PARAMS, |row| { let acct: u32 = row.get(0)?; - let extfvk = row.get(1).map(|extfvk: String| { - decode_extended_full_viewing_key( - wdb.params.hrp_sapling_extended_full_viewing_key(), - &extfvk, - ) - .map_err(SqliteClientError::Bech32) - .and_then(|k| k.ok_or(SqliteClientError::IncorrectHrpExtFvk)) - })?; + let account = AccountId::from(acct); + let ufvk_str: String = row.get(1)?; + let ufvk = UnifiedFullViewingKey::decode(&wdb.params, &ufvk_str, account) + .map_err(SqliteClientError::CorruptedData); + // TODO: Return the UFVK, not its Sapling component. + let extfvk = + ufvk.map(|ufvk| ufvk.sapling().cloned().expect("TODO: Add Orchard support")); - Ok((AccountId::from(acct), extfvk)) + Ok((account, extfvk)) }) .map_err(SqliteClientError::from)?; @@ -218,16 +223,22 @@ pub fn is_valid_account_extfvk( extfvk: &ExtendedFullViewingKey, ) -> Result { wdb.conn - .prepare("SELECT * FROM accounts WHERE account = ? AND extfvk = ?")? - .exists(&[ - u32::from(account).to_sql()?, - encode_extended_full_viewing_key( - wdb.params.hrp_sapling_extended_full_viewing_key(), - extfvk, - ) - .to_sql()?, - ]) + .prepare("SELECT ufvk FROM accounts WHERE account = ?")? + .query_row(&[u32::from(account).to_sql()?], |row| { + row.get(0).map(|ufvk_str: String| { + UnifiedFullViewingKey::decode(&wdb.params, &ufvk_str, account) + .map_err(SqliteClientError::CorruptedData) + }) + }) + .optional() .map_err(SqliteClientError::from) + .and_then(|row| { + if let Some(ufvk) = row { + ufvk.map(|ufvk| ufvk.sapling() == Some(extfvk)) + } else { + Ok(false) + } + }) } /// Returns the balance for the account, including all mined unspent notes that we know diff --git a/zcash_client_sqlite/src/wallet/init.rs b/zcash_client_sqlite/src/wallet/init.rs index 7488bb16c..7ec505cc0 100644 --- a/zcash_client_sqlite/src/wallet/init.rs +++ b/zcash_client_sqlite/src/wallet/init.rs @@ -7,11 +7,9 @@ use zcash_primitives::{ consensus::{self, BlockHeight}, }; -use zcash_client_backend::{ - encoding::encode_extended_full_viewing_key, keys::UnifiedFullViewingKey, -}; +use zcash_client_backend::keys::UnifiedFullViewingKey; -use crate::{address_from_extfvk, error::SqliteClientError, WalletDb}; +use crate::{error::SqliteClientError, WalletDb}; #[cfg(feature = "transparent-inputs")] use { @@ -44,10 +42,12 @@ use { /// init_wallet_db(&db).unwrap(); /// ``` pub fn init_wallet_db

(wdb: &WalletDb

) -> Result<(), rusqlite::Error> { + // TODO: Add migrations (https://github.com/zcash/librustzcash/issues/489) + // - extfvk column -> ufvk column wdb.conn.execute( "CREATE TABLE IF NOT EXISTS accounts ( account INTEGER PRIMARY KEY, - extfvk TEXT, + ufvk TEXT, address TEXT, transparent_address TEXT )", @@ -200,16 +200,8 @@ pub fn init_accounts_table( // Insert accounts atomically wdb.conn.execute("BEGIN IMMEDIATE", NO_PARAMS)?; for key in keys.iter() { - let extfvk_str: Option = key.sapling().map(|extfvk| { - encode_extended_full_viewing_key( - wdb.params.hrp_sapling_extended_full_viewing_key(), - extfvk, - ) - }); - - let address_str: Option = key - .sapling() - .map(|extfvk| address_from_extfvk(&wdb.params, extfvk)); + let ufvk_str: String = key.encode(&wdb.params); + let address_str: String = key.default_address().0.encode(&wdb.params); #[cfg(feature = "transparent-inputs")] let taddress_str: Option = key.transparent().and_then(|k| { k.derive_external_ivk() @@ -220,11 +212,11 @@ pub fn init_accounts_table( let taddress_str: Option = None; wdb.conn.execute( - "INSERT INTO accounts (account, extfvk, address, transparent_address) + "INSERT INTO accounts (account, ufvk, address, transparent_address) VALUES (?, ?, ?, ?)", params![ u32::from(key.account()), - extfvk_str, + ufvk_str, address_str, taddress_str, ], From c0e8ee0fa0e16c830c771dcab2df17cd75499d7c Mon Sep 17 00:00:00 2001 From: Jack Grigg Date: Tue, 14 Jun 2022 00:57:20 +0000 Subject: [PATCH 4/9] zcash_client_backend: Return UFVKs from `WalletRead` instead of ExtFVKs --- zcash_client_backend/CHANGELOG.md | 5 ++++ zcash_client_backend/src/data_api.rs | 12 ++++---- zcash_client_backend/src/data_api/chain.rs | 11 +++++-- zcash_client_backend/src/data_api/wallet.rs | 13 +++++++-- zcash_client_sqlite/src/lib.rs | 13 +++++---- zcash_client_sqlite/src/wallet.rs | 32 +++++++++++++++------ 6 files changed, 61 insertions(+), 25 deletions(-) diff --git a/zcash_client_backend/CHANGELOG.md b/zcash_client_backend/CHANGELOG.md index af2455155..6b6fbc13b 100644 --- a/zcash_client_backend/CHANGELOG.md +++ b/zcash_client_backend/CHANGELOG.md @@ -27,6 +27,8 @@ and this library adheres to Rust's notion of provides substantially greater flexibility in transaction creation. - `zcash_client_backend::address`: - `RecipientAddress::Unified` +- `zcash_client_backend::data_api`: + `WalletRead::get_unified_full_viewing_keys` - `zcash_client_backend::proto`: - `actions` field on `compact_formats::CompactTx` - `compact_formats::CompactOrchardAction` @@ -103,6 +105,9 @@ and this library adheres to Rust's notion of - `Zip321Error::ParseError(String)` ### Removed +- `zcash_client_backend::data_api`: + - `WalletRead::get_extended_full_viewing_keys` (use + `WalletRead::get_unified_full_viewing_keys` instead). - The hardcoded `data_api::wallet::ANCHOR_OFFSET` constant. - `zcash_client_backend::wallet::AccountId` (moved to `zcash_primitives::zip32::AccountId`). diff --git a/zcash_client_backend/src/data_api.rs b/zcash_client_backend/src/data_api.rs index 86714048d..f79a3d08f 100644 --- a/zcash_client_backend/src/data_api.rs +++ b/zcash_client_backend/src/data_api.rs @@ -17,6 +17,7 @@ use zcash_primitives::{ use crate::{ address::RecipientAddress, decrypt::DecryptedOutput, + keys::UnifiedFullViewingKey, proto::compact_formats::CompactBlock, wallet::{SpendableNote, WalletTx}, }; @@ -118,10 +119,10 @@ pub trait WalletRead { // TODO: This does not appear to be the case. fn get_address(&self, account: AccountId) -> Result, Self::Error>; - /// Returns all extended full viewing keys known about by this wallet. - fn get_extended_full_viewing_keys( + /// Returns all unified full viewing keys known to this wallet. + fn get_unified_full_viewing_keys( &self, - ) -> Result, Self::Error>; + ) -> Result, Self::Error>; /// Checks whether the specified extended full viewing key is /// associated with the account. @@ -330,6 +331,7 @@ pub mod testing { }; use crate::{ + keys::UnifiedFullViewingKey, proto::compact_formats::CompactBlock, wallet::{SpendableNote, WalletTransparentOutput}, }; @@ -386,9 +388,9 @@ pub mod testing { Ok(None) } - fn get_extended_full_viewing_keys( + fn get_unified_full_viewing_keys( &self, - ) -> Result, Self::Error> { + ) -> Result, Self::Error> { Ok(HashMap::new()) } diff --git a/zcash_client_backend/src/data_api/chain.rs b/zcash_client_backend/src/data_api/chain.rs index 076912737..a8bade252 100644 --- a/zcash_client_backend/src/data_api/chain.rs +++ b/zcash_client_backend/src/data_api/chain.rs @@ -210,9 +210,14 @@ where .unwrap_or(sapling_activation_height - 1) })?; - // Fetch the ExtendedFullViewingKeys we are tracking - let extfvks = data.get_extended_full_viewing_keys()?; - let extfvks: Vec<(&AccountId, &ExtendedFullViewingKey)> = extfvks.iter().collect(); + // Fetch the UnifiedFullViewingKeys we are tracking + let ufvks = data.get_unified_full_viewing_keys()?; + // TODO: Change `scan_block` to also scan Orchard. + // https://github.com/zcash/librustzcash/issues/403 + let extfvks: Vec<(&AccountId, &ExtendedFullViewingKey)> = ufvks + .iter() + .map(|(account, ufvk)| (account, ufvk.sapling().expect("TODO Add Orchard support"))) + .collect(); // Get the most recent CommitmentTree let mut tree = data diff --git a/zcash_client_backend/src/data_api/wallet.rs b/zcash_client_backend/src/data_api/wallet.rs index fee32c2dd..b6e4c6c77 100644 --- a/zcash_client_backend/src/data_api/wallet.rs +++ b/zcash_client_backend/src/data_api/wallet.rs @@ -41,8 +41,17 @@ where P: consensus::Parameters, D: WalletWrite, { - // Fetch the ExtendedFullViewingKeys we are tracking - let extfvks = data.get_extended_full_viewing_keys()?; + // Fetch the UnifiedFullViewingKeys we are tracking + let ufvks = data.get_unified_full_viewing_keys()?; + let extfvks = ufvks + .into_iter() + .map(|(account, ufvk)| { + ( + account, + ufvk.sapling().cloned().expect("TODO Add Orchard support"), + ) + }) + .collect(); // Height is block height for mined transactions, and the "mempool height" (chain height + 1) // for mempool transactions. diff --git a/zcash_client_sqlite/src/lib.rs b/zcash_client_sqlite/src/lib.rs index a892cdcfd..a1fbe4d44 100644 --- a/zcash_client_sqlite/src/lib.rs +++ b/zcash_client_sqlite/src/lib.rs @@ -53,6 +53,7 @@ use zcash_client_backend::{ data_api::{ BlockSource, DecryptedTransaction, PrunedBlock, SentTransaction, WalletRead, WalletWrite, }, + keys::UnifiedFullViewingKey, proto::compact_formats::CompactBlock, wallet::SpendableNote, }; @@ -224,11 +225,11 @@ impl WalletRead for WalletDb

{ wallet::get_tx_height(self, txid).map_err(SqliteClientError::from) } - fn get_extended_full_viewing_keys( + fn get_unified_full_viewing_keys( &self, - ) -> Result, Self::Error> { + ) -> Result, Self::Error> { #[allow(deprecated)] - wallet::get_extended_full_viewing_keys(self) + wallet::get_unified_full_viewing_keys(self) } fn get_address(&self, account: AccountId) -> Result, Self::Error> { @@ -380,10 +381,10 @@ impl<'a, P: consensus::Parameters> WalletRead for DataConnStmtCache<'a, P> { self.wallet_db.get_tx_height(txid) } - fn get_extended_full_viewing_keys( + fn get_unified_full_viewing_keys( &self, - ) -> Result, Self::Error> { - self.wallet_db.get_extended_full_viewing_keys() + ) -> Result, Self::Error> { + self.wallet_db.get_unified_full_viewing_keys() } fn get_address(&self, account: AccountId) -> Result, Self::Error> { diff --git a/zcash_client_sqlite/src/wallet.rs b/zcash_client_sqlite/src/wallet.rs index 7186f4876..23fd17604 100644 --- a/zcash_client_sqlite/src/wallet.rs +++ b/zcash_client_sqlite/src/wallet.rs @@ -176,12 +176,29 @@ pub fn get_address( /// /// [`ExtendedFullViewingKey`]: zcash_primitives::zip32::ExtendedFullViewingKey #[deprecated( - note = "This function will be removed in a future release. Use zcash_client_backend::data_api::WalletRead::get_extended_full_viewing_keys instead." + note = "This function will be removed in a future release. Use zcash_client_backend::data_api::WalletRead::get_unified_full_viewing_keys instead." )] pub fn get_extended_full_viewing_keys( wdb: &WalletDb

, ) -> Result, SqliteClientError> { - // Fetch the ExtendedFullViewingKeys we are tracking + get_unified_full_viewing_keys(wdb).map(|ufvks| { + ufvks + .into_iter() + .map(|(account, ufvk)| { + ( + account, + ufvk.sapling().cloned().expect("TODO: Add Orchard support"), + ) + }) + .collect() + }) +} + +/// Returns the [`UnifiedFullViewingKey`]s for the wallet. +pub(crate) fn get_unified_full_viewing_keys( + wdb: &WalletDb

, +) -> Result, SqliteClientError> { + // Fetch the UnifiedFullViewingKeys we are tracking let mut stmt_fetch_accounts = wdb .conn .prepare("SELECT account, ufvk FROM accounts ORDER BY account ASC")?; @@ -193,18 +210,15 @@ pub fn get_extended_full_viewing_keys( let ufvk_str: String = row.get(1)?; let ufvk = UnifiedFullViewingKey::decode(&wdb.params, &ufvk_str, account) .map_err(SqliteClientError::CorruptedData); - // TODO: Return the UFVK, not its Sapling component. - let extfvk = - ufvk.map(|ufvk| ufvk.sapling().cloned().expect("TODO: Add Orchard support")); - Ok((account, extfvk)) + Ok((account, ufvk)) }) .map_err(SqliteClientError::from)?; - let mut res: HashMap = HashMap::new(); + let mut res: HashMap = HashMap::new(); for row in rows { - let (account_id, efvkr) = row?; - res.insert(account_id, efvkr?); + let (account_id, ufvkr) = row?; + res.insert(account_id, ufvkr?); } Ok(res) From 1ce289e56818746f520b0bcc19af606cb04d3008 Mon Sep 17 00:00:00 2001 From: Jack Grigg Date: Tue, 14 Jun 2022 01:13:00 +0000 Subject: [PATCH 5/9] zcash_client_backend: Pass UFVKs into `decrypt_transaction` --- zcash_client_backend/CHANGELOG.md | 3 +++ zcash_client_backend/src/data_api/wallet.rs | 11 +---------- zcash_client_backend/src/decrypt.rs | 11 +++++++---- 3 files changed, 11 insertions(+), 14 deletions(-) diff --git a/zcash_client_backend/CHANGELOG.md b/zcash_client_backend/CHANGELOG.md index 6b6fbc13b..922ba99d9 100644 --- a/zcash_client_backend/CHANGELOG.md +++ b/zcash_client_backend/CHANGELOG.md @@ -90,6 +90,9 @@ and this library adheres to Rust's notion of - An `Error::MemoForbidden` error has been added to the `data_api::error::Error` enum to report the condition where a memo was specified to be sent to a transparent recipient. +- `zcash_client_backend::decrypt`: + - `decrypt_transaction` now takes a `HashMap<_, UnifiedFullViewingKey>` + instead of `HashMap<_, ExtendedFullViewingKey>`. - If no memo is provided when sending to a shielded recipient, the empty memo will be used - `zcash_client_backend::keys::spending_key` has been moved to the diff --git a/zcash_client_backend/src/data_api/wallet.rs b/zcash_client_backend/src/data_api/wallet.rs index b6e4c6c77..13a4f1b96 100644 --- a/zcash_client_backend/src/data_api/wallet.rs +++ b/zcash_client_backend/src/data_api/wallet.rs @@ -43,15 +43,6 @@ where { // Fetch the UnifiedFullViewingKeys we are tracking let ufvks = data.get_unified_full_viewing_keys()?; - let extfvks = ufvks - .into_iter() - .map(|(account, ufvk)| { - ( - account, - ufvk.sapling().cloned().expect("TODO Add Orchard support"), - ) - }) - .collect(); // Height is block height for mined transactions, and the "mempool height" (chain height + 1) // for mempool transactions. @@ -63,7 +54,7 @@ where .or_else(|| params.activation_height(NetworkUpgrade::Sapling)) .ok_or(Error::SaplingNotActive)?; - let sapling_outputs = decrypt_transaction(params, height, tx, &extfvks); + let sapling_outputs = decrypt_transaction(params, height, tx, &ufvks); if !(sapling_outputs.is_empty() && tx.transparent_bundle().iter().all(|b| b.vout.is_empty())) { data.store_decrypted_tx(&DecryptedTransaction { diff --git a/zcash_client_backend/src/decrypt.rs b/zcash_client_backend/src/decrypt.rs index 16e59ad93..f7400e3a1 100644 --- a/zcash_client_backend/src/decrypt.rs +++ b/zcash_client_backend/src/decrypt.rs @@ -8,9 +8,11 @@ use zcash_primitives::{ Note, PaymentAddress, }, transaction::Transaction, - zip32::{AccountId, ExtendedFullViewingKey}, + zip32::AccountId, }; +use crate::keys::UnifiedFullViewingKey; + /// A decrypted shielded output. pub struct DecryptedOutput { /// The index of the output within [`shielded_outputs`]. @@ -33,17 +35,18 @@ pub struct DecryptedOutput { } /// Scans a [`Transaction`] for any information that can be decrypted by the set of -/// [`ExtendedFullViewingKey`]s. +/// [`UnifiedFullViewingKey`]s. pub fn decrypt_transaction( params: &P, height: BlockHeight, tx: &Transaction, - extfvks: &HashMap, + ufvks: &HashMap, ) -> Vec { let mut decrypted = vec![]; if let Some(bundle) = tx.sapling_bundle() { - for (account, extfvk) in extfvks.iter() { + for (account, ufvk) in ufvks.iter() { + let extfvk = ufvk.sapling().expect("TODO: Add Orchard support"); let ivk = extfvk.fvk.vk.ivk(); let ovk = extfvk.fvk.ovk; From d8b860207d3f9bbab33b3e50b72999b6674be9a8 Mon Sep 17 00:00:00 2001 From: Jack Grigg Date: Tue, 14 Jun 2022 01:56:46 +0000 Subject: [PATCH 6/9] zcash_client_backend: Remove account from `UnifiedFullViewingKey` The account number is not stored in the ZIP 316 UFVK encoding, and in general won't necessarily be known (e.g. if a UFVK is being imported into a wallet). `zcash_client_sqlite::wallet::init::init_accounts_table` reverts to its previous behaviour of requiring the provided `&[UnifiedFullViewingKey]` to be indexed by account number. --- zcash_client_backend/src/keys.rs | 21 +--------- zcash_client_sqlite/src/lib.rs | 1 - zcash_client_sqlite/src/wallet.rs | 4 +- zcash_client_sqlite/src/wallet/init.rs | 14 ++----- zcash_client_sqlite/src/wallet/transact.rs | 46 ++++++++-------------- 5 files changed, 24 insertions(+), 62 deletions(-) diff --git a/zcash_client_backend/src/keys.rs b/zcash_client_backend/src/keys.rs index f4a599247..9b0b89889 100644 --- a/zcash_client_backend/src/keys.rs +++ b/zcash_client_backend/src/keys.rs @@ -107,7 +107,6 @@ impl UnifiedSpendingKey { pub fn to_unified_full_viewing_key(&self) -> UnifiedFullViewingKey { UnifiedFullViewingKey { - account: self.account, #[cfg(feature = "transparent-inputs")] transparent: Some(self.transparent.to_account_pubkey()), sapling: Some(sapling::ExtendedFullViewingKey::from(&self.sapling)), @@ -137,7 +136,6 @@ impl UnifiedSpendingKey { #[derive(Clone, Debug)] #[doc(hidden)] pub struct UnifiedFullViewingKey { - account: AccountId, #[cfg(feature = "transparent-inputs")] transparent: Option, // TODO: This type is invalid for a UFVK; create a `sapling::DiversifiableFullViewingKey` @@ -149,7 +147,6 @@ pub struct UnifiedFullViewingKey { impl UnifiedFullViewingKey { /// Construct a new unified full viewing key, if the required components are present. pub fn new( - account: AccountId, #[cfg(feature = "transparent-inputs")] transparent: Option, sapling: Option, ) -> Option { @@ -157,7 +154,6 @@ impl UnifiedFullViewingKey { None } else { Some(UnifiedFullViewingKey { - account, #[cfg(feature = "transparent-inputs")] transparent, sapling, @@ -167,11 +163,7 @@ impl UnifiedFullViewingKey { /// Attempts to decode the given string as an encoding of a `UnifiedFullViewingKey` /// for the given network. - pub fn decode( - params: &P, - encoding: &str, - account: AccountId, - ) -> Result { + pub fn decode(params: &P, encoding: &str) -> Result { encoding .strip_prefix("DONOTUSEUFVK") .and_then(|data| hex::decode(data).ok()) @@ -199,7 +191,6 @@ impl UnifiedFullViewingKey { }; Some(Self { - account, #[cfg(feature = "transparent-inputs")] transparent, sapling, @@ -227,12 +218,6 @@ impl UnifiedFullViewingKey { format!("DONOTUSEUFVK{}", hex::encode(&ufvk)) } - /// Returns the ZIP32 account identifier to which all component - /// keys are related. - pub fn account(&self) -> AccountId { - self.account - } - /// Returns the transparent component of the unified key at the /// BIP44 path `m/44'/'/'`. #[cfg(feature = "transparent-inputs")] @@ -370,7 +355,6 @@ mod tests { let transparent = { None }; let ufvk = UnifiedFullViewingKey::new( - account, #[cfg(feature = "transparent-inputs")] transparent, sapling, @@ -378,8 +362,7 @@ mod tests { .unwrap(); let encoding = ufvk.encode(&MAIN_NETWORK); - let decoded = UnifiedFullViewingKey::decode(&MAIN_NETWORK, &encoding, account).unwrap(); - assert_eq!(decoded.account, ufvk.account); + let decoded = UnifiedFullViewingKey::decode(&MAIN_NETWORK, &encoding).unwrap(); #[cfg(feature = "transparent-inputs")] assert_eq!( decoded.transparent.map(|t| t.serialize()), diff --git a/zcash_client_sqlite/src/lib.rs b/zcash_client_sqlite/src/lib.rs index a1fbe4d44..425757c54 100644 --- a/zcash_client_sqlite/src/lib.rs +++ b/zcash_client_sqlite/src/lib.rs @@ -802,7 +802,6 @@ mod tests { let taddr = None; let ufvk = UnifiedFullViewingKey::new( - account, #[cfg(feature = "transparent-inputs")] tkey, Some(extfvk.clone()), diff --git a/zcash_client_sqlite/src/wallet.rs b/zcash_client_sqlite/src/wallet.rs index 23fd17604..aa7f1b14b 100644 --- a/zcash_client_sqlite/src/wallet.rs +++ b/zcash_client_sqlite/src/wallet.rs @@ -208,7 +208,7 @@ pub(crate) fn get_unified_full_viewing_keys( let acct: u32 = row.get(0)?; let account = AccountId::from(acct); let ufvk_str: String = row.get(1)?; - let ufvk = UnifiedFullViewingKey::decode(&wdb.params, &ufvk_str, account) + let ufvk = UnifiedFullViewingKey::decode(&wdb.params, &ufvk_str) .map_err(SqliteClientError::CorruptedData); Ok((account, ufvk)) @@ -240,7 +240,7 @@ pub fn is_valid_account_extfvk( .prepare("SELECT ufvk FROM accounts WHERE account = ?")? .query_row(&[u32::from(account).to_sql()?], |row| { row.get(0).map(|ufvk_str: String| { - UnifiedFullViewingKey::decode(&wdb.params, &ufvk_str, account) + UnifiedFullViewingKey::decode(&wdb.params, &ufvk_str) .map_err(SqliteClientError::CorruptedData) }) }) diff --git a/zcash_client_sqlite/src/wallet/init.rs b/zcash_client_sqlite/src/wallet/init.rs index 7ec505cc0..d589f8ba0 100644 --- a/zcash_client_sqlite/src/wallet/init.rs +++ b/zcash_client_sqlite/src/wallet/init.rs @@ -180,7 +180,7 @@ pub fn init_wallet_db

(wdb: &WalletDb

) -> Result<(), rusqlite::Error> { /// let account = AccountId::from(0); /// let extsk = sapling::spending_key(&seed, Network::TestNetwork.coin_type(), account); /// let extfvk = ExtendedFullViewingKey::from(&extsk); -/// let ufvk = UnifiedFullViewingKey::new(account, None, Some(extfvk)).unwrap(); +/// let ufvk = UnifiedFullViewingKey::new(None, Some(extfvk)).unwrap(); /// init_accounts_table(&db_data, &[ufvk]).unwrap(); /// # } /// ``` @@ -199,7 +199,7 @@ pub fn init_accounts_table( // Insert accounts atomically wdb.conn.execute("BEGIN IMMEDIATE", NO_PARAMS)?; - for key in keys.iter() { + for (account, key) in (0u32..).zip(keys) { let ufvk_str: String = key.encode(&wdb.params); let address_str: String = key.default_address().0.encode(&wdb.params); #[cfg(feature = "transparent-inputs")] @@ -214,12 +214,7 @@ pub fn init_accounts_table( wdb.conn.execute( "INSERT INTO accounts (account, ufvk, address, transparent_address) VALUES (?, ?, ?, ?)", - params![ - u32::from(key.account()), - ufvk_str, - address_str, - taddress_str, - ], + params![account, ufvk_str, address_str, taddress_str], )?; } wdb.conn.execute("COMMIT", NO_PARAMS)?; @@ -328,7 +323,6 @@ mod tests { #[cfg(feature = "transparent-inputs")] let ufvk = UnifiedFullViewingKey::new( - account, Some( transparent::AccountPrivKey::from_seed(&network(), &seed, account) .unwrap() @@ -339,7 +333,7 @@ mod tests { .unwrap(); #[cfg(not(feature = "transparent-inputs"))] - let ufvk = UnifiedFullViewingKey::new(account, Some(extfvk)).unwrap(); + let ufvk = UnifiedFullViewingKey::new(Some(extfvk)).unwrap(); init_accounts_table(&db_data, &[ufvk.clone()]).unwrap(); diff --git a/zcash_client_sqlite/src/wallet/transact.rs b/zcash_client_sqlite/src/wallet/transact.rs index c6ea61916..ef1aa6067 100644 --- a/zcash_client_sqlite/src/wallet/transact.rs +++ b/zcash_client_sqlite/src/wallet/transact.rs @@ -219,24 +219,14 @@ mod tests { transparent::AccountPrivKey::from_seed(&network(), &[1u8; 32], AccountId::from(1)) .unwrap(); [ - UnifiedFullViewingKey::new( - AccountId::from(0), - Some(tsk0.to_account_pubkey()), - Some(extfvk0), - ) - .unwrap(), - UnifiedFullViewingKey::new( - AccountId::from(1), - Some(tsk1.to_account_pubkey()), - Some(extfvk1), - ) - .unwrap(), + UnifiedFullViewingKey::new(Some(tsk0.to_account_pubkey()), Some(extfvk0)).unwrap(), + UnifiedFullViewingKey::new(Some(tsk1.to_account_pubkey()), Some(extfvk1)).unwrap(), ] }; #[cfg(not(feature = "transparent-inputs"))] let ufvks = [ - UnifiedFullViewingKey::new(AccountId::from(0), Some(extfvk0)).unwrap(), - UnifiedFullViewingKey::new(AccountId::from(1), Some(extfvk1)).unwrap(), + UnifiedFullViewingKey::new(Some(extfvk0)).unwrap(), + UnifiedFullViewingKey::new(Some(extfvk1)).unwrap(), ]; init_accounts_table(&db_data, &ufvks).unwrap(); @@ -288,9 +278,9 @@ mod tests { let extfvk = ExtendedFullViewingKey::from(&extsk); #[cfg(feature = "transparent-inputs")] - let ufvk = UnifiedFullViewingKey::new(AccountId::from(0), None, Some(extfvk)).unwrap(); + let ufvk = UnifiedFullViewingKey::new(None, Some(extfvk)).unwrap(); #[cfg(not(feature = "transparent-inputs"))] - let ufvk = UnifiedFullViewingKey::new(AccountId::from(0), Some(extfvk)).unwrap(); + let ufvk = UnifiedFullViewingKey::new(Some(extfvk)).unwrap(); init_accounts_table(&db_data, &[ufvk]).unwrap(); let to = extsk.default_address().1.into(); @@ -331,9 +321,9 @@ mod tests { let extsk = sapling::spending_key(&[0u8; 32], network().coin_type(), AccountId::from(0)); let extfvk = ExtendedFullViewingKey::from(&extsk); #[cfg(feature = "transparent-inputs")] - let ufvk = UnifiedFullViewingKey::new(AccountId::from(0), None, Some(extfvk)).unwrap(); + let ufvk = UnifiedFullViewingKey::new(None, Some(extfvk)).unwrap(); #[cfg(not(feature = "transparent-inputs"))] - let ufvk = UnifiedFullViewingKey::new(AccountId::from(0), Some(extfvk)).unwrap(); + let ufvk = UnifiedFullViewingKey::new(Some(extfvk)).unwrap(); init_accounts_table(&db_data, &[ufvk]).unwrap(); let to = extsk.default_address().1.into(); @@ -379,10 +369,9 @@ mod tests { let extsk = sapling::spending_key(&[0u8; 32], network().coin_type(), AccountId::from(0)); let extfvk = ExtendedFullViewingKey::from(&extsk); #[cfg(feature = "transparent-inputs")] - let ufvk = - UnifiedFullViewingKey::new(AccountId::from(0), None, Some(extfvk.clone())).unwrap(); + let ufvk = UnifiedFullViewingKey::new(None, Some(extfvk.clone())).unwrap(); #[cfg(not(feature = "transparent-inputs"))] - let ufvk = UnifiedFullViewingKey::new(AccountId::from(0), Some(extfvk.clone())).unwrap(); + let ufvk = UnifiedFullViewingKey::new(Some(extfvk.clone())).unwrap(); init_accounts_table(&db_data, &[ufvk]).unwrap(); // Add funds to the wallet in a single note @@ -523,10 +512,9 @@ mod tests { let extsk = sapling::spending_key(&[0u8; 32], network().coin_type(), AccountId::from(0)); let extfvk = ExtendedFullViewingKey::from(&extsk); #[cfg(feature = "transparent-inputs")] - let ufvk = - UnifiedFullViewingKey::new(AccountId::from(0), None, Some(extfvk.clone())).unwrap(); + let ufvk = UnifiedFullViewingKey::new(None, Some(extfvk.clone())).unwrap(); #[cfg(not(feature = "transparent-inputs"))] - let ufvk = UnifiedFullViewingKey::new(AccountId::from(0), Some(extfvk.clone())).unwrap(); + let ufvk = UnifiedFullViewingKey::new(Some(extfvk.clone())).unwrap(); init_accounts_table(&db_data, &[ufvk]).unwrap(); // Add funds to the wallet in a single note @@ -653,10 +641,9 @@ mod tests { let extsk = sapling::spending_key(&[0u8; 32], network.coin_type(), AccountId::from(0)); let extfvk = ExtendedFullViewingKey::from(&extsk); #[cfg(feature = "transparent-inputs")] - let ufvk = - UnifiedFullViewingKey::new(AccountId::from(0), None, Some(extfvk.clone())).unwrap(); + let ufvk = UnifiedFullViewingKey::new(None, Some(extfvk.clone())).unwrap(); #[cfg(not(feature = "transparent-inputs"))] - let ufvk = UnifiedFullViewingKey::new(AccountId::from(0), Some(extfvk.clone())).unwrap(); + let ufvk = UnifiedFullViewingKey::new(Some(extfvk.clone())).unwrap(); init_accounts_table(&db_data, &[ufvk]).unwrap(); // Add funds to the wallet in a single note @@ -764,10 +751,9 @@ mod tests { let extsk = sapling::spending_key(&[0u8; 32], network().coin_type(), AccountId::from(0)); let extfvk = ExtendedFullViewingKey::from(&extsk); #[cfg(feature = "transparent-inputs")] - let ufvk = - UnifiedFullViewingKey::new(AccountId::from(0), None, Some(extfvk.clone())).unwrap(); + let ufvk = UnifiedFullViewingKey::new(None, Some(extfvk.clone())).unwrap(); #[cfg(not(feature = "transparent-inputs"))] - let ufvk = UnifiedFullViewingKey::new(AccountId::from(0), Some(extfvk.clone())).unwrap(); + let ufvk = UnifiedFullViewingKey::new(Some(extfvk.clone())).unwrap(); init_accounts_table(&db_data, &[ufvk]).unwrap(); // Add funds to the wallet in a single note From 76d015ed119cd0ee0cf306690693efba75daa0b4 Mon Sep 17 00:00:00 2001 From: Jack Grigg Date: Tue, 14 Jun 2022 02:27:55 +0000 Subject: [PATCH 7/9] zcash_client_backend: Fix `UnifiedFullViewingKey` Sapling item type Per ZIP 316, the Sapling FVK Encoding only includes `(ak, nk, ovk, dk)` which is a subset of the Sapling `ExtendedFullViewingKey`. We therefore need to use `DiversifiableFullViewingKey` inside `UnifiedFullViewingKey` in order to make it parseable from the UFVK string encoding. `zcash_client_sqlite::wallet::get_extended_full_viewing_keys` has been removed as a consequence of this change: we can no longer reconstruct the correct `ExtendedFullViewingKey` from the `UnifiedFullViewingKey`. --- zcash_client_backend/src/data_api/chain.rs | 5 +- zcash_client_backend/src/decrypt.rs | 6 +- zcash_client_backend/src/keys.rs | 31 ++++---- zcash_client_backend/src/welding_rig.rs | 21 ++++++ zcash_client_sqlite/CHANGELOG.md | 6 ++ zcash_client_sqlite/src/chain.rs | 58 +++++++-------- zcash_client_sqlite/src/lib.rs | 28 +++---- zcash_client_sqlite/src/wallet.rs | 29 ++------ zcash_client_sqlite/src/wallet/init.rs | 11 +-- zcash_client_sqlite/src/wallet/transact.rs | 87 ++++++++++------------ zcash_primitives/src/sapling/keys.rs | 7 ++ 11 files changed, 149 insertions(+), 140 deletions(-) diff --git a/zcash_client_backend/src/data_api/chain.rs b/zcash_client_backend/src/data_api/chain.rs index a8bade252..37faba2f5 100644 --- a/zcash_client_backend/src/data_api/chain.rs +++ b/zcash_client_backend/src/data_api/chain.rs @@ -84,7 +84,6 @@ use zcash_primitives::{ consensus::{self, BlockHeight, NetworkUpgrade}, merkle_tree::CommitmentTree, sapling::Nullifier, - zip32::{AccountId, ExtendedFullViewingKey}, }; use crate::{ @@ -214,7 +213,7 @@ where let ufvks = data.get_unified_full_viewing_keys()?; // TODO: Change `scan_block` to also scan Orchard. // https://github.com/zcash/librustzcash/issues/403 - let extfvks: Vec<(&AccountId, &ExtendedFullViewingKey)> = ufvks + let dfvks: Vec<_> = ufvks .iter() .map(|(account, ufvk)| (account, ufvk.sapling().expect("TODO Add Orchard support"))) .collect(); @@ -249,7 +248,7 @@ where scan_block( params, block, - &extfvks, + &dfvks, &nullifiers, &mut tree, &mut witness_refs[..], diff --git a/zcash_client_backend/src/decrypt.rs b/zcash_client_backend/src/decrypt.rs index f7400e3a1..b845d3dee 100644 --- a/zcash_client_backend/src/decrypt.rs +++ b/zcash_client_backend/src/decrypt.rs @@ -46,9 +46,9 @@ pub fn decrypt_transaction( if let Some(bundle) = tx.sapling_bundle() { for (account, ufvk) in ufvks.iter() { - let extfvk = ufvk.sapling().expect("TODO: Add Orchard support"); - let ivk = extfvk.fvk.vk.ivk(); - let ovk = extfvk.fvk.ovk; + let dfvk = ufvk.sapling().expect("TODO: Add Orchard support"); + let ivk = dfvk.fvk().vk.ivk(); + let ovk = dfvk.fvk().ovk; for (index, output) in bundle.shielded_outputs.iter().enumerate() { let ((note, to, memo), outgoing) = diff --git a/zcash_client_backend/src/keys.rs b/zcash_client_backend/src/keys.rs index 9b0b89889..5aac47b96 100644 --- a/zcash_client_backend/src/keys.rs +++ b/zcash_client_backend/src/keys.rs @@ -1,12 +1,12 @@ //! Helper functions for managing light client key material. use zcash_primitives::{ consensus, + sapling::keys as sapling_keys, zip32::{AccountId, DiversifierIndex}, }; use crate::address::UnifiedAddress; -#[cfg(feature = "transparent-inputs")] use std::convert::TryInto; #[cfg(feature = "transparent-inputs")] @@ -109,7 +109,7 @@ impl UnifiedSpendingKey { UnifiedFullViewingKey { #[cfg(feature = "transparent-inputs")] transparent: Some(self.transparent.to_account_pubkey()), - sapling: Some(sapling::ExtendedFullViewingKey::from(&self.sapling)), + sapling: Some(sapling::ExtendedFullViewingKey::from(&self.sapling).into()), } } @@ -138,9 +138,7 @@ impl UnifiedSpendingKey { pub struct UnifiedFullViewingKey { #[cfg(feature = "transparent-inputs")] transparent: Option, - // TODO: This type is invalid for a UFVK; create a `sapling::DiversifiableFullViewingKey` - // to replace it. - sapling: Option, + sapling: Option, } #[doc(hidden)] @@ -148,7 +146,7 @@ impl UnifiedFullViewingKey { /// Construct a new unified full viewing key, if the required components are present. pub fn new( #[cfg(feature = "transparent-inputs")] transparent: Option, - sapling: Option, + sapling: Option, ) -> Option { if sapling.is_none() { None @@ -185,7 +183,12 @@ impl UnifiedFullViewingKey { }; let sapling = if flag & 2 != 0 { - Some(sapling::ExtendedFullViewingKey::read(data).ok()?) + if data.len() != 128 { + return None; + } + Some(sapling_keys::DiversifiableFullViewingKey::from_bytes( + data.try_into().unwrap(), + )?) } else { None }; @@ -212,7 +215,7 @@ impl UnifiedFullViewingKey { }; if let Some(sapling) = self.sapling.as_ref() { - sapling.write(&mut ufvk).unwrap(); + ufvk.extend_from_slice(&sapling.to_bytes()); } format!("DONOTUSEUFVK{}", hex::encode(&ufvk)) @@ -225,9 +228,8 @@ impl UnifiedFullViewingKey { self.transparent.as_ref() } - /// Returns the Sapling extended full viewing key component of this - /// unified key. - pub fn sapling(&self) -> Option<&sapling::ExtendedFullViewingKey> { + /// Returns the Sapling diversifiable full viewing key component of this unified key. + pub fn sapling(&self) -> Option<&sapling_keys::DiversifiableFullViewingKey> { self.sapling.as_ref() } @@ -348,7 +350,7 @@ mod tests { let sapling = { let extsk = sapling::spending_key(&[0; 32], 0, account); - Some(ExtendedFullViewingKey::from(&extsk)) + Some(ExtendedFullViewingKey::from(&extsk).into()) }; #[cfg(feature = "transparent-inputs")] @@ -368,6 +370,9 @@ mod tests { decoded.transparent.map(|t| t.serialize()), ufvk.transparent.map(|t| t.serialize()), ); - assert_eq!(decoded.sapling, ufvk.sapling); + assert_eq!( + decoded.sapling.map(|s| s.to_bytes()), + ufvk.sapling.map(|s| s.to_bytes()), + ); } } diff --git a/zcash_client_backend/src/welding_rig.rs b/zcash_client_backend/src/welding_rig.rs index 662dea3a1..b1c9251b5 100644 --- a/zcash_client_backend/src/welding_rig.rs +++ b/zcash_client_backend/src/welding_rig.rs @@ -9,6 +9,7 @@ use zcash_primitives::{ consensus::{self, BlockHeight}, merkle_tree::{CommitmentTree, IncrementalWitness}, sapling::{ + keys::DiversifiableFullViewingKey, note_encryption::{try_sapling_compact_note_decryption, SaplingDomain}, Node, Note, Nullifier, PaymentAddress, SaplingIvk, }, @@ -127,6 +128,26 @@ pub trait ScanningKey { fn nf(&self, note: &Note, witness: &IncrementalWitness) -> Self::Nf; } +impl ScanningKey for DiversifiableFullViewingKey { + type Nf = Nullifier; + + fn try_decryption< + P: consensus::Parameters, + Output: ShieldedOutput, COMPACT_NOTE_SIZE>, + >( + &self, + params: &P, + height: BlockHeight, + output: &Output, + ) -> Option<(Note, PaymentAddress)> { + try_sapling_compact_note_decryption(params, height, &self.fvk().vk.ivk(), output) + } + + fn nf(&self, note: &Note, witness: &IncrementalWitness) -> Self::Nf { + note.nf(&self.fvk().vk, witness.position() as u64) + } +} + /// The [`ScanningKey`] implementation for [`ExtendedFullViewingKey`]s. /// Nullifiers may be derived when scanning with these keys. /// diff --git a/zcash_client_sqlite/CHANGELOG.md b/zcash_client_sqlite/CHANGELOG.md index 7f660a4e8..02168d2a7 100644 --- a/zcash_client_sqlite/CHANGELOG.md +++ b/zcash_client_sqlite/CHANGELOG.md @@ -49,6 +49,12 @@ and this library adheres to Rust's notion of constructed, rather than only in the case that a transaction has been decrypted after being retrieved from the network. +### Removed +- `zcash_client_sqlite::wallet`: + - `get_extended_full_viewing_keys` (use + `zcash_client_backend::data_api::WalletRead::get_unified_full_viewing_keys` + instead). + ### Deprecated - A number of public API methods that are used internally to support the `zcash_client_backend::data_api::{WalletRead, WalletWrite}` interfaces have diff --git a/zcash_client_sqlite/src/chain.rs b/zcash_client_sqlite/src/chain.rs index 66866f655..5902d9542 100644 --- a/zcash_client_sqlite/src/chain.rs +++ b/zcash_client_sqlite/src/chain.rs @@ -101,7 +101,7 @@ mod tests { init_wallet_db(&db_data).unwrap(); // Add an account to the wallet - let (extfvk, _taddr) = init_test_accounts_table(&db_data); + let (dfvk, _taddr) = init_test_accounts_table(&db_data); // Empty chain should be valid validate_chain( @@ -115,7 +115,7 @@ mod tests { let (cb, _) = fake_compact_block( sapling_activation_height(), BlockHash([0; 32]), - extfvk.clone(), + &dfvk, Amount::from_u64(5).unwrap(), ); insert_into_cache(&db_cache, &cb); @@ -144,7 +144,7 @@ mod tests { let (cb2, _) = fake_compact_block( sapling_activation_height() + 1, cb.hash(), - extfvk, + &dfvk, Amount::from_u64(7).unwrap(), ); insert_into_cache(&db_cache, &cb2); @@ -180,19 +180,19 @@ mod tests { init_wallet_db(&db_data).unwrap(); // Add an account to the wallet - let (extfvk, _taddr) = init_test_accounts_table(&db_data); + let (dfvk, _taddr) = init_test_accounts_table(&db_data); // Create some fake CompactBlocks let (cb, _) = fake_compact_block( sapling_activation_height(), BlockHash([0; 32]), - extfvk.clone(), + &dfvk, Amount::from_u64(5).unwrap(), ); let (cb2, _) = fake_compact_block( sapling_activation_height() + 1, cb.hash(), - extfvk.clone(), + &dfvk, Amount::from_u64(7).unwrap(), ); insert_into_cache(&db_cache, &cb); @@ -214,13 +214,13 @@ mod tests { let (cb3, _) = fake_compact_block( sapling_activation_height() + 2, BlockHash([1; 32]), - extfvk.clone(), + &dfvk, Amount::from_u64(8).unwrap(), ); let (cb4, _) = fake_compact_block( sapling_activation_height() + 3, cb3.hash(), - extfvk, + &dfvk, Amount::from_u64(3).unwrap(), ); insert_into_cache(&db_cache, &cb3); @@ -250,19 +250,19 @@ mod tests { init_wallet_db(&db_data).unwrap(); // Add an account to the wallet - let (extfvk, _taddr) = init_test_accounts_table(&db_data); + let (dfvk, _taddr) = init_test_accounts_table(&db_data); // Create some fake CompactBlocks let (cb, _) = fake_compact_block( sapling_activation_height(), BlockHash([0; 32]), - extfvk.clone(), + &dfvk, Amount::from_u64(5).unwrap(), ); let (cb2, _) = fake_compact_block( sapling_activation_height() + 1, cb.hash(), - extfvk.clone(), + &dfvk, Amount::from_u64(7).unwrap(), ); insert_into_cache(&db_cache, &cb); @@ -284,13 +284,13 @@ mod tests { let (cb3, _) = fake_compact_block( sapling_activation_height() + 2, cb2.hash(), - extfvk.clone(), + &dfvk, Amount::from_u64(8).unwrap(), ); let (cb4, _) = fake_compact_block( sapling_activation_height() + 3, BlockHash([1; 32]), - extfvk, + &dfvk, Amount::from_u64(3).unwrap(), ); insert_into_cache(&db_cache, &cb3); @@ -320,7 +320,7 @@ mod tests { init_wallet_db(&db_data).unwrap(); // Add an account to the wallet - let (extfvk, _taddr) = init_test_accounts_table(&db_data); + let (dfvk, _taddr) = init_test_accounts_table(&db_data); // Account balance should be zero assert_eq!( @@ -334,12 +334,12 @@ mod tests { let (cb, _) = fake_compact_block( sapling_activation_height(), BlockHash([0; 32]), - extfvk.clone(), + &dfvk, value, ); let (cb2, _) = - fake_compact_block(sapling_activation_height() + 1, cb.hash(), extfvk, value2); + fake_compact_block(sapling_activation_height() + 1, cb.hash(), &dfvk, value2); insert_into_cache(&db_cache, &cb); insert_into_cache(&db_cache, &cb2); @@ -389,14 +389,14 @@ mod tests { init_wallet_db(&db_data).unwrap(); // Add an account to the wallet - let (extfvk, _taddr) = init_test_accounts_table(&db_data); + let (dfvk, _taddr) = init_test_accounts_table(&db_data); // Create a block with height SAPLING_ACTIVATION_HEIGHT let value = Amount::from_u64(50000).unwrap(); let (cb1, _) = fake_compact_block( sapling_activation_height(), BlockHash([0; 32]), - extfvk.clone(), + &dfvk, value, ); insert_into_cache(&db_cache, &cb1); @@ -405,14 +405,10 @@ mod tests { assert_eq!(get_balance(&db_data, AccountId::from(0)).unwrap(), value); // We cannot scan a block of height SAPLING_ACTIVATION_HEIGHT + 2 next - let (cb2, _) = fake_compact_block( - sapling_activation_height() + 1, - cb1.hash(), - extfvk.clone(), - value, - ); + let (cb2, _) = + fake_compact_block(sapling_activation_height() + 1, cb1.hash(), &dfvk, value); let (cb3, _) = - fake_compact_block(sapling_activation_height() + 2, cb2.hash(), extfvk, value); + fake_compact_block(sapling_activation_height() + 2, cb2.hash(), &dfvk, value); insert_into_cache(&db_cache, &cb3); match scan_cached_blocks(&tests::network(), &db_cache, &mut db_write, None) { Err(SqliteClientError::BackendError(e)) => { @@ -448,7 +444,7 @@ mod tests { init_wallet_db(&db_data).unwrap(); // Add an account to the wallet - let (extfvk, _taddr) = init_test_accounts_table(&db_data); + let (dfvk, _taddr) = init_test_accounts_table(&db_data); // Account balance should be zero assert_eq!( @@ -461,7 +457,7 @@ mod tests { let (cb, _) = fake_compact_block( sapling_activation_height(), BlockHash([0; 32]), - extfvk.clone(), + &dfvk, value, ); insert_into_cache(&db_cache, &cb); @@ -476,7 +472,7 @@ mod tests { // Create a second fake CompactBlock sending more value to the address let value2 = Amount::from_u64(7).unwrap(); let (cb2, _) = - fake_compact_block(sapling_activation_height() + 1, cb.hash(), extfvk, value2); + fake_compact_block(sapling_activation_height() + 1, cb.hash(), &dfvk, value2); insert_into_cache(&db_cache, &cb2); // Scan the cache again @@ -500,7 +496,7 @@ mod tests { init_wallet_db(&db_data).unwrap(); // Add an account to the wallet - let (extfvk, _taddr) = init_test_accounts_table(&db_data); + let (dfvk, _taddr) = init_test_accounts_table(&db_data); // Account balance should be zero assert_eq!( @@ -513,7 +509,7 @@ mod tests { let (cb, nf) = fake_compact_block( sapling_activation_height(), BlockHash([0; 32]), - extfvk.clone(), + &dfvk, value, ); insert_into_cache(&db_cache, &cb); @@ -535,7 +531,7 @@ mod tests { sapling_activation_height() + 1, cb.hash(), (nf, value), - extfvk, + &dfvk, to2, value2, ), diff --git a/zcash_client_sqlite/src/lib.rs b/zcash_client_sqlite/src/lib.rs index 425757c54..742cfe220 100644 --- a/zcash_client_sqlite/src/lib.rs +++ b/zcash_client_sqlite/src/lib.rs @@ -745,8 +745,8 @@ mod tests { legacy::TransparentAddress, memo::MemoBytes, sapling::{ - note_encryption::sapling_note_encryption, util::generate_random_rseed, Note, Nullifier, - PaymentAddress, + keys::DiversifiableFullViewingKey, note_encryption::sapling_note_encryption, + util::generate_random_rseed, Note, Nullifier, PaymentAddress, }, transaction::components::Amount, zip32::ExtendedFullViewingKey, @@ -783,11 +783,11 @@ mod tests { #[cfg(test)] pub(crate) fn init_test_accounts_table( db_data: &WalletDb, - ) -> (ExtendedFullViewingKey, Option) { + ) -> (DiversifiableFullViewingKey, Option) { let seed = [0u8; 32]; let account = AccountId::from(0); let extsk = sapling::spending_key(&seed, network().coin_type(), account); - let extfvk = ExtendedFullViewingKey::from(&extsk); + let dfvk = DiversifiableFullViewingKey::from(ExtendedFullViewingKey::from(&extsk)); #[cfg(feature = "transparent-inputs")] let (tkey, taddr) = { @@ -804,13 +804,13 @@ mod tests { let ufvk = UnifiedFullViewingKey::new( #[cfg(feature = "transparent-inputs")] tkey, - Some(extfvk.clone()), + Some(dfvk.clone()), ) .unwrap(); init_accounts_table(db_data, &[ufvk]).unwrap(); - (extfvk, taddr) + (dfvk, taddr) } /// Create a fake CompactBlock at the given height, containing a single output paying @@ -818,10 +818,10 @@ mod tests { pub(crate) fn fake_compact_block( height: BlockHeight, prev_hash: BlockHash, - extfvk: ExtendedFullViewingKey, + dfvk: &DiversifiableFullViewingKey, value: Amount, ) -> (CompactBlock, Nullifier) { - let to = extfvk.default_address().1; + let to = dfvk.default_address().1; // Create a fake Note for the account let mut rng = OsRng; @@ -833,7 +833,7 @@ mod tests { rseed, }; let encryptor = sapling_note_encryption::<_, Network>( - Some(extfvk.fvk.ovk), + Some(dfvk.fvk().ovk), note.clone(), to, MemoBytes::empty(), @@ -859,7 +859,7 @@ mod tests { rng.fill_bytes(&mut cb.hash); cb.prevHash.extend_from_slice(&prev_hash.0); cb.vtx.push(ctx); - (cb, note.nf(&extfvk.fvk.vk, 0)) + (cb, note.nf(&dfvk.fvk().vk, 0)) } /// Create a fake CompactBlock at the given height, spending a single note from the @@ -868,7 +868,7 @@ mod tests { height: BlockHeight, prev_hash: BlockHash, (nf, in_value): (Nullifier, Amount), - extfvk: ExtendedFullViewingKey, + dfvk: &DiversifiableFullViewingKey, to: PaymentAddress, value: Amount, ) -> CompactBlock { @@ -893,7 +893,7 @@ mod tests { rseed, }; let encryptor = sapling_note_encryption::<_, Network>( - Some(extfvk.fvk.ovk), + Some(dfvk.fvk().ovk), note.clone(), to, MemoBytes::empty(), @@ -912,7 +912,7 @@ mod tests { // Create a fake Note for the change ctx.outputs.push({ - let change_addr = extfvk.default_address().1; + let change_addr = dfvk.default_address().1; let rseed = generate_random_rseed(&network(), height, &mut rng); let note = Note { g_d: change_addr.diversifier().g_d().unwrap(), @@ -921,7 +921,7 @@ mod tests { rseed, }; let encryptor = sapling_note_encryption::<_, Network>( - Some(extfvk.fvk.ovk), + Some(dfvk.fvk().ovk), note.clone(), change_addr, MemoBytes::empty(), diff --git a/zcash_client_sqlite/src/wallet.rs b/zcash_client_sqlite/src/wallet.rs index aa7f1b14b..974f883fb 100644 --- a/zcash_client_sqlite/src/wallet.rs +++ b/zcash_client_sqlite/src/wallet.rs @@ -17,7 +17,7 @@ use zcash_primitives::{ consensus::{self, BlockHeight, BranchId, NetworkUpgrade, Parameters}, memo::{Memo, MemoBytes}, merkle_tree::{CommitmentTree, IncrementalWitness}, - sapling::{Node, Note, Nullifier, PaymentAddress}, + sapling::{keys::DiversifiableFullViewingKey, Node, Note, Nullifier, PaymentAddress}, transaction::{components::Amount, Transaction, TxId}, zip32::{AccountId, ExtendedFullViewingKey}, }; @@ -172,28 +172,6 @@ pub fn get_address( }) } -/// Returns the [`ExtendedFullViewingKey`]s for the wallet. -/// -/// [`ExtendedFullViewingKey`]: zcash_primitives::zip32::ExtendedFullViewingKey -#[deprecated( - note = "This function will be removed in a future release. Use zcash_client_backend::data_api::WalletRead::get_unified_full_viewing_keys instead." -)] -pub fn get_extended_full_viewing_keys( - wdb: &WalletDb

, -) -> Result, SqliteClientError> { - get_unified_full_viewing_keys(wdb).map(|ufvks| { - ufvks - .into_iter() - .map(|(account, ufvk)| { - ( - account, - ufvk.sapling().cloned().expect("TODO: Add Orchard support"), - ) - }) - .collect() - }) -} - /// Returns the [`UnifiedFullViewingKey`]s for the wallet. pub(crate) fn get_unified_full_viewing_keys( wdb: &WalletDb

, @@ -248,7 +226,10 @@ pub fn is_valid_account_extfvk( .map_err(SqliteClientError::from) .and_then(|row| { if let Some(ufvk) = row { - ufvk.map(|ufvk| ufvk.sapling() == Some(extfvk)) + ufvk.map(|ufvk| { + ufvk.sapling().map(|dfvk| dfvk.to_bytes()) + == Some(DiversifiableFullViewingKey::from(extfvk.clone()).to_bytes()) + }) } else { Ok(false) } diff --git a/zcash_client_sqlite/src/wallet/init.rs b/zcash_client_sqlite/src/wallet/init.rs index d589f8ba0..ed405030f 100644 --- a/zcash_client_sqlite/src/wallet/init.rs +++ b/zcash_client_sqlite/src/wallet/init.rs @@ -179,8 +179,8 @@ pub fn init_wallet_db

(wdb: &WalletDb

) -> Result<(), rusqlite::Error> { /// let seed = [0u8; 32]; // insecure; replace with a strong random seed /// let account = AccountId::from(0); /// let extsk = sapling::spending_key(&seed, Network::TestNetwork.coin_type(), account); -/// let extfvk = ExtendedFullViewingKey::from(&extsk); -/// let ufvk = UnifiedFullViewingKey::new(None, Some(extfvk)).unwrap(); +/// let dfvk = ExtendedFullViewingKey::from(&extsk).into(); +/// let ufvk = UnifiedFullViewingKey::new(None, Some(dfvk)).unwrap(); /// init_accounts_table(&db_data, &[ufvk]).unwrap(); /// # } /// ``` @@ -293,6 +293,7 @@ mod tests { use zcash_primitives::{ block::BlockHash, consensus::{BlockHeight, Parameters}, + sapling::keys::DiversifiableFullViewingKey, zip32::ExtendedFullViewingKey, }; @@ -319,7 +320,7 @@ mod tests { // First call with data should initialise the accounts table let extsk = sapling::spending_key(&seed, network().coin_type(), account); - let extfvk = ExtendedFullViewingKey::from(&extsk); + let dfvk = DiversifiableFullViewingKey::from(ExtendedFullViewingKey::from(&extsk)); #[cfg(feature = "transparent-inputs")] let ufvk = UnifiedFullViewingKey::new( @@ -328,12 +329,12 @@ mod tests { .unwrap() .to_account_pubkey(), ), - Some(extfvk), + Some(dfvk), ) .unwrap(); #[cfg(not(feature = "transparent-inputs"))] - let ufvk = UnifiedFullViewingKey::new(Some(extfvk)).unwrap(); + let ufvk = UnifiedFullViewingKey::new(Some(dfvk), None).unwrap(); init_accounts_table(&db_data, &[ufvk.clone()]).unwrap(); diff --git a/zcash_client_sqlite/src/wallet/transact.rs b/zcash_client_sqlite/src/wallet/transact.rs index ef1aa6067..4bb345fa3 100644 --- a/zcash_client_sqlite/src/wallet/transact.rs +++ b/zcash_client_sqlite/src/wallet/transact.rs @@ -165,7 +165,10 @@ mod tests { block::BlockHash, consensus::{BlockHeight, BranchId, Parameters}, legacy::TransparentAddress, - sapling::{note_encryption::try_sapling_output_recovery, prover::TxProver}, + sapling::{ + keys::DiversifiableFullViewingKey, note_encryption::try_sapling_output_recovery, + prover::TxProver, + }, transaction::{components::Amount, Transaction}, zip32::{ExtendedFullViewingKey, ExtendedSpendingKey}, }; @@ -207,8 +210,8 @@ mod tests { // Add two accounts to the wallet let extsk0 = sapling::spending_key(&[0u8; 32], network().coin_type(), AccountId::from(0)); let extsk1 = sapling::spending_key(&[1u8; 32], network().coin_type(), AccountId::from(1)); - let extfvk0 = ExtendedFullViewingKey::from(&extsk0); - let extfvk1 = ExtendedFullViewingKey::from(&extsk1); + let dfvk0 = DiversifiableFullViewingKey::from(ExtendedFullViewingKey::from(&extsk0)); + let dfvk1 = DiversifiableFullViewingKey::from(ExtendedFullViewingKey::from(&extsk1)); #[cfg(feature = "transparent-inputs")] let ufvks = { @@ -219,14 +222,14 @@ mod tests { transparent::AccountPrivKey::from_seed(&network(), &[1u8; 32], AccountId::from(1)) .unwrap(); [ - UnifiedFullViewingKey::new(Some(tsk0.to_account_pubkey()), Some(extfvk0)).unwrap(), - UnifiedFullViewingKey::new(Some(tsk1.to_account_pubkey()), Some(extfvk1)).unwrap(), + UnifiedFullViewingKey::new(Some(tsk0.to_account_pubkey()), Some(dfvk0)).unwrap(), + UnifiedFullViewingKey::new(Some(tsk1.to_account_pubkey()), Some(dfvk1)).unwrap(), ] }; #[cfg(not(feature = "transparent-inputs"))] let ufvks = [ - UnifiedFullViewingKey::new(Some(extfvk0)).unwrap(), - UnifiedFullViewingKey::new(Some(extfvk1)).unwrap(), + UnifiedFullViewingKey::new(Some(dfvk0), None).unwrap(), + UnifiedFullViewingKey::new(Some(dfvk1), None).unwrap(), ]; init_accounts_table(&db_data, &ufvks).unwrap(); @@ -275,12 +278,12 @@ mod tests { // Add an account to the wallet let extsk = sapling::spending_key(&[0u8; 32], network().coin_type(), AccountId::from(0)); - let extfvk = ExtendedFullViewingKey::from(&extsk); + let dfvk = DiversifiableFullViewingKey::from(ExtendedFullViewingKey::from(&extsk)); #[cfg(feature = "transparent-inputs")] - let ufvk = UnifiedFullViewingKey::new(None, Some(extfvk)).unwrap(); + let ufvk = UnifiedFullViewingKey::new(None, Some(dfvk)).unwrap(); #[cfg(not(feature = "transparent-inputs"))] - let ufvk = UnifiedFullViewingKey::new(Some(extfvk)).unwrap(); + let ufvk = UnifiedFullViewingKey::new(Some(dfvk)).unwrap(); init_accounts_table(&db_data, &[ufvk]).unwrap(); let to = extsk.default_address().1.into(); @@ -319,11 +322,11 @@ mod tests { // Add an account to the wallet let extsk = sapling::spending_key(&[0u8; 32], network().coin_type(), AccountId::from(0)); - let extfvk = ExtendedFullViewingKey::from(&extsk); + let dfvk = DiversifiableFullViewingKey::from(ExtendedFullViewingKey::from(&extsk)); #[cfg(feature = "transparent-inputs")] - let ufvk = UnifiedFullViewingKey::new(None, Some(extfvk)).unwrap(); + let ufvk = UnifiedFullViewingKey::new(None, Some(dfvk)).unwrap(); #[cfg(not(feature = "transparent-inputs"))] - let ufvk = UnifiedFullViewingKey::new(Some(extfvk)).unwrap(); + let ufvk = UnifiedFullViewingKey::new(Some(dfvk)).unwrap(); init_accounts_table(&db_data, &[ufvk]).unwrap(); let to = extsk.default_address().1.into(); @@ -367,11 +370,11 @@ mod tests { // Add an account to the wallet let extsk = sapling::spending_key(&[0u8; 32], network().coin_type(), AccountId::from(0)); - let extfvk = ExtendedFullViewingKey::from(&extsk); + let dfvk = DiversifiableFullViewingKey::from(ExtendedFullViewingKey::from(&extsk)); #[cfg(feature = "transparent-inputs")] - let ufvk = UnifiedFullViewingKey::new(None, Some(extfvk.clone())).unwrap(); + let ufvk = UnifiedFullViewingKey::new(None, Some(dfvk.clone())).unwrap(); #[cfg(not(feature = "transparent-inputs"))] - let ufvk = UnifiedFullViewingKey::new(Some(extfvk.clone())).unwrap(); + let ufvk = UnifiedFullViewingKey::new(Some(dfvk.clone())).unwrap(); init_accounts_table(&db_data, &[ufvk]).unwrap(); // Add funds to the wallet in a single note @@ -379,7 +382,7 @@ mod tests { let (cb, _) = fake_compact_block( sapling_activation_height(), BlockHash([0; 32]), - extfvk.clone(), + &dfvk, value, ); insert_into_cache(&db_cache, &cb); @@ -398,12 +401,7 @@ mod tests { ); // Add more funds to the wallet in a second note - let (cb, _) = fake_compact_block( - sapling_activation_height() + 1, - cb.hash(), - extfvk.clone(), - value, - ); + let (cb, _) = fake_compact_block(sapling_activation_height() + 1, cb.hash(), &dfvk, value); insert_into_cache(&db_cache, &cb); scan_cached_blocks(&tests::network(), &db_cache, &mut db_write, None).unwrap(); @@ -446,12 +444,8 @@ mod tests { // Mine blocks SAPLING_ACTIVATION_HEIGHT + 2 to 9 until just before the second // note is verified for i in 2..10 { - let (cb, _) = fake_compact_block( - sapling_activation_height() + i, - cb.hash(), - extfvk.clone(), - value, - ); + let (cb, _) = + fake_compact_block(sapling_activation_height() + i, cb.hash(), &dfvk, value); insert_into_cache(&db_cache, &cb); } scan_cached_blocks(&tests::network(), &db_cache, &mut db_write, None).unwrap(); @@ -477,8 +471,7 @@ mod tests { } // Mine block 11 so that the second note becomes verified - let (cb, _) = - fake_compact_block(sapling_activation_height() + 10, cb.hash(), extfvk, value); + let (cb, _) = fake_compact_block(sapling_activation_height() + 10, cb.hash(), &dfvk, value); insert_into_cache(&db_cache, &cb); scan_cached_blocks(&tests::network(), &db_cache, &mut db_write, None).unwrap(); @@ -510,11 +503,11 @@ mod tests { // Add an account to the wallet let extsk = sapling::spending_key(&[0u8; 32], network().coin_type(), AccountId::from(0)); - let extfvk = ExtendedFullViewingKey::from(&extsk); + let dfvk = DiversifiableFullViewingKey::from(ExtendedFullViewingKey::from(&extsk)); #[cfg(feature = "transparent-inputs")] - let ufvk = UnifiedFullViewingKey::new(None, Some(extfvk.clone())).unwrap(); + let ufvk = UnifiedFullViewingKey::new(None, Some(dfvk.clone())).unwrap(); #[cfg(not(feature = "transparent-inputs"))] - let ufvk = UnifiedFullViewingKey::new(Some(extfvk.clone())).unwrap(); + let ufvk = UnifiedFullViewingKey::new(Some(dfvk.clone())).unwrap(); init_accounts_table(&db_data, &[ufvk]).unwrap(); // Add funds to the wallet in a single note @@ -522,7 +515,7 @@ mod tests { let (cb, _) = fake_compact_block( sapling_activation_height(), BlockHash([0; 32]), - extfvk, + &dfvk, value, ); insert_into_cache(&db_cache, &cb); @@ -573,7 +566,7 @@ mod tests { let (cb, _) = fake_compact_block( sapling_activation_height() + i, cb.hash(), - ExtendedFullViewingKey::from(&ExtendedSpendingKey::master(&[i as u8])), + &ExtendedFullViewingKey::from(&ExtendedSpendingKey::master(&[i as u8])).into(), value, ); insert_into_cache(&db_cache, &cb); @@ -604,7 +597,7 @@ mod tests { let (cb, _) = fake_compact_block( sapling_activation_height() + 22, cb.hash(), - ExtendedFullViewingKey::from(&ExtendedSpendingKey::master(&[22])), + &ExtendedFullViewingKey::from(&ExtendedSpendingKey::master(&[22])).into(), value, ); insert_into_cache(&db_cache, &cb); @@ -639,11 +632,11 @@ mod tests { // Add an account to the wallet let extsk = sapling::spending_key(&[0u8; 32], network.coin_type(), AccountId::from(0)); - let extfvk = ExtendedFullViewingKey::from(&extsk); + let dfvk = DiversifiableFullViewingKey::from(ExtendedFullViewingKey::from(&extsk)); #[cfg(feature = "transparent-inputs")] - let ufvk = UnifiedFullViewingKey::new(None, Some(extfvk.clone())).unwrap(); + let ufvk = UnifiedFullViewingKey::new(None, Some(dfvk.clone())).unwrap(); #[cfg(not(feature = "transparent-inputs"))] - let ufvk = UnifiedFullViewingKey::new(Some(extfvk.clone())).unwrap(); + let ufvk = UnifiedFullViewingKey::new(Some(dfvk.clone())).unwrap(); init_accounts_table(&db_data, &[ufvk]).unwrap(); // Add funds to the wallet in a single note @@ -651,7 +644,7 @@ mod tests { let (cb, _) = fake_compact_block( sapling_activation_height(), BlockHash([0; 32]), - extfvk.clone(), + &dfvk, value, ); insert_into_cache(&db_cache, &cb); @@ -708,7 +701,7 @@ mod tests { try_sapling_output_recovery( &network, sapling_activation_height(), - &extfvk.fvk.ovk, + &dfvk.fvk().ovk, output, ) }; @@ -725,7 +718,7 @@ mod tests { let (cb, _) = fake_compact_block( sapling_activation_height() + i, cb.hash(), - ExtendedFullViewingKey::from(&ExtendedSpendingKey::master(&[i as u8])), + &ExtendedFullViewingKey::from(&ExtendedSpendingKey::master(&[i as u8])).into(), value, ); insert_into_cache(&db_cache, &cb); @@ -749,11 +742,11 @@ mod tests { // Add an account to the wallet let extsk = sapling::spending_key(&[0u8; 32], network().coin_type(), AccountId::from(0)); - let extfvk = ExtendedFullViewingKey::from(&extsk); + let dfvk = DiversifiableFullViewingKey::from(ExtendedFullViewingKey::from(&extsk)); #[cfg(feature = "transparent-inputs")] - let ufvk = UnifiedFullViewingKey::new(None, Some(extfvk.clone())).unwrap(); + let ufvk = UnifiedFullViewingKey::new(None, Some(dfvk.clone())).unwrap(); #[cfg(not(feature = "transparent-inputs"))] - let ufvk = UnifiedFullViewingKey::new(Some(extfvk.clone())).unwrap(); + let ufvk = UnifiedFullViewingKey::new(Some(dfvk.clone())).unwrap(); init_accounts_table(&db_data, &[ufvk]).unwrap(); // Add funds to the wallet in a single note @@ -761,7 +754,7 @@ mod tests { let (cb, _) = fake_compact_block( sapling_activation_height(), BlockHash([0; 32]), - extfvk, + &dfvk, value, ); insert_into_cache(&db_cache, &cb); diff --git a/zcash_primitives/src/sapling/keys.rs b/zcash_primitives/src/sapling/keys.rs index 297ef2bad..822f3a438 100644 --- a/zcash_primitives/src/sapling/keys.rs +++ b/zcash_primitives/src/sapling/keys.rs @@ -276,6 +276,13 @@ impl DiversifiableFullViewingKey { zip32::sapling_find_address(&self.fvk, &self.dk, j) } + /// Returns the payment address corresponding to the smallest valid diversifier index, + /// along with that index. + // TODO: See if this is only used in tests. + pub fn default_address(&self) -> (zip32::DiversifierIndex, PaymentAddress) { + zip32::sapling_default_address(&self.fvk, &self.dk) + } + /// Attempts to decrypt the given address's diversifier with this full viewing key. /// /// This method extracts the diversifier from the given address and decrypts it as a From b52e949bd6ee1dbbc0324437440bed8b3c423f79 Mon Sep 17 00:00:00 2001 From: Jack Grigg Date: Tue, 14 Jun 2022 02:41:01 +0000 Subject: [PATCH 8/9] zcash_client_backend: Migrate to correct ZIP 316 UFVK encoding We also add support for parsing Orchard full viewing keys from encoded UFVKs (rather than treating them as unknown). `UnifiedSpendingKey` still does not have Orchard support, so `UnifiedFullViewingKey`s will be generated without Orchard components. --- zcash_client_backend/src/address.rs | 2 +- zcash_client_backend/src/keys.rs | 164 ++++++++++++++------- zcash_client_sqlite/src/lib.rs | 1 + zcash_client_sqlite/src/wallet/init.rs | 3 +- zcash_client_sqlite/src/wallet/transact.rs | 30 ++-- 5 files changed, 132 insertions(+), 68 deletions(-) diff --git a/zcash_client_backend/src/address.rs b/zcash_client_backend/src/address.rs index 126b9e589..319c9a999 100644 --- a/zcash_client_backend/src/address.rs +++ b/zcash_client_backend/src/address.rs @@ -8,7 +8,7 @@ use zcash_address::{ }; use zcash_primitives::{consensus, constants, legacy::TransparentAddress, sapling::PaymentAddress}; -fn params_to_network(params: &P) -> Network { +pub(crate) fn params_to_network(params: &P) -> Network { // Use the Sapling HRP as an indicator of network. match params.hrp_sapling_payment_address() { constants::mainnet::HRP_SAPLING_PAYMENT_ADDRESS => Network::Main, diff --git a/zcash_client_backend/src/keys.rs b/zcash_client_backend/src/keys.rs index 5aac47b96..0515ff3e9 100644 --- a/zcash_client_backend/src/keys.rs +++ b/zcash_client_backend/src/keys.rs @@ -1,12 +1,14 @@ //! Helper functions for managing light client key material. +use zcash_address::unified::{self, Container, Encoding}; use zcash_primitives::{ consensus, sapling::keys as sapling_keys, zip32::{AccountId, DiversifierIndex}, }; -use crate::address::UnifiedAddress; +use crate::address::{params_to_network, UnifiedAddress}; +#[cfg(feature = "transparent-inputs")] use std::convert::TryInto; #[cfg(feature = "transparent-inputs")] @@ -110,6 +112,8 @@ impl UnifiedSpendingKey { #[cfg(feature = "transparent-inputs")] transparent: Some(self.transparent.to_account_pubkey()), sapling: Some(sapling::ExtendedFullViewingKey::from(&self.sapling).into()), + orchard: None, + unknown: vec![], } } @@ -139,6 +143,8 @@ pub struct UnifiedFullViewingKey { #[cfg(feature = "transparent-inputs")] transparent: Option, sapling: Option, + orchard: Option, + unknown: Vec<(u32, Vec)>, } #[doc(hidden)] @@ -147,6 +153,7 @@ impl UnifiedFullViewingKey { pub fn new( #[cfg(feature = "transparent-inputs")] transparent: Option, sapling: Option, + orchard: Option, ) -> Option { if sapling.is_none() { None @@ -155,70 +162,113 @@ impl UnifiedFullViewingKey { #[cfg(feature = "transparent-inputs")] transparent, sapling, + orchard, + // We don't allow constructing new UFVKs with unknown items, but we store + // this to allow parsing such UFVKs. + unknown: vec![], }) } } - /// Attempts to decode the given string as an encoding of a `UnifiedFullViewingKey` - /// for the given network. + /// Parses a `UnifiedFullViewingKey` from its [ZIP 316] string encoding. + /// + /// [ZIP 316]: https://zips.z.cash/zip-0316 pub fn decode(params: &P, encoding: &str) -> Result { - encoding - .strip_prefix("DONOTUSEUFVK") - .and_then(|data| hex::decode(data).ok()) - .as_ref() - .and_then(|data| data.split_first()) - .and_then(|(flag, data)| { + let (net, ufvk) = unified::Ufvk::decode(encoding).map_err(|e| e.to_string())?; + let expected_net = params_to_network(params); + if net != expected_net { + return Err(format!( + "UFVK is for network {:?} but we expected {:?}", + net, expected_net, + )); + } + + let mut orchard = None; + let mut sapling = None; + #[cfg(feature = "transparent-inputs")] + let mut transparent = None; + + // We can use as-parsed order here for efficiency, because we're breaking out the + // receivers we support from the unknown receivers. + let unknown = ufvk + .items_as_parsed() + .iter() + .filter_map(|receiver| match receiver { + unified::Fvk::Orchard(data) => orchard::keys::FullViewingKey::from_bytes(data) + .ok_or("Invalid Orchard FVK in Unified FVK") + .map(|addr| { + orchard = Some(addr); + None + }) + .transpose(), + unified::Fvk::Sapling(data) => { + sapling_keys::DiversifiableFullViewingKey::from_bytes(data) + .ok_or("Invalid Sapling FVK in Unified FVK") + .map(|pa| { + sapling = Some(pa); + None + }) + .transpose() + } #[cfg(feature = "transparent-inputs")] - let (transparent, data) = if flag & 1 != 0 { - if data.len() < 65 { - return None; - } - let (tfvk, data) = data.split_at(65); - ( - Some(legacy::AccountPubKey::deserialize(tfvk.try_into().unwrap()).ok()?), - data, - ) - } else { - (None, data) - }; - - let sapling = if flag & 2 != 0 { - if data.len() != 128 { - return None; - } - Some(sapling_keys::DiversifiableFullViewingKey::from_bytes( - data.try_into().unwrap(), - )?) - } else { - None - }; - - Some(Self { - #[cfg(feature = "transparent-inputs")] - transparent, - sapling, - }) + unified::Fvk::P2pkh(data) => legacy::AccountPubKey::deserialize(data) + .map_err(|_| "Invalid transparent FVK in Unified FVK") + .map(|tfvk| { + transparent = Some(tfvk); + None + }) + .transpose(), + #[cfg(not(feature = "transparent-inputs"))] + unified::Fvk::P2pkh(data) => { + Some(Ok((unified::Typecode::P2pkh.into(), data.to_vec()))) + } + unified::Fvk::Unknown { typecode, data } => Some(Ok((*typecode, data.clone()))), }) - .ok_or("TODO Implement real UFVK encoding after fixing struct".to_string()) + .collect::>()?; + + Ok(Self { + #[cfg(feature = "transparent-inputs")] + transparent, + sapling, + orchard, + unknown, + }) } /// Returns the string encoding of this `UnifiedFullViewingKey` for the given network. pub fn encode(&self, params: &P) -> String { - let flag = if self.sapling.is_some() { 2 } else { 0 }; + let items = std::iter::empty() + .chain( + self.orchard + .as_ref() + .map(|fvk| fvk.to_bytes()) + .map(unified::Fvk::Orchard), + ) + .chain( + self.sapling + .as_ref() + .map(|dfvk| dfvk.to_bytes()) + .map(unified::Fvk::Sapling), + ) + .chain( + self.unknown + .iter() + .map(|(typecode, data)| unified::Fvk::Unknown { + typecode: *typecode, + data: data.clone(), + }), + ); #[cfg(feature = "transparent-inputs")] - let flag = flag | if self.transparent.is_some() { 1 } else { 0 }; - let mut ufvk = vec![flag]; + let items = items.chain( + self.transparent + .as_ref() + .map(|tfvk| tfvk.serialize().try_into().unwrap()) + .map(unified::Fvk::P2pkh), + ); - #[cfg(feature = "transparent-inputs")] - if let Some(transparent) = self.transparent.as_ref() { - ufvk.append(&mut transparent.serialize()); - }; - - if let Some(sapling) = self.sapling.as_ref() { - ufvk.extend_from_slice(&sapling.to_bytes()); - } - - format!("DONOTUSEUFVK{}", hex::encode(&ufvk)) + let ufvk = unified::Ufvk::try_from_items(items.collect()) + .expect("UnifiedFullViewingKey should only be constructed safely"); + ufvk.encode(¶ms_to_network(params)) } /// Returns the transparent component of the unified key at the @@ -348,6 +398,11 @@ mod tests { fn ufvk_round_trip() { let account = 0.into(); + let orchard = { + let sk = orchard::keys::SpendingKey::from_zip32_seed(&[0; 32], 0, 0).unwrap(); + Some(orchard::keys::FullViewingKey::from(&sk)) + }; + let sapling = { let extsk = sapling::spending_key(&[0; 32], 0, account); Some(ExtendedFullViewingKey::from(&extsk).into()) @@ -360,6 +415,7 @@ mod tests { #[cfg(feature = "transparent-inputs")] transparent, sapling, + orchard, ) .unwrap(); @@ -374,5 +430,9 @@ mod tests { decoded.sapling.map(|s| s.to_bytes()), ufvk.sapling.map(|s| s.to_bytes()), ); + assert_eq!( + decoded.orchard.map(|o| o.to_bytes()), + ufvk.orchard.map(|o| o.to_bytes()), + ); } } diff --git a/zcash_client_sqlite/src/lib.rs b/zcash_client_sqlite/src/lib.rs index 742cfe220..f9f1fe7b4 100644 --- a/zcash_client_sqlite/src/lib.rs +++ b/zcash_client_sqlite/src/lib.rs @@ -805,6 +805,7 @@ mod tests { #[cfg(feature = "transparent-inputs")] tkey, Some(dfvk.clone()), + None, ) .unwrap(); diff --git a/zcash_client_sqlite/src/wallet/init.rs b/zcash_client_sqlite/src/wallet/init.rs index ed405030f..4a6cf8075 100644 --- a/zcash_client_sqlite/src/wallet/init.rs +++ b/zcash_client_sqlite/src/wallet/init.rs @@ -180,7 +180,7 @@ pub fn init_wallet_db

(wdb: &WalletDb

) -> Result<(), rusqlite::Error> { /// let account = AccountId::from(0); /// let extsk = sapling::spending_key(&seed, Network::TestNetwork.coin_type(), account); /// let dfvk = ExtendedFullViewingKey::from(&extsk).into(); -/// let ufvk = UnifiedFullViewingKey::new(None, Some(dfvk)).unwrap(); +/// let ufvk = UnifiedFullViewingKey::new(None, Some(dfvk), None).unwrap(); /// init_accounts_table(&db_data, &[ufvk]).unwrap(); /// # } /// ``` @@ -330,6 +330,7 @@ mod tests { .to_account_pubkey(), ), Some(dfvk), + None, ) .unwrap(); diff --git a/zcash_client_sqlite/src/wallet/transact.rs b/zcash_client_sqlite/src/wallet/transact.rs index 4bb345fa3..8b703dfc6 100644 --- a/zcash_client_sqlite/src/wallet/transact.rs +++ b/zcash_client_sqlite/src/wallet/transact.rs @@ -222,8 +222,10 @@ mod tests { transparent::AccountPrivKey::from_seed(&network(), &[1u8; 32], AccountId::from(1)) .unwrap(); [ - UnifiedFullViewingKey::new(Some(tsk0.to_account_pubkey()), Some(dfvk0)).unwrap(), - UnifiedFullViewingKey::new(Some(tsk1.to_account_pubkey()), Some(dfvk1)).unwrap(), + UnifiedFullViewingKey::new(Some(tsk0.to_account_pubkey()), Some(dfvk0), None) + .unwrap(), + UnifiedFullViewingKey::new(Some(tsk1.to_account_pubkey()), Some(dfvk1), None) + .unwrap(), ] }; #[cfg(not(feature = "transparent-inputs"))] @@ -281,9 +283,9 @@ mod tests { let dfvk = DiversifiableFullViewingKey::from(ExtendedFullViewingKey::from(&extsk)); #[cfg(feature = "transparent-inputs")] - let ufvk = UnifiedFullViewingKey::new(None, Some(dfvk)).unwrap(); + let ufvk = UnifiedFullViewingKey::new(None, Some(dfvk), None).unwrap(); #[cfg(not(feature = "transparent-inputs"))] - let ufvk = UnifiedFullViewingKey::new(Some(dfvk)).unwrap(); + let ufvk = UnifiedFullViewingKey::new(Some(dfvk), None).unwrap(); init_accounts_table(&db_data, &[ufvk]).unwrap(); let to = extsk.default_address().1.into(); @@ -324,9 +326,9 @@ mod tests { let extsk = sapling::spending_key(&[0u8; 32], network().coin_type(), AccountId::from(0)); let dfvk = DiversifiableFullViewingKey::from(ExtendedFullViewingKey::from(&extsk)); #[cfg(feature = "transparent-inputs")] - let ufvk = UnifiedFullViewingKey::new(None, Some(dfvk)).unwrap(); + let ufvk = UnifiedFullViewingKey::new(None, Some(dfvk), None).unwrap(); #[cfg(not(feature = "transparent-inputs"))] - let ufvk = UnifiedFullViewingKey::new(Some(dfvk)).unwrap(); + let ufvk = UnifiedFullViewingKey::new(Some(dfvk), None).unwrap(); init_accounts_table(&db_data, &[ufvk]).unwrap(); let to = extsk.default_address().1.into(); @@ -372,9 +374,9 @@ mod tests { let extsk = sapling::spending_key(&[0u8; 32], network().coin_type(), AccountId::from(0)); let dfvk = DiversifiableFullViewingKey::from(ExtendedFullViewingKey::from(&extsk)); #[cfg(feature = "transparent-inputs")] - let ufvk = UnifiedFullViewingKey::new(None, Some(dfvk.clone())).unwrap(); + let ufvk = UnifiedFullViewingKey::new(None, Some(dfvk.clone()), None).unwrap(); #[cfg(not(feature = "transparent-inputs"))] - let ufvk = UnifiedFullViewingKey::new(Some(dfvk.clone())).unwrap(); + let ufvk = UnifiedFullViewingKey::new(Some(dfvk.clone()), None).unwrap(); init_accounts_table(&db_data, &[ufvk]).unwrap(); // Add funds to the wallet in a single note @@ -505,9 +507,9 @@ mod tests { let extsk = sapling::spending_key(&[0u8; 32], network().coin_type(), AccountId::from(0)); let dfvk = DiversifiableFullViewingKey::from(ExtendedFullViewingKey::from(&extsk)); #[cfg(feature = "transparent-inputs")] - let ufvk = UnifiedFullViewingKey::new(None, Some(dfvk.clone())).unwrap(); + let ufvk = UnifiedFullViewingKey::new(None, Some(dfvk.clone()), None).unwrap(); #[cfg(not(feature = "transparent-inputs"))] - let ufvk = UnifiedFullViewingKey::new(Some(dfvk.clone())).unwrap(); + let ufvk = UnifiedFullViewingKey::new(Some(dfvk.clone()), None).unwrap(); init_accounts_table(&db_data, &[ufvk]).unwrap(); // Add funds to the wallet in a single note @@ -634,9 +636,9 @@ mod tests { let extsk = sapling::spending_key(&[0u8; 32], network.coin_type(), AccountId::from(0)); let dfvk = DiversifiableFullViewingKey::from(ExtendedFullViewingKey::from(&extsk)); #[cfg(feature = "transparent-inputs")] - let ufvk = UnifiedFullViewingKey::new(None, Some(dfvk.clone())).unwrap(); + let ufvk = UnifiedFullViewingKey::new(None, Some(dfvk.clone()), None).unwrap(); #[cfg(not(feature = "transparent-inputs"))] - let ufvk = UnifiedFullViewingKey::new(Some(dfvk.clone())).unwrap(); + let ufvk = UnifiedFullViewingKey::new(Some(dfvk.clone()), None).unwrap(); init_accounts_table(&db_data, &[ufvk]).unwrap(); // Add funds to the wallet in a single note @@ -744,9 +746,9 @@ mod tests { let extsk = sapling::spending_key(&[0u8; 32], network().coin_type(), AccountId::from(0)); let dfvk = DiversifiableFullViewingKey::from(ExtendedFullViewingKey::from(&extsk)); #[cfg(feature = "transparent-inputs")] - let ufvk = UnifiedFullViewingKey::new(None, Some(dfvk.clone())).unwrap(); + let ufvk = UnifiedFullViewingKey::new(None, Some(dfvk.clone()), None).unwrap(); #[cfg(not(feature = "transparent-inputs"))] - let ufvk = UnifiedFullViewingKey::new(Some(dfvk.clone())).unwrap(); + let ufvk = UnifiedFullViewingKey::new(Some(dfvk.clone()), None).unwrap(); init_accounts_table(&db_data, &[ufvk]).unwrap(); // Add funds to the wallet in a single note From 7236204b14c13ed76bdf4ad2d70a3f8f3663d870 Mon Sep 17 00:00:00 2001 From: Kris Nuttycombe Date: Mon, 27 Jun 2022 10:19:23 -0600 Subject: [PATCH 9/9] Don't panic if the sapling key is missing from the UFVK Instead, just skip Sapling decryption. Also, a trivial namespacing fix. --- zcash_client_backend/src/data_api/chain.rs | 2 +- zcash_client_backend/src/decrypt.rs | 42 +++++++++++----------- zcash_client_backend/src/welding_rig.rs | 3 +- 3 files changed, 25 insertions(+), 22 deletions(-) diff --git a/zcash_client_backend/src/data_api/chain.rs b/zcash_client_backend/src/data_api/chain.rs index 37faba2f5..7a7006c5f 100644 --- a/zcash_client_backend/src/data_api/chain.rs +++ b/zcash_client_backend/src/data_api/chain.rs @@ -215,7 +215,7 @@ where // https://github.com/zcash/librustzcash/issues/403 let dfvks: Vec<_> = ufvks .iter() - .map(|(account, ufvk)| (account, ufvk.sapling().expect("TODO Add Orchard support"))) + .filter_map(|(account, ufvk)| ufvk.sapling().map(move |k| (account, k))) .collect(); // Get the most recent CommitmentTree diff --git a/zcash_client_backend/src/decrypt.rs b/zcash_client_backend/src/decrypt.rs index b845d3dee..29a666c83 100644 --- a/zcash_client_backend/src/decrypt.rs +++ b/zcash_client_backend/src/decrypt.rs @@ -46,28 +46,30 @@ pub fn decrypt_transaction( if let Some(bundle) = tx.sapling_bundle() { for (account, ufvk) in ufvks.iter() { - let dfvk = ufvk.sapling().expect("TODO: Add Orchard support"); - let ivk = dfvk.fvk().vk.ivk(); - let ovk = dfvk.fvk().ovk; + if let Some(dfvk) = ufvk.sapling() { + let ivk = dfvk.fvk().vk.ivk(); + let ovk = dfvk.fvk().ovk; - for (index, output) in bundle.shielded_outputs.iter().enumerate() { - let ((note, to, memo), outgoing) = - match try_sapling_note_decryption(params, height, &ivk, output) { - Some(ret) => (ret, false), - None => match try_sapling_output_recovery(params, height, &ovk, output) { - Some(ret) => (ret, true), - None => continue, - }, - }; + for (index, output) in bundle.shielded_outputs.iter().enumerate() { + let ((note, to, memo), outgoing) = + match try_sapling_note_decryption(params, height, &ivk, output) { + Some(ret) => (ret, false), + None => match try_sapling_output_recovery(params, height, &ovk, output) + { + Some(ret) => (ret, true), + None => continue, + }, + }; - decrypted.push(DecryptedOutput { - index, - note, - account: *account, - to, - memo, - outgoing, - }) + decrypted.push(DecryptedOutput { + index, + note, + account: *account, + to, + memo, + outgoing, + }) + } } } } diff --git a/zcash_client_backend/src/welding_rig.rs b/zcash_client_backend/src/welding_rig.rs index b1c9251b5..2b651d680 100644 --- a/zcash_client_backend/src/welding_rig.rs +++ b/zcash_client_backend/src/welding_rig.rs @@ -9,6 +9,7 @@ use zcash_primitives::{ consensus::{self, BlockHeight}, merkle_tree::{CommitmentTree, IncrementalWitness}, sapling::{ + self, keys::DiversifiableFullViewingKey, note_encryption::{try_sapling_compact_note_decryption, SaplingDomain}, Node, Note, Nullifier, PaymentAddress, SaplingIvk, @@ -129,7 +130,7 @@ pub trait ScanningKey { } impl ScanningKey for DiversifiableFullViewingKey { - type Nf = Nullifier; + type Nf = sapling::Nullifier; fn try_decryption< P: consensus::Parameters,