From 5859fac5b12170a93e9b872321dafa529caa07a2 Mon Sep 17 00:00:00 2001 From: Alfredo Garcia Date: Tue, 4 Jul 2023 18:29:41 -0300 Subject: [PATCH] docs(state): Use different terms for block verification and state queues (#7061) * claridy some checkpoint verifier docs * update documentation of `Request::CommitSemanticallyVerifiedBlock` and `Request::CommitCheckpointVerifiedBlock` * replace `prepared` with `semantically_verified` in state service checks code * replace `non-finalized` where needed in docs of the state service * fix double space in doc * replace `finalized` where needed in docs of the state service * change some docs in state queued_blocks.rs * Rewrite pending UTXO checkpoint block comment * Fix trailing space in docs * Apply suggestions from code review Co-authored-by: teor --------- Co-authored-by: teor --- zebra-consensus/src/block/check.rs | 2 +- zebra-consensus/src/checkpoint.rs | 6 +-- zebra-state/src/request.rs | 26 +++++----- zebra-state/src/service.rs | 56 +++++++++++----------- zebra-state/src/service/check.rs | 24 +++++----- zebra-state/src/service/check/anchors.rs | 29 ++++++----- zebra-state/src/service/check/nullifier.rs | 10 ++-- zebra-state/src/service/check/utxo.rs | 33 ++++++++----- zebra-state/src/service/queued_blocks.rs | 8 ++-- 9 files changed, 103 insertions(+), 91 deletions(-) diff --git a/zebra-consensus/src/block/check.rs b/zebra-consensus/src/block/check.rs index ddd3dbef..5f4aaa6c 100644 --- a/zebra-consensus/src/block/check.rs +++ b/zebra-consensus/src/block/check.rs @@ -315,7 +315,7 @@ pub fn merkle_root_validity( // // Duplicate transactions should cause a block to be // rejected, as duplicate transactions imply that the block contains a - // double-spend. As a defense-in-depth, however, we also check that there + // double-spend. As a defense-in-depth, however, we also check that there // are no duplicate transaction hashes. // // ## Checkpoint Validation diff --git a/zebra-consensus/src/checkpoint.rs b/zebra-consensus/src/checkpoint.rs index 2334383b..adbe69de 100644 --- a/zebra-consensus/src/checkpoint.rs +++ b/zebra-consensus/src/checkpoint.rs @@ -4,11 +4,11 @@ //! speed up the initial chain sync for Zebra. This list is distributed //! with Zebra. //! -//! The checkpoint verifier queues pending blocks. Once there is a +//! The checkpoint verifier queues pending blocks. Once there is a //! chain from the previous checkpoint to a target checkpoint, it //! verifies all the blocks in that chain, and sends accepted blocks to -//! the state service as finalized chain state, skipping contextual -//! verification checks. +//! the state service as finalized chain state, skipping the majority of +//! contextual verification checks. //! //! Verification starts at the first checkpoint, which is the genesis //! block for the configured network. diff --git a/zebra-state/src/request.rs b/zebra-state/src/request.rs index 5a942d99..cc3df2fd 100644 --- a/zebra-state/src/request.rs +++ b/zebra-state/src/request.rs @@ -163,7 +163,7 @@ pub struct SemanticallyVerifiedBlock { } /// A block ready to be committed directly to the finalized state with -/// no checks. +/// a small number of checks if compared with a `ContextuallyVerifiedBlock`. /// /// This is exposed for use in checkpointing. /// @@ -455,12 +455,11 @@ impl DerefMut for CheckpointVerifiedBlock { /// A query about or modification to the chain state, via the /// [`StateService`](crate::service::StateService). pub enum Request { - /// Performs contextual validation of the given block, committing it to the - /// state if successful. + /// Performs contextual validation of the given semantically verified block, + /// committing it to the state if successful. /// - /// It is the caller's responsibility to perform semantic validation. This - /// request can be made out-of-order; the state service will queue it until - /// its parent is ready. + /// This request can be made out-of-order; the state service will queue it + /// until its parent is ready. /// /// Returns [`Response::Committed`] with the hash of the block when it is /// committed to the state, or an error if the block fails contextual @@ -478,12 +477,12 @@ pub enum Request { /// documentation for details. CommitSemanticallyVerifiedBlock(SemanticallyVerifiedBlock), - /// Commit a checkpointed block to the state, skipping most block validation. + /// Commit a checkpointed block to the state, skipping most but not all + /// contextual validation. /// - /// This is exposed for use in checkpointing, which produces finalized - /// blocks. It is the caller's responsibility to ensure that the block is - /// semantically valid and final. This request can be made out-of-order; - /// the state service will queue it until its parent is ready. + /// This is exposed for use in checkpointing, which produces checkpoint vefified + /// blocks. This request can be made out-of-order; the state service will queue + /// it until its parent is ready. /// /// Returns [`Response::Committed`] with the hash of the newly committed /// block, or an error. @@ -495,8 +494,9 @@ pub enum Request { /// /// # Note /// - /// Finalized and non-finalized blocks are an internal Zebra implementation detail. - /// There is no difference between these blocks on the network, or in Zebra's + /// [`SemanticallyVerifiedBlock`], [`ContextuallyVerifiedBlock`] and + /// [`CheckpointVerifiedBlock`] are an internal Zebra implementation detail. + /// There is no difference between these blocks on the Zcash network, or in Zebra's /// network or syncer implementations. /// /// # Consensus diff --git a/zebra-state/src/service.rs b/zebra-state/src/service.rs index 0e7c96d1..897903ed 100644 --- a/zebra-state/src/service.rs +++ b/zebra-state/src/service.rs @@ -141,7 +141,7 @@ pub(crate) struct StateService { /// so they can be written to the [`FinalizedState`]. /// /// This sender is dropped after the state has finished sending all the checkpointed blocks, - /// and the lowest non-finalized block arrives. + /// and the lowest semantically verified block arrives. finalized_block_write_sender: Option>, @@ -154,11 +154,6 @@ pub(crate) struct StateService { /// /// If `invalid_block_write_reset_receiver` gets a reset, this is: /// - the hash of the last valid committed block (the parent of the invalid block). - // - // TODO: - // - turn this into an IndexMap containing recent non-finalized block hashes and heights - // (they are all potential tips) - // - remove block hashes once their heights are strictly less than the finalized tip finalized_block_write_last_sent_hash: block::Hash, /// A set of block hashes that have been sent to the block write task. @@ -455,7 +450,7 @@ impl StateService { (state, read_service, latest_chain_tip, chain_tip_change) } - /// Queue a finalized block for verification and storage in the finalized state. + /// Queue a checkpoint verified block for verification and storage in the finalized state. /// /// Returns a channel receiver that provides the result of the block commit. fn queue_and_commit_to_finalized_state( @@ -471,7 +466,7 @@ impl StateService { let queued_height = checkpoint_verified.height; // If we're close to the final checkpoint, make the block's UTXOs available for - // full verification of non-finalized blocks, even when it is in the channel. + // semantic block verification, even when it is in the channel. if self.is_close_to_final_checkpoint(queued_height) { self.non_finalized_block_write_sent_hashes .add_finalized(&checkpoint_verified) @@ -481,32 +476,32 @@ impl StateService { let queued = (checkpoint_verified, rsp_tx); if self.finalized_block_write_sender.is_some() { - // We're still committing finalized blocks + // We're still committing checkpoint verified blocks if let Some(duplicate_queued) = self .finalized_state_queued_blocks .insert(queued_prev_hash, queued) { Self::send_checkpoint_verified_block_error( duplicate_queued, - "dropping older finalized block: got newer duplicate block", + "dropping older checkpoint verified block: got newer duplicate block", ); } self.drain_finalized_queue_and_commit(); } else { - // We've finished committing finalized blocks, so drop any repeated queued blocks, - // and return an error. + // We've finished committing checkpoint verified blocks to the finalized state, + // so drop any repeated queued blocks, and return an error. // // TODO: track the latest sent height, and drop any blocks under that height // every time we send some blocks (like QueuedSemanticallyVerifiedBlocks) Self::send_checkpoint_verified_block_error( queued, - "already finished committing finalized blocks: dropped duplicate block, \ + "already finished committing checkpoint verified blocks: dropped duplicate block, \ block is already committed to the state", ); self.clear_finalized_block_queue( - "already finished committing finalized blocks: dropped duplicate block, \ + "already finished committing checkpoint verified blocks: dropped duplicate block, \ block is already committed to the state", ); } @@ -636,7 +631,7 @@ impl StateService { std::mem::drop(finalized); } - /// Queue a non finalized block for verification and check if any queued + /// Queue a semantically verified block for contextual verification and check if any queued /// blocks are ready to be verified and committed to the state. /// /// This function encodes the logic for [committing non-finalized blocks][1] @@ -694,8 +689,8 @@ impl StateService { rsp_rx }; - // We've finished sending finalized blocks when: - // - we've sent the finalized block for the last checkpoint, and + // We've finished sending checkpoint verified blocks when: + // - we've sent the verified block for the last checkpoint, and // - it has been successfully written to disk. // // We detect the last checkpoint by looking for non-finalized blocks @@ -709,13 +704,13 @@ impl StateService { && self.read_service.db.finalized_tip_hash() == self.finalized_block_write_last_sent_hash { - // Tell the block write task to stop committing finalized blocks, - // and move on to committing non-finalized blocks. + // Tell the block write task to stop committing checkpoint verified blocks to the finalized state, + // and move on to committing semantically verified blocks to the non-finalized state. std::mem::drop(self.finalized_block_write_sender.take()); - // We've finished committing finalized blocks, so drop any repeated queued blocks. + // We've finished committing checkpoint verified blocks to finalized state, so drop any repeated queued blocks. self.clear_finalized_block_queue( - "already finished committing finalized blocks: dropped duplicate block, \ + "already finished committing checkpoint verified blocks: dropped duplicate block, \ block is already committed to the state", ); } @@ -754,7 +749,7 @@ impl StateService { /// Returns `true` if `queued_height` is near the final checkpoint. /// - /// The non-finalized block verifier needs access to UTXOs from finalized blocks + /// The semantic block verifier needs access to UTXOs from checkpoint verified blocks /// near the final checkpoint, so that it can verify blocks that spend those UTXOs. /// /// If it doesn't have the required UTXOs, some blocks will time out, @@ -818,7 +813,7 @@ impl StateService { // required by `Request::CommitSemanticallyVerifiedBlock` call assert!( block.height > self.network.mandatory_checkpoint_height(), - "invalid non-finalized block height: the canopy checkpoint is mandatory, pre-canopy \ + "invalid semantically verified block height: the canopy checkpoint is mandatory, pre-canopy \ blocks, and the canopy activation block, must be committed to the state as finalized \ blocks" ); @@ -970,11 +965,16 @@ impl Service for StateService { Request::CommitCheckpointVerifiedBlock(finalized) => { // # Consensus // - // A non-finalized block verification could have called AwaitUtxo - // before this finalized block arrived in the state. - // So we need to check for pending UTXOs here for non-finalized blocks, - // even though it is redundant for most finalized blocks. - // (Finalized blocks are verified using block hash checkpoints + // A semantic block verification could have called AwaitUtxo + // before this checkpoint verified block arrived in the state. + // So we need to check for pending UTXO requests sent by running + // semantic block verifications. + // + // This check is redundant for most checkpoint verified blocks, + // because semantic verification can only succeed near the final + // checkpoint, when all the UTXOs are available for the verifying block. + // + // (Checkpoint block UTXOs are verified using block hash checkpoints // and transaction merkle tree block header commitments.) self.pending_utxos .check_against_ordered(&finalized.new_outputs); diff --git a/zebra-state/src/service/check.rs b/zebra-state/src/service/check.rs index f1e45010..bd8dd8b8 100644 --- a/zebra-state/src/service/check.rs +++ b/zebra-state/src/service/check.rs @@ -38,8 +38,8 @@ mod tests; pub(crate) use difficulty::AdjustedDifficulty; -/// Check that the `prepared` block is contextually valid for `network`, based -/// on the `finalized_tip_height` and `relevant_chain`. +/// Check that the semantically verified 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. @@ -50,9 +50,9 @@ pub(crate) use difficulty::AdjustedDifficulty; /// # Panics /// /// If the state contains less than 28 ([`POW_ADJUSTMENT_BLOCK_SPAN`]) blocks. -#[tracing::instrument(skip(prepared, finalized_tip_height, relevant_chain))] +#[tracing::instrument(skip(semantically_verified, finalized_tip_height, relevant_chain))] pub(crate) fn block_is_valid_for_recent_chain( - prepared: &SemanticallyVerifiedBlock, + semantically_verified: &SemanticallyVerifiedBlock, network: Network, finalized_tip_height: Option, relevant_chain: C, @@ -64,7 +64,7 @@ where { let finalized_tip_height = finalized_tip_height .expect("finalized state must contain at least one block to do contextual validation"); - check::block_is_not_orphaned(finalized_tip_height, prepared.height)?; + check::block_is_not_orphaned(finalized_tip_height, semantically_verified.height)?; let relevant_chain: Vec<_> = relevant_chain .into_iter() @@ -78,7 +78,7 @@ where let parent_height = parent_block .coinbase_height() .expect("valid blocks have a coinbase height"); - check::height_one_more_than_parent_height(parent_height, prepared.height)?; + check::height_one_more_than_parent_height(parent_height, semantically_verified.height)?; if relevant_chain.len() < POW_ADJUSTMENT_BLOCK_SPAN { // skip this check during tests if we don't have enough blocks in the chain @@ -107,9 +107,9 @@ where ) }); let difficulty_adjustment = - AdjustedDifficulty::new_from_block(&prepared.block, network, relevant_data); + AdjustedDifficulty::new_from_block(&semantically_verified.block, network, relevant_data); check::difficulty_threshold_and_time_are_valid( - prepared.block.header.difficulty_threshold, + semantically_verified.block.header.difficulty_threshold, difficulty_adjustment, )?; @@ -375,23 +375,23 @@ where pub(crate) fn initial_contextual_validity( finalized_state: &ZebraDb, non_finalized_state: &NonFinalizedState, - prepared: &SemanticallyVerifiedBlock, + semantically_verified: &SemanticallyVerifiedBlock, ) -> Result<(), ValidateContextError> { let relevant_chain = any_ancestor_blocks( non_finalized_state, finalized_state, - prepared.block.header.previous_block_hash, + semantically_verified.block.header.previous_block_hash, ); // Security: check proof of work before any other checks check::block_is_valid_for_recent_chain( - prepared, + semantically_verified, non_finalized_state.network, finalized_state.finalized_tip_height(), relevant_chain, )?; - check::nullifier::no_duplicates_in_finalized_chain(prepared, finalized_state)?; + check::nullifier::no_duplicates_in_finalized_chain(semantically_verified, finalized_state)?; Ok(()) } diff --git a/zebra-state/src/service/check/anchors.rs b/zebra-state/src/service/check/anchors.rs index f410abd8..471f3917 100644 --- a/zebra-state/src/service/check/anchors.rs +++ b/zebra-state/src/service/check/anchors.rs @@ -190,7 +190,7 @@ fn fetch_sprout_final_treestates( /// treestate of any prior `JoinSplit` _within the same transaction_. /// /// This method searches for anchors in the supplied `sprout_final_treestates` -/// (which must be populated with all treestates pointed to in the `prepared` block; +/// (which must be populated with all treestates pointed to in the `semantically_verified` block; /// see [`fetch_sprout_final_treestates()`]); or in the interstitial /// treestates which are computed on the fly in this function. #[tracing::instrument(skip(sprout_final_treestates, transaction))] @@ -322,20 +322,23 @@ fn sprout_anchors_refer_to_treestates( pub(crate) fn block_sapling_orchard_anchors_refer_to_final_treestates( finalized_state: &ZebraDb, parent_chain: &Arc, - prepared: &SemanticallyVerifiedBlock, + semantically_verified: &SemanticallyVerifiedBlock, ) -> Result<(), ValidateContextError> { - prepared.block.transactions.iter().enumerate().try_for_each( - |(tx_index_in_block, transaction)| { + semantically_verified + .block + .transactions + .iter() + .enumerate() + .try_for_each(|(tx_index_in_block, transaction)| { sapling_orchard_anchors_refer_to_final_treestates( finalized_state, Some(parent_chain), transaction, - prepared.transaction_hashes[tx_index_in_block], + semantically_verified.transaction_hashes[tx_index_in_block], Some(tx_index_in_block), - Some(prepared.height), + Some(semantically_verified.height), ) - }, - ) + }) } /// Accepts a [`ZebraDb`], [`Arc`](Chain), and [`SemanticallyVerifiedBlock`]. @@ -353,18 +356,20 @@ pub(crate) fn block_sapling_orchard_anchors_refer_to_final_treestates( pub(crate) fn block_fetch_sprout_final_treestates( finalized_state: &ZebraDb, parent_chain: &Arc, - prepared: &SemanticallyVerifiedBlock, + semantically_verified: &SemanticallyVerifiedBlock, ) -> HashMap> { let mut sprout_final_treestates = HashMap::new(); - for (tx_index_in_block, transaction) in prepared.block.transactions.iter().enumerate() { + for (tx_index_in_block, transaction) in + semantically_verified.block.transactions.iter().enumerate() + { fetch_sprout_final_treestates( &mut sprout_final_treestates, finalized_state, Some(parent_chain), transaction, Some(tx_index_in_block), - Some(prepared.height), + Some(semantically_verified.height), ); } @@ -381,7 +386,7 @@ pub(crate) fn block_fetch_sprout_final_treestates( /// treestate of any prior `JoinSplit` _within the same transaction_. /// /// This method searches for anchors in the supplied `sprout_final_treestates` -/// (which must be populated with all treestates pointed to in the `prepared` block; +/// (which must be populated with all treestates pointed to in the `semantically_verified` block; /// see [`fetch_sprout_final_treestates()`]); or in the interstitial /// treestates which are computed on the fly in this function. #[tracing::instrument(skip(sprout_final_treestates, block, transaction_hashes))] diff --git a/zebra-state/src/service/check/nullifier.rs b/zebra-state/src/service/check/nullifier.rs index 4f638b24..809e7838 100644 --- a/zebra-state/src/service/check/nullifier.rs +++ b/zebra-state/src/service/check/nullifier.rs @@ -30,24 +30,24 @@ use crate::service; /// > even if they have the same bit pattern. /// /// -#[tracing::instrument(skip(prepared, finalized_state))] +#[tracing::instrument(skip(semantically_verified, finalized_state))] pub(crate) fn no_duplicates_in_finalized_chain( - prepared: &SemanticallyVerifiedBlock, + semantically_verified: &SemanticallyVerifiedBlock, finalized_state: &ZebraDb, ) -> Result<(), ValidateContextError> { - for nullifier in prepared.block.sprout_nullifiers() { + for nullifier in semantically_verified.block.sprout_nullifiers() { if finalized_state.contains_sprout_nullifier(nullifier) { Err(nullifier.duplicate_nullifier_error(true))?; } } - for nullifier in prepared.block.sapling_nullifiers() { + for nullifier in semantically_verified.block.sapling_nullifiers() { if finalized_state.contains_sapling_nullifier(nullifier) { Err(nullifier.duplicate_nullifier_error(true))?; } } - for nullifier in prepared.block.orchard_nullifiers() { + for nullifier in semantically_verified.block.orchard_nullifiers() { if finalized_state.contains_orchard_nullifier(nullifier) { Err(nullifier.duplicate_nullifier_error(true))?; } diff --git a/zebra-state/src/service/check/utxo.rs b/zebra-state/src/service/check/utxo.rs index c8a79852..186f89d8 100644 --- a/zebra-state/src/service/check/utxo.rs +++ b/zebra-state/src/service/check/utxo.rs @@ -36,14 +36,16 @@ use crate::{ /// - spends of an immature transparent coinbase output, /// - unshielded spends of a transparent coinbase output. pub fn transparent_spend( - prepared: &SemanticallyVerifiedBlock, + semantically_verified: &SemanticallyVerifiedBlock, non_finalized_chain_unspent_utxos: &HashMap, non_finalized_chain_spent_utxos: &HashSet, finalized_state: &ZebraDb, ) -> Result, ValidateContextError> { let mut block_spends = HashMap::new(); - for (spend_tx_index_in_block, transaction) in prepared.block.transactions.iter().enumerate() { + for (spend_tx_index_in_block, transaction) in + semantically_verified.block.transactions.iter().enumerate() + { // Coinbase inputs represent new coins, // so there are no UTXOs to mark as spent. let spends = transaction @@ -55,7 +57,7 @@ pub fn transparent_spend( let utxo = transparent_spend_chain_order( spend, spend_tx_index_in_block, - &prepared.new_outputs, + &semantically_verified.new_outputs, non_finalized_chain_unspent_utxos, non_finalized_chain_spent_utxos, finalized_state, @@ -70,7 +72,8 @@ pub fn transparent_spend( // We don't want to use UTXOs from invalid pending blocks, // so we check transparent coinbase maturity and shielding // using known valid UTXOs during non-finalized chain validation. - let spend_restriction = transaction.coinbase_spend_restriction(prepared.height); + let spend_restriction = + transaction.coinbase_spend_restriction(semantically_verified.height); transparent_coinbase_spend(spend, spend_restriction, utxo.as_ref())?; // We don't delete the UTXOs until the block is committed, @@ -86,7 +89,7 @@ pub fn transparent_spend( } } - remaining_transaction_value(prepared, &block_spends)?; + remaining_transaction_value(semantically_verified, &block_spends)?; Ok(block_spends) } @@ -225,10 +228,12 @@ pub fn transparent_coinbase_spend( /// /// pub fn remaining_transaction_value( - prepared: &SemanticallyVerifiedBlock, + semantically_verified: &SemanticallyVerifiedBlock, utxos: &HashMap, ) -> Result<(), ValidateContextError> { - for (tx_index_in_block, transaction) in prepared.block.transactions.iter().enumerate() { + for (tx_index_in_block, transaction) in + semantically_verified.block.transactions.iter().enumerate() + { if transaction.is_coinbase() { continue; } @@ -243,26 +248,28 @@ pub fn remaining_transaction_value( { Err(ValidateContextError::NegativeRemainingTransactionValue { amount_error, - height: prepared.height, + height: semantically_verified.height, tx_index_in_block, - transaction_hash: prepared.transaction_hashes[tx_index_in_block], + transaction_hash: semantically_verified.transaction_hashes + [tx_index_in_block], }) } Err(amount_error) => { Err(ValidateContextError::CalculateRemainingTransactionValue { amount_error, - height: prepared.height, + height: semantically_verified.height, tx_index_in_block, - transaction_hash: prepared.transaction_hashes[tx_index_in_block], + transaction_hash: semantically_verified.transaction_hashes + [tx_index_in_block], }) } }, Err(value_balance_error) => { Err(ValidateContextError::CalculateTransactionValueBalances { value_balance_error, - height: prepared.height, + height: semantically_verified.height, tx_index_in_block, - transaction_hash: prepared.transaction_hashes[tx_index_in_block], + transaction_hash: semantically_verified.transaction_hashes[tx_index_in_block], }) } }? diff --git a/zebra-state/src/service/queued_blocks.rs b/zebra-state/src/service/queued_blocks.rs index 41e93812..dabd3608 100644 --- a/zebra-state/src/service/queued_blocks.rs +++ b/zebra-state/src/service/queued_blocks.rs @@ -15,13 +15,13 @@ use crate::{BoxError, CheckpointVerifiedBlock, SemanticallyVerifiedBlock}; #[cfg(test)] mod tests; -/// A finalized state queue block, and its corresponding [`Result`] channel. +/// A queued checkpoint verified block, and its corresponding [`Result`] channel. pub type QueuedCheckpointVerified = ( CheckpointVerifiedBlock, oneshot::Sender>, ); -/// A non-finalized state queue block, and its corresponding [`Result`] channel. +/// A queued semantically verified block, and its corresponding [`Result`] channel. pub type QueuedSemanticallyVerified = ( SemanticallyVerifiedBlock, oneshot::Sender>, @@ -264,10 +264,10 @@ impl SentHashes { self.update_metrics_for_block(block.height); } - /// Stores the finalized `block`'s hash, height, and UTXOs, so they can be used to check if a + /// Stores the checkpoint verified `block`'s hash, height, and UTXOs, so they can be used to check if a /// block or UTXO is available in the state. /// - /// Used for finalized blocks close to the final checkpoint, so non-finalized blocks can look up + /// Used for checkpoint verified blocks close to the final checkpoint, so the semantic block verifier can look up /// their UTXOs. /// /// Assumes that blocks are added in the order of their height between `finish_batch` calls