diff --git a/zcash_client_backend/CHANGELOG.md b/zcash_client_backend/CHANGELOG.md index d287b59f2a..e83d4c0d88 100644 --- a/zcash_client_backend/CHANGELOG.md +++ b/zcash_client_backend/CHANGELOG.md @@ -15,6 +15,8 @@ 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 methods to `zcash_client_backend::wallet::input_selection::Proposal`: + `{from_parts, min_confirmations}` - Added methods to `zcash_client_backend::wallet::ReceivedSaplingNote`: `{from_parts, txid, output_index, diversifier, rseed, note_commitment_tree_position}`. @@ -45,10 +47,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 4fd3199580..90cb0af120 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 1c3685ed03..03d94ad609 100644 --- a/zcash_client_backend/src/data_api/wallet/input_selection.rs +++ b/zcash_client_backend/src/data_api/wallet/input_selection.rs @@ -81,12 +81,39 @@ 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, } impl Proposal { + /// Constructs a [`Proposal`] from its constituent parts. + #[allow(clippy::too_many_arguments)] + pub fn from_parts( + transaction_request: TransactionRequest, + transparent_inputs: Vec, + sapling_inputs: Vec>, + balance: TransactionBalance, + fee_rule: FeeRuleT, + min_confirmations: NonZeroU32, + min_target_height: BlockHeight, + min_anchor_height: BlockHeight, + is_shielding: bool, + ) -> Self { + Self { + transaction_request, + transparent_inputs, + sapling_inputs, + balance, + fee_rule, + min_confirmations, + min_target_height, + min_anchor_height, + is_shielding, + } + } + /// Returns the transaction request that describes the payments to be made. pub fn transaction_request(&self) -> &TransactionRequest { &self.transaction_request @@ -107,6 +134,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 +430,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 +537,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 08de60b4b6..662bdaab8e 100644 --- a/zcash_client_sqlite/src/testing.rs +++ b/zcash_client_sqlite/src/testing.rs @@ -602,8 +602,7 @@ impl TestState { &mut self, usk: &UnifiedSpendingKey, ovk_policy: OvkPolicy, - proposal: Proposal, - min_confirmations: NonZeroU32, + proposal: &Proposal, ) -> Result< TxId, data_api::error::Error< @@ -617,14 +616,13 @@ impl TestState { FeeRuleT: FeeRule, { let params = self.network(); - create_proposed_transaction( + create_proposed_transaction::<_, _, InputsErrT, _>( &mut self.db_data, ¶ms, test_prover(), usk, ovk_policy, &proposal, - min_confirmations, ) } diff --git a/zcash_client_sqlite/src/wallet/sapling.rs b/zcash_client_sqlite/src/wallet/sapling.rs index c0cc7cf983..589fa969fb 100644 --- a/zcash_client_sqlite/src/wallet/sapling.rs +++ b/zcash_client_sqlite/src/wallet/sapling.rs @@ -546,26 +546,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 +684,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 +842,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 +899,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 +978,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 +1044,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 +1143,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 +1196,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(_) ); }