From 145d1a57f7fc3e6af1d9594c9f350c5d882b0a80 Mon Sep 17 00:00:00 2001
From: Kris Nuttycombe <kris@nutty.land>
Date: Thu, 23 Dec 2021 13:52:30 -0700
Subject: [PATCH] Make transparent signatures commit to all input amounts &
 scripts.

Transparent UTXOs are referred as pair of previous txid and index. In
order to verify UTXO's amount in HWW (hardware wallet), whole previous
transaction containing this UTXO must be streamed into the device. This
increases complexity of signing process significantly.

zcash/zips#574 identifies this problem and suggests a modification
to ZIP-244 to resolve this issue, by adding three new fields to
section S.2 of the signature hash.
---
 .../src/transaction/components/sprout.rs      |   2 +-
 .../components/transparent/builder.rs         |  60 +++--
 zcash_primitives/src/transaction/mod.rs       |   5 +-
 zcash_primitives/src/transaction/sighash.rs   | 125 +++-------
 .../src/transaction/sighash_v4.rs             |  24 +-
 .../src/transaction/sighash_v5.rs             | 219 ++++++++++--------
 zcash_primitives/src/transaction/tests.rs     | 151 +++++++++---
 zcash_primitives/src/transaction/txid.rs      |  36 +--
 8 files changed, 367 insertions(+), 255 deletions(-)

diff --git a/zcash_primitives/src/transaction/components/sprout.rs b/zcash_primitives/src/transaction/components/sprout.rs
index b1f3215a5..79f563330 100644
--- a/zcash_primitives/src/transaction/components/sprout.rs
+++ b/zcash_primitives/src/transaction/components/sprout.rs
@@ -10,7 +10,7 @@ const PHGR_PROOF_SIZE: usize = 33 + 33 + 65 + 33 + 33 + 33 + 33 + 33;
 const ZC_NUM_JS_INPUTS: usize = 2;
 const ZC_NUM_JS_OUTPUTS: usize = 2;
 
