Apply suggestions from code review.

Co-authored by: Jack Grig <[email protected]>

zcash_client_backend/src/fees.rs

@@ -368,6 +368,8 @@ impl SplitPolicy {
     /// In the case that no other conditions provided by the user are available to fall back on,
     /// a default value of [`MARGINAL_FEE`] * 100 will be used as the "minimum usable note value"
     /// when retrieving wallet metadata.
+    ///
+    /// [`MARGINAL_FEE`]: zcash_primitives::transaction::fees::zip317::MARGINAL_FEE
     pub(crate) const MIN_NOTE_VALUE: NonNegativeAmount = NonNegativeAmount::const_from_u64(500000);
 
     /// Constructs a new [`SplitPolicy`] that splits change to ensure the given number of spendable
@@ -404,9 +406,9 @@ impl SplitPolicy {
     /// Returns the number of output notes to produce from the given total change value, given the
     /// total value and number of existing unspent notes in the account and this policy.
     ///
-    /// If splitting change to produce [`Self::target_output_count`] would result in notes of value less than
-    /// [`Self::min_split_output_value`], then this will produce
-    ///
+    /// If splitting change to produce [`Self::target_output_count`] would result in notes of value
+    /// less than [`Self::min_split_output_value`], then this will suggest a smaller number of
+    /// splits so that each resulting change note has sufficient value.
     pub fn split_count(
         &self,
         existing_notes: Option<usize>,

zcash_client_sqlite/CHANGELOG.md

@@ -15,7 +15,7 @@ and this library adheres to Rust's notion of
 - MSRV is now 1.77.0.
 - Migrated from `schemer` to our fork `schemerz`.
 - Migrated to `rusqlite 0.32`.
-- `error::SqliteClientError` has additional variant `NoteSelectorInvalid`
+- `error::SqliteClientError` has additional variant `NoteFilterInvalid`
 
 ## [0.12.2] - 2024-10-21
 

zcash_client_sqlite/src/error.rs

@@ -191,7 +191,7 @@ impl fmt::Display for SqliteClientError {
             SqliteClientError::ChainHeightUnknown => write!(f, "Chain height unknown; please call `update_chain_tip`"),
             SqliteClientError::UnsupportedPoolType(t) => write!(f, "Pool type is not currently supported: {}", t),
             SqliteClientError::BalanceError(e) => write!(f, "Balance error: {}", e),
-            SqliteClientError::NoteFilterInvalid(s) => write!(f, "Could not evaluate selection query: {:?}", s),
+            SqliteClientError::NoteFilterInvalid(s) => write!(f, "Could not evaluate filter query: {:?}", s),
             #[cfg(feature = "transparent-inputs")]
             SqliteClientError::ReachedGapLimit(account_id, bad_index) => write!(f,
                 "The proposal cannot be constructed until transactions with previously reserved ephemeral address outputs have been mined. \

zcash_client_sqlite/src/wallet/common.rs

@@ -241,7 +241,7 @@ pub(crate) fn spendable_notes_meta(
     protocol: ShieldedProtocol,
     chain_tip_height: BlockHeight,
     account: AccountId,
-    selector: &NoteFilter,
+    filter: &NoteFilter,
     exclude: &[ReceivedNoteId],
 ) -> Result<Option<PoolMeta>, SqliteClientError> {
     let (table_prefix, _, _) = per_protocol_names(protocol);
@@ -306,10 +306,10 @@ pub(crate) fn spendable_notes_meta(
         conn: &rusqlite::Connection,
         table_prefix: &str,
         account: AccountId,
-        selector: &NoteFilter,
+        filter: &NoteFilter,
         chain_tip_height: BlockHeight,
     ) -> Result<Option<NonNegativeAmount>, SqliteClientError> {
-        match selector {
+        match filter {
             NoteFilter::ExceedsMinValue(v) => Ok(Some(*v)),
             NoteFilter::ExceedsPriorSendPercentile(n) => {
                 let mut bucket_query = conn.prepare(
@@ -360,12 +360,11 @@ pub(crate) fn spendable_notes_meta(
                          AND a.ufvk IS NOT NULL
                          AND transactions.mined_height IS NOT NULL
                          AND rn.pool != :transparent_pool
-                         AND rn.id_within_pool_table NOT IN (
-                           SELECT rns.received_output_id
+                         AND (rn.pool, rn.id_within_pool_table) NOT IN (
+                           SELECT rns.pool, rns.received_output_id
                            FROM v_received_output_spends rns
                            JOIN transactions stx ON stx.id_tx = rns.transaction_id
-                           WHERE rns.pool == rn.pool
-                           AND (
+                           WHERE (
                                stx.block IS NOT NULL -- the spending tx is mined
                                OR stx.expiry_height IS NULL -- the spending tx will not expire
                                OR stx.expiry_height > :chain_tip_height -- the spending tx is unexpired
@@ -423,8 +422,7 @@ pub(crate) fn spendable_notes_meta(
 
     // TODO: Simplify the query before executing it. Not worrying about this now because queries
     // will be developer-configured, not end-user defined.
-    if let Some(min_value) =
-        min_note_value(conn, table_prefix, account, selector, chain_tip_height)?
+    if let Some(min_value) = min_note_value(conn, table_prefix, account, filter, chain_tip_height)?
     {
         let (note_count, total_value) = run_selection(min_value)?;