From 802c01002a39109490d744018e843a6137b655ac Mon Sep 17 00:00:00 2001 From: Kris Nuttycombe Date: Fri, 23 Feb 2024 14:40:45 -0700 Subject: [PATCH] zcash_client_backend: Rework scanning key identifiers. In the process of making the internals of `scan_block_with_runner` reusable across Sapling and Orchard, it became evident that key identifier abstraction along the lines of #1175 is needed more generally. This commit refactors the use of ZIP 32 account identifiers and key scopes to better separate scanning concerns from ZIP 32 key derivation. In the process, this removes a fair amount of unnecessary polymorphism from `zcash_client_backend::wallet::WalletTx` and related types. --- zcash_client_backend/CHANGELOG.md | 14 +- zcash_client_backend/src/data_api.rs | 20 +- zcash_client_backend/src/data_api/chain.rs | 57 +- zcash_client_backend/src/scan.rs | 92 +-- zcash_client_backend/src/scanning.rs | 551 +++++++++--------- zcash_client_backend/src/wallet.rs | 81 ++- zcash_client_sqlite/src/lib.rs | 24 +- zcash_client_sqlite/src/wallet.rs | 21 +- .../init/migrations/receiving_key_scopes.rs | 14 +- zcash_client_sqlite/src/wallet/sapling.rs | 32 +- 10 files changed, 477 insertions(+), 429 deletions(-) diff --git a/zcash_client_backend/CHANGELOG.md b/zcash_client_backend/CHANGELOG.md index 090b10a04..984f5a27f 100644 --- a/zcash_client_backend/CHANGELOG.md +++ b/zcash_client_backend/CHANGELOG.md @@ -84,6 +84,8 @@ and this library adheres to Rust's notion of - `ProposalDecodingError` - `proposal` module, for parsing and serializing transaction proposals. - `impl TryFrom<&CompactSaplingOutput> for CompactOutputDescription` +- `zcash_client_backend::scanning`: + - `ScanningKeys` - `impl Clone for zcash_client_backend::{ zip321::{Payment, TransactionRequest, Zip321Error, parse::Param, parse::IndexedParam}, wallet::WalletTransparentOutput, @@ -136,13 +138,11 @@ and this library adheres to Rust's notion of - `zcash_client_backend::data_api`: - `BlockMetadata::sapling_tree_size` now returns an `Option` instead of a `u32` for future consistency with Orchard. - - `WalletShieldedOutput` has an additional type parameter which is used for - key scope. `WalletShieldedOutput::from_parts` now takes an additional - argument of this type. - - `WalletTx` has an additional type parameter as a consequence of the - `WalletShieldedOutput` change. - - `ScannedBlock` has an additional type parameter as a consequence of the - `WalletTx` change. + - `WalletShieldedOutput::from_parts` now takes an additional key source metadata. + - `WalletTx` is no longer parameterized by the nullifier type; instead, the + nullifier is present as an optional value. + - `ScannedBlock` is no longer parameterized by the nullifier type as a consequence + of the `WalletTx` change. - `ScannedBlock::metadata` has been renamed to `to_block_metadata` and now returns an owned value rather than a reference. - Fields of `Balance` and `AccountBalance` have been made private and the values diff --git a/zcash_client_backend/src/data_api.rs b/zcash_client_backend/src/data_api.rs index 8af3b4b04..32d670065 100644 --- a/zcash_client_backend/src/data_api.rs +++ b/zcash_client_backend/src/data_api.rs @@ -29,7 +29,6 @@ use zcash_primitives::{ components::amount::{Amount, BalanceError, NonNegativeAmount}, Transaction, TxId, }, - zip32::Scope, }; #[cfg(feature = "transparent-inputs")] @@ -713,23 +712,23 @@ pub struct ScannedBlockCommitments { /// decrypted and extracted from a [`CompactBlock`]. /// /// [`CompactBlock`]: crate::proto::compact_formats::CompactBlock -pub struct ScannedBlock { +pub struct ScannedBlock { block_height: BlockHeight, block_hash: BlockHash, block_time: u32, - transactions: Vec>, + transactions: Vec>, sapling: ScannedBundles, #[cfg(feature = "orchard")] orchard: ScannedBundles, } -impl ScannedBlock { +impl ScannedBlock { /// Constructs a new `ScannedBlock` pub(crate) fn from_parts( block_height: BlockHeight, block_hash: BlockHash, block_time: u32, - transactions: Vec>, + transactions: Vec>, sapling: ScannedBundles, #[cfg(feature = "orchard")] orchard: ScannedBundles< orchard::note::NoteCommitment, @@ -763,7 +762,7 @@ impl ScannedBlock { } /// Returns the list of transactions from this block that are relevant to the wallet. - pub fn transactions(&self) -> &[WalletTx] { + pub fn transactions(&self) -> &[WalletTx] { &self.transactions } @@ -1057,10 +1056,8 @@ pub trait WalletWrite: WalletRead { /// pertaining to this wallet. /// /// `blocks` must be sequential, in order of increasing block height - fn put_blocks( - &mut self, - blocks: Vec>, - ) -> Result<(), Self::Error>; + fn put_blocks(&mut self, blocks: Vec>) + -> Result<(), Self::Error>; /// Updates the wallet's view of the blockchain. /// @@ -1188,7 +1185,6 @@ pub mod testing { consensus::{BlockHeight, Network}, memo::Memo, transaction::{components::Amount, Transaction, TxId}, - zip32::Scope, }; use crate::{ @@ -1424,7 +1420,7 @@ pub mod testing { #[allow(clippy::type_complexity)] fn put_blocks( &mut self, - _blocks: Vec>, + _blocks: Vec>, ) -> Result<(), Self::Error> { Ok(()) } diff --git a/zcash_client_backend/src/data_api/chain.rs b/zcash_client_backend/src/data_api/chain.rs index dbc40e2a8..123b520d2 100644 --- a/zcash_client_backend/src/data_api/chain.rs +++ b/zcash_client_backend/src/data_api/chain.rs @@ -145,7 +145,6 @@ use std::ops::Range; -use sapling::note_encryption::PreparedIncomingViewingKey; use subtle::ConditionallySelectable; use zcash_primitives::consensus::{self, BlockHeight}; @@ -153,7 +152,7 @@ use crate::{ data_api::{NullifierQuery, WalletWrite}, proto::compact_formats::CompactBlock, scan::BatchRunner, - scanning::{add_block_to_runner, scan_block_with_runner, ScanningKey}, + scanning::{add_block_to_runner, scan_block_with_runner, ScanningKey, ScanningKeys}, }; pub mod error; @@ -280,27 +279,25 @@ where ::AccountId: ConditionallySelectable + Default + Send + 'static, { // Fetch the UnifiedFullViewingKeys we are tracking - let ufvks = data_db - .get_unified_full_viewing_keys() - .map_err(Error::Wallet)?; - // TODO: Change `scan_block` to also scan Orchard. - // https://github.com/zcash/librustzcash/issues/403 - let sapling_ivks: Vec<_> = ufvks - .iter() - .filter_map(|(account, ufvk)| ufvk.sapling().map(move |k| (account, k))) - .flat_map(|(account, dfvk)| dfvk.to_ivks().into_iter().map(move |key| (account, key))) - .collect::>(); + let mut scanning_keys = ScanningKeys::from_account_ufvks( + data_db + .get_unified_full_viewing_keys() + .map_err(Error::Wallet)?, + ); // Get the nullifiers for the unspent notes we are tracking - let mut sapling_nullifiers = data_db - .get_sapling_nullifiers(NullifierQuery::Unspent) - .map_err(Error::Wallet)?; + scanning_keys.extend_sapling_nullifiers( + data_db + .get_sapling_nullifiers(NullifierQuery::Unspent) + .map_err(Error::Wallet)?, + ); let mut sapling_runner = BatchRunner::<_, _, _, _, ()>::new( 100, - sapling_ivks.iter().map(|(account, (scope, ivk, _))| { - ((**account, *scope), PreparedIncomingViewingKey::new(ivk)) - }), + scanning_keys + .sapling_keys() + .iter() + .map(|(id, key)| (*id, key.prepare())), ); block_source.with_blocks::<_, DbT::Error>( @@ -335,8 +332,7 @@ where let scanned_block = scan_block_with_runner( params, block, - &sapling_ivks, - &sapling_nullifiers, + &scanning_keys, prior_block_metadata.as_ref(), Some(&mut sapling_runner), ) @@ -346,7 +342,10 @@ where .transactions .iter() .fold((0, 0), |(s, r), wtx| { - (s + wtx.sapling_spends.len(), r + wtx.sapling_outputs.len()) + ( + s + wtx.sapling_spends().len(), + r + wtx.sapling_outputs().len(), + ) }); spent_note_count += s; received_note_count += r; @@ -354,15 +353,17 @@ where let spent_nf: Vec<&sapling::Nullifier> = scanned_block .transactions .iter() - .flat_map(|tx| tx.sapling_spends.iter().map(|spend| spend.nf())) + .flat_map(|tx| tx.sapling_spends().iter().map(|spend| spend.nf())) .collect(); - sapling_nullifiers.retain(|(_, nf)| !spent_nf.contains(&nf)); - sapling_nullifiers.extend(scanned_block.transactions.iter().flat_map(|tx| { - tx.sapling_outputs - .iter() - .map(|out| (*out.account(), *out.nf())) - })); + scanning_keys.retain_sapling_nullifiers(|(_, nf)| !spent_nf.contains(&nf)); + scanning_keys.extend_sapling_nullifiers(scanned_block.transactions.iter().flat_map( + |tx| { + tx.sapling_outputs() + .iter() + .flat_map(|out| out.nf().into_iter().map(|nf| (*out.account_id(), *nf))) + }, + )); prior_block_metadata = Some(scanned_block.to_block_metadata()); scanned_blocks.push(scanned_block); diff --git a/zcash_client_backend/src/scan.rs b/zcash_client_backend/src/scan.rs index 98a768622..613602af4 100644 --- a/zcash_client_backend/src/scan.rs +++ b/zcash_client_backend/src/scan.rs @@ -14,9 +14,9 @@ use zcash_note_encryption::{ use zcash_primitives::{block::BlockHash, transaction::TxId}; /// A decrypted transaction output. -pub(crate) struct DecryptedOutput { +pub(crate) struct DecryptedOutput { /// The tag corresponding to the incoming viewing key used to decrypt the note. - pub(crate) ivk_tag: A, + pub(crate) ivk_tag: IvkTag, /// The recipient of the note. pub(crate) recipient: D::Recipient, /// The note! @@ -25,9 +25,9 @@ pub(crate) struct DecryptedOutput { pub(crate) memo: M, } -impl fmt::Debug for DecryptedOutput +impl fmt::Debug for DecryptedOutput where - A: fmt::Debug, + IvkTag: fmt::Debug, D::IncomingViewingKey: fmt::Debug, D::Recipient: fmt::Debug, D::Note: fmt::Debug, @@ -48,11 +48,11 @@ pub(crate) trait Decryptor { type Memo; // Once we reach MSRV 1.75.0, this can return `impl Iterator`. - fn batch_decrypt( - tags: &[A], + fn batch_decrypt( + tags: &[IvkTag], ivks: &[D::IncomingViewingKey], outputs: &[(D, Output)], - ) -> Vec>>; + ) -> Vec>>; } /// A decryptor of outputs as encoded in transactions. @@ -63,11 +63,11 @@ impl> Decryptor( - tags: &[A], + fn batch_decrypt( + tags: &[IvkTag], ivks: &[D::IncomingViewingKey], outputs: &[(D, Output)], - ) -> Vec>> { + ) -> Vec>> { batch::try_note_decryption(ivks, outputs) .into_iter() .map(|res| { @@ -90,11 +90,11 @@ impl> Decryptor( - tags: &[A], + fn batch_decrypt( + tags: &[IvkTag], ivks: &[D::IncomingViewingKey], outputs: &[(D, Output)], - ) -> Vec>> { + ) -> Vec>> { batch::try_compact_note_decryption(ivks, outputs) .into_iter() .map(|res| { @@ -117,12 +117,12 @@ struct OutputIndex { value: V, } -type OutputItem = OutputIndex>; +type OutputItem = OutputIndex>; /// The sender for the result of batch scanning a specific transaction output. -struct OutputReplier(OutputIndex>>); +struct OutputReplier(OutputIndex>>); -impl DynamicUsage for OutputReplier { +impl DynamicUsage for OutputReplier { #[inline(always)] fn dynamic_usage(&self) -> usize { // We count the memory usage of items in the channel on the receiver side. @@ -136,9 +136,9 @@ impl DynamicUsage for OutputReplier { } /// The receiver for the result of batch scanning a specific transaction. -struct BatchReceiver(channel::Receiver>); +struct BatchReceiver(channel::Receiver>); -impl DynamicUsage for BatchReceiver { +impl DynamicUsage for BatchReceiver { fn dynamic_usage(&self) -> usize { // We count the memory usage of items in the channel on the receiver side. let num_items = self.0.len(); @@ -155,7 +155,7 @@ impl DynamicUsage for BatchReceiver { // - Space for an item. // - The state of the slot, stored as an AtomicUsize. const PTR_SIZE: usize = std::mem::size_of::(); - let item_size = std::mem::size_of::>(); + let item_size = std::mem::size_of::>(); const ATOMIC_USIZE_SIZE: usize = std::mem::size_of::(); let block_size = PTR_SIZE + ITEMS_PER_BLOCK * (item_size + ATOMIC_USIZE_SIZE); @@ -279,8 +279,8 @@ impl Task for WithUsageTask { } /// A batch of outputs to trial decrypt. -pub(crate) struct Batch> { - tags: Vec, +pub(crate) struct Batch> { + tags: Vec, ivks: Vec, /// We currently store outputs and repliers as parallel vectors, because /// [`batch::try_note_decryption`] accepts a slice of domain/output pairs @@ -290,12 +290,12 @@ pub(crate) struct Batch> { /// all be part of the same struct, which would also track the output index /// (that is captured in the outer `OutputIndex` of each `OutputReplier`). outputs: Vec<(D, Output)>, - repliers: Vec>, + repliers: Vec>, } -impl DynamicUsage for Batch +impl DynamicUsage for Batch where - A: DynamicUsage, + IvkTag: DynamicUsage, D: BatchDomain + DynamicUsage, D::IncomingViewingKey: DynamicUsage, Output: DynamicUsage, @@ -325,14 +325,14 @@ where } } -impl Batch +impl Batch where - A: Clone, + IvkTag: Clone, D: BatchDomain, Dec: Decryptor, { /// Constructs a new batch. - fn new(tags: Vec, ivks: Vec) -> Self { + fn new(tags: Vec, ivks: Vec) -> Self { assert_eq!(tags.len(), ivks.len()); Self { tags, @@ -348,9 +348,9 @@ where } } -impl Task for Batch +impl Task for Batch where - A: Clone + Send + 'static, + IvkTag: Clone + Send + 'static, D: BatchDomain + Send + 'static, D::IncomingViewingKey: Send, D::Memo: Send, @@ -393,7 +393,7 @@ where } } -impl Batch +impl Batch where D: BatchDomain, Output: Clone, @@ -406,7 +406,7 @@ where &mut self, domain: impl Fn() -> D, outputs: &[Output], - replier: channel::Sender>, + replier: channel::Sender>, ) { self.outputs .extend(outputs.iter().cloned().map(|output| (domain(), output))); @@ -436,29 +436,29 @@ impl DynamicUsage for ResultKey { } /// Logic to run batches of trial decryptions on the global threadpool. -pub(crate) struct BatchRunner +pub(crate) struct BatchRunner where D: BatchDomain, Dec: Decryptor, - T: Tasks>, + T: Tasks>, { batch_size_threshold: usize, // The batch currently being accumulated. - acc: Batch, + acc: Batch, // The running batches. running_tasks: T, // Receivers for the results of the running batches. - pending_results: HashMap>, + pending_results: HashMap>, } -impl DynamicUsage for BatchRunner +impl DynamicUsage for BatchRunner where - A: DynamicUsage, + IvkTag: DynamicUsage, D: BatchDomain + DynamicUsage, D::IncomingViewingKey: DynamicUsage, Output: DynamicUsage, Dec: Decryptor, - T: Tasks> + DynamicUsage, + T: Tasks> + DynamicUsage, { fn dynamic_usage(&self) -> usize { self.acc.dynamic_usage() @@ -484,17 +484,17 @@ where } } -impl BatchRunner +impl BatchRunner where - A: Clone, + IvkTag: Clone, D: BatchDomain, Dec: Decryptor, - T: Tasks>, + T: Tasks>, { /// Constructs a new batch runner for the given incoming viewing keys. pub(crate) fn new( batch_size_threshold: usize, - ivks: impl Iterator, + ivks: impl Iterator, ) -> Self { let (tags, ivks) = ivks.unzip(); Self { @@ -506,9 +506,9 @@ where } } -impl BatchRunner +impl BatchRunner where - A: Clone + Send + 'static, + IvkTag: Clone + Send + 'static, D: BatchDomain + Send + 'static, D::IncomingViewingKey: Clone + Send, D::Memo: Send, @@ -516,7 +516,7 @@ where D::Recipient: Send, Output: Clone + Send + 'static, Dec: Decryptor, - T: Tasks>, + T: Tasks>, { /// Batches the given outputs for trial decryption. /// @@ -564,7 +564,7 @@ where &mut self, block_tag: BlockHash, txid: TxId, - ) -> HashMap<(TxId, usize), DecryptedOutput> { + ) -> HashMap<(TxId, usize), DecryptedOutput> { self.pending_results .remove(&ResultKey(block_tag, txid)) // We won't have a pending result if the transaction didn't have outputs of diff --git a/zcash_client_backend/src/scanning.rs b/zcash_client_backend/src/scanning.rs index 84279c1e7..957468ade 100644 --- a/zcash_client_backend/src/scanning.rs +++ b/zcash_client_backend/src/scanning.rs @@ -7,12 +7,12 @@ use std::hash::Hash; use incrementalmerkletree::{Position, Retention}; use sapling::{ - note_encryption::{CompactOutputDescription, PreparedIncomingViewingKey, SaplingDomain}, - zip32::DiversifiableFullViewingKey, + note_encryption::{CompactOutputDescription, SaplingDomain}, SaplingIvk, }; use subtle::{ConditionallySelectable, ConstantTimeEq, CtOption}; -use zcash_note_encryption::{batch, BatchDomain, ShieldedOutput}; +use zcash_keys::keys::UnifiedFullViewingKey; +use zcash_note_encryption::{batch, BatchDomain, Domain, ShieldedOutput}; use zcash_primitives::{ consensus::{self, BlockHeight, NetworkUpgrade}, transaction::TxId, @@ -40,139 +40,183 @@ use crate::{ /// /// [`CompactSaplingOutput`]: crate::proto::compact_formats::CompactSaplingOutput /// [`scan_block`]: crate::scanning::scan_block -pub trait ScanningKey { - /// The type representing the scope of the scanning key. - type Scope: Clone + Eq + std::hash::Hash + Send + 'static; - - /// The type of key that is used to decrypt outputs belonging to the wallet. - type IncomingViewingKey: Clone; - +pub trait ScanningKey { /// The type of key that is used to derive nullifiers. type NullifierDerivingKey: Clone; /// The type of nullifier extracted when a note is successfully obtained by trial decryption. type Nf; - /// The type of notes obtained by trial decryption. - type Note; + /// Prepare the key for use in batch trial decryption. + fn prepare(&self) -> D::IncomingViewingKey; - /// Obtain the underlying incoming viewing key(s) for this scanning key. - fn to_ivks( - &self, - ) -> Vec<( - Self::Scope, - Self::IncomingViewingKey, - Self::NullifierDerivingKey, - )>; + /// Returns the account identifier for this key. An account identifier corresponds + /// to at most a single unified spending key's worth of spend authority, such that + /// both received notes and change spendable by that spending authority will be + /// interpreted as belonging to that account. + fn account_id(&self) -> &AccountId; + + /// Returns the [`zip32::Scope`] for which this key was derived, if known. + fn key_scope(&self) -> Option; /// Produces the nullifier for the specified note and witness, if possible. /// /// IVK-based implementations of this trait cannot successfully derive /// nullifiers, in which case `Self::Nf` should be set to the unit type /// and this function is a no-op. - fn nf(key: &Self::NullifierDerivingKey, note: &Self::Note, note_position: Position) - -> Self::Nf; + fn nf(&self, note: &D::Note, note_position: Position) -> Option; } -impl ScanningKey for &K { - type Scope = K::Scope; - type IncomingViewingKey = K::IncomingViewingKey; +impl> ScanningKey for &K { type NullifierDerivingKey = K::NullifierDerivingKey; type Nf = K::Nf; - type Note = K::Note; - fn to_ivks( - &self, - ) -> Vec<( - Self::Scope, - Self::IncomingViewingKey, - Self::NullifierDerivingKey, - )> { - (*self).to_ivks() + fn prepare(&self) -> D::IncomingViewingKey { + (*self).prepare() } - fn nf(key: &Self::NullifierDerivingKey, note: &Self::Note, position: Position) -> Self::Nf { - K::nf(key, note, position) + fn account_id(&self) -> &AccountId { + (*self).account_id() + } + + fn key_scope(&self) -> Option { + (*self).key_scope() + } + + fn nf(&self, note: &D::Note, note_position: Position) -> Option { + (*self).nf(note, note_position) } } -impl ScanningKey for DiversifiableFullViewingKey { - type Scope = Scope; - type IncomingViewingKey = SaplingIvk; +pub struct SaplingScanningKey { + ivk: SaplingIvk, + nk: Option, + account_id: AccountId, + key_scope: Option, +} + +impl ScanningKey for SaplingScanningKey { type NullifierDerivingKey = sapling::NullifierDerivingKey; type Nf = sapling::Nullifier; - type Note = sapling::Note; - fn to_ivks( - &self, - ) -> Vec<( - Self::Scope, - Self::IncomingViewingKey, - Self::NullifierDerivingKey, - )> { - vec![ - ( - Scope::External, - self.to_ivk(Scope::External), - self.to_nk(Scope::External), - ), - ( - Scope::Internal, - self.to_ivk(Scope::Internal), - self.to_nk(Scope::Internal), - ), - ] + fn prepare(&self) -> sapling::note_encryption::PreparedIncomingViewingKey { + sapling::note_encryption::PreparedIncomingViewingKey::new(&self.ivk) } - fn nf(key: &Self::NullifierDerivingKey, note: &Self::Note, position: Position) -> Self::Nf { - note.nf(key, position.into()) + fn nf(&self, note: &sapling::Note, position: Position) -> Option { + self.nk.as_ref().map(|key| note.nf(key, position.into())) + } + + fn account_id(&self) -> &AccountId { + &self.account_id + } + + fn key_scope(&self) -> Option { + self.key_scope } } -impl ScanningKey for (Scope, SaplingIvk, sapling::NullifierDerivingKey) { - type Scope = Scope; - type IncomingViewingKey = SaplingIvk; +impl ScanningKey for (AccountId, SaplingIvk) { type NullifierDerivingKey = sapling::NullifierDerivingKey; type Nf = sapling::Nullifier; - type Note = sapling::Note; - fn to_ivks( - &self, - ) -> Vec<( - Self::Scope, - Self::IncomingViewingKey, - Self::NullifierDerivingKey, - )> { - vec![self.clone()] + fn prepare(&self) -> sapling::note_encryption::PreparedIncomingViewingKey { + sapling::note_encryption::PreparedIncomingViewingKey::new(&self.1) } - fn nf(key: &Self::NullifierDerivingKey, note: &Self::Note, position: Position) -> Self::Nf { - note.nf(key, position.into()) + fn nf(&self, _note: &sapling::Note, _position: Position) -> Option { + None + } + + fn account_id(&self) -> &AccountId { + &self.0 + } + + fn key_scope(&self) -> Option { + None } } -/// The [`ScanningKey`] implementation for [`SaplingIvk`]s. -/// Nullifiers cannot be derived when scanning with these keys. -/// -/// [`SaplingIvk`]: sapling::SaplingIvk -impl ScanningKey for SaplingIvk { - type Scope = (); - type IncomingViewingKey = SaplingIvk; - type NullifierDerivingKey = (); - type Nf = (); - type Note = sapling::Note; +pub struct ScanningKeys { + sapling_keys: HashMap>, + sapling_nullifiers: Vec<(AccountId, sapling::Nullifier)>, +} - fn to_ivks( - &self, - ) -> Vec<( - Self::Scope, - Self::IncomingViewingKey, - Self::NullifierDerivingKey, - )> { - vec![((), self.clone(), ())] +impl ScanningKeys { + pub fn empty() -> Self { + Self { + sapling_keys: HashMap::new(), + sapling_nullifiers: vec![], + } } - fn nf(_key: &Self::NullifierDerivingKey, _note: &Self::Note, _position: Position) -> Self::Nf {} + pub fn sapling_keys(&self) -> &HashMap> { + &self.sapling_keys + } + + pub fn sapling_nullifiers(&self) -> &[(AccountId, sapling::Nullifier)] { + self.sapling_nullifiers.as_ref() + } +} + +impl ScanningKeys { + pub fn from_account_ufvks( + ufvks: impl IntoIterator, + ) -> Self { + let sapling_keys = ufvks + .into_iter() + .flat_map(|(account_id, ufvk)| { + if let Some(dfvk) = ufvk.sapling() { + vec![ + SaplingScanningKey { + ivk: dfvk.to_ivk(Scope::External), + nk: Some(dfvk.to_nk(Scope::External)), + account_id, + key_scope: Some(Scope::External), + }, + SaplingScanningKey { + ivk: dfvk.to_ivk(Scope::Internal), + nk: Some(dfvk.to_nk(Scope::Internal)), + account_id, + key_scope: Some(Scope::Internal), + }, + ] + } else { + vec![] + } + .into_iter() + }) + .map(|key| { + ( + ( + *key.account_id(), + key.key_scope() + .expect("Key scope is available for each key"), + ), + key, + ) + }) + .collect::>(); + + Self { + sapling_keys, + sapling_nullifiers: vec![], + } + } + + pub(crate) fn retain_sapling_nullifiers( + &mut self, + f: impl Fn(&(AccountId, sapling::Nullifier)) -> bool, + ) { + self.sapling_nullifiers.retain(f); + } + + pub(crate) fn extend_sapling_nullifiers( + &mut self, + nfs: impl IntoIterator, + ) { + self.sapling_nullifiers.extend(nfs); + } } /// Errors that may occur in chain scanning @@ -266,65 +310,40 @@ impl fmt::Display for ScanError { } } -/// Scans a [`CompactBlock`] with a set of [`ScanningKey`]s. +/// Scans a [`CompactBlock`] with a set of [`ScanningKeys`]. /// -/// Returns a vector of [`WalletTx`]s belonging to any of the given -/// [`ScanningKey`]s. If scanning with a full viewing key, the nullifiers -/// of the resulting [`WalletSaplingOutput`]s will also be computed. +/// Returns a vector of [`WalletTx`]s decryptable by any of the given keys. If an output is +/// decrypted by a full viewing key, the nullifiers of that output will also be computed. /// -/// The given [`CommitmentTree`] and existing [`IncrementalWitness`]es are -/// incremented appropriately. -/// -/// The implementation of [`ScanningKey`] may either support or omit the computation of -/// the nullifiers for received notes; the implementation for [`ExtendedFullViewingKey`] -/// will derive the nullifiers for received notes and return them as part of the resulting -/// [`WalletSaplingOutput`]s, whereas the implementation for [`SaplingIvk`] cannot -/// do so and will return the unit value in those outputs instead. -/// -/// [`ExtendedFullViewingKey`]: sapling::zip32::ExtendedFullViewingKey -/// [`SaplingIvk`]: sapling::SaplingIvk /// [`CompactBlock`]: crate::proto::compact_formats::CompactBlock -/// [`ScanningKey`]: crate::scanning::ScanningKey -/// [`CommitmentTree`]: sapling::CommitmentTree -/// [`IncrementalWitness`]: sapling::IncrementalWitness -/// [`WalletSaplingOutput`]: crate::wallet::WalletSaplingOutput /// [`WalletTx`]: crate::wallet::WalletTx -pub fn scan_block( +pub fn scan_block( params: &P, block: CompactBlock, - sapling_keys: &[(&A, &SK)], - sapling_nullifiers: &[(A, sapling::Nullifier)], + scanning_keys: &ScanningKeys, prior_block_metadata: Option<&BlockMetadata>, -) -> Result, ScanError> +) -> Result, ScanError> where P: consensus::Parameters + Send + 'static, - A: Default + Eq + Hash + Send + ConditionallySelectable + 'static, - SK: ScanningKey, + AccountId: Default + Eq + Hash + ConditionallySelectable + Send + 'static, + IvkTag: Copy + std::hash::Hash + Eq + Send + 'static, { - scan_block_with_runner::<_, A, _, ()>( - params, - block, - sapling_keys, - sapling_nullifiers, - prior_block_metadata, - None, - ) + scan_block_with_runner::<_, _, _, ()>(params, block, scanning_keys, prior_block_metadata, None) } -type TaggedBatch = Batch<(A, S), SaplingDomain, CompactOutputDescription, CompactDecryptor>; -type TaggedBatchRunner = - BatchRunner<(A, S), SaplingDomain, CompactOutputDescription, CompactDecryptor, T>; +type TaggedBatch = Batch; +type TaggedBatchRunner = + BatchRunner; #[tracing::instrument(skip_all, fields(height = block.height))] -pub(crate) fn add_block_to_runner( +pub(crate) fn add_block_to_runner( params: &P, block: CompactBlock, - batch_runner: &mut TaggedBatchRunner, + batch_runner: &mut TaggedBatchRunner, ) where P: consensus::Parameters + Send + 'static, - S: Clone + Send + 'static, - T: Tasks>, - A: Copy + Default + Eq + Send + 'static, + IvkTag: Copy + Send + 'static, + T: Tasks>, { let block_hash = block.hash(); let block_height = block.height(); @@ -373,19 +392,18 @@ fn check_hash_continuity( } #[tracing::instrument(skip_all, fields(height = block.height))] -pub(crate) fn scan_block_with_runner( +pub(crate) fn scan_block_with_runner( params: &P, block: CompactBlock, - sapling_keys: &[(&A, SK)], - sapling_nullifiers: &[(A, sapling::Nullifier)], + scanning_keys: &ScanningKeys, prior_block_metadata: Option<&BlockMetadata>, - mut sapling_batch_runner: Option<&mut TaggedBatchRunner>, -) -> Result, ScanError> + mut sapling_batch_runner: Option<&mut TaggedBatchRunner>, +) -> Result, ScanError> where P: consensus::Parameters + Send + 'static, - SK: ScanningKey, - T: Tasks> + Sync, - A: Default + Eq + Hash + ConditionallySelectable + Send + 'static, + AccountId: Default + Eq + Hash + ConditionallySelectable + Send + 'static, + IvkTag: Copy + std::hash::Hash + Eq + Send + 'static, + T: Tasks> + Sync, { if let Some(scan_error) = check_hash_continuity(&block, prior_block_metadata) { return Err(scan_error); @@ -489,7 +507,7 @@ where )?; let compact_block_tx_count = block.vtx.len(); - let mut wtxs: Vec> = vec![]; + let mut wtxs: Vec> = vec![]; let mut sapling_nullifier_map = Vec::with_capacity(block.vtx.len()); let mut sapling_note_commitments: Vec<(sapling::Node, Retention)> = vec![]; for (tx_idx, tx) in block.vtx.into_iter().enumerate() { @@ -499,7 +517,7 @@ where let (sapling_spends, sapling_unlinked_nullifiers) = find_spent( &tx.spends, - sapling_nullifiers, + &scanning_keys.sapling_nullifiers, |spend| { spend.nf().expect( "Could not deserialize nullifier for spend from protobuf representation.", @@ -522,7 +540,7 @@ where txid, tx_idx, sapling_commitment_tree_size, - sapling_keys, + &scanning_keys.sapling_keys, &spent_from_accounts, &tx.outputs .iter() @@ -537,31 +555,30 @@ where sapling_batch_runner .as_mut() .map(|runner| |txid| runner.collect_results(cur_hash, txid)), - PreparedIncomingViewingKey::new, |output| sapling::Node::from_cmu(&output.cmu), - |output_idx, output, account, note, is_change, position, nf, scope| { + |output_idx, output, note, is_change, position, nf, account_id, key_scope| { WalletSaplingOutput::from_parts( output_idx, output.cmu, output.ephemeral_key.clone(), - account, + account_id, note, is_change, position, nf, - scope, + key_scope, ) }, ); sapling_note_commitments.append(&mut sapling_nc); if !(sapling_spends.is_empty() && sapling_outputs.is_empty()) { - wtxs.push(WalletTx { + wtxs.push(WalletTx::new( txid, - index: tx_index as usize, + tx_index as usize, sapling_spends, sapling_outputs, - }); + )); } sapling_commitment_tree_size += @@ -615,11 +632,16 @@ where // Check for spent notes. The comparison against known-unspent nullifiers is done // in constant time. -fn find_spent( +fn find_spent< + AccountId: ConditionallySelectable + Default, + Spend, + Nf: ConstantTimeEq + Copy, + WS, +>( spends: &[Spend], - nullifiers: &[(A, Nf)], + nullifiers: &[(AccountId, Nf)], extract_nf: impl Fn(&Spend) -> Nf, - construct_wallet_spend: impl Fn(usize, Nf, A) -> WS, + construct_wallet_spend: impl Fn(usize, Nf, AccountId) -> WS, ) -> (Vec, Vec) { // TODO: this is O(|nullifiers| * |notes|); does using constant-time operations here really // make sense? @@ -628,17 +650,18 @@ fn find_spent, + IvkTag: Copy + std::hash::Hash + Eq + Send + 'static, + SK: ScanningKey, Output: ShieldedOutput, WalletOutput, NoteCommitment, @@ -664,23 +689,22 @@ fn find_received< txid: TxId, tx_idx: usize, commitment_tree_size: u32, - keys: &[(&A, SK)], - spent_from_accounts: &HashSet, + keys: &HashMap, + spent_from_accounts: &HashSet, decoded: &[(D, Output)], batch_results: Option< - impl FnOnce(TxId) -> HashMap<(TxId, usize), DecryptedOutput<(A, SK::Scope), D, ()>>, + impl FnOnce(TxId) -> HashMap<(TxId, usize), DecryptedOutput>, >, - prepare_key: impl Fn(&SK::IncomingViewingKey) -> D::IncomingViewingKey, extract_note_commitment: impl Fn(&Output) -> NoteCommitment, new_wallet_output: impl Fn( usize, &Output, - A, - SK::Note, + D::Note, bool, Position, - SK::Nf, - SK::Scope, + Option, + AccountId, + Option, ) -> WalletOutput, ) -> ( Vec, @@ -688,45 +712,25 @@ fn find_received< ) { // Check for incoming notes while incrementing tree and witnesses let (decrypted_opts, decrypted_len) = if let Some(collect_results) = batch_results { - let tagged_keys = keys - .iter() - .flat_map(|(a, k)| { - k.to_ivks() - .into_iter() - .map(move |(scope, _, nk)| ((**a, scope), nk)) - }) - .collect::>(); - let mut decrypted = collect_results(txid); let decrypted_len = decrypted.len(); ( (0..decoded.len()) .map(|i| { - decrypted.remove(&(txid, i)).map(|d_out| { - let nk = tagged_keys.get(&d_out.ivk_tag).expect( - "The batch runner and scan_block must use the same set of IVKs.", - ); - - (d_out.note, d_out.ivk_tag.0, d_out.ivk_tag.1, (*nk).clone()) - }) + decrypted + .remove(&(txid, i)) + .map(|d_out| (d_out.ivk_tag, d_out.note)) }) .collect::>(), decrypted_len, ) } else { - let tagged_keys = keys - .iter() - .flat_map(|(a, k)| { - k.to_ivks() - .into_iter() - .map(move |(scope, ivk, nk)| (**a, scope, ivk, nk)) - }) - .collect::>(); - - let ivks = tagged_keys - .iter() - .map(|(_, _, ivk, _)| prepare_key(ivk)) - .collect::>(); + let mut ivks = Vec::with_capacity(keys.len()); + let mut ivk_lookup = Vec::with_capacity(keys.len()); + for (key_id, key) in keys.iter() { + ivks.push(key.prepare()); + ivk_lookup.push(key_id); + } let mut decrypted_len = 0; ( @@ -735,8 +739,7 @@ fn find_received< .map(|v| { v.map(|((note, _), ivk_idx)| { decrypted_len += 1; - let (a, scope, _, nk) = &tagged_keys[ivk_idx]; - (note, *a, scope.clone(), (*nk).clone()) + (*ivk_lookup[ivk_idx], note) }) }) .collect::>(), @@ -746,11 +749,13 @@ fn find_received< let mut shielded_outputs = Vec::with_capacity(decrypted_len); let mut note_commitments = Vec::with_capacity(decoded.len()); - for (output_idx, ((_, output), dec_output)) in decoded.iter().zip(decrypted_opts).enumerate() { + for (output_idx, ((_, output), decrypted_note)) in + decoded.iter().zip(decrypted_opts).enumerate() + { // Collect block note commitments let node = extract_note_commitment(output); let is_checkpoint = output_idx + 1 == decoded.len() && tx_idx + 1 == block_tx_count; - let retention = match (dec_output.is_some(), is_checkpoint) { + let retention = match (decrypted_note.is_some(), is_checkpoint) { (is_marked, true) => Retention::Checkpoint { id: block_height, is_marked, @@ -759,28 +764,32 @@ fn find_received< (false, false) => Retention::Ephemeral, }; - if let Some((note, account, scope, nk)) = dec_output { + if let Some((key_id, note)) = decrypted_note { + let key = keys + .get(&key_id) + .expect("Key is available for decrypted output"); + // A note is marked as "change" if the account that received it // also spent notes in the same transaction. This will catch, // for instance: // - Change created by spending fractions of notes. // - Notes created by consolidation transactions. // - Notes sent from one account to itself. - let is_change = spent_from_accounts.contains(&account); + let is_change = spent_from_accounts.contains(key.account_id()); let note_commitment_tree_position = Position::from(u64::from( commitment_tree_size + u32::try_from(output_idx).unwrap(), )); - let nf = SK::nf(&nk, ¬e, note_commitment_tree_position); + let nf = key.nf(¬e, note_commitment_tree_position); shielded_outputs.push(new_wallet_output( output_idx, output, - account, note, is_change, note_commitment_tree_position, nf, - scope, + *key.account_id(), + key.key_scope(), )); } @@ -792,6 +801,7 @@ fn find_received< #[cfg(test)] mod tests { + use group::{ ff::{Field, PrimeField}, GroupEncoding, @@ -800,12 +810,13 @@ mod tests { use rand_core::{OsRng, RngCore}; use sapling::{ constants::SPENDING_KEY_GENERATOR, - note_encryption::{sapling_note_encryption, PreparedIncomingViewingKey, SaplingDomain}, + note_encryption::{sapling_note_encryption, SaplingDomain}, util::generate_random_rseed, value::NoteValue, - zip32::{DiversifiableFullViewingKey, ExtendedSpendingKey}, - Nullifier, SaplingIvk, + zip32::DiversifiableFullViewingKey, + Nullifier, }; + use zcash_keys::keys::UnifiedSpendingKey; use zcash_note_encryption::Domain; use zcash_primitives::{ block::BlockHash, @@ -821,9 +832,10 @@ mod tests { self as compact, CompactBlock, CompactSaplingOutput, CompactSaplingSpend, CompactTx, }, scan::BatchRunner, + scanning::{ScanningKey, ScanningKeys}, }; - use super::{add_block_to_runner, scan_block, scan_block_with_runner, ScanningKey}; + use super::{add_block_to_runner, scan_block, scan_block_with_runner}; fn random_compact_tx(mut rng: impl RngCore) -> CompactTx { let fake_nf = { @@ -946,15 +958,19 @@ mod tests { #[test] fn scan_block_with_my_tx() { fn go(scan_multithreaded: bool) { + let network = Network::TestNetwork; let account = AccountId::ZERO; - let extsk = ExtendedSpendingKey::master(&[]); - let dfvk = extsk.to_diversifiable_full_viewing_key(); + let usk = + UnifiedSpendingKey::from_seed(&network, &[0u8; 32], account).expect("Valid USK"); + let ufvk = usk.to_unified_full_viewing_key(); + let sapling_dfvk = ufvk.sapling().expect("Sapling key is present").clone(); + let scanning_keys = ScanningKeys::from_account_ufvks([(account, ufvk)]); let cb = fake_compact_block( 1u32.into(), BlockHash([0; 32]), Nullifier([0; 32]), - &dfvk, + &sapling_dfvk, NonNegativeAmount::const_from_u64(5), false, None, @@ -964,10 +980,10 @@ mod tests { let mut batch_runner = if scan_multithreaded { let mut runner = BatchRunner::<_, _, _, _, ()>::new( 10, - dfvk.to_ivks() + scanning_keys + .sapling_keys() .iter() - .map(|(scope, ivk, _)| ((account, *scope), ivk)) - .map(|(tag, ivk)| (tag, PreparedIncomingViewingKey::new(ivk))), + .map(|(key_id, key)| (*key_id, key.prepare())), ); add_block_to_runner(&Network::TestNetwork, cb.clone(), &mut runner); @@ -979,10 +995,9 @@ mod tests { }; let scanned_block = scan_block_with_runner( - &Network::TestNetwork, + &network, cb, - &[(&account, &dfvk)], - &[], + &scanning_keys, Some(&BlockMetadata::from_parts( BlockHeight::from(0), BlockHash([0u8; 32]), @@ -997,14 +1012,14 @@ mod tests { assert_eq!(txs.len(), 1); let tx = &txs[0]; - assert_eq!(tx.index, 1); - assert_eq!(tx.sapling_spends.len(), 0); - assert_eq!(tx.sapling_outputs.len(), 1); - assert_eq!(tx.sapling_outputs[0].index(), 0); - assert_eq!(*tx.sapling_outputs[0].account(), account); - assert_eq!(tx.sapling_outputs[0].note().value().inner(), 5); + assert_eq!(tx.block_index(), 1); + assert_eq!(tx.sapling_spends().len(), 0); + assert_eq!(tx.sapling_outputs().len(), 1); + assert_eq!(tx.sapling_outputs()[0].index(), 0); + assert_eq!(tx.sapling_outputs()[0].account_id(), &account); + assert_eq!(tx.sapling_outputs()[0].note().value().inner(), 5); assert_eq!( - tx.sapling_outputs[0].note_commitment_tree_position(), + tx.sapling_outputs()[0].note_commitment_tree_position(), Position::from(1) ); @@ -1033,15 +1048,19 @@ mod tests { #[test] fn scan_block_with_txs_after_my_tx() { fn go(scan_multithreaded: bool) { + let network = Network::TestNetwork; let account = AccountId::ZERO; - let extsk = ExtendedSpendingKey::master(&[]); - let dfvk = extsk.to_diversifiable_full_viewing_key(); + let usk = + UnifiedSpendingKey::from_seed(&network, &[0u8; 32], account).expect("Valid USK"); + let ufvk = usk.to_unified_full_viewing_key(); + let sapling_dfvk = ufvk.sapling().expect("Sapling key is present").clone(); + let scanning_keys = ScanningKeys::from_account_ufvks([(account, ufvk)]); let cb = fake_compact_block( 1u32.into(), BlockHash([0; 32]), Nullifier([0; 32]), - &dfvk, + &sapling_dfvk, NonNegativeAmount::const_from_u64(5), true, Some((0, 0)), @@ -1051,10 +1070,10 @@ mod tests { let mut batch_runner = if scan_multithreaded { let mut runner = BatchRunner::<_, _, _, _, ()>::new( 10, - dfvk.to_ivks() + scanning_keys + .sapling_keys() .iter() - .map(|(scope, ivk, _)| ((account, *scope), ivk)) - .map(|(tag, ivk)| (tag, PreparedIncomingViewingKey::new(ivk))), + .map(|(key_id, key)| (*key_id, key.prepare())), ); add_block_to_runner(&Network::TestNetwork, cb.clone(), &mut runner); @@ -1065,25 +1084,19 @@ mod tests { None }; - let scanned_block = scan_block_with_runner( - &Network::TestNetwork, - cb, - &[(&AccountId::ZERO, &dfvk)], - &[], - None, - batch_runner.as_mut(), - ) - .unwrap(); + let scanned_block = + scan_block_with_runner(&network, cb, &scanning_keys, None, batch_runner.as_mut()) + .unwrap(); let txs = scanned_block.transactions(); assert_eq!(txs.len(), 1); let tx = &txs[0]; - assert_eq!(tx.index, 1); - assert_eq!(tx.sapling_spends.len(), 0); - assert_eq!(tx.sapling_outputs.len(), 1); - assert_eq!(tx.sapling_outputs[0].index(), 0); - assert_eq!(*tx.sapling_outputs[0].account(), AccountId::ZERO); - assert_eq!(tx.sapling_outputs[0].note().value().inner(), 5); + assert_eq!(tx.block_index(), 1); + assert_eq!(tx.sapling_spends().len(), 0); + assert_eq!(tx.sapling_outputs().len(), 1); + assert_eq!(tx.sapling_outputs()[0].index(), 0); + assert_eq!(tx.sapling_outputs()[0].account_id(), &AccountId::ZERO); + assert_eq!(tx.sapling_outputs()[0].note().value().inner(), 5); assert_eq!( scanned_block @@ -1109,41 +1122,37 @@ mod tests { #[test] fn scan_block_with_my_spend() { - let extsk = ExtendedSpendingKey::master(&[]); - let dfvk = extsk.to_diversifiable_full_viewing_key(); - let nf = Nullifier([7; 32]); + let network = Network::TestNetwork; let account = AccountId::try_from(12).unwrap(); + let usk = UnifiedSpendingKey::from_seed(&network, &[0u8; 32], account).expect("Valid USK"); + let ufvk = usk.to_unified_full_viewing_key(); + let mut scanning_keys = ScanningKeys::empty(); + + let nf = Nullifier([7; 32]); + scanning_keys.extend_sapling_nullifiers([(account, nf)]); let cb = fake_compact_block( 1u32.into(), BlockHash([0; 32]), nf, - &dfvk, + ufvk.sapling().unwrap(), NonNegativeAmount::const_from_u64(5), false, Some((0, 0)), ); assert_eq!(cb.vtx.len(), 2); - let sapling_keys: Vec<(&AccountId, &SaplingIvk)> = vec![]; - let scanned_block = scan_block( - &Network::TestNetwork, - cb, - &sapling_keys[..], - &[(account, nf)], - None, - ) - .unwrap(); + let scanned_block = scan_block(&network, cb, &scanning_keys, None).unwrap(); let txs = scanned_block.transactions(); assert_eq!(txs.len(), 1); let tx = &txs[0]; - assert_eq!(tx.index, 1); - assert_eq!(tx.sapling_spends.len(), 1); - assert_eq!(tx.sapling_outputs.len(), 0); - assert_eq!(tx.sapling_spends[0].index(), 0); - assert_eq!(tx.sapling_spends[0].nf(), &nf); - assert_eq!(tx.sapling_spends[0].account().to_owned(), account); + assert_eq!(tx.block_index(), 1); + assert_eq!(tx.sapling_spends().len(), 1); + assert_eq!(tx.sapling_outputs().len(), 0); + assert_eq!(tx.sapling_spends()[0].index(), 0); + assert_eq!(tx.sapling_spends()[0].nf(), &nf); + assert_eq!(tx.sapling_spends()[0].account(), &account); assert_eq!( scanned_block diff --git a/zcash_client_backend/src/wallet.rs b/zcash_client_backend/src/wallet.rs index ea0f3372c..c439ed654 100644 --- a/zcash_client_backend/src/wallet.rs +++ b/zcash_client_backend/src/wallet.rs @@ -94,14 +94,53 @@ impl Recipient> { } } -/// A subset of a [`Transaction`] relevant to wallets and light clients. +/// The shielded subset of a [`Transaction`]'s data that is relevant to a particular wallet. /// /// [`Transaction`]: zcash_primitives::transaction::Transaction -pub struct WalletTx { - pub txid: TxId, - pub index: usize, - pub sapling_spends: Vec>, - pub sapling_outputs: Vec>, +pub struct WalletTx { + txid: TxId, + block_index: usize, + sapling_spends: Vec>, + sapling_outputs: Vec>, +} + +impl WalletTx { + /// Constructs a new [`WalletTx`] from its constituent parts + pub fn new( + txid: TxId, + block_index: usize, + sapling_spends: Vec>, + sapling_outputs: Vec>, + ) -> Self { + Self { + txid, + block_index, + sapling_spends, + sapling_outputs, + } + } + + /// Returns the [`TxId`] for the corresponding [`Transaction`] + pub fn txid(&self) -> TxId { + self.txid + } + + /// Returns the index of the transaction in the containing block. + pub fn block_index(&self) -> usize { + self.block_index + } + + /// Returns a record for each Sapling note belonging to the wallet that was spent in the + /// transaction. + pub fn sapling_spends(&self) -> &[WalletSaplingSpend] { + self.sapling_spends.as_ref() + } + + /// Returns a record for each Sapling note belonging to and/or produced by the wallet in the + /// transaction. + pub fn sapling_outputs(&self) -> &[WalletSaplingOutput] { + self.sapling_outputs.as_ref() + } } #[derive(Debug, Clone, PartialEq, Eq)] @@ -195,41 +234,41 @@ impl WalletSaplingSpend { /// `()` for sent notes. /// /// [`OutputDescription`]: sapling::bundle::OutputDescription -pub struct WalletSaplingOutput { +pub struct WalletSaplingOutput { index: usize, cmu: sapling::note::ExtractedNoteCommitment, ephemeral_key: EphemeralKeyBytes, - account: A, + account_id: AccountId, note: sapling::Note, is_change: bool, note_commitment_tree_position: Position, - nf: N, - recipient_key_scope: S, + nf: Option, + recipient_key_scope: Option, } -impl WalletSaplingOutput { +impl WalletSaplingOutput { /// Constructs a new `WalletSaplingOutput` value from its constituent parts. #[allow(clippy::too_many_arguments)] pub fn from_parts( index: usize, cmu: sapling::note::ExtractedNoteCommitment, ephemeral_key: EphemeralKeyBytes, - account: A, + account_id: AccountId, note: sapling::Note, is_change: bool, note_commitment_tree_position: Position, - nf: N, - recipient_key_scope: S, + nf: Option, + recipient_key_scope: Option, ) -> Self { Self { index, cmu, ephemeral_key, - account, note, is_change, note_commitment_tree_position, nf, + account_id, recipient_key_scope, } } @@ -243,8 +282,8 @@ impl WalletSaplingOutput { pub fn ephemeral_key(&self) -> &EphemeralKeyBytes { &self.ephemeral_key } - pub fn account(&self) -> &A { - &self.account + pub fn account_id(&self) -> &AccountId { + &self.account_id } pub fn note(&self) -> &sapling::Note { &self.note @@ -255,11 +294,11 @@ impl WalletSaplingOutput { pub fn note_commitment_tree_position(&self) -> Position { self.note_commitment_tree_position } - pub fn nf(&self) -> &N { - &self.nf + pub fn nf(&self) -> Option<&sapling::Nullifier> { + self.nf.as_ref() } - pub fn recipient_key_scope(&self) -> &S { - &self.recipient_key_scope + pub fn recipient_key_scope(&self) -> Option { + self.recipient_key_scope } } diff --git a/zcash_client_sqlite/src/lib.rs b/zcash_client_sqlite/src/lib.rs index af9c3dbe4..918854482 100644 --- a/zcash_client_sqlite/src/lib.rs +++ b/zcash_client_sqlite/src/lib.rs @@ -454,7 +454,7 @@ impl WalletWrite for WalletDb #[allow(clippy::type_complexity)] fn put_blocks( &mut self, - blocks: Vec>, + blocks: Vec>, ) -> Result<(), Self::Error> { self.transactionally(|wdb| { let start_positions = blocks.first().map(|block| { @@ -491,18 +491,24 @@ impl WalletWrite for WalletDb let tx_row = wallet::put_tx_meta(wdb.conn.0, tx, block.height())?; // Mark notes as spent and remove them from the scanning cache - for spend in &tx.sapling_spends { + for spend in tx.sapling_spends() { wallet::sapling::mark_sapling_note_spent(wdb.conn.0, tx_row, spend.nf())?; } - for output in &tx.sapling_outputs { + for output in tx.sapling_outputs() { // Check whether this note was spent in a later block range that // we previously scanned. - let spent_in = wallet::query_nullifier_map::<_, Scope>( - wdb.conn.0, - ShieldedProtocol::Sapling, - output.nf(), - )?; + let spent_in = output + .nf() + .map(|nf| { + wallet::query_nullifier_map::<_, Scope>( + wdb.conn.0, + ShieldedProtocol::Sapling, + nf, + ) + }) + .transpose()? + .flatten(); wallet::sapling::put_received_note(wdb.conn.0, output, tx_row, spent_in)?; } @@ -517,7 +523,7 @@ impl WalletWrite for WalletDb )?; note_positions.extend(block.transactions().iter().flat_map(|wtx| { - wtx.sapling_outputs + wtx.sapling_outputs() .iter() .map(|out| out.note_commitment_tree_position()) })); diff --git a/zcash_client_sqlite/src/wallet.rs b/zcash_client_sqlite/src/wallet.rs index 7db47d201..ac6216d13 100644 --- a/zcash_client_sqlite/src/wallet.rs +++ b/zcash_client_sqlite/src/wallet.rs @@ -1605,9 +1605,9 @@ pub(crate) fn put_block( /// Inserts information about a mined transaction that was observed to /// contain a note related to this wallet into the database. -pub(crate) fn put_tx_meta( +pub(crate) fn put_tx_meta( conn: &rusqlite::Connection, - tx: &WalletTx, + tx: &WalletTx, height: BlockHeight, ) -> Result { // It isn't there, so insert our transaction into the database. @@ -1620,10 +1620,11 @@ pub(crate) fn put_tx_meta( RETURNING id_tx", )?; + let txid_bytes = tx.txid(); let tx_params = named_params![ - ":txid": &tx.txid.as_ref()[..], + ":txid": &txid_bytes.as_ref()[..], ":block": u32::from(height), - ":tx_index": i64::try_from(tx.index).expect("transaction indices are representable as i64"), + ":tx_index": i64::try_from(tx.block_index()).expect("transaction indices are representable as i64"), ]; stmt_upsert_tx_meta @@ -2031,17 +2032,7 @@ pub(crate) fn query_nullifier_map, S>( // have been created during the same scan that the locator was added to the nullifier // map, but it would not happen if the transaction in question spent the note with no // change or explicit in-wallet recipient. - put_tx_meta( - conn, - &WalletTx:: { - txid, - index, - sapling_spends: vec![], - sapling_outputs: vec![], - }, - height, - ) - .map(Some) + put_tx_meta(conn, &WalletTx::new(txid, index, vec![], vec![]), height).map(Some) } /// Deletes from the nullifier map any entries with a locator referencing a block height diff --git a/zcash_client_sqlite/src/wallet/init/migrations/receiving_key_scopes.rs b/zcash_client_sqlite/src/wallet/init/migrations/receiving_key_scopes.rs index 9836bb8d5..677da2a00 100644 --- a/zcash_client_sqlite/src/wallet/init/migrations/receiving_key_scopes.rs +++ b/zcash_client_sqlite/src/wallet/init/migrations/receiving_key_scopes.rs @@ -284,7 +284,7 @@ mod tests { }, decrypt_transaction, proto::compact_formats::{CompactBlock, CompactTx}, - scanning::scan_block, + scanning::{scan_block, ScanningKeys}, wallet::Recipient, PoolType, ShieldedProtocol, TransferType, }; @@ -439,10 +439,11 @@ mod tests { let to = output.note().recipient(); let diversifier = to.diversifier(); + let account = output.account_id(); let sql_args = named_params![ ":tx": &tx_ref, ":output_index": i64::try_from(output.index()).expect("output indices are representable as i64"), - ":account": u32::from(output.account()), + ":account": u32::from(account), ":diversifier": &diversifier.0.as_ref(), ":value": output.note().value().inner(), ":rcm": &rcm.as_ref(), @@ -599,11 +600,12 @@ mod tests { ..Default::default() }; block.vtx.push(compact_tx); + let scanning_keys = ScanningKeys::from_account_ufvks([(AccountId::ZERO, ufvk0)]); + let scanned_block = scan_block( ¶ms, block, - &[(&AccountId::ZERO, ufvk0.sapling().unwrap())], - &[], + &scanning_keys, Some(&BlockMetadata::from_parts( height - 1, prev_hash, @@ -652,13 +654,13 @@ mod tests { for tx in block.transactions() { let tx_row = crate::wallet::put_tx_meta(wdb.conn.0, tx, block.height())?; - for output in &tx.sapling_outputs { + for output in tx.sapling_outputs() { put_received_note_before_migration(wdb.conn.0, output, tx_row, None)?; } } note_positions.extend(block.transactions().iter().flat_map(|wtx| { - wtx.sapling_outputs + wtx.sapling_outputs() .iter() .map(|out| out.note_commitment_tree_position()) })); diff --git a/zcash_client_sqlite/src/wallet/sapling.rs b/zcash_client_sqlite/src/wallet/sapling.rs index 84a86d190..117e6b88c 100644 --- a/zcash_client_sqlite/src/wallet/sapling.rs +++ b/zcash_client_sqlite/src/wallet/sapling.rs @@ -29,21 +29,21 @@ use super::{memo_repr, parse_scope, scope_code, wallet_birthday}; /// This trait provides a generalization over shielded output representations. pub(crate) trait ReceivedSaplingOutput { fn index(&self) -> usize; - fn account(&self) -> AccountId; + fn account_id(&self) -> AccountId; fn note(&self) -> &sapling::Note; fn memo(&self) -> Option<&MemoBytes>; fn is_change(&self) -> bool; fn nullifier(&self) -> Option<&sapling::Nullifier>; fn note_commitment_tree_position(&self) -> Option; - fn recipient_key_scope(&self) -> Scope; + fn recipient_key_scope(&self) -> Option; } -impl ReceivedSaplingOutput for WalletSaplingOutput { +impl ReceivedSaplingOutput for WalletSaplingOutput { fn index(&self) -> usize { self.index() } - fn account(&self) -> AccountId { - *WalletSaplingOutput::account(self) + fn account_id(&self) -> AccountId { + *WalletSaplingOutput::account_id(self) } fn note(&self) -> &sapling::Note { WalletSaplingOutput::note(self) @@ -55,14 +55,14 @@ impl ReceivedSaplingOutput for WalletSaplingOutput Option<&sapling::Nullifier> { - Some(self.nf()) + self.nf() } fn note_commitment_tree_position(&self) -> Option { Some(WalletSaplingOutput::note_commitment_tree_position(self)) } - fn recipient_key_scope(&self) -> Scope { - *self.recipient_key_scope() + fn recipient_key_scope(&self) -> Option { + self.recipient_key_scope() } } @@ -70,7 +70,7 @@ impl ReceivedSaplingOutput for DecryptedOutput { fn index(&self) -> usize { self.index } - fn account(&self) -> AccountId { + fn account_id(&self) -> AccountId { self.account } fn note(&self) -> &sapling::Note { @@ -88,11 +88,11 @@ impl ReceivedSaplingOutput for DecryptedOutput { fn note_commitment_tree_position(&self) -> Option { None } - fn recipient_key_scope(&self) -> Scope { + fn recipient_key_scope(&self) -> Option { if self.transfer_type == TransferType::WalletInternal { - Scope::Internal + Some(Scope::Internal) } else { - Scope::External + Some(Scope::External) } } } @@ -439,10 +439,14 @@ pub(crate) fn put_received_note( let to = output.note().recipient(); let diversifier = to.diversifier(); + let account = output.account_id(); + let scope = output + .recipient_key_scope() + .expect("Key import is not yet supported."); let sql_args = named_params![ ":tx": &tx_ref, ":output_index": i64::try_from(output.index()).expect("output indices are representable as i64"), - ":account": u32::from(output.account()), + ":account": u32::from(account), ":diversifier": &diversifier.0.as_ref(), ":value": output.note().value().inner(), ":rcm": &rcm.as_ref(), @@ -451,7 +455,7 @@ pub(crate) fn put_received_note( ":is_change": output.is_change(), ":spent": spent_in, ":commitment_tree_position": output.note_commitment_tree_position().map(u64::from), - ":recipient_key_scope": scope_code(output.recipient_key_scope()), + ":recipient_key_scope": scope_code(scope), ]; stmt_upsert_received_note