diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index fc4cd3f29..220f693fc 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -69,6 +69,15 @@ jobs: with: command: test args: --verbose --all + + - name: Run tests with fake activation heights + uses: actions-rs/cargo@v1.0.3 + env: + TEST_FAKE_ACTIVATION_HEIGHTS: + with: + command: test + args: --verbose --all -- with_fake_activation_heights + # Explicitly run any tests that are usually #[ignored] - name: Run zebrad large sync tests diff --git a/Cargo.lock b/Cargo.lock index dd8c12fb3..c7e0500cc 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -4639,7 +4639,6 @@ name = "zebra-state" version = "1.0.0-alpha.15" dependencies = [ "bincode", - "blake2b_simd", "chrono", "color-eyre", "dirs", diff --git a/zebra-chain/build.rs b/zebra-chain/build.rs new file mode 100644 index 000000000..91ba008f7 --- /dev/null +++ b/zebra-chain/build.rs @@ -0,0 +1,9 @@ +use std::env; + +fn main() { + let use_fake_heights = env::var_os("TEST_FAKE_ACTIVATION_HEIGHTS").is_some(); + println!("cargo:rerun-if-env-changed=TEST_FAKE_ACTIVATION_HEIGHTS"); + if use_fake_heights { + println!("cargo:rustc-cfg=test_fake_activation_heights"); + } +} diff --git a/zebra-chain/src/block.rs b/zebra-chain/src/block.rs index 73f3553d5..ecea4a1c5 100644 --- a/zebra-chain/src/block.rs +++ b/zebra-chain/src/block.rs @@ -16,7 +16,9 @@ pub mod tests; use std::{collections::HashMap, convert::TryInto, fmt, ops::Neg}; -pub use commitment::{ChainHistoryMmrRootHash, Commitment, CommitmentError}; +pub use commitment::{ + ChainHistoryBlockTxAuthCommitmentHash, ChainHistoryMmrRootHash, Commitment, CommitmentError, +}; pub use hash::Hash; pub use header::{BlockTimeError, CountedHeader, Header}; pub use height::Height; diff --git a/zebra-chain/src/block/arbitrary.rs b/zebra-chain/src/block/arbitrary.rs index f7aa470f3..408c69c6f 100644 --- a/zebra-chain/src/block/arbitrary.rs +++ b/zebra-chain/src/block/arbitrary.rs @@ -11,6 +11,7 @@ use crate::{ amount::NonNegative, block, fmt::SummaryDebug, + history_tree::HistoryTree, parameters::{ Network, NetworkUpgrade::{self, *}, @@ -375,10 +376,15 @@ impl Block { /// `check_transparent_coinbase_spend` is used to check if /// transparent coinbase UTXOs are valid, before using them in blocks. /// Use [`allow_all_transparent_coinbase_spends`] to disable this check. + /// + /// `generate_valid_commitments` specifies if the generated blocks + /// should have valid commitments. This makes it much slower so it's better + /// to enable only when needed. pub fn partial_chain_strategy( mut current: LedgerState, count: usize, check_transparent_coinbase_spend: F, + generate_valid_commitments: bool, ) -> BoxedStrategy>>> where F: Fn( @@ -402,6 +408,15 @@ impl Block { let mut previous_block_hash = None; let mut utxos = HashMap::new(); let mut chain_value_pools = ValueBalance::zero(); + let mut sapling_tree = sapling::tree::NoteCommitmentTree::default(); + let mut orchard_tree = orchard::tree::NoteCommitmentTree::default(); + // The history tree usually takes care of "creating itself". But this + // only works when blocks are pushed into it starting from genesis + // (or at least pre-Heartwood, where the tree is not required). + // However, this strategy can generate blocks from an arbitrary height, + // so we must wait for the first block to create the history tree from it. + // This is why `Option` is used here. + let mut history_tree: Option = None; for (height, block) in vec.iter_mut() { // fixup the previous block hash @@ -419,6 +434,19 @@ impl Block { &mut utxos, check_transparent_coinbase_spend, ) { + // The FinalizedState does not update the note commitment trees with the genesis block, + // because it doesn't need to (the trees are not used at that point) and updating them + // would be awkward since the genesis block is handled separatedly there. + // This forces us to skip the genesis block here too in order to able to use + // this to test the finalized state. + if generate_valid_commitments && *height != Height(0) { + for sapling_note_commitment in transaction.sapling_note_commitments() { + sapling_tree.append(*sapling_note_commitment).unwrap(); + } + for orchard_note_commitment in transaction.orchard_note_commitments() { + orchard_tree.append(*orchard_note_commitment).unwrap(); + } + } new_transactions.push(Arc::new(transaction)); } } @@ -426,9 +454,61 @@ impl Block { // delete invalid transactions block.transactions = new_transactions; - // TODO: if needed, fixup after modifying the block: - // - history and authorizing data commitments - // - the transaction merkle root + // fix commitment (must be done after finishing changing the block) + if generate_valid_commitments { + let current_height = block.coinbase_height().unwrap(); + let heartwood_height = NetworkUpgrade::Heartwood + .activation_height(current.network) + .unwrap(); + let nu5_height = NetworkUpgrade::Nu5.activation_height(current.network); + match current_height.cmp(&heartwood_height) { + std::cmp::Ordering::Less => {} + std::cmp::Ordering::Equal => { + block.header.commitment_bytes = [0u8; 32]; + } + std::cmp::Ordering::Greater => { + let history_tree_root = match &history_tree { + Some(tree) => tree.hash().unwrap_or_else(|| [0u8; 32].into()), + None => [0u8; 32].into(), + }; + if nu5_height.is_some() && current_height >= nu5_height.unwrap() { + // From zebra-state/src/service/check.rs + let auth_data_root = block.auth_data_root(); + let hash_block_commitments = + ChainHistoryBlockTxAuthCommitmentHash::from_commitments( + &history_tree_root, + &auth_data_root, + ); + block.header.commitment_bytes = hash_block_commitments.into(); + } else { + block.header.commitment_bytes = history_tree_root.into(); + } + } + } + // update history tree for the next block + if history_tree.is_none() { + history_tree = Some( + HistoryTree::from_block( + current.network, + Arc::new(block.clone()), + &sapling_tree.root(), + &orchard_tree.root(), + ) + .unwrap(), + ); + } else { + history_tree + .as_mut() + .unwrap() + .push( + current.network, + Arc::new(block.clone()), + sapling_tree.root(), + orchard_tree.root(), + ) + .unwrap(); + } + } // now that we've made all the changes, calculate our block hash, // so the next block can use it diff --git a/zebra-chain/src/block/commitment.rs b/zebra-chain/src/block/commitment.rs index eb2dda6a3..8b98125ff 100644 --- a/zebra-chain/src/block/commitment.rs +++ b/zebra-chain/src/block/commitment.rs @@ -2,10 +2,13 @@ use thiserror::Error; +use std::convert::TryInto; + use crate::parameters::{Network, NetworkUpgrade, NetworkUpgrade::*}; use crate::sapling; use super::super::block; +use super::merkle::AuthDataRoot; /// Zcash blocks contain different kinds of commitments to their contents, /// depending on the network and height. @@ -161,11 +164,6 @@ impl From for [u8; 32] { /// - the transaction authorising data in this block. /// /// Introduced in NU5. -// -// TODO: -// - add auth data type -// - add a method for hashing chain history and auth data together -// - move to a separate file #[derive(Clone, Copy, Debug, Eq, PartialEq, Serialize, Deserialize)] pub struct ChainHistoryBlockTxAuthCommitmentHash([u8; 32]); @@ -181,6 +179,40 @@ impl From for [u8; 32] { } } +impl ChainHistoryBlockTxAuthCommitmentHash { + /// Compute the block commitment from the history tree root and the + /// authorization data root, as specified in [ZIP-244]. + /// + /// `history_tree_root` is the root of the history tree up to and including + /// the *previous* block. + /// `auth_data_root` is the root of the Merkle tree of authorizing data + /// commmitments of each transaction in the *current* block. + /// + /// [ZIP-244]: https://zips.z.cash/zip-0244#block-header-changes + pub fn from_commitments( + history_tree_root: &ChainHistoryMmrRootHash, + auth_data_root: &AuthDataRoot, + ) -> Self { + // > 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] + 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"); + Self(hash_block_commitments) + } +} + /// 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/tests/prop.rs b/zebra-chain/src/block/tests/prop.rs index 8399fa0ed..b7bc34b64 100644 --- a/zebra-chain/src/block/tests/prop.rs +++ b/zebra-chain/src/block/tests/prop.rs @@ -173,6 +173,7 @@ fn genesis_partial_chain_strategy() -> Result<()> { init, PREVOUTS_CHAIN_HEIGHT, allow_all_transparent_coinbase_spends, + false, ) }); @@ -222,6 +223,7 @@ fn arbitrary_height_partial_chain_strategy() -> Result<()> { init, PREVOUTS_CHAIN_HEIGHT, allow_all_transparent_coinbase_spends, + false, ) }); diff --git a/zebra-chain/src/parameters/network_upgrade.rs b/zebra-chain/src/parameters/network_upgrade.rs index 597eacc1e..297dc0d17 100644 --- a/zebra-chain/src/parameters/network_upgrade.rs +++ b/zebra-chain/src/parameters/network_upgrade.rs @@ -48,6 +48,7 @@ pub enum NetworkUpgrade { /// /// This is actually a bijective map, but it is const, so we use a vector, and /// do the uniqueness check in the unit tests. +#[cfg(not(test_fake_activation_heights))] pub(crate) const MAINNET_ACTIVATION_HEIGHTS: &[(block::Height, NetworkUpgrade)] = &[ (block::Height(0), Genesis), (block::Height(1), BeforeOverwinter), @@ -59,10 +60,23 @@ pub(crate) const MAINNET_ACTIVATION_HEIGHTS: &[(block::Height, NetworkUpgrade)] // TODO: Add Nu5 mainnet activation height ]; +#[cfg(test_fake_activation_heights)] +pub(crate) const MAINNET_ACTIVATION_HEIGHTS: &[(block::Height, NetworkUpgrade)] = &[ + (block::Height(0), Genesis), + (block::Height(5), BeforeOverwinter), + (block::Height(10), Overwinter), + (block::Height(15), Sapling), + (block::Height(20), Blossom), + (block::Height(25), Heartwood), + (block::Height(30), Canopy), + (block::Height(35), Nu5), +]; + /// Testnet network upgrade activation heights. /// /// This is actually a bijective map, but it is const, so we use a vector, and /// do the uniqueness check in the unit tests. +#[cfg(not(test_fake_activation_heights))] pub(crate) const TESTNET_ACTIVATION_HEIGHTS: &[(block::Height, NetworkUpgrade)] = &[ (block::Height(0), Genesis), (block::Height(1), BeforeOverwinter), @@ -74,6 +88,18 @@ pub(crate) const TESTNET_ACTIVATION_HEIGHTS: &[(block::Height, NetworkUpgrade)] // TODO: Add Nu5 testnet activation height ]; +#[cfg(test_fake_activation_heights)] +pub(crate) const TESTNET_ACTIVATION_HEIGHTS: &[(block::Height, NetworkUpgrade)] = &[ + (block::Height(0), Genesis), + (block::Height(5), BeforeOverwinter), + (block::Height(10), Overwinter), + (block::Height(15), Sapling), + (block::Height(20), Blossom), + (block::Height(25), Heartwood), + (block::Height(30), Canopy), + (block::Height(35), Nu5), +]; + /// The Consensus Branch Id, used to bind transactions and blocks to a /// particular network upgrade. #[derive(Copy, Clone, Debug, Eq, Hash, PartialEq)] diff --git a/zebra-consensus/src/checkpoint.rs b/zebra-consensus/src/checkpoint.rs index b87c68e84..23fc6124e 100644 --- a/zebra-consensus/src/checkpoint.rs +++ b/zebra-consensus/src/checkpoint.rs @@ -17,7 +17,7 @@ use std::{ collections::BTreeMap, ops::{Bound, Bound::*}, pin::Pin, - sync::Arc, + sync::{mpsc, Arc}, task::{Context, Poll}, }; @@ -96,6 +96,26 @@ pub const MAX_CHECKPOINT_HEIGHT_GAP: usize = 400; /// serialized size. pub const MAX_CHECKPOINT_BYTE_COUNT: u64 = 32 * 1024 * 1024; +/// Convert a tip into its hash and matching progress. +fn progress_from_tip( + checkpoint_list: &CheckpointList, + tip: Option<(block::Height, block::Hash)>, +) -> (Option, Progress) { + match tip { + Some((height, hash)) => { + if height >= checkpoint_list.max_height() { + (None, Progress::FinalCheckpoint) + } else { + metrics::gauge!("checkpoint.verified.height", height.0 as f64); + metrics::gauge!("checkpoint.processing.next.height", height.0 as f64); + (Some(hash), Progress::InitialTip(height)) + } + } + // We start by verifying the genesis block, by itself + None => (None, Progress::BeforeGenesis), + } +} + /// A checkpointing block verifier. /// /// Verifies blocks using a supplied list of checkpoints. There must be at @@ -132,6 +152,13 @@ where /// The current progress of this verifier. verifier_progress: Progress, + + /// A channel to receive requests to reset the verifier, + /// receiving the tip of the state. + reset_receiver: mpsc::Receiver>, + /// A channel to send requests to reset the verifier, + /// passing the tip of the state. + reset_sender: mpsc::Sender>, } impl CheckpointVerifier @@ -212,19 +239,10 @@ where ) -> Self { // All the initialisers should call this function, so we only have to // change fields or default values in one place. - let (initial_tip_hash, verifier_progress) = match initial_tip { - Some((height, hash)) => { - if height >= checkpoint_list.max_height() { - (None, Progress::FinalCheckpoint) - } else { - metrics::gauge!("checkpoint.verified.height", height.0 as f64); - metrics::gauge!("checkpoint.processing.next.height", height.0 as f64); - (Some(hash), Progress::InitialTip(height)) - } - } - // We start by verifying the genesis block, by itself - None => (None, Progress::BeforeGenesis), - }; + let (initial_tip_hash, verifier_progress) = + progress_from_tip(&checkpoint_list, initial_tip); + + let (sender, receiver) = mpsc::channel(); CheckpointVerifier { checkpoint_list, network, @@ -232,9 +250,18 @@ where state_service, queued: BTreeMap::new(), verifier_progress, + reset_receiver: receiver, + reset_sender: sender, } } + /// Reset the verifier progress back to given tip. + fn reset_progress(&mut self, tip: Option<(block::Height, block::Hash)>) { + let (initial_tip_hash, verifier_progress) = progress_from_tip(&self.checkpoint_list, tip); + self.initial_tip_hash = initial_tip_hash; + self.verifier_progress = verifier_progress; + } + /// Return the current verifier's progress. /// /// If verification has not started yet, returns `BeforeGenesis`, @@ -825,6 +852,8 @@ pub enum VerifyCheckpointError { #[error(transparent)] CommitFinalized(BoxError), #[error(transparent)] + Tip(BoxError), + #[error(transparent)] CheckpointList(BoxError), #[error(transparent)] VerifyBlock(BoxError), @@ -876,6 +905,12 @@ where #[instrument(name = "checkpoint", skip(self, block))] fn call(&mut self, block: Arc) -> Self::Future { + // Reset the verifier back to the state tip if requested + // (e.g. due to an error when committing a block to to the state) + if let Ok(tip) = self.reset_receiver.try_recv() { + self.reset_progress(tip); + } + // Immediately reject all incoming blocks that arrive after we've finished. if let FinalCheckpoint = self.previous_checkpoint_height() { return async { Err(VerifyCheckpointError::Finished) }.boxed(); @@ -910,17 +945,12 @@ where .map_err(VerifyCheckpointError::CommitFinalized) .expect("CheckpointVerifier does not leave dangling receivers")?; - // Once we get a verified hash, we must commit it to the chain state - // as a finalized block, or exit the program, so .expect rather than - // propagate errors from the state service. - // // We use a `ServiceExt::oneshot`, so that every state service // `poll_ready` has a corresponding `call`. See #1593. match state_service .oneshot(zs::Request::CommitFinalizedBlock(block.into())) .map_err(VerifyCheckpointError::CommitFinalized) - .await - .expect("state service commit block failed: verified checkpoints must be committed transactionally") + .await? { zs::Response::Committed(committed_hash) => { assert_eq!(committed_hash, hash, "state must commit correct hash"); @@ -930,6 +960,8 @@ where } }); + let state_service = self.state_service.clone(); + let reset_sender = self.reset_sender.clone(); async move { let result = commit_finalized_block.await; // Avoid a panic on shutdown @@ -939,11 +971,29 @@ where // so we don't need to panic here. The persistent state is correct even when the // task is cancelled, because block data is committed inside transactions, in // height order. - if zebra_chain::shutdown::is_shutting_down() { + let result = if zebra_chain::shutdown::is_shutting_down() { Err(VerifyCheckpointError::ShuttingDown) } else { result.expect("commit_finalized_block should not panic") + }; + if result.is_err() { + // If there was an error comitting the block, then this verifier + // will be out of sync with the state. In that case, reset + // its progress back to the state tip. + let tip = match state_service + .oneshot(zs::Request::Tip) + .await + .map_err(Into::into) + .map_err(VerifyCheckpointError::Tip)? + { + zs::Response::Tip(tip) => tip, + _ => unreachable!("wrong response for Tip"), + }; + // Ignore errors since send() can fail only when the verifier + // is being dropped, and then it doesn't matter anymore. + let _ = reset_sender.send(tip); } + result } .boxed() } diff --git a/zebra-state/Cargo.toml b/zebra-state/Cargo.toml index 89c540b80..00a30e8a6 100644 --- a/zebra-state/Cargo.toml +++ b/zebra-state/Cargo.toml @@ -32,7 +32,6 @@ 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/build.rs b/zebra-state/build.rs new file mode 100644 index 000000000..91ba008f7 --- /dev/null +++ b/zebra-state/build.rs @@ -0,0 +1,9 @@ +use std::env; + +fn main() { + let use_fake_heights = env::var_os("TEST_FAKE_ACTIVATION_HEIGHTS").is_some(); + println!("cargo:rerun-if-env-changed=TEST_FAKE_ACTIVATION_HEIGHTS"); + if use_fake_heights { + println!("cargo:rustc-cfg=test_fake_activation_heights"); + } +} diff --git a/zebra-state/src/service/arbitrary.rs b/zebra-state/src/service/arbitrary.rs index 5110c1599..3d99cf6b0 100644 --- a/zebra-state/src/service/arbitrary.rs +++ b/zebra-state/src/service/arbitrary.rs @@ -58,6 +58,7 @@ pub struct PreparedChain { chain: std::sync::Mutex>>, HistoryTree)>>, // the strategy for generating LedgerStates. If None, it calls [`LedgerState::genesis_strategy`]. ledger_strategy: Option>, + generate_valid_commitments: bool, } impl PreparedChain { @@ -87,6 +88,15 @@ impl PreparedChain { ..Default::default() } } + + /// Transform the strategy to use valid commitments in the block. + /// + /// This is slower so it should be used only when needed. + #[allow(dead_code)] + pub(crate) fn with_valid_commitments(mut self) -> Self { + self.generate_valid_commitments = true; + self + } } impl Strategy for PreparedChain { @@ -112,6 +122,7 @@ impl Strategy for PreparedChain { ledger, MAX_PARTIAL_CHAIN_BLOCKS, check::utxo::transparent_coinbase_spend, + self.generate_valid_commitments, ), ) }) diff --git a/zebra-state/src/service/check.rs b/zebra-state/src/service/check.rs index c69b95de0..88cc4ba26 100644 --- a/zebra-state/src/service/check.rs +++ b/zebra-state/src/service/check.rs @@ -1,18 +1,18 @@ //! Consensus critical contextual checks -use std::{borrow::Borrow, convert::TryInto}; +use std::{borrow::Borrow, sync::Arc}; use chrono::Duration; use zebra_chain::{ - block::{self, Block, CommitmentError}, + block::{self, Block, ChainHistoryBlockTxAuthCommitmentHash, CommitmentError}, history_tree::HistoryTree, parameters::POW_AVERAGING_WINDOW, parameters::{Network, NetworkUpgrade}, work::difficulty::CompactDifficulty, }; -use crate::{PreparedBlock, ValidateContextError}; +use crate::{FinalizedBlock, PreparedBlock, ValidateContextError}; use super::check; @@ -103,12 +103,31 @@ where /// 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( +pub(crate) fn prepared_block_commitment_is_valid_for_chain_history( prepared: &PreparedBlock, network: Network, history_tree: &HistoryTree, ) -> Result<(), ValidateContextError> { - match prepared.block.commitment(network)? { + block_commitment_is_valid_for_chain_history(prepared.block.clone(), network, history_tree) +} + +/// Check that the `finalized` block is contextually valid for `network`, using +/// the `history_tree` up to and including the previous block. +#[tracing::instrument(skip(finalized, history_tree))] +pub(crate) fn finalized_block_commitment_is_valid_for_chain_history( + finalized: &FinalizedBlock, + network: Network, + history_tree: &HistoryTree, +) -> Result<(), ValidateContextError> { + block_commitment_is_valid_for_chain_history(finalized.block.clone(), network, history_tree) +} + +fn block_commitment_is_valid_for_chain_history( + block: Arc, + network: Network, + history_tree: &HistoryTree, +) -> Result<(), ValidateContextError> { + match block.commitment(network)? { block::Commitment::PreSaplingReserved(_) | block::Commitment::FinalSaplingRoot(_) | block::Commitment::ChainHistoryActivationReserved => { @@ -131,37 +150,23 @@ pub(crate) fn block_commitment_is_valid_for_chain_history( } } 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(); + let auth_data_root = 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"); + let hash_block_commitments = ChainHistoryBlockTxAuthCommitmentHash::from_commitments( + &history_tree_root, + &auth_data_root, + ); - if actual_block_commitments == hash_block_commitments { + if actual_hash_block_commitments == hash_block_commitments { Ok(()) } else { Err(ValidateContextError::InvalidBlockCommitment( CommitmentError::InvalidChainHistoryBlockTxAuthCommitment { - actual: actual_block_commitments, - expected: hash_block_commitments, + actual: actual_hash_block_commitments.into(), + expected: hash_block_commitments.into(), }, )) } diff --git a/zebra-state/src/service/finalized_state.rs b/zebra-state/src/service/finalized_state.rs index 1d971daa2..97d6d9f56 100644 --- a/zebra-state/src/service/finalized_state.rs +++ b/zebra-state/src/service/finalized_state.rs @@ -19,7 +19,7 @@ use zebra_chain::{ value_balance::ValueBalance, }; -use crate::{BoxError, Config, FinalizedBlock, HashOrHeight}; +use crate::{service::check, BoxError, Config, FinalizedBlock, HashOrHeight}; use self::disk_format::{DiskDeserialize, DiskSerialize, FromDisk, IntoDisk, TransactionLocation}; @@ -207,6 +207,8 @@ impl FinalizedState { /// /// - Propagates any errors from writing to the DB /// - Propagates any errors from updating history and note commitment trees + /// - If `hashFinalSaplingRoot` / `hashLightClientRoot` / `hashBlockCommitments` + /// does not match the expected value pub fn commit_finalized_direct( &mut self, finalized: FinalizedBlock, @@ -214,14 +216,6 @@ impl FinalizedState { ) -> Result { block_precommit_metrics(&finalized); - let FinalizedBlock { - block, - hash, - height, - new_outputs, - transaction_hashes, - } = finalized; - let finalized_tip_height = self.finalized_tip_height(); let hash_by_height = self.db.cf_handle("hash_by_height").unwrap(); @@ -248,27 +242,27 @@ impl FinalizedState { // Assert that callers (including unit tests) get the chain order correct if self.is_empty(hash_by_height) { assert_eq!( - GENESIS_PREVIOUS_BLOCK_HASH, block.header.previous_block_hash, + GENESIS_PREVIOUS_BLOCK_HASH, finalized.block.header.previous_block_hash, "the first block added to an empty state must be a genesis block, source: {}", source, ); assert_eq!( block::Height(0), - height, + finalized.height, "cannot commit genesis: invalid height, source: {}", source, ); } else { assert_eq!( finalized_tip_height.expect("state must have a genesis block committed") + 1, - Some(height), + Some(finalized.height), "committed block height must be 1 more than the finalized tip height, source: {}", source, ); assert_eq!( self.finalized_tip_hash(), - block.header.previous_block_hash, + finalized.block.header.previous_block_hash, "committed block must be a child of the finalized tip, source: {}", source, ); @@ -280,6 +274,27 @@ impl FinalizedState { let mut orchard_note_commitment_tree = self.orchard_note_commitment_tree(); let mut history_tree = self.history_tree(); + // Check the block commitment. For Nu5-onward, the block hash commits only + // to non-authorizing data (see ZIP-244). This checks the authorizing data + // commitment, making sure the entire block contents were commited to. + // The test is done here (and not during semantic validation) because it needs + // the history tree root. While it _is_ checked during contextual validation, + // that is not called by the checkpoint verifier, and keeping a history tree there + // would be harder to implement. + check::finalized_block_commitment_is_valid_for_chain_history( + &finalized, + self.network, + &history_tree, + )?; + + let FinalizedBlock { + block, + hash, + height, + new_outputs, + transaction_hashes, + } = finalized; + // Prepare a batch of DB modifications and return it (without actually writing anything). // We use a closure so we can use an early return for control flow in // the genesis case. diff --git a/zebra-state/src/service/finalized_state/tests/prop.rs b/zebra-state/src/service/finalized_state/tests/prop.rs index 232ebfc95..854250e74 100644 --- a/zebra-state/src/service/finalized_state/tests/prop.rs +++ b/zebra-state/src/service/finalized_state/tests/prop.rs @@ -1,6 +1,6 @@ use std::env; -use zebra_chain::block::Height; +use zebra_chain::{block::Height, parameters::NetworkUpgrade}; use zebra_test::prelude::*; use crate::{ @@ -9,6 +9,7 @@ use crate::{ arbitrary::PreparedChain, finalized_state::{FinalizedBlock, FinalizedState}, }, + tests::FakeChainHelper, }; const DEFAULT_PARTIAL_CHAIN_PROPTEST_CASES: u32 = 1; @@ -38,3 +39,59 @@ fn blocks_with_v5_transactions() -> Result<()> { Ok(()) } + +/// Test if committing blocks from all upgrades work correctly, to make +/// sure the contextual validation done by the finalized state works. +/// Also test if a block with the wrong commitment is correctly rejected. +#[allow(dead_code)] +#[cfg_attr(test_fake_activation_heights, test)] +fn all_upgrades_and_wrong_commitments_with_fake_activation_heights() -> Result<()> { + zebra_test::init(); + // Use no_shrink() because we're ignoring _count and there is nothing to actually shrink. + proptest!(ProptestConfig::with_cases(env::var("PROPTEST_CASES") + .ok() + .and_then(|v| v.parse().ok()) + .unwrap_or(DEFAULT_PARTIAL_CHAIN_PROPTEST_CASES)), + |((chain, _count, network, _history_tree) in PreparedChain::default().with_valid_commitments().no_shrink())| { + + let mut state = FinalizedState::new(&Config::ephemeral(), network); + let mut height = Height(0); + let heartwood_height = NetworkUpgrade::Heartwood.activation_height(network).unwrap(); + let heartwood_height_plus1 = (heartwood_height + 1).unwrap(); + let nu5_height = NetworkUpgrade::Nu5.activation_height(network).unwrap(); + let nu5_height_plus1 = (nu5_height + 1).unwrap(); + + let mut failure_count = 0; + for block in chain.iter() { + let block_hash = block.hash; + let current_height = block.block.coinbase_height().unwrap(); + // For some specific heights, try to commit a block with + // corrupted commitment. + match current_height { + h if h == heartwood_height || + h == heartwood_height_plus1 || + h == nu5_height || + h == nu5_height_plus1 => { + let block = block.block.clone().set_block_commitment([0x42; 32]); + state.commit_finalized_direct( + FinalizedBlock::from(block), + "all_upgrades test" + ).expect_err("Must fail commitment check"); + failure_count += 1; + }, + _ => {}, + } + let hash = state.commit_finalized_direct( + FinalizedBlock::from(block.block.clone()), + "all_upgrades test" + ).unwrap(); + prop_assert_eq!(Some(height), state.finalized_tip_height()); + prop_assert_eq!(hash, block_hash); + height = Height(height.0 + 1); + } + // Make sure the failure path was triggered + prop_assert_eq!(failure_count, 4); + }); + + Ok(()) +} diff --git a/zebra-state/src/service/non_finalized_state.rs b/zebra-state/src/service/non_finalized_state.rs index 3a73d8f25..f42097475 100644 --- a/zebra-state/src/service/non_finalized_state.rs +++ b/zebra-state/src/service/non_finalized_state.rs @@ -191,7 +191,7 @@ impl NonFinalizedState { &parent_chain.spent_utxos, finalized_state, )?; - check::block_commitment_is_valid_for_chain_history( + check::prepared_block_commitment_is_valid_for_chain_history( &prepared, self.network, &parent_chain.history_tree, diff --git a/zebra-state/src/service/non_finalized_state/tests/prop.rs b/zebra-state/src/service/non_finalized_state/tests/prop.rs index a9516eb36..cc066e9e1 100644 --- a/zebra-state/src/service/non_finalized_state/tests/prop.rs +++ b/zebra-state/src/service/non_finalized_state/tests/prop.rs @@ -244,7 +244,7 @@ fn different_blocks_different_chains() -> Result<()> { if is_nu5 && is_v5 { 5 } else { 4 }, true, )}) - .prop_map(|ledger_state| Block::partial_chain_strategy(ledger_state, 2, allow_all_transparent_coinbase_spends)) + .prop_map(|ledger_state| Block::partial_chain_strategy(ledger_state, 2, allow_all_transparent_coinbase_spends, false)) .prop_flat_map(|block_strategy| (block_strategy.clone(), block_strategy)) )| { let prev_block1 = vec1[0].clone(); diff --git a/zebra-state/src/tests/setup.rs b/zebra-state/src/tests/setup.rs index d490a561a..d61d88d05 100644 --- a/zebra-state/src/tests/setup.rs +++ b/zebra-state/src/tests/setup.rs @@ -70,6 +70,7 @@ pub(crate) fn partial_nu5_chain_strategy( init, blocks_after_nu_activation as usize, check::utxo::transparent_coinbase_spend, + false, ) }) .prop_map(move |partial_chain| (network, nu_activation, partial_chain))