Merge pull request #1182 from nuttycom/sqlite_wallet/orchard_support

`zcash_client_sqlite`: Add Orchard wallet support

zcash_client_backend/CHANGELOG.md

@@ -76,6 +76,8 @@ and this library adheres to Rust's notion of
     constraint on its `<AccountId>` parameter has been strengthened to `Copy`.
 - `zcash_client_backend::fees`:
   - Arguments to `ChangeStrategy::compute_balance` have changed.
+  - `ChangeError::DustInputs` now has an `orchard` field behind the `orchard`
+    feature flag.
 - `zcash_client_backend::proto`:
   - `ProposalDecodingError` has a new variant `TransparentMemo`.
 - `zcash_client_backend::zip321::render::amount_str` now takes a

zcash_client_backend/src/data_api/wallet/input_selection.rs

@@ -445,8 +445,15 @@ where
                     )
                     .map_err(InputSelectorError::Proposal);
                 }
-                Err(ChangeError::DustInputs { mut sapling, .. }) => {
+                Err(ChangeError::DustInputs {
+                    mut sapling,
+                    #[cfg(feature = "orchard")]
+                    mut orchard,
+                    ..
+                }) => {
                     exclude.append(&mut sapling);
+                    #[cfg(feature = "orchard")]
+                    exclude.append(&mut orchard);
                 }
                 Err(ChangeError::InsufficientFunds { required, .. }) => {
                     amount_required = required;

zcash_client_backend/src/fees.rs

@@ -149,6 +149,9 @@ pub enum ChangeError<E, NoteRefT> {
         transparent: Vec<OutPoint>,
         /// The identifiers for Sapling inputs having no current economic value
         sapling: Vec<NoteRefT>,
+        /// The identifiers for Orchard inputs having no current economic value
+        #[cfg(feature = "orchard")]
+        orchard: Vec<NoteRefT>,
     },
     /// An error occurred that was specific to the change selection strategy in use.
     StrategyError(E),
@@ -169,9 +172,13 @@ impl<E, NoteRefT> ChangeError<E, NoteRefT> {
             ChangeError::DustInputs {
                 transparent,
                 sapling,
+                #[cfg(feature = "orchard")]
+                orchard,
             } => ChangeError::DustInputs {
                 transparent,
                 sapling,
+                #[cfg(feature = "orchard")]
+                orchard,
             },
             ChangeError::StrategyError(e) => ChangeError::StrategyError(f(e)),
             ChangeError::BundleError(e) => ChangeError::BundleError(e),
@@ -194,10 +201,21 @@ impl<CE: fmt::Display, N: fmt::Display> fmt::Display for ChangeError<CE, N> {
             ChangeError::DustInputs {
                 transparent,
                 sapling,
+                #[cfg(feature = "orchard")]
+                orchard,
             } => {
+                #[cfg(feature = "orchard")]
+                let orchard_len = orchard.len();
+                #[cfg(not(feature = "orchard"))]
+                let orchard_len = 0;
+
                 // we can't encode the UA to its string representation because we
                 // don't have network parameters here
-                write!(f, "Insufficient funds: {} dust inputs were present, but would cost more to spend than they are worth.", transparent.len() + sapling.len())
+                write!(
+                    f,
+                    "Insufficient funds: {} dust inputs were present, but would cost more to spend than they are worth.",
+                    transparent.len() + sapling.len() + orchard_len,
+                )
             }
             ChangeError::StrategyError(err) => {
                 write!(f, "{}", err)

zcash_client_backend/src/fees/common.rs

@@ -17,30 +17,26 @@ use super::{
 #[cfg(feature = "orchard")]
 use super::orchard as orchard_fees;
 
+pub(crate) struct NetFlows {
+    t_in: NonNegativeAmount,
+    t_out: NonNegativeAmount,
+    sapling_in: NonNegativeAmount,
+    sapling_out: NonNegativeAmount,
+    orchard_in: NonNegativeAmount,
+    orchard_out: NonNegativeAmount,
+}
+
 #[allow(clippy::too_many_arguments)]
-pub(crate) fn single_change_output_balance<
+pub(crate) fn calculate_net_flows<NoteRefT: Clone, F: FeeRule, E>(
-    P: consensus::Parameters,
-    NoteRefT: Clone,
-    F: FeeRule,
-    E,
->(
-    params: &P,
-    fee_rule: &F,
-    target_height: BlockHeight,
     transparent_inputs: &[impl transparent::InputView],
     transparent_outputs: &[impl transparent::OutputView],
     sapling: &impl sapling_fees::BundleView<NoteRefT>,
     #[cfg(feature = "orchard")] orchard: &impl orchard_fees::BundleView<NoteRefT>,
-    dust_output_policy: &DustOutputPolicy,
+) -> Result<NetFlows, ChangeError<E, NoteRefT>>
-    default_dust_threshold: NonNegativeAmount,
-    change_memo: Option<MemoBytes>,
-    _fallback_change_pool: ShieldedProtocol,
-) -> Result<TransactionBalance, ChangeError<E, NoteRefT>>
 where
     E: From<F::Error> + From<BalanceError>,
 {
     let overflow = || ChangeError::StrategyError(E::from(BalanceError::Overflow));
-    let underflow = || ChangeError::StrategyError(E::from(BalanceError::Underflow));
 
     let t_in = transparent_inputs
         .iter()
@@ -85,13 +81,30 @@ where
     #[cfg(not(feature = "orchard"))]
     let orchard_out = NonNegativeAmount::ZERO;
 
+    Ok(NetFlows {
+        t_in,
+        t_out,
+        sapling_in,
+        sapling_out,
+        orchard_in,
+        orchard_out,
+    })
+}
+
+pub(crate) fn single_change_output_policy<NoteRefT: Clone, F: FeeRule, E>(
+    _net_flows: &NetFlows,
+    _fallback_change_pool: ShieldedProtocol,
+) -> Result<(ShieldedProtocol, usize, usize), ChangeError<E, NoteRefT>>
+where
+    E: From<F::Error> + From<BalanceError>,
+{
     // TODO: implement a less naive strategy for selecting the pool to which change will be sent.
     #[cfg(feature = "orchard")]
     let (change_pool, sapling_change, orchard_change) =
-        if orchard_in.is_positive() || orchard_out.is_positive() {
+        if _net_flows.orchard_in.is_positive() || _net_flows.orchard_out.is_positive() {
             // Send change to Orchard if we're spending any Orchard inputs or creating any Orchard outputs
             (ShieldedProtocol::Orchard, 0, 1)
-        } else if sapling_in.is_positive() || sapling_out.is_positive() {
+        } else if _net_flows.sapling_in.is_positive() || _net_flows.sapling_out.is_positive() {
             // Otherwise, send change to Sapling if we're spending any Sapling inputs or creating any
             // Sapling outputs, so that we avoid pool-crossing.
             (ShieldedProtocol::Sapling, 1, 0)
@@ -104,18 +117,68 @@ where
             }
         };
     #[cfg(not(feature = "orchard"))]
-    let (change_pool, sapling_change) = (ShieldedProtocol::Sapling, 1);
+    let (change_pool, sapling_change, orchard_change) = (ShieldedProtocol::Sapling, 1, 0);
+
+    Ok((change_pool, sapling_change, orchard_change))
+}
+
+#[allow(clippy::too_many_arguments)]
+pub(crate) fn single_change_output_balance<
+    P: consensus::Parameters,
+    NoteRefT: Clone,
+    F: FeeRule,
+    E,
+>(
+    params: &P,
+    fee_rule: &F,
+    target_height: BlockHeight,
+    transparent_inputs: &[impl transparent::InputView],
+    transparent_outputs: &[impl transparent::OutputView],
+    sapling: &impl sapling_fees::BundleView<NoteRefT>,
+    #[cfg(feature = "orchard")] orchard: &impl orchard_fees::BundleView<NoteRefT>,
+    dust_output_policy: &DustOutputPolicy,
+    default_dust_threshold: NonNegativeAmount,
+    change_memo: Option<MemoBytes>,
+    _fallback_change_pool: ShieldedProtocol,
+) -> Result<TransactionBalance, ChangeError<E, NoteRefT>>
+where
+    E: From<F::Error> + From<BalanceError>,
+{
+    let overflow = || ChangeError::StrategyError(E::from(BalanceError::Overflow));
+    let underflow = || ChangeError::StrategyError(E::from(BalanceError::Underflow));
+
+    let net_flows = calculate_net_flows::<NoteRefT, F, E>(
+        transparent_inputs,
+        transparent_outputs,
+        sapling,
+        #[cfg(feature = "orchard")]
+        orchard,
+    )?;
+    let (change_pool, sapling_change, _orchard_change) =
+        single_change_output_policy::<NoteRefT, F, E>(&net_flows, _fallback_change_pool)?;
+
+    let sapling_input_count = sapling
+        .bundle_type()
+        .num_spends(sapling.inputs().len())
+        .map_err(ChangeError::BundleError)?;
+    let sapling_output_count = sapling
+        .bundle_type()
+        .num_outputs(
+            sapling.inputs().len(),
+            sapling.outputs().len() + sapling_change,
+        )
+        .map_err(ChangeError::BundleError)?;
 
     #[cfg(feature = "orchard")]
-    let orchard_num_actions = orchard
+    let orchard_action_count = orchard
         .bundle_type()
         .num_actions(
             orchard.inputs().len(),
-            orchard.outputs().len() + orchard_change,
+            orchard.outputs().len() + _orchard_change,
         )
         .map_err(ChangeError::BundleError)?;
     #[cfg(not(feature = "orchard"))]
-    let orchard_num_actions = 0;
+    let orchard_action_count = 0;
 
     let fee_amount = fee_rule
         .fee_required(
@@ -123,23 +186,16 @@ where
             target_height,
             transparent_inputs,
             transparent_outputs,
-            sapling
+            sapling_input_count,
-                .bundle_type()
+            sapling_output_count,
-                .num_spends(sapling.inputs().len())
+            orchard_action_count,
-                .map_err(ChangeError::BundleError)?,
-            sapling
-                .bundle_type()
-                .num_outputs(
-                    sapling.inputs().len(),
-                    sapling.outputs().len() + sapling_change,
-                )
-                .map_err(ChangeError::BundleError)?,
-            orchard_num_actions,
         )
         .map_err(|fee_error| ChangeError::StrategyError(E::from(fee_error)))?;
 
-    let total_in = (t_in + sapling_in + orchard_in).ok_or_else(overflow)?;
+    let total_in =
-    let total_out = (t_out + sapling_out + orchard_out + fee_amount).ok_or_else(overflow)?;
+        (net_flows.t_in + net_flows.sapling_in + net_flows.orchard_in).ok_or_else(overflow)?;
+    let total_out = (net_flows.t_out + net_flows.sapling_out + net_flows.orchard_out + fee_amount)
+        .ok_or_else(overflow)?;
 
     let proposed_change = (total_in - total_out).ok_or(ChangeError::InsufficientFunds {
         available: total_in,

zcash_client_backend/src/fees/zip317.rs

@@ -16,8 +16,8 @@ use zcash_primitives::{
 use crate::ShieldedProtocol;
 
 use super::{
-    common::single_change_output_balance, sapling as sapling_fees, ChangeError, ChangeStrategy,
+    common::{calculate_net_flows, single_change_output_balance, single_change_output_policy},
-    DustOutputPolicy, TransactionBalance,
+    sapling as sapling_fees, ChangeError, ChangeStrategy, DustOutputPolicy, TransactionBalance,
 };
 
 #[cfg(feature = "orchard")]
@@ -93,32 +93,73 @@ impl ChangeStrategy for SingleOutputChangeStrategy {
             })
             .collect();
 
+        #[cfg(feature = "orchard")]
+        let mut orchard_dust: Vec<NoteRefT> = orchard
+            .inputs()
+            .iter()
+            .filter_map(|i| {
+                if orchard_fees::InputView::<NoteRefT>::value(i) < self.fee_rule.marginal_fee() {
+                    Some(orchard_fees::InputView::<NoteRefT>::note_id(i).clone())
+                } else {
+                    None
+                }
+            })
+            .collect();
+        #[cfg(not(feature = "orchard"))]
+        let mut orchard_dust: Vec<NoteRefT> = vec![];
+
         // Depending on the shape of the transaction, we may be able to spend up to
         // `grace_actions - 1` dust inputs. If we don't have any dust inputs though,
         // we don't need to worry about any of that.
-        if !(transparent_dust.is_empty() && sapling_dust.is_empty()) {
+        if !(transparent_dust.is_empty() && sapling_dust.is_empty() && orchard_dust.is_empty()) {
             let t_non_dust = transparent_inputs.len() - transparent_dust.len();
             let t_allowed_dust = transparent_outputs.len().saturating_sub(t_non_dust);
 
-            // We add one to the sapling outputs for the (single) change output Note that this
+            // We add one to either the Sapling or Orchard outputs for the (single)
-            // means that wallet-internal shielding transactions are an opportunity to spend a dust
+            // change output. Note that this means that wallet-internal shielding
-            // note.
+            // transactions are an opportunity to spend a dust note.
+            let net_flows = calculate_net_flows::<NoteRefT, Self::FeeRule, Self::Error>(
+                transparent_inputs,
+                transparent_outputs,
+                sapling,
+                #[cfg(feature = "orchard")]
+                orchard,
+            )?;
+            let (_, sapling_change, orchard_change) =
+                single_change_output_policy::<NoteRefT, Self::FeeRule, Self::Error>(
+                    &net_flows,
+                    self.fallback_change_pool,
+                )?;
+
             let s_non_dust = sapling.inputs().len() - sapling_dust.len();
-            let s_allowed_dust = (sapling.outputs().len() + 1).saturating_sub(s_non_dust);
+            let s_allowed_dust =
+                (sapling.outputs().len() + sapling_change).saturating_sub(s_non_dust);
+
+            #[cfg(feature = "orchard")]
+            let (orchard_inputs_len, orchard_outputs_len) =
+                (orchard.inputs().len(), orchard.outputs().len());
+            #[cfg(not(feature = "orchard"))]
+            let (orchard_inputs_len, orchard_outputs_len) = (0, 0);
+
+            let o_non_dust = orchard_inputs_len - orchard_dust.len();
+            let o_allowed_dust = (orchard_outputs_len + orchard_change).saturating_sub(o_non_dust);
 
             let available_grace_inputs = self
                 .fee_rule
                 .grace_actions()
                 .saturating_sub(t_non_dust)
-                .saturating_sub(s_non_dust);
+                .saturating_sub(s_non_dust)
+                .saturating_sub(o_non_dust);
 
             let mut t_disallowed_dust = transparent_dust.len().saturating_sub(t_allowed_dust);
             let mut s_disallowed_dust = sapling_dust.len().saturating_sub(s_allowed_dust);
+            let mut o_disallowed_dust = orchard_dust.len().saturating_sub(o_allowed_dust);
 
             if available_grace_inputs > 0 {
                 // If we have available grace inputs, allocate them first to transparent dust
-                // and then to Sapling dust. The caller has provided inputs that it is willing
+                // and then to Sapling dust followed by Orchard dust. The caller has provided
-                // to spend, so we don't need to consider privacy effects at this layer.
+                // inputs that it is willing to spend, so we don't need to consider privacy
+                // effects at this layer.
                 let t_grace_dust = available_grace_inputs.saturating_sub(t_disallowed_dust);
                 t_disallowed_dust = t_disallowed_dust.saturating_sub(t_grace_dust);
 
@@ -126,6 +167,12 @@ impl ChangeStrategy for SingleOutputChangeStrategy {
                     .saturating_sub(t_grace_dust)
                     .saturating_sub(s_disallowed_dust);
                 s_disallowed_dust = s_disallowed_dust.saturating_sub(s_grace_dust);
+
+                let o_grace_dust = available_grace_inputs
+                    .saturating_sub(t_grace_dust)
+                    .saturating_sub(s_grace_dust)
+                    .saturating_sub(o_disallowed_dust);
+                o_disallowed_dust = o_disallowed_dust.saturating_sub(o_grace_dust);
             }
 
             // Truncate the lists of inputs to be disregarded in input selection to just the
@@ -135,11 +182,16 @@ impl ChangeStrategy for SingleOutputChangeStrategy {
             transparent_dust.truncate(t_disallowed_dust);
             sapling_dust.reverse();
             sapling_dust.truncate(s_disallowed_dust);
+            orchard_dust.reverse();
+            orchard_dust.truncate(o_disallowed_dust);
 
-            if !(transparent_dust.is_empty() && sapling_dust.is_empty()) {
+            if !(transparent_dust.is_empty() && sapling_dust.is_empty() && orchard_dust.is_empty())
+            {
                 return Err(ChangeError::DustInputs {
                     transparent: transparent_dust,
                     sapling: sapling_dust,
+                    #[cfg(feature = "orchard")]
+                    orchard: orchard_dust,
                 });
             }
         }

zcash_client_sqlite/src/lib.rs

@@ -69,13 +69,13 @@ use zcash_client_backend::{
     },
     proto::compact_formats::CompactBlock,
     wallet::{Note, NoteId, ReceivedNote, Recipient, WalletTransparentOutput},
-    DecryptedOutput, ShieldedProtocol, TransferType,
+    DecryptedOutput, PoolType, ShieldedProtocol, TransferType,
 };
 
 use crate::{error::SqliteClientError, wallet::commitment_tree::SqliteShardStore};
 
 #[cfg(feature = "orchard")]
-use zcash_client_backend::{data_api::ORCHARD_SHARD_HEIGHT, PoolType};
+use zcash_client_backend::data_api::ORCHARD_SHARD_HEIGHT;
 
 #[cfg(feature = "transparent-inputs")]
 use {
@@ -111,6 +111,7 @@ pub(crate) const PRUNING_DEPTH: u32 = 100;
 pub(crate) const VERIFY_LOOKAHEAD: u32 = 10;
 
 pub(crate) const SAPLING_TABLES_PREFIX: &str = "sapling";
+
 #[cfg(feature = "orchard")]
 pub(crate) const ORCHARD_TABLES_PREFIX: &str = "orchard";
 
@@ -208,7 +209,20 @@ impl<C: Borrow<rusqlite::Connection>, P: consensus::Parameters> InputSource for
                 txid,
                 index,
             ),
-            ShieldedProtocol::Orchard => Ok(None),
+            ShieldedProtocol::Orchard => {
+                #[cfg(feature = "orchard")]
+                return wallet::orchard::get_spendable_orchard_note(
+                    self.conn.borrow(),
+                    &self.params,
+                    txid,
+                    index,
+                );
+
+                #[cfg(not(feature = "orchard"))]
+                return Err(SqliteClientError::UnsupportedPoolType(PoolType::Shielded(
+                    ShieldedProtocol::Orchard,
+                )));
+            }
         }
     }
 
@@ -220,14 +234,25 @@ impl<C: Borrow<rusqlite::Connection>, P: consensus::Parameters> InputSource for
         anchor_height: BlockHeight,
         exclude: &[Self::NoteRef],
     ) -> Result<Vec<ReceivedNote<Self::NoteRef, Note>>, Self::Error> {
-        wallet::sapling::select_spendable_sapling_notes(
+        let received_iter = std::iter::empty();
+        let received_iter = received_iter.chain(wallet::sapling::select_spendable_sapling_notes(
             self.conn.borrow(),
             &self.params,
             account,
             target_value,
             anchor_height,
             exclude,
-        )
+        )?);
+        #[cfg(feature = "orchard")]
+        let received_iter = received_iter.chain(wallet::orchard::select_spendable_orchard_notes(
+            self.conn.borrow(),
+            &self.params,
+            account,
+            target_value,
+            anchor_height,
+            exclude,
+        )?);
+        Ok(received_iter.collect())
     }
 
     #[cfg(feature = "transparent-inputs")]
@@ -719,7 +744,7 @@ impl<P: consensus::Parameters> WalletWrite for WalletDb<rusqlite::Connection, P>
                         .map(|res| (res.subtree, res.checkpoints))
                         .collect::<Vec<_>>();
 
-                    // Update the Sapling note commitment tree with all newly read note commitments
+                    // Update the Orchard note commitment tree with all newly read note commitments
                     let mut orchard_subtrees = orchard_subtrees.into_iter();
                     wdb.with_orchard_tree_mut::<_, _, Self::Error>(move |orchard_tree| {
                         for (tree, checkpoints) in &mut orchard_subtrees {
@@ -935,6 +960,19 @@ impl<P: consensus::Parameters> WalletWrite for WalletDb<rusqlite::Connection, P>
                     )?;
                 }
             }
+            if let Some(_bundle) = sent_tx.tx().orchard_bundle() {
+                #[cfg(feature = "orchard")]
+                for action in _bundle.actions() {
+                    wallet::orchard::mark_orchard_note_spent(
+                        wdb.conn.0,
+                        tx_ref,
+                        action.nullifier(),
+                    )?;
+                }
+
+                #[cfg(not(feature = "orchard"))]
+                panic!("Sent a transaction with Orchard Actions without `orchard` enabled?");
+            }
 
             #[cfg(feature = "transparent-inputs")]
             for utxo_outpoint in sent_tx.utxos_spent() {

zcash_client_sqlite/src/wallet.rs

@@ -129,6 +129,7 @@ use {
 };
 
 pub mod commitment_tree;
+pub(crate) mod common;
 pub mod init;
 #[cfg(feature = "orchard")]
 pub(crate) mod orchard;
@@ -723,6 +724,15 @@ pub(crate) trait ScanProgress {
         fully_scanned_height: BlockHeight,
         chain_tip_height: BlockHeight,
     ) -> Result<Option<Ratio<u64>>, SqliteClientError>;
+
+    #[cfg(feature = "orchard")]
+    fn orchard_scan_progress(
+        &self,
+        conn: &rusqlite::Connection,
+        birthday_height: BlockHeight,
+        fully_scanned_height: BlockHeight,
+        chain_tip_height: BlockHeight,
+    ) -> Result<Option<Ratio<u64>>, SqliteClientError>;
 }
 
 #[derive(Debug)]
@@ -804,6 +814,83 @@ impl ScanProgress for SubtreeScanProgress {
                 .flatten())
         }
     }
+
+    #[cfg(feature = "orchard")]
+    #[tracing::instrument(skip(conn))]
+    fn orchard_scan_progress(
+        &self,
+        conn: &rusqlite::Connection,
+        birthday_height: BlockHeight,
+        fully_scanned_height: BlockHeight,
+        chain_tip_height: BlockHeight,
+    ) -> Result<Option<Ratio<u64>>, SqliteClientError> {
+        if fully_scanned_height == chain_tip_height {
+            // Compute the total blocks scanned since the wallet birthday
+            conn.query_row(
+                "SELECT SUM(orchard_action_count)
+                 FROM blocks
+                 WHERE height >= :birthday_height",
+                named_params![":birthday_height": u32::from(birthday_height)],
+                |row| {
+                    let scanned = row.get::<_, Option<u64>>(0)?;
+                    Ok(scanned.map(|n| Ratio::new(n, n)))
+                },
+            )
+            .map_err(SqliteClientError::from)
+        } else {
+            let start_height = birthday_height;
+            // Compute the starting number of notes directly from the blocks table
+            let start_size = conn.query_row(
+                "SELECT MAX(orchard_commitment_tree_size)
+                 FROM blocks
+                 WHERE height <= :start_height",
+                named_params![":start_height": u32::from(start_height)],
+                |row| row.get::<_, Option<u64>>(0),
+            )?;
+
+            // Compute the total blocks scanned so far above the starting height
+            let scanned_count = conn.query_row(
+                "SELECT SUM(orchard_action_count)
+                 FROM blocks
+                 WHERE height > :start_height",
+                named_params![":start_height": u32::from(start_height)],
+                |row| row.get::<_, Option<u64>>(0),
+            )?;
+
+            // We don't have complete information on how many actions will exist in the shard at
+            // the chain tip without having scanned the chain tip block, so we overestimate by
+            // computing the maximum possible number of notes directly from the shard indices.
+            //
+            // TODO: it would be nice to be able to reliably have the size of the commitment tree
+            // at the chain tip without having to have scanned that block.
+            Ok(conn
+                .query_row(
+                    "SELECT MIN(shard_index), MAX(shard_index)
+                     FROM orchard_tree_shards
+                     WHERE subtree_end_height > :start_height
+                     OR subtree_end_height IS NULL",
+                    named_params![":start_height": u32::from(start_height)],
+                    |row| {
+                        let min_tree_size = row
+                            .get::<_, Option<u64>>(0)?
+                            .map(|min| min << ORCHARD_SHARD_HEIGHT);
+                        let max_idx = row.get::<_, Option<u64>>(1)?;
+                        Ok(start_size
+                            .or(min_tree_size)
+                            .zip(max_idx)
+                            .map(|(min_tree_size, max)| {
+                                let max_tree_size = (max + 1) << ORCHARD_SHARD_HEIGHT;
+                                Ratio::new(
+                                    scanned_count.unwrap_or(0),
+                                    max_tree_size - min_tree_size,
+                                )
+                            }))
+                    },
+                )
+                .optional()?
+                .flatten())
+        }
+    }
 }
 
 /// Returns the spendable balance for the account at the specified height.
@@ -841,27 +928,27 @@ pub(crate) fn get_wallet_summary<P: consensus::Parameters>(
         chain_tip_height,
     )?;
 
-    // If the shard containing the summary height contains any unscanned ranges that start below or
+    #[cfg(feature = "orchard")]
-    // including that height, none of our balance is currently spendable.
+    let orchard_scan_progress = progress.orchard_scan_progress(
-    #[tracing::instrument(skip_all)]
+        tx,
-    fn is_any_spendable(
+        birthday_height,
-        conn: &rusqlite::Connection,
+        fully_scanned_height,
-        summary_height: BlockHeight,
+        chain_tip_height,
-    ) -> Result<bool, SqliteClientError> {
+    )?;
-        conn.query_row(
+    #[cfg(not(feature = "orchard"))]
-            "SELECT NOT EXISTS(
+    let orchard_scan_progress: Option<Ratio<u64>> = None;
-                 SELECT 1 FROM v_sapling_shard_unscanned_ranges
+
-                 WHERE :summary_height
+    // Treat Sapling and Orchard outputs as having the same cost to scan.
-                    BETWEEN subtree_start_height
+    let scan_progress = sapling_scan_progress
-                    AND IFNULL(subtree_end_height, :summary_height)
+        .zip(orchard_scan_progress)
-                 AND block_range_start <= :summary_height
+        .map(|(s, o)| {
-             )",
+            Ratio::new(
-            named_params![":summary_height": u32::from(summary_height)],
+                s.numerator() + o.numerator(),
-            |row| row.get::<_, bool>(0),
+                s.denominator() + o.denominator(),
             )
-        .map_err(|e| e.into())
+        })
-    }
+        .or(sapling_scan_progress)
-    let any_spendable = is_any_spendable(tx, summary_height)?;
+        .or(orchard_scan_progress);
 
     let mut stmt_accounts = tx.prepare_cached("SELECT id FROM accounts")?;
     let mut account_balances = stmt_accounts
@@ -871,12 +958,51 @@ pub(crate) fn get_wallet_summary<P: consensus::Parameters>(
         })
         .collect::<Result<HashMap<AccountId, AccountBalance>, _>>()?;
 
-    let sapling_trace = tracing::info_span!("stmt_select_notes").entered();
+    fn count_notes<F>(
-    let mut stmt_select_notes = tx.prepare_cached(
+        tx: &rusqlite::Transaction,
+        summary_height: BlockHeight,
+        account_balances: &mut HashMap<AccountId, AccountBalance>,
+        table_prefix: &'static str,
+        with_pool_balance: F,
+    ) -> Result<(), SqliteClientError>
+    where
+        F: Fn(
+            &mut AccountBalance,
+            NonNegativeAmount,
+            NonNegativeAmount,
+            NonNegativeAmount,
+        ) -> Result<(), SqliteClientError>,
+    {
+        // If the shard containing the summary height contains any unscanned ranges that start below or
+        // including that height, none of our balance is currently spendable.
+        #[tracing::instrument(skip_all)]
+        fn is_any_spendable(
+            conn: &rusqlite::Connection,
+            summary_height: BlockHeight,
+            table_prefix: &'static str,
+        ) -> Result<bool, SqliteClientError> {
+            conn.query_row(
+                &format!(
+                    "SELECT NOT EXISTS(
+                         SELECT 1 FROM v_{table_prefix}_shard_unscanned_ranges
+                         WHERE :summary_height
+                            BETWEEN subtree_start_height
+                            AND IFNULL(subtree_end_height, :summary_height)
+                         AND block_range_start <= :summary_height
+                     )"
+                ),
+                named_params![":summary_height": u32::from(summary_height)],
+                |row| row.get::<_, bool>(0),
+            )
+            .map_err(|e| e.into())
+        }
+
+        let any_spendable = is_any_spendable(tx, summary_height, table_prefix)?;
+        let mut stmt_select_notes = tx.prepare_cached(&format!(
             "SELECT n.account_id, n.value, n.is_change, scan_state.max_priority, t.block
-         FROM sapling_received_notes n
+             FROM {table_prefix}_received_notes n
              JOIN transactions t ON t.id_tx = n.tx
-         LEFT OUTER JOIN v_sapling_shards_scan_state scan_state
+             LEFT OUTER JOIN v_{table_prefix}_shards_scan_state scan_state
                 ON n.commitment_tree_position >= scan_state.start_position
                 AND n.commitment_tree_position < scan_state.end_position_exclusive
              WHERE n.spent IS NULL
@@ -885,7 +1011,7 @@ pub(crate) fn get_wallet_summary<P: consensus::Parameters>(
                  OR t.block IS NOT NULL
                  OR t.expiry_height >= :summary_height
              )",
-    )?;
+        ))?;
 
         let mut rows =
             stmt_select_notes.query(named_params![":summary_height": u32::from(summary_height)])?;
@@ -894,7 +1020,10 @@ pub(crate) fn get_wallet_summary<P: consensus::Parameters>(
 
             let value_raw = row.get::<_, i64>(1)?;
             let value = NonNegativeAmount::from_nonnegative_i64(value_raw).map_err(|_| {
-            SqliteClientError::CorruptedData(format!("Negative received note value: {}", value_raw))
+                SqliteClientError::CorruptedData(format!(
+                    "Negative received note value: {}",
+                    value_raw
+                ))
             })?;
 
             let is_change = row.get::<_, bool>(2)?;
@@ -921,7 +1050,8 @@ pub(crate) fn get_wallet_summary<P: consensus::Parameters>(
                 && received_height.iter().any(|h| h <= &summary_height)
                 && max_priority <= ScanPriority::Scanned;
 
-        let is_pending_change = is_change && received_height.iter().all(|h| h > &summary_height);
+            let is_pending_change =
+                is_change && received_height.iter().all(|h| h > &summary_height);
 
             let (spendable_value, change_pending_confirmation, value_pending_spendability) = {
                 let zero = NonNegativeAmount::ZERO;
@@ -935,14 +1065,52 @@ pub(crate) fn get_wallet_summary<P: consensus::Parameters>(
             };
 
             if let Some(balances) = account_balances.get_mut(&account) {
+                with_pool_balance(
+                    balances,
+                    spendable_value,
+                    change_pending_confirmation,
+                    value_pending_spendability,
+                )?;
+            }
+        }
+        Ok(())
+    }
+
+    #[cfg(feature = "orchard")]
+    {
+        let orchard_trace = tracing::info_span!("orchard_balances").entered();
+        count_notes(
+            tx,
+            summary_height,
+            &mut account_balances,
+            ORCHARD_TABLES_PREFIX,
+            |balances, spendable_value, change_pending_confirmation, value_pending_spendability| {
+                balances.with_orchard_balance_mut::<_, SqliteClientError>(|bal| {
+                    bal.add_spendable_value(spendable_value)?;
+                    bal.add_pending_change_value(change_pending_confirmation)?;
+                    bal.add_pending_spendable_value(value_pending_spendability)?;
+                    Ok(())
+                })
+            },
+        )?;
+        drop(orchard_trace);
+    }
+
+    let sapling_trace = tracing::info_span!("sapling_balances").entered();
+    count_notes(
+        tx,
+        summary_height,
+        &mut account_balances,
+        SAPLING_TABLES_PREFIX,
+        |balances, spendable_value, change_pending_confirmation, value_pending_spendability| {
             balances.with_sapling_balance_mut::<_, SqliteClientError>(|bal| {
                 bal.add_spendable_value(spendable_value)?;
                 bal.add_pending_change_value(change_pending_confirmation)?;
                 bal.add_pending_spendable_value(value_pending_spendability)?;
                 Ok(())
-            })?;
+            })
-        }
+        },
-    }
+    )?;
     drop(sapling_trace);
 
     #[cfg(feature = "transparent-inputs")]
@@ -1025,7 +1193,7 @@ pub(crate) fn get_wallet_summary<P: consensus::Parameters>(
         account_balances,
         chain_tip_height,
         fully_scanned_height,
-        sapling_scan_progress,
+        scan_progress,
         next_sapling_subtree_index,
         #[cfg(feature = "orchard")]
         next_orchard_subtree_index,
@@ -1039,24 +1207,31 @@ pub(crate) fn get_received_memo(
     conn: &rusqlite::Connection,
     note_id: NoteId,
 ) -> Result<Option<Memo>, SqliteClientError> {
-    let memo_bytes: Option<Vec<_>> = match note_id.protocol() {
+    let fetch_memo = |table_prefix: &'static str, output_col: &'static str| {
-        ShieldedProtocol::Sapling => conn
+        conn.query_row(
-            .query_row(
+            &format!(
-                "SELECT memo FROM sapling_received_notes
+                "SELECT memo FROM {table_prefix}_received_notes
-                JOIN transactions ON sapling_received_notes.tx = transactions.id_tx
+                JOIN transactions ON {table_prefix}_received_notes.tx = transactions.id_tx
                 WHERE transactions.txid = :txid
-                AND sapling_received_notes.output_index = :output_index",
+                AND {table_prefix}_received_notes.{output_col} = :output_index"
+            ),
             named_params![
                 ":txid": note_id.txid().as_ref(),
                 ":output_index": note_id.output_index()
             ],
             |row| row.get(0),
         )
-            .optional()?
+        .optional()
-            .flatten(),
+    };
-        _ => {
+
+    let memo_bytes: Option<Vec<_>> = match note_id.protocol() {
+        ShieldedProtocol::Sapling => fetch_memo(SAPLING_TABLES_PREFIX, "output_index")?.flatten(),
+        #[cfg(feature = "orchard")]
+        ShieldedProtocol::Orchard => fetch_memo(ORCHARD_TABLES_PREFIX, "action_index")?.flatten(),
+        #[cfg(not(feature = "orchard"))]
+        ShieldedProtocol::Orchard => {
             return Err(SqliteClientError::UnsupportedPoolType(PoolType::Shielded(
-                note_id.protocol(),
+                ShieldedProtocol::Orchard,
             )))
         }
     };
@@ -1321,7 +1496,24 @@ pub(crate) fn get_target_and_anchor_heights(
                 min_confirmations,
             )?;
 
-            Ok(sapling_anchor_height.map(|h| (chain_tip_height + 1, h)))
+            #[cfg(feature = "orchard")]
+            let orchard_anchor_height = get_max_checkpointed_height(
+                conn,
+                ORCHARD_TABLES_PREFIX,
+                chain_tip_height,
+                min_confirmations,
+            )?;
+
+            #[cfg(not(feature = "orchard"))]
+            let orchard_anchor_height: Option<BlockHeight> = None;
+
+            let anchor_height = sapling_anchor_height
+                .zip(orchard_anchor_height)
+                .map(|(s, o)| std::cmp::min(s, o))
+                .or(sapling_anchor_height)
+                .or(orchard_anchor_height);
+
+            Ok(anchor_height.map(|h| (chain_tip_height + 1, h)))
         }
         None => Ok(None),
     }
@@ -1541,7 +1733,7 @@ pub(crate) fn get_max_height_hash(
 pub(crate) fn get_min_unspent_height(
     conn: &rusqlite::Connection,
 ) -> Result<Option<BlockHeight>, SqliteClientError> {
-    conn.query_row(
+    let min_sapling: Option<BlockHeight> = conn.query_row(
         "SELECT MIN(tx.block)
          FROM sapling_received_notes n
          JOIN transactions tx ON tx.id_tx = n.tx
@@ -1551,8 +1743,27 @@ pub(crate) fn get_min_unspent_height(
             row.get(0)
                 .map(|maybe_height: Option<u32>| maybe_height.map(|height| height.into()))
         },
-    )
+    )?;
-    .map_err(SqliteClientError::from)
+    #[cfg(feature = "orchard")]
+    let min_orchard: Option<BlockHeight> = conn.query_row(
+        "SELECT MIN(tx.block)
+         FROM orchard_received_notes n
+         JOIN transactions tx ON tx.id_tx = n.tx
+         WHERE n.spent IS NULL",
+        [],
+        |row| {
+            row.get(0)
+                .map(|maybe_height: Option<u32>| maybe_height.map(|height| height.into()))
+        },
+    )?;
+    #[cfg(not(feature = "orchard"))]
+    let min_orchard = None;
+
+    Ok(min_sapling
+        .zip(min_orchard)
+        .map(|(s, o)| s.min(o))
+        .or(min_sapling)
+        .or(min_orchard))
 }
 
 /// Truncates the database to the given height.
@@ -1594,6 +1805,10 @@ pub(crate) fn truncate_to_height<P: consensus::Parameters>(
         wdb.with_sapling_tree_mut(|tree| {
             tree.truncate_removing_checkpoint(&block_height).map(|_| ())
         })?;
+        #[cfg(feature = "orchard")]
+        wdb.with_orchard_tree_mut(|tree| {
+            tree.truncate_removing_checkpoint(&block_height).map(|_| ())
+        })?;
 
         // Rewind received notes
         conn.execute(
@@ -1607,6 +1822,18 @@ pub(crate) fn truncate_to_height<P: consensus::Parameters>(
             );",
             [u32::from(block_height)],
         )?;
+        #[cfg(feature = "orchard")]
+        conn.execute(
+            "DELETE FROM orchard_received_notes
+            WHERE id IN (
+                SELECT rn.id
+                FROM orchard_received_notes rn
+                LEFT OUTER JOIN transactions tx
+                ON tx.id_tx = rn.tx
+                WHERE tx.block IS NOT NULL AND tx.block > ?
+            );",
+            [u32::from(block_height)],
+        )?;
 
         // Do not delete sent notes; this can contain data that is not recoverable
         // from the chain. Wallets must continue to operate correctly in the

zcash_client_sqlite/src/wallet/common.rs

@@ -0,0 +1,216 @@
+//! Functions common to Sapling and Orchard support in the wallet.
+
+use rusqlite::{named_params, types::Value, Connection, Row};
+use std::rc::Rc;
+
+use zcash_client_backend::{
+    wallet::{Note, ReceivedNote},
+    ShieldedProtocol,
+};
+use zcash_primitives::transaction::{components::amount::NonNegativeAmount, TxId};
+use zcash_protocol::consensus::{self, BlockHeight};
+
+use super::wallet_birthday;
+use crate::{error::SqliteClientError, AccountId, ReceivedNoteId, SAPLING_TABLES_PREFIX};
+
+#[cfg(feature = "orchard")]
+use crate::ORCHARD_TABLES_PREFIX;
+
+fn per_protocol_names(protocol: ShieldedProtocol) -> (&'static str, &'static str, &'static str) {
+    match protocol {
+        ShieldedProtocol::Sapling => (SAPLING_TABLES_PREFIX, "output_index", "rcm"),
+        #[cfg(feature = "orchard")]
+        ShieldedProtocol::Orchard => (ORCHARD_TABLES_PREFIX, "action_index", "rho, rseed"),
+        #[cfg(not(feature = "orchard"))]
+        ShieldedProtocol::Orchard => {
+            unreachable!("Should never be called unless the `orchard` feature is enabled")
+        }
+    }
+}
+
+fn unscanned_tip_exists(
+    conn: &Connection,
+    anchor_height: BlockHeight,
+    table_prefix: &'static str,
+) -> Result<bool, rusqlite::Error> {
+    // v_sapling_shard_unscanned_ranges only returns ranges ending on or after wallet birthday, so
+    // we don't need to refer to the birthday in this query.
+    conn.query_row(
+        &format!(
+            "SELECT EXISTS (
+                 SELECT 1 FROM v_{table_prefix}_shard_unscanned_ranges range
+                 WHERE range.block_range_start <= :anchor_height
+                 AND :anchor_height BETWEEN
+                    range.subtree_start_height
+                    AND IFNULL(range.subtree_end_height, :anchor_height)
+             )"
+        ),
+        named_params![":anchor_height": u32::from(anchor_height),],
+        |row| row.get::<_, bool>(0),
+    )
+}
+
+// The `clippy::let_and_return` lint is explicitly allowed here because a bug in Clippy
+// (https://github.com/rust-lang/rust-clippy/issues/11308) means it fails to identify that the `result` temporary
+// is required in order to resolve the borrows involved in the `query_and_then` call.
+#[allow(clippy::let_and_return)]
+pub(crate) fn get_spendable_note<P: consensus::Parameters, F>(
+    conn: &Connection,
+    params: &P,
+    txid: &TxId,
+    index: u32,
+    protocol: ShieldedProtocol,
+    to_spendable_note: F,
+) -> Result<Option<ReceivedNote<ReceivedNoteId, Note>>, SqliteClientError>
+where
+    F: Fn(&P, &Row) -> Result<Option<ReceivedNote<ReceivedNoteId, Note>>, SqliteClientError>,
+{
+    let (table_prefix, index_col, note_reconstruction_cols) = per_protocol_names(protocol);
+    let result = conn.query_row_and_then(
+        &format!(
+            "SELECT {table_prefix}_received_notes.id, txid, {index_col},
+                diversifier, value, {note_reconstruction_cols}, commitment_tree_position,
+                accounts.ufvk, recipient_key_scope
+             FROM {table_prefix}_received_notes
+             INNER JOIN accounts ON accounts.id = {table_prefix}_received_notes.account_id
+             INNER JOIN transactions ON transactions.id_tx = {table_prefix}_received_notes.tx
+             WHERE txid = :txid
+             AND {index_col} = :output_index
+             AND accounts.ufvk IS NOT NULL
+             AND recipient_key_scope IS NOT NULL
+             AND nf IS NOT NULL
+             AND commitment_tree_position IS NOT NULL
+             AND spent IS NULL"
+        ),
+        named_params![
+           ":txid": txid.as_ref(),
+           ":output_index": index,
+        ],
+        |row| to_spendable_note(params, row),
+    );
+
+    // `OptionalExtension` doesn't work here because the error type of `Result` is already
+    // `SqliteClientError`
+    match result {
+        Ok(r) => Ok(r),
+        Err(SqliteClientError::DbError(rusqlite::Error::QueryReturnedNoRows)) => Ok(None),
+        Err(e) => Err(e),
+    }
+}
+
+#[allow(clippy::too_many_arguments)]
+pub(crate) fn select_spendable_notes<P: consensus::Parameters, F>(
+    conn: &Connection,
+    params: &P,
+    account: AccountId,
+    target_value: NonNegativeAmount,
+    anchor_height: BlockHeight,
+    exclude: &[ReceivedNoteId],
+    protocol: ShieldedProtocol,
+    to_spendable_note: F,
+) -> Result<Vec<ReceivedNote<ReceivedNoteId, Note>>, SqliteClientError>
+where
+    F: Fn(&P, &Row) -> Result<Option<ReceivedNote<ReceivedNoteId, Note>>, SqliteClientError>,
+{
+    let birthday_height = match wallet_birthday(conn)? {
+        Some(birthday) => birthday,
+        None => {
+            // the wallet birthday can only be unknown if there are no accounts in the wallet; in
+            // such a case, the wallet has no notes to spend.
+            return Ok(vec![]);
+        }
+    };
+
+    let (table_prefix, index_col, note_reconstruction_cols) = per_protocol_names(protocol);
+    if unscanned_tip_exists(conn, anchor_height, table_prefix)? {
+        return Ok(vec![]);
+    }
+
+    // The goal of this SQL statement is to select the oldest notes until the required
+    // value has been reached.
+    // 1) Use a window function to create a view of all notes, ordered from oldest to
+    //    newest, with an additional column containing a running sum:
+    //    - Unspent notes accumulate the values of all unspent notes in that note's
+    //      account, up to itself.
+    //    - Spent notes accumulate the values of all notes in the transaction they were
+    //      spent in, up to itself.
+    //
+    // 2) Select all unspent notes in the desired account, along with their running sum.
+    //
+    // 3) Select all notes for which the running sum was less than the required value, as
+    //    well as a single note for which the sum was greater than or equal to the
+    //    required value, bringing the sum of all selected notes across the threshold.
+    let mut stmt_select_notes = conn.prepare_cached(
+        &format!(
+            "WITH eligible AS (
+                 SELECT
+                     {table_prefix}_received_notes.id AS id, txid, {index_col},
+                     diversifier, value, {note_reconstruction_cols}, commitment_tree_position,
+                     SUM(value) OVER (
+                         PARTITION BY {table_prefix}_received_notes.account_id, spent
+                         ORDER BY {table_prefix}_received_notes.id
+                     ) AS so_far,
+                     accounts.ufvk as ufvk, recipient_key_scope
+                 FROM {table_prefix}_received_notes
+                 INNER JOIN accounts
+                    ON accounts.id = {table_prefix}_received_notes.account_id
+                 INNER JOIN transactions
+                    ON transactions.id_tx = {table_prefix}_received_notes.tx
+                 WHERE {table_prefix}_received_notes.account_id = :account
+                 AND accounts.ufvk IS NOT NULL
+                 AND recipient_key_scope IS NOT NULL
+                 AND nf IS NOT NULL
+                 AND commitment_tree_position IS NOT NULL
+                 AND spent IS NULL
+                 AND transactions.block <= :anchor_height
+                 AND {table_prefix}_received_notes.id NOT IN rarray(:exclude)
+                 AND NOT EXISTS (
+                    SELECT 1 FROM v_{table_prefix}_shard_unscanned_ranges unscanned
+                    -- select all the unscanned ranges involving the shard containing this note
+                    WHERE {table_prefix}_received_notes.commitment_tree_position >= unscanned.start_position
+                    AND {table_prefix}_received_notes.commitment_tree_position < unscanned.end_position_exclusive
+                    -- exclude unscanned ranges that start above the anchor height (they don't affect spendability)
+                    AND unscanned.block_range_start <= :anchor_height
+                    -- exclude unscanned ranges that end below the wallet birthday
+                    AND unscanned.block_range_end > :wallet_birthday
+                 )
+             )
+             SELECT id, txid, {index_col},
+                    diversifier, value, {note_reconstruction_cols}, commitment_tree_position,
+                    ufvk, recipient_key_scope
+             FROM eligible WHERE so_far < :target_value
+             UNION
+             SELECT id, txid, {index_col},
+                    diversifier, value, {note_reconstruction_cols}, commitment_tree_position,
+                    ufvk, recipient_key_scope
+             FROM (SELECT * from eligible WHERE so_far >= :target_value LIMIT 1)",
+        )
+    )?;
+
+    let excluded: Vec<Value> = exclude
+        .iter()
+        .filter_map(|ReceivedNoteId(p, n)| {
+            if *p == protocol {
+                Some(Value::from(*n))
+            } else {
+                None
+            }
+        })
+        .collect();
+    let excluded_ptr = Rc::new(excluded);
+
+    let notes = stmt_select_notes.query_and_then(
+        named_params![
+            ":account": account.0,
+            ":anchor_height": &u32::from(anchor_height),
+            ":target_value": &u64::from(target_value),
+            ":exclude": &excluded_ptr,
+            ":wallet_birthday": u32::from(birthday_height)
+        ],
+        |r| to_spendable_note(params, r),
+    )?;
+
+    notes
+        .filter_map(|r| r.transpose())
+        .collect::<Result<_, _>>()
+}

zcash_client_sqlite/src/wallet/orchard.rs

@@ -1,24 +1,36 @@
 use incrementalmerkletree::Position;
-use rusqlite::{named_params, params, Connection};
+use orchard::{
+    keys::Diversifier,
+    note::{Note, Nullifier, RandomSeed},
+};
+use rusqlite::{named_params, params, Connection, Row};
 
 use zcash_client_backend::{
-    data_api::NullifierQuery, wallet::WalletOrchardOutput, DecryptedOutput, TransferType,
+    data_api::NullifierQuery,
+    wallet::{ReceivedNote, WalletOrchardOutput},
+    DecryptedOutput, ShieldedProtocol, TransferType,
+};
+use zcash_keys::keys::UnifiedFullViewingKey;
+use zcash_primitives::transaction::TxId;
+use zcash_protocol::{
+    consensus::{self, BlockHeight},
+    memo::MemoBytes,
+    value::Zatoshis,
 };
-use zcash_protocol::memo::MemoBytes;
 use zip32::Scope;
 
-use crate::{error::SqliteClientError, AccountId};
+use crate::{error::SqliteClientError, AccountId, ReceivedNoteId};
 
-use super::{memo_repr, scope_code};
+use super::{memo_repr, parse_scope, scope_code};
 
 /// This trait provides a generalization over shielded output representations.
 pub(crate) trait ReceivedOrchardOutput {
     fn index(&self) -> usize;
     fn account_id(&self) -> AccountId;
-    fn note(&self) -> &orchard::note::Note;
+    fn note(&self) -> &Note;
     fn memo(&self) -> Option<&MemoBytes>;
     fn is_change(&self) -> bool;
-    fn nullifier(&self) -> Option<&orchard::note::Nullifier>;
+    fn nullifier(&self) -> Option<&Nullifier>;
     fn note_commitment_tree_position(&self) -> Option<Position>;
     fn recipient_key_scope(&self) -> Option<Scope>;
 }
@@ -30,7 +42,7 @@ impl ReceivedOrchardOutput for WalletOrchardOutput<AccountId> {
     fn account_id(&self) -> AccountId {
         *WalletOrchardOutput::account_id(self)
     }
-    fn note(&self) -> &orchard::note::Note {
+    fn note(&self) -> &Note {
         WalletOrchardOutput::note(self)
     }
     fn memo(&self) -> Option<&MemoBytes> {
@@ -39,7 +51,7 @@ impl ReceivedOrchardOutput for WalletOrchardOutput<AccountId> {
     fn is_change(&self) -> bool {
         WalletOrchardOutput::is_change(self)
     }
-    fn nullifier(&self) -> Option<&orchard::note::Nullifier> {
+    fn nullifier(&self) -> Option<&Nullifier> {
         self.nf()
     }
     fn note_commitment_tree_position(&self) -> Option<Position> {
@@ -50,7 +62,7 @@ impl ReceivedOrchardOutput for WalletOrchardOutput<AccountId> {
     }
 }
 
-impl ReceivedOrchardOutput for DecryptedOutput<orchard::note::Note, AccountId> {
+impl ReceivedOrchardOutput for DecryptedOutput<Note, AccountId> {
     fn index(&self) -> usize {
         self.index()
     }
@@ -66,7 +78,7 @@ impl ReceivedOrchardOutput for DecryptedOutput<orchard::note::Note, AccountId> {
     fn is_change(&self) -> bool {
         self.transfer_type() == TransferType::WalletInternal
     }
-    fn nullifier(&self) -> Option<&orchard::note::Nullifier> {
+    fn nullifier(&self) -> Option<&Nullifier> {
         None
     }
     fn note_commitment_tree_position(&self) -> Option<Position> {
@@ -81,6 +93,136 @@ impl ReceivedOrchardOutput for DecryptedOutput<orchard::note::Note, AccountId> {
     }
 }
 
+fn to_spendable_note<P: consensus::Parameters>(
+    params: &P,
+    row: &Row,
+) -> Result<
+    Option<ReceivedNote<ReceivedNoteId, zcash_client_backend::wallet::Note>>,
+    SqliteClientError,
+> {
+    let note_id = ReceivedNoteId(ShieldedProtocol::Orchard, row.get(0)?);
+    let txid = row.get::<_, [u8; 32]>(1).map(TxId::from_bytes)?;
+    let action_index = row.get(2)?;
+    let diversifier = {
+        let d: Vec<_> = row.get(3)?;
+        if d.len() != 11 {
+            return Err(SqliteClientError::CorruptedData(
+                "Invalid diversifier length".to_string(),
+            ));
+        }
+        let mut tmp = [0; 11];
+        tmp.copy_from_slice(&d);
+        Diversifier::from_bytes(tmp)
+    };
+
+    let note_value: u64 = row.get::<_, i64>(4)?.try_into().map_err(|_e| {
+        SqliteClientError::CorruptedData("Note values must be nonnegative".to_string())
+    })?;
+
+    let rho = {
+        let rho_bytes: [u8; 32] = row.get(5)?;
+        Option::from(Nullifier::from_bytes(&rho_bytes))
+            .ok_or_else(|| SqliteClientError::CorruptedData("Invalid rho.".to_string()))
+    }?;
+
+    let rseed = {
+        let rseed_bytes: [u8; 32] = row.get(6)?;
+        Option::from(RandomSeed::from_bytes(rseed_bytes, &rho)).ok_or_else(|| {
+            SqliteClientError::CorruptedData("Invalid Orchard random seed.".to_string())
+        })
+    }?;
+
+    let note_commitment_tree_position =
+        Position::from(u64::try_from(row.get::<_, i64>(7)?).map_err(|_| {
+            SqliteClientError::CorruptedData("Note commitment tree position invalid.".to_string())
+        })?);
+
+    let ufvk_str: Option<String> = row.get(8)?;
+    let scope_code: Option<i64> = row.get(9)?;
+
+    // If we don't have information about the recipient key scope or the ufvk we can't determine
+    // which spending key to use. This may be because the received note was associated with an
+    // imported viewing key, so we treat such notes as not spendable. Although this method is
+    // presently only called using the results of queries where both the ufvk and
+    // recipient_key_scope columns are checked to be non-null, this is method is written
+    // defensively to account for the fact that both of these are nullable columns in case it
+    // is used elsewhere in the future.
+    ufvk_str
+        .zip(scope_code)
+        .map(|(ufvk_str, scope_code)| {
+            let ufvk = UnifiedFullViewingKey::decode(params, &ufvk_str)
+                .map_err(SqliteClientError::CorruptedData)?;
+
+            let spending_key_scope = parse_scope(scope_code).ok_or_else(|| {
+                SqliteClientError::CorruptedData(format!("Invalid key scope code {}", scope_code))
+            })?;
+            let recipient = ufvk
+                .orchard()
+                .map(|fvk| fvk.to_ivk(spending_key_scope).address(diversifier))
+                .ok_or_else(|| {
+                    SqliteClientError::CorruptedData("Diversifier invalid.".to_owned())
+                })?;
+
+            let note = Option::from(Note::from_parts(
+                recipient,
+                orchard::value::NoteValue::from_raw(note_value),
+                rho,
+                rseed,
+            ))
+            .ok_or_else(|| SqliteClientError::CorruptedData("Invalid Orchard note.".to_string()))?;
+
+            Ok(ReceivedNote::from_parts(
+                note_id,
+                txid,
+                action_index,
+                zcash_client_backend::wallet::Note::Orchard(note),
+                spending_key_scope,
+                note_commitment_tree_position,
+            ))
+        })
+        .transpose()
+}
+
+pub(crate) fn get_spendable_orchard_note<P: consensus::Parameters>(
+    conn: &Connection,
+    params: &P,
+    txid: &TxId,
+    index: u32,
+) -> Result<
+    Option<ReceivedNote<ReceivedNoteId, zcash_client_backend::wallet::Note>>,
+    SqliteClientError,
+> {
+    super::common::get_spendable_note(
+        conn,
+        params,
+        txid,
+        index,
+        ShieldedProtocol::Orchard,
+        to_spendable_note,
+    )
+}
+
+pub(crate) fn select_spendable_orchard_notes<P: consensus::Parameters>(
+    conn: &Connection,
+    params: &P,
+    account: AccountId,
+    target_value: Zatoshis,
+    anchor_height: BlockHeight,
+    exclude: &[ReceivedNoteId],
+) -> Result<Vec<ReceivedNote<ReceivedNoteId, zcash_client_backend::wallet::Note>>, SqliteClientError>
+{
+    super::common::select_spendable_notes(
+        conn,
+        params,
+        account,
+        target_value,
+        anchor_height,
+        exclude,
+        ShieldedProtocol::Orchard,
+        to_spendable_note,
+    )
+}
+
 /// Records the specified shielded output as having been received.
 ///
 /// This implementation relies on the facts that:
@@ -94,27 +236,23 @@ pub(crate) fn put_received_note<T: ReceivedOrchardOutput>(
 ) -> Result<(), SqliteClientError> {
     let mut stmt_upsert_received_note = conn.prepare_cached(
         "INSERT INTO orchard_received_notes
-        (tx, action_index, account_id, diversifier, value, rseed, memo, nf,
+        (
+            tx, action_index, account_id,
+            diversifier, value, rho, rseed, memo, nf,
             is_change, spent, commitment_tree_position,
-         recipient_key_scope)
+            recipient_key_scope
+        )
         VALUES (
-            :tx,
+            :tx, :action_index, :account_id,
-            :action_index,
+            :diversifier, :value, :rho, :rseed, :memo, :nf,
-            :account_id,
+            :is_change, :spent, :commitment_tree_position,
-            :diversifier,
-            :value,
-            :rseed,
-            :memo,
-            :nf,
-            :is_change,
-            :spent,
-            :commitment_tree_position,
             :recipient_key_scope
         )
         ON CONFLICT (tx, action_index) DO UPDATE
         SET account_id = :account_id,
             diversifier = :diversifier,
             value = :value,
+            rho = :rho,
             rseed = :rseed,
             nf = IFNULL(:nf, nf),
             memo = IFNULL(:memo, memo),
@@ -130,10 +268,11 @@ pub(crate) fn put_received_note<T: ReceivedOrchardOutput>(
 
     let sql_args = named_params![
         ":tx": &tx_ref,
-        ":output_index": i64::try_from(output.index()).expect("output indices are representable as i64"),
+        ":action_index": i64::try_from(output.index()).expect("output indices are representable as i64"),
         ":account_id": output.account_id().0,
         ":diversifier": diversifier.as_array(),
         ":value": output.note().value().inner(),
+        ":rho": output.note().rho().to_bytes(),
         ":rseed": &rseed.as_bytes(),
         ":nf": output.nullifier().map(|nf| nf.to_bytes()),
         ":memo": memo_repr(output.memo()),
@@ -159,11 +298,11 @@ pub(crate) fn put_received_note<T: ReceivedOrchardOutput>(
 pub(crate) fn get_orchard_nullifiers(
     conn: &Connection,
     query: NullifierQuery,
-) -> Result<Vec<(AccountId, orchard::note::Nullifier)>, SqliteClientError> {
+) -> Result<Vec<(AccountId, Nullifier)>, SqliteClientError> {
     // Get the nullifiers for the notes we are tracking
     let mut stmt_fetch_nullifiers = match query {
         NullifierQuery::Unspent => conn.prepare(
-            "SELECT rn.id, rn.account_id, rn.nf
+            "SELECT rn.account_id, rn.nf
              FROM orchard_received_notes rn
              LEFT OUTER JOIN transactions tx
              ON tx.id_tx = rn.spent
@@ -171,19 +310,16 @@ pub(crate) fn get_orchard_nullifiers(
              AND nf IS NOT NULL",
         )?,
         NullifierQuery::All => conn.prepare(
-            "SELECT rn.id, rn.account_id, rn.nf
+            "SELECT rn.account_id, rn.nf
              FROM orchard_received_notes rn
              WHERE nf IS NOT NULL",
         )?,
     };
 
     let nullifiers = stmt_fetch_nullifiers.query_and_then([], |row| {
-        let account = AccountId(row.get(1)?);
+        let account = AccountId(row.get(0)?);
-        let nf_bytes: [u8; 32] = row.get(2)?;
+        let nf_bytes: [u8; 32] = row.get(1)?;
-        Ok::<_, rusqlite::Error>((
+        Ok::<_, rusqlite::Error>((account, Nullifier::from_bytes(&nf_bytes).unwrap()))
-            account,
-            orchard::note::Nullifier::from_bytes(&nf_bytes).unwrap(),
-        ))
     })?;
 
     let res: Vec<_> = nullifiers.collect::<Result<_, _>>()?;
@@ -198,7 +334,7 @@ pub(crate) fn get_orchard_nullifiers(
 pub(crate) fn mark_orchard_note_spent(
     conn: &Connection,
     tx_ref: i64,
-    nf: &orchard::note::Nullifier,
+    nf: &Nullifier,
 ) -> Result<bool, SqliteClientError> {
     let mut stmt_mark_orchard_note_spent =
         conn.prepare_cached("UPDATE orchard_received_notes SET spent = ? WHERE nf = ?")?;
@@ -233,6 +369,7 @@ pub(crate) mod tests {
     use zcash_primitives::transaction::Transaction;
     use zcash_protocol::{consensus::BlockHeight, memo::MemoBytes, ShieldedProtocol};
 
+    use super::select_spendable_orchard_notes;
     use crate::{
         error::SqliteClientError,
         testing::{
@@ -321,7 +458,14 @@ pub(crate) mod tests {
             anchor_height: BlockHeight,
             exclude: &[crate::ReceivedNoteId],
         ) -> Result<Vec<ReceivedNote<crate::ReceivedNoteId, Note>>, SqliteClientError> {
-            todo!()
+            select_spendable_orchard_notes(
+                &st.wallet().conn,
+                &st.wallet().params,
+                account,
+                target_value,
+                anchor_height,
+                exclude,
+            )
         }
 
         fn decrypted_pool_outputs_count(

zcash_client_sqlite/src/wallet/sapling.rs

@@ -2,8 +2,7 @@
 
 use group::ff::PrimeField;
 use incrementalmerkletree::Position;
-use rusqlite::{named_params, params, types::Value, Connection, Row};
+use rusqlite::{named_params, params, Connection, Row};
-use std::rc::Rc;
 
 use sapling::{self, Diversifier, Nullifier, Rseed};
 use zcash_client_backend::{
@@ -21,7 +20,7 @@ use zip32::Scope;
 
 use crate::{error::SqliteClientError, AccountId, ReceivedNoteId};
 
-use super::{memo_repr, parse_scope, scope_code, wallet_birthday};
+use super::{memo_repr, parse_scope, scope_code};
 
 /// This trait provides a generalization over shielded output representations.
 pub(crate) trait ReceivedSaplingOutput {
@@ -192,32 +191,14 @@ pub(crate) fn get_spendable_sapling_note<P: consensus::Parameters>(
     txid: &TxId,
     index: u32,
 ) -> Result<Option<ReceivedNote<ReceivedNoteId, Note>>, SqliteClientError> {
-    let result = conn.query_row_and_then(
+    super::common::get_spendable_note(
-        "SELECT sapling_received_notes.id, txid, output_index,
+        conn,
-                diversifier, value, rcm, commitment_tree_position,
+        params,
-                accounts.ufvk, recipient_key_scope
+        txid,
-         FROM sapling_received_notes
+        index,
-         INNER JOIN accounts on accounts.id = sapling_received_notes.account_id
+        ShieldedProtocol::Sapling,
-         INNER JOIN transactions ON transactions.id_tx = sapling_received_notes.tx
+        to_spendable_note,
-         WHERE txid = :txid
+    )
-         AND accounts.ufvk IS NOT NULL
-         AND recipient_key_scope IS NOT NULL
-         AND output_index = :output_index
-         AND spent IS NULL",
-        named_params![
-           ":txid": txid.as_ref(),
-           ":output_index": index,
-        ],
-        |row| to_spendable_note(params, row),
-    );
-
-    // `OptionalExtension` doesn't work here because the error type of `Result` is already
-    // `SqliteClientError`
-    match result {
-        Ok(r) => Ok(r),
-        Err(SqliteClientError::DbError(rusqlite::Error::QueryReturnedNoRows)) => Ok(None),
-        Err(e) => Err(e),
-    }
 }
 
 /// Utility method for determining whether we have any spendable notes
@@ -225,25 +206,6 @@ pub(crate) fn get_spendable_sapling_note<P: consensus::Parameters>(
 /// If the tip shard has unscanned ranges below the anchor height and greater than or equal to
 /// the wallet birthday, none of our notes can be spent because we cannot construct witnesses at
 /// the provided anchor height.
-fn unscanned_tip_exists(
-    conn: &Connection,
-    anchor_height: BlockHeight,
-) -> Result<bool, rusqlite::Error> {
-    // v_sapling_shard_unscanned_ranges only returns ranges ending on or after wallet birthday, so
-    // we don't need to refer to the birthday in this query.
-    conn.query_row(
-        "SELECT EXISTS (
-             SELECT 1 FROM v_sapling_shard_unscanned_ranges range
-             WHERE range.block_range_start <= :anchor_height
-             AND :anchor_height BETWEEN
-                range.subtree_start_height
-                AND IFNULL(range.subtree_end_height, :anchor_height)
-         )",
-        named_params![":anchor_height": u32::from(anchor_height),],
-        |row| row.get::<_, bool>(0),
-    )
-}
-
 pub(crate) fn select_spendable_sapling_notes<P: consensus::Parameters>(
     conn: &Connection,
     params: &P,
@@ -252,88 +214,16 @@ pub(crate) fn select_spendable_sapling_notes<P: consensus::Parameters>(
     anchor_height: BlockHeight,
     exclude: &[ReceivedNoteId],
 ) -> Result<Vec<ReceivedNote<ReceivedNoteId, Note>>, SqliteClientError> {
-    let birthday_height = match wallet_birthday(conn)? {
+    super::common::select_spendable_notes(
-        Some(birthday) => birthday,
+        conn,
-        None => {
+        params,
-            // the wallet birthday can only be unknown if there are no accounts in the wallet; in
+        account,
-            // such a case, the wallet has no notes to spend.
+        target_value,
-            return Ok(vec![]);
+        anchor_height,
-        }
+        exclude,
-    };
+        ShieldedProtocol::Sapling,
-
+        to_spendable_note,
-    if unscanned_tip_exists(conn, anchor_height)? {
-        return Ok(vec![]);
-    }
-
-    // The goal of this SQL statement is to select the oldest notes until the required
-    // value has been reached.
-    // 1) Use a window function to create a view of all notes, ordered from oldest to
-    //    newest, with an additional column containing a running sum:
-    //    - Unspent notes accumulate the values of all unspent notes in that note's
-    //      account, up to itself.
-    //    - Spent notes accumulate the values of all notes in the transaction they were
-    //      spent in, up to itself.
-    //
-    // 2) Select all unspent notes in the desired account, along with their running sum.
-    //
-    // 3) Select all notes for which the running sum was less than the required value, as
-    //    well as a single note for which the sum was greater than or equal to the
-    //    required value, bringing the sum of all selected notes across the threshold.
-    //
-    // 4) Match the selected notes against the witnesses at the desired height.
-    let mut stmt_select_notes = conn.prepare_cached(
-        "WITH eligible AS (
-             SELECT
-                 sapling_received_notes.id AS id, txid, output_index, diversifier, value, rcm, commitment_tree_position,
-                 SUM(value)
-                    OVER (PARTITION BY sapling_received_notes.account_id, spent ORDER BY sapling_received_notes.id) AS so_far,
-                 accounts.ufvk as ufvk, recipient_key_scope
-             FROM sapling_received_notes
-             INNER JOIN accounts on accounts.id = sapling_received_notes.account_id
-             INNER JOIN transactions
-                ON transactions.id_tx = sapling_received_notes.tx
-             WHERE sapling_received_notes.account_id = :account
-             AND ufvk IS NOT NULL
-             AND recipient_key_scope IS NOT NULL
-             AND commitment_tree_position IS NOT NULL
-             AND spent IS NULL
-             AND transactions.block <= :anchor_height
-             AND sapling_received_notes.id NOT IN rarray(:exclude)
-             AND NOT EXISTS (
-                SELECT 1 FROM v_sapling_shard_unscanned_ranges unscanned
-                -- select all the unscanned ranges involving the shard containing this note
-                WHERE sapling_received_notes.commitment_tree_position >= unscanned.start_position
-                AND sapling_received_notes.commitment_tree_position < unscanned.end_position_exclusive
-                -- exclude unscanned ranges that start above the anchor height (they don't affect spendability)
-                AND unscanned.block_range_start <= :anchor_height
-                -- exclude unscanned ranges that end below the wallet birthday
-                AND unscanned.block_range_end > :wallet_birthday
     )
-         )
-         SELECT id, txid, output_index, diversifier, value, rcm, commitment_tree_position, ufvk, recipient_key_scope
-         FROM eligible WHERE so_far < :target_value
-         UNION
-         SELECT id, txid, output_index, diversifier, value, rcm, commitment_tree_position, ufvk, recipient_key_scope
-         FROM (SELECT * from eligible WHERE so_far >= :target_value LIMIT 1)",
-    )?;
-
-    let excluded: Vec<Value> = exclude.iter().map(|n| Value::from(n.1)).collect();
-    let excluded_ptr = Rc::new(excluded);
-
-    let notes = stmt_select_notes.query_and_then(
-        named_params![
-            ":account": account.0,
-            ":anchor_height": &u32::from(anchor_height),
-            ":target_value": &u64::from(target_value),
-            ":exclude": &excluded_ptr,
-            ":wallet_birthday": u32::from(birthday_height)
-        ],
-        |r| to_spendable_note(params, r),
-    )?;
-
-    notes
-        .filter_map(|r| r.transpose())
-        .collect::<Result<_, _>>()
 }
 
 /// Retrieves the set of nullifiers for "potentially spendable" Sapling notes that the

zcash_client_sqlite/src/wallet/scanning.rs

@@ -381,15 +381,12 @@ pub(crate) fn scan_complete<P: consensus::Parameters>(
         .map(|extended| ScanRange::from_parts(range.end..extended.end, ScanPriority::FoundNote))
         .filter(|range| !range.is_empty());
 
-    replace_queue_entries::<SqliteClientError>(
+    let replacement = Some(scanned)
-        conn,
-        &query_range,
-        Some(scanned)
         .into_iter()
         .chain(extended_before)
-            .chain(extended_after),
+        .chain(extended_after);
-        false,
+
-    )?;
+    replace_queue_entries::<SqliteClientError>(conn, &query_range, replacement, false)?;
 
     Ok(())
 }
@@ -445,7 +442,7 @@ pub(crate) fn update_chain_tip<P: consensus::Parameters>(
     // `ScanRange` uses an exclusive upper bound.
     let chain_end = new_tip + 1;
 
-    // Read the maximum height from each of the the shards tables. The minimum of the two
+    // Read the maximum height from each of the shards tables. The minimum of the two
     // gives the start of a height range that covers the last incomplete shard of both the
     // Sapling and Orchard pools.
     let sapling_shard_tip = tip_shard_end_height(conn, SAPLING_TABLES_PREFIX)?;