From f0d487a7c1402a9cfe4653993d51b9ec643e2d10 Mon Sep 17 00:00:00 2001 From: Marek Date: Tue, 20 Jun 2023 18:01:22 +0200 Subject: [PATCH] Add and use `FinalizableBlock` This commit adds `FinalizableBlock`, and uses it instead of `ContextuallyVerifiedBlockWithTrees` in `commit_finalized_direct()` --- zebra-state/src/request.rs | 61 ++++++--- zebra-state/src/service/finalized_state.rs | 129 ++++++++++-------- .../src/service/non_finalized_state.rs | 6 +- .../non_finalized_state/tests/vectors.rs | 11 +- 4 files changed, 117 insertions(+), 90 deletions(-) diff --git a/zebra-state/src/request.rs b/zebra-state/src/request.rs index 4a396aa74d9..fb2f3085b9c 100644 --- a/zebra-state/src/request.rs +++ b/zebra-state/src/request.rs @@ -257,41 +257,54 @@ impl Treestate { /// database and recompute a new one. pub struct ContextuallyVerifiedBlockWithTrees { /// A block ready to be committed. - pub block: SemanticallyVerifiedBlock, + pub verified: SemanticallyVerifiedBlock, /// The tresstate associated with the block. - pub treestate: Option, + pub treestate: Treestate, } -impl ContextuallyVerifiedBlockWithTrees { +pub enum FinalizableBlock { + Checkpoint { + checkpoint_verified: CheckpointVerifiedBlock, + }, + Contextual { + contextually_verified: ContextuallyVerifiedBlock, + treestate: Treestate, + }, +} + +impl FinalizableBlock { pub fn new(contextually_verified: ContextuallyVerifiedBlock, treestate: Treestate) -> Self { - Self { - block: SemanticallyVerifiedBlock::from(contextually_verified), - treestate: Some(treestate), + Self::Contextual { + contextually_verified, + treestate, } } -} -impl From> for ContextuallyVerifiedBlockWithTrees { - fn from(block: Arc) -> Self { - Self::from(SemanticallyVerifiedBlock::from(block)) + #[cfg(test)] + pub fn inner_block(&self) -> Arc { + match self { + FinalizableBlock::Checkpoint { + checkpoint_verified, + } => checkpoint_verified.block.clone(), + FinalizableBlock::Contextual { + contextually_verified, + .. + } => contextually_verified.block.clone(), + } } } -impl From for ContextuallyVerifiedBlockWithTrees { - fn from(semantically_verified: SemanticallyVerifiedBlock) -> Self { - Self { - block: semantically_verified, - treestate: None, +impl From for FinalizableBlock { + fn from(checkpoint_verified: CheckpointVerifiedBlock) -> Self { + Self::Checkpoint { + checkpoint_verified, } } } -impl From for ContextuallyVerifiedBlockWithTrees { - fn from(checkpoint_verified: CheckpointVerifiedBlock) -> Self { - Self { - block: checkpoint_verified.0, - treestate: None, - } +impl From> for FinalizableBlock { + fn from(block: Arc) -> Self { + Self::from(CheckpointVerifiedBlock::from(block)) } } @@ -413,6 +426,12 @@ impl From for SemanticallyVerifiedBlock { } } +impl From for SemanticallyVerifiedBlock { + fn from(checkpoint_verified: CheckpointVerifiedBlock) -> Self { + checkpoint_verified.0 + } +} + impl Deref for CheckpointVerifiedBlock { type Target = SemanticallyVerifiedBlock; diff --git a/zebra-state/src/service/finalized_state.rs b/zebra-state/src/service/finalized_state.rs index ca1f5887051..bbb51ec1780 100644 --- a/zebra-state/src/service/finalized_state.rs +++ b/zebra-state/src/service/finalized_state.rs @@ -23,7 +23,7 @@ use std::{ use zebra_chain::{block, parameters::Network}; use crate::{ - request::ContextuallyVerifiedBlockWithTrees, + request::{ContextuallyVerifiedBlockWithTrees, FinalizableBlock, Treestate}, service::{check, QueuedCheckpointVerified}, BoxError, CheckpointVerifiedBlock, CloneError, Config, }; @@ -225,53 +225,22 @@ impl FinalizedState { #[allow(clippy::unwrap_in_result)] pub fn commit_finalized_direct( &mut self, - contextually_verified_with_trees: ContextuallyVerifiedBlockWithTrees, + finalizable_block: FinalizableBlock, source: &str, ) -> Result { - let finalized = contextually_verified_with_trees.block; - let committed_tip_hash = self.db.finalized_tip_hash(); - let committed_tip_height = self.db.finalized_tip_height(); - - // Assert that callers (including unit tests) get the chain order correct - if self.db.is_empty() { - assert_eq!( - committed_tip_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), - finalized.height, - "cannot commit genesis: invalid height, source: {source}", - ); - } else { - assert_eq!( - committed_tip_height.expect("state must have a genesis block committed") + 1, - Some(finalized.height), - "committed block height must be 1 more than the finalized tip height, source: {source}", - ); - - assert_eq!( - committed_tip_hash, finalized.block.header.previous_block_hash, - "committed block must be a child of the finalized tip, source: {source}", - ); - } - - let (history_tree, note_commitment_trees) = match contextually_verified_with_trees.treestate - { - // If the treestate associated with the block was supplied, use it - // without recomputing it. - Some(ref treestate) => ( - treestate.history_tree.clone(), - treestate.note_commitment_trees.clone(), - ), - // If the treestate was not supplied, retrieve a previous treestate - // from the database, and update it for the block being committed. - None => { + let (height, hash, finalized) = match finalizable_block { + FinalizableBlock::Checkpoint { + checkpoint_verified, + } => { + // Checkpoint-verified blocks don't have an associated treestate, so we get the most + // recent one from the database and update it for the block being committed. + + let block = checkpoint_verified.block.clone(); let mut history_tree = self.db.history_tree(); let mut note_commitment_trees = self.db.note_commitment_trees(); // Update the note commitment trees. - note_commitment_trees.update_trees_parallel(&finalized.block)?; + note_commitment_trees.update_trees_parallel(&block)?; // Check the block commitment if the history tree was not // supplied by the non-finalized state. Note that we don't do @@ -291,7 +260,7 @@ impl FinalizedState { // TODO: run this CPU-intensive cryptography in a parallel rayon // thread, if it shows up in profiles check::block_commitment_is_valid_for_chain_history( - finalized.block.clone(), + block.clone(), self.network, &history_tree, )?; @@ -303,27 +272,67 @@ impl FinalizedState { let history_tree_mut = Arc::make_mut(&mut history_tree); let sapling_root = note_commitment_trees.sapling.root(); let orchard_root = note_commitment_trees.orchard.root(); - history_tree_mut.push( - self.network(), - finalized.block.clone(), - sapling_root, - orchard_root, - )?; - - (history_tree, note_commitment_trees) + history_tree_mut.push(self.network(), block.clone(), sapling_root, orchard_root)?; + + ( + checkpoint_verified.height, + checkpoint_verified.hash, + ContextuallyVerifiedBlockWithTrees { + verified: checkpoint_verified.0, + treestate: Treestate { + note_commitment_trees, + history_tree, + }, + }, + ) } + FinalizableBlock::Contextual { + contextually_verified, + treestate, + } => ( + contextually_verified.height, + contextually_verified.hash, + ContextuallyVerifiedBlockWithTrees { + verified: contextually_verified.into(), + treestate, + }, + ), }; - let finalized_height = finalized.height; - let finalized_hash = finalized.hash; + let committed_tip_hash = self.db.finalized_tip_hash(); + let committed_tip_height = self.db.finalized_tip_height(); + + // Assert that callers (including unit tests) get the chain order correct + if self.db.is_empty() { + assert_eq!( + committed_tip_hash, finalized.verified.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, + "cannot commit genesis: invalid height, source: {source}", + ); + } else { + assert_eq!( + committed_tip_height.expect("state must have a genesis block committed") + 1, + Some(height), + "committed block height must be 1 more than the finalized tip height, source: {source}", + ); + + assert_eq!( + committed_tip_hash, finalized.verified.block.header.previous_block_hash, + "committed block must be a child of the finalized tip, source: {source}", + ); + } #[cfg(feature = "elasticsearch")] - let finalized_block = finalized.block.clone(); + let finalized_block = finalized.verified.block.clone(); let result = self.db.write_block( - finalized, - history_tree, - note_commitment_trees, + finalized.verified, + finalized.treestate.history_tree, + finalized.treestate.note_commitment_trees, self.network, source, ); @@ -334,10 +343,10 @@ impl FinalizedState { self.elasticsearch(&finalized_block); // TODO: move the stop height check to the syncer (#3442) - if self.is_at_stop_height(finalized_height) { + if self.is_at_stop_height(height) { tracing::info!( - height = ?finalized_height, - hash = ?finalized_hash, + height = ?height, + hash = ?hash, block_source = ?source, "stopping at configured height, flushing database to disk" ); diff --git a/zebra-state/src/service/non_finalized_state.rs b/zebra-state/src/service/non_finalized_state.rs index bdcb1c10eb2..f9c4b174e7e 100644 --- a/zebra-state/src/service/non_finalized_state.rs +++ b/zebra-state/src/service/non_finalized_state.rs @@ -16,7 +16,7 @@ use zebra_chain::{ use crate::{ constants::MAX_NON_FINALIZED_CHAIN_FORKS, - request::{ContextuallyVerifiedBlock, ContextuallyVerifiedBlockWithTrees}, + request::{ContextuallyVerifiedBlock, FinalizableBlock}, service::{check, finalized_state::ZebraDb}, SemanticallyVerifiedBlock, ValidateContextError, }; @@ -174,7 +174,7 @@ impl NonFinalizedState { /// Finalize the lowest height block in the non-finalized portion of the best /// chain and update all side-chains to match. - pub fn finalize(&mut self) -> ContextuallyVerifiedBlockWithTrees { + pub fn finalize(&mut self) -> FinalizableBlock { // Chain::cmp uses the partial cumulative work, and the hash of the tip block. // Neither of these fields has interior mutability. // (And when the tip block is dropped for a chain, the chain is also dropped.) @@ -226,7 +226,7 @@ impl NonFinalizedState { self.update_metrics_for_chains(); // Add the treestate to the finalized block. - ContextuallyVerifiedBlockWithTrees::new(best_chain_root, root_treestate) + FinalizableBlock::new(best_chain_root, root_treestate) } /// Commit block to the non-finalized state, on top of: 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 a7e008bcf57..9179dee7f89 100644 --- a/zebra-state/src/service/non_finalized_state/tests/vectors.rs +++ b/zebra-state/src/service/non_finalized_state/tests/vectors.rs @@ -213,13 +213,12 @@ fn finalize_pops_from_best_chain_for_network(network: Network) -> Result<()> { state.commit_block(block2.clone().prepare(), &finalized_state)?; state.commit_block(child.prepare(), &finalized_state)?; - let finalized_with_trees = state.finalize(); - let finalized = finalized_with_trees.block; - assert_eq!(block1, finalized.block); + let finalized = state.finalize().inner_block(); - let finalized_with_trees = state.finalize(); - let finalized = finalized_with_trees.block; - assert_eq!(block2, finalized.block); + assert_eq!(block1, finalized); + + let finalized = state.finalize().inner_block(); + assert_eq!(block2, finalized); assert!(state.best_chain().is_none());