Scan with External IVKs only; recover change notes via enhancement

zcash_client_backend/CHANGELOG.md

@@ -18,6 +18,34 @@ workspace.
   - `wallet::ConfirmationsPolicy::confirmations_until_spendable`
   - `DecryptableTransaction`
   - `ReceivedTransactionOutput`
+  - `WalletWrite::notify_wallet_note_positions` with a default no-op
+    implementation. Backends should override this to mark note commitment tree
+    positions for notes discovered during transaction enhancement, so that those
+    notes become spendable.
+  - `WalletWrite::prune_tracked_nullifiers` with a default no-op implementation.
+    Backends that record an internal nullifier-to-locator map during scanning
+    should override this to prune entries below the pruning horizon. Callers
+    must only invoke it after the `TransactionDataRequest` queue has been
+    drained for the range being pruned.
+- `zcash_client_backend::decrypt`:
+  - `DecryptedOutput::nullifier_bytes`
+  - `DecryptedOutput::note_commitment_tree_position`
+  - `DecryptedOutput::with_spend_metadata`
+  - `TxBundlePositions` struct describing the per-pool base positions of a
+    transaction's shielded bundles within the global note commitment trees.
+  - `compute_enriched_outputs` function that re-maps a `DecryptedTransaction`
+    with per-output nullifier bytes and commitment-tree positions derived from
+    a `TxBundlePositions`. Used by the enhancement path to make late-discovered
+    change notes spendable.
+  - `collect_wallet_note_positions` function (gated on `sync` or
+    `test-dependencies`) that extracts the non-outgoing note positions from a
+    `DecryptedTransaction` for passing to
+    `WalletWrite::notify_wallet_note_positions`.
+- `zcash_client_backend::scanning::ScanningKeys::from_account_ufvks_with_scopes`
+- The `sync` module now services queued transaction-data requests
+  (enhancement, status retrieval, and transparent address history) so that
+  compact scanning, transaction enhancement, and transparent history discovery
+  converge to a complete wallet view during recovery.
 - in `zcash_client_backend::proto::compact_formats`:
   - `CompactTx` has added fields `vin` and `vout`
   - Added types `CompactTxIn`, `TxOut`
@@ -42,6 +70,50 @@ workspace.
 - `zcash_client_backend::wallet::WalletTransparentOutput::with_known_input_size`
 
 ### Changed
+- `zcash_client_backend::data_api::ll::ReceivedShieldedOutput::nullifier` now
+  returns `Option<Self::Nullifier>` (by value) instead of `Option<&Self::Nullifier>`.
+- `zcash_client_backend::data_api::chain::scan_cached_blocks` now uses
+  External-scope IVKs only for batch trial decryption, halving key-agreement
+  work per output. Change notes (Internal IVK) are recovered via the
+  enhancement phase: when a note's nullifier matches one already in the
+  nullifier map, the spending transaction is queued for enhancement, where
+  `decrypt_and_store_transaction` tries all key scopes. Callers that use
+  `scan_cached_blocks` directly (outside the `sync` module) must ensure that
+  enhancement requests are serviced for change notes to be discovered.
+- `zcash_client_backend::data_api::wallet::decrypt_and_store_transaction` now
+  enriches the decrypted transaction via `decrypt::compute_enriched_outputs`
+  before handing it to `WalletWrite::store_decrypted_tx`. This populates
+  Orchard nullifier bytes on non-outgoing outputs (Orchard nullifier
+  computation does not depend on the note's commitment tree position, so it
+  succeeds even without a block source). Sapling nullifier bytes remain unset
+  when called through this entry point because computing them requires knowing
+  the bundle's base position in the global note commitment tree; callers that
+  need Sapling nullifiers should go through the `sync` module's enhancement
+  pipeline instead.
+- `zcash_client_backend::sync::run` now treats a stabilized but non-empty
+  transaction-data request queue as a terminal state once scanning is idle,
+  preventing an infinite outer-loop spin when lightwalletd cannot resolve
+  a queued txid. The internal `service_transaction_data_requests` helper
+  now returns a `ServiceOutcome` (`Drained` or `Stabilized`) so the outer
+  loop can distinguish "queue empty" from "queue stuck". Behavior is
+  unchanged when scanning still has work to do — the outer loop continues
+  and a later scan chunk may unblock previously-stuck requests.
+- `zcash_client_backend::sync` `TransactionDataRequest::TransactionsInvolvingAddress`
+  handling now always calls `notify_address_checked` after a successful
+  address-history lookup, instead of only when the server returned zero
+  transactions. This advances the per-output `max_observed_unspent_height`
+  cursor past address ranges that returned only unrelated activity, so a
+  spend-search request for a reused address no longer re-surfaces forever.
+- `zcash_client_backend::sync`: `prune_tracked_nullifiers` is now called
+  exclusively from `run()`, after the post-`running()` drain, and only when
+  that drain returns `ServiceOutcome::Drained`. The previous per-chunk and
+  per-verify-range prune calls inside `running()` violated the contract on
+  `WalletWrite::prune_tracked_nullifiers` (which requires the request queue
+  to be drained for the range being pruned): a stabilized-but-non-empty
+  drain from inside `running()` would still proceed to prune locators that
+  an unresolved enhancement retry would later need. The per-chunk
+  `service_transaction_data_requests` call inside `running()` is preserved
+  for cross-chunk cascade discovery; only the prune is hoisted out.
 - `zcash_client_backend::data_api::error::Error::MemoForbidden` has been replaced
   by `Error::Payment(zip321::PaymentError)`, which can represent both memo-to-transparent
   and zero-valued-transparent-output errors.
