Validate nConsensusBranchId (#2100)

* validate nConsensusBranchId
* add tests

* fix bug in transaction_to_fake_v5

Co-authored-by: teor <teor@riseup.net>

zebra-chain/src/transaction/arbitrary.rs

@@ -6,7 +6,7 @@ use proptest::{arbitrary::any, array, collection::vec, option, prelude::*};
 use crate::{
     amount::Amount,
     block,
-    parameters::NetworkUpgrade,
+    parameters::{Network, NetworkUpgrade},
     primitives::{Bctv14Proof, Groth16Proof, ZkSnarkProof},
     sapling, sprout, transparent, LedgerState,
 };
@@ -349,21 +349,24 @@ impl Arbitrary for Transaction {
 
 // Utility functions
 
-/// The network upgrade for any fake transactions we will create.
-const FAKE_NETWORK_UPGRADE: NetworkUpgrade = NetworkUpgrade::Nu5;
-
 /// Convert `trans` into a fake v5 transaction,
 /// converting sapling shielded data from v4 to v5 if possible.
-pub fn transaction_to_fake_v5(trans: &Transaction) -> Transaction {
+pub fn transaction_to_fake_v5(
+    trans: &Transaction,
+    network: Network,
+    height: block::Height,
+) -> Transaction {
     use Transaction::*;
 
+    let block_nu = NetworkUpgrade::current(network, height);
+
     match trans {
         V1 {
             inputs,
             outputs,
             lock_time,
         } => V5 {
-            network_upgrade: FAKE_NETWORK_UPGRADE,
+            network_upgrade: block_nu,
             inputs: inputs.to_vec(),
             outputs: outputs.to_vec(),
             lock_time: *lock_time,
@@ -376,7 +379,7 @@ pub fn transaction_to_fake_v5(trans: &Transaction) -> Transaction {
             lock_time,
             ..
         } => V5 {
-            network_upgrade: FAKE_NETWORK_UPGRADE,
+            network_upgrade: block_nu,
             inputs: inputs.to_vec(),
             outputs: outputs.to_vec(),
             lock_time: *lock_time,
@@ -390,7 +393,7 @@ pub fn transaction_to_fake_v5(trans: &Transaction) -> Transaction {
             expiry_height,
             ..
         } => V5 {
-            network_upgrade: FAKE_NETWORK_UPGRADE,
+            network_upgrade: block_nu,
             inputs: inputs.to_vec(),
             outputs: outputs.to_vec(),
             lock_time: *lock_time,
@@ -405,7 +408,7 @@ pub fn transaction_to_fake_v5(trans: &Transaction) -> Transaction {
             sapling_shielded_data,
             ..
         } => V5 {
-            network_upgrade: FAKE_NETWORK_UPGRADE,
+            network_upgrade: block_nu,
             inputs: inputs.to_vec(),
             outputs: outputs.to_vec(),
             lock_time: *lock_time,

zebra-chain/src/transaction/tests/vectors.rs

@@ -1,7 +1,8 @@
 use super::super::*;
 
 use crate::{
-    block::{Block, MAX_BLOCK_BYTES},
+    block::{Block, Height, MAX_BLOCK_BYTES},
+    parameters::{Network, NetworkUpgrade},
     serialization::{ZcashDeserialize, ZcashDeserializeInto, ZcashSerialize},
 };
 
@@ -164,11 +165,31 @@ fn empty_v4_round_trip() {
 fn fake_v5_round_trip() {
     zebra_test::init();
 
-    for original_bytes in zebra_test::vectors::BLOCKS.iter() {
+    fake_v5_round_trip_for_network(Network::Mainnet);
+    fake_v5_round_trip_for_network(Network::Testnet);
+}
+
+fn fake_v5_round_trip_for_network(network: Network) {
+    let block_iter = match network {
+        Network::Mainnet => zebra_test::vectors::MAINNET_BLOCKS.iter(),
+        Network::Testnet => zebra_test::vectors::TESTNET_BLOCKS.iter(),
+    };
+
+    for (height, original_bytes) in block_iter {
         let original_block = original_bytes
             .zcash_deserialize_into::<Block>()
             .expect("block is structurally valid");
 
+        // skip blocks that are before overwinter as they will not have a valid consensus branch id
+        if *height
+            < NetworkUpgrade::Overwinter
+                .activation_height(network)
+                .expect("a valid height")
+                .0
+        {
+            continue;
+        }
+
         // skip this block if it only contains v5 transactions,
         // the block round-trip test covers it already
         if original_block
@@ -184,7 +205,7 @@ fn fake_v5_round_trip() {
             .transactions
             .iter()
             .map(AsRef::as_ref)
-            .map(arbitrary::transaction_to_fake_v5)
+            .map(|t| arbitrary::transaction_to_fake_v5(t, network, Height(*height)))
             .map(Into::into)
             .collect();
 

zebra-consensus/src/block.rs

@@ -161,7 +161,7 @@ where
                 .map(|t| t.hash())
                 .collect::<Vec<_>>();
 
-            check::merkle_root_validity(&block, &transaction_hashes)?;
+            check::merkle_root_validity(network, &block, &transaction_hashes)?;
 
             // Since errors cause an early exit, try to do the
             // quick checks first.

zebra-consensus/src/block/check.rs

@@ -168,10 +168,48 @@ pub fn time_is_valid_at(
 /// Check Merkle root validity.
 ///
 /// `transaction_hashes` is a precomputed list of transaction hashes.
+///
+/// # Consensus rules:
+///
+/// - The nConsensusBranchId field MUST match the consensus branch ID used for
+///  SIGHASH transaction hashes, as specifed in [ZIP-244] ([7.1]).
+/// - A SHA-256d hash in internal byte order. The merkle root is derived from the
+///  hashes of all transactions included in this block, ensuring that none of
+///  those transactions can be modified without modifying the header. [7.6]
+///
+/// # Panics
+///
+/// - If block does not have a coinbase transaction.
+///
+/// [ZIP-244]: https://zips.z.cash/zip-0244
+/// [7.1]: https://zips.z.cash/protocol/nu5.pdf#txnencodingandconsensus
+/// [7.6]: https://zips.z.cash/protocol/nu5.pdf#blockheader
 pub fn merkle_root_validity(
+    network: Network,
     block: &Block,
     transaction_hashes: &[transaction::Hash],
 ) -> Result<(), BlockError> {
+    use transaction::Transaction;
+    let block_nu =
+        NetworkUpgrade::current(network, block.coinbase_height().expect("a valid height"));
+
+    if block
+        .transactions
+        .iter()
+        .filter_map(|trans| match *trans.as_ref() {
+            Transaction::V5 {
+                network_upgrade, ..
+            } => Some(network_upgrade),
+            Transaction::V1 { .. }
+            | Transaction::V2 { .. }
+            | Transaction::V3 { .. }
+            | Transaction::V4 { .. } => None,
+        })
+        .any(|trans_nu| trans_nu != block_nu)
+    {
+        return Err(BlockError::WrongTransactionConsensusBranchId);
+    }
+
     let merkle_root = transaction_hashes.iter().cloned().collect();
 
     if block.header.merkle_root != merkle_root {

zebra-consensus/src/block/tests.rs

@@ -13,8 +13,9 @@ use tower::buffer::Buffer;
 
 use zebra_chain::{
     block::{self, Block, Height},
-    parameters::Network,
+    parameters::{Network, NetworkUpgrade},
     serialization::{ZcashDeserialize, ZcashDeserializeInto},
+    transaction::{arbitrary::transaction_to_fake_v5, Transaction},
     work::difficulty::{ExpandedDifficulty, INVALID_COMPACT_DIFFICULTY},
 };
 use zebra_test::transcript::{TransError, Transcript};
@@ -435,3 +436,92 @@ fn time_is_valid_for_historical_blocks() -> Result<(), Report> {
 
     Ok(())
 }
+
+#[test]
+fn merkle_root_is_valid() -> Result<(), Report> {
+    zebra_test::init();
+
+    // test all original blocks available, all blocks validate
+    merkle_root_is_valid_for_network(Network::Mainnet)?;
+    merkle_root_is_valid_for_network(Network::Testnet)?;
+
+    // create and test fake blocks with v5 transactions, all blocks fail validation
+    merkle_root_fake_v5_for_network(Network::Mainnet)?;
+    merkle_root_fake_v5_for_network(Network::Testnet)?;
+
+    Ok(())
+}
+
+fn merkle_root_is_valid_for_network(network: Network) -> Result<(), Report> {
+    let block_iter = match network {
+        Network::Mainnet => zebra_test::vectors::MAINNET_BLOCKS.iter(),
+        Network::Testnet => zebra_test::vectors::TESTNET_BLOCKS.iter(),
+    };
+
+    for (_height, block) in block_iter {
+        let block = block
+            .zcash_deserialize_into::<Block>()
+            .expect("block is structurally valid");
+
+        let transaction_hashes = block
+            .transactions
+            .iter()
+            .map(|tx| tx.hash())
+            .collect::<Vec<_>>();
+
+        check::merkle_root_validity(network, &block, &transaction_hashes)
+            .expect("merkle root should be valid for this block");
+    }
+
+    Ok(())
+}
+
+fn merkle_root_fake_v5_for_network(network: Network) -> Result<(), Report> {
+    let block_iter = match network {
+        Network::Mainnet => zebra_test::vectors::MAINNET_BLOCKS.iter(),
+        Network::Testnet => zebra_test::vectors::TESTNET_BLOCKS.iter(),
+    };
+
+    for (height, block) in block_iter {
+        let mut block = block
+            .zcash_deserialize_into::<Block>()
+            .expect("block is structurally valid");
+
+        // skip blocks that are before overwinter as they will not have a valid consensus branch id
+        if *height
+            < NetworkUpgrade::Overwinter
+                .activation_height(network)
+                .expect("a valid overwinter activation height")
+                .0
+        {
+            continue;
+        }
+
+        // convert all transactions from the block to V5
+        let transactions: Vec<Arc<Transaction>> = block
+            .transactions
+            .iter()
+            .map(AsRef::as_ref)
+            .map(|t| transaction_to_fake_v5(t, network, Height(*height)))
+            .map(Into::into)
+            .collect();
+
+        block.transactions = transactions;
+
+        let transaction_hashes = block
+            .transactions
+            .iter()
+            .map(|tx| tx.hash())
+            .collect::<Vec<_>>();
+
+        // Replace the merkle root so that it matches the modified transactions.
+        // This test provides some transaction id and merkle root coverage,
+        // but we also need to test against zcashd test vectors.
+        block.header.merkle_root = transaction_hashes.iter().cloned().collect();
+
+        check::merkle_root_validity(network, &block, &transaction_hashes)
+            .expect("merkle root should be valid for this block");
+    }
+
+    Ok(())
+}

zebra-consensus/src/checkpoint.rs

@@ -464,7 +464,7 @@ where
             .map(|tx| tx.hash())
             .collect::<Vec<_>>();
 
-        crate::block::check::merkle_root_validity(&block, &transaction_hashes)?;
+        crate::block::check::merkle_root_validity(self.network, &block, &transaction_hashes)?;
 
         Ok(height)
     }

zebra-consensus/src/error.rs

@@ -146,4 +146,7 @@ pub enum BlockError {
         zebra_chain::work::difficulty::ExpandedDifficulty,
         zebra_chain::parameters::Network,
     ),
+
+    #[error("transaction has wrong consensus branch id for block network upgrade")]
+    WrongTransactionConsensusBranchId,
 }

zebra-consensus/src/transaction/tests.rs

@@ -1,5 +1,6 @@
 use zebra_chain::{
-    block::Block,
+    block::{Block, Height},
+    parameters::Network,
     serialization::ZcashDeserializeInto,
     transaction::{arbitrary::transaction_to_fake_v5, Transaction},
 };
@@ -11,8 +12,22 @@ use color_eyre::eyre::Report;
 fn v5_fake_transactions() -> Result<(), Report> {
     zebra_test::init();
 
+    v5_fake_transactions_for_network(Network::Mainnet)?;
+    v5_fake_transactions_for_network(Network::Testnet)?;
+
+    Ok(())
+}
+
+fn v5_fake_transactions_for_network(network: Network) -> Result<(), Report> {
+    zebra_test::init();
+
     // get all the blocks we have available
-    for original_bytes in zebra_test::vectors::BLOCKS.iter() {
+    let block_iter = match network {
+        Network::Mainnet => zebra_test::vectors::MAINNET_BLOCKS.iter(),
+        Network::Testnet => zebra_test::vectors::TESTNET_BLOCKS.iter(),
+    };
+
+    for (height, original_bytes) in block_iter {
         let original_block = original_bytes
             .zcash_deserialize_into::<Block>()
             .expect("block is structurally valid");
@@ -22,7 +37,7 @@ fn v5_fake_transactions() -> Result<(), Report> {
             .transactions
             .iter()
             .map(AsRef::as_ref)
-            .map(transaction_to_fake_v5)
+            .map(|t| transaction_to_fake_v5(t, network, Height(*height)))
             .map(Into::into)
             .collect();