fix(consensus): Verify consensus branch ID in SIGHASH precomputation

zebra-chain/src/tests/vectors.rs

@@ -65,6 +65,7 @@ impl Network {
                     transaction,
                     Amount::try_from(1_000_000).expect("invalid value"),
                     0,
+                    false,
                 )
                 .ok()
             })

zebra-chain/src/transaction.rs

@@ -258,6 +258,16 @@ impl Transaction {
         !self.inputs().is_empty()
     }
 
+    /// Does this transaction have transparent outputs?
+    pub fn has_transparent_outputs(&self) -> bool {
+        !self.outputs().is_empty()
+    }
+
+    /// Does this transaction have transparent inputs or outputs?
+    pub fn has_transparent_inputs_or_outputs(&self) -> bool {
+        self.has_transparent_inputs() || self.has_transparent_outputs()
+    }
+
     /// Does this transaction have transparent or shielded inputs?
     pub fn has_transparent_or_shielded_inputs(&self) -> bool {
         self.has_transparent_inputs() || self.has_shielded_inputs()
@@ -276,11 +286,6 @@ impl Transaction {
                     .contains(orchard::Flags::ENABLE_SPENDS))
     }
 
-    /// Does this transaction have transparent or shielded outputs?
-    pub fn has_transparent_or_shielded_outputs(&self) -> bool {
-        !self.outputs().is_empty() || self.has_shielded_outputs()
-    }
-
     /// Does this transaction have shielded outputs?
     ///
     /// See [`Self::has_transparent_or_shielded_outputs`] for details.
@@ -294,6 +299,11 @@ impl Transaction {
                     .contains(orchard::Flags::ENABLE_OUTPUTS))
     }
 
+    /// Does this transaction have transparent or shielded outputs?
+    pub fn has_transparent_or_shielded_outputs(&self) -> bool {
+        self.has_transparent_outputs() || self.has_shielded_outputs()
+    }
+
     /// Does this transaction has at least one flag when we have at least one orchard action?
     pub fn has_enough_orchard_flags(&self) -> bool {
         if self.version() < 5 || self.orchard_actions().count() == 0 {

zebra-chain/src/transaction/unmined.rs

@@ -385,21 +385,31 @@ impl VerifiedUnminedTx {
         transaction: UnminedTx,
         miner_fee: Amount<NonNegative>,
         legacy_sigop_count: u64,
+        // Allows skipping ZIP 317 checks, only in tests.
+        #[cfg(feature = "proptest-impl")] skip_checks: bool,
     ) -> Result<Self, zip317::Error> {
         let fee_weight_ratio = zip317::conventional_fee_weight_ratio(&transaction, miner_fee);
         let conventional_actions = zip317::conventional_actions(&transaction.transaction);
         let unpaid_actions = zip317::unpaid_actions(&transaction, miner_fee);
+        let tx_size = transaction.size;
 
-        zip317::mempool_checks(unpaid_actions, miner_fee, transaction.size)?;
-
-        Ok(Self {
+        let verified_unmined_tx = Self {
             transaction,
             miner_fee,
             legacy_sigop_count,
             fee_weight_ratio,
             conventional_actions,
             unpaid_actions,
-        })
+        };
+
+        #[cfg(feature = "proptest-impl")]
+        if skip_checks {
+            return Ok(verified_unmined_tx);
+        }
+
+        zip317::mempool_checks(unpaid_actions, miner_fee, tx_size)?;
+
+        Ok(verified_unmined_tx)
     }
 
     /// Returns `true` if the transaction pays at least the [ZIP-317] conventional fee.

zebra-consensus/src/block.rs

@@ -251,6 +251,8 @@ where
                         known_utxos: known_utxos.clone(),
                         height,
                         time: block.header.time,
+                        #[cfg(any(test, feature = "proptest-impl"))]
+                        skip_checks: None,
                     });
                 async_checks.push(rsp);
             }

zebra-consensus/src/transaction.rs

