From fe6bea1fcea2b96890cd783191e1c39628e94b69 Mon Sep 17 00:00:00 2001
From: Kris Nuttycombe <kris@electriccoin.co>
Date: Tue, 2 Jun 2020 16:20:44 -0600
Subject: [PATCH] Check transparent input for correctness before modifying vin.

---
 zcash_extensions/src/transparent/demo.rs    | 26 +++++++++++++--------
 zcash_primitives/src/transaction/builder.rs |  7 +++---
 zcash_primitives/src/transaction/sighash.rs |  4 ++--
 3 files changed, 21 insertions(+), 16 deletions(-)

diff --git a/zcash_extensions/src/transparent/demo.rs b/zcash_extensions/src/transparent/demo.rs
index 68da1599d..db740a7e8 100644
--- a/zcash_extensions/src/transparent/demo.rs
+++ b/zcash_extensions/src/transparent/demo.rs
@@ -419,17 +419,14 @@ impl<'a, B: ExtensionTxBuilder<'a>> DemoBuilder<&mut B> {
 
 #[cfg(test)]
 mod tests {
-    use ff::{Field, PrimeField};
     use blake2b_simd::Params;
+    use ff::{Field, PrimeField};
     use rand_core::OsRng;
 
     use zcash_proofs::prover::LocalTxProver;
 
     use zcash_primitives::{
-        consensus::{
-            BranchId,
-            TestNetwork,
-        },
+        consensus::{BranchId, TestNetwork},
         extensions::transparent::{self as tze, Extension, FromPayload, ToPayload},
         legacy::TransparentAddress,
         merkle_tree::{CommitmentTree, IncrementalWitness},
@@ -445,7 +442,6 @@ mod tests {
 
     use super::{close, open, Context, DemoBuilder, Precondition, Program, Witness};
 
-
     #[test]
     fn precondition_open_round_trip() {
         let data = vec![7; 32];
@@ -673,7 +669,10 @@ mod tests {
             txn_builder: &mut builder_b,
             extension_id: 0,
         };
-        let prevout_a = (OutPoint::new(tx_a.txid().0, 0), tx_a.data.tze_outputs[0].clone());
+        let prevout_a = (
+            OutPoint::new(tx_a.txid().0, 0),
+            tx_a.data.tze_outputs[0].clone(),
+        );
         let value_xfr = Amount::from_u64(90000).unwrap();
         db_b.demo_transfer_to_close(prevout_a, value_xfr, preimage_1, preimage_2)
             .map_err(|e| format!("transfer failure: {:?}", e))
@@ -692,13 +691,20 @@ mod tests {
             txn_builder: &mut builder_c,
             extension_id: 0,
         };
-        let prevout_b = (OutPoint::new(tx_a.txid().0, 0), tx_b.data.tze_outputs[0].clone());
+        let prevout_b = (
+            OutPoint::new(tx_a.txid().0, 0),
+            tx_b.data.tze_outputs[0].clone(),
+        );
         db_c.demo_close(prevout_b, preimage_2)
             .map_err(|e| format!("close failure: {:?}", e))
             .unwrap();
 
-        builder_c.add_transparent_output(&TransparentAddress::PublicKey([0; 20]), Amount::from_u64(80000).unwrap())
-                 .unwrap();
+        builder_c
+            .add_transparent_output(
+                &TransparentAddress::PublicKey([0; 20]),
+                Amount::from_u64(80000).unwrap(),
+            )
+            .unwrap();
 
         let (tx_c, _) = builder_c
             .build(BranchId::Canopy, &prover)
diff --git a/zcash_primitives/src/transaction/builder.rs b/zcash_primitives/src/transaction/builder.rs
index 0c7f1dbf9..33a1349d8 100644
--- a/zcash_primitives/src/transaction/builder.rs
+++ b/zcash_primitives/src/transaction/builder.rs
@@ -478,8 +478,9 @@ impl<'a, P: consensus::Parameters, R: RngCore + CryptoRng> Builder<'a, P, R> {
         utxo: OutPoint,
         coin: TxOut,
     ) -> Result<(), Error> {
+        self.transparent_inputs.push(sk, coin)?;
         self.mtx.vin.push(TxIn::new(utxo));
-        self.transparent_inputs.push(sk, coin)
+        Ok(());
     }
 
     /// Adds a transparent address to send funds to.
@@ -529,9 +530,7 @@ impl<'a, P: consensus::Parameters, R: RngCore + CryptoRng> Builder<'a, P, R> {
         //
 
         // Valid change
-        let change = self.mtx.value_balance
-            - self.fee
-            + self.transparent_inputs.value_sum()
+        let change = self.mtx.value_balance - self.fee + self.transparent_inputs.value_sum()
             - self.mtx.vout.iter().map(|vo| vo.value).sum::<Amount>()
             + self
                 .tze_inputs
diff --git a/zcash_primitives/src/transaction/sighash.rs b/zcash_primitives/src/transaction/sighash.rs
index f8f0e010e..a59b235dc 100644
--- a/zcash_primitives/src/transaction/sighash.rs
+++ b/zcash_primitives/src/transaction/sighash.rs
@@ -5,8 +5,8 @@ use group::GroupEncoding;
 
 use super::{
     components::{Amount, TxOut},
-    Transaction, TransactionData, OVERWINTER_VERSION_GROUP_ID, SAPLING_TX_VERSION,
-    SAPLING_VERSION_GROUP_ID, FUTURE_VERSION_GROUP_ID,
+    Transaction, TransactionData, FUTURE_VERSION_GROUP_ID, OVERWINTER_VERSION_GROUP_ID,
+    SAPLING_TX_VERSION, SAPLING_VERSION_GROUP_ID,
 };
 use crate::{consensus, legacy::Script};