refactor test_batch_commit_gap with test fixture

jonastheis · jonastheis · commit c19beb824fc7 · 2025-11-21T16:59:07.000+08:00
diff --git a/crates/node/src/test_utils/event_utils.rs b/crates/node/src/test_utils/event_utils.rs
@@ -143,6 +143,53 @@ impl<'a> EventWaiter<'a> {
         Ok(())
     }
 
+    /// Wait for batch commit indexed event on all specified nodes.
+    pub async fn batch_commit_indexed(self, target_batch_index: u64, target_l1_block_number: u64) -> eyre::Result<()> {
+        self.wait_for_event_on_all(|e| {
+            if let ChainOrchestratorEvent::BatchCommitIndexed {batch_info, l1_block_number} = e {
+                (batch_info.index == target_batch_index && *l1_block_number == target_l1_block_number).then_some(())
+            } else {
+                None
+            }
+        })
+        .await?;
+        Ok(())
+    }
+
+    /// Wait for batch commit duplicate event on all specified nodes.
+    pub async fn batch_commit_duplicates(self, target_batch_index: u64) -> eyre::Result<()> {
+        self.wait_for_event_on_all(|e| {
+            if let ChainOrchestratorEvent::BatchCommitDuplicate(batch_index) = e {
+                (*batch_index == target_batch_index).then_some(())
+            } else {
+                None
+            }
+        })
+        .await?;
+        Ok(())
+    }
+
+    /// Wait for batch commit gap event on all specified nodes.
+    pub async fn batch_commit_gap(
+        self,
+        expected_missing_index: u64,
+        expected_l1_block_number_reset: u64,
+    ) -> eyre::Result<()> {
+        self.wait_for_event_on_all(|e| {
+            if let ChainOrchestratorEvent::BatchCommitGap { missing_index, l1_block_number_reset } =
+                e
+            {
+                (*missing_index == expected_missing_index
+                    && *l1_block_number_reset == expected_l1_block_number_reset)
+                    .then_some(())
+            } else {
+                None
+            }
+        })
+        .await?;
+        Ok(())
+    }
+
     /// Wait for batch reverted event on all specified nodes.
     pub async fn batch_reverted(self) -> eyre::Result<()> {
         self.wait_for_event_on_all(|e| {
diff --git a/crates/node/src/test_utils/fixture.rs b/crates/node/src/test_utils/fixture.rs
@@ -27,7 +27,7 @@ use reth_tokio_util::EventStream;
 use rollup_node_chain_orchestrator::{ChainOrchestratorEvent, ChainOrchestratorHandle};
 use rollup_node_primitives::BlockInfo;
 use rollup_node_sequencer::L1MessageInclusionMode;
-use rollup_node_watcher::L1Notification;
+use rollup_node_watcher::{L1Notification, L1WatcherCommand};
 use scroll_alloy_consensus::ScrollPooledTransaction;
 use scroll_alloy_provider::{ScrollAuthApiEngineClient, ScrollEngineApi};
 use scroll_alloy_rpc_types::Transaction;
@@ -36,8 +36,9 @@ use std::{
     fmt::{Debug, Formatter},
     path::PathBuf,
     sync::Arc,
+    time::Duration,
 };
-use tokio::sync::{mpsc, Mutex};
+use tokio::sync::{mpsc, mpsc::UnboundedReceiver, Mutex};
 
 /// Main test fixture providing a high-level interface for testing rollup nodes.
 #[derive(Debug)]
@@ -77,6 +78,8 @@ pub struct NodeHandle {
     pub engine: Engine<Arc<dyn ScrollEngineApi + Send + Sync + 'static>>,
     /// L1 watcher notification channel.
     pub l1_watcher_tx: Option<mpsc::Sender<Arc<L1Notification>>>,
+    /// L1 watcher command receiver.
+    pub l1_watcher_command_rx: Arc<Mutex<UnboundedReceiver<L1WatcherCommand>>>,
     /// Chain orchestrator listener.
     pub chain_orchestrator_rx: EventStream<ChainOrchestratorEvent>,
     /// Chain orchestrator handle.
@@ -201,6 +204,27 @@ impl TestFixture {
     ) -> eyre::Result<rollup_node_chain_orchestrator::ChainOrchestratorStatus> {
         self.get_status(0).await
     }
+
+    /// Wait for an L1 watcher command on the sequencer node.
+    ///
+    /// Returns the received command or an error if timeout is reached.
+    pub async fn expect_l1_watcher_command(&self) -> eyre::Result<L1WatcherCommand> {
+        self.expect_l1_watcher_command_on(0).await
+    }
+
+    /// Wait for an L1 watcher command on a specific node.
+    ///
+    /// Returns the received command or an error if timeout is reached.
+    pub async fn expect_l1_watcher_command_on(
+        &self,
+        node_index: usize,
+    ) -> eyre::Result<L1WatcherCommand> {
+        let mut command_rx = self.nodes[node_index].l1_watcher_command_rx.lock().await;
+        tokio::time::timeout(Duration::from_secs(5), command_rx.recv())
+            .await
+            .map_err(|_| eyre::eyre!("Timeout waiting for L1 watcher command"))?
+            .ok_or_else(|| eyre::eyre!("L1 watcher command channel closed"))
+    }
 }
 
 /// Builder for creating test fixtures with a fluent API.
@@ -452,6 +476,7 @@ impl TestFixtureBuilder {
 
             // Get handles if available
             let l1_watcher_tx = node.inner.add_ons_handle.l1_watcher_tx.clone();
+            let l1_watcher_command_rx = node.inner.add_ons_handle.l1_watcher_command_rx.clone();
             let rollup_manager_handle = node.inner.add_ons_handle.rollup_manager_handle.clone();
             let chain_orchestrator_rx =
                 node.inner.add_ons_handle.rollup_manager_handle.get_event_listener().await?;
@@ -461,6 +486,7 @@ impl TestFixtureBuilder {
                 engine,
                 chain_orchestrator_rx,
                 l1_watcher_tx,
+                l1_watcher_command_rx,
                 rollup_manager_handle,
                 typ: if config.sequencer_args.sequencer_enabled && index == 0 {
                     NodeType::Sequencer
diff --git a/crates/node/tests/e2e.rs b/crates/node/tests/e2e.rs
@@ -1814,104 +1814,38 @@ async fn signer_rotation() -> eyre::Result<()> {
 
 // Test that the chain orchestrator detects gaps in batch commits, triggers a reset command to the
 // L1 watcher for self-healing and skips duplicate batch commits.
-// #[tokio::test]
-// async fn test_batch_commit_gap() -> eyre::Result<()> {
-//     reth_tracing::init_test_tracing();
-//
-//     let (mut nodes, _tasks, _wallet) = setup_engine(
-//         default_test_scroll_rollup_node_config(),
-//         1,
-//         (*SCROLL_DEV).clone(),
-//         false,
-//         false,
-//     )
-//     .await?;
-//     let node = nodes.pop().unwrap();
-//
-//     // Get handles for sending L1 notifications and receiving commands
-//     let l1_watcher_tx = node.inner.add_ons_handle.l1_watcher_tx.clone().unwrap();
-//     let l1_watcher_command_rx = node.inner.add_ons_handle.l1_watcher_command_rx.clone();
-//     let chain_orchestrator = node.inner.add_ons_handle.rollup_manager_handle.clone();
-//
-//     // Get event listener to monitor chain orchestrator events
-//     let mut events = chain_orchestrator.get_event_listener().await?;
-//
-//     // Node is unsynced initially -> does not derive batches (which is what we want)
-//
-//     // Send batch commit 1 to populate the database
-//     let batch_commit_1 =
-//         BatchCommitData { hash: B256::random(), index: 1, block_number: 1, ..Default::default()
-// };
-//
-//     l1_watcher_tx
-//         .send(Arc::new(L1Notification::BatchCommit {
-//             block_info: BlockInfo { number: batch_commit_1.block_number, hash: B256::random() },
-//             data: batch_commit_1.clone(),
-//         }))
-//         .await?;
-//     wait_for_event_5s(
-//         &mut events,
-//         ChainOrchestratorEvent::BatchCommitIndexed {
-//             batch_info: BatchInfo { index: batch_commit_1.index, hash: batch_commit_1.hash },
-//             l1_block_number: batch_commit_1.block_number,
-//         },
-//     )
-//     .await?;
-//
-//     // Send duplicate batch commit 1 - should be skipped and duplicate detected
-//     l1_watcher_tx
-//         .send(Arc::new(L1Notification::BatchCommit {
-//             block_info: BlockInfo { number: batch_commit_1.block_number, hash: B256::random() },
-//             data: batch_commit_1.clone(),
-//         }))
-//         .await?;
-//     wait_for_event_5s(
-//         &mut events,
-//         ChainOrchestratorEvent::BatchCommitDuplicate(batch_commit_1.index),
-//     )
-//     .await?;
-//
-//     // Send batch commit 3 - should trigger reset due to gap (missing batch 2)
-//     let batch_commit_3 = BatchCommitData {
-//         hash: B256::random(),
-//         index: 3, // Gap! Missing index 2
-//         block_number: 3,
-//         ..Default::default()
-//     };
-//
-//     l1_watcher_tx
-//         .send(Arc::new(L1Notification::BatchCommit {
-//             block_info: BlockInfo { number: batch_commit_3.block_number, hash: B256::random() },
-//             data: batch_commit_3.clone(),
-//         }))
-//         .await?;
-//     wait_for_event_5s(
-//         &mut events,
-//         ChainOrchestratorEvent::BatchCommitGap {
-//             missing_index: batch_commit_3.index,
-//             l1_block_number_reset: batch_commit_1.block_number,
-//         },
-//     )
-//     .await?;
-//
-//     let mut command_rx = l1_watcher_command_rx.lock().await;
-//     let command = tokio::time::timeout(Duration::from_secs(5), command_rx.recv())
-//         .await
-//         .expect("should receive command within timeout")
-//         .expect("should receive Some(command)");
-//
-//     // Verify it's a ResetToBlock command with the correct block number
-//     match command {
-//         L1WatcherCommand::ResetToBlock { block, .. } => {
-//             assert_eq!(
-//                 block, batch_commit_1.block_number,
-//                 "Reset block should be the L1 block of the last known batch"
-//             );
-//         }
-//     }
-//
-//     Ok(())
-// }
+#[tokio::test]
+async fn test_batch_commit_gap() -> eyre::Result<()> {
+    reth_tracing::init_test_tracing();
+
+    let mut fixture = TestFixture::builder().sequencer().build().await?;
+
+    // Node is unsynced initially -> does not derive batches (which is what we want)
+
+    let batch_1_hash = B256::random();
+    // Send batch commit 1 to populate the database
+    fixture.l1().commit_batch().hash(batch_1_hash).index(1).block_number(1).send().await?;
+    fixture.expect_event().batch_commit_indexed(1, 1).await?;
+
+    // Send duplicate batch commit 1 - should be skipped and duplicate detected
+    fixture.l1().commit_batch().hash(batch_1_hash).index(1).block_number(1).send().await?;
+    fixture.expect_event().batch_commit_duplicates(1).await?;
+
+    // Send batch commit 3 - should trigger reset due to gap (missing batch 2)
+    fixture.l1().commit_batch().index(3).block_number(3).send().await?;
+    // Expect gap event: missing index 3, reset to L1 block 1 (where batch 1 was committed)
+    fixture.expect_event().batch_commit_gap(3, 1).await?;
+
+    // Verify that a ResetToBlock command was sent to the L1 watcher
+    let command = fixture.expect_l1_watcher_command().await?;
+    match command {
+        rollup_node_watcher::L1WatcherCommand::ResetToBlock { block, .. } => {
+            assert_eq!(block, 1, "Reset block should be the L1 block of the last known batch");
+        }
+    }
+
+    Ok(())
+}
 
 // Test that the chain orchestrator detects gaps in L1 messages, triggers a reset command to the
 // L1 watcher for self-healing and skips duplicate L1 messages received.
