From 57c10e5b6594d68c3ce6b88be6554d243043366f Mon Sep 17 00:00:00 2001 From: Alexey Shekhirin Date: Tue, 19 Sep 2023 17:36:24 +0100 Subject: [PATCH] refactor(engine, tree): connect buffered blocks on pruner finish (#4613) --- crates/blockchain-tree/src/blockchain_tree.rs | 54 +++++++++++++------ crates/blockchain-tree/src/shareable.rs | 13 ++--- .../consensus/beacon/src/engine/hooks/mod.rs | 5 +- .../beacon/src/engine/hooks/prune.rs | 2 +- crates/consensus/beacon/src/engine/mod.rs | 8 +-- crates/interfaces/src/blockchain_tree/mod.rs | 9 ++-- crates/storage/provider/src/providers/mod.rs | 8 +-- 7 files changed, 63 insertions(+), 36 deletions(-) diff --git a/crates/blockchain-tree/src/blockchain_tree.rs b/crates/blockchain-tree/src/blockchain_tree.rs index 7302baab47cd..3c6b04202dca 100644 --- a/crates/blockchain-tree/src/blockchain_tree.rs +++ b/crates/blockchain-tree/src/blockchain_tree.rs @@ -744,7 +744,8 @@ impl BlockchainTree } /// 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 BlockchainTree /// /// 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 BlockchainTree } } - // check unconnected block buffer for the childs of new added blocks, - for added_block in last_canonical_hashes.into_iter() { + self.connect_buffered_blocks_to_hashes(last_canonical_hashes)?; + + 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::()? + .walk_back(None)? + .take(num_of_canonical_hashes as usize) + .collect::, _>>()?; + + self.connect_buffered_blocks_to_hashes(last_canonical_hashes)?; + + Ok(()) + } + + fn connect_buffered_blocks_to_hashes( + &mut self, + hashes: impl IntoIterator>, + ) -> 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)); diff --git a/crates/blockchain-tree/src/shareable.rs b/crates/blockchain-tree/src/shareable.rs index ba766b236ea0..28ede546636a 100644 --- a/crates/blockchain-tree/src/shareable.rs +++ b/crates/blockchain-tree/src/shareable.rs @@ -66,21 +66,22 @@ impl 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> { - trace!(target: "blockchain_tree", "Restoring canonical hashes"); + fn connect_buffered_blocks_to_canonical_hashes(&self) -> Result<(), Error> { + 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 } diff --git a/crates/consensus/beacon/src/engine/hooks/mod.rs b/crates/consensus/beacon/src/engine/hooks/mod.rs index 8f0877aa135f..d2770a77c1c7 100644 --- a/crates/consensus/beacon/src/engine/hooks/mod.rs +++ b/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 - /// the blockchain tree. - RestoreCanonicalHashes, + /// Connect blocks buffered during the hook execution to canonical hashes. + ConnectBufferedBlocks, } /// An error returned by [hook][`EngineHook`]. diff --git a/crates/consensus/beacon/src/engine/hooks/prune.rs b/crates/consensus/beacon/src/engine/hooks/prune.rs index 1650f9f52158..f720082ee3bc 100644 --- a/crates/consensus/beacon/src/engine/hooks/prune.rs +++ b/crates/consensus/beacon/src/engine/hooks/prune.rs @@ -74,7 +74,7 @@ impl PruneHook { }; let action = if matches!(event, EngineHookEvent::Finished(Ok(_))) { - Some(EngineHookAction::RestoreCanonicalHashes) + Some(EngineHookAction::ConnectBufferedBlocks) } else { None }; diff --git a/crates/consensus/beacon/src/engine/mod.rs b/crates/consensus/beacon/src/engine/mod.rs index 66df6a083e5b..2d6d12714ba3 100644 --- a/crates/consensus/beacon/src/engine/mod.rs +++ b/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 => { - if let Err(error) = self.blockchain.restore_canonical_hashes() { + // TODO(alexey): always connect buffered blocks if hook had the + // `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()) } diff --git a/crates/interfaces/src/blockchain_tree/mod.rs b/crates/interfaces/src/blockchain_tree/mod.rs index d8334df7decb..896434eb6cdc 100644 --- a/crates/interfaces/src/blockchain_tree/mod.rs +++ b/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. diff --git a/crates/storage/provider/src/providers/mod.rs b/crates/storage/provider/src/providers/mod.rs index 9b46aa2d6e96..d12c93d452f9 100644 --- a/crates/storage/provider/src/providers/mod.rs +++ b/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<()> { - self.tree.restore_canonical_hashes() + fn connect_buffered_blocks_to_canonical_hashes(&self) -> Result<()> { + self.tree.connect_buffered_blocks_to_canonical_hashes() } fn make_canonical(&self, block_hash: &BlockHash) -> Result {