diff --git a/zcash_client_backend/src/data_api.rs b/zcash_client_backend/src/data_api.rs index 38243417de..a23459569e 100644 --- a/zcash_client_backend/src/data_api.rs +++ b/zcash_client_backend/src/data_api.rs @@ -1943,6 +1943,15 @@ pub trait WalletRead { /// responding to a set of transaction data requests may result in the creation of new /// transaction data requests, such as when it is necessary to fill in purely-transparent /// transaction history by walking the chain backwards via transparent inputs. + /// + /// Servicing these requests is required in order for the wallet to converge to a complete + /// view of transaction history when compact block scanning omits internal-scope shielded + /// trial decryption. In particular, the wallet relies on: + /// - [`TransactionDataRequest::Enhancement`] to recover internal/change shielded outputs and + /// outgoing metadata from full transactions. + /// - [`TransactionDataRequest::TransactionsInvolvingAddress`] to discover shielding and other + /// transparent-address transactions that are not fully identifiable from compact block data + /// alone. fn transaction_data_requests(&self) -> Result, Self::Error>; /// Returns a vector of [`ReceivedTransactionOutput`] values describing the outputs of the @@ -3141,6 +3150,51 @@ pub trait WalletWrite: WalletRead { received_tx: DecryptedTransaction, ) -> Result<(), Self::Error>; + /// Notifies the wallet that the given note positions have been discovered as belonging + /// to the wallet during enhancement, so that witnesses can be maintained for them. + /// + /// This is needed because External-only batch scanning may not discover all wallet + /// outputs; those found later via enhancement need their commitment tree positions + /// registered so they are spendable. + fn notify_wallet_note_positions( + &mut self, + _block_range: std::ops::Range, + _wallet_note_positions: &[(ShieldedProtocol, incrementalmerkletree::Position)], + ) -> Result<(), Self::Error> { + Ok(()) + } + + /// Prunes backend-tracked metadata about nullifiers whose owning wallet note has + /// not yet been identified, for blocks that are at least `pruning_depth` below the + /// wallet's current fully-scanned chain tip. + /// + /// During compact-block scanning, a wallet backend may observe nullifiers for which + /// the owning note has not yet been decrypted (for example, a spend of a change note + /// whose [`Scope::Internal`] output will only be recovered via + /// [`TransactionDataRequest::Enhancement`] of the output-producing transaction). If + /// the backend retains these observations, a later enhancement step can use them to + /// link the newly-recovered note to its spending transaction; without them, such a + /// note would remain unspent in the wallet's view even after its spend has been + /// mined, producing an inflated balance. + /// + /// Callers MUST only invoke this method after the wallet's + /// [`TransactionDataRequest`] queue has been drained for the range being pruned. + /// Pruning prematurely releases the information the backend needs to attach + /// late-discovered notes to their spends. + /// + /// Because pruning is defined relative to the wallet's contiguous-from-birthday + /// fully-scanned chain tip, the effective pruning floor will not advance during + /// non-linear scan passes (for example, when a higher-priority [`ScanPriority::FoundNote`] + /// range is being scanned ahead of the historical fill). This is correct but means + /// that under non-linear scan plans, tracked nullifier metadata may retain more + /// entries than under purely linear scanning. + /// + /// [`Scope::Internal`]: zip32::Scope::Internal + /// [`ScanPriority::FoundNote`]: scanning::ScanPriority::FoundNote + fn prune_tracked_nullifiers(&mut self, _pruning_depth: u32) -> Result<(), Self::Error> { + Ok(()) + } + /// Sets the trust status of the given transaction to either trusted or untrusted. /// /// The outputs of a trusted transaction will be available for spending with diff --git a/zcash_client_backend/src/data_api/ll.rs b/zcash_client_backend/src/data_api/ll.rs index ae3f852ffd..de5e712bc6 100644 --- a/zcash_client_backend/src/data_api/ll.rs +++ b/zcash_client_backend/src/data_api/ll.rs @@ -615,7 +615,7 @@ pub trait ReceivedShieldedOutput { fn is_change(&self) -> bool; /// Returns the nullifier that will be revealed when the note is spent, if the output was /// observed using a key that provides the capability for nullifier computation. - fn nullifier(&self) -> Option<&Self::Nullifier>; + fn nullifier(&self) -> Option; /// Returns the position of the note in the note commitment tree, if the transaction that /// produced the output has been mined. fn note_commitment_tree_position(&self) -> Option; @@ -661,8 +661,8 @@ impl ReceivedShieldedOutput for WalletSaplingOutput fn is_change(&self) -> bool { WalletSaplingOutput::is_change(self) } - fn nullifier(&self) -> Option<&::sapling::Nullifier> { - self.nf() + fn nullifier(&self) -> Option<::sapling::Nullifier> { + self.nf().copied() } fn note_commitment_tree_position(&self) -> Option { Some(WalletSaplingOutput::note_commitment_tree_position(self)) @@ -696,11 +696,12 @@ impl ReceivedShieldedOutput for DecryptedOutput<::sapling::Note fn is_change(&self) -> bool { self.transfer_type() == TransferType::WalletInternal } - fn nullifier(&self) -> Option<&::sapling::Nullifier> { - None + fn nullifier(&self) -> Option<::sapling::Nullifier> { + self.nullifier_bytes() + .and_then(|bytes| ::sapling::Nullifier::from_slice(&bytes).ok()) } fn note_commitment_tree_position(&self) -> Option { - None + self.note_commitment_tree_position() } fn recipient_key_scope(&self) -> Option { if self.transfer_type() == TransferType::WalletInternal { @@ -752,8 +753,8 @@ impl ReceivedShieldedOutput for WalletOrchardOutput fn is_change(&self) -> bool { WalletOrchardOutput::is_change(self) } - fn nullifier(&self) -> Option<&::orchard::note::Nullifier> { - self.nf() + fn nullifier(&self) -> Option<::orchard::note::Nullifier> { + self.nf().copied() } fn note_commitment_tree_position(&self) -> Option { Some(WalletOrchardOutput::note_commitment_tree_position(self)) @@ -788,11 +789,12 @@ impl ReceivedShieldedOutput for DecryptedOutput<::orchard::Note fn is_change(&self) -> bool { self.transfer_type() == TransferType::WalletInternal } - fn nullifier(&self) -> Option<&::orchard::note::Nullifier> { - None + fn nullifier(&self) -> Option<::orchard::note::Nullifier> { + self.nullifier_bytes() + .and_then(|bytes| Option::from(::orchard::note::Nullifier::from_bytes(&bytes))) } fn note_commitment_tree_position(&self) -> Option { - None + self.note_commitment_tree_position() } fn recipient_key_scope(&self) -> Option { if self.transfer_type() == TransferType::WalletInternal { diff --git a/zcash_client_backend/src/data_api/ll/wallet.rs b/zcash_client_backend/src/data_api/ll/wallet.rs index fb29ec6f55..1297fbc4cf 100644 --- a/zcash_client_backend/src/data_api/ll/wallet.rs +++ b/zcash_client_backend/src/data_api/ll/wallet.rs @@ -732,7 +732,13 @@ where tx_ref, funding_account, d_tx.sapling_outputs(), - |_, _| Ok(None), + |wallet_db, output| { + Ok(output + .nullifier() + .map(|nf| wallet_db.detect_sapling_spend(&nf)) + .transpose()? + .flatten()) + }, |wallet_db, output, tx_ref, spent_in| { wallet_db.put_received_sapling_note(output, tx_ref, d_tx.mined_height(), spent_in) }, @@ -749,7 +755,13 @@ where tx_ref, funding_account, d_tx.orchard_outputs(), - |_, _| Ok(None), + |wallet_db, output| { + Ok(output + .nullifier() + .map(|nf| wallet_db.detect_orchard_spend(&nf)) + .transpose()? + .flatten()) + }, |wallet_db, output, tx_ref, spent_in| { wallet_db.put_received_orchard_note(output, tx_ref, d_tx.mined_height(), spent_in) }, diff --git a/zcash_client_backend/src/data_api/wallet.rs b/zcash_client_backend/src/data_api/wallet.rs index 242808c738..a8a91aec91 100644 --- a/zcash_client_backend/src/data_api/wallet.rs +++ b/zcash_client_backend/src/data_api/wallet.rs @@ -51,6 +51,7 @@ use crate::{ Account, MaxSpendMode, SentTransaction, SentTransactionOutput, WalletCommitmentTrees, WalletRead, WalletWrite, error::Error, wallet::input_selection::propose_send_max, }, + decrypt::compute_enriched_outputs, decrypt_transaction, fees::{ ChangeStrategy, DustOutputPolicy, StandardFeeRule, standard::SingleOutputChangeStrategy, @@ -204,6 +205,22 @@ impl PcztRecipient { /// Scans a [`Transaction`] for any information that can be decrypted by the accounts in /// the wallet, and saves it to the wallet. +/// +/// In addition to running [`decrypt_transaction`] over the input transaction, this +/// function enriches the resulting [`crate::data_api::DecryptedTransaction`] via +/// `crate::decrypt::compute_enriched_outputs` so that any change notes recovered via +/// Internal-IVK decryption have their nullifier metadata populated. Without this +/// enrichment, change notes would be stored in the wallet without nullifiers, and +/// subsequent transactions that spend those change notes would fail to mark them as +/// spent — resulting in an inflated wallet balance. +/// +/// Because this entry point does not have access to a block source, it cannot compute +/// per-output Sapling note commitment tree positions; the enrichment is therefore best-effort +/// for Sapling. Orchard nullifiers do not depend on note positions, so they ARE populated +/// regardless. Callers that require Sapling nullifier population should use the +/// [`crate::sync`] module's enhancement pipeline (which fetches block contents from +/// lightwalletd to derive positions) or pass enriched transactions directly via +/// [`WalletWrite::store_decrypted_tx`]. pub fn decrypt_and_store_transaction( params: &ParamsT, data: &mut DbT, @@ -217,13 +234,23 @@ where // Fetch the UnifiedFullViewingKeys we are tracking let ufvks = data.get_unified_full_viewing_keys()?; - data.store_decrypted_tx(decrypt_transaction( - params, - mined_height.map_or_else(|| data.get_tx_height(tx.txid()), |h| Ok(Some(h)))?, - data.chain_height()?, - tx, - &ufvks, - ))?; + let resolved_height = + mined_height.map_or_else(|| data.get_tx_height(tx.txid()), |h| Ok(Some(h)))?; + let chain_tip = data.chain_height()?; + + let d_tx = decrypt_transaction(params, resolved_height, chain_tip, tx, &ufvks); + + // Enrich the decrypted outputs with nullifier metadata. We do not have access + // to bundle base positions here (computing them requires block contents), so + // Sapling nullifiers will be left unset; Orchard nullifiers ARE populated + // because they are derived from the note + FVK alone, without dependence on + // the note's commitment tree position. This is sufficient to enable + // `mark_notes_spent` to detect spends of Orchard change notes in subsequent + // transactions, which is the primary failure mode this enrichment guards + // against. + let d_tx = compute_enriched_outputs(tx, &d_tx, None, &ufvks); + + data.store_decrypted_tx(d_tx)?; Ok(()) } diff --git a/zcash_client_backend/src/decrypt.rs b/zcash_client_backend/src/decrypt.rs index 552c62051a..89d5bfc55a 100644 --- a/zcash_client_backend/src/decrypt.rs +++ b/zcash_client_backend/src/decrypt.rs @@ -1,5 +1,6 @@ use std::collections::HashMap; +use incrementalmerkletree::Position; use sapling::note_encryption::{PreparedIncomingViewingKey, SaplingDomain}; use zcash_keys::keys::UnifiedFullViewingKey; use zcash_note_encryption::{try_note_decryption, try_output_recovery_with_ovk}; @@ -39,6 +40,8 @@ pub struct DecryptedOutput { account: AccountId, memo: MemoBytes, transfer_type: TransferType, + nullifier_bytes: Option<[u8; 32]>, + note_commitment_tree_position: Option, } impl DecryptedOutput { @@ -55,6 +58,8 @@ impl DecryptedOutput { account, memo, transfer_type, + nullifier_bytes: None, + note_commitment_tree_position: None, } } @@ -84,6 +89,39 @@ impl DecryptedOutput { pub fn transfer_type(&self) -> TransferType { self.transfer_type } + + /// Returns the serialized nullifier for the note, if known. + /// + /// This is populated during transaction enhancement when the note commitment tree + /// position is available, enabling the wallet to detect future spends of this note. + /// Returns `None` for [`TransferType::Outgoing`] outputs (which the wallet cannot spend) + /// or when the position is not yet known. + pub fn nullifier_bytes(&self) -> Option<[u8; 32]> { + self.nullifier_bytes + } + + /// Returns the position of the note in the note commitment tree, if known. + /// + /// This is populated during transaction enhancement from compact block metadata. + /// A note without a known position cannot be spent, because the wallet cannot + /// construct a Merkle path (witness) for it. + pub fn note_commitment_tree_position(&self) -> Option { + self.note_commitment_tree_position + } + + /// Attaches note commitment tree position and nullifier metadata to this output. + /// + /// This is used during the enhancement phase to enrich a [`DecryptedOutput`] with + /// the information needed to make the note spendable. + pub fn with_spend_metadata( + mut self, + note_commitment_tree_position: Option, + nullifier_bytes: Option<[u8; 32]>, + ) -> Self { + self.note_commitment_tree_position = note_commitment_tree_position; + self.nullifier_bytes = nullifier_bytes; + self + } } impl DecryptedOutput { @@ -248,3 +286,130 @@ pub fn decrypt_transaction<'a, P: consensus::Parameters, AccountId: Copy>( orchard_outputs, ) } + +/// Note commitment tree base positions for the Sapling and Orchard bundles of a single +/// transaction. +/// +/// `sapling_base` is the position of the first Sapling output in the transaction's bundle +/// within the global Sapling note commitment tree. `orchard_base` is the analogous position +/// for the first Orchard action. These bases are used by [`compute_enriched_outputs`] to +/// compute per-output positions and (for Sapling) per-output nullifiers. +/// +/// For Orchard, nullifier computation does not depend on position, so `orchard_base` is +/// only required to populate the `commitment_tree_position` field on stored notes (which +/// is needed for witness construction during spending). For Sapling, nullifier computation +/// REQUIRES the position because Sapling's nullifier hash mixes the note's tree position. +#[derive(Clone, Copy, Debug, Default)] +pub struct TxBundlePositions { + /// Position of the first Sapling output in the transaction's bundle within the global + /// Sapling note commitment tree. + pub sapling_base: Option, + /// Position of the first Orchard action in the transaction's bundle within the global + /// Orchard note commitment tree. + #[cfg(feature = "orchard")] + pub orchard_base: Option, +} + +/// Enriches the outputs of a [`DecryptedTransaction`] with note commitment tree positions +/// and nullifier bytes, returning a new `DecryptedTransaction` with the same outputs but +/// with [`DecryptedOutput::nullifier_bytes`] and +/// [`DecryptedOutput::note_commitment_tree_position`] populated where possible. +/// +/// This is used to add spend-tracking metadata to outputs that were discovered via +/// [`decrypt_transaction`] (typically during transaction enhancement, when change notes +/// recovered via Internal-IVK decryption need their nullifiers computed so that subsequent +/// spends of those change notes can be detected). +/// +/// # Arguments +/// - `tx`: The transaction whose outputs are being enriched. +/// - `d_tx`: The decrypted form of the transaction, as returned by [`decrypt_transaction`]. +/// - `positions`: Optional pre-computed bundle base positions. When `None`, Sapling outputs +/// will not get nullifiers (because Sapling nullifier computation requires position), +/// but Orchard outputs WILL get nullifiers (because Orchard nullifier computation does +/// not depend on position). When `Some`, both pools get nullifiers. +/// - `ufvks`: The wallet's full viewing keys, keyed by account identifier. +/// +/// # Returns +/// A new `DecryptedTransaction` containing the enriched outputs. +pub fn compute_enriched_outputs<'a, AccountId: Copy + std::hash::Hash + Eq>( + tx: &'a Transaction, + d_tx: &DecryptedTransaction<'a, Transaction, AccountId>, + positions: Option<&TxBundlePositions>, + ufvks: &HashMap, +) -> DecryptedTransaction<'a, Transaction, AccountId> { + let sapling_outputs = d_tx + .sapling_outputs() + .iter() + .map(|output| { + let position = positions + .and_then(|pos| pos.sapling_base) + .map(|base| Position::from(base + output.index() as u64)); + let nullifier_bytes = if output.transfer_type() == TransferType::Outgoing { + None + } else { + let scope = match output.transfer_type() { + TransferType::WalletInternal => Scope::Internal, + TransferType::Incoming => Scope::External, + TransferType::Outgoing => unreachable!(), + }; + position.and_then(|position| { + ufvks + .get(output.account()) + .and_then(|ufvk| ufvk.sapling()) + .map(|dfvk| output.note().nf(&dfvk.to_nk(scope), position.into()).0) + }) + }; + + DecryptedOutput::new( + output.index(), + output.note().clone(), + *output.account(), + output.memo().clone(), + output.transfer_type(), + ) + .with_spend_metadata(position, nullifier_bytes) + }) + .collect(); + + #[cfg(feature = "orchard")] + let orchard_outputs = d_tx + .orchard_outputs() + .iter() + .map(|output| { + let position = positions + .and_then(|pos| pos.orchard_base) + .map(|base| Position::from(base + output.index() as u64)); + // Orchard nullifier computation does not depend on the note commitment tree + // position; it can be computed from the note plus the wallet's full viewing + // key alone. This means change notes recovered via Internal-IVK enhancement + // can have their nullifiers populated even when the caller does not know the + // tx's position in the global tree (e.g., when called from + // `decrypt_and_store_transaction` which has no block-source access). + let nullifier_bytes = if output.transfer_type() == TransferType::Outgoing { + None + } else { + ufvks + .get(output.account()) + .and_then(|ufvk| ufvk.orchard()) + .map(|fvk| output.note().nullifier(fvk).to_bytes()) + }; + + DecryptedOutput::new( + output.index(), + *output.note(), + *output.account(), + output.memo().clone(), + output.transfer_type(), + ) + .with_spend_metadata(position, nullifier_bytes) + }) + .collect(); + + DecryptedTransaction::new( + d_tx.mined_height(), + tx, + sapling_outputs, + #[cfg(feature = "orchard")] + orchard_outputs, + ) +} diff --git a/zcash_client_backend/src/scanning.rs b/zcash_client_backend/src/scanning.rs index bc2d6a6131..b6a84198d5 100644 --- a/zcash_client_backend/src/scanning.rs +++ b/zcash_client_backend/src/scanning.rs @@ -271,6 +271,24 @@ impl /// along with the account identifiers corresponding to those UFVKs. pub fn from_account_ufvks( ufvks: impl IntoIterator, + ) -> Self { + Self::from_account_ufvks_with_scopes(ufvks, &[Scope::External, Scope::Internal]) + } + + /// Constructs a [`ScanningKeys`] from an iterator of [`UnifiedFullViewingKey`]s, + /// using only the specified scopes for key derivation. + /// + /// Passing `&[Scope::External]` omits internal (change) keys from trial + /// decryption, which halves the key-agreement work per output. Change notes + /// are still recoverable: when a note is discovered whose nullifier matches + /// one already in the `nullifier_map` (from a previously-scanned block), the + /// spending transaction can be queued for enhancement, and + /// [`decrypt_and_store_transaction`] will find the change via IVK Internal. + /// + /// [`decrypt_and_store_transaction`]: crate::data_api::wallet::decrypt_and_store_transaction + pub fn from_account_ufvks_with_scopes( + ufvks: impl IntoIterator, + scopes: &[Scope], ) -> Self { #![allow(clippy::type_complexity)] @@ -290,7 +308,7 @@ impl for (account_id, ufvk) in ufvks { if let Some(dfvk) = ufvk.sapling() { - for scope in [Scope::External, Scope::Internal] { + for &scope in scopes { sapling.insert( (account_id, scope), Box::new(ScanningKey { @@ -305,7 +323,7 @@ impl #[cfg(feature = "orchard")] if let Some(fvk) = ufvk.orchard() { - for scope in [Scope::External, Scope::Internal] { + for &scope in scopes { orchard.insert( (account_id, scope), Box::new(ScanningKey {