zcash_client_backend: Move `min_confirmations` into `Proposal`

This fixes an API issue whereby it was possible to execute a `Proposal`
with a different value of `min_confirmations` than that with which the
`Proposal` was constructed.

zcash_client_backend/CHANGELOG.md

@@ -15,6 +15,7 @@ and this library adheres to Rust's notion of
 - `zcash_client_backend::data_api::error::Error` has new error variant:
   - `Error::UnsupportedPoolType(zcash_client_backend::data_api::PoolType)`
 - Added module `zcash_client_backend::fees::standard`
+- Added `zcash_client_backend::wallet::input_selection::Proposal::min_confirmations`
 - Added methods to `zcash_client_backend::wallet::ReceivedSaplingNote`:
   `{from_parts, txid, output_index, diversifier, rseed, note_commitment_tree_position}`.
 
@@ -45,10 +46,12 @@ and this library adheres to Rust's notion of
     `change_memo` argument; instead, change memos are represented in the
     individual values of the `proposed_change` field of the `Proposal`'s
     `TransactionBalance`.
+  - `wallet::create_proposed_transaction` now takes its `proposal` argument
+    by reference instead of as an owned value.
+  - `wallet::create_proposed_transaction` no longer takes a `min_confirmations`
+    argument. Instead, `min_confirmations` is stored in the `Proposal`
   - `wallet::create_spend_to_address` now takes an additional
     `change_memo` argument.
-  - `wallet::create_proposed_transaction` now takes its `proposal` argument
-      by reference instead of as an owned value.
 - `zcash_client_backend::fees::ChangeValue::Sapling` is now a structured variant.
   In addition to the existing change value, it now also carries an optional memo
   to be associated with the change output.

zcash_client_backend/src/data_api/wallet.rs

@@ -231,15 +231,7 @@ where
         change_memo,
     )?;
 
-    create_proposed_transaction(
+    create_proposed_transaction(wallet_db, params, prover, usk, ovk_policy, &proposal)
-        wallet_db,
-        params,
-        prover,
-        usk,
-        ovk_policy,
-        &proposal,
-        min_confirmations,
-    )
 }
 
 /// Constructs a transaction that sends funds as specified by the `request` argument
@@ -332,15 +324,7 @@ where
         min_confirmations,
     )?;
 
-    create_proposed_transaction(
+    create_proposed_transaction(wallet_db, params, prover, usk, ovk_policy, &proposal)
-        wallet_db,
-        params,
-        prover,
-        usk,
-        ovk_policy,
-        &proposal,
-        min_confirmations,
-    )
 }
 
 /// Select transaction inputs, compute fees, and construct a proposal for a transaction
@@ -495,7 +479,6 @@ pub fn create_proposed_transaction<DbT, ParamsT, InputsErrT, FeeRuleT>(
     usk: &UnifiedSpendingKey,
     ovk_policy: OvkPolicy,
     proposal: &Proposal<FeeRuleT, DbT::NoteRef>,
-    min_confirmations: NonZeroU32,
 ) -> Result<
     TxId,
     Error<
@@ -546,7 +529,7 @@ where
     // are no possible transparent inputs, so we ignore those
     let mut builder = Builder::new(params.clone(), proposal.min_target_height(), None);
 
