consensus: check Merkle roots

As a side effect of computing Merkle roots, we build a list of
transaction hashes.  Instead of discarding these, add them to
PreparedBlock and FinalizedBlock so that they can be reused rather than
recomputed.

This commit adds Merkle root validation to:

1. the block verifier;
2. the checkpoint verifier.

In the first case, Bitcoin Merkle tree malleability has no effect,
because only a single Merkle tree in each malleablity set is valid (the
others have duplicate transactions).

In the second case, we need to check that the Merkle tree does not contain any
duplicate transactions.

Closes #1385
Closes #906

zebra-consensus/src/block.rs

@@ -25,12 +25,12 @@ use tracing::Instrument;
 use zebra_chain::{
     block::{self, Block},
     parameters::Network,
-    transparent,
+    transaction, transparent,
     work::equihash,
 };
 use zebra_state as zs;
 
-use crate::{error::*, transaction};
+use crate::{error::*, transaction as tx};
 use crate::{script, BoxError};
 
 mod check;
@@ -44,7 +44,7 @@ pub struct BlockVerifier<S> {
     /// The network to be verified.
     network: Network,
     state_service: S,
-    transaction_verifier: transaction::Verifier<S>,
+    transaction_verifier: tx::Verifier<S>,
 }
 
 // TODO: dedupe with crate::error::BlockError
