perf: introduce BlockValidationKind (#5195)

crates/blockchain-tree/src/blockchain_tree.rs

@@ -10,7 +10,7 @@ use reth_db::database::Database;
 use reth_interfaces::{
     blockchain_tree::{
         error::{BlockchainTreeError, CanonicalError, InsertBlockError, InsertBlockErrorKind},
-        BlockStatus, CanonicalOutcome, InsertPayloadOk,
+        BlockStatus, BlockValidationKind, CanonicalOutcome, InsertPayloadOk,
     },
     consensus::{Consensus, ConsensusError},
     executor::{BlockExecutionError, BlockValidationError},
@@ -286,12 +286,13 @@ impl<DB: Database, EF: ExecutorFactory> BlockchainTree<DB, EF> {
 
     /// Try inserting a validated [Self::validate_block] block inside the tree.
     ///
-    /// If blocks does not have parent [`BlockStatus::Disconnected`] would be returned, in which
+    /// If the block's parent block is unknown, this returns [`BlockStatus::Disconnected`] and the
-    /// case it is buffered for future inclusion.
+    /// block will be buffered until the parent block is inserted and then attached.
     #[instrument(level = "trace", skip_all, fields(block = ?block.num_hash()), target = "blockchain_tree", ret)]
     fn try_insert_validated_block(
         &mut self,
         block: SealedBlockWithSenders,
+        block_validation_kind: BlockValidationKind,
     ) -> Result<BlockStatus, InsertBlockError> {
         debug_assert!(self.validate_block(&block).is_ok(), "Block must be validated");
 
@@ -300,7 +301,7 @@ impl<DB: Database, EF: ExecutorFactory> BlockchainTree<DB, EF> {
         // check if block parent can be found in Tree
         if let Some(chain_id) = self.block_indices().get_blocks_chain_id(&parent.hash) {
             // found parent in side tree, try to insert there
-            return self.try_insert_block_into_side_chain(block, chain_id)
+            return self.try_insert_block_into_side_chain(block, chain_id, block_validation_kind)
         }
 
         // if not found, check if the parent can be found inside canonical chain.
@@ -308,7 +309,7 @@ impl<DB: Database, EF: ExecutorFactory> BlockchainTree<DB, EF> {
             .is_block_hash_canonical(&parent.hash)
             .map_err(|err| InsertBlockError::new(block.block.clone(), err.into()))?
         {
-            return self.try_append_canonical_chain(block)
+            return self.try_append_canonical_chain(block, block_validation_kind)
         }
 
         // this is another check to ensure that if the block points to a canonical block its block
@@ -362,6 +363,7 @@ impl<DB: Database, EF: ExecutorFactory> BlockchainTree<DB, EF> {
     fn try_append_canonical_chain(
         &mut self,
         block: SealedBlockWithSenders,
+        block_validation_kind: BlockValidationKind,
     ) -> Result<BlockStatus, InsertBlockError> {
         let parent = block.parent_num_hash();
         let block_num_hash = block.num_hash();
@@ -417,8 +419,15 @@ impl<DB: Database, EF: ExecutorFactory> BlockchainTree<DB, EF> {
                     canonical_chain.inner(),
                     parent,
                     &self.externals,
+                    block_validation_kind,
                 )?;
-                (BlockStatus::Valid, chain)
+                let status = if block_validation_kind.is_exhaustive() {
+                    BlockStatus::Valid
+                } else {
+                    BlockStatus::Accepted
+                };
+
+                (status, chain)
             } else {
                 let chain = AppendableChain::new_canonical_fork(
                     block,
@@ -444,6 +453,7 @@ impl<DB: Database, EF: ExecutorFactory> BlockchainTree<DB, EF> {
         &mut self,
         block: SealedBlockWithSenders,
         chain_id: BlockChainId,
+        block_validation_kind: BlockValidationKind,
     ) -> Result<BlockStatus, InsertBlockError> {
         debug!(target: "blockchain_tree", "Inserting block into side chain");
         let block_num_hash = block.num_hash();
@@ -495,11 +505,12 @@ impl<DB: Database, EF: ExecutorFactory> BlockchainTree<DB, EF> {
                 &self.externals,
                 canonical_fork,
                 block_kind,
+                block_validation_kind,
             )?;
 
             self.block_indices_mut().insert_non_fork_block(block_number, block_hash, chain_id);
 
-            if block_kind.extends_canonical_head() {
+            if block_kind.extends_canonical_head() && block_validation_kind.is_exhaustive() {
                 // if the block can be traced back to the canonical head, we were able to fully
                 // validate it
                 Ok(BlockStatus::Valid)
@@ -602,7 +613,7 @@ impl<DB: Database, EF: ExecutorFactory> BlockchainTree<DB, EF> {
         block: SealedBlock,
     ) -> Result<InsertPayloadOk, InsertBlockError> {
         match block.try_seal_with_senders() {
-            Ok(block) => self.insert_block(block),
+            Ok(block) => self.insert_block(block, BlockValidationKind::Exhaustive),
             Err(block) => Err(InsertBlockError::sender_recovery_error(block)),
         }
     }
@@ -681,6 +692,9 @@ impl<DB: Database, EF: ExecutorFactory> BlockchainTree<DB, EF> {
     /// This means that if the block becomes canonical, we need to fetch the missing blocks over
     /// P2P.
     ///
+    /// If the [BlockValidationKind::SkipStateRootValidation] is provided the state root is not
+    /// validated.
+    ///
     /// # Note
     ///
     /// If the senders have not already been recovered, call
@@ -688,6 +702,7 @@ impl<DB: Database, EF: ExecutorFactory> BlockchainTree<DB, EF> {
     pub fn insert_block(
         &mut self,
         block: SealedBlockWithSenders,
+        block_validation_kind: BlockValidationKind,
     ) -> Result<InsertPayloadOk, InsertBlockError> {
         // check if we already have this block
         match self.is_block_known(block.num_hash()) {
@@ -701,7 +716,9 @@ impl<DB: Database, EF: ExecutorFactory> BlockchainTree<DB, EF> {
             return Err(InsertBlockError::consensus_error(err, block.block))
         }
 
-        Ok(InsertPayloadOk::Inserted(self.try_insert_validated_block(block)?))
+        Ok(InsertPayloadOk::Inserted(
+            self.try_insert_validated_block(block, block_validation_kind)?,
+        ))
     }
 
     /// Finalize blocks up until and including `finalized_block`, and remove them from the tree.
@@ -803,17 +820,20 @@ impl<DB: Database, EF: ExecutorFactory> BlockchainTree<DB, EF> {
     fn try_connect_buffered_blocks(&mut self, new_block: BlockNumHash) {
         trace!(target: "blockchain_tree", ?new_block, "try_connect_buffered_blocks");
 
+        // first remove all the children of the new block from the buffer
         let include_blocks = self.state.buffered_blocks.remove_with_children(new_block);
-        // insert block children
+        // then try to reinsert them into the tree
         for block in include_blocks.into_iter() {
             // dont fail on error, just ignore the block.
-            let _ = self.try_insert_validated_block(block).map_err(|err| {
+            let _ = self
-                debug!(
+                .try_insert_validated_block(block, BlockValidationKind::SkipStateRootValidation)
-                    target: "blockchain_tree", ?err,
+                .map_err(|err| {
-                    "Failed to insert buffered block",
+                    debug!(
-                );
+                        target: "blockchain_tree", ?err,
-                err
+                        "Failed to insert buffered block",
-            });
+                    );
+                    err
+                });
         }
     }
 
@@ -1314,7 +1334,7 @@ mod tests {
 
         // block 2 parent is not known, block2 is buffered.
         assert_eq!(
-            tree.insert_block(block2.clone()).unwrap(),
+            tree.insert_block(block2.clone(), BlockValidationKind::Exhaustive).unwrap(),
             InsertPayloadOk::Inserted(BlockStatus::Disconnected {
                 missing_ancestor: block2.parent_num_hash()
             })
@@ -1346,7 +1366,7 @@ mod tests {
 
         // insert block1 and buffered block2 is inserted
         assert_eq!(
-            tree.insert_block(block1.clone()).unwrap(),
+            tree.insert_block(block1.clone(), BlockValidationKind::Exhaustive).unwrap(),
             InsertPayloadOk::Inserted(BlockStatus::Valid)
         );
 
@@ -1369,13 +1389,13 @@ mod tests {
 
         // already inserted block will `InsertPayloadOk::AlreadySeen(_)`
         assert_eq!(
-            tree.insert_block(block1.clone()).unwrap(),
+            tree.insert_block(block1.clone(), BlockValidationKind::Exhaustive).unwrap(),
             InsertPayloadOk::AlreadySeen(BlockStatus::Valid)
         );
 
         // block two is already inserted.
         assert_eq!(
-            tree.insert_block(block2.clone()).unwrap(),
+            tree.insert_block(block2.clone(), BlockValidationKind::Exhaustive).unwrap(),
             InsertPayloadOk::AlreadySeen(BlockStatus::Valid)
         );
 
@@ -1415,7 +1435,7 @@ mod tests {
 
         // reinsert two blocks that point to canonical chain
         assert_eq!(
-            tree.insert_block(block1a.clone()).unwrap(),
+            tree.insert_block(block1a.clone(), BlockValidationKind::Exhaustive).unwrap(),
             InsertPayloadOk::Inserted(BlockStatus::Accepted)
         );
 
@@ -1430,7 +1450,7 @@ mod tests {
             .assert(&tree);
 
         assert_eq!(
-            tree.insert_block(block2a.clone()).unwrap(),
+            tree.insert_block(block2a.clone(), BlockValidationKind::Exhaustive).unwrap(),
             InsertPayloadOk::Inserted(BlockStatus::Accepted)
         );
         // Trie state:
@@ -1620,7 +1640,7 @@ mod tests {
         block2b.parent_hash = B256::new([0x88; 32]);
 
         assert_eq!(
-            tree.insert_block(block2b.clone()).unwrap(),
+            tree.insert_block(block2b.clone(), BlockValidationKind::Exhaustive).unwrap(),
             InsertPayloadOk::Inserted(BlockStatus::Disconnected {
                 missing_ancestor: block2b.parent_num_hash()
             })

crates/blockchain-tree/src/chain.rs

@@ -6,7 +6,10 @@ use super::externals::TreeExternals;
 use crate::BundleStateDataRef;
 use reth_db::database::Database;
 use reth_interfaces::{
-    blockchain_tree::error::{BlockchainTreeError, InsertBlockError},
+    blockchain_tree::{
+        error::{BlockchainTreeError, InsertBlockError},
+        BlockValidationKind,
+    },
     consensus::{Consensus, ConsensusError},
     RethResult,
 };
@@ -54,15 +57,17 @@ impl AppendableChain {
         self.chain
     }
 
-    /// Create a new chain that forks off the canonical.
+    /// Create a new chain that forks off the canonical chain.
     ///
-    /// This will also verify the state root of the block extending the canonical chain.
+    /// if [BlockValidationKind::Exhaustive] is provides this will verify the state root of the
+    /// block extending the canonical chain.
     pub fn new_canonical_head_fork<DB, EF>(
         block: SealedBlockWithSenders,
         parent_header: &SealedHeader,
         canonical_block_hashes: &BTreeMap<BlockNumber, BlockHash>,
         canonical_fork: ForkBlock,
         externals: &TreeExternals<DB, EF>,
+        block_validation_kind: BlockValidationKind,
     ) -> Result<Self, InsertBlockError>
     where
         DB: Database,
@@ -78,11 +83,13 @@ impl AppendableChain {
             canonical_fork,
         };
 
-        let bundle_state = Self::validate_and_execute_canonical_head_descendant(
+        let bundle_state = Self::validate_and_execute(
             block.clone(),
             parent_header,
             state_provider,
             externals,
+            BlockKind::ExtendsCanonicalHead,
+            block_validation_kind,
         )
         .map_err(|err| InsertBlockError::new(block.block.clone(), err.into()))?;
 
@@ -170,13 +177,21 @@ impl AppendableChain {
     }
 
     /// Validate and execute the given block that _extends the canonical chain_, validating its
-    /// state root after execution.
+    /// state root after execution if possible and requested.
+    ///
+    /// Note: State root validation is limited to blocks that extend the canonical chain and is
+    /// optional, see [BlockValidationKind]. So this function takes two parameters to determine
+    /// if the state can and should be validated.
+    ///   - [BlockKind] represents if the block extends the canonical chain, and thus if the state
+    ///     root __can__ be validated.
+    ///   - [BlockValidationKind] determines if the state root __should__ be validated.
     fn validate_and_execute<BSDP, DB, EF>(
         block: SealedBlockWithSenders,
         parent_block: &SealedHeader,
         post_state_data_provider: BSDP,
         externals: &TreeExternals<DB, EF>,
         block_kind: BlockKind,
+        block_validation_kind: BlockValidationKind,
     ) -> RethResult<BundleStateWithReceipts>
     where
         BSDP: BundleStateDataProvider,
@@ -200,8 +215,9 @@ impl AppendableChain {
         executor.execute_and_verify_receipt(&block, U256::MAX, Some(senders))?;
         let bundle_state = executor.take_output_state();
 
-        // check state root if the block extends the canonical chain.
+        // check state root if the block extends the canonical chain __and__ if state root
-        if block_kind.extends_canonical_head() {
+        // validation was requested.
+        if block_kind.extends_canonical_head() && block_validation_kind.is_exhaustive() {
             // check state root
             let state_root = provider.state_root(&bundle_state)?;
             if block.state_root != state_root {
@@ -216,28 +232,6 @@ impl AppendableChain {
         Ok(bundle_state)
     }
 
-    /// Validate and execute the given block that _extends the canonical chain_, validating its
-    /// state root after execution.
-    fn validate_and_execute_canonical_head_descendant<BSDP, DB, EF>(
-        block: SealedBlockWithSenders,
-        parent_block: &SealedHeader,
-        post_state_data_provider: BSDP,
-        externals: &TreeExternals<DB, EF>,
-    ) -> RethResult<BundleStateWithReceipts>
-    where
-        BSDP: BundleStateDataProvider,
-        DB: Database,
-        EF: ExecutorFactory,
-    {
-        Self::validate_and_execute(
-            block,
-            parent_block,
-            post_state_data_provider,
-            externals,
-            BlockKind::ExtendsCanonicalHead,
-        )
-    }
-
     /// Validate and execute the given sidechain block, skipping state root validation.
     fn validate_and_execute_sidechain<BSDP, DB, EF>(
         block: SealedBlockWithSenders,
@@ -256,6 +250,7 @@ impl AppendableChain {
             post_state_data_provider,
             externals,
             BlockKind::ForksHistoricalBlock,
+            BlockValidationKind::SkipStateRootValidation,
         )
     }
 
@@ -271,6 +266,7 @@ impl AppendableChain {
     /// is the canonical head, or: state root check can't be performed if the given canonical is
     /// __not__ the canonical head.
     #[track_caller]
+    #[allow(clippy::too_many_arguments)]
     pub(crate) fn append_block<DB, EF>(
         &mut self,
         block: SealedBlockWithSenders,
@@ -279,6 +275,7 @@ impl AppendableChain {
         externals: &TreeExternals<DB, EF>,
         canonical_fork: ForkBlock,
         block_kind: BlockKind,
+        block_validation_kind: BlockValidationKind,
     ) -> Result<(), InsertBlockError>
     where
         DB: Database,
@@ -299,6 +296,7 @@ impl AppendableChain {
             post_state_data,
             externals,
             block_kind,
+            block_validation_kind,
         )
         .map_err(|err| InsertBlockError::new(block.block.clone(), err.into()))?;
         // extend the state.

crates/blockchain-tree/src/noop.rs

@@ -1,7 +1,8 @@
 use reth_interfaces::{
     blockchain_tree::{
         error::{BlockchainTreeError, InsertBlockError},
-        BlockchainTreeEngine, BlockchainTreeViewer, CanonicalOutcome, InsertPayloadOk,
+        BlockValidationKind, BlockchainTreeEngine, BlockchainTreeViewer, CanonicalOutcome,
+        InsertPayloadOk,
     },
     RethResult,
 };
@@ -30,6 +31,7 @@ impl BlockchainTreeEngine for NoopBlockchainTree {
     fn insert_block(
         &self,
         block: SealedBlockWithSenders,
+        _validation_kind: BlockValidationKind,
     ) -> Result<InsertPayloadOk, InsertBlockError> {
         Err(InsertBlockError::tree_error(
             BlockchainTreeError::BlockHashNotFoundInChain { block_hash: block.hash },

crates/blockchain-tree/src/shareable.rs

@@ -4,8 +4,8 @@ use parking_lot::RwLock;
 use reth_db::database::Database;
 use reth_interfaces::{
     blockchain_tree::{
-        error::InsertBlockError, BlockchainTreeEngine, BlockchainTreeViewer, CanonicalOutcome,
+        error::InsertBlockError, BlockValidationKind, BlockchainTreeEngine, BlockchainTreeViewer,
-        InsertPayloadOk,
+        CanonicalOutcome, InsertPayloadOk,
     },
     RethResult,
 };
@@ -48,10 +48,11 @@ impl<DB: Database, EF: ExecutorFactory> BlockchainTreeEngine for ShareableBlockc
     fn insert_block(
         &self,
         block: SealedBlockWithSenders,
+        validation_kind: BlockValidationKind,
     ) -> Result<InsertPayloadOk, InsertBlockError> {
         trace!(target: "blockchain_tree", hash=?block.hash, number=block.number, parent_hash=?block.parent_hash, "Inserting block");
         let mut tree = self.tree.write();
-        let res = tree.insert_block(block);
+        let res = tree.insert_block(block, validation_kind);
         tree.update_chains_metrics();
         res
     }

crates/consensus/beacon/src/engine/mod.rs

@@ -72,6 +72,7 @@ pub use handle::BeaconConsensusEngineHandle;
 mod forkchoice;
 use crate::hooks::{EngineHookEvent, EngineHooks, PolledHook};
 pub use forkchoice::ForkchoiceStatus;
+use reth_interfaces::blockchain_tree::BlockValidationKind;
 
 mod metrics;
 
@@ -1294,7 +1295,9 @@ where
         debug_assert!(self.sync.is_pipeline_idle(), "pipeline must be idle");
 
         let block_hash = block.hash;
-        let status = self.blockchain.insert_block_without_senders(block.clone())?;
+        let status = self
+            .blockchain
+            .insert_block_without_senders(block.clone(), BlockValidationKind::Exhaustive)?;
         let mut latest_valid_hash = None;
         let block = Arc::new(block);
         let status = match status {
@@ -1415,7 +1418,10 @@ where
             return
         }
 
-        match self.blockchain.insert_block_without_senders(block) {
+        match self
+            .blockchain
+            .insert_block_without_senders(block, BlockValidationKind::SkipStateRootValidation)
+        {
             Ok(status) => {
                 match status {
                     InsertPayloadOk::Inserted(BlockStatus::Valid) => {

crates/interfaces/src/blockchain_tree/mod.rs

@@ -22,9 +22,10 @@ pub trait BlockchainTreeEngine: BlockchainTreeViewer + Send + Sync {
     fn insert_block_without_senders(
         &self,
         block: SealedBlock,
+        validation_kind: BlockValidationKind,
     ) -> Result<InsertPayloadOk, InsertBlockError> {
         match block.try_seal_with_senders() {
-            Ok(block) => self.insert_block(block),
+            Ok(block) => self.insert_block(block, validation_kind),
             Err(block) => Err(InsertBlockError::sender_recovery_error(block)),
         }
     }
@@ -43,10 +44,17 @@ pub trait BlockchainTreeEngine: BlockchainTreeViewer + Send + Sync {
     /// Buffer block with senders
     fn buffer_block(&self, block: SealedBlockWithSenders) -> Result<(), InsertBlockError>;
 
-    /// Insert block with senders
+    /// Inserts block with senders
+    ///
+    /// The `validation_kind` parameter controls which validation checks are performed.
+    ///
+    /// Caution: If the block was received from the consensus layer, this should always be called
+    /// with [BlockValidationKind::Exhaustive] to validate the state root, if possible to adhere to
+    /// the engine API spec.
     fn insert_block(
         &self,
         block: SealedBlockWithSenders,
+        validation_kind: BlockValidationKind,
     ) -> Result<InsertPayloadOk, InsertBlockError>;
 
     /// Finalize blocks up until and including `finalized_block`, and remove them from the tree.
@@ -92,6 +100,48 @@ pub trait BlockchainTreeEngine: BlockchainTreeViewer + Send + Sync {
     fn unwind(&self, unwind_to: BlockNumber) -> RethResult<()>;
 }
 
+/// Represents the kind of validation that should be performed when inserting a block.
+///
+/// The motivation for this is that the engine API spec requires that block's state root is
+/// validated when received from the CL.
+///
+/// This step is very expensive due to how changesets are stored in the database, so we want to
+/// avoid doing it if not necessary. Blocks can also originate from the network where this step is
+/// not required.
+#[derive(Debug, Clone, Copy, PartialEq, Eq, Default)]
+pub enum BlockValidationKind {
+    /// All validation checks that can be performed.
+    ///
+    /// This includes validating the state root, if possible.
+    ///
+    /// Note: This should always be used when inserting blocks that originate from the consensus
+    /// layer.
+    #[default]
+    Exhaustive,
+    /// Perform all validation checks except for state root validation.
+    SkipStateRootValidation,
+}
+
+impl BlockValidationKind {
+    /// Returns true if the state root should be validated if possible.
+    pub fn is_exhaustive(&self) -> bool {
+        matches!(self, BlockValidationKind::Exhaustive)
+    }
+}
+
+impl std::fmt::Display for BlockValidationKind {
+    fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
+        match self {
+            BlockValidationKind::Exhaustive => {
+                write!(f, "Exhaustive")
+            }
+            BlockValidationKind::SkipStateRootValidation => {
+                write!(f, "SkipStateRootValidation")
+            }
+        }
+    }
+}
+
 /// All possible outcomes of a canonicalization attempt of [BlockchainTreeEngine::make_canonical].
 #[derive(Debug, Clone, PartialEq, Eq)]
 pub enum CanonicalOutcome {

crates/storage/provider/src/providers/mod.rs

@@ -44,7 +44,7 @@ pub use bundle_state_provider::BundleStateProvider;
 pub use database::*;
 use reth_db::models::AccountBeforeTx;
 use reth_interfaces::blockchain_tree::{
-    error::InsertBlockError, CanonicalOutcome, InsertPayloadOk,
+    error::InsertBlockError, BlockValidationKind, CanonicalOutcome, InsertPayloadOk,
 };
 
 /// The main type for interacting with the blockchain.
@@ -574,8 +574,9 @@ where
     fn insert_block(
         &self,
         block: SealedBlockWithSenders,
+        validation_kind: BlockValidationKind,
     ) -> Result<InsertPayloadOk, InsertBlockError> {
-        self.tree.insert_block(block)
+        self.tree.insert_block(block, validation_kind)
     }
 
     fn finalize_block(&self, finalized_block: BlockNumber) {