Merge pull request #1466 from daira/transactional-create_proposed_transactions

zcash_client_backend/src/data_api/wallet.rs

@@ -574,6 +574,13 @@ where
         .map_err(Error::from)
 }
 
+type StepResult<DbT> = (
+    BuildResult,
+    Vec<SentTransactionOutput<<DbT as WalletRead>::AccountId>>,
+    NonNegativeAmount,
+    Vec<OutPoint>,
+);
+
 /// 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,14 +614,22 @@ where
     #[cfg(feature = "transparent-inputs")]
     let mut unused_transparent_outputs = HashMap::new();
 
+    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: StepResult<DbT> = create_proposed_transaction(
             wallet_db,
             params,
             spend_prover,
             output_prover,
             usk,
+            account_id,
             ovk_policy.clone(),
             proposal.fee_rule(),
             proposal.min_target_height(),
@@ -637,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
@@ -657,16 +690,17 @@ fn create_proposed_transaction<DbT, ParamsT, InputsErrT, FeeRuleT, N>(
     spend_prover: &impl SpendProver,
     output_prover: &impl OutputProver,
     usk: &UnifiedSpendingKey,
+    account_id: <DbT as WalletRead>::AccountId,
     ovk_policy: OvkPolicy,
     fee_rule: &FeeRuleT,
     min_target_height: BlockHeight,
-    prior_step_results: &[(&Step<N>, BuildResult)],
+    prior_step_results: &[(&Step<N>, StepResult<DbT>)],
     proposal_step: &Step<N>,
     #[cfg(feature = "transparent-inputs")] unused_transparent_outputs: &mut HashMap<
         StepOutput,
         (TransparentAddress, OutPoint),
     >,
-) -> Result<BuildResult, ErrorT<DbT, InputsErrT, FeeRuleT>>
+) -> Result<StepResult<DbT>, ErrorT<DbT, InputsErrT, FeeRuleT>>
 where
     DbT: WalletWrite + WalletCommitmentTrees,
     ParamsT: consensus::Parameters + Clone,
@@ -708,12 +742,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(
@@ -888,6 +916,7 @@ where
 
             let txout = &prior_step_results[input_ref.step_index()]
                 .1
+                 .0
                 .transaction()
                 .transparent_bundle()
                 .ok_or(ProposalError::ReferenceError(*input_ref))?
@@ -903,6 +932,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 +1309,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

zcash_client_sqlite/src/wallet/init/migrations/ephemeral_addresses.rs

@@ -81,21 +81,22 @@ impl<P: consensus::Parameters> RusqliteMigration for Migration<P> {
 
 #[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<Connection, Network>,
         seed: &SecretVec<u8>,
@@ -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))
         })