From 7c5ac7b15139356be8df75b0dfecbe0f2822cd7c Mon Sep 17 00:00:00 2001 From: Daira-Emma Hopwood Date: Sat, 27 Jul 2024 04:44:26 +0100 Subject: [PATCH 1/3] Fix warnings when "transparent-inputs" is disabled. Signed-off-by: Daira-Emma Hopwood --- .../init/migrations/ephemeral_addresses.rs | 23 ++++++++++--------- 1 file changed, 12 insertions(+), 11 deletions(-) diff --git a/zcash_client_sqlite/src/wallet/init/migrations/ephemeral_addresses.rs b/zcash_client_sqlite/src/wallet/init/migrations/ephemeral_addresses.rs index 02df28563..b3135d896 100644 --- a/zcash_client_sqlite/src/wallet/init/migrations/ephemeral_addresses.rs +++ b/zcash_client_sqlite/src/wallet/init/migrations/ephemeral_addresses.rs @@ -81,21 +81,22 @@ impl RusqliteMigration for Migration

{ #[cfg(test)] mod tests { - use rusqlite::{named_params, Connection}; - use secrecy::{ExposeSecret, SecretVec}; - use zcash_client_backend::data_api::AccountBirthday; - use zcash_keys::keys::UnifiedSpendingKey; - use zcash_protocol::consensus::Network; - use zip32::fingerprint::SeedFingerprint; + use crate::wallet::init::migrations::tests::test_migrate; - use crate::{ - error::SqliteClientError, - wallet::{self, init::migrations::tests::test_migrate, transparent}, - AccountId, WalletDb, + #[cfg(feature = "transparent-inputs")] + use { + crate::{error::SqliteClientError, wallet, AccountId, WalletDb}, + rusqlite::{named_params, Connection}, + secrecy::{ExposeSecret, SecretVec}, + zcash_client_backend::data_api::AccountBirthday, + zcash_keys::keys::UnifiedSpendingKey, + zcash_protocol::consensus::Network, + zip32::fingerprint::SeedFingerprint, }; /// This is a minimized copy of [`wallet::create_account`] as of the time of the /// creation of this migration. + #[cfg(feature = "transparent-inputs")] fn create_account( wdb: &mut WalletDb, seed: &SecretVec, @@ -168,7 +169,7 @@ mod tests { // Initialize the `ephemeral_addresses` table. #[cfg(feature = "transparent-inputs")] - transparent::ephemeral::init_account(wdb.conn.0, &wdb.params, account_id)?; + wallet::transparent::ephemeral::init_account(wdb.conn.0, &wdb.params, account_id)?; Ok((account_id, usk)) }) From 40358f4b69ab23f5aefea875decd24cc887005c0 Mon Sep 17 00:00:00 2001 From: Daira-Emma Hopwood Date: Sat, 27 Jul 2024 04:50:07 +0100 Subject: [PATCH 2/3] Refactor `create_proposed_transaction` to return the information needed to construct a `SentTransaction`, and then call `wallet_db.store_sent_tx` outside the `create_proposed_transaction` call. This should have no semantic effect by itself, but is preparation to move where we call `store_sent_tx`. Signed-off-by: Daira-Emma Hopwood --- zcash_client_backend/src/data_api/wallet.rs | 54 +++++++++++++-------- 1 file changed, 34 insertions(+), 20 deletions(-) diff --git a/zcash_client_backend/src/data_api/wallet.rs b/zcash_client_backend/src/data_api/wallet.rs index 29486b54b..9a92a36ad 100644 --- a/zcash_client_backend/src/data_api/wallet.rs +++ b/zcash_client_backend/src/data_api/wallet.rs @@ -607,14 +607,23 @@ where #[cfg(feature = "transparent-inputs")] let mut unused_transparent_outputs = HashMap::new(); + let created = time::OffsetDateTime::now_utc(); + let account_id = wallet_db + .get_account_for_ufvk(&usk.to_unified_full_viewing_key()) + .map_err(Error::DataSource)? + .ok_or(Error::KeyNotRecognized)? + .id(); + let mut step_results = Vec::with_capacity(proposal.steps().len()); for step in proposal.steps() { - let step_result = create_proposed_transaction( + #[allow(unused_variables)] + let (step_result, outputs, fee_amount, utxos_spent) = create_proposed_transaction( wallet_db, params, spend_prover, output_prover, usk, + account_id, ovk_policy.clone(), proposal.fee_rule(), proposal.min_target_height(), @@ -623,6 +632,16 @@ where #[cfg(feature = "transparent-inputs")] &mut unused_transparent_outputs, )?; + let tx = SentTransaction { + tx: step_result.transaction(), + created, + account: account_id, + outputs, + fee_amount, + #[cfg(feature = "transparent-inputs")] + utxos_spent, + }; + wallet_db.store_sent_tx(&tx).map_err(Error::DataSource)?; step_results.push((step, step_result)); } @@ -657,6 +676,7 @@ fn create_proposed_transaction( spend_prover: &impl SpendProver, output_prover: &impl OutputProver, usk: &UnifiedSpendingKey, + account_id: ::AccountId, ovk_policy: OvkPolicy, fee_rule: &FeeRuleT, min_target_height: BlockHeight, @@ -666,7 +686,15 @@ fn create_proposed_transaction( StepOutput, (TransparentAddress, OutPoint), >, -) -> Result> +) -> Result< + ( + BuildResult, + Vec::AccountId>>, + NonNegativeAmount, + Vec, + ), + ErrorT, +> where DbT: WalletWrite + WalletCommitmentTrees, ParamsT: consensus::Parameters + Clone, @@ -708,12 +736,6 @@ where return Err(Error::ProposalNotSupported); } - let account_id = wallet_db - .get_account_for_ufvk(&usk.to_unified_full_viewing_key()) - .map_err(Error::DataSource)? - .ok_or(Error::KeyNotRecognized)? - .id(); - let (sapling_anchor, sapling_inputs) = if proposal_step.involves(PoolType::Shielded(ShieldedProtocol::Sapling)) { proposal_step.shielded_inputs().map_or_else( @@ -903,6 +925,8 @@ where } utxos_spent }; + #[cfg(not(feature = "transparent-inputs"))] + let utxos_spent = vec![]; #[cfg(feature = "orchard")] let orchard_fvk: orchard::keys::FullViewingKey = usk.orchard().into(); @@ -1278,19 +1302,9 @@ where outputs.extend(sapling_outputs); outputs.extend(transparent_outputs); - wallet_db - .store_sent_tx(&SentTransaction { - tx: build_result.transaction(), - created: time::OffsetDateTime::now_utc(), - account: account_id, - outputs, - fee_amount: proposal_step.balance().fee_required(), - #[cfg(feature = "transparent-inputs")] - utxos_spent, - }) - .map_err(Error::DataSource)?; + let fee_amount = proposal_step.balance().fee_required(); - Ok(build_result) + Ok((build_result, outputs, fee_amount, utxos_spent)) } /// Constructs a transaction that consumes available transparent UTXOs belonging to the specified From 163425add6d68e0588033a39f972f5452065b0a0 Mon Sep 17 00:00:00 2001 From: Daira-Emma Hopwood Date: Sat, 27 Jul 2024 05:29:02 +0100 Subject: [PATCH 3/3] In `create_proposed_transactions`, store the transactions only after creating all of them. This avoids undesired retransmissions in case a transaction is stored and the creation of a subsequent transaction fails. Signed-off-by: Daira-Emma Hopwood --- zcash_client_backend/src/data_api/wallet.rs | 65 ++++++++++++--------- 1 file changed, 36 insertions(+), 29 deletions(-) diff --git a/zcash_client_backend/src/data_api/wallet.rs b/zcash_client_backend/src/data_api/wallet.rs index 9a92a36ad..ef4f70e7b 100644 --- a/zcash_client_backend/src/data_api/wallet.rs +++ b/zcash_client_backend/src/data_api/wallet.rs @@ -574,6 +574,13 @@ where .map_err(Error::from) } +type StepResult = ( + BuildResult, + Vec::AccountId>>, + NonNegativeAmount, + Vec, +); + /// Construct, prove, and sign a transaction or series of transactions using the inputs supplied by /// the given proposal, and persist it to the wallet database. /// @@ -607,7 +614,6 @@ where #[cfg(feature = "transparent-inputs")] let mut unused_transparent_outputs = HashMap::new(); - let created = time::OffsetDateTime::now_utc(); let account_id = wallet_db .get_account_for_ufvk(&usk.to_unified_full_viewing_key()) .map_err(Error::DataSource)? @@ -617,7 +623,7 @@ where let mut step_results = Vec::with_capacity(proposal.steps().len()); for step in proposal.steps() { #[allow(unused_variables)] - let (step_result, outputs, fee_amount, utxos_spent) = create_proposed_transaction( + let step_result: StepResult = create_proposed_transaction( wallet_db, params, spend_prover, @@ -632,16 +638,6 @@ where #[cfg(feature = "transparent-inputs")] &mut unused_transparent_outputs, )?; - let tx = SentTransaction { - tx: step_result.transaction(), - created, - account: account_id, - outputs, - fee_amount, - #[cfg(feature = "transparent-inputs")] - utxos_spent, - }; - wallet_db.store_sent_tx(&tx).map_err(Error::DataSource)?; step_results.push((step, step_result)); } @@ -656,13 +652,31 @@ where } } - Ok(NonEmpty::from_vec( - step_results - .iter() - .map(|(_, r)| r.transaction().txid()) - .collect(), - ) - .expect("proposal.steps is NonEmpty")) + let created = time::OffsetDateTime::now_utc(); + + // Store the transactions only after creating all of them. This avoids undesired + // retransmissions in case a transaction is stored and the creation of a subsequent + // transaction fails. + let mut txids = Vec::with_capacity(proposal.steps().len()); + #[allow(unused_variables)] + for (_, (build_result, outputs, fee_amount, utxos_spent)) in step_results { + let tx = build_result.transaction(); + let sent_tx = SentTransaction { + tx, + created, + account: account_id, + outputs, + fee_amount, + #[cfg(feature = "transparent-inputs")] + utxos_spent, + }; + wallet_db + .store_sent_tx(&sent_tx) + .map_err(Error::DataSource)?; + txids.push(tx.txid()); + } + + Ok(NonEmpty::from_vec(txids).expect("proposal.steps is NonEmpty")) } // `unused_transparent_outputs` maps `StepOutput`s for transparent outputs @@ -680,21 +694,13 @@ fn create_proposed_transaction( ovk_policy: OvkPolicy, fee_rule: &FeeRuleT, min_target_height: BlockHeight, - prior_step_results: &[(&Step, BuildResult)], + prior_step_results: &[(&Step, StepResult)], proposal_step: &Step, #[cfg(feature = "transparent-inputs")] unused_transparent_outputs: &mut HashMap< StepOutput, (TransparentAddress, OutPoint), >, -) -> Result< - ( - BuildResult, - Vec::AccountId>>, - NonNegativeAmount, - Vec, - ), - ErrorT, -> +) -> Result, ErrorT> where DbT: WalletWrite + WalletCommitmentTrees, ParamsT: consensus::Parameters + Clone, @@ -910,6 +916,7 @@ where let txout = &prior_step_results[input_ref.step_index()] .1 + .0 .transaction() .transparent_bundle() .ok_or(ProposalError::ReferenceError(*input_ref))?