Spend dynamic-key notes first in spare input slots

librustzcash/zcash_client_backend/src/data_api.rs

@@ -1834,9 +1834,10 @@ pub trait InputSource {
     /// preserving the transaction's fee and observable shape.
     ///
     /// Implementations backed by a queryable store should minimize the funding-note count within
-    /// lock-tier preference, and return the smallest eligible notes from the funding tier as
-    /// additional candidates. The default implementation preserves the data source's ordinary
-    /// funding behavior and returns no additional candidates.
+    /// lock-tier preference, and return additional candidates from the funding tier: any notes
+    /// the store wants to retire first, then the smallest eligible notes. The default
+    /// implementation preserves the data source's ordinary funding behavior and returns no
+    /// additional candidates.
     #[allow(clippy::too_many_arguments)]
     fn select_spendable_notes_for_consolidation(
         &self,

librustzcash/zcash_client_backend/src/data_api/wallet/input_selection.rs

@@ -509,8 +509,9 @@ pub enum NoteSelection {
     /// changing the transaction's fee or observable shape.
     ///
     /// Necessary funding is not capped. When the selected pool's existing transaction shape has
-    /// no more than five actions, the selector may add the smallest eligible notes while keeping
-    /// the total selected from that pool at no more than five and preserving the existing shape.
+    /// no more than five actions, the selector may add the data source's additional candidates
+    /// (see [`InputSource::select_spendable_notes_for_consolidation`]) while keeping the total
+    /// selected from that pool at no more than five and preserving the existing shape.
     /// Pool and locked-input preferences still take precedence; if no single permitted pool can
     /// fund the payment, selection falls back to ordinary multi-pool funding. If consolidation is
     /// not possible, the funding-only proposal is returned.

librustzcash/zcash_client_sqlite/CHANGELOG.md

@@ -163,13 +163,15 @@ workspace.
   recoverable from the seed. `WalletDb` implements the backend's `DynamicIvkRead` and
   `DynamicIvkWrite`. Keys the wallet issues are trial-decrypted in every batch of
   `scan_cached_blocks_with_dynamic_ivks` until they close, and their notes spend into
-  ordinary change. Full-transaction and Enhance PIR retrieval authenticate these notes
-  with their registered key, but only a full transaction stores their memo: they carry
-  none the wallet reads, since the refund marker is on internal change. While a dynamic
-  key is open, blocks and transactions processed without the dynamic keys are refused
-  (`SqliteClientError::DynamicIvksNotUsed`). A build without `orchard` refuses to
-  open or initialize a database holding dynamic IVK state
-  (`WalletMigrationError::DynamicIvksRequireOrchard`).
+  ordinary change. With `NoteSelection::PreferConsolidation`, dynamic-key notes fill spare
+  input slots first, so ordinary sends move their value into ordinary internal change
+  without changing the fee or transaction shape. Full-transaction and Enhance PIR
+  retrieval authenticate these notes with their registered key, but only a full
+  transaction stores their memo: they carry none the wallet reads, since the refund
+  marker is on internal change. While a dynamic key is open, blocks and transactions
+  processed without the dynamic keys are refused (`SqliteClientError::DynamicIvksNotUsed`).
+  A build without `orchard` refuses to open or initialize a database holding dynamic IVK
+  state (`WalletMigrationError::DynamicIvksRequireOrchard`).
   Invalid input to these calls is `SqliteClientError::InvalidDynamicIvkInput`, and a call
   that must wait returns its `ReservationPolicy` or `SweepDeferral` as an `Ok` value.
 - Issuance: `WalletDb::reserve_refund_key` and `prepare_receive_reservation`, which require

librustzcash/zcash_client_sqlite/src/wallet/common.rs

@@ -397,8 +397,9 @@ where
     }
 }
 
-/// Selects the fewest eligible notes needed to cover `value`, followed by small notes that the
-/// input selector may use for shape-preserving consolidation.
+/// Selects the fewest eligible notes needed to cover `value`, followed by notes that the input
+/// selector may use for shape-preserving consolidation: dynamic-key notes first, so spare input
+/// slots move their value into ordinary change, then the smallest notes.
 #[allow(clippy::too_many_arguments)]
 pub(crate) fn select_spendable_notes_for_consolidation<P: consensus::Parameters, F, Note>(
     conn: &Connection,
@@ -676,7 +677,8 @@ enum ValueSelection {
     Accumulate,
     /// Accumulate larger notes first after honoring lock-tier preference.
     LargestFirst,
-    /// Return up to `limit` notes in ascending value order from the specified lock tier.
+    /// Return up to `limit` notes from the specified lock tier: dynamic-key notes first, then in
+    /// ascending value order.
     SmallestFirst { lock_tier: i64, limit: usize },
 }
 
@@ -773,6 +775,11 @@ where
         .unwrap_or(("0", "ASC"));
     let crossing_note_subquery =
         "SELECT * from eligible WHERE so_far >= :target_value ORDER BY so_far LIMIT 1";
+    let dynamic_note = if protocol == ShieldedPool::Ironwood {
+        "rn.receiving_key_id IS NOT NULL"
+    } else {
+        "0"
+    };
     let result_columns = format!(
         "id, txid, {output_index_col},
                 diversifier, value, {note_reconstruction_cols}, commitment_tree_position,
@@ -800,10 +807,12 @@ where
          FROM eligible
          ORDER BY lock_tier {tier_direction}, value DESC, commitment_tree_position, id"
         ),
+        // Dynamic-key notes come first; one that changes the change shape ends slot filling
+        // (an optional consolidation is missed, the proposal itself is unaffected).
         ValueSelection::SmallestFirst { .. } => format!(
             "SELECT {result_columns}
          FROM eligible WHERE lock_tier = :selected_lock_tier
-         ORDER BY value ASC, commitment_tree_position, id"
+         ORDER BY dynamic_note DESC, value ASC, commitment_tree_position, id"
         ),
     };
     let eligible_condition = output_eligible_condition(lock_filter, "rn");
@@ -814,6 +823,7 @@ where
                  rn.diversifier, rn.value,
                  {note_reconstruction_cols}, rn.commitment_tree_position,
                  {tier_column} AS lock_tier,
+                 {dynamic_note} AS dynamic_note,
                  {so_far} AS so_far,
                  accounts.ufvk as ufvk, rn.recipient_key_scope,
                  t.block AS mined_height,

librustzcash/zcash_client_sqlite/src/wallet/dynamic_ivk/tests/scanning.rs

@@ -208,6 +208,105 @@ fn scan_reopen_and_spend(close_keys: bool) {
     assert_eq!(all_keys(st.wallet().db(), account.id()).unwrap().len(), 2);
 }
 
+#[test]
+fn spare_input_slots_spend_dynamic_key_notes_first() {
+    use std::convert::Infallible;
+    use zcash_client_backend::{
+        data_api::wallet::{
+            ConfirmationsPolicy,
+            input_selection::{GreedyInputSelector, NoteSelection, SpendPolicy},
+        },
+        fees::{DustOutputPolicy, StandardFeeRule, standard},
+        wallet::OvkPolicy,
+    };
+    use zcash_keys::address::{Address, UnifiedAddress};
+    use zcash_protocol::ShieldedPool;
+    use zip321::{Payment, TransactionRequest};
+
+    let mut st = ironwood_wallet();
+    let network = *st.network();
+    let account = st.test_account().cloned().unwrap();
+    let ordinary = IronwoodFvk(FullViewingKey::from(account.usk().orchard()));
+    let start = st.sapling_activation_height();
+    let refund = reserve_key(
+        st.wallet_mut().db_mut(),
+        account.id(),
+        Purpose::Refund,
+        start,
+    );
+    let (first, _, _) = st.generate_next_block(
+        &ordinary,
+        AddressType::DefaultExternal,
+        Zatoshis::const_from_u64(1_000_000),
+    );
+    st.generate_next_block(
+        &IronwoodFvk(refund.full_viewing_key().clone()),
+        AddressType::DefaultExternal,
+        Zatoshis::const_from_u64(200_000),
+    );
+    st.generate_next_block(
+        &ordinary,
+        AddressType::DefaultExternal,
+        Zatoshis::const_from_u64(100_000),
+    );
+    st.scan_cached_blocks_with_dynamic_ivks(first, 3);
+    st.generate_and_scan_empty_blocks_with_dynamic_ivks(5);
+
+    // The largest note funds the payment alone. Its payment and change outputs leave one
+    // spare spend slot, which the swap note takes instead of the smaller ordinary note.
+    let receiver =
+        FullViewingKey::from(&orchard::keys::SpendingKey::from_bytes([0xf5; 32]).unwrap())
+            .address_at(0u32, Scope::External);
+    let address =
+        Address::Unified(UnifiedAddress::from_receivers(Some(receiver), None, None).unwrap());
+    let request = TransactionRequest::new(vec![Payment::without_memo(
+        address.to_zcash_address(&network),
+        Zatoshis::const_from_u64(300_000),
+    )])
+    .unwrap();
+    let change_strategy = standard::SingleOutputChangeStrategy::<TestDb>::new(
+        StandardFeeRule::Zip317,
+        None,
+        ShieldedPool::Orchard,
+        DustOutputPolicy::default(),
+    );
+    let policy = SpendPolicy::shielded_pools([ShieldedPool::Ironwood])
+        .with_note_selection(NoteSelection::PreferConsolidation);
+    let proposal = st
+        .propose_transfer_with_policy(
+            account.id(),
+            &GreedyInputSelector::new(),
+            &change_strategy,
+            request,
+            ConfirmationsPolicy::MIN,
+            &policy,
+        )
+        .unwrap();
+    let mut inputs: Vec<_> = proposal
+        .steps()
+        .first()
+        .shielded_inputs()
+        .unwrap()
+        .notes()
+        .iter()
+        .map(|note| u64::from(note.note().value()))
+        .collect();
+    inputs.sort_unstable();
+    assert_eq!(inputs, [200_000, 1_000_000]);
+
+    let created = st
+        .create_proposed_transactions::<Infallible, _, Infallible, _>(
+            account.usk(),
+            OvkPolicy::Sender,
+            &proposal,
+        )
+        .unwrap();
+    let (height, _) = st.generate_next_block_including(created[0]);
+    st.scan_cached_blocks_with_dynamic_ivks(height, 1);
+    // The dynamic-key note's value now sits in ordinary internal change.
+    assert_eq!(unspent_keys(&st, height), [None, None]);
+}
+
 #[test]
 fn dynamic_ivk_rejects_mismatched_note_metadata() {
     use incrementalmerkletree::Position;