chore(docs): clarify tree canonical chain docs (#8408)

crates/blockchain-tree/src/block_indices.rs

@@ -17,9 +17,9 @@ use std::collections::{btree_map, hash_map, BTreeMap, BTreeSet, HashMap, HashSet
 pub struct BlockIndices {
     /// Last finalized block.
     last_finalized_block: BlockNumber,
-    /// Canonical chain. Contains N number (depends on `finalization_depth`) of blocks.
-    /// These blocks are found in fork_to_child but not inside `blocks_to_chain` or
-    /// `number_to_block` as those are chain specific indices.
+    /// Non-finalized canonical chain. Contains N number (depends on `finalization_depth`) of
+    /// blocks. These blocks are found in fork_to_child but not inside `blocks_to_chain` or
+    /// `number_to_block` as those are sidechain specific indices.
     canonical_chain: CanonicalChain,
     /// Index needed when discarding the chain, so we can remove connected chains from tree.
     ///
@@ -101,14 +101,6 @@ 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<BlockNumber> {
-        self.canonical_chain.get_canonical_block_number(self.last_finalized_block, block_hash)
-    }
-
     /// Last finalized block
     pub fn last_finalized_block(&self) -> BlockNumber {
         self.last_finalized_block
@@ -138,8 +130,8 @@ impl BlockIndices {
         self.fork_to_child.entry(first.parent_hash).or_default().insert_if_absent(first.hash());
     }
 
-    /// Get the chain ID the block belongs to
-    pub(crate) fn get_blocks_chain_id(&self, block: &BlockHash) -> Option<BlockchainId> {
+    /// Get the [BlockchainId] the given block belongs to if it exists.
+    pub(crate) fn get_block_chain_id(&self, block: &BlockHash) -> Option<BlockchainId> {
         self.blocks_to_chain.get(block).cloned()
     }
 
@@ -370,7 +362,7 @@ impl BlockIndices {
 
     /// Returns the block number of the canonical block with the given hash.
     #[inline]
-    pub fn canonical_number(&self, block_hash: BlockHash) -> Option<BlockNumber> {
+    pub fn canonical_number(&self, block_hash: &BlockHash) -> Option<BlockNumber> {
         self.canonical_chain.canonical_number(block_hash)
     }
 

crates/blockchain-tree/src/blockchain_tree.rs

@@ -215,7 +215,7 @@ where
         }
 
         // is block inside chain
-        if let Some(attachment) = self.is_block_inside_chain(&block) {
+        if let Some(attachment) = self.is_block_inside_sidechain(&block) {
             return Ok(Some(BlockStatus::Valid(attachment)))
         }
 
@@ -260,7 +260,7 @@ where
     }
 
     /// Returns true if the block is included in a side-chain.
-    fn is_block_hash_inside_chain(&self, block_hash: BlockHash) -> bool {
+    fn is_block_hash_inside_sidechain(&self, block_hash: BlockHash) -> bool {
         self.block_by_hash(block_hash).is_some()
     }
 
@@ -287,7 +287,7 @@ where
         let canonical_chain = self.state.block_indices.canonical_chain();
 
         // if it is part of the chain
-        if let Some(chain_id) = self.block_indices().get_blocks_chain_id(&block_hash) {
+        if let Some(chain_id) = self.block_indices().get_block_chain_id(&block_hash) {
             trace!(target: "blockchain_tree", ?block_hash, "Constructing post state data based on non-canonical chain");
             // get block state
             let Some(chain) = self.state.chains.get(&chain_id) else {
@@ -318,7 +318,7 @@ where
         }
 
         // check if there is canonical block
-        if let Some(canonical_number) = canonical_chain.canonical_number(block_hash) {
+        if let Some(canonical_number) = canonical_chain.canonical_number(&block_hash) {
             trace!(target: "blockchain_tree", %block_hash, "Constructing post state data based on canonical chain");
             return Some(BundleStateData {
                 canonical_fork: ForkBlock { number: canonical_number, hash: block_hash },
@@ -345,7 +345,7 @@ where
         let parent = block.parent_num_hash();
 
         // check if block parent can be found in any side chain.
-        if let Some(chain_id) = self.block_indices().get_blocks_chain_id(&parent.hash) {
+        if let Some(chain_id) = self.block_indices().get_block_chain_id(&parent.hash) {
             // found parent in side tree, try to insert there
             return self.try_insert_block_into_side_chain(block, chain_id, block_validation_kind)
         }
@@ -358,7 +358,7 @@ where
         // this is another check to ensure that if the block points to a canonical block its block
         // is valid
         if let Some(canonical_parent_number) =
-            self.block_indices().canonical_number(block.parent_hash)
+            self.block_indices().canonical_number(&block.parent_hash)
         {
             // we found the parent block in canonical chain
             if canonical_parent_number != parent.number {
@@ -458,7 +458,7 @@ where
 
     /// Try inserting a block into the given side chain.
     ///
-    /// WARNING: This expects a valid side chain id, see [BlockIndices::get_blocks_chain_id]
+    /// WARNING: This expects a valid side chain id, see [BlockIndices::get_block_chain_id]
     #[instrument(level = "trace", skip_all, target = "blockchain_tree")]
     fn try_insert_block_into_side_chain(
         &mut self,
@@ -557,8 +557,7 @@ where
             }
 
             let fork_block = chain.fork_block();
-            if let Some(next_chain_id) = self.block_indices().get_blocks_chain_id(&fork_block.hash)
-            {
+            if let Some(next_chain_id) = self.block_indices().get_block_chain_id(&fork_block.hash) {
                 chain_id = next_chain_id;
             } else {
                 // if there is no fork block that point to other chains, break the loop.
@@ -582,7 +581,7 @@ where
             // chain fork block
             fork = self.state.chains.get(&chain_id)?.fork_block();
             // get fork block chain
-            if let Some(fork_chain_id) = self.block_indices().get_blocks_chain_id(&fork.hash) {
+            if let Some(fork_chain_id) = self.block_indices().get_block_chain_id(&fork.hash) {
                 chain_id = fork_chain_id;
                 continue
             }
@@ -608,7 +607,7 @@ where
 
         while let Some(block) = dependent_block.pop_back() {
             // Get chain of dependent block.
-            let Some(chain_id) = self.block_indices().get_blocks_chain_id(&block) else {
+            let Some(chain_id) = self.block_indices().get_block_chain_id(&block) else {
                 debug!(target: "blockchain_tree", ?block, "Block not in tree");
                 return Default::default();
             };
@@ -735,15 +734,15 @@ where
         Ok(())
     }
 
-    /// Check if block is found inside chain and its attachment.
+    /// Check if block is found inside a sidechain and its attachment.
     ///
     /// if it is canonical or extends the canonical chain, return [BlockAttachment::Canonical]
     /// if it does not extend the canonical chain, return [BlockAttachment::HistoricalFork]
     /// if the block is not in the tree or its chain id is not valid, return None
     #[track_caller]
-    fn is_block_inside_chain(&self, block: &BlockNumHash) -> Option<BlockAttachment> {
+    fn is_block_inside_sidechain(&self, block: &BlockNumHash) -> Option<BlockAttachment> {
         // check if block known and is already in the tree
-        if let Some(chain_id) = self.block_indices().get_blocks_chain_id(&block.hash) {
+        if let Some(chain_id) = self.block_indices().get_block_chain_id(&block.hash) {
             // find the canonical fork of this chain
             let Some(canonical_fork) = self.canonical_fork(chain_id) else {
                 debug!(target: "blockchain_tree", chain_id=?chain_id, block=?block.hash, "Chain id not valid");
@@ -981,22 +980,25 @@ where
     ///
     /// Returns `Ok(None)` if the block hash is not canonical (block hash does not exist, or is
     /// included in a sidechain).
+    ///
+    /// Note: this does not distinguish between a block that is finalized and a block that is not
+    /// finalized yet, only whether it is part of the canonical chain or not.
     pub fn find_canonical_header(
         &self,
         hash: &BlockHash,
     ) -> Result<Option<SealedHeader>, ProviderError> {
         // 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.
+        // canonical, but in the db. If it is in a sidechain, it is not canonical. If it is missing
+        // in the db, then it is also not canonical.
 
         let provider = self.externals.provider_factory.provider()?;
 
         let mut header = None;
-        if let Some(num) = self.block_indices().get_canonical_block_number(hash) {
+        if let Some(num) = self.block_indices().canonical_number(hash) {
             header = provider.header_by_number(num)?;
         }
 
-        if header.is_none() && self.is_block_hash_inside_chain(*hash) {
+        if header.is_none() && self.is_block_hash_inside_sidechain(*hash) {
             return Ok(None)
         }
 
@@ -1008,6 +1010,9 @@ where
     }
 
     /// Determines whether or not a block is canonical, checking the db if necessary.
+    ///
+    /// Note: this does not distinguish between a block that is finalized and a block that is not
+    /// finalized yet, only whether it is part of the canonical chain or not.
     pub fn is_block_hash_canonical(&self, hash: &BlockHash) -> Result<bool, ProviderError> {
         self.find_canonical_header(hash).map(|header| header.is_some())
     }
@@ -1062,7 +1067,7 @@ where
             return Ok(CanonicalOutcome::AlreadyCanonical { header })
         }
 
-        let Some(chain_id) = self.block_indices().get_blocks_chain_id(&block_hash) else {
+        let Some(chain_id) = self.block_indices().get_block_chain_id(&block_hash) else {
             debug!(target: "blockchain_tree", ?block_hash, "Block hash not found in block indices");
             return Err(CanonicalError::from(BlockchainTreeError::BlockHashNotFoundInChain {
                 block_hash,
@@ -1085,7 +1090,7 @@ where
         let mut chains_to_promote = vec![canonical];
 
         // loop while fork blocks are found in Tree.
-        while let Some(chain_id) = self.block_indices().get_blocks_chain_id(&fork_block.hash) {
+        while let Some(chain_id) = self.block_indices().get_block_chain_id(&fork_block.hash) {
             // canonical chain is lower part of the chain.
             let Some(canonical) =
                 self.remove_and_split_chain(chain_id, ChainSplitTarget::Number(fork_block.number))
@@ -1200,7 +1205,7 @@ where
             .unwrap_or_default()
             .into_iter()
             .for_each(|child| {
-                if let Some(chain_id) = self.block_indices().get_blocks_chain_id(&child) {
+                if let Some(chain_id) = self.block_indices().get_block_chain_id(&child) {
                     if let Some(chain) = self.state.chains.get_mut(&chain_id) {
                         chain.clear_trie_updates();
                     }
@@ -1307,8 +1312,8 @@ where
         // This should only happen when an optimistic sync target was re-orged.
         //
         // Static files generally contain finalized data. The blockchain tree only deals
-        // with unfinalized data. The only scenario where canonical reverts go past the highest
-        // static file is when an optimistic sync occured and unfinalized data was written to
+        // with non-finalized data. The only scenario where canonical reverts go past the highest
+        // static file is when an optimistic sync occurred and non-finalized data was written to
         // static files.
         if self
             .externals
@@ -1780,7 +1785,7 @@ mod tests {
         );
 
         let block3a_chain_id =
-            tree.state.block_indices.get_blocks_chain_id(&block3a.hash()).unwrap();
+            tree.state.block_indices.get_block_chain_id(&block3a.hash()).unwrap();
         assert_eq!(
             tree.all_chain_hashes(block3a_chain_id),
             BTreeMap::from([
@@ -1820,7 +1825,7 @@ mod tests {
             tree.insert_block(block1.clone(), BlockValidationKind::Exhaustive).unwrap(),
             InsertPayloadOk::Inserted(BlockStatus::Valid(BlockAttachment::Canonical))
         );
-        let block1_chain_id = tree.state.block_indices.get_blocks_chain_id(&block1.hash()).unwrap();
+        let block1_chain_id = tree.state.block_indices.get_block_chain_id(&block1.hash()).unwrap();
         let block1_chain = tree.state.chains.get(&block1_chain_id).unwrap();
         assert!(block1_chain.trie_updates().is_some());
 
@@ -1828,7 +1833,7 @@ mod tests {
             tree.insert_block(block2.clone(), BlockValidationKind::Exhaustive).unwrap(),
             InsertPayloadOk::Inserted(BlockStatus::Valid(BlockAttachment::Canonical))
         );
-        let block2_chain_id = tree.state.block_indices.get_blocks_chain_id(&block2.hash()).unwrap();
+        let block2_chain_id = tree.state.block_indices.get_block_chain_id(&block2.hash()).unwrap();
         let block2_chain = tree.state.chains.get(&block2_chain_id).unwrap();
         assert!(block2_chain.trie_updates().is_none());
 
@@ -1841,7 +1846,7 @@ mod tests {
             tree.insert_block(block3.clone(), BlockValidationKind::Exhaustive).unwrap(),
             InsertPayloadOk::Inserted(BlockStatus::Valid(BlockAttachment::Canonical))
         );
-        let block3_chain_id = tree.state.block_indices.get_blocks_chain_id(&block3.hash()).unwrap();
+        let block3_chain_id = tree.state.block_indices.get_block_chain_id(&block3.hash()).unwrap();
         let block3_chain = tree.state.chains.get(&block3_chain_id).unwrap();
         assert!(block3_chain.trie_updates().is_some());
 
@@ -1854,7 +1859,7 @@ mod tests {
             tree.insert_block(block4.clone(), BlockValidationKind::Exhaustive).unwrap(),
             InsertPayloadOk::Inserted(BlockStatus::Valid(BlockAttachment::Canonical))
         );
-        let block4_chain_id = tree.state.block_indices.get_blocks_chain_id(&block4.hash()).unwrap();
+        let block4_chain_id = tree.state.block_indices.get_block_chain_id(&block4.hash()).unwrap();
         let block4_chain = tree.state.chains.get(&block4_chain_id).unwrap();
         assert!(block4_chain.trie_updates().is_some());
 
@@ -1863,7 +1868,7 @@ mod tests {
             InsertPayloadOk::Inserted(BlockStatus::Valid(BlockAttachment::Canonical))
         );
 
-        let block5_chain_id = tree.state.block_indices.get_blocks_chain_id(&block5.hash()).unwrap();
+        let block5_chain_id = tree.state.block_indices.get_block_chain_id(&block5.hash()).unwrap();
         let block5_chain = tree.state.chains.get(&block5_chain_id).unwrap();
         assert!(block5_chain.trie_updates().is_none());
 

crates/blockchain-tree/src/canonical_chain.rs

@@ -1,13 +1,13 @@
 use reth_primitives::{BlockHash, BlockNumHash, BlockNumber};
 use std::collections::BTreeMap;
 
-/// This keeps track of all blocks of the canonical chain.
+/// This keeps track of (non-finalized) blocks of the canonical chain.
 ///
 /// This is a wrapper type around an ordered set of block numbers and hashes that belong to the
-/// canonical chain.
+/// canonical chain that is not yet finalized.
 #[derive(Debug, Clone, Default)]
 pub(crate) struct CanonicalChain {
-    /// All blocks of the canonical chain in order.
+    /// All blocks of the canonical chain in order of their block number.
     chain: BTreeMap<BlockNumber, BlockHash>,
 }
 
@@ -22,18 +22,18 @@ impl CanonicalChain {
         self.chain = chain;
     }
 
-    /// Returns the block hash of the canonical block with the given number.
+    /// Returns the block hash of the (non-finalized) canonical block with the given number.
     #[inline]
     pub(crate) fn canonical_hash(&self, number: &BlockNumber) -> Option<BlockHash> {
         self.chain.get(number).cloned()
     }
 
-    /// Returns the block number of the canonical block with the given hash.
+    /// Returns the block number of the (non-finalized) canonical block with the given hash.
     #[inline]
-    pub(crate) fn canonical_number(&self, block_hash: BlockHash) -> Option<BlockNumber> {
+    pub(crate) fn canonical_number(&self, block_hash: &BlockHash) -> Option<BlockNumber> {
         self.chain.iter().find_map(
             |(number, hash)| {
-                if *hash == block_hash {
+                if hash == block_hash {
                     Some(*number)
                 } else {
                     None
@@ -42,20 +42,6 @@ impl CanonicalChain {
         )
     }
 
-    /// 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,
-        last_finalized_block: BlockNumber,
-        block_hash: &BlockHash,
-    ) -> Option<BlockNumber> {
-        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.
     #[inline]
     pub(crate) fn extend(&mut self, blocks: impl Iterator<Item = (BlockNumber, BlockHash)>) {

crates/blockchain-tree/src/state.rs

@@ -68,7 +68,7 @@ impl TreeState {
         &self,
         block_hash: BlockHash,
     ) -> Option<&SealedBlockWithSenders> {
-        let id = self.block_indices.get_blocks_chain_id(&block_hash)?;
+        let id = self.block_indices.get_block_chain_id(&block_hash)?;
         let chain = self.chains.get(&id)?;
         chain.block_with_senders(block_hash)
     }
@@ -77,7 +77,7 @@ impl TreeState {
     ///
     /// Caution: This will not return blocks from the canonical chain.
     pub(crate) fn receipts_by_block_hash(&self, block_hash: BlockHash) -> Option<Vec<&Receipt>> {
-        let id = self.block_indices.get_blocks_chain_id(&block_hash)?;
+        let id = self.block_indices.get_block_chain_id(&block_hash)?;
         let chain = self.chains.get(&id)?;
         chain.receipts_by_block_hash(block_hash)
     }