fix(tree): overlapping parent block hashes (#7669)

crates/blockchain-tree/src/blockchain_tree.rs

@@ -28,7 +28,7 @@ use reth_provider::{
 };
 use reth_stages::{MetricEvent, MetricEventsSender};
 use std::{
-    collections::{BTreeMap, HashSet},
+    collections::{btree_map::Entry, BTreeMap, HashSet},
     sync::Arc,
 };
 use tracing::{debug, error, info, instrument, trace, warn};
@@ -495,19 +495,30 @@ where
     }
 
     /// Get all block hashes from a sidechain that are not part of the canonical chain.
-    ///
     /// This is a one time operation per block.
     ///
     /// # Note
     ///
     /// This is not cached in order to save memory.
     fn all_chain_hashes(&self, chain_id: BlockChainId) -> BTreeMap<BlockNumber, BlockHash> {
-        // find chain and iterate over it,
         let mut chain_id = chain_id;
         let mut hashes = BTreeMap::new();
         loop {
             let Some(chain) = self.state.chains.get(&chain_id) else { return hashes };
-            hashes.extend(chain.blocks().values().map(|b| (b.number, b.hash())));
+
+            // The parent chains might contain blocks with overlapping numbers or numbers greater
+            // than original chain tip. Insert the block hash only if it's not present
+            // for the given block number and the block number does not exceed the
+            // original chain tip.
+            let latest_block_number = hashes
+                .last_key_value()
+                .map(|(number, _)| *number)
+                .unwrap_or_else(|| chain.tip().number);
+            for block in chain.blocks().values().filter(|b| b.number <= latest_block_number) {
+                if let Entry::Vacant(e) = hashes.entry(block.number) {
+                    e.insert(block.hash());
+                }
+            }
 
             let fork_block = chain.fork_block();
             if let Some(next_chain_id) = self.block_indices().get_blocks_chain_id(&fork_block.hash)
@@ -1590,6 +1601,82 @@ mod tests {
         );
     }
 
+    #[test]
+    fn sidechain_block_hashes() {
+        let data = BlockChainTestData::default_from_number(11);
+        let (block1, exec1) = data.blocks[0].clone();
+        let (block2, exec2) = data.blocks[1].clone();
+        let (block3, exec3) = data.blocks[2].clone();
+        let (block4, exec4) = data.blocks[3].clone();
+        let genesis = data.genesis;
+
+        // test pops execution results from vector, so order is from last to first.
+        let externals =
+            setup_externals(vec![exec3.clone(), exec2.clone(), exec4, exec3, exec2, exec1]);
+
+        // last finalized block would be number 9.
+        setup_genesis(&externals.provider_factory, genesis);
+
+        // make tree
+        let config = BlockchainTreeConfig::new(1, 2, 3, 2);
+        let mut tree = BlockchainTree::new(externals, config, None).expect("failed to create tree");
+        // genesis block 10 is already canonical
+        tree.make_canonical(B256::ZERO).unwrap();
+
+        // make genesis block 10 as finalized
+        tree.finalize_block(10);
+
+        assert_eq!(
+            tree.insert_block(block1.clone(), BlockValidationKind::Exhaustive).unwrap(),
+            InsertPayloadOk::Inserted(BlockStatus::Valid(BlockAttachment::Canonical))
+        );
+
+        assert_eq!(
+            tree.insert_block(block2.clone(), BlockValidationKind::Exhaustive).unwrap(),
+            InsertPayloadOk::Inserted(BlockStatus::Valid(BlockAttachment::Canonical))
+        );
+
+        assert_eq!(
+            tree.insert_block(block3.clone(), BlockValidationKind::Exhaustive).unwrap(),
+            InsertPayloadOk::Inserted(BlockStatus::Valid(BlockAttachment::Canonical))
+        );
+
+        assert_eq!(
+            tree.insert_block(block4, BlockValidationKind::Exhaustive).unwrap(),
+            InsertPayloadOk::Inserted(BlockStatus::Valid(BlockAttachment::Canonical))
+        );
+
+        let mut block2a = block2;
+        let block2a_hash = B256::new([0x34; 32]);
+        block2a.set_hash(block2a_hash);
+
+        assert_eq!(
+            tree.insert_block(block2a.clone(), BlockValidationKind::Exhaustive).unwrap(),
+            InsertPayloadOk::Inserted(BlockStatus::Valid(BlockAttachment::HistoricalFork))
+        );
+
+        let mut block3a = block3;
+        let block3a_hash = B256::new([0x35; 32]);
+        block3a.set_hash(block3a_hash);
+        block3a.set_parent_hash(block2a.hash());
+
+        assert_eq!(
+            tree.insert_block(block3a.clone(), BlockValidationKind::Exhaustive).unwrap(),
+            InsertPayloadOk::Inserted(BlockStatus::Valid(BlockAttachment::Canonical)) /* TODO: this is incorrect, figure out why */
+        );
+
+        let block3a_chain_id =
+            tree.state.block_indices.get_blocks_chain_id(&block3a.hash()).unwrap();
+        assert_eq!(
+            tree.all_chain_hashes(block3a_chain_id),
+            BTreeMap::from([
+                (block1.number, block1.hash()),
+                (block2a.number, block2a.hash()),
+                (block3a.number, block3a.hash()),
+            ])
+        );
+    }
+
     #[test]
     fn cached_trie_updates() {
         let data = BlockChainTestData::default_from_number(11);

crates/storage/provider/src/test_utils/blocks.rs

@@ -266,7 +266,9 @@ fn block3(
                         .clone()
                         .map(|slot| (U256::from(slot), (U256::ZERO, U256::from(slot)))),
                 ),
-            );
+            )
+            .revert_account_info(number, address, Some(None))
+            .revert_storage(number, address, Vec::new());
     }
     let bundle = BundleStateWithReceipts::new(
         bundle_state_builder.build(),