refactor(engine, tree): connect buffered blocks on pruner finish (#4613)

crates/blockchain-tree/src/blockchain_tree.rs

@@ -744,7 +744,8 @@ impl<DB: Database, C: Consensus, EF: ExecutorFactory> BlockchainTree<DB, C, EF>
     }
 
     /// Reads the last `N` canonical hashes from the database and updates the block indices of the
-    /// tree.
+    /// tree by attempting to connect the buffered blocks to canonical hashes.
+    ///
     ///
     /// `N` is the `max_reorg_depth` plus the number of block hashes needed to satisfy the
     /// `BLOCKHASH` opcode in the EVM.
@@ -753,21 +754,12 @@ impl<DB: Database, C: Consensus, EF: ExecutorFactory> BlockchainTree<DB, C, EF>
     ///
     /// This finalizes `last_finalized_block` prior to reading the canonical hashes (using
     /// [`BlockchainTree::finalize_block`]).
-    pub fn restore_canonical_hashes_and_finalize(
+    pub fn connect_buffered_blocks_to_canonical_hashes_and_finalize(
         &mut self,
         last_finalized_block: BlockNumber,
     ) -> Result<(), Error> {
         self.finalize_block(last_finalized_block);
 
-        self.restore_canonical_hashes()
-    }
-
-    /// Reads the last `N` canonical hashes from the database and updates the block indices of the
-    /// tree.
-    ///
-    /// `N` is the `max_reorg_depth` plus the number of block hashes needed to satisfy the
-    /// `BLOCKHASH` opcode in the EVM.
-    pub fn restore_canonical_hashes(&mut self) -> Result<(), Error> {
         let num_of_canonical_hashes =
             self.config.max_reorg_depth() + self.config.num_of_additional_canonical_block_hashes();
 
@@ -790,12 +782,44 @@ impl<DB: Database, C: Consensus, EF: ExecutorFactory> BlockchainTree<DB, C, EF>
             }
         }
 