-#[derive(Debug)]
+#[derive(Debug, Clone)]
 pub struct Bundle {
     pub joinsplits: Vec<JsDescription>,
     pub joinsplit_pubkey: [u8; 32],
diff --git a/zcash_primitives/src/transaction/components/transparent/builder.rs b/zcash_primitives/src/transaction/components/transparent/builder.rs
index da202f690..d9cd11603 100644
--- a/zcash_primitives/src/transaction/components/transparent/builder.rs
+++ b/zcash_primitives/src/transaction/components/transparent/builder.rs
@@ -6,22 +6,22 @@ use std::fmt;
 use blake2b_simd::Hash as Blake2bHash;
 
 use crate::{
-    legacy::TransparentAddress,
-    transaction::components::{
-        amount::Amount,
-        transparent::{self, Authorization, Authorized, Bundle, TxIn, TxOut},
+    legacy::{Script, TransparentAddress},
+    transaction::{
+        components::{
+            amount::Amount,
+            transparent::{self, Authorization, Authorized, Bundle, TxIn, TxOut},
+        },
+        sighash::TransparentAuthorizingContext,
     },
 };
 
 #[cfg(feature = "transparent-inputs")]
-use crate::{
-    legacy::Script,
-    transaction::{
-        self as tx,
-        components::OutPoint,
-        sighash::{signature_hash, SignableInput, SIGHASH_ALL},
-        TransactionData, TxDigests,
-    },
+use crate::transaction::{
+    self as tx,
+    components::OutPoint,
+    sighash::{signature_hash, SignableInput, SIGHASH_ALL},
+    TransactionData, TxDigests,
 };
 
 #[derive(Debug, PartialEq)]
@@ -188,6 +188,36 @@ impl TxIn<Unauthorized> {
     }
 }
 
+#[cfg(not(feature = "transparent-inputs"))]
+impl TransparentAuthorizingContext for Unauthorized {
+    fn input_amounts(&self) -> Vec<Amount> {
+        vec![]
+    }
+
+    fn input_scripts(&self) -> Vec<Script> {
+        vec![]
+    }
+}
+
+#[cfg(feature = "transparent-inputs")]
+impl TransparentAuthorizingContext for Unauthorized {
+    fn input_amounts(&self) -> Vec<Amount> {
+        return self
+            .inputs
+            .iter()
+            .map(|txin| txin.coin.value.clone())
+            .collect();
+    }
+
+    fn input_scripts(&self) -> Vec<Script> {
+        return self
+            .inputs
+            .iter()
+            .map(|txin| txin.coin.script_pubkey.clone())
+            .collect();
+    }
+}
+
 impl Bundle<Unauthorized> {
     pub fn apply_signatures(
         self,
@@ -204,7 +234,11 @@ impl Bundle<Unauthorized> {
                 let sighash = signature_hash(
                     mtx,
                     SIGHASH_ALL,
-                    &SignableInput::transparent(i, &info.coin.script_pubkey, info.coin.value),
+                    &SignableInput::Transparent {
+                        index: i,
+                        script_code: &info.coin.script_pubkey,
+                        value: info.coin.value,
+                    },
                     txid_parts_cache,
                 );
 
diff --git a/zcash_primitives/src/transaction/mod.rs b/zcash_primitives/src/transaction/mod.rs
index 25755c38d..a834d1de9 100644
--- a/zcash_primitives/src/transaction/mod.rs
+++ b/zcash_primitives/src/transaction/mod.rs
@@ -330,6 +330,10 @@ impl<A: Authorization> TransactionData<A> {
         self.consensus_branch_id
     }
 
+    pub fn lock_time(&self) -> u32 {
+        self.lock_time
+    }
+
     pub fn expiry_height(&self) -> BlockHeight {
         self.expiry_height
     }
@@ -927,7 +931,6 @@ pub struct TransparentDigests<A> {
     pub prevout_digest: A,
     pub sequence_digest: A,
     pub outputs_digest: A,
-    pub per_input_digest: Option<A>,
 }
 
 #[derive(Clone)]
diff --git a/zcash_primitives/src/transaction/sighash.rs b/zcash_primitives/src/transaction/sighash.rs
index 0c928fd0f..e0c2a209e 100644
--- a/zcash_primitives/src/transaction/sighash.rs
+++ b/zcash_primitives/src/transaction/sighash.rs
@@ -5,7 +5,7 @@ use std::convert::TryInto;
 use super::{
     components::{
         sapling::{self, GrothProofBytes},
-        Amount,
+        transparent, Amount,
     },
     sighash_v4::v4_signature_hash,
     sighash_v5::v5_signature_hash,
@@ -15,94 +15,25 @@ use super::{
 #[cfg(feature = "zfuture")]
 use crate::extensions::transparent::Precondition;
 
-pub const SIGHASH_ALL: u32 = 1;
-pub const SIGHASH_NONE: u32 = 2;
-pub const SIGHASH_SINGLE: u32 = 3;
-pub const SIGHASH_MASK: u32 = 0x1f;
-pub const SIGHASH_ANYONECANPAY: u32 = 0x80;
-
-pub struct TransparentInput<'a> {
-    index: usize,
-    script_code: &'a Script,
-    value: Amount,
-}
-
-impl<'a> TransparentInput<'a> {
-    pub fn new(index: usize, script_code: &'a Script, value: Amount) -> Self {
-        TransparentInput {
-            index,
-            script_code,
-            value,
-        }
-    }
-
-    pub fn index(&self) -> usize {
-        self.index
-    }
-
-    pub fn script_code(&self) -> &'a Script {
-        self.script_code
-    }
-
-    pub fn value(&self) -> Amount {
-        self.value
-    }
-}
-
-#[cfg(feature = "zfuture")]
-pub struct TzeInput<'a> {
-    index: usize,
-    precondition: &'a Precondition,
-    value: Amount,
-}
-
-#[cfg(feature = "zfuture")]
-impl<'a> TzeInput<'a> {
-    pub fn new(index: usize, precondition: &'a Precondition, value: Amount) -> Self {
-        TzeInput {
-            index,
-            precondition,
-            value,
-        }
-    }
-
-    pub fn index(&self) -> usize {
-        self.index
-    }
-
-    pub fn precondition(&self) -> &'a Precondition {
-        self.precondition
-    }
-
-    pub fn value(&self) -> Amount {
-        self.value
-    }
-}
+pub const SIGHASH_ALL: u8 = 0x01;
+pub const SIGHASH_NONE: u8 = 0x02;
+pub const SIGHASH_SINGLE: u8 = 0x03;
+pub const SIGHASH_MASK: u8 = 0x1f;
+pub const SIGHASH_ANYONECANPAY: u8 = 0x80;
 
 pub enum SignableInput<'a> {
     Shielded,
-    Transparent(TransparentInput<'a>),
+    Transparent {
+        index: usize,
+        script_code: &'a Script,
+        value: Amount,
+    },
     #[cfg(feature = "zfuture")]
-    Tze(TzeInput<'a>),
-}
-
-impl<'a> SignableInput<'a> {
-    pub fn transparent(index: usize, script_code: &'a Script, value: Amount) -> Self {
-        SignableInput::Transparent(TransparentInput {
-            index,
-            script_code,
-            value,
-        })
-    }
-
-    #[cfg(feature = "zfuture")]
-    pub fn tze(index: usize, precondition: &'a Precondition, value: Amount) -> Self {
-        SignableInput::Tze(TzeInput {
-            index,
-            precondition,
-            value,
-        })
-    }
+    Tze {
+        index: usize,
+        precondition: &'a Precondition,
+        value: Amount,
+    },
 }
 
 pub struct SignatureHash(Blake2bHash);
@@ -113,13 +44,33 @@ impl AsRef<[u8; 32]> for SignatureHash {
     }
 }
 
+/// Addtional context that is needed to compute signature hashes
+/// for transactions that include transparent inputs or outputs.
+pub trait TransparentAuthorizingContext: transparent::Authorization {
+    /// Returns the list of all transparent input amounts, provided
+    /// so that wallets can commit to the transparent input breakdown
+    /// without requiring the full data of the previous transactions
+    /// providing these inputs.
+    fn input_amounts(&self) -> Vec<Amount>;
+    /// Returns the list of all transparent input scripts, provided
+    /// so that wallets can commit to the transparent input breakdown
+    /// without requiring the full data of the previous transactions
+    /// providing these inputs.
+    fn input_scripts(&self) -> Vec<Script>;
+}
+
+/// Computes the signature hash for an input to a transaction, given
+/// the full data of the transaction, the input being signed, and the
+/// set of precomputed hashes produced in the construction of the
+/// transaction ID.
 pub fn signature_hash<
     'a,
+    TA: TransparentAuthorizingContext,
     SA: sapling::Authorization<Proof = GrothProofBytes>,
-    A: Authorization<SaplingAuth = SA>,
+    A: Authorization<SaplingAuth = SA, TransparentAuth = TA>,
 >(
     tx: &TransactionData<A>,
-    hash_type: u32,
+    hash_type: u8,
     signable_input: &SignableInput<'a>,
     txid_parts: &TxDigests<Blake2bHash>,
 ) -> SignatureHash {
diff --git a/zcash_primitives/src/transaction/sighash_v4.rs b/zcash_primitives/src/transaction/sighash_v4.rs
index 575e28e40..b0b8a68b2 100644
--- a/zcash_primitives/src/transaction/sighash_v4.rs
+++ b/zcash_primitives/src/transaction/sighash_v4.rs
@@ -140,7 +140,7 @@ pub fn v4_signature_hash<
     A: Authorization<SaplingAuth = SA>,
 >(
     tx: &TransactionData<A>,
-    hash_type: u32,
+    hash_type: u8,
     signable_input: &SignableInput<'_>,
 ) -> Blake2bHash {
     if tx.version.has_overwinter() {
@@ -192,8 +192,8 @@ pub fn v4_signature_hash<
             );
         } else if (hash_type & SIGHASH_MASK) == SIGHASH_SINGLE {
             match (tx.transparent_bundle.as_ref(), signable_input) {
-                (Some(b), SignableInput::Transparent(input)) if input.index() < b.vout.len() => {
-                    h.update(single_output_hash(&b.vout[input.index()]).as_bytes())
+                (Some(b), SignableInput::Transparent { index, .. }) if index < &b.vout.len() => {
+                    h.update(single_output_hash(&b.vout[*index]).as_bytes())
                 }
                 _ => h.update(&[0; 32]),
             };
@@ -237,18 +237,22 @@ pub fn v4_signature_hash<
         if tx.version.has_sapling() {
             h.update(&tx.sapling_value_balance().to_i64_le_bytes());
         }
-        update_u32!(h, hash_type, tmp);
+        update_u32!(h, hash_type.into(), tmp);
 
         match signable_input {
             SignableInput::Shielded => (),
-            SignableInput::Transparent(input) => {
+            SignableInput::Transparent {
+                index,
+                script_code,
+                value,
+            } => {
                 if let Some(bundle) = tx.transparent_bundle.as_ref() {
                     let mut data = vec![];
-                    bundle.vin[input.index()].prevout.write(&mut data).unwrap();
-                    input.script_code().write(&mut data).unwrap();
-                    data.extend_from_slice(&input.value().to_i64_le_bytes());
+                    bundle.vin[*index].prevout.write(&mut data).unwrap();
+                    script_code.write(&mut data).unwrap();
+                    data.extend_from_slice(&value.to_i64_le_bytes());
                     (&mut data)
-                        .write_u32::<LittleEndian>(bundle.vin[input.index()].sequence)
+                        .write_u32::<LittleEndian>(bundle.vin[*index].sequence)
                         .unwrap();
                     h.update(&data);
                 } else {
@@ -259,7 +263,7 @@ pub fn v4_signature_hash<
             }
 
             #[cfg(feature = "zfuture")]
-            SignableInput::Tze(_) => {
+            SignableInput::Tze { .. } => {
                 panic!("A request has been made to sign a TZE input, but the transaction version is not ZFuture");
             }
         }
diff --git a/zcash_primitives/src/transaction/sighash_v5.rs b/zcash_primitives/src/transaction/sighash_v5.rs
index 286192e18..701f4ba65 100644
--- a/zcash_primitives/src/transaction/sighash_v5.rs
+++ b/zcash_primitives/src/transaction/sighash_v5.rs
@@ -2,15 +2,17 @@ use std::io::Write;
 
 use blake2b_simd::{Hash as Blake2bHash, Params, State};
 use byteorder::{LittleEndian, WriteBytesExt};
+use zcash_encoding::Array;
 
 use crate::transaction::{
     components::transparent::{self, TxOut},
     sighash::{
-        SignableInput, TransparentInput, SIGHASH_ANYONECANPAY, SIGHASH_MASK, SIGHASH_NONE,
-        SIGHASH_SINGLE,
+        SignableInput, TransparentAuthorizingContext, SIGHASH_ANYONECANPAY, SIGHASH_MASK,
+        SIGHASH_NONE, SIGHASH_SINGLE,
     },
     txid::{
-        to_hash, transparent_outputs_hash, transparent_prevout_hash, transparent_sequence_hash,
+        hash_transparent_txid_data, to_hash, transparent_outputs_hash, transparent_prevout_hash,
+        transparent_sequence_hash, ZCASH_TRANSPARENT_HASH_PERSONALIZATION,
     },
     Authorization, TransactionData, TransparentDigests, TxDigests,
 };
@@ -22,9 +24,11 @@ use std::convert::TryInto;
 use zcash_encoding::{CompactSize, Vector};
 
 #[cfg(feature = "zfuture")]
-use crate::transaction::{components::tze, sighash::TzeInput, TzeDigests};
+use crate::transaction::{components::tze, TzeDigests};
 
 const ZCASH_TRANSPARENT_INPUT_HASH_PERSONALIZATION: &[u8; 16] = b"Zcash___TxInHash";
+const ZCASH_TRANSPARENT_AMOUNTS_HASH_PERSONALIZATION: &[u8; 16] = b"ZTxTrAmountsHash";
+const ZCASH_TRANSPARENT_SCRIPTS_HASH_PERSONALIZATION: &[u8; 16] = b"ZTxTrScriptsHash";
 
 #[cfg(feature = "zfuture")]
 const ZCASH_TZE_INPUT_HASH_PERSONALIZATION: &[u8; 16] = b"Zcash__TzeInHash";
@@ -33,12 +37,13 @@ fn hasher(personal: &[u8; 16]) -> State {
     Params::new().hash_length(32).personal(personal).to_state()
 }
 
-fn transparent_input_sigdigests<A: transparent::Authorization>(
-    bundle: &transparent::Bundle<A>,
-    input: &TransparentInput<'_>,
+/// Implements [ZIP 244 section S.2](https://zips.z.cash/zip-0244#s-2-transparent-sig-digest)
+fn transparent_sig_digest<A: TransparentAuthorizingContext>(
     txid_digests: &TransparentDigests<Blake2bHash>,
-    hash_type: u32,
-) -> TransparentDigests<Blake2bHash> {
+    bundle: &transparent::Bundle<A>,
+    input: &SignableInput<'_>,
+    hash_type: u8,
+) -> Blake2bHash {
     let flag_anyonecanpay = hash_type & SIGHASH_ANYONECANPAY != 0;
     let flag_single = hash_type & SIGHASH_MASK == SIGHASH_SINGLE;
     let flag_none = hash_type & SIGHASH_MASK == SIGHASH_NONE;
@@ -49,20 +54,54 @@ fn transparent_input_sigdigests<A: transparent::Authorization>(
         txid_digests.prevout_digest
     };
 
-    let sequence_digest = if flag_anyonecanpay || flag_single || flag_none {
+    let amounts_digest = {
+        let mut h = hasher(ZCASH_TRANSPARENT_AMOUNTS_HASH_PERSONALIZATION);
+        Array::write(
+            &mut h,
+            if flag_anyonecanpay {
+                vec![]
+            } else {
+                bundle.authorization.input_amounts()
+            },
+            |w, amount| w.write_all(&amount.to_i64_le_bytes()),
+        )
+        .unwrap();
+        h.finalize()
+    };
+
+    let scripts_digest = {
+        let mut h = hasher(ZCASH_TRANSPARENT_SCRIPTS_HASH_PERSONALIZATION);
+        Array::write(
+            &mut h,
+            if flag_anyonecanpay {
+                vec![]
+            } else {
+                bundle.authorization.input_scripts()
+            },
+            |w, script| script.write(w),
+        )
+        .unwrap();
+        h.finalize()
+    };
+
+    let sequence_digest = if flag_anyonecanpay {
         transparent_sequence_hash::<A>(&[])
     } else {
         txid_digests.sequence_digest
     };
 
-    let outputs_digest = if flag_single {
-        if input.index() < bundle.vout.len() {
-            transparent_outputs_hash(&[&bundle.vout[input.index()]])
-        } else {
+    let outputs_digest = if let SignableInput::Transparent { index, .. } = input {
+        if flag_single {
+            if *index < bundle.vout.len() {
+                transparent_outputs_hash(&[&bundle.vout[*index]])
+            } else {
+                transparent_outputs_hash::<TxOut>(&[])
+            }
+        } else if flag_none {
             transparent_outputs_hash::<TxOut>(&[])
+        } else {
+            txid_digests.outputs_digest
         }
-    } else if flag_none {
-        transparent_outputs_hash::<TxOut>(&[])
     } else {
         txid_digests.outputs_digest
     };
@@ -73,41 +112,51 @@ fn transparent_input_sigdigests<A: transparent::Authorization>(
     //   c. value of the output spent by this input (8-byte little endian)
     //   d. nSequence of the input (4-byte little endian)
     let mut ch = hasher(ZCASH_TRANSPARENT_INPUT_HASH_PERSONALIZATION);
-    let txin = &bundle.vin[input.index()];
-    txin.prevout.write(&mut ch).unwrap();
-    input.script_code().write(&mut ch).unwrap();
-    ch.write_all(&input.value().to_i64_le_bytes()).unwrap();
-    ch.write_u32::<LittleEndian>(txin.sequence).unwrap();
+    if let SignableInput::Transparent {
+        index,
+        script_code,
+        value,
+    } = input
+    {
+        let txin = &bundle.vin[*index];
+        txin.prevout.write(&mut ch).unwrap();
+        ch.write_all(&value.to_i64_le_bytes()).unwrap();
+        script_code.write(&mut ch).unwrap();
+        ch.write_u32::<LittleEndian>(txin.sequence).unwrap();
+    }
     let per_input_digest = ch.finalize();
 
-    TransparentDigests {
-        prevout_digest,
-        sequence_digest,
-        outputs_digest,
-        per_input_digest: Some(per_input_digest),
-    }
+    let mut h = hasher(ZCASH_TRANSPARENT_HASH_PERSONALIZATION);
+    h.write_all(&[hash_type]).unwrap();
+    h.write_all(prevout_digest.as_bytes()).unwrap();
+    h.write_all(amounts_digest.as_bytes()).unwrap();
+    h.write_all(scripts_digest.as_bytes()).unwrap();
+    h.write_all(sequence_digest.as_bytes()).unwrap();
+    h.write_all(outputs_digest.as_bytes()).unwrap();
+    h.write_all(per_input_digest.as_bytes()).unwrap();
+    h.finalize()
 }
 
 #[cfg(feature = "zfuture")]
 fn tze_input_sigdigests<A: tze::Authorization>(
     bundle: &tze::Bundle<A>,
-    input: &TzeInput<'_>,
+    input: &SignableInput<'_>,
     txid_digests: &TzeDigests<Blake2bHash>,
 ) -> TzeDigests<Blake2bHash> {
     let mut ch = hasher(ZCASH_TZE_INPUT_HASH_PERSONALIZATION);
-    let tzein = &bundle.vin[input.index()];
-    tzein.prevout.write(&mut ch).unwrap();
-    CompactSize::write(
-        &mut ch,
-        input.precondition().extension_id.try_into().unwrap(),
-    )
-    .unwrap();
-    CompactSize::write(&mut ch, input.precondition().mode.try_into().unwrap()).unwrap();
-    Vector::write(&mut ch, &input.precondition().payload, |w, e| {
-        w.write_u8(*e)
-    })
-    .unwrap();
-    ch.write_all(&input.value().to_i64_le_bytes()).unwrap();
+    if let SignableInput::Tze {
+        index,
+        precondition,
+        value,
+    } = input
+    {
+        let tzein = &bundle.vin[*index];
+        tzein.prevout.write(&mut ch).unwrap();
+        CompactSize::write(&mut ch, precondition.extension_id.try_into().unwrap()).unwrap();
+        CompactSize::write(&mut ch, precondition.mode.try_into().unwrap()).unwrap();
+        Vector::write(&mut ch, &precondition.payload, |w, e| w.write_u8(*e)).unwrap();
+        ch.write_all(&value.to_i64_le_bytes()).unwrap();
+    }
     let per_input_digest = ch.finalize();
 
     TzeDigests {
@@ -117,66 +166,40 @@ fn tze_input_sigdigests<A: tze::Authorization>(
     }
 }
 
-pub fn v5_signature_hash<A: Authorization>(
+/// Implements the [Signature Digest section of ZIP 244](https://zips.z.cash/zip-0244#signature-digest)
+pub fn v5_signature_hash<
+    TA: TransparentAuthorizingContext,
+    A: Authorization<TransparentAuth = TA>,
+>(
     tx: &TransactionData<A>,
-    hash_type: u32,
+    hash_type: u8,
     signable_input: &SignableInput<'_>,
     txid_parts: &TxDigests<Blake2bHash>,
 ) -> Blake2bHash {
-    match signable_input {
-        SignableInput::Shielded => to_hash(
-            tx.version,
-            tx.consensus_branch_id,
-            txid_parts.header_digest,
-            txid_parts.transparent_digests.as_ref(),
-            txid_parts.sapling_digest,
-            txid_parts.orchard_digest,
-            #[cfg(feature = "zfuture")]
-            txid_parts.tze_digests.as_ref(),
-        ),
-        SignableInput::Transparent(input) => {
-            if let Some((bundle, txid_digests)) = tx
-                .transparent_bundle
-                .as_ref()
-                .zip(txid_parts.transparent_digests.as_ref())
-            {
-                to_hash(
-                    tx.version,
-                    tx.consensus_branch_id,
-                    txid_parts.header_digest,
-                    Some(&transparent_input_sigdigests(
-                        bundle,
-                        input,
-                        txid_digests,
-                        hash_type,
-                    )),
-                    txid_parts.sapling_digest,
-                    txid_parts.orchard_digest,
-                    #[cfg(feature = "zfuture")]
-                    txid_parts.tze_digests.as_ref(),
-                )
-            } else {
-                panic!("Transaction has no transparent inputs to sign.")
-            }
-        }
+    to_hash(
+        tx.version,
+        tx.consensus_branch_id,
+        txid_parts.header_digest,
+        if let Some(bundle) = &tx.transparent_bundle {
+            transparent_sig_digest(
+                txid_parts
+                    .transparent_digests
+                    .as_ref()
+                    .expect("Transparent txid digests are missing."),
+                &bundle,
+                signable_input,
+                hash_type,
+            )
+        } else {
+            hash_transparent_txid_data(txid_parts.transparent_digests.as_ref())
+        },
+        txid_parts.sapling_digest,
+        txid_parts.orchard_digest,
         #[cfg(feature = "zfuture")]
-        SignableInput::Tze(input) => {
-            if let Some((bundle, txid_digests)) =
-                tx.tze_bundle.as_ref().zip(txid_parts.tze_digests.as_ref())
-            {
-                to_hash(
-                    tx.version,
-                    tx.consensus_branch_id,
-                    txid_parts.header_digest,
-                    txid_parts.transparent_digests.as_ref(),
-                    txid_parts.sapling_digest,
-                    txid_parts.orchard_digest,
-                    #[cfg(feature = "zfuture")]
-                    Some(&tze_input_sigdigests(bundle, input, txid_digests)),
-                )
-            } else {
-                panic!("Transaction has no TZE inputs to sign.")
-            }
-        }
-    }
+        tx.tze_bundle
+            .as_ref()
+            .zip(txid_parts.tze_digests.as_ref())
+            .map(|(bundle, tze_digests)| tze_input_sigdigests(bundle, signable_input, tze_digests))
+            .as_ref(),
+    )
 }
diff --git a/zcash_primitives/src/transaction/tests.rs b/zcash_primitives/src/transaction/tests.rs
index f5533213f..8f46aa158 100644
--- a/zcash_primitives/src/transaction/tests.rs
+++ b/zcash_primitives/src/transaction/tests.rs
@@ -1,3 +1,4 @@
+use blake2b_simd::Hash as Blake2bHash;
 use std::ops::Deref;
 
 use proptest::prelude::*;
@@ -6,14 +7,19 @@ use crate::{consensus::BranchId, legacy::Script};
 
 use super::{
     components::Amount,
+    sapling,
     sighash::{SignableInput, SIGHASH_ALL, SIGHASH_ANYONECANPAY, SIGHASH_NONE, SIGHASH_SINGLE},
     sighash_v4::v4_signature_hash,
     sighash_v5::v5_signature_hash,
     testing::arb_tx,
+    transparent::{self, builder::AuthorizingContext},
     txid::TxIdDigester,
-    Transaction,
+    Authorization, Transaction, TransactionData, TxDigests, TxIn,
 };
 
+#[cfg(feature = "zfuture")]
+use super::components::tze;
+
 #[test]
 fn tx_read_write() {
     let data = &self::data::tx_read_write::TX_READ_WRITE;
@@ -120,16 +126,16 @@ fn zip_0143() {
     for tv in self::data::zip_0143::make_test_vectors() {
         let tx = Transaction::read(&tv.tx[..], tv.consensus_branch_id).unwrap();
         let signable_input = match tv.transparent_input {
-            Some(n) => SignableInput::transparent(
-                n as usize,
-                &tv.script_code,
-                Amount::from_nonnegative_i64(tv.amount).unwrap(),
-            ),
+            Some(n) => SignableInput::Transparent {
+                index: n as usize,
+                script_code: &tv.script_code,
+                value: Amount::from_nonnegative_i64(tv.amount).unwrap(),
+            },
             _ => SignableInput::Shielded,
         };
 
         assert_eq!(
-            v4_signature_hash(tx.deref(), tv.hash_type, &signable_input).as_ref(),
+            v4_signature_hash(tx.deref(), tv.hash_type as u8, &signable_input).as_ref(),
             tv.sighash
         );
     }
@@ -140,59 +146,145 @@ fn zip_0243() {
     for tv in self::data::zip_0243::make_test_vectors() {
         let tx = Transaction::read(&tv.tx[..], tv.consensus_branch_id).unwrap();
         let signable_input = match tv.transparent_input {
-            Some(n) => SignableInput::transparent(
-                n as usize,
-                &tv.script_code,
-                Amount::from_nonnegative_i64(tv.amount).unwrap(),
-            ),
+            Some(n) => SignableInput::Transparent {
+                index: n as usize,
+                script_code: &tv.script_code,
+                value: Amount::from_nonnegative_i64(tv.amount).unwrap(),
+            },
             _ => SignableInput::Shielded,
         };
 
         assert_eq!(
-            v4_signature_hash(tx.deref(), tv.hash_type, &signable_input).as_ref(),
+            v4_signature_hash(tx.deref(), tv.hash_type as u8, &signable_input).as_ref(),
             tv.sighash
         );
     }
 }
 
+#[derive(Debug)]
+struct TestTransparentUnauthorized {
+    input_amounts: Vec<Amount>,
+    input_scripts: Vec<Script>,
+}
+
+impl transparent::Authorization for TestTransparentUnauthorized {
+    type ScriptSig = ();
+}
+
+impl AuthorizingContext for TestTransparentUnauthorized {
+    #[cfg(feature = "transparent-inputs")]
+    fn input_amounts(&self) -> Vec<Amount> {
+        self.input_amounts.clone();
+    }
+
+    #[cfg(feature = "transparent-inputs")]
+    fn input_scripts(&self) -> Vec<Script> {
+        self.input_scripts.clone()
+    }
+}
+
+struct TestUnauthorized;
+
+impl Authorization for TestUnauthorized {
+    type TransparentAuth = TestTransparentUnauthorized;
+    type SaplingAuth = sapling::Authorized;
+    type OrchardAuth = orchard::bundle::Authorized;
+
+    #[cfg(feature = "zfuture")]
+    type TzeAuth = tze::Authorized;
+}
+
 #[test]
 fn zip_0244() {
-    for tv in self::data::zip_0244::make_test_vectors() {
+    fn to_test_txdata(
+        tv: &self::data::zip_0244::TestVector,
+    ) -> (TransactionData<TestUnauthorized>, TxDigests<Blake2bHash>) {
         let tx = Transaction::read(&tv.tx[..], BranchId::Nu5).unwrap();
+
         assert_eq!(tx.txid.as_ref(), &tv.txid);
         assert_eq!(tx.auth_commitment().as_ref(), &tv.auth_digest);
 
-        let txid_parts = tx.deref().digest(TxIdDigester);
+        let txdata = tx.deref();
+
+        let input_amounts = if tv.transparent_input.is_some() {
+            vec![Amount::from_nonnegative_i64(tv.amount.unwrap()).unwrap()]
+        } else {
+            vec![]
+        };
+        let input_scripts = if tv.transparent_input.is_some() {
+            vec![Script(tv.script_code.clone().unwrap())]
+        } else {
+            vec![]
+        };
+
+        let test_bundle = txdata
+            .transparent_bundle
+            .as_ref()
+            .map(|b| transparent::Bundle {
+                vin: b
+                    .vin
+                    .iter()
+                    .map(|v| TxIn {
+                        prevout: v.prevout.clone(),
+                        script_sig: (),
+                        sequence: v.sequence,
+                    })
+                    .collect(),
+                vout: b.vout.clone(),
+                authorization: TestTransparentUnauthorized {
+                    input_amounts,
+                    input_scripts,
+                },
+            });
+
+        (
+            TransactionData::from_parts(
+                txdata.version(),
+                txdata.consensus_branch_id(),
+                txdata.lock_time(),
+                txdata.expiry_height(),
+                test_bundle,
+                txdata.sprout_bundle().cloned(),
+                txdata.sapling_bundle().cloned(),
+                txdata.orchard_bundle().cloned(),
+                #[cfg(feature = "zfuture")]
+                txdata.tze_bundle().cloned(),
+            ),
+            txdata.digest(TxIdDigester),
+        )
+    }
+
+    for tv in self::data::zip_0244::make_test_vectors() {
+        let (txdata, txid_parts) = to_test_txdata(&tv);
+
         match tv.transparent_input {
             Some(n) => {
                 let script = Script(tv.script_code.unwrap());
-                let signable_input = SignableInput::transparent(
-                    n as usize,
-                    &script,
-                    Amount::from_nonnegative_i64(tv.amount.unwrap()).unwrap(),
-                );
+                let signable_input = SignableInput::Transparent {
+                    index: n as usize,
+                    script_code: &script,
+                    value: Amount::from_nonnegative_i64(tv.amount.unwrap()).unwrap(),
+                };
 
                 assert_eq!(
-                    v5_signature_hash(tx.deref(), SIGHASH_ALL, &signable_input, &txid_parts)
-                        .as_ref(),
+                    v5_signature_hash(&txdata, SIGHASH_ALL, &signable_input, &txid_parts).as_ref(),
                     &tv.sighash_all
                 );
 
                 assert_eq!(
-                    v5_signature_hash(tx.deref(), SIGHASH_NONE, &signable_input, &txid_parts)
-                        .as_ref(),
+                    v5_signature_hash(&txdata, SIGHASH_NONE, &signable_input, &txid_parts).as_ref(),
                     &tv.sighash_none.unwrap()
                 );
 
                 assert_eq!(
-                    v5_signature_hash(tx.deref(), SIGHASH_SINGLE, &signable_input, &txid_parts)
+                    v5_signature_hash(&txdata, SIGHASH_SINGLE, &signable_input, &txid_parts)
                         .as_ref(),
                     &tv.sighash_single.unwrap()
                 );
 
                 assert_eq!(
                     v5_signature_hash(
-                        tx.deref(),
+                        &txdata,
                         SIGHASH_ALL | SIGHASH_ANYONECANPAY,
                         &signable_input,
                         &txid_parts,
@@ -203,7 +295,7 @@ fn zip_0244() {
 
                 assert_eq!(
                     v5_signature_hash(
-                        tx.deref(),
+                        &txdata,
                         SIGHASH_NONE | SIGHASH_ANYONECANPAY,
                         &signable_input,
                         &txid_parts,
@@ -214,7 +306,7 @@ fn zip_0244() {
 
                 assert_eq!(
                     v5_signature_hash(
-                        tx.deref(),
+                        &txdata,
                         SIGHASH_SINGLE | SIGHASH_ANYONECANPAY,
                         &signable_input,
                         &txid_parts,
@@ -227,8 +319,7 @@ fn zip_0244() {
                 let signable_input = SignableInput::Shielded;
 
                 assert_eq!(
-                    v5_signature_hash(tx.deref(), SIGHASH_ALL, &signable_input, &txid_parts)
-                        .as_ref(),
+                    v5_signature_hash(&txdata, SIGHASH_ALL, &signable_input, &txid_parts).as_ref(),
                     tv.sighash_all
                 );
             }
diff --git a/zcash_primitives/src/transaction/txid.rs b/zcash_primitives/src/transaction/txid.rs
index d8ee16aae..3734d655e 100644
--- a/zcash_primitives/src/transaction/txid.rs
+++ b/zcash_primitives/src/transaction/txid.rs
@@ -30,7 +30,7 @@ const ZCASH_TX_PERSONALIZATION_PREFIX: &[u8; 12] = b"ZcashTxHash_";
 
 // TxId level 1 node personalization
 const ZCASH_HEADERS_HASH_PERSONALIZATION: &[u8; 16] = b"ZTxIdHeadersHash";
-const ZCASH_TRANSPARENT_HASH_PERSONALIZATION: &[u8; 16] = b"ZTxIdTranspaHash";
+pub(crate) const ZCASH_TRANSPARENT_HASH_PERSONALIZATION: &[u8; 16] = b"ZTxIdTranspaHash";
 const ZCASH_SAPLING_HASH_PERSONALIZATION: &[u8; 16] = b"ZTxIdSaplingHash";
 #[cfg(feature = "zfuture")]
 const ZCASH_TZE_HASH_PERSONALIZATION: &[u8; 16] = b"ZTxIdTZE____Hash";
@@ -130,6 +130,8 @@ pub(crate) fn hash_tze_outputs(tze_outputs: &[TzeOut]) -> Blake2bHash {
     h.finalize()
 }
 
+/// Implements [ZIP 244 section T.3a](https://zips.z.cash/zip-0244#t-3a-sapling-spends-digest)
+///
 /// Write disjoint parts of each Sapling shielded spend to a pair of hashes:
 /// * \[nullifier*\] - personalized with ZCASH_SAPLING_SPENDS_COMPACT_HASH_PERSONALIZATION
 /// * \[(cv, anchor, rk, zkproof)*\] - personalized with ZCASH_SAPLING_SPENDS_NONCOMPACT_HASH_PERSONALIZATION
@@ -159,6 +161,8 @@ pub(crate) fn hash_sapling_spends<A: sapling::Authorization>(
     h.finalize()
 }
 
+/// Implements [ZIP 244 section T.3b](https://zips.z.cash/zip-0244#t-3b-sapling-outputs-digest)
+///
 /// Write disjoint parts of each Sapling shielded output as 3 separate hashes:
 /// * \[(cmu, epk, enc_ciphertext\[..52\])*\] personalized with ZCASH_SAPLING_OUTPUTS_COMPACT_HASH_PERSONALIZATION
 /// * \[enc_ciphertext\[52..564\]*\] (memo ciphertexts) personalized with ZCASH_SAPLING_OUTPUTS_MEMOS_HASH_PERSONALIZATION
@@ -199,7 +203,6 @@ fn transparent_digests<A: transparent::Authorization>(
         prevout_digest: transparent_prevout_hash(&bundle.vin),
         sequence_digest: transparent_sequence_hash(&bundle.vin),
         outputs_digest: transparent_outputs_hash(&bundle.vout),
-        per_input_digest: None,
     }
 }
 
@@ -213,6 +216,7 @@ fn tze_digests<A: tze::Authorization>(bundle: &tze::Bundle<A>) -> TzeDigests<Bla
     }
 }
 
+/// Implements [ZIP 244 section T.1](https://zips.z.cash/zip-0244#t-1-header-digest)
 fn hash_header_txid_data(
     version: TxVersion,
     // we commit to the consensus branch ID with the header
@@ -233,19 +237,20 @@ fn hash_header_txid_data(
     h.finalize()
 }
 
-fn hash_transparent_txid_data(t_digests: Option<&TransparentDigests<Blake2bHash>>) -> Blake2bHash {
+/// Implements [ZIP 244 section T.2](https://zips.z.cash/zip-0244#t-2-transparent-digest)
+pub(crate) fn hash_transparent_txid_data(
+    t_digests: Option<&TransparentDigests<Blake2bHash>>,
+) -> Blake2bHash {
     let mut h = hasher(ZCASH_TRANSPARENT_HASH_PERSONALIZATION);
     if let Some(d) = t_digests {
         h.write_all(d.prevout_digest.as_bytes()).unwrap();
         h.write_all(d.sequence_digest.as_bytes()).unwrap();
         h.write_all(d.outputs_digest.as_bytes()).unwrap();
-        if let Some(s) = d.per_input_digest {
-            h.write_all(s.as_bytes()).unwrap();
-        };
     }
     h.finalize()
 }
 
+/// Implements [ZIP 244 section T.3](https://zips.z.cash/zip-0244#t-3-sapling-digest)
 fn hash_sapling_txid_data<A: sapling::Authorization>(bundle: &sapling::Bundle<A>) -> Blake2bHash {
     let mut h = hasher(ZCASH_SAPLING_HASH_PERSONALIZATION);
     if !(bundle.shielded_spends.is_empty() && bundle.shielded_outputs.is_empty()) {
@@ -278,13 +283,15 @@ fn hash_tze_txid_data(tze_digests: Option<&TzeDigests<Blake2bHash>>) -> Blake2bH
     h.finalize()
 }
 
+/// A TransactionDigest implementation that commits to all of the effecting
+/// data of a transaction to produce a nonmalleable transaction identifier.
+///
+/// This expects and relies upon the existence of canonical encodings for
+/// each effecting component of a transaction.
+///
+/// This implements the [TxId Digest section of ZIP 244](https://zips.z.cash/zip-0244#txid-digest)
 pub struct TxIdDigester;
 
-// A TransactionDigest implementation that commits to all of the effecting
-// data of a transaction to produce a nonmalleable transaction identifier.
-//
-// This expects and relies upon the existence of canonical encodings for
-// each effecting component of a transaction.
 impl<A: Authorization> TransactionDigest<A> for TxIdDigester {
     type HeaderDigest = Blake2bHash;
     type TransparentDigest = Option<TransparentDigests<Blake2bHash>>;
@@ -355,7 +362,7 @@ pub(crate) fn to_hash(
     _txversion: TxVersion,
     consensus_branch_id: BranchId,
     header_digest: Blake2bHash,
-    transparent_digests: Option<&TransparentDigests<Blake2bHash>>,
+    transparent_digest: Blake2bHash,
     sapling_digest: Option<Blake2bHash>,
     orchard_digest: Option<Blake2bHash>,
     #[cfg(feature = "zfuture")] tze_digests: Option<&TzeDigests<Blake2bHash>>,
@@ -368,8 +375,7 @@ pub(crate) fn to_hash(
 
     let mut h = hasher(&personal);
     h.write_all(header_digest.as_bytes()).unwrap();
-    h.write_all(hash_transparent_txid_data(transparent_digests).as_bytes())
-        .unwrap();
+    h.write_all(transparent_digest.as_bytes()).unwrap();
     h.write_all(
         sapling_digest
             .unwrap_or_else(hash_sapling_txid_empty)
@@ -401,7 +407,7 @@ pub fn to_txid(
         txversion,
         consensus_branch_id,
         digests.header_digest,
-        digests.transparent_digests.as_ref(),
+        hash_transparent_txid_data(digests.transparent_digests.as_ref()),
         digests.sapling_digest,
         digests.orchard_digest,
         #[cfg(feature = "zfuture")]