diff --git a/zebra-consensus/src/block.rs b/zebra-consensus/src/block.rs index f1623b78..b044f4f9 100644 --- a/zebra-consensus/src/block.rs +++ b/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 { /// The network to be verified. network: Network, state_service: S, - transaction_verifier: transaction::Verifier, + transaction_verifier: tx::Verifier, } // 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::>(); + + 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> { +/// 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> { let mut new_outputs = HashMap::default(); let height = block.coinbase_height().expect("block has coinbase height"); - 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; diff --git a/zebra-consensus/src/block/check.rs b/zebra-consensus/src/block/check.rs index c06d9992..e59c24b3 100644 --- a/zebra-consensus/src/block/check.rs +++ b/zebra-consensus/src/block/check.rs @@ -3,10 +3,9 @@ use chrono::{DateTime, Utc}; use zebra_chain::{ - block::Hash, - block::Height, - block::{Block, Header}, + block::{Block, Hash, Header, Height}, 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, + }) + } +} diff --git a/zebra-consensus/src/checkpoint.rs b/zebra-consensus/src/checkpoint.rs index ec98e0ad..a74eea74 100644 --- a/zebra-consensus/src/checkpoint.rs +++ b/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::>(); + 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::>().len() { + tx.send(Err(VerifyCheckpointError::DuplicateTransaction)) + .expect("rx has not been dropped yet"); + return rx; + } + // Since we're using Arc, 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)] diff --git a/zebra-consensus/src/error.rs b/zebra-consensus/src/error.rs index 419dc2e8..d0fb1603 100644 --- a/zebra-consensus/src/error.rs +++ b/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), diff --git a/zebra-state/src/request.rs b/zebra-state/src/request.rs index 6bfb28ac..a47a271e 100644 --- a/zebra-state/src/request.rs +++ b/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, + /// A precomputed list of the hashes of the transactions in this block. + pub transaction_hashes: Vec, // 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, + /// A precomputed list of the hashes of the transactions in this block. + pub(crate) transaction_hashes: Vec, } // Doing precomputation in this From impl means that it will be done in @@ -108,10 +112,19 @@ impl From> 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::>(); 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> for FinalizedBlock { height, hash, new_outputs, + transaction_hashes, } } } @@ -142,12 +156,14 @@ impl From for FinalizedBlock { height, hash, new_outputs, + transaction_hashes, } = prepared; Self { block, height, hash, new_outputs, + transaction_hashes, } } } diff --git a/zebra-state/src/service/finalized_state.rs b/zebra-state/src/service/finalized_state.rs index ba93f9c0..4793aeef 100644 --- a/zebra-state/src/service/finalized_state.rs +++ b/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() { - let transaction_hash = transaction.hash(); + for (transaction_index, (transaction, transaction_hash)) in block + .transactions + .iter() + .zip(transaction_hashes.into_iter()) + .enumerate() + { let transaction_location = TransactionLocation { height, index: transaction_index diff --git a/zebra-state/src/service/non_finalized_state/chain.rs b/zebra-state/src/service/non_finalized_state/chain.rs index 39c0e26c..82946e6c 100644 --- a/zebra-state/src/service/non_finalized_state/chain.rs +++ b/zebra-state/src/service/non_finalized_state/chain.rs @@ -136,7 +136,12 @@ trait UpdateWith { impl UpdateWith 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 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 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 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 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 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" ); diff --git a/zebra-state/src/tests.rs b/zebra-state/src/tests.rs index 3c788967..8e17019e 100644 --- a/zebra-state/src/tests.rs +++ b/zebra-state/src/tests.rs @@ -21,6 +21,7 @@ impl Prepare for Arc { 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 { hash, height, new_outputs, + transaction_hashes, } } }