From c3b84715a2af18abf70cbb76cb3ab8100512c3d2 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/src/data_api.rs | 24 +- zcash_client_backend/src/data_api/chain.rs | 40 +- zcash_client_backend/src/scan.rs | 103 ++--- zcash_client_backend/src/scanning.rs | 374 ++++++++---------- zcash_client_backend/src/wallet.rs | 79 ++-- zcash_client_sqlite/src/lib.rs | 27 +- zcash_client_sqlite/src/wallet.rs | 21 +- .../init/migrations/receiving_key_scopes.rs | 19 +- zcash_client_sqlite/src/wallet/sapling.rs | 41 +- 9 files changed, 364 insertions(+), 364 deletions(-) diff --git a/zcash_client_backend/src/data_api.rs b/zcash_client_backend/src/data_api.rs index 16e0f757f2..ad2a56fa9d 100644 --- a/zcash_client_backend/src/data_api.rs +++ b/zcash_client_backend/src/data_api.rs @@ -18,7 +18,7 @@ use zcash_primitives::{ components::amount::{Amount, BalanceError, NonNegativeAmount}, Transaction, TxId, }, - zip32::{AccountId, Scope}, + zip32::AccountId, }; use crate::{ @@ -700,23 +700,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, @@ -750,7 +750,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 } @@ -1042,10 +1042,7 @@ 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. /// @@ -1167,7 +1164,7 @@ pub mod testing { consensus::{BlockHeight, Network}, memo::Memo, transaction::{components::Amount, Transaction, TxId}, - zip32::{AccountId, Scope}, + zip32::AccountId, }; use crate::{ @@ -1396,10 +1393,7 @@ pub mod testing { } #[allow(clippy::type_complexity)] - fn put_blocks( - &mut self, - _blocks: Vec>, - ) -> Result<(), Self::Error> { + fn put_blocks(&mut self, _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 223918c342..0f7efe7ad1 100644 --- a/zcash_client_backend/src/data_api/chain.rs +++ b/zcash_client_backend/src/data_api/chain.rs @@ -143,16 +143,15 @@ //! # } //! ``` -use std::ops::Range; +use std::{collections::HashMap, ops::Range}; -use sapling::note_encryption::PreparedIncomingViewingKey; use zcash_primitives::consensus::{self, BlockHeight}; 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, SaplingScanningKey, ScanningKey}, }; pub mod error; @@ -281,17 +280,24 @@ where .map_err(Error::Wallet)?; // TODO: Change `scan_block` to also scan Orchard. // https://github.com/zcash/librustzcash/issues/403 - let sapling_ivks: Vec<_> = ufvks + let sapling_ivks = 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::>(); + .flat_map(|(account, dfvk)| { + SaplingScanningKey::from_account_dfvk(dfvk, Some(*account)).into_iter() + }) + .map(|key| { + ( + key.account_scope() + .expect("Account ID is available for each key"), + key, + ) + }) + .collect::>(); let mut sapling_runner = BatchRunner::<_, _, _, _, ()>::new( 100, - sapling_ivks.iter().map(|(account, (scope, ivk, _))| { - ((**account, *scope), PreparedIncomingViewingKey::new(ivk)) - }), + sapling_ivks.iter().map(|(id, key)| (*id, key.prepare())), ); block_source.with_blocks::<_, DbT::Error>( @@ -342,7 +348,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; @@ -350,14 +359,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())) + tx.sapling_outputs().iter().flat_map(|out| { + out.recipient_key_meta() + .zip(out.nf()) + .into_iter() + .map(|((account_id, _), nf)| (account_id, *nf)) + }) })); prior_block_metadata = Some(scanned_block.to_block_metadata()); diff --git a/zcash_client_backend/src/scan.rs b/zcash_client_backend/src/scan.rs index 6b792f29f1..c64a9a9cbf 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: KeyId, /// 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, + KeyId: 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: &[KeyId], 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: &[KeyId], 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: &[KeyId], 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, + KeyId: DynamicUsage, D: BatchDomain + DynamicUsage, D::IncomingViewingKey: DynamicUsage, Output: DynamicUsage, @@ -325,14 +325,14 @@ where } } -impl Batch +impl Batch where - A: Clone, + KeyId: 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, + KeyId: 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, + KeyId: 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, + KeyId: 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,17 +506,17 @@ where } } -pub(crate) trait BatchResults { +pub(crate) trait BatchResults { fn collect_results( &mut self, block_tag: BlockHash, txid: TxId, - ) -> HashMap<(TxId, usize), DecryptedOutput>; + ) -> HashMap<(TxId, usize), DecryptedOutput>; } -impl BatchRunner +impl BatchRunner where - A: Clone + Send + 'static, + KeyId: Clone + Send + 'static, D: BatchDomain + Send + 'static, D::IncomingViewingKey: Clone + Send, D::Memo: Send, @@ -524,7 +524,7 @@ where D::Recipient: Send, Output: Clone + Send + 'static, Dec: Decryptor, - T: Tasks>, + T: Tasks>, { /// Batches the given outputs for trial decryption. /// @@ -564,9 +564,10 @@ where } } -impl BatchResults for BatchRunner +impl BatchResults + for BatchRunner where - A: Clone + Send + 'static, + KeyId: Clone + Send + 'static, D: BatchDomain + Send + 'static, //D::IncomingViewingKey: Clone + Send, D::Memo: Send, @@ -574,7 +575,7 @@ where D::Recipient: Send, Output: Clone + Send + 'static, Dec: Decryptor, - T: Tasks>, + T: Tasks>, { /// Collects the pending decryption results for the given transaction. /// @@ -585,7 +586,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 637ae12157..ea7aea0cbf 100644 --- a/zcash_client_backend/src/scanning.rs +++ b/zcash_client_backend/src/scanning.rs @@ -6,12 +6,11 @@ use std::fmt::{self, Debug}; 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_note_encryption::{batch, BatchDomain, Domain, ShieldedOutput}; use zcash_primitives::block::BlockHash; use zcash_primitives::consensus::{self, BlockHeight, NetworkUpgrade}; use zcash_primitives::transaction::TxId; @@ -39,139 +38,102 @@ 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 ZIP 32 [`AccountId`] and [`Scope`] for which this key was derived, if known. + fn account_scope(&self) -> Option<(AccountId, Scope)>; /// 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_scope(&self) -> Option<(AccountId, Scope)> { + (*self).account_scope() } + + fn nf(&self, note: &D::Note, note_position: Position) -> Option { + (*self).nf(note, note_position) + } +} + +pub struct SaplingScanningKey { + ivk: SaplingIvk, + nk: Option, + account_scope: Option<(AccountId, Scope)>, } -impl ScanningKey for DiversifiableFullViewingKey { - type Scope = Scope; - type IncomingViewingKey = SaplingIvk; +impl SaplingScanningKey { + pub fn from_account_dfvk( + dfvk: &sapling::zip32::DiversifiableFullViewingKey, + account: Option, + ) -> [SaplingScanningKey; 2] { + [ + SaplingScanningKey { + ivk: dfvk.to_ivk(Scope::External), + nk: Some(dfvk.to_nk(Scope::External)), + account_scope: account.map(|a| (a, Scope::External)), + }, + SaplingScanningKey { + ivk: dfvk.to_ivk(Scope::Internal), + nk: Some(dfvk.to_nk(Scope::Internal)), + account_scope: account.map(|a| (a, Scope::Internal)), + }, + ] + } +} + +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_scope(&self) -> Option<(AccountId, Scope)> { + self.account_scope } } -impl ScanningKey for (Scope, SaplingIvk, sapling::NullifierDerivingKey) { - type Scope = Scope; - type IncomingViewingKey = SaplingIvk; +impl ScanningKey for 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 nf(key: &Self::NullifierDerivingKey, note: &Self::Note, position: Position) -> Self::Nf { - note.nf(key, position.into()) + fn prepare(&self) -> sapling::note_encryption::PreparedIncomingViewingKey { + sapling::note_encryption::PreparedIncomingViewingKey::new(self) } -} -/// 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; - - fn to_ivks( - &self, - ) -> Vec<( - Self::Scope, - Self::IncomingViewingKey, - Self::NullifierDerivingKey, - )> { - vec![((), self.clone(), ())] + fn nf(&self, _note: &sapling::Note, _position: Position) -> Option { + None } - fn nf(_key: &Self::NullifierDerivingKey, _note: &Self::Note, _position: Position) -> Self::Nf {} + fn account_scope(&self) -> Option<(AccountId, Scope)> { + None + } } /// Errors that may occur in chain scanning @@ -290,14 +252,14 @@ impl fmt::Display for ScanError { /// [`WalletTx`]: crate::wallet::WalletTx pub fn scan_block< P: consensus::Parameters + Send + 'static, - SK: ScanningKey, + KeyId: Copy + std::hash::Hash + Eq + Send + 'static, >( params: &P, block: CompactBlock, - sapling_keys: &[(&AccountId, &SK)], + sapling_keys: &HashMap, sapling_nullifiers: &[(AccountId, sapling::Nullifier)], prior_block_metadata: Option<&BlockMetadata>, -) -> Result, ScanError> { +) -> Result { scan_block_with_runner::<_, _, ()>( params, block, @@ -308,20 +270,19 @@ pub fn scan_block< ) } -type TaggedBatch = - Batch<(AccountId, S), SaplingDomain, CompactOutputDescription, CompactDecryptor>; -type TaggedBatchRunner = - BatchRunner<(AccountId, 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>, + KeyId: Copy + Send + 'static, + T: Tasks>, { let block_hash = block.hash(); let block_height = block.height(); @@ -372,16 +333,16 @@ fn check_hash_continuity( #[tracing::instrument(skip_all, fields(height = block.height))] pub(crate) fn scan_block_with_runner< P: consensus::Parameters + Send + 'static, - SK: ScanningKey, - T: Tasks> + Sync, + KeyId: Copy + std::hash::Hash + Eq + Send + 'static, + T: Tasks> + Sync, >( params: &P, block: CompactBlock, - sapling_keys: &[(&AccountId, SK)], + sapling_keys: &HashMap, sapling_nullifiers: &[(AccountId, sapling::Nullifier)], prior_block_metadata: Option<&BlockMetadata>, - mut sapling_batch_runner: Option<&mut TaggedBatchRunner>, -) -> Result, ScanError> { + mut sapling_batch_runner: Option<&mut TaggedBatchRunner>, +) -> Result { if let Some(scan_error) = check_hash_continuity(&block, prior_block_metadata) { return Err(scan_error); } @@ -484,7 +445,7 @@ pub(crate) fn scan_block_with_runner< )?; 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() { @@ -528,31 +489,29 @@ pub(crate) fn scan_block_with_runner< }) .collect::>(), &mut sapling_batch_runner, - 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_scope| { WalletSaplingOutput::from_parts( output_idx, output.cmu, output.ephemeral_key.clone(), - account, note, is_change, position, nf, - scope, + account_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 += @@ -619,9 +578,9 @@ fn find_spent( for (index, spend) in spends.iter().enumerate() { let spend_nf = extract_nf(spend); - // Find the first tracked nullifier that matches this spend, and produce - // a WalletShieldedSpend if there is a match, in constant time. - let spend = nullifiers + // Find whether any tracked nullifier that matches this spend, and produce a + // WalletShieldedSpend in constant time. + let ct_spend = nullifiers .iter() .map(|&(account, nf)| CtOption::new(account, nf.ct_eq(&spend_nf))) .fold(CtOption::new(AccountId::ZERO, 0.into()), |first, next| { @@ -629,7 +588,7 @@ fn find_spent( }) .map(|account| construct_wallet_spend(index, spend_nf, account)); - if let Some(spend) = spend.into() { + if let Some(spend) = ct_spend.into() { found_spent.push(spend); } else { // This nullifier didn't match any we are currently tracking; save it in @@ -637,15 +596,19 @@ fn find_spent( unlinked_nullifiers.push(spend_nf); } } + (found_spent, unlinked_nullifiers) } +#[allow(clippy::too_many_arguments)] +#[allow(clippy::type_complexity)] fn find_received< M, D: BatchDomain, - SK: ScanningKey, + KeyId: Copy + std::hash::Hash + Eq + Send + 'static, + SK: ScanningKey, Output: ShieldedOutput, - B: BatchResults<(AccountId, SK::Scope), D, M>, + B: BatchResults, WalletOutput, NoteCommitment, >( @@ -655,21 +618,19 @@ fn find_received< txid: TxId, tx_idx: usize, commitment_tree_size: u32, - keys: &[(&AccountId, SK)], + keys: &HashMap, spent_from_accounts: &HashSet, decoded: &[(D, Output)], batch_runner: &mut Option<&mut B>, - prepare_key: impl Fn(&SK::IncomingViewingKey) -> D::IncomingViewingKey, extract_note_commitment: impl Fn(&Output) -> NoteCommitment, new_wallet_output: impl Fn( usize, &Output, - AccountId, - SK::Note, + D::Note, bool, Position, - SK::Nf, - SK::Scope, + Option, + Option<(AccountId, zip32::Scope)>, ) -> WalletOutput, ) -> ( Vec, @@ -677,56 +638,34 @@ fn find_received< ) { // Check for incoming notes while incrementing tree and witnesses let (decrypted_opts, decrypted_len) = if let Some(runner) = batch_runner.as_mut() { - 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 = runner.collect_results(block_tag, txid); let decrypted_len = decrypted.len(); ( (0..decoded.len()) .map(|i| { - decrypted.remove(&(txid, i)).map(|d_note| { - let a = d_note.ivk_tag.0; - let nk = tagged_keys.get(&d_note.ivk_tag).expect( - "The batch runner and scan_block must use the same set of IVKs.", - ); - - (d_note.note, a, d_note.ivk_tag.1, (*nk).clone()) - }) + decrypted + .remove(&(txid, i)) + .map(|d_note| (d_note.ivk_tag, d_note.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; ( - batch::try_compact_note_decryption(&ivks, &decoded[..]) + batch::try_compact_note_decryption(&ivks, decoded) .into_iter() .map(|v| { v.map(|((note, _), ivk_idx)| { decrypted_len += 1; - let (account, scope, _, nk) = &tagged_keys[ivk_idx]; - (note, *account, scope.clone(), (*nk).clone()) + (*ivk_lookup[ivk_idx], note) }) }) .collect::>(), @@ -736,11 +675,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, @@ -749,28 +690,34 @@ 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 = key + .account_scope() + .iter() + .any(|(account, _)| spent_from_accounts.contains(account)); 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_scope(), )); } @@ -782,6 +729,8 @@ fn find_received< #[cfg(test)] mod tests { + use std::collections::HashMap; + use group::{ ff::{Field, PrimeField}, GroupEncoding, @@ -790,11 +739,11 @@ 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, + Nullifier, }; use zcash_note_encryption::Domain; use zcash_primitives::{ @@ -804,6 +753,7 @@ mod tests { transaction::components::amount::NonNegativeAmount, zip32::AccountId, }; + use zip32::Scope; use crate::{ data_api::BlockMetadata, @@ -811,9 +761,10 @@ mod tests { self as compact, CompactBlock, CompactSaplingOutput, CompactSaplingSpend, CompactTx, }, scan::BatchRunner, + scanning::{SaplingScanningKey, ScanningKey}, }; - 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 = { @@ -951,13 +902,17 @@ mod tests { ); assert_eq!(cb.vtx.len(), 2); + let scanning_keys = SaplingScanningKey::from_account_dfvk(&dfvk, Some(account)) + .into_iter() + .map(|k| (k.account_scope().unwrap(), k)) + .collect::>(); + let mut batch_runner = if scan_multithreaded { let mut runner = BatchRunner::<_, _, _, _, ()>::new( 10, - dfvk.to_ivks() + scanning_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); @@ -971,7 +926,7 @@ mod tests { let scanned_block = scan_block_with_runner( &Network::TestNetwork, cb, - &[(&account, &dfvk)], + &scanning_keys, &[], Some(&BlockMetadata::from_parts( BlockHeight::from(0), @@ -987,14 +942,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_matches!(tx.sapling_outputs()[0].recipient_key_meta(), Some((a, _)) if a == 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) ); @@ -1038,13 +993,17 @@ mod tests { ); assert_eq!(cb.vtx.len(), 3); + let scanning_keys = SaplingScanningKey::from_account_dfvk(&dfvk, Some(account)) + .into_iter() + .map(|k| (k.account_scope().unwrap(), k)) + .collect::>(); + let mut batch_runner = if scan_multithreaded { let mut runner = BatchRunner::<_, _, _, _, ()>::new( 10, - dfvk.to_ivks() + scanning_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); @@ -1058,7 +1017,7 @@ mod tests { let scanned_block = scan_block_with_runner( &Network::TestNetwork, cb, - &[(&AccountId::ZERO, &dfvk)], + &scanning_keys, &[], None, batch_runner.as_mut(), @@ -1068,12 +1027,12 @@ 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(), 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_matches!(tx.sapling_outputs()[0].recipient_key_meta(), Some((a, _)) if a == AccountId::ZERO); + assert_eq!(tx.sapling_outputs()[0].note().value().inner(), 5); assert_eq!( scanned_block @@ -1114,12 +1073,11 @@ mod tests { 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[..], + &HashMap::<(AccountId, Scope), _>::new(), &[(account, nf)], None, ) @@ -1128,12 +1086,12 @@ mod tests { 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(), 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 c12143df76..8f920d08f3 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,42 +234,39 @@ 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: AccountId, note: sapling::Note, is_change: bool, note_commitment_tree_position: Position, - nf: N, - recipient_key_scope: S, + nf: Option, + recipient_key_meta: Option<(AccountId, zip32::Scope)>, } -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: AccountId, note: sapling::Note, is_change: bool, note_commitment_tree_position: Position, - nf: N, - recipient_key_scope: S, + nf: Option, + recipient_key_meta: Option<(AccountId, zip32::Scope)>, ) -> Self { Self { index, cmu, ephemeral_key, - account, note, is_change, note_commitment_tree_position, nf, - recipient_key_scope, + recipient_key_meta, } } @@ -243,9 +279,6 @@ impl WalletSaplingOutput { pub fn ephemeral_key(&self) -> &EphemeralKeyBytes { &self.ephemeral_key } - pub fn account(&self) -> AccountId { - self.account - } pub fn note(&self) -> &sapling::Note { &self.note } @@ -255,11 +288,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_meta(&self) -> Option<(AccountId, zip32::Scope)> { + self.recipient_key_meta } } diff --git a/zcash_client_sqlite/src/lib.rs b/zcash_client_sqlite/src/lib.rs index 365d9b1833..5c7b890d04 100644 --- a/zcash_client_sqlite/src/lib.rs +++ b/zcash_client_sqlite/src/lib.rs @@ -450,10 +450,7 @@ impl WalletWrite for WalletDb #[tracing::instrument(skip_all, fields(height = blocks.first().map(|b| u32::from(b.height()))))] #[allow(clippy::type_complexity)] - fn put_blocks( - &mut self, - blocks: Vec>, - ) -> Result<(), Self::Error> { + fn put_blocks(&mut self, blocks: Vec) -> Result<(), Self::Error> { self.transactionally(|wdb| { let start_positions = blocks.first().map(|block| { ( @@ -489,18 +486,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)?; } @@ -515,7 +518,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 b0b1aa03e4..5ffa815d9d 100644 --- a/zcash_client_sqlite/src/wallet.rs +++ b/zcash_client_sqlite/src/wallet.rs @@ -1606,9 +1606,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. @@ -1621,10 +1621,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 @@ -2032,17 +2033,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 9836bb8d57..1a48ba58aa 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 @@ -268,7 +268,7 @@ impl RusqliteMigration for Migration

