chore(engine): reduce number of canonical tip lookups from engine (#8421)

crates/blockchain-tree-api/src/lib.rs

@@ -162,7 +162,10 @@ impl std::fmt::Display for BlockValidationKind {
 pub enum CanonicalOutcome {
     /// The block is already canonical.
     AlreadyCanonical {
-        /// The corresponding [SealedHeader] that is already canonical.
+        /// Block number and hash of current head.
+        head: BlockNumHash,
+        /// The corresponding [SealedHeader] that was attempted to be made a current head and
+        /// is already canonical.
         header: SealedHeader,
     },
     /// Committed the block to the database.
@@ -176,7 +179,7 @@ impl CanonicalOutcome {
     /// Returns the header of the block that was made canonical.
     pub fn header(&self) -> &SealedHeader {
         match self {
-            CanonicalOutcome::AlreadyCanonical { header } => header,
+            CanonicalOutcome::AlreadyCanonical { header, .. } => header,
             CanonicalOutcome::Committed { head } => head,
         }
     }
@@ -184,7 +187,7 @@ impl CanonicalOutcome {
     /// 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::AlreadyCanonical { header, .. } => header,
             CanonicalOutcome::Committed { head } => head,
         }
     }
@@ -209,6 +212,8 @@ pub enum BlockStatus {
     /// If block is valid and block forks off canonical chain.
     /// If blocks is not connected to canonical chain.
     Disconnected {
+        /// Current canonical head.
+        head: BlockNumHash,
         /// The lowest ancestor block that is not connected to the canonical chain.
         missing_ancestor: BlockNumHash,
     },

crates/blockchain-tree/src/blockchain_tree.rs

@@ -221,7 +221,10 @@ where
 
         // check if block is disconnected
         if let Some(block) = self.state.buffered_blocks.block(&block.hash) {
-            return Ok(Some(BlockStatus::Disconnected { missing_ancestor: block.parent_num_hash() }))
+            return Ok(Some(BlockStatus::Disconnected {
+                head: self.state.block_indices.canonical_tip(),
+                missing_ancestor: block.parent_num_hash(),
+            }))
         }
 
         Ok(None)
@@ -388,7 +391,10 @@ where
             .lowest_ancestor(&block_hash)
             .ok_or(BlockchainTreeError::BlockBufferingFailed { block_hash })?;
 
-        Ok(BlockStatus::Disconnected { missing_ancestor: lowest_ancestor.parent_num_hash() })
+        Ok(BlockStatus::Disconnected {
+            head: self.state.block_indices.canonical_tip(),
+            missing_ancestor: lowest_ancestor.parent_num_hash(),
+        })
     }
 
     /// This tries to append the given block to the canonical chain.
@@ -1064,7 +1070,9 @@ where
                     hash: block_hash,
                 }))
             }
