From a39dbc43d29eced87abd43f00d404efd659efa07 Mon Sep 17 00:00:00 2001 From: Conrado Gouvea Date: Wed, 11 Aug 2021 11:41:49 -0300 Subject: [PATCH] Add validation of ZIP-221 and ZIP-244 commitments --- Cargo.lock | 1 + zebra-chain/src/block/commitment.rs | 12 ++ zebra-chain/src/block/merkle.rs | 12 ++ zebra-chain/src/history_tree.rs | 5 + zebra-state/Cargo.toml | 1 + zebra-state/src/error.rs | 3 + zebra-state/src/service.rs | 2 +- zebra-state/src/service/arbitrary.rs | 4 +- zebra-state/src/service/check.rs | 89 ++++++++++++-- .../src/service/non_finalized_state.rs | 5 + .../non_finalized_state/tests/vectors.rs | 112 +++++++++++++++++- zebra-state/src/tests.rs | 8 ++ 12 files changed, 237 insertions(+), 17 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index f28cbeecefe..8f0ee5ac6fd 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -4639,6 +4639,7 @@ name = "zebra-state" version = "1.0.0-alpha.14" dependencies = [ "bincode", + "blake2b_simd", "chrono", "color-eyre", "dirs", diff --git a/zebra-chain/src/block/commitment.rs b/zebra-chain/src/block/commitment.rs index 9b55adb37ad..eb2dda6a3cf 100644 --- a/zebra-chain/src/block/commitment.rs +++ b/zebra-chain/src/block/commitment.rs @@ -169,6 +169,18 @@ impl From 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 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 diff --git a/zebra-chain/src/block/merkle.rs b/zebra-chain/src/block/merkle.rs index cf95737eb5f..270c0ae0018 100644 --- a/zebra-chain/src/block/merkle.rs +++ b/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 for [u8; 32] { + fn from(hash: AuthDataRoot) -> Self { + hash.0 + } +} + impl std::iter::FromIterator for AuthDataRoot where T: std::convert::AsRef, diff --git a/zebra-chain/src/history_tree.rs b/zebra-chain/src/history_tree.rs index 2485e4f25c4..852fc707caa 100644 --- a/zebra-chain/src/history_tree.rs +++ b/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 { + Some(self.0.as_ref()?.hash()) + } } impl From for HistoryTree { diff --git a/zebra-state/Cargo.toml b/zebra-state/Cargo.toml index 7fac2b1a9c8..708dbadae27 100644 --- a/zebra-state/Cargo.toml +++ b/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 } diff --git a/zebra-state/src/error.rs b/zebra-state/src/error.rs index 7f57dcc7208..43dbdd3e845 100644 --- a/zebra-state/src/error.rs +++ b/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. diff --git a/zebra-state/src/service.rs b/zebra-state/src/service.rs index b2ddfdf0b87..065e2be1872 100644 --- a/zebra-state/src/service.rs +++ b/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(), diff --git a/zebra-state/src/service/arbitrary.rs b/zebra-state/src/service/arbitrary.rs index 982b82831c7..5110c159918 100644 --- a/zebra-state/src/service/arbitrary.rs +++ b/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 diff --git a/zebra-state/src/service/check.rs b/zebra-state/src/service/check.rs index 4bde7dd8d2b..ca2ada2790b 100644 --- a/zebra-state/src/service/check.rs +++ b/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 -/// `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 +38,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( +#[tracing::instrument(skip(prepared, finalized_tip_height, relevant_chain))] +pub(crate) fn block_is_valid_for_recent_chain( prepared: &PreparedBlock, network: Network, finalized_tip_height: Option, @@ -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))] +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( diff --git a/zebra-state/src/service/non_finalized_state.rs b/zebra-state/src/service/non_finalized_state.rs index 491f208abde..3a73d8f2525 100644 --- a/zebra-state/src/service/non_finalized_state.rs +++ b/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) } diff --git a/zebra-state/src/service/non_finalized_state/tests/vectors.rs b/zebra-state/src/service/non_finalized_state/tests/vectors.rs index 712cf828c3d..81eb41f999b 100644 --- a/zebra-state/src/service/non_finalized_state/tests/vectors.rs +++ b/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::() .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::() + .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(); +} diff --git a/zebra-state/src/tests.rs b/zebra-state/src/tests.rs index 4ed05bd8779..9c7296874f3 100644 --- a/zebra-state/src/tests.rs +++ b/zebra-state/src/tests.rs @@ -17,6 +17,8 @@ pub trait FakeChainHelper { fn make_fake_child(&self) -> Arc; fn set_work(self, work: u128) -> Arc; + + fn set_block_commitment(self, commitment: [u8; 32]) -> Arc; } impl FakeChainHelper for Arc { @@ -53,6 +55,12 @@ impl FakeChainHelper for Arc { block.header.difficulty_threshold = expanded.into(); self } + + fn set_block_commitment(mut self, block_commitment: [u8; 32]) -> Arc { + let block = Arc::make_mut(&mut self); + block.header.commitment_bytes = block_commitment; + self + } } fn work_to_expanded(work: U256) -> ExpandedDifficulty {