diff --git a/zcash_client_backend/CHANGELOG.md b/zcash_client_backend/CHANGELOG.md index d287b59f2..bd0375093 100644 --- a/zcash_client_backend/CHANGELOG.md +++ b/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. diff --git a/zcash_client_backend/src/data_api/wallet.rs b/zcash_client_backend/src/data_api/wallet.rs index 18f2b9ab7..a5b9fef55 100644 --- a/zcash_client_backend/src/data_api/wallet.rs +++ b/zcash_client_backend/src/data_api/wallet.rs @@ -231,15 +231,7 @@ where change_memo, )?; - create_proposed_transaction( - wallet_db, - params, - prover, - usk, - ovk_policy, - &proposal, - min_confirmations, - ) + create_proposed_transaction(wallet_db, params, prover, usk, ovk_policy, &proposal) } /// Constructs a transaction that sends funds as specified by the `request` argument @@ -332,15 +324,7 @@ where min_confirmations, )?; - create_proposed_transaction( - wallet_db, - params, - prover, - usk, - ovk_policy, - &proposal, - min_confirmations, - ) + create_proposed_transaction(wallet_db, params, prover, usk, ovk_policy, &proposal) } /// Select transaction inputs, compute fees, and construct a proposal for a transaction @@ -495,7 +479,6 @@ pub fn create_proposed_transaction( usk: &UnifiedSpendingKey, ovk_policy: OvkPolicy, proposal: &Proposal, - 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( - wallet_db, - params, - prover, - usk, - OvkPolicy::Sender, - &proposal, - min_confirmations, - ) + create_proposed_transaction(wallet_db, params, prover, usk, OvkPolicy::Sender, &proposal) } #[allow(clippy::type_complexity)] diff --git a/zcash_client_backend/src/data_api/wallet/input_selection.rs b/zcash_client_backend/src/data_api/wallet/input_selection.rs index 1c3685ed0..297ab4c84 100644 --- a/zcash_client_backend/src/data_api/wallet/input_selection.rs +++ b/zcash_client_backend/src/data_api/wallet/input_selection.rs @@ -81,6 +81,7 @@ pub struct Proposal { sapling_inputs: Vec>, balance: TransactionBalance, fee_rule: FeeRuleT, + min_confirmations: NonZeroU32, min_target_height: BlockHeight, min_anchor_height: BlockHeight, is_shielding: bool, @@ -107,6 +108,10 @@ impl Proposal { 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, diff --git a/zcash_client_sqlite/src/testing.rs b/zcash_client_sqlite/src/testing.rs index ab8a0a29f..d2c6d9192 100644 --- a/zcash_client_sqlite/src/testing.rs +++ b/zcash_client_sqlite/src/testing.rs @@ -600,8 +600,7 @@ impl TestState { &mut self, usk: &UnifiedSpendingKey, ovk_policy: OvkPolicy, - proposal: Proposal, - min_confirmations: NonZeroU32, + proposal: &Proposal, ) -> Result< TxId, data_api::error::Error< @@ -621,8 +620,7 @@ impl TestState { test_prover(), usk, ovk_policy, - &proposal, - min_confirmations, + proposal, ) } diff --git a/zcash_client_sqlite/src/wallet/sapling.rs b/zcash_client_sqlite/src/wallet/sapling.rs index c0cc7cf98..129c65f5d 100644 --- a/zcash_client_sqlite/src/wallet/sapling.rs +++ b/zcash_client_sqlite/src/wallet/sapling.rs @@ -438,7 +438,6 @@ pub(crate) fn put_received_note( } #[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::().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::( - &usk, - OvkPolicy::Sender, - proposal_result.unwrap(), - NonZeroU32::new(1).unwrap(), - ); + let proposal = st + .propose_transfer( + account, + input_selector, + request, + NonZeroU32::new(1).unwrap(), + ) + .unwrap(); + + let create_proposed_result = + st.create_proposed_transaction::(&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::( - &usk, - OvkPolicy::Sender, - proposal, - min_confirmations, - ) + .create_proposed_transaction::(&usk, OvkPolicy::Sender, &proposal) .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::( - &usk, - OvkPolicy::Sender, - proposal, - min_confirmations - ), + st.create_proposed_transaction::(&usk, OvkPolicy::Sender, &proposal,), Ok(_) ); @@ -985,12 +977,7 @@ pub(crate) mod tests { .unwrap(); let txid2 = st - .create_proposed_transaction::( - &usk, - OvkPolicy::Sender, - proposal, - min_confirmations, - ) + .create_proposed_transaction::(&usk, OvkPolicy::Sender, &proposal) .unwrap(); let (h, _) = st.generate_next_block_including(txid2); @@ -1056,8 +1043,7 @@ pub(crate) mod tests { )?; // Executing the proposal should succeed - let txid = - st.create_proposed_transaction(&usk, ovk_policy, proposal, min_confirmations)?; + let txid = st.create_proposed_transaction(&usk, ovk_policy, &proposal)?; // 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::( - &usk, - OvkPolicy::Sender, - proposal, - min_confirmations - ), + st.create_proposed_transaction::(&usk, OvkPolicy::Sender, &proposal), Ok(_) ); } @@ -1214,12 +1195,7 @@ pub(crate) mod tests { // Executing the proposal should succeed assert_matches!( - st.create_proposed_transaction::( - &usk, - OvkPolicy::Sender, - proposal, - min_confirmations - ), + st.create_proposed_transaction::(&usk, OvkPolicy::Sender, &proposal), 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(), );