ZIP-221: Validate chain history commitments in the non-finalized state (

#2301)

* sketch of implementation

* refined implementation; still incomplete

* update librustzcash, change zcash_history to work with it

* simplified code per review; renamed MMR to HistoryTree

* expand HistoryTree implementation

* handle and propagate errors

* simplify check.rs tracing

* add suggested TODO

* add HistoryTree::prune

* fix bug in pruning

* fix compilation of tests; still need to make them pass

* Apply suggestions from code review

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

* Apply suggestions from code review

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

* improvements from code review

* improve check.rs comments and variable names

* fix HistoryTree which should use BTreeMap and not HashMap; fix non_finalized_state prop tests

* fix finalized_state proptest

* fix non_finalized_state tests by setting the correct commitments

* renamed mmr.rs to history_tree.rs

* Add HistoryTree struct

* expand non_finalized_state protest

* fix typo

* Add HistoryTree struct

* Update zebra-chain/src/primitives/zcash_history.rs

Co-authored-by: Deirdre Connolly <deirdre@zfnd.org>

* fix formatting

* Apply suggestions from code review

Co-authored-by: Deirdre Connolly <deirdre@zfnd.org>

* history_tree.rs: fixes from code review

* fixes to work with updated HistoryTree

* Improvements from code review

* Add Debug implementations to allow comparing Chains with proptest_assert_eq

Co-authored-by: teor <teor@riseup.net>
Co-authored-by: Deirdre Connolly <deirdre@zfnd.org>

zebra-chain/src/block/commitment.rs

@@ -149,6 +149,12 @@ impl From<[u8; 32]> for ChainHistoryMmrRootHash {
     }
 }
 
+impl From<ChainHistoryMmrRootHash> for [u8; 32] {
+    fn from(hash: ChainHistoryMmrRootHash) -> Self {
+        hash.0
+    }
+}
+
 /// A block commitment to chain history and transaction auth.
 /// - the chain history tree for all ancestors in the current network upgrade,
 ///   and

zebra-chain/src/history_tree.rs

@@ -461,3 +461,11 @@ impl Deref for HistoryTree {
         &self.0
     }
 }
+
+impl PartialEq for HistoryTree {
+    fn eq(&self, other: &Self) -> bool {
+        self.hash() == other.hash()
+    }
+}
+
+impl Eq for HistoryTree {}

zebra-chain/src/primitives/zcash_history.rs

@@ -244,6 +244,15 @@ impl<V: zcash_history::Version> std::fmt::Debug for Tree<V> {
     }
 }
 