@@ -83,7 +83,7 @@ where
 {
     pub fn new(network: Network, state_service: S) -> Self {
         let transaction_verifier =
-            transaction::Verifier::new(network, script::Verifier::new(state_service.clone()));
+            tx::Verifier::new(network, script::Verifier::new(state_service.clone()));
 
         Self {
             network,
@@ -161,17 +161,24 @@ where
             check::coinbase_is_first(&block)?;
             check::subsidy_is_valid(&block, network)?;
 
-            // TODO: context-free header verification: merkle root
+            // Precomputing this avoids duplicating transaction hash computations.
+            let transaction_hashes = block
+                .transactions
+                .iter()
+                .map(|t| t.hash())
+                .collect::<Vec<_>>();
+
+            check::merkle_root_validity(&block, &transaction_hashes)?;
 
             let mut async_checks = FuturesUnordered::new();
 
-            let known_utxos = new_outputs(&block);
+            let known_utxos = new_outputs(&block, &transaction_hashes);
             for transaction in &block.transactions {
                 let rsp = transaction_verifier
                     .ready_and()
                     .await
                     .expect("transaction verifier is always ready")
-                    .call(transaction::Request::Block {
+                    .call(tx::Request::Block {
                         transaction: transaction.clone(),
                         known_utxos: known_utxos.clone(),
                         height,
@@ -199,6 +206,7 @@ where
                 hash,
                 height,
                 new_outputs,
+                transaction_hashes,
             };
             match state_service
                 .ready_and()
@@ -220,11 +228,19 @@ where
     }
 }
 
-fn new_outputs(block: &Block) -> Arc<HashMap<transparent::OutPoint, zs::Utxo>> {
+/// Compute an index of newly created transparent outputs, given a block and a
+/// list of precomputed transaction hashes.
+fn new_outputs(
+    block: &Block,
+    transaction_hashes: &[transaction::Hash],
+) -> Arc<HashMap<transparent::OutPoint, zs::Utxo>> {
     let mut new_outputs = HashMap::default();
     let height = block.coinbase_height().expect("block has coinbase height");
-    for transaction in &block.transactions {
+    for (transaction, hash) in block
-        let hash = transaction.hash();
+        .transactions
+        .iter()
+        .zip(transaction_hashes.iter().cloned())
+    {
         let from_coinbase = transaction.is_coinbase();
         for (index, output) in transaction.outputs().iter().cloned().enumerate() {
             let index = index as u32;

zebra-consensus/src/block/check.rs

@@ -3,10 +3,9 @@
 use chrono::{DateTime, Utc};
 
 use zebra_chain::{
-    block::Hash,
+    block::{Block, Hash, Header, Height},
-    block::Height,
-    block::{Block, Header},
     parameters::{Network, NetworkUpgrade},
+    transaction,
     work::{difficulty::ExpandedDifficulty, equihash},
 };
 
@@ -165,3 +164,21 @@ pub fn time_is_valid_at(
 ) -> Result<(), zebra_chain::block::BlockTimeError> {
     header.time_is_valid_at(now, height, hash)
 }
+
+/// Check Merkle root validity.
+///
+/// `transaction_hashes` is a precomputed list of transaction hashes.
+pub fn merkle_root_validity(
+    block: &Block,
+    transaction_hashes: &[transaction::Hash],
+) -> Result<(), BlockError> {
+    let merkle_root = transaction_hashes.iter().cloned().collect();
+    if block.header.merkle_root == merkle_root {
+        Ok(())
+    } else {
+        Err(BlockError::BadMerkleRoot {
+            actual: merkle_root,
+            expected: block.header.merkle_root,
+        })
+    }
+}

zebra-consensus/src/checkpoint.rs

@@ -14,7 +14,7 @@
 //! block for the configured network.
 
 use std::{
-    collections::BTreeMap,
+    collections::{BTreeMap, HashSet},
     future::Future,
     ops::{Bound, Bound::*},
     pin::Pin,
@@ -459,6 +459,33 @@ where
             }
         };
 
+        // Check for a valid Merkle root. To prevent malleability (CVE-2012-2459),
+        // we also need to check whether the transaction hashes are unique.
+
+        let transaction_hashes = block
+            .transactions
+            .iter()
+            .map(|tx| tx.hash())
+            .collect::<Vec<_>>();
+        let merkle_root = transaction_hashes.iter().cloned().collect();
+
+        if block.header.merkle_root != merkle_root {
+            tx.send(Err(VerifyCheckpointError::BadMerkleRoot {
+                expected: block.header.merkle_root,
+                actual: merkle_root,
+            }))
+            .expect("rx has not been dropped yet");
+            return rx;
+        }
+
+        // Collecting into a HashSet deduplicates, so this checks that there
+        // are no duplicate transaction hashes, preventing Merkle root malleability.
+        if transaction_hashes.len() != transaction_hashes.iter().collect::<HashSet<_>>().len() {
+            tx.send(Err(VerifyCheckpointError::DuplicateTransaction))
+                .expect("rx has not been dropped yet");
+            return rx;
+        }
+
         // Since we're using Arc<Block>, each entry is a single pointer to the
         // Arc. But there are a lot of QueuedBlockLists in the queue, so we keep
         // allocations as small as possible.
@@ -779,6 +806,13 @@ pub enum VerifyCheckpointError {
     },
     #[error("the block {hash:?} does not have a coinbase height")]
     CoinbaseHeight { hash: block::Hash },
+    #[error("merkle root {actual:?} does not match expected {expected:?}")]
+    BadMerkleRoot {
+        actual: block::merkle::Root,
+        expected: block::merkle::Root,
+    },
+    #[error("duplicate transactions in block")]
+    DuplicateTransaction,
     #[error("checkpoint verifier was dropped")]
     Dropped,
     #[error(transparent)]

zebra-consensus/src/error.rs

@@ -7,7 +7,7 @@
 
 use thiserror::Error;
 
-use zebra_chain::primitives::ed25519;
+use zebra_chain::{block, primitives::ed25519};
 
 use crate::BoxError;
 
@@ -99,6 +99,12 @@ pub enum BlockError {
     #[error("block has no transactions")]
     NoTransactions,
 
+    #[error("block has mismatched merkle root")]
+    BadMerkleRoot {
+        actual: block::merkle::Root,
+        expected: block::merkle::Root,
+    },
+
     #[error("block {0:?} is already in the chain at depth {1:?}")]
     AlreadyInChain(zebra_chain::block::Hash, u32),
 

zebra-state/src/request.rs

@@ -74,6 +74,8 @@ pub struct PreparedBlock {
     /// be unspent, since a later transaction in a block can spend outputs of an
     /// earlier transaction.
     pub new_outputs: HashMap<transparent::OutPoint, Utxo>,
+    /// A precomputed list of the hashes of the transactions in this block.
+    pub transaction_hashes: Vec<transaction::Hash>,
     // TODO: add these parameters when we can compute anchors.
     // sprout_anchor: sprout::tree::Root,
     // sapling_anchor: sapling::tree::Root,
@@ -97,6 +99,8 @@ pub struct FinalizedBlock {
     /// be unspent, since a later transaction in a block can spend outputs of an
     /// earlier transaction.
     pub(crate) new_outputs: HashMap<transparent::OutPoint, Utxo>,
+    /// A precomputed list of the hashes of the transactions in this block.
+    pub(crate) transaction_hashes: Vec<transaction::Hash>,
 }
 
 // Doing precomputation in this From impl means that it will be done in
@@ -108,10 +112,19 @@ impl From<Arc<Block>> for FinalizedBlock {
             .coinbase_height()
             .expect("finalized blocks must have a valid coinbase height");
         let hash = block.hash();
+        let transaction_hashes = block
+            .transactions
+            .iter()
+            .map(|tx| tx.hash())
+            .collect::<Vec<_>>();
 
         let mut new_outputs = HashMap::default();
-        for transaction in &block.transactions {
+
-            let hash = transaction.hash();
+        for (transaction, hash) in block
+            .transactions
+            .iter()
+            .zip(transaction_hashes.iter().cloned())
+        {
             let from_coinbase = transaction.is_coinbase();
             for (index, output) in transaction.outputs().iter().cloned().enumerate() {
                 let index = index as u32;
@@ -131,6 +144,7 @@ impl From<Arc<Block>> for FinalizedBlock {
             height,
             hash,
             new_outputs,
+            transaction_hashes,
         }
     }
 }
@@ -142,12 +156,14 @@ impl From<PreparedBlock> for FinalizedBlock {
             height,
             hash,
             new_outputs,
+            transaction_hashes,
         } = prepared;
         Self {
             block,
             height,
             hash,
             new_outputs,
+            transaction_hashes,
         }
     }
 }

zebra-state/src/service/finalized_state.rs

@@ -152,6 +152,7 @@ impl FinalizedState {
             hash,
             height,
             new_outputs,
+            transaction_hashes,
         } = finalized;
 
         let hash_by_height = self.db.cf_handle("hash_by_height").unwrap();
@@ -215,8 +216,12 @@ impl FinalizedState {
 
             // Index each transaction, spent inputs, nullifiers
             // TODO: move computation into FinalizedBlock as with transparent outputs
-            for (transaction_index, transaction) in block.transactions.iter().enumerate() {
+            for (transaction_index, (transaction, transaction_hash)) in block
-                let transaction_hash = transaction.hash();
+                .transactions
+                .iter()
+                .zip(transaction_hashes.into_iter())
+                .enumerate()
+            {
                 let transaction_location = TransactionLocation {
                     height,
                     index: transaction_index

zebra-state/src/service/non_finalized_state/chain.rs

@@ -136,7 +136,12 @@ trait UpdateWith<T> {
 
 impl UpdateWith<PreparedBlock> for Chain {
     fn update_chain_state_with(&mut self, prepared: &PreparedBlock) {
-        let (block, hash, height) = (prepared.block.as_ref(), prepared.hash, prepared.height);
+        let (block, hash, height, transaction_hashes) = (
+            prepared.block.as_ref(),
+            prepared.hash,
+            prepared.height,
+            &prepared.transaction_hashes,
+        );
 
         // add hash to height_by_hash
         let prior_height = self.height_by_hash.insert(hash, height);
@@ -154,7 +159,12 @@ impl UpdateWith<PreparedBlock> for Chain {
         self.partial_cumulative_work += block_work;
 
         // for each transaction in block
-        for (transaction_index, transaction) in block.transactions.iter().enumerate() {
+        for (transaction_index, (transaction, transaction_hash)) in block
+            .transactions
+            .iter()
+            .zip(transaction_hashes.iter().cloned())
+            .enumerate()
+        {
             let (inputs, shielded_data, joinsplit_data) = match transaction.deref() {
                 transaction::Transaction::V4 {
                     inputs,
@@ -168,7 +178,6 @@ impl UpdateWith<PreparedBlock> for Chain {
             };
 
             // add key `transaction.hash` and value `(height, tx_index)` to `tx_by_hash`
-            let transaction_hash = transaction.hash();
             let prior_pair = self
                 .tx_by_hash
                 .insert(transaction_hash, (height, transaction_index));
@@ -190,7 +199,11 @@ impl UpdateWith<PreparedBlock> for Chain {
 
     #[instrument(skip(self, prepared), fields(block = %prepared.block))]
     fn revert_chain_state_with(&mut self, prepared: &PreparedBlock) {
-        let (block, hash) = (prepared.block.as_ref(), prepared.hash);
+        let (block, hash, transaction_hashes) = (
+            prepared.block.as_ref(),
+            prepared.hash,
+            &prepared.transaction_hashes,
+        );
 
         // remove the blocks hash from `height_by_hash`
         assert!(
@@ -207,7 +220,9 @@ impl UpdateWith<PreparedBlock> for Chain {
         self.partial_cumulative_work -= block_work;
 
         // for each transaction in block
-        for transaction in &block.transactions {
+        for (transaction, transaction_hash) in
+            block.transactions.iter().zip(transaction_hashes.iter())
+        {
             let (inputs, shielded_data, joinsplit_data) = match transaction.deref() {
                 transaction::Transaction::V4 {
                     inputs,
@@ -221,9 +236,8 @@ impl UpdateWith<PreparedBlock> for Chain {
             };
 
             // remove `transaction.hash` from `tx_by_hash`
-            let transaction_hash = transaction.hash();
             assert!(
-                self.tx_by_hash.remove(&transaction_hash).is_some(),
+                self.tx_by_hash.remove(transaction_hash).is_some(),
                 "transactions must be present if block was"
             );
 

zebra-state/src/tests.rs

@@ -21,6 +21,7 @@ impl Prepare for Arc<Block> {
         let block = self;
         let hash = block.hash();
         let height = block.coinbase_height().unwrap();
+        let transaction_hashes = block.transactions.iter().map(|tx| tx.hash()).collect();
         let new_outputs = crate::utxo::new_outputs(&block);
 
         PreparedBlock {
@@ -28,6 +29,7 @@ impl Prepare for Arc<Block> {
             hash,
             height,
             new_outputs,
+            transaction_hashes,
         }
     }
 }