1/5: Enrich DecryptedOutput with nullifier and position metadata

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<Vec<TransactionDataRequest>, Self::Error>;
 
     /// Returns a vector of [`ReceivedTransactionOutput`] values describing the outputs of the
@@ -3141,6 +3150,51 @@ pub trait WalletWrite: WalletRead {
         received_tx: DecryptedTransaction<Transaction, Self::AccountId>,
     ) -> 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<BlockHeight>,
+        _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

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<Self::Nullifier>;
     /// 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<Position>;
@@ -661,8 +661,8 @@ impl<AccountId: Copy> ReceivedShieldedOutput for WalletSaplingOutput<AccountId>
     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<Position> {
         Some(WalletSaplingOutput::note_commitment_tree_position(self))
@@ -696,11 +696,12 @@ impl<AccountId: Copy> 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<Position> {
-        None
+        self.note_commitment_tree_position()
     }
     fn recipient_key_scope(&self) -> Option<Scope> {
         if self.transfer_type() == TransferType::WalletInternal {
@@ -752,8 +753,8 @@ impl<AccountId: Copy> ReceivedShieldedOutput for WalletOrchardOutput<AccountId>
     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<Position> {
         Some(WalletOrchardOutput::note_commitment_tree_position(self))
@@ -788,11 +789,12 @@ impl<AccountId: Copy> 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<Position> {
-        None
+        self.note_commitment_tree_position()
     }
     fn recipient_key_scope(&self) -> Option<Scope> {
         if self.transfer_type() == TransferType::WalletInternal {

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)
         },

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<AccountId: Copy> PcztRecipient<AccountId> {
 
 /// 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<ParamsT, DbT>(
     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(())
 }