-            return Ok(CanonicalOutcome::AlreadyCanonical { header })
+
+            let head = self.state.block_indices.canonical_tip();
+            return Ok(CanonicalOutcome::AlreadyCanonical { header, head })
         }
 
         let Some(chain_id) = self.block_indices().get_block_chain_id(&block_hash) else {
@@ -2001,18 +2009,20 @@ mod tests {
 
         let mut canon_notif = tree.subscribe_canon_state();
         // genesis block 10 is already canonical
-        tree.make_canonical(B256::ZERO).unwrap();
+        let head = BlockNumHash::new(10, B256::ZERO);
+        tree.make_canonical(head.hash).unwrap();
 
         // make sure is_block_hash_canonical returns true for genesis block
         tree.is_block_hash_canonical(&B256::ZERO).unwrap();
 
         // make genesis block 10 as finalized
-        tree.finalize_block(10);
+        tree.finalize_block(head.number);
 
         // block 2 parent is not known, block2 is buffered.
         assert_eq!(
             tree.insert_block(block2.clone(), BlockValidationKind::Exhaustive).unwrap(),
             InsertPayloadOk::Inserted(BlockStatus::Disconnected {
+                head,
                 missing_ancestor: block2.parent_num_hash()
             })
         );
@@ -2029,7 +2039,7 @@ mod tests {
 
         assert_eq!(
             tree.is_block_known(block2.num_hash()).unwrap(),
-            Some(BlockStatus::Disconnected { missing_ancestor: block2.parent_num_hash() })
+            Some(BlockStatus::Disconnected { head, missing_ancestor: block2.parent_num_hash() })
         );
 
         // check if random block is known
@@ -2328,6 +2338,7 @@ mod tests {
         assert_eq!(
             tree.insert_block(block2b.clone(), BlockValidationKind::Exhaustive).unwrap(),
             InsertPayloadOk::Inserted(BlockStatus::Disconnected {
+                head: block2.header.num_hash(),
                 missing_ancestor: block2b.parent_num_hash()
             })
         );

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

@@ -393,8 +393,8 @@ where
         match make_canonical_result {
             Ok(outcome) => {
                 let should_update_head = match &outcome {
-                    CanonicalOutcome::AlreadyCanonical { header } => {
-                        self.on_head_already_canonical(header, &mut attrs)
+                    CanonicalOutcome::AlreadyCanonical { head, header } => {
+                        self.on_head_already_canonical(head, header, &mut attrs)
                     }
                     CanonicalOutcome::Committed { head } => {
                         // new VALID update that moved the canonical chain forward
@@ -448,6 +448,7 @@ where
     /// Returns `true` if the head needs to be updated.
     fn on_head_already_canonical(
         &self,
+        head: &BlockNumHash,
         header: &SealedHeader,
         attrs: &mut Option<EngineT::PayloadAttributes>,
     ) -> bool {
@@ -457,7 +458,7 @@ where
             debug!(
                 target: "consensus::engine",
                 fcu_head_num=?header.number,
-                current_head_num=?self.blockchain.canonical_tip().number,
+                current_head_num=?head.number,
                 "[Optimism] Allowing beacon reorg to old head"
             );
             return true
@@ -469,14 +470,14 @@ where
         //    and deemed `VALID`. In the case of such an event, client software MUST return
         //    `{payloadStatus: {status: VALID, latestValidHash: forkchoiceState.headBlockHash,
         //    validationError: null}, payloadId: null}`
-        if self.blockchain.canonical_tip() != header.num_hash() {
+        if head != &header.num_hash() {
             attrs.take();
         }
 
         debug!(
             target: "consensus::engine",
             fcu_head_num=?header.number,
-            current_head_num=?self.blockchain.canonical_tip().number,
+            current_head_num=?head.number,
             "Ignoring beacon update to old head"
         );
         false
@@ -1285,12 +1286,11 @@ where
         &mut self,
         downloaded_block: BlockNumHash,
         missing_parent: BlockNumHash,
+        head: BlockNumHash,
     ) {
         // compare the missing parent with the canonical tip
-        let canonical_tip_num = self.blockchain.canonical_tip().number;
-
         if let Some(target) = self.can_pipeline_sync_to_finalized(
-            canonical_tip_num,
+            head.number,
             missing_parent.number,
             Some(downloaded_block),
         ) {
@@ -1310,9 +1310,7 @@ where
         //  * the missing parent block num >= canonical tip num, but the number of missing blocks is
         //    less than the pipeline threshold
         //    * this case represents a potentially long range of blocks to download and execute
-        if let Some(distance) =
-            self.distance_from_local_tip(canonical_tip_num, missing_parent.number)
-        {
+        if let Some(distance) = self.distance_from_local_tip(head.number, missing_parent.number) {
             self.sync.download_block_range(missing_parent.hash, distance)
         } else {
             // This happens when the missing parent is on an outdated
@@ -1753,11 +1751,16 @@ where
                                 }
                             }
                             InsertPayloadOk::Inserted(BlockStatus::Disconnected {
+                                head,
                                 missing_ancestor: missing_parent,
                             }) => {
                                 // block is not connected to the canonical head, we need to download
                                 // its missing branch first
-                                self.on_disconnected_block(downloaded_num_hash, missing_parent);
+                                self.on_disconnected_block(
+                                    downloaded_num_hash,
+                                    missing_parent,
+                                    head,
+                                );
                             }
                             _ => (),
                         }