Add and use FinalizableBlock

This commit adds `FinalizableBlock`, and uses it instead of
`ContextuallyVerifiedBlockWithTrees` in `commit_finalized_direct()`

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<Treestate>,
+    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<Arc<Block>> for ContextuallyVerifiedBlockWithTrees {
-    fn from(block: Arc<Block>) -> Self {
-        Self::from(SemanticallyVerifiedBlock::from(block))
+    #[cfg(test)]
+    pub fn inner_block(&self) -> Arc<Block> {
+        match self {
+            FinalizableBlock::Checkpoint {
+                checkpoint_verified,
+            } => checkpoint_verified.block.clone(),
+            FinalizableBlock::Contextual {
+                contextually_verified,
+                ..
+            } => contextually_verified.block.clone(),
+        }
     }
 }
 
-impl From<SemanticallyVerifiedBlock> for ContextuallyVerifiedBlockWithTrees {
-    fn from(semantically_verified: SemanticallyVerifiedBlock) -> Self {
-        Self {
-            block: semantically_verified,
-            treestate: None,
+impl From<CheckpointVerifiedBlock> for FinalizableBlock {
+    fn from(checkpoint_verified: CheckpointVerifiedBlock) -> Self {
+        Self::Checkpoint {
+            checkpoint_verified,
         }
     }
 }
 
-impl From<CheckpointVerifiedBlock> for ContextuallyVerifiedBlockWithTrees {
-    fn from(checkpoint_verified: CheckpointVerifiedBlock) -> Self {
-        Self {
-            block: checkpoint_verified.0,
-            treestate: None,
-        }
+impl From<Arc<Block>> for FinalizableBlock {
+    fn from(block: Arc<Block>) -> Self {
+        Self::from(CheckpointVerifiedBlock::from(block))
     }
 }
 
@@ -413,6 +426,12 @@ impl From<ContextuallyVerifiedBlock> for SemanticallyVerifiedBlock {
     }
 }
 
+impl From<CheckpointVerifiedBlock> for SemanticallyVerifiedBlock {
+    fn from(checkpoint_verified: CheckpointVerifiedBlock) -> Self {
+        checkpoint_verified.0
+    }
+}
+
 impl Deref for CheckpointVerifiedBlock {
     type Target = SemanticallyVerifiedBlock;
 

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<block::Hash, BoxError> {
-        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"
                 );

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:

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());