+impl std::fmt::Debug for Tree {
+    fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
+        f.debug_struct("Tree")
+            .field("network", &self.network)
+            .field("network_upgrade", &self.network_upgrade)
+            .finish()
+    }
+}
+
 impl Version for zcash_history::V1 {
     /// Convert a Block into a V1::NodeData used in the MMR tree.
     ///

zebra-state/src/error.rs

@@ -3,11 +3,7 @@ use std::sync::Arc;
 use chrono::{DateTime, Utc};
 use thiserror::Error;
 
-use zebra_chain::{
-    block, orchard, sapling, sprout, transparent, work::difficulty::CompactDifficulty,
-};
-
-use crate::constants::MIN_TRANSPARENT_COINBASE_MATURITY;
+use zebra_chain::{block, work::difficulty::CompactDifficulty};
 
 /// A wrapper for type erased errors that is itself clonable and implements the
 /// Error trait
@@ -152,6 +148,20 @@ pub enum ValidateContextError {
 
     #[error("error in Orchard note commitment tree")]
     OrchardNoteCommitmentTreeError(#[from] zebra_chain::orchard::tree::NoteCommitmentTreeError),
+
+
+    #[error("block contains an invalid commitment")]
+    InvalidBlockCommitment(#[from] block::CommitmentError),
+
+    #[error("block history commitment {candidate_commitment:?} is different to the expected commitment {expected_commitment:?}")]
+    #[non_exhaustive]
+    InvalidHistoryCommitment {
+        candidate_commitment: ChainHistoryMmrRootHash,
+        expected_commitment: ChainHistoryMmrRootHash,
+    },
+
+    #[error("error building the history tree")]
+    HistoryTreeError(#[from] HistoryTreeError),
 }
 
 /// Trait for creating the corresponding duplicate nullifier error from a nullifier.

zebra-state/src/service.rs

@@ -276,7 +276,7 @@ impl StateService {
         let relevant_chain = self.any_ancestor_blocks(prepared.block.header.previous_block_hash);
 
         // Security: check proof of work before any other checks
-        check::block_is_contextually_valid(
+        check::block_is_valid_for_recent_chain(
             prepared,
             self.network,
             self.disk.finalized_tip_height(),

zebra-state/src/service/arbitrary.rs

@@ -60,6 +60,29 @@ impl ValueTree for PreparedChainTree {
 pub struct PreparedChain {
     // the proptests are threaded (not async), so we want to use a threaded mutex here
     chain: std::sync::Mutex<Option<(Network, Arc<SummaryDebug<Vec<PreparedBlock>>>)>>,
+    // the height from which to start the chain. If None, starts at the genesis block
+    start_height: Option<Height>,
+}
+
+impl PreparedChain {
+    /// Create a PreparedChain strategy with Heartwood-onward blocks.
+    pub(super) fn new_heartwood() -> Self {
+        // The history tree only works with Heartwood onward.
+        // Since the network will be chosen later, we pick the larger
+        // between the mainnet and testnet Heartwood activation heights.
+        let main_height = NetworkUpgrade::Heartwood
+            .activation_height(Network::Mainnet)
+            .expect("must have height");
+        let test_height = NetworkUpgrade::Heartwood
+            .activation_height(Network::Testnet)
+            .expect("must have height");
+        let height = (std::cmp::max(main_height, test_height) + 1).expect("must be valid");
+
+        PreparedChain {
+            start_height: Some(height),
+            ..Default::default()
+        }
+    }
 }
 
 impl Strategy for PreparedChain {
@@ -70,7 +93,12 @@ impl Strategy for PreparedChain {
         let mut chain = self.chain.lock().unwrap();
         if chain.is_none() {
             // TODO: use the latest network upgrade (#1974)
-            let ledger_strategy = LedgerState::genesis_strategy(NetworkUpgrade::Nu5, None, false);
+            let ledger_strategy = match self.start_height {
+                Some(start_height) => {
+                    LedgerState::height_strategy(start_height, NetworkUpgrade::Nu5, None, false)
+                }
+                None => LedgerState::genesis_strategy(NetworkUpgrade::Nu5, None, false),
+            };
 
             let (network, blocks) = ledger_strategy
                 .prop_flat_map(|ledger| {
@@ -97,7 +125,9 @@ impl Strategy for PreparedChain {
         }
 
         let chain = chain.clone().expect("should be generated");
-        let count = (1..chain.1.len()).new_tree(runner)?;
+        // `count` must be 1 less since the first block is used to build the
+        // history tree.
+        let count = (1..chain.1.len() - 1).new_tree(runner)?;
         Ok(PreparedChainTree {
             chain: chain.1,
             count,

zebra-state/src/service/check.rs

@@ -5,7 +5,7 @@ use std::borrow::Borrow;
 use chrono::Duration;
 
 use zebra_chain::{
-    block::{self, Block},
+    block::{self, Block, ChainHistoryMmrRootHash},
     parameters::POW_AVERAGING_WINDOW,
     parameters::{Network, NetworkUpgrade},
     work::difficulty::CompactDifficulty,
@@ -24,8 +24,11 @@ pub(crate) mod utxo;
 #[cfg(test)]
 mod tests;
 
-/// Check that `block` is contextually valid for `network`, based on the
-/// `finalized_tip_height` and `relevant_chain`.
+/// Check that the `prepared` block is contextually valid for `network`, based
+/// on the `finalized_tip_height` and `relevant_chain`.
+///
+/// This function performs checks that require a small number of recent blocks,
+/// including previous hash, previous height, and block difficulty.
 ///
 /// The relevant chain is an iterator over the ancestors of `block`, starting
 /// with its parent block.
@@ -34,12 +37,8 @@ mod tests;
 ///
 /// If the state contains less than 28
 /// (`POW_AVERAGING_WINDOW + POW_MEDIAN_BLOCK_SPAN`) blocks.
-#[tracing::instrument(
-    name = "contextual_validation",
-    fields(?network),
-    skip(prepared, network, finalized_tip_height, relevant_chain)
-)]
-pub(crate) fn block_is_contextually_valid<C>(
+#[tracing::instrument(skip(prepared, finalized_tip_height, relevant_chain))]
+pub(crate) fn block_is_valid_for_recent_chain<C>(
     prepared: &PreparedBlock,
     network: Network,
     finalized_tip_height: Option<block::Height>,
@@ -100,6 +99,40 @@ where
     Ok(())
 }
 
+/// Check that the `prepared` block is contextually valid for `network`, based
+/// on the `history_root_hash` of the history tree up to and including the
+/// previous block.
+#[tracing::instrument(skip(prepared))]
+pub(crate) fn block_commitment_is_valid_for_chain_history(
+    prepared: &PreparedBlock,
+    network: Network,
+    history_root_hash: &ChainHistoryMmrRootHash,
+) -> Result<(), ValidateContextError> {
+    match prepared.block.commitment(network)? {
+        block::Commitment::PreSaplingReserved(_)
+        | block::Commitment::FinalSaplingRoot(_)
+        | block::Commitment::ChainHistoryActivationReserved => {
+            // No contextual checks needed for those.
+            Ok(())
+        }
+        block::Commitment::ChainHistoryRoot(block_history_root_hash) => {
+            if block_history_root_hash == *history_root_hash {
+                Ok(())
+            } else {
+                Err(ValidateContextError::InvalidHistoryCommitment {
+                    candidate_commitment: block_history_root_hash,
+                    expected_commitment: *history_root_hash,
+                })
+            }
+        }
+        block::Commitment::ChainHistoryBlockTxAuthCommitment(_) => {
+            // TODO: Get auth_hash from block (ZIP-244), e.g.
+            // let auth_hash = prepared.block.auth_hash();
+            todo!("hash mmr_hash and auth_hash per ZIP-244 and compare")
+        }
+    }
+}
+
 /// Returns `ValidateContextError::OrphanedBlock` if the height of the given
 /// block is less than or equal to the finalized tip height.
 fn block_is_not_orphaned(

zebra-state/src/service/finalized_state.rs

@@ -7,6 +7,7 @@ mod tests;
 
 use std::{collections::HashMap, convert::TryInto, path::Path, sync::Arc};
 
+use zebra_chain::history_tree::HistoryTree;
 use zebra_chain::{
     block::{self, Block},
     history_tree::{HistoryTree, NonEmptyHistoryTree},

zebra-state/src/service/non_finalized_state.rs

@@ -14,9 +14,7 @@ use std::{collections::BTreeSet, mem, ops::Deref, sync::Arc};
 
 use zebra_chain::{
     block::{self, Block},
-    orchard,
     parameters::Network,
-    sapling,
     transaction::{self, Transaction},
     transparent,
 };
@@ -28,6 +26,8 @@ use crate::{FinalizedBlock, HashOrHeight, PreparedBlock, ValidateContextError};
 
 use self::chain::Chain;
 
+use super::check;
+
 use super::{check, finalized_state::FinalizedState};
 
 /// The state of the chains in memory, incuding queued blocks.
@@ -362,6 +362,7 @@ impl NonFinalizedState {
         parent_hash: block::Hash,
         sapling_note_commitment_tree: sapling::tree::NoteCommitmentTree,
         orchard_note_commitment_tree: orchard::tree::NoteCommitmentTree,
+        finalized_tip_history_tree: &HistoryTree,
     ) -> Result<Box<Chain>, ValidateContextError> {
         match self.take_chain_if(|chain| chain.non_finalized_tip_hash() == parent_hash) {
             // An existing chain in the non-finalized state

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

@@ -8,8 +8,14 @@ use multiset::HashMultiSet;
 use tracing::instrument;
 
 use zebra_chain::{
-    block, orchard, primitives::Groth16Proof, sapling, sprout, transaction,
-    transaction::Transaction::*, transparent, work::difficulty::PartialCumulativeWork,
+    block::{self, ChainHistoryMmrRootHash},
+    history_tree::HistoryTree,
+    orchard,
+    primitives::Groth16Proof,
+    sapling, sprout, transaction,
+    transaction::Transaction::*,
+    transparent,
+    work::difficulty::PartialCumulativeWork,
 };
 
 use crate::{service::check, ContextuallyValidBlock, PreparedBlock, ValidateContextError};
@@ -56,13 +62,15 @@ pub struct Chain {
 
     /// The cumulative work represented by this partial non-finalized chain.
     pub(super) partial_cumulative_work: PartialCumulativeWork,
+    pub(crate) history_tree: HistoryTree,
 }
 
 impl Chain {
-    // Create a new Chain with the given note commitment trees.
+    /// Create a new Chain with the given note commitment trees.
     pub(crate) fn new(
         sapling_note_commitment_tree: sapling::tree::NoteCommitmentTree,
         orchard_note_commitment_tree: orchard::tree::NoteCommitmentTree,
+        history_tree: HistoryTree,
     ) -> Self {
         Self {
             blocks: Default::default(),
@@ -80,6 +88,7 @@ impl Chain {
             sapling_nullifiers: Default::default(),
             orchard_nullifiers: Default::default(),
             partial_cumulative_work: Default::default(),
+            history_tree,
         }
     }
 
@@ -170,18 +179,21 @@ impl Chain {
     /// chain.
     ///
     /// The note commitment trees must be the trees of the finalized tip.
+    /// `finalized_tip_history_tree`: the history tree for the finalized tip
+    /// from which the tree of the fork will be computed.
     pub fn fork(
         &self,
         fork_tip: block::Hash,
         sapling_note_commitment_tree: sapling::tree::NoteCommitmentTree,
         orchard_note_commitment_tree: orchard::tree::NoteCommitmentTree,
+        finalized_tip_history_tree: &HistoryTree,
     ) -> Result<Option<Self>, ValidateContextError> {
         if !self.height_by_hash.contains_key(&fork_tip) {
             return Ok(None);
         }
 
         let mut forked =
-            self.with_trees(sapling_note_commitment_tree, orchard_note_commitment_tree);
+            self.with_trees(sapling_note_commitment_tree, orchard_note_commitment_tree, finalized_tip_history_tree.clone());
 
         while forked.non_finalized_tip_hash() != fork_tip {
             forked.pop_tip();
@@ -259,6 +271,10 @@ impl Chain {
         unspent_utxos
     }
 
+    pub fn history_root_hash(&self) -> ChainHistoryMmrRootHash {
+        self.history_tree.hash()
+    }
+
     /// Clone the Chain but not the history and note commitment trees, using
     /// the specified trees instead.
     ///
@@ -267,6 +283,7 @@ impl Chain {
         &self,
         sapling_note_commitment_tree: sapling::tree::NoteCommitmentTree,
         orchard_note_commitment_tree: orchard::tree::NoteCommitmentTree,
+        history_tree: HistoryTree,
     ) -> Self {
         Chain {
             blocks: self.blocks.clone(),
@@ -284,6 +301,7 @@ impl Chain {
             sapling_nullifiers: self.sapling_nullifiers.clone(),
             orchard_nullifiers: self.orchard_nullifiers.clone(),
             partial_cumulative_work: self.partial_cumulative_work,
+            history_tree,
         }
     }
 }
@@ -334,6 +352,10 @@ impl UpdateWith<ContextuallyValidBlock> for Chain {
             .expect("work has already been validated");
         self.partial_cumulative_work += block_work;
 
+        // TODO: pass Sapling and Orchard roots
+        self.history_tree
+            .push(prepared.block.clone(), &sapling::tree::Root([0; 32]), None)?;
+
         // for each transaction in block
         for (transaction_index, (transaction, transaction_hash)) in block
             .transactions
@@ -429,6 +451,11 @@ impl UpdateWith<ContextuallyValidBlock> for Chain {
             .expect("work has already been validated");
         self.partial_cumulative_work -= block_work;
 
+        // Note: the history tree is not modified in this method.
+        // This method is called on two scenarios:
+        // - When popping the root: the history tree does not change.
+        // - When popping the tip: the history tree is rebuilt in fork().
+
         // for each transaction in block
         for (transaction, transaction_hash) in
             block.transactions.iter().zip(transaction_hashes.iter())

zebra-state/src/tests.rs

@@ -17,6 +17,8 @@ pub trait FakeChainHelper {
     fn make_fake_child(&self) -> Arc<Block>;
 
     fn set_work(self, work: u128) -> Arc<Block>;
+
+    fn set_block_commitment(self, commitment: [u8; 32]) -> Arc<Block>;
 }
 
 impl FakeChainHelper for Arc<Block> {
@@ -53,6 +55,12 @@ impl FakeChainHelper for Arc<Block> {
         block.header.difficulty_threshold = expanded.into();
         self
     }
+
+    fn set_block_commitment(mut self, block_commitment: [u8; 32]) -> Arc<Block> {
+        let block = Arc::make_mut(&mut self);
+        block.header.commitment_bytes = block_commitment;
+        self
+    }
 }
 
 fn work_to_expanded(work: U256) -> ExpandedDifficulty {