-        // check unconnected block buffer for the childs of new added blocks,
+        self.connect_buffered_blocks_to_hashes(last_canonical_hashes)?;
-        for added_block in last_canonical_hashes.into_iter() {
+
+        Ok(())
+    }
+
+    /// Reads the last `N` canonical hashes from the database and updates the block indices of the
+    /// tree by attempting to connect the buffered blocks to canonical hashes.
+    ///
+    /// `N` is the `max_reorg_depth` plus the number of block hashes needed to satisfy the
+    /// `BLOCKHASH` opcode in the EVM.
+    pub fn connect_buffered_blocks_to_canonical_hashes(&mut self) -> Result<(), Error> {
+        let num_of_canonical_hashes =
+            self.config.max_reorg_depth() + self.config.num_of_additional_canonical_block_hashes();
+
+        let last_canonical_hashes = self
+            .externals
+            .db
+            .tx()?
+            .cursor_read::<tables::CanonicalHeaders>()?
+            .walk_back(None)?
+            .take(num_of_canonical_hashes as usize)
+            .collect::<Result<BTreeMap<BlockNumber, BlockHash>, _>>()?;
+
+        self.connect_buffered_blocks_to_hashes(last_canonical_hashes)?;
+
+        Ok(())
+    }
+
+    fn connect_buffered_blocks_to_hashes(
+        &mut self,
+        hashes: impl IntoIterator<Item = impl Into<BlockNumHash>>,
+    ) -> Result<(), Error> {
+        // check unconnected block buffer for childs of the canonical hashes
+        for added_block in hashes.into_iter() {
             self.try_connect_buffered_blocks(added_block.into())
         }
 
-        // check unconnected block buffer for childs of the chains.
+        // check unconnected block buffer for childs of the chains
         let mut all_chain_blocks = Vec::new();
         for (_, chain) in self.chains.iter() {
             for (&number, blocks) in chain.blocks.iter() {
@@ -1626,7 +1650,7 @@ mod tests {
             .assert(&tree);
 
         // update canonical block to b2, this would make b2a be removed
-        assert_eq!(tree.restore_canonical_hashes_and_finalize(12), Ok(()));
+        assert_eq!(tree.connect_buffered_blocks_to_canonical_hashes_and_finalize(12), Ok(()));
 
         assert_eq!(tree.is_block_known(block2.num_hash()).unwrap(), Some(BlockStatus::Valid));
 

crates/blockchain-tree/src/shareable.rs

@@ -66,21 +66,22 @@ impl<DB: Database, C: Consensus, EF: ExecutorFactory> BlockchainTreeEngine
         tree.update_chains_metrics();
     }
 
-    fn restore_canonical_hashes_and_finalize(
+    fn connect_buffered_blocks_to_canonical_hashes_and_finalize(
         &self,
         last_finalized_block: BlockNumber,
     ) -> Result<(), Error> {
-        trace!(target: "blockchain_tree", ?last_finalized_block, "Restoring canonical hashes for last finalized block");
+        trace!(target: "blockchain_tree", ?last_finalized_block, "Connecting buffered blocks to canonical hashes and finalizing the tree");
         let mut tree = self.tree.write();
-        let res = tree.restore_canonical_hashes_and_finalize(last_finalized_block);
+        let res =
+            tree.connect_buffered_blocks_to_canonical_hashes_and_finalize(last_finalized_block);
         tree.update_chains_metrics();
         res
     }
 
-    fn restore_canonical_hashes(&self) -> Result<(), Error> {
+    fn connect_buffered_blocks_to_canonical_hashes(&self) -> Result<(), Error> {
-        trace!(target: "blockchain_tree", "Restoring canonical hashes");
+        trace!(target: "blockchain_tree", "Connecting buffered blocks to canonical hashes");
         let mut tree = self.tree.write();
-        let res = tree.restore_canonical_hashes();
+        let res = tree.connect_buffered_blocks_to_canonical_hashes();
         tree.update_chains_metrics();
         res
     }

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

@@ -88,9 +88,8 @@ impl EngineHookEvent {
 pub enum EngineHookAction {
     /// Notify about a [SyncState] update.
     UpdateSyncState(SyncState),
-    /// Read the last relevant canonical hashes from the database and update the block indices of
+    /// Connect blocks buffered during the hook execution to canonical hashes.
-    /// the blockchain tree.
+    ConnectBufferedBlocks,
-    RestoreCanonicalHashes,
 }
 
 /// An error returned by [hook][`EngineHook`].

crates/consensus/beacon/src/engine/hooks/prune.rs

@@ -74,7 +74,7 @@ impl<DB: Database + 'static> PruneHook<DB> {
         };
 
         let action = if matches!(event, EngineHookEvent::Finished(Ok(_))) {
-            Some(EngineHookAction::RestoreCanonicalHashes)
+            Some(EngineHookAction::ConnectBufferedBlocks)
         } else {
             None
         };

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

@@ -1334,7 +1334,7 @@ where
         let synced_to_finalized = match self.blockchain.block_number(block_hash)? {
             Some(number) => {
                 // Attempt to restore the tree.
-                self.blockchain.restore_canonical_hashes_and_finalize(number)?;
+                self.blockchain.connect_buffered_blocks_to_canonical_hashes_and_finalize(number)?;
                 true
             }
             None => false,
@@ -1686,8 +1686,10 @@ where
             EngineHookAction::UpdateSyncState(state) => {
                 self.sync_state_updater.update_sync_state(state)
             }
-            EngineHookAction::RestoreCanonicalHashes => {
+            // TODO(alexey): always connect buffered blocks if hook had the
-                if let Err(error) = self.blockchain.restore_canonical_hashes() {
+            //  `EngineHookDBAccessLevel::ReadWrite`
+            EngineHookAction::ConnectBufferedBlocks => {
+                if let Err(error) = self.blockchain.connect_buffered_blocks_to_canonical_hashes() {
                     error!(target: "consensus::engine", ?error, "Error restoring blockchain tree state");
                     return Err(error.into())
                 }

crates/interfaces/src/blockchain_tree/mod.rs

@@ -53,7 +53,8 @@ pub trait BlockchainTreeEngine: BlockchainTreeViewer + Send + Sync {
     fn finalize_block(&self, finalized_block: BlockNumber);
 
     /// Reads the last `N` canonical hashes from the database and updates the block indices of the
-    /// tree.
+    /// tree by attempting to connect the buffered blocks to canonical hashes.
+    ///
     ///
     /// `N` is the `max_reorg_depth` plus the number of block hashes needed to satisfy the
     /// `BLOCKHASH` opcode in the EVM.
@@ -62,17 +63,17 @@ pub trait BlockchainTreeEngine: BlockchainTreeViewer + Send + Sync {
     ///
     /// This finalizes `last_finalized_block` prior to reading the canonical hashes (using
     /// [`BlockchainTreeEngine::finalize_block`]).
-    fn restore_canonical_hashes_and_finalize(
+    fn connect_buffered_blocks_to_canonical_hashes_and_finalize(
         &self,
         last_finalized_block: BlockNumber,
     ) -> Result<(), Error>;
 
     /// Reads the last `N` canonical hashes from the database and updates the block indices of the
-    /// tree.
+    /// tree by attempting to connect the buffered blocks to canonical hashes.
     ///
     /// `N` is the `max_reorg_depth` plus the number of block hashes needed to satisfy the
     /// `BLOCKHASH` opcode in the EVM.
-    fn restore_canonical_hashes(&self) -> Result<(), Error>;
+    fn connect_buffered_blocks_to_canonical_hashes(&self) -> Result<(), Error>;
 
     /// Make a block and its parent chain part of the canonical chain by committing it to the
     /// database.

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

@@ -549,15 +549,15 @@ where
         self.tree.finalize_block(finalized_block)
     }
 
-    fn restore_canonical_hashes_and_finalize(
+    fn connect_buffered_blocks_to_canonical_hashes_and_finalize(
         &self,
         last_finalized_block: BlockNumber,
     ) -> Result<()> {
-        self.tree.restore_canonical_hashes_and_finalize(last_finalized_block)
+        self.tree.connect_buffered_blocks_to_canonical_hashes_and_finalize(last_finalized_block)
     }
 
-    fn restore_canonical_hashes(&self) -> Result<()> {
+    fn connect_buffered_blocks_to_canonical_hashes(&self) -> Result<()> {
-        self.tree.restore_canonical_hashes()
+        self.tree.connect_buffered_blocks_to_canonical_hashes()
     }
 
     fn make_canonical(&self, block_hash: &BlockHash) -> Result<CanonicalOutcome> {