ZIP-221 and ZIP-244 commitment validation in non-finalized state (#2609)

* Add validation of ZIP-221 and ZIP-244 commitments

* Apply suggestions from code review

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

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

Cargo.lock

@@ -4639,6 +4639,7 @@ name = "zebra-state"
 version = "1.0.0-alpha.15"
 dependencies = [
  "bincode",
+ "blake2b_simd",
  "chrono",
  "color-eyre",
  "dirs",

zebra-chain/src/block/commitment.rs

@@ -169,6 +169,18 @@ impl From<ChainHistoryMmrRootHash> for [u8; 32] {
 #[derive(Clone, Copy, Debug, Eq, PartialEq, Serialize, Deserialize)]
 pub struct ChainHistoryBlockTxAuthCommitmentHash([u8; 32]);
 
+impl From<[u8; 32]> for ChainHistoryBlockTxAuthCommitmentHash {
+    fn from(hash: [u8; 32]) -> Self {
+        ChainHistoryBlockTxAuthCommitmentHash(hash)
+    }
+}
+
+impl From<ChainHistoryBlockTxAuthCommitmentHash> for [u8; 32] {
+    fn from(hash: ChainHistoryBlockTxAuthCommitmentHash) -> Self {
+        hash.0
+    }
+}
+
 /// Errors that can occur when checking RootHash consensus rules.
 ///
 /// Each error variant corresponds to a consensus rule, so enumerating

zebra-chain/src/block/merkle.rs

@@ -155,6 +155,18 @@ impl fmt::Debug for AuthDataRoot {
     }
 }
 
+impl From<[u8; 32]> for AuthDataRoot {
+    fn from(hash: [u8; 32]) -> Self {
+        AuthDataRoot(hash)
+    }
+}
+
+impl From<AuthDataRoot> for [u8; 32] {
+    fn from(hash: AuthDataRoot) -> Self {
+        hash.0
+    }
+}
+
 impl<T> std::iter::FromIterator<T> for AuthDataRoot
 where
     T: std::convert::AsRef<Transaction>,

zebra-chain/src/history_tree.rs

@@ -481,6 +481,11 @@ impl HistoryTree {
         };
         Ok(())
     }
+
+    /// Return the hash of the tree root if the tree is not empty.
+    pub fn hash(&self) -> Option<ChainHistoryMmrRootHash> {
+        Some(self.0.as_ref()?.hash())
+    }
 }
 
 impl From<NonEmptyHistoryTree> for HistoryTree {

zebra-state/Cargo.toml

@@ -32,6 +32,7 @@ rlimit = "0.5.4"
 # TODO: this crate is not maintained anymore. Replace it?
 # https://github.com/ZcashFoundation/zebra/issues/2523
 multiset = "0.0.5"
+blake2b_simd = "0.5.11"
 
 proptest = { version = "0.10.1", optional = true }
 zebra-test = { path = "../zebra-test/", optional = true }

zebra-state/src/error.rs

@@ -188,6 +188,9 @@ pub enum ValidateContextError {
 
     #[error("error building the history tree")]
     HistoryTreeError(#[from] HistoryTreeError),
+
+    #[error("block contains an invalid commitment")]
+    InvalidBlockCommitment(#[from] block::CommitmentError),
 }
 
 /// 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

@@ -62,7 +62,9 @@ pub struct PreparedChain {
 
 impl PreparedChain {
     /// Create a PreparedChain strategy with Heartwood-onward blocks.
-    #[cfg(test)]
+    // dead_code is allowed because the function is called only by tests,
+    // but the code is also compiled when proptest-impl is activated.
+    #[allow(dead_code)]
     pub(crate) fn new_heartwood() -> Self {
         // The history tree only works with Heartwood onward.
         // Since the network will be chosen later, we pick the larger

zebra-state/src/service/check.rs

@@ -1,11 +1,12 @@
 //! Consensus critical contextual checks
 
-use std::borrow::Borrow;
+use std::{borrow::Borrow, convert::TryInto};
 
 use chrono::Duration;
 
 use zebra_chain::{
-    block::{self, Block},
+    block::{self, Block, CommitmentError},
+    history_tree::HistoryTree,
     parameters::POW_AVERAGING_WINDOW,
     parameters::{Network, NetworkUpgrade},
     work::difficulty::CompactDifficulty,
@@ -24,8 +25,11 @@ pub(crate) mod utxo;
 #[cfg(test)]
 mod tests;
 
-/// Check that `block` is contextually valid for `network`, based on the
+/// Check that the `prepared` block is contextually valid for `network`, based
-/// `finalized_tip_height` and `relevant_chain`.
+/// 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 +38,8 @@ mod tests;
 ///
 /// If the state contains less than 28
 /// (`POW_AVERAGING_WINDOW + POW_MEDIAN_BLOCK_SPAN`) blocks.
-#[tracing::instrument(
+#[tracing::instrument(skip(prepared, finalized_tip_height, relevant_chain))]
-    name = "contextual_validation",
+pub(crate) fn block_is_valid_for_recent_chain<C>(
-    fields(?network),
-    skip(prepared, network, finalized_tip_height, relevant_chain)
-)]
-pub(crate) fn block_is_contextually_valid<C>(
     prepared: &PreparedBlock,
     network: Network,
     finalized_tip_height: Option<block::Height>,
@@ -100,6 +100,75 @@ where
     Ok(())
 }
 
+/// Check that the `prepared` block is contextually valid for `network`, using
+/// the `history_tree` up to and including the previous block.
+#[tracing::instrument(skip(prepared, history_tree))]
+pub(crate) fn block_commitment_is_valid_for_chain_history(
+    prepared: &PreparedBlock,
+    network: Network,
+    history_tree: &HistoryTree,
+) -> 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(actual_history_tree_root) => {
+            let history_tree_root = history_tree
+                .hash()
+                .expect("the history tree of the previous block must exist since the current block has a ChainHistoryRoot");
+            if actual_history_tree_root == history_tree_root {
+                Ok(())
+            } else {
+                Err(ValidateContextError::InvalidBlockCommitment(
+                    CommitmentError::InvalidChainHistoryRoot {
+                        actual: actual_history_tree_root.into(),
+                        expected: history_tree_root.into(),
+                    },
+                ))
+            }
+        }
+        block::Commitment::ChainHistoryBlockTxAuthCommitment(actual_hash_block_commitments) => {
+            let actual_block_commitments: [u8; 32] = actual_hash_block_commitments.into();
+            let history_tree_root = history_tree
+                .hash()
+                .expect("the history tree of the previous block must exist since the current block has a ChainHistoryBlockTxAuthCommitment");
+            let auth_data_root = prepared.block.auth_data_root();
+
+            // > The value of this hash [hashBlockCommitments] is the BLAKE2b-256 hash personalized
+            // > by the string "ZcashBlockCommit" of the following elements:
+            // >   hashLightClientRoot (as described in ZIP 221)
+            // >   hashAuthDataRoot    (as described below)
+            // >   terminator          [0u8;32]
+            // https://zips.z.cash/zip-0244#block-header-changes
+            let hash_block_commitments: [u8; 32] = blake2b_simd::Params::new()
+                .hash_length(32)
+                .personal(b"ZcashBlockCommit")
+                .to_state()
+                .update(&<[u8; 32]>::from(history_tree_root)[..])
+                .update(&<[u8; 32]>::from(auth_data_root))
+                .update(&[0u8; 32])
+                .finalize()
+                .as_bytes()
+                .try_into()
+                .expect("32 byte array");
+
+            if actual_block_commitments == hash_block_commitments {
+                Ok(())
+            } else {
+                Err(ValidateContextError::InvalidBlockCommitment(
+                    CommitmentError::InvalidChainHistoryBlockTxAuthCommitment {
+                        actual: actual_block_commitments,
+                        expected: hash_block_commitments,
+                    },
+                ))
+            }
+        }
+    }
+}
+
 /// 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/non_finalized_state.rs

@@ -191,6 +191,11 @@ impl NonFinalizedState {
             &parent_chain.spent_utxos,
             finalized_state,
         )?;
+        check::block_commitment_is_valid_for_chain_history(
+            &prepared,
+            self.network,
+            &parent_chain.history_tree,
+        )?;
 
         parent_chain.push(prepared)
     }

zebra-state/src/service/non_finalized_state/tests/vectors.rs

@@ -2,6 +2,7 @@ use std::sync::Arc;
 
 use zebra_chain::{
     block::Block,
+    history_tree::NonEmptyHistoryTree,
     parameters::{Network, NetworkUpgrade},
     serialization::ZcashDeserializeInto,
 };
@@ -392,13 +393,13 @@ fn history_tree_is_updated_for_network_upgrade(
             .zcash_deserialize_into::<Block>()
             .expect("block is structurally valid"),
     );
-    let activation_block = prev_block.make_fake_child();
-    let next_block = activation_block.make_fake_child();
 
     let mut state = NonFinalizedState::new(network);
     let finalized_state = FinalizedState::new(&Config::ephemeral(), network);
 
-    state.commit_new_chain(prev_block.prepare(), &finalized_state)?;
+    state
+        .commit_new_chain(prev_block.clone().prepare(), &finalized_state)
+        .unwrap();
 
     let chain = state.best_chain().unwrap();
     if network_upgrade == NetworkUpgrade::Heartwood {
@@ -413,7 +414,12 @@ fn history_tree_is_updated_for_network_upgrade(
         );
     }
 
-    state.commit_block(activation_block.prepare(), &finalized_state)?;
+    // The Heartwood activation block has an all-zero commitment
+    let activation_block = prev_block.make_fake_child().set_block_commitment([0u8; 32]);
+
+    state
+        .commit_block(activation_block.clone().prepare(), &finalized_state)
+        .unwrap();
 
     let chain = state.best_chain().unwrap();
     assert!(
@@ -426,7 +432,22 @@ fn history_tree_is_updated_for_network_upgrade(
         "history tree must have a single node"
     );
 
-    state.commit_block(next_block.prepare(), &finalized_state)?;
+    // To fix the commitment in the next block we must recreate the history tree
+    let tree = NonEmptyHistoryTree::from_block(
+        Network::Mainnet,
+        activation_block.clone(),
+        &chain.sapling_note_commitment_tree.root(),
+        &chain.orchard_note_commitment_tree.root(),
+    )
+    .unwrap();
+
+    let next_block = activation_block
+        .make_fake_child()
+        .set_block_commitment(tree.hash().into());
+
+    state
+        .commit_block(next_block.prepare(), &finalized_state)
+        .unwrap();
 
     assert!(
         state.best_chain().unwrap().history_tree.as_ref().is_some(),
@@ -435,3 +456,84 @@ fn history_tree_is_updated_for_network_upgrade(
 
     Ok(())
 }
+
+#[test]
+fn commitment_is_validated() {
+    commitment_is_validated_for_network_upgrade(Network::Mainnet, NetworkUpgrade::Heartwood);
+    commitment_is_validated_for_network_upgrade(Network::Testnet, NetworkUpgrade::Heartwood);
+    // TODO: we can't test other upgrades until we have a method for creating a FinalizedState
+    // with a HistoryTree.
+}
+
+fn commitment_is_validated_for_network_upgrade(network: Network, network_upgrade: NetworkUpgrade) {
+    let blocks = match network {
+        Network::Mainnet => &*zebra_test::vectors::MAINNET_BLOCKS,
+        Network::Testnet => &*zebra_test::vectors::TESTNET_BLOCKS,
+    };
+    let height = network_upgrade.activation_height(network).unwrap().0;
+
+    let prev_block = Arc::new(
+        blocks
+            .get(&(height - 1))
+            .expect("test vector exists")
+            .zcash_deserialize_into::<Block>()
+            .expect("block is structurally valid"),
+    );
+
+    let mut state = NonFinalizedState::new(network);
+    let finalized_state = FinalizedState::new(&Config::ephemeral(), network);
+
+    state
+        .commit_new_chain(prev_block.clone().prepare(), &finalized_state)
+        .unwrap();
+
+    // The Heartwood activation block must have an all-zero commitment.
+    // Test error return when committing the block with the wrong commitment
+    let activation_block = prev_block.make_fake_child();
+    let err = state
+        .commit_block(activation_block.clone().prepare(), &finalized_state)
+        .unwrap_err();
+    match err {
+        crate::ValidateContextError::InvalidBlockCommitment(
+            zebra_chain::block::CommitmentError::InvalidChainHistoryActivationReserved { .. },
+        ) => {},
+        _ => panic!("Error must be InvalidBlockCommitment::InvalidChainHistoryActivationReserved instead of {:?}", err),
+    };
+
+    // Test committing the Heartwood activation block with the correct commitment
+    let activation_block = activation_block.set_block_commitment([0u8; 32]);
+    state
+        .commit_block(activation_block.clone().prepare(), &finalized_state)
+        .unwrap();
+
+    // To fix the commitment in the next block we must recreate the history tree
+    let chain = state.best_chain().unwrap();
+    let tree = NonEmptyHistoryTree::from_block(
+        Network::Mainnet,
+        activation_block.clone(),
+        &chain.sapling_note_commitment_tree.root(),
+        &chain.orchard_note_commitment_tree.root(),
+    )
+    .unwrap();
+
+    // Test committing the next block with the wrong commitment
+    let next_block = activation_block.make_fake_child();
+    let err = state
+        .commit_block(next_block.clone().prepare(), &finalized_state)
+        .unwrap_err();
+    match err {
+        crate::ValidateContextError::InvalidBlockCommitment(
+            zebra_chain::block::CommitmentError::InvalidChainHistoryRoot { .. },
+        ) => {}
+        _ => panic!(
+            "Error must be InvalidBlockCommitment::InvalidChainHistoryRoot instead of {:?}",
+            err
+        ),
+    };
+
+    // Test committing the next block with the correct commitment
+    let next_block = next_block.set_block_commitment(tree.hash().into());
+    state
+        .commit_block(next_block.prepare(), &finalized_state)
+        .unwrap();
+}

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 {