-    let checkpoint_depth = wallet_db.get_checkpoint_depth(min_confirmations)?;
+    let checkpoint_depth = wallet_db.get_checkpoint_depth(proposal.min_confirmations())?;
     wallet_db.with_sapling_tree_mut::<_, _, Error<_, _, _, _>>(|sapling_tree| {
         for selected in proposal.sapling_inputs() {
             let (note, key, merkle_path) = select_key_for_note(
@@ -815,15 +798,7 @@ where
         min_confirmations,
     )?;
 
-    create_proposed_transaction(
+    create_proposed_transaction(wallet_db, params, prover, usk, OvkPolicy::Sender, &proposal)
-        wallet_db,
-        params,
-        prover,
-        usk,
-        OvkPolicy::Sender,
-        &proposal,
-        min_confirmations,
-    )
 }
 
 #[allow(clippy::type_complexity)]

zcash_client_backend/src/data_api/wallet/input_selection.rs

@@ -81,6 +81,7 @@ pub struct Proposal<FeeRuleT, NoteRef> {
     sapling_inputs: Vec<ReceivedSaplingNote<NoteRef>>,
     balance: TransactionBalance,
     fee_rule: FeeRuleT,
+    min_confirmations: NonZeroU32,
     min_target_height: BlockHeight,
     min_anchor_height: BlockHeight,
     is_shielding: bool,
@@ -107,6 +108,10 @@ impl<FeeRuleT, NoteRef> Proposal<FeeRuleT, NoteRef> {
     pub fn fee_rule(&self) -> &FeeRuleT {
         &self.fee_rule
     }
+    /// Returns the value of `min_confirmations` used in input selection.
+    pub fn min_confirmations(&self) -> NonZeroU32 {
+        self.min_confirmations
+    }
     /// Returns the target height for which the proposal was prepared.
     ///
     /// The chain must contain at least this many blocks in order for the proposal to
@@ -399,6 +404,7 @@ where
                         sapling_inputs,
                         balance,
                         fee_rule: (*self.change_strategy.fee_rule()).clone(),
+                        min_confirmations,
                         min_target_height: target_height,
                         min_anchor_height: anchor_height,
                         is_shielding: false,
@@ -505,6 +511,7 @@ where
                 sapling_inputs: vec![],
                 balance,
                 fee_rule: (*self.change_strategy.fee_rule()).clone(),
+                min_confirmations,
                 min_target_height: target_height,
                 min_anchor_height: latest_anchor,
                 is_shielding: true,

zcash_client_sqlite/src/testing.rs

@@ -600,8 +600,7 @@ impl<Cache> TestState<Cache> {
         &mut self,
         usk: &UnifiedSpendingKey,
         ovk_policy: OvkPolicy,
-        proposal: Proposal<FeeRuleT, ReceivedNoteId>,
+        proposal: &Proposal<FeeRuleT, ReceivedNoteId>,
-        min_confirmations: NonZeroU32,
     ) -> Result<
         TxId,
         data_api::error::Error<
@@ -621,8 +620,7 @@ impl<Cache> TestState<Cache> {
             test_prover(),
             usk,
             ovk_policy,
-            &proposal,
+            proposal,
-            min_confirmations,
         )
     }
 

zcash_client_sqlite/src/wallet/sapling.rs

@@ -438,7 +438,6 @@ pub(crate) fn put_received_note<T: ReceivedSaplingOutput>(
 }
 
 #[cfg(test)]
-#[allow(deprecated)]
 pub(crate) mod tests {
     use std::{convert::Infallible, num::NonZeroU32};
 
@@ -546,26 +545,28 @@ pub(crate) mod tests {
         }])
         .unwrap();
 
+        // TODO: This test was originally written to use the pre-zip-313 fee rule
+        // and has not yet been updated.
+        #[allow(deprecated)]
         let fee_rule = StandardFeeRule::PreZip313;
+
         let change_memo = "Test change memo".parse::<Memo>().unwrap();
         let change_strategy =
             standard::SingleOutputChangeStrategy::new(fee_rule, Some(change_memo.clone().into()));
         let input_selector =
             &GreedyInputSelector::new(change_strategy, DustOutputPolicy::default());
-        let proposal_result = st.propose_transfer(
-            account,
-            input_selector,
-            request,
-            NonZeroU32::new(1).unwrap(),
-        );
-        assert_matches!(proposal_result, Ok(_));
 
-        let create_proposed_result = st.create_proposed_transaction::<Infallible, _>(
+        let proposal = st
-            &usk,
+            .propose_transfer(
-            OvkPolicy::Sender,
+                account,
-            proposal_result.unwrap(),
+                input_selector,
-            NonZeroU32::new(1).unwrap(),
+                request,
-        );
+                NonZeroU32::new(1).unwrap(),
+            )
+            .unwrap();
+
+        let create_proposed_result =
+            st.create_proposed_transaction::<Infallible, _>(&usk, OvkPolicy::Sender, &proposal);
         assert_matches!(create_proposed_result, Ok(_));
 
         let sent_tx_id = create_proposed_result.unwrap();
@@ -682,7 +683,8 @@ pub(crate) mod tests {
     }
 
     #[test]
-    fn create_to_address_fails_with_no_blocks() {
+    #[allow(deprecated)]
+    fn proposal_fails_with_no_blocks() {
         let mut st = TestBuilder::new()
             .with_test_account(AccountBirthday::from_sapling_activation)
             .build();
@@ -839,12 +841,7 @@ pub(crate) mod tests {
 
         // Executing the proposal should succeed
         let txid = st
-            .create_proposed_transaction::<Infallible, _>(
+            .create_proposed_transaction::<Infallible, _>(&usk, OvkPolicy::Sender, &proposal)
-                &usk,
-                OvkPolicy::Sender,
-                proposal,
-                min_confirmations,
-            )
             .unwrap();
 
         let (h, _) = st.generate_next_block_including(txid);
@@ -901,12 +898,7 @@ pub(crate) mod tests {
 
         // Executing the proposal should succeed
         assert_matches!(
-            st.create_proposed_transaction::<Infallible, _>(
+            st.create_proposed_transaction::<Infallible, _>(&usk, OvkPolicy::Sender, &proposal,),
-                &usk,
-                OvkPolicy::Sender,
-                proposal,
-                min_confirmations
-            ),
             Ok(_)
         );
 
@@ -985,12 +977,7 @@ pub(crate) mod tests {
             .unwrap();
 
         let txid2 = st
-            .create_proposed_transaction::<Infallible, _>(
+            .create_proposed_transaction::<Infallible, _>(&usk, OvkPolicy::Sender, &proposal)
-                &usk,
-                OvkPolicy::Sender,
-                proposal,
-                min_confirmations,
-            )
             .unwrap();
 
         let (h, _) = st.generate_next_block_including(txid2);
@@ -1056,8 +1043,7 @@ pub(crate) mod tests {
             )?;
 
             // Executing the proposal should succeed
-            let txid =
+            let txid = st.create_proposed_transaction(&usk, ovk_policy, &proposal)?;
-                st.create_proposed_transaction(&usk, ovk_policy, proposal, min_confirmations)?;
 
             // Fetch the transaction from the database
             let raw_tx: Vec<_> = st
@@ -1156,12 +1142,7 @@ pub(crate) mod tests {
 
         // Executing the proposal should succeed
         assert_matches!(
-            st.create_proposed_transaction::<Infallible, _>(
+            st.create_proposed_transaction::<Infallible, _>(&usk, OvkPolicy::Sender, &proposal),
-                &usk,
-                OvkPolicy::Sender,
-                proposal,
-                min_confirmations
-            ),
             Ok(_)
         );
     }
@@ -1214,12 +1195,7 @@ pub(crate) mod tests {
 
         // Executing the proposal should succeed
         assert_matches!(
-            st.create_proposed_transaction::<Infallible, _>(
+            st.create_proposed_transaction::<Infallible, _>(&usk, OvkPolicy::Sender, &proposal),
-                &usk,
-                OvkPolicy::Sender,
-                proposal,
-                min_confirmations
-            ),
             Ok(_)
         );
     }
@@ -1475,8 +1451,13 @@ pub(crate) mod tests {
         let res0 = st.wallet_mut().put_received_transparent_utxo(&utxo);
         assert!(matches!(res0, Ok(_)));
 
+        // TODO: This test was originally written to use the pre-zip-313 fee rule
+        // and has not yet been updated.
+        #[allow(deprecated)]
+        let fee_rule = StandardFeeRule::PreZip313;
+
         let input_selector = GreedyInputSelector::new(
-            fixed::SingleOutputChangeStrategy::new(FixedFeeRule::standard(), None),
+            standard::SingleOutputChangeStrategy::new(fee_rule, None),
             DustOutputPolicy::default(),
         );