@@ -158,6 +158,9 @@ pub enum Request {
         height: block::Height,
         /// The time that the block was mined.
         time: DateTime<Utc>,
+        /// Verification checks to skip, only in tests.
+        #[cfg(any(test, feature = "proptest-impl"))]
+        skip_checks: Option<HashSet<SkipCheck>>,
     },
     /// Verify the supplied transaction as part of the mempool.
     ///
@@ -172,6 +175,9 @@ pub enum Request {
         /// The next block is the first block that could possibly contain a
         /// mempool transaction.
         height: block::Height,
+        /// Verification checks to skip, only in tests.
+        #[cfg(any(test, feature = "proptest-impl"))]
+        skip_checks: Option<HashSet<SkipCheck>>,
     },
 }
 
@@ -318,6 +324,15 @@ impl Request {
     pub fn is_mempool(&self) -> bool {
         matches!(self, Request::Mempool { .. })
     }
+
+    /// Returns the verification checks that should be skipped, only in tests.
+    #[cfg(any(test, feature = "proptest-impl"))]
+    pub fn skip_checks(&self) -> Option<HashSet<SkipCheck>> {
+        match self {
+            Request::Block { skip_checks, .. } => skip_checks.clone(),
+            Request::Mempool { skip_checks, .. } => skip_checks.clone(),
+        }
+    }
 }
 
 impl Response {
@@ -415,7 +430,12 @@ where
             // Do quick checks first
             check::has_inputs_and_outputs(&tx)?;
             check::has_enough_orchard_flags(&tx)?;
+            #[cfg(not(test))]
             check::consensus_branch_id(&tx, req.height(), &network)?;
+            #[cfg(test)]
+            if req.skip_checks().is_none_or(|c| !c.contains(&SkipCheck::ConsensusBranchId)) {
+                check::consensus_branch_id(&tx, req.height(), &network)?;
+            }
 
             // Validate the coinbase input consensus rules
             if req.is_mempool() && tx.is_coinbase() {
@@ -432,7 +452,12 @@ where
             if tx.is_coinbase() {
                 check::coinbase_expiry_height(&req.height(), &tx, &network)?;
             } else {
+                #[cfg(not(test))]
                 check::non_coinbase_expiry_height(&req.height(), &tx)?;
+                #[cfg(test)]
+                if req.skip_checks().is_none_or(|c| !c.contains(&SkipCheck::ExpiryHeight)) {
+                    check::non_coinbase_expiry_height(&req.height(), &tx)?;
+                }
             }
 
             // Consensus rule:
@@ -575,12 +600,20 @@ where
                     miner_fee,
                     legacy_sigop_count,
                 },
-                Request::Mempool { transaction, .. } => {
+                Request::Mempool { transaction: ref tx, .. } => {
+                    // #[cfg(test)]
+                    #[cfg(any(test, feature = "proptest-impl"))]
                     let transaction = VerifiedUnminedTx::new(
-                        transaction,
-                        miner_fee.expect(
-                            "unexpected mempool coinbase transaction: should have already rejected",
-                        ),
+                        tx.clone(),
+                        miner_fee.expect("fee should have been checked earlier"),
+                        legacy_sigop_count,
+                        req.skip_checks().is_some_and(|checks| checks.contains(&SkipCheck::Zip317))
+                    )?;
+
+                    #[cfg(not(any(test, feature = "proptest-impl")))]
+                    let transaction = VerifiedUnminedTx::new(
+                        tx.clone(),
+                        miner_fee.expect("fee should have been checked earlier"),
                         legacy_sigop_count,
                     )?;
 
@@ -1388,3 +1421,15 @@ where
         AsyncChecks(iterator.into_iter().map(FutureExt::boxed).collect())
     }
 }
+
+/// Checks the tx verifier should skip, only in tests.
+#[cfg(any(test, feature = "proptest-impl"))]
+#[derive(Eq, Hash, PartialEq, Debug, Clone)]
+pub enum SkipCheck {
+    /// Skip checks related to the transaction consensus branch ID.
+    ConsensusBranchId,
+    /// Skip mempool checks defined in ZIP 317.
+    Zip317,
+    /// Skip the expiry height check.
+    ExpiryHeight,
+}