{ #[cfg(feature = "transparent-inputs")] #[cfg(test)] mod tests { - use std::convert::Infallible; + use std::{collections::HashMap, convert::Infallible}; use incrementalmerkletree::Position; use maybe_rayon::{ @@ -284,7 +284,7 @@ mod tests { }, decrypt_transaction, proto::compact_formats::{CompactBlock, CompactTx}, - scanning::scan_block, + scanning::{scan_block, SaplingScanningKey, ScanningKey}, wallet::Recipient, PoolType, ShieldedProtocol, TransferType, }; @@ -439,10 +439,11 @@ mod tests { let to = output.note().recipient(); let diversifier = to.diversifier(); + let account = output.recipient_key_meta().expect("Keys should be available for all non-imported wallet keys.").0; 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,10 +600,16 @@ mod tests { ..Default::default() }; block.vtx.push(compact_tx); + let sapling_keys = + SaplingScanningKey::from_account_dfvk(ufvk0.sapling().unwrap(), Some(AccountId::ZERO)) + .into_iter() + .map(|k| (k.account_scope().unwrap(), k)) + .collect::>(); + let scanned_block = scan_block( ¶ms, block, - &[(&AccountId::ZERO, ufvk0.sapling().unwrap())], + &sapling_keys, &[], Some(&BlockMetadata::from_parts( height - 1, @@ -652,13 +659,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 7061e62e8a..d439eea045 100644 --- a/zcash_client_sqlite/src/wallet/sapling.rs +++ b/zcash_client_sqlite/src/wallet/sapling.rs @@ -29,22 +29,18 @@ 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 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_meta(&self) -> Option<(AccountId, Scope)>; } -impl ReceivedSaplingOutput for WalletSaplingOutput { +impl ReceivedSaplingOutput for WalletSaplingOutput { fn index(&self) -> usize { self.index() } - fn account(&self) -> AccountId { - WalletSaplingOutput::account(self) - } fn note(&self) -> &sapling::Note { WalletSaplingOutput::note(self) } @@ -55,14 +51,14 @@ impl ReceivedSaplingOutput for WalletSaplingOutput { WalletSaplingOutput::is_change(self) } fn nullifier(&self) -> 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_meta(&self) -> Option<(AccountId, Scope)> { + self.recipient_key_meta() } } @@ -70,9 +66,6 @@ impl ReceivedSaplingOutput for DecryptedOutput { fn index(&self) -> usize { self.index } - fn account(&self) -> AccountId { - self.account - } fn note(&self) -> &sapling::Note { &self.note } @@ -88,12 +81,15 @@ impl ReceivedSaplingOutput for DecryptedOutput { fn note_commitment_tree_position(&self) -> Option { None } - fn recipient_key_scope(&self) -> Scope { - if self.transfer_type == TransferType::WalletInternal { - Scope::Internal - } else { - Scope::External - } + fn recipient_key_meta(&self) -> Option<(AccountId, Scope)> { + Some(( + self.account, + if self.transfer_type == TransferType::WalletInternal { + Scope::Internal + } else { + Scope::External + }, + )) } } @@ -439,10 +435,15 @@ pub(crate) fn put_received_note( let to = output.note().recipient(); let diversifier = to.diversifier(); + // FIXME: recipient key metadata will always be available until key import is supported. + // Remove this expectation after #1175 merges. + let (account, scope) = output + .recipient_key_meta() + .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 +452,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