fix(tree): retain max(additional, max_reorg_depth) block hashes (#4612)

crates/blockchain-tree/src/blockchain_tree.rs

@@ -120,7 +120,7 @@ impl<DB: Database, C: Consensus, EF: ExecutorFactory> BlockchainTree<DB, C, EF>
             .tx()?
             .cursor_read::<tables::CanonicalHeaders>()?
             .walk_back(None)?
-            .take((max_reorg_depth + config.num_of_additional_canonical_block_hashes()) as usize)
+            .take(config.num_of_canonical_hashes() as usize)
             .collect::<Result<Vec<(BlockNumber, BlockHash)>, _>>()?;
 
         // TODO(rakita) save last finalized block inside database but for now just take
@@ -747,7 +747,7 @@ impl<DB: Database, C: Consensus, EF: ExecutorFactory> BlockchainTree<DB, C, EF>
     /// 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
+    /// `N` is the maximum of `max_reorg_depth` and the number of block hashes needed to satisfy the
     /// `BLOCKHASH` opcode in the EVM.
     ///
     /// # Note
@@ -760,16 +760,13 @@ impl<DB: Database, C: Consensus, EF: ExecutorFactory> BlockchainTree<DB, C, EF>
     ) -> RethResult<()> {
         self.finalize_block(last_finalized_block);
 
-        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)
+            .take(self.config.num_of_canonical_hashes() as usize)
             .collect::<Result<BTreeMap<BlockNumber, BlockHash>, _>>()?;
 
         let (mut remove_chains, _) =
@@ -790,19 +787,16 @@ 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 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
+    /// `N` is the maximum of `max_reorg_depth` and the number of block hashes needed to satisfy the
     /// `BLOCKHASH` opcode in the EVM.
     pub fn connect_buffered_blocks_to_canonical_hashes(&mut self) -> RethResult<()> {
-        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)
+            .take(self.config.num_of_canonical_hashes() as usize)
             .collect::<Result<BTreeMap<BlockNumber, BlockHash>, _>>()?;
 
         self.connect_buffered_blocks_to_hashes(last_canonical_hashes)?;

crates/blockchain-tree/src/config.rs

@@ -11,9 +11,13 @@ pub struct BlockchainTreeConfig {
     max_reorg_depth: u64,
     /// The number of unconnected blocks that we are buffering
     max_unconnected_blocks: usize,
-    /// For EVM's "BLOCKHASH" opcode we require last 256 block hashes. So we need to specify
+    /// Number of additional block hashes to save in blockchain tree. For `BLOCKHASH` EVM opcode we
-    /// at least `additional_canonical_block_hashes`+`max_reorg_depth`, for eth that would be
+    /// need last 256 block hashes.
-    /// 256+64.
+    ///
+    /// The total number of block hashes retained in-memory will be
+    /// `max(additional_canonical_block_hashes, max_reorg_depth)`, and for Ethereum that would
+    /// be 256. It covers both number of blocks required for reorg, and number of blocks
+    /// required for `BLOCKHASH` EVM opcode.
     num_of_additional_canonical_block_hashes: u64,
 }
 
@@ -68,6 +72,18 @@ impl BlockchainTreeConfig {
         self.num_of_additional_canonical_block_hashes
     }
 
+    /// Return total number of canonical hashes that we need to retain in order to have enough
+    /// information for reorg and EVM execution.
+    ///
+    /// It is calculated as the maximum of `max_reorg_depth` (which is the number of blocks required
+    /// for the deepest reorg possible according to the consensus protocol) and
+    /// `num_of_additional_canonical_block_hashes` (which is the number of block hashes needed to
+    /// satisfy the `BLOCKHASH` opcode in the EVM. See [`crate::BundleStateDataRef`] and
+    /// [`crate::AppendableChain::new_canonical_head_fork`] where it's used).
+    pub fn num_of_canonical_hashes(&self) -> u64 {
+        self.max_reorg_depth.max(self.num_of_additional_canonical_block_hashes)
+    }
+
     /// Return max number of unconnected blocks that we are buffering
     pub fn max_unconnected_blocks(&self) -> usize {
         self.max_unconnected_blocks

crates/interfaces/src/blockchain_tree/mod.rs

@@ -56,7 +56,7 @@ pub trait BlockchainTreeEngine: BlockchainTreeViewer + Send + Sync {
     /// 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
+    /// `N` is the maximum of `max_reorg_depth` and the number of block hashes needed to satisfy the
     /// `BLOCKHASH` opcode in the EVM.
     ///
     /// # Note
@@ -71,7 +71,7 @@ pub trait BlockchainTreeEngine: BlockchainTreeViewer + Send + Sync {
     /// 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
+    /// `N` is the maximum of `max_reorg_depth` and the number of block hashes needed to satisfy the
     /// `BLOCKHASH` opcode in the EVM.
     fn connect_buffered_blocks_to_canonical_hashes(&self) -> RethResult<()>;