diff --git a/crates/blockchain-tree/src/block_indices.rs b/crates/blockchain-tree/src/block_indices.rs index 21a58b61ad..e3395cc7b1 100644 --- a/crates/blockchain-tree/src/block_indices.rs +++ b/crates/blockchain-tree/src/block_indices.rs @@ -93,9 +93,17 @@ impl BlockIndices { (canonical_tip.number + 1, pending_blocks) } + /// Returns the block number of the canonical block with the given hash. + /// + /// Returns `None` if no block could be found in the canonical chain. + #[inline] + pub(crate) fn get_canonical_block_number(&self, block_hash: &BlockHash) -> Option { + self.canonical_chain.get_canonical_block_number(self.last_finalized_block, block_hash) + } + /// Check if block hash belongs to canonical chain. - pub fn is_block_hash_canonical(&self, block_hash: &BlockHash) -> bool { - self.canonical_chain.is_block_hash_canonical(self.last_finalized_block, block_hash) + pub(crate) fn is_block_hash_canonical(&self, block_hash: &BlockHash) -> bool { + self.get_canonical_block_number(block_hash).is_some() } /// Last finalized block diff --git a/crates/blockchain-tree/src/blockchain_tree.rs b/crates/blockchain-tree/src/blockchain_tree.rs index d902e93b7e..e764c129ed 100644 --- a/crates/blockchain-tree/src/blockchain_tree.rs +++ b/crates/blockchain-tree/src/blockchain_tree.rs @@ -7,7 +7,7 @@ use reth_db::{cursor::DbCursorRO, database::Database, tables, transaction::DbTx} use reth_interfaces::{ blockchain_tree::{ error::{BlockchainTreeError, InsertBlockError, InsertBlockErrorKind}, - BlockStatus, + BlockStatus, CanonicalOutcome, }, consensus::{Consensus, ConsensusError}, executor::BlockExecutionError, @@ -15,7 +15,7 @@ use reth_interfaces::{ }; use reth_primitives::{ BlockHash, BlockNumHash, BlockNumber, ForkBlock, Hardfork, SealedBlock, SealedBlockWithSenders, - U256, + SealedHeader, U256, }; use reth_provider::{ chain::{ChainSplit, SplitAt}, @@ -211,6 +211,11 @@ impl BlockchainTree chain.block(block_hash) } + /// Returns true if the block is included in a side-chain. + fn is_block_hash_inside_chain(&self, block_hash: BlockHash) -> bool { + self.block_by_hash(block_hash).is_some() + } + /// Returns the block that's considered the `Pending` block, if it exists. pub fn pending_block(&self) -> Option<&SealedBlock> { let b = self.block_indices.pending_block_num_hash()?; @@ -775,19 +780,34 @@ impl BlockchainTree } } - /// Determines whether or not a block is canonical, checking the db if necessary. - pub fn is_block_hash_canonical(&self, hash: &BlockHash) -> Result { + /// Attempts to find the header for the given block hash if it is canonical. + /// + /// Returns `Ok(None)` if the block hash is not canonical (block hash does not exist, or is + /// included in a sidechain). + pub fn find_canonical_header(&self, hash: &BlockHash) -> Result, Error> { // if the indices show that the block hash is not canonical, it's either in a sidechain or // canonical, but in the db. If it is in a sidechain, it is not canonical. If it is not in // the db, then it is not canonical. - if !self.block_indices.is_block_hash_canonical(hash) && - (self.block_by_hash(*hash).is_some() || - self.externals.database().header(hash)?.is_none()) - { - return Ok(false) + + let mut header = None; + if let Some(num) = self.block_indices.get_canonical_block_number(hash) { + header = self.externals.database().header_by_number(num)?; } - Ok(true) + if header.is_none() && self.is_block_hash_inside_chain(*hash) { + return Ok(None) + } + + if header.is_none() { + header = self.externals.database().header(hash)? + } + + Ok(header.map(|header| header.seal(*hash))) + } + + /// Determines whether or not a block is canonical, checking the db if necessary. + pub fn is_block_hash_canonical(&self, hash: &BlockHash) -> Result { + self.find_canonical_header(hash).map(|header| header.is_some()) } /// Make a block and its parent(s) part of the canonical chain. @@ -802,12 +822,12 @@ impl BlockchainTree /// Returns `Ok` if the blocks were canonicalized, or if the blocks were already canonical. #[track_caller] #[instrument(skip(self), target = "blockchain_tree")] - pub fn make_canonical(&mut self, block_hash: &BlockHash) -> Result<(), Error> { + pub fn make_canonical(&mut self, block_hash: &BlockHash) -> Result { let old_block_indices = self.block_indices.clone(); let old_buffered_blocks = self.buffered_blocks.parent_to_child.clone(); // If block is already canonical don't return error. - if self.is_block_hash_canonical(block_hash)? { + if let Some(header) = self.find_canonical_header(block_hash)? { info!(target: "blockchain_tree", ?block_hash, "Block is already canonical"); let td = self .externals @@ -817,7 +837,7 @@ impl BlockchainTree if !self.externals.chain_spec.fork(Hardfork::Paris).active_at_ttd(td, U256::ZERO) { return Err(BlockExecutionError::BlockPreMerge { hash: *block_hash }.into()) } - return Ok(()) + return Ok(CanonicalOutcome::AlreadyCanonical { header }) } let Some(chain_id) = self.block_indices.get_blocks_chain_id(block_hash) else { @@ -912,10 +932,13 @@ impl BlockchainTree } } + // + let head = chain_notification.tip().header.clone(); + // send notification about new canonical chain. let _ = self.canon_state_notification_sender.send(chain_notification); - Ok(()) + Ok(CanonicalOutcome::Committed { head }) } /// Subscribe to new blocks events. @@ -1127,7 +1150,7 @@ mod tests { BlockchainTree::new(externals, sender, config).expect("failed to create tree"); // genesis block 10 is already canonical - assert_eq!(tree.make_canonical(&H256::zero()), Ok(())); + assert!(tree.make_canonical(&H256::zero()).is_ok()); // make sure is_block_hash_canonical returns true for genesis block assert!(tree.is_block_hash_canonical(&H256::zero()).unwrap()); @@ -1189,12 +1212,12 @@ mod tests { assert_eq!(tree.insert_block(block2.clone()).unwrap(), BlockStatus::Valid); // make block1 canonical - assert_eq!(tree.make_canonical(&block1.hash()), Ok(())); + assert!(tree.make_canonical(&block1.hash()).is_ok()); // check notification assert_matches!(canon_notif.try_recv(), Ok(CanonStateNotification::Commit{ new}) if *new.blocks() == BTreeMap::from([(block1.number,block1.clone())])); // make block2 canonicals - assert_eq!(tree.make_canonical(&block2.hash()), Ok(())); + assert!(tree.make_canonical(&block2.hash()).is_ok()); // check notification. assert_matches!(canon_notif.try_recv(), Ok(CanonStateNotification::Commit{ new}) if *new.blocks() == BTreeMap::from([(block2.number,block2.clone())])); @@ -1256,7 +1279,7 @@ mod tests { .assert(&tree); // make b2a canonical - assert_eq!(tree.make_canonical(&block2a_hash), Ok(())); + assert!(tree.make_canonical(&block2a_hash).is_ok()); // check notification. assert_matches!(canon_notif.try_recv(), Ok(CanonStateNotification::Reorg{ old, new}) @@ -1282,7 +1305,7 @@ mod tests { .with_pending_blocks((block2.number + 1, HashSet::new())) .assert(&tree); - assert_eq!(tree.make_canonical(&block1a_hash), Ok(())); + assert!(tree.make_canonical(&block1a_hash).is_ok()); // Trie state: // b2a b2 (side chain) // | / @@ -1320,7 +1343,7 @@ mod tests { assert!(tree.is_block_hash_canonical(&block1a.hash).unwrap()); // make b2 canonical - assert_eq!(tree.make_canonical(&block2.hash()), Ok(())); + assert!(tree.make_canonical(&block2.hash()).is_ok()); // Trie state: // b2 b2a (side chain) @@ -1390,7 +1413,7 @@ mod tests { .assert(&tree); // commit b2a - assert_eq!(tree.make_canonical(&block2.hash), Ok(())); + assert!(tree.make_canonical(&block2.hash).is_ok()); // Trie state: // b2 b2a (side chain) diff --git a/crates/blockchain-tree/src/canonical_chain.rs b/crates/blockchain-tree/src/canonical_chain.rs index c1b5057746..e641e455a7 100644 --- a/crates/blockchain-tree/src/canonical_chain.rs +++ b/crates/blockchain-tree/src/canonical_chain.rs @@ -42,14 +42,18 @@ impl CanonicalChain { ) } - /// Check if block hash belongs to canonical chain. + /// Returns the block number of the canonical block with the given hash. + /// + /// Returns `None` if no block could be found in the canonical chain. #[inline] - pub(crate) fn is_block_hash_canonical( + pub(crate) fn get_canonical_block_number( &self, last_finalized_block: BlockNumber, block_hash: &BlockHash, - ) -> bool { - self.chain.range(last_finalized_block..).any(|(_, &h)| h == *block_hash) + ) -> Option { + self.chain + .range(last_finalized_block..) + .find_map(|(num, &h)| (h == *block_hash).then_some(*num)) } /// Extends all items from the given iterator to the chain. diff --git a/crates/blockchain-tree/src/shareable.rs b/crates/blockchain-tree/src/shareable.rs index c95b1c8138..a3064e4b1a 100644 --- a/crates/blockchain-tree/src/shareable.rs +++ b/crates/blockchain-tree/src/shareable.rs @@ -5,6 +5,7 @@ use reth_db::database::Database; use reth_interfaces::{ blockchain_tree::{ error::InsertBlockError, BlockStatus, BlockchainTreeEngine, BlockchainTreeViewer, + CanonicalOutcome, }, consensus::Consensus, Error, @@ -68,7 +69,7 @@ impl BlockchainTreeEngine self.tree.write().restore_canonical_hashes(last_finalized_block) } - fn make_canonical(&self, block_hash: &BlockHash) -> Result<(), Error> { + fn make_canonical(&self, block_hash: &BlockHash) -> Result { trace!(target: "blockchain_tree", ?block_hash, "Making block canonical"); self.tree.write().make_canonical(block_hash) } diff --git a/crates/consensus/beacon/src/engine/mod.rs b/crates/consensus/beacon/src/engine/mod.rs index 6c0851a656..d115c4372a 100644 --- a/crates/consensus/beacon/src/engine/mod.rs +++ b/crates/consensus/beacon/src/engine/mod.rs @@ -389,30 +389,23 @@ where // We can only process new forkchoice updates if the pipeline is idle, since it requires // exclusive access to the database match self.blockchain.make_canonical(&state.head_block_hash) { - Ok(_) => { - let head_block_number = self - .get_block_number(state.head_block_hash)? - .expect("was canonicalized, so it exists"); - debug!(target: "consensus::engine", hash=?state.head_block_hash, number=head_block_number, "canonicalized new head"); + Ok(outcome) => { + let header = outcome.into_header(); + debug!(target: "consensus::engine", hash=?state.head_block_hash, number=header.number, "canonicalized new head"); let pipeline_min_progress = FINISH.get_checkpoint(&self.db.tx()?)?.unwrap_or_default().block_number; - if pipeline_min_progress < head_block_number { - debug!(target: "consensus::engine", last_finished=pipeline_min_progress, head_number=head_block_number, "pipeline run to head required"); + if pipeline_min_progress < header.number { + debug!(target: "consensus::engine", last_finished=pipeline_min_progress, head_number=header.number, "pipeline run to head required"); // TODO(mattsse) ideally sync blockwise self.sync.set_pipeline_sync_target(state.head_block_hash); } if let Some(attrs) = attrs { - // get header for further validation - let header = self - .load_header(head_block_number)? - .expect("was canonicalized, so it exists"); - let payload_response = - self.process_payload_attributes(attrs, header, state); + self.process_payload_attributes(attrs, header.unseal(), state); if payload_response.is_valid_update() { // we will return VALID, so let's make sure the info tracker is // properly updated diff --git a/crates/interfaces/src/blockchain_tree/mod.rs b/crates/interfaces/src/blockchain_tree/mod.rs index 294776a7b4..8161d9a12a 100644 --- a/crates/interfaces/src/blockchain_tree/mod.rs +++ b/crates/interfaces/src/blockchain_tree/mod.rs @@ -60,7 +60,8 @@ pub trait BlockchainTreeEngine: BlockchainTreeViewer + Send + Sync { /// [`BlockchainTreeEngine::finalize_block`]). fn restore_canonical_hashes(&self, last_finalized_block: BlockNumber) -> Result<(), Error>; - /// Make a block and its parent part of the canonical chain by committing it to the database. + /// Make a block and its parent chain part of the canonical chain by committing it to the + /// database. /// /// # Note /// @@ -70,12 +71,45 @@ pub trait BlockchainTreeEngine: BlockchainTreeViewer + Send + Sync { /// # Returns /// /// Returns `Ok` if the blocks were canonicalized, or if the blocks were already canonical. - fn make_canonical(&self, block_hash: &BlockHash) -> Result<(), Error>; + fn make_canonical(&self, block_hash: &BlockHash) -> Result; /// Unwind tables and put it inside state fn unwind(&self, unwind_to: BlockNumber) -> Result<(), Error>; } +/// All possible outcomes of a canonicalization attempt of [BlockchainTreeEngine::make_canonical]. +#[derive(Debug, Clone, PartialEq, Eq)] +pub enum CanonicalOutcome { + /// The block is already canonical. + AlreadyCanonical { + /// The corresponding [SealedHeader] that is already canonical. + header: SealedHeader, + }, + /// Committed the block to the database. + Committed { + /// The new corresponding canonical head + head: SealedHeader, + }, +} + +impl CanonicalOutcome { + /// Returns the header of the block that was made canonical. + pub fn header(&self) -> &SealedHeader { + match self { + CanonicalOutcome::AlreadyCanonical { header } => header, + CanonicalOutcome::Committed { head } => head, + } + } + + /// Consumes the outcome and returns the header of the block that was made canonical. + pub fn into_header(self) -> SealedHeader { + match self { + CanonicalOutcome::AlreadyCanonical { header } => header, + CanonicalOutcome::Committed { head } => head, + } + } +} + /// From Engine API spec, block inclusion can be valid, accepted or invalid. /// Invalid case is already covered by error, but we need to make distinction /// between if it is valid (extends canonical chain) or just accepted (is side chain). diff --git a/crates/storage/provider/src/providers/mod.rs b/crates/storage/provider/src/providers/mod.rs index 1958a4695c..30f98894b8 100644 --- a/crates/storage/provider/src/providers/mod.rs +++ b/crates/storage/provider/src/providers/mod.rs @@ -35,7 +35,7 @@ mod state; use crate::{providers::chain_info::ChainInfoTracker, traits::BlockSource}; pub use database::*; pub use post_state_provider::PostStateProvider; -use reth_interfaces::blockchain_tree::error::InsertBlockError; +use reth_interfaces::blockchain_tree::{error::InsertBlockError, CanonicalOutcome}; /// The main type for interacting with the blockchain. /// @@ -401,7 +401,7 @@ where self.tree.restore_canonical_hashes(last_finalized_block) } - fn make_canonical(&self, block_hash: &BlockHash) -> Result<()> { + fn make_canonical(&self, block_hash: &BlockHash) -> Result { self.tree.make_canonical(block_hash) } diff --git a/crates/storage/provider/src/traits/chain.rs b/crates/storage/provider/src/traits/chain.rs index da1388edae..dc5c8a168d 100644 --- a/crates/storage/provider/src/traits/chain.rs +++ b/crates/storage/provider/src/traits/chain.rs @@ -1,6 +1,7 @@ //! Canonical chain state notification trait and types. use crate::{chain::BlockReceipts, Chain}; use auto_impl::auto_impl; +use reth_primitives::SealedBlockWithSenders; use std::{ pin::Pin, sync::Arc, @@ -98,7 +99,11 @@ impl CanonStateNotification { } } - /// Get new chain if any. + /// Get the new chain if any. + /// + /// Returns the new committed [Chain] for [Self::Reorg] and [Self::Commit] variants. + /// + /// Returns None for [Self::Revert] variant. pub fn committed(&self) -> Option> { match self { Self::Reorg { new, .. } => Some(new.clone()), @@ -107,6 +112,19 @@ impl CanonStateNotification { } } + /// Returns the new tip of the chain. + /// + /// Returns the new tip for [Self::Reorg] and [Self::Commit] variants which commit at least 1 + /// new block. Returns the first block of the chain for [Self::Revert] variant, which is the + /// block that the chain reverted to. + pub fn tip(&self) -> &SealedBlockWithSenders { + match self { + Self::Reorg { new, .. } => new.tip(), + Self::Revert { old } => old.first(), + Self::Commit { new } => new.tip(), + } + } + /// Return receipt with its block number and transaction hash. /// /// Last boolean is true if receipt is from reverted block.