diff --git a/crates/blockchain-tree/src/blockchain_tree.rs b/crates/blockchain-tree/src/blockchain_tree.rs index 66c03e512..7f58c2a0b 100644 --- a/crates/blockchain-tree/src/blockchain_tree.rs +++ b/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 BlockchainTree { /// Try inserting a validated [Self::validate_block] block inside the tree. /// - /// If blocks does not have parent [`BlockStatus::Disconnected`] would be returned, in which - /// case it is buffered for future inclusion. + /// If the block's parent block is unknown, this returns [`BlockStatus::Disconnected`] and the + /// 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 { debug_assert!(self.validate_block(&block).is_ok(), "Block must be validated"); @@ -300,7 +301,7 @@ impl BlockchainTree { // 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 BlockchainTree { .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 BlockchainTree { fn try_append_canonical_chain( &mut self, block: SealedBlockWithSenders, + block_validation_kind: BlockValidationKind, ) -> Result { let parent = block.parent_num_hash(); let block_num_hash = block.num_hash(); @@ -417,8 +419,15 @@ impl BlockchainTree { 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 BlockchainTree { &mut self, block: SealedBlockWithSenders, chain_id: BlockChainId, + block_validation_kind: BlockValidationKind, ) -> Result { debug!(target: "blockchain_tree", "Inserting block into side chain"); let block_num_hash = block.num_hash(); @@ -495,11 +505,12 @@ impl BlockchainTree { &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 BlockchainTree { block: SealedBlock, ) -> Result { 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 BlockchainTree { /// 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 BlockchainTree { pub fn insert_block( &mut self, block: SealedBlockWithSenders, + block_validation_kind: BlockValidationKind, ) -> Result { // check if we already have this block match self.is_block_known(block.num_hash()) { @@ -701,7 +716,9 @@ impl BlockchainTree { 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 BlockchainTree { 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| { - debug!( - target: "blockchain_tree", ?err, - "Failed to insert buffered block", - ); - err - }); + let _ = self + .try_insert_validated_block(block, BlockValidationKind::SkipStateRootValidation) + .map_err(|err| { + debug!( + target: "blockchain_tree", ?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() }) diff --git a/crates/blockchain-tree/src/chain.rs b/crates/blockchain-tree/src/chain.rs index 12460f3f7..48f5974bf 100644 --- a/crates/blockchain-tree/src/chain.rs +++ b/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( block: SealedBlockWithSenders, parent_header: &SealedHeader, canonical_block_hashes: &BTreeMap, canonical_fork: ForkBlock, externals: &TreeExternals, + block_validation_kind: BlockValidationKind, ) -> Result 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( block: SealedBlockWithSenders, parent_block: &SealedHeader, post_state_data_provider: BSDP, externals: &TreeExternals, block_kind: BlockKind, + block_validation_kind: BlockValidationKind, ) -> RethResult 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. - if block_kind.extends_canonical_head() { + // check state root if the block extends the canonical chain __and__ if state root + // 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( - block: SealedBlockWithSenders, - parent_block: &SealedHeader, - post_state_data_provider: BSDP, - externals: &TreeExternals, - ) -> RethResult - 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( 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( &mut self, block: SealedBlockWithSenders, @@ -279,6 +275,7 @@ impl AppendableChain { externals: &TreeExternals, 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. diff --git a/crates/blockchain-tree/src/noop.rs b/crates/blockchain-tree/src/noop.rs index d9c87e6d7..95709dc7d 100644 --- a/crates/blockchain-tree/src/noop.rs +++ b/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 { Err(InsertBlockError::tree_error( BlockchainTreeError::BlockHashNotFoundInChain { block_hash: block.hash }, diff --git a/crates/blockchain-tree/src/shareable.rs b/crates/blockchain-tree/src/shareable.rs index e8b7759b1..ebb57ca1c 100644 --- a/crates/blockchain-tree/src/shareable.rs +++ b/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, - InsertPayloadOk, + error::InsertBlockError, BlockValidationKind, BlockchainTreeEngine, BlockchainTreeViewer, + CanonicalOutcome, InsertPayloadOk, }, RethResult, }; @@ -48,10 +48,11 @@ impl BlockchainTreeEngine for ShareableBlockc fn insert_block( &self, block: SealedBlockWithSenders, + validation_kind: BlockValidationKind, ) -> Result { 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 } diff --git a/crates/consensus/beacon/src/engine/mod.rs b/crates/consensus/beacon/src/engine/mod.rs index 746e3a544..641376824 100644 --- a/crates/consensus/beacon/src/engine/mod.rs +++ b/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) => { diff --git a/crates/interfaces/src/blockchain_tree/mod.rs b/crates/interfaces/src/blockchain_tree/mod.rs index 3ad73c69d..8a365361b 100644 --- a/crates/interfaces/src/blockchain_tree/mod.rs +++ b/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 { 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; /// 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 { diff --git a/crates/storage/provider/src/providers/mod.rs b/crates/storage/provider/src/providers/mod.rs index f07f9f5a0..bc0704f07 100644 --- a/crates/storage/provider/src/providers/mod.rs +++ b/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 { - self.tree.insert_block(block) + self.tree.insert_block(block, validation_kind) } fn finalize_block(&self, finalized_block: BlockNumber) {