fix: db

Author: yiweichi

Cargo.lock

@@ -156,6 +156,7 @@ reth-db = { git = "https://github.com/scroll-tech/reth.git", tag = "scroll-v91.3
 reth-e2e-test-utils = { git = "https://github.com/scroll-tech/reth.git", tag = "scroll-v91.3" }
 reth-eth-wire = { git = "https://github.com/scroll-tech/reth.git", tag = "scroll-v91.3", default-features = false }
 reth-eth-wire-types = { git = "https://github.com/scroll-tech/reth.git", tag = "scroll-v91.3", default-features = false }
+reth-fs-util = { git = "https://github.com/scroll-tech/reth.git", tag = "scroll-v91.3", default-features = false }
 reth-network = { git = "https://github.com/scroll-tech/reth.git", tag = "scroll-v91.3", default-features = false }
 reth-network-api = { git = "https://github.com/scroll-tech/reth.git", tag = "scroll-v91.3", default-features = false }
 reth-network-p2p = { git = "https://github.com/scroll-tech/reth.git", tag = "scroll-v91.3", default-features = false }

Cargo.toml

@@ -84,6 +84,7 @@ alloy-rpc-types-anvil = { workspace = true, optional = true }
 reth-db = { workspace = true, optional = true, features = ["test-utils"] }
 reth-e2e-test-utils = { workspace = true, optional = true }
 reth-engine-local = { workspace = true, optional = true }
+reth-fs-util = { workspace = true, optional = true }
 reth-provider = { workspace = true, optional = true }
 reth-rpc-layer = { workspace = true, optional = true }
 reth-rpc-server-types = { workspace = true, optional = true }
@@ -136,6 +137,7 @@ js-tracer = ["reth-scroll-node/js-tracer", "reth-scroll-rpc/js-tracer"]
 test-utils = [
     "reth-db",
     "reth-engine-local",
+    "reth-fs-util",
     "reth-trie-db/test-utils",
     "reth-chainspec/test-utils",
     "reth-evm/test-utils",

crates/node/Cargo.toml

@@ -21,6 +21,7 @@ use alloy_transport::layers::RetryBackoffLayer;
 use reth_chainspec::EthChainSpec;
 use reth_e2e_test_utils::{wallet::Wallet, NodeHelperType, TmpDB};
 use reth_eth_wire_types::BasicNetworkPrimitives;
+use reth_fs_util::remove_dir_all;
 use reth_network::NetworkHandle;
 use reth_node_builder::NodeTypes;
 use reth_node_types::NodeTypesWithDBAdapter;
@@ -76,6 +77,21 @@ impl Debug for TestFixture {
     }
 }
 
+impl Drop for TestFixture {
+    fn drop(&mut self) {
+        // Manually cleanup test directories.
+        // TempDatabase's automatic drop only removes the database file itself,
+        // but the parent directory also contains other files (static files, blob store, etc.)
+        // that need to be cleaned up to avoid accumulating test artifacts.
+        let parent_paths: Vec<_> =
+            self.dbs.iter().filter_map(|db| db.path().parent().map(|p| p.to_path_buf())).collect();
+        // Delete parent directories containing all database files
+        for path in parent_paths {
+            let _ = remove_dir_all(&path);
+        }
+    }
+}
+
 /// The network handle to the Scroll network.
 pub type ScrollNetworkHandle =
     NetworkHandle<BasicNetworkPrimitives<ScrollPrimitives, ScrollPooledTransaction>>;
@@ -616,7 +632,7 @@ impl TestFixtureBuilder {
             &tasks,
             self.config.clone(),
             self.num_nodes,
-            Vec::new(), // Empty dbs for initial setup
+            None,
             chain_spec.clone(),
             self.is_dev,
             self.no_local_transactions_propagation,

crates/node/src/test_utils/fixture.rs

@@ -113,7 +113,7 @@ pub async fn setup_engine(
     tasks: &TaskManager,
     mut scroll_node_config: ScrollRollupNodeConfig,
     num_nodes: usize,
-    mut dbs: Vec<Arc<reth_db::test_utils::TempDatabase<reth_db::DatabaseEnv>>>,
+    db_provided: Option<Arc<reth_db::test_utils::TempDatabase<reth_db::DatabaseEnv>>>,
     chain_spec: Arc<<ScrollRollupNode as NodeTypes>::ChainSpec>,
     is_dev: bool,
     no_local_transactions_propagation: bool,
@@ -144,6 +144,7 @@ where
 
     // Create nodes and peer them
     let mut nodes: Vec<NodeTestContext<_, _>> = Vec::with_capacity(num_nodes);
+    let mut dbs: Vec<Arc<reth_db::test_utils::TempDatabase<reth_db::DatabaseEnv>>> = Vec::new();
 
     for idx in 0..num_nodes {
         // disable sequencer nodes after the first one
@@ -164,22 +165,18 @@ where
             .set_dev(is_dev)
             .with_txpool(TxPoolArgs { no_local_transactions_propagation, ..Default::default() });
 
-        // Check if we already have a database for this index (reboot scenario)
-        let db: Arc<reth_db::test_utils::TempDatabase<reth_db::DatabaseEnv>> = if idx < dbs.len() {
+        // Check if we already have provided a database for a node (reboot scenario)
+        let db = if let Some(ref provided_db) = db_provided {
             // Reuse existing database for reboot
-            let existing_db = dbs[idx].clone();
-
-            // Get the existing datadir from the database path
-            let existing_db_path = existing_db.path();
-            let test_data_dir =
-                existing_db_path.parent().expect("db path should have a parent directory");
+            let db_path = provided_db.path();
+            let test_data_dir = db_path.parent().expect("db path should have a parent directory");
 
             // Set the datadir in node_config to reuse the same directory
             node_config.datadir.datadir =
                 reth_node_core::dirs::MaybePlatformPath::from(test_data_dir.to_path_buf());
 
             tracing::warn!("Reusing existing database for node {} at {:?}", idx, test_data_dir);
-            existing_db
+            provided_db.clone()
         } else {
             // Create a unique persistent test directory for both Reth and Scroll databases
             // Using process ID and node index to ensure uniqueness
@@ -241,7 +238,8 @@ where
             NodeTestContext::new(node, |_| panic!("should not build payloads using this method"))
                 .await?;
 
-        if idx >= dbs.len() {
+        // skip the forkchoice update when a database is provided (reboot scenario)
+        if !db_provided.is_some() {
             let genesis = node.block_hash(0);
             node.update_forkchoice(genesis, genesis).await?;
         }

crates/node/src/test_utils/mod.rs

@@ -105,7 +105,11 @@ impl TestFixture {
     /// ```
     pub async fn shutdown_node(&mut self, node_index: usize) -> eyre::Result<()> {
         if node_index >= self.nodes.len() {
-            return Err(eyre::eyre!("Node index {} out of bounds (total nodes: {})", node_index, self.nodes.len()));
+            return Err(eyre::eyre!(
+                "Node index {} out of bounds (total nodes: {})",
+                node_index,
+                self.nodes.len()
+            ));
         }
 
         if self.nodes[node_index].is_none() {
@@ -115,7 +119,8 @@ impl TestFixture {
         tracing::info!("Shutting down node at index {}", node_index);
 
         // Step 1: Explicitly shutdown the ChainOrchestrator
-        // This sends a Shutdown command that will make the ChainOrchestrator exit its event loop immediately
+        // This sends a Shutdown command that will make the ChainOrchestrator exit its event loop
+        // immediately
         if let Some(node) = &self.nodes[node_index] {
             tracing::info!("Sending shutdown command to ChainOrchestrator...");
             if let Err(e) = node.rollup_manager_handle.shutdown().await {
@@ -175,12 +180,13 @@ impl TestFixture {
     /// fixture.l1().sync().await?;
     /// fixture.expect_event().l1_synced().await?;
     /// ```
-    pub async fn start_node(
-        &mut self,
-        node_index: usize,
-    ) -> eyre::Result<()> {
+    pub async fn start_node(&mut self, node_index: usize) -> eyre::Result<()> {
         if node_index >= self.nodes.len() {
-            return Err(eyre::eyre!("Node index {} out of bounds (total nodes: {})", node_index, self.nodes.len()));
+            return Err(eyre::eyre!(
+                "Node index {} out of bounds (total nodes: {})",
+                node_index,
+                self.nodes.len()
+            ));
         }
 
         if self.nodes[node_index].is_some() {
@@ -189,24 +195,17 @@ impl TestFixture {
 
         tracing::info!("Starting node at index {} (reusing database)", node_index);
 
-        // Step 1: Prepare database for reuse
-        // Pass only the database for this specific node (preserves all state)
-        let node_dbs = if node_index < self.dbs.len() {
-            vec![self.dbs[node_index].clone()]
-        } else {
-            Vec::new() // Should not happen, but handle gracefully
-        };
-
         // Step 2: Create a new node instance with the existing database
         let (mut new_nodes, _, _) = setup_engine(
             &self.tasks,
             self.config.clone(),
-            1, // Create just one node
-            node_dbs, // Reuse the database (state is preserved)
+            1,
+            Some(self.dbs[node_index].clone()), // reuse provided database
             self.chain_spec.clone(),
-            true, // is_dev
-            false, // no_local_transactions_propagation
-        ).await?;
+            true,
+            false,
+        )
+        .await?;
 
         if new_nodes.is_empty() {
             return Err(eyre::eyre!("Failed to create new node"));
@@ -240,9 +239,8 @@ impl TestFixture {
 
         // Step 6: Initialize Engine with the restored ForkchoiceState
         let engine = Engine::new(Arc::new(engine_client), fcs);
-        let chain_orchestrator_rx = new_node.inner.add_ons_handle.rollup_manager_handle
-            .get_event_listener()
-            .await?;
+        let chain_orchestrator_rx =
+            new_node.inner.add_ons_handle.rollup_manager_handle.get_event_listener().await?;
 
         // Step 7: Determine node type (sequencer vs follower)
         let was_sequencer = self.config.sequencer_args.sequencer_enabled && node_index == 0;
@@ -267,4 +265,4 @@ impl TestFixture {
 
         Ok(())
     }
-}
+}

crates/node/src/test_utils/reboot.rs

@@ -298,16 +298,14 @@ async fn can_forward_tx_to_sequencer() -> eyre::Result<()> {
     let chain_spec = (*SCROLL_DEV).clone();
     let tasks = TaskManager::current();
     let (mut sequencer_node, _, _) =
-        setup_engine(&tasks, sequencer_node_config, 1, Vec::new(), chain_spec.clone(), false, true)
+        setup_engine(&tasks, sequencer_node_config, 1, None, chain_spec.clone(), false, true)
             .await
             .unwrap();
 
     let sequencer_url = format!("http://localhost:{}", sequencer_node[0].rpc_url().port().unwrap());
     follower_node_config.network_args.sequencer_url = Some(sequencer_url);
     let (mut follower_node, _, wallet) =
-        setup_engine(&tasks, follower_node_config, 1, Vec::new(), chain_spec, false, true)
-            .await
-            .unwrap();
+        setup_engine(&tasks, follower_node_config, 1, None, chain_spec, false, true).await.unwrap();
 
     let wallet = Arc::new(Mutex::new(wallet));
 
@@ -469,7 +467,7 @@ async fn can_bridge_blocks() -> eyre::Result<()> {
         &tasks,
         default_test_scroll_rollup_node_config(),
         1,
-        Vec::new(),
+        None,
         chain_spec.clone(),
         false,
         false,
@@ -577,7 +575,7 @@ async fn shutdown_consolidates_most_recent_batch_on_startup() -> eyre::Result<()
         &tasks,
         default_test_scroll_rollup_node_config(),
         1,
-        Vec::new(),
+        None,
         chain_spec.clone(),
         false,
         false,
@@ -852,8 +850,7 @@ async fn graceful_shutdown_sets_fcs_to_latest_signed_block_in_db_on_start_up() -
     // Launch a node
     let tasks = TaskManager::current();
     let (mut nodes, _, _) =
-        setup_engine(&tasks, config.clone(), 1, Vec::new(), chain_spec.clone(), false, false)
-            .await?;
+        setup_engine(&tasks, config.clone(), 1, None, chain_spec.clone(), false, false).await?;
     let node = nodes.pop().unwrap();
 
     // Instantiate the rollup node manager.