@@ -125,6 +197,25 @@ workspace.
 - `zcash_client_backend::data_api::testing::transparent::GapLimits` use
   `zcash_keys::keys::transparent::GapLimits` instead.
 
+### Fixed
+- `zcash_client_memory`: `TransferType::Incoming` shielded outputs handed to
+  `WalletWrite::store_decrypted_tx` are no longer misclassified as change
+  notes in the internal scope. Previously the memory backend wrapped them
+  in `Recipient::InternalAccount` and routed them through
+  `ReceivedNote::from_sent_tx_output`, which hardcoded `is_change = true`
+  and `recipient_key_scope = Some(Scope::Internal)`. Two new constructors
+  (`ReceivedNote::from_decrypted_sapling_output` and
+  `from_decrypted_orchard_output`) derive `is_change` and
+  `recipient_key_scope` from the output's `TransferType`, so external
+  incoming receipts are now stored correctly.
+- `zcash_client_memory`: `GetStatus` retrieval requests queued during
+  `store_decrypted_tx` for unmined transparent-bundle transactions are no
+  longer wiped by the end-of-function cleanup. The cleanup helper
+  `TransactionDataRequestQueue::remove_entries_for_txid` was renamed to
+  `remove_enhancement_entries_for_txid` and narrowed to only strip
+  `Enhancement` entries, leaving `GetStatus` work intact for the sync
+  orchestrator.
+
 ## [0.21.2] - 2026-03-10
 - The following APIs no longer crash in certain regtest mode configurations with
   fewer NUs active:

zcash_client_backend/Cargo.toml

@@ -264,6 +264,9 @@ test-dependencies = [
 ## Exposes APIs that allow calculation of non-standard fees.
 non-standard-fees = ["zcash_primitives/non-standard-fees"]
 
+## Enables the prospective ZIP 233 (Network Sustainability Mechanism) feature.
+zip-233 = ["zcash_primitives/zip-233"]
+
 #! ### Experimental features
 
 ## Exposes unstable APIs. Their behaviour may change at any time.

zcash_client_backend/src/data_api.rs

@@ -1911,6 +1911,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
@@ -3109,6 +3118,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/chain.rs

@@ -600,7 +600,13 @@ where
     let account_ufvks = data_db
         .get_unified_full_viewing_keys()
         .map_err(Error::Wallet)?;
-    let scanning_keys = ScanningKeys::from_account_ufvks(account_ufvks);
+    // Use External IVK only for batch trial decryption. This halves the
+    // key-agreement work per output. Change notes (Internal IVK) are recovered
+    // via the nullifier_map: when a newly discovered note's nullifier matches
+    // one from a previously-scanned block, the spending transaction is queued
+    // for enhancement, where decrypt_and_store_transaction tries all key scopes.
+    let scanning_keys =
+        ScanningKeys::from_account_ufvks_with_scopes(account_ufvks, &[zip32::Scope::External]);
     let mut runners = BatchRunners::<_, (), ()>::for_keys(100, &scanning_keys);
 
     block_source.with_blocks::<_, DbT::Error>(Some(from_height), Some(limit), |block| {

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

@@ -459,10 +459,17 @@ where
         }
     }
 
-    // Prune the nullifier map of entries we no longer need.
-    wallet_db
-        .prune_tracked_nullifiers(PRUNING_DEPTH)
-        .map_err(PutBlocksError::Storage)?;
+    // NOTE: Nullifier-map pruning is intentionally NOT performed here. Under the
+    // External-only batch scanning optimization, Internal-scope change notes are
+    // discovered only during post-scan transaction enhancement, and
+    // `detect_*_spend` consults the nullifier map to link those late-discovered
+    // notes to their spending transactions. Pruning here — before enhancement
+    // has a chance to run — would cascade-delete those locator entries (see
+    // ON DELETE CASCADE on the `nullifier_map` foreign key in `db.rs`) and cause
+    // the wallet to report an inflated balance. The sync orchestrator
+    // (`zcash_client_backend::sync::run` via `WalletWrite::prune_tracked_nullifiers`)
+    // is now responsible for invoking pruning after it has drained the
+    // transaction-data request queue for the scanned range.
 
     // We will have a start position and a last scanned height in all cases where
     // `blocks` is non-empty.
@@ -732,7 +739,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 +762,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)
         },