feat: skip empty blocks (#287)

* feat: skip empty blocks

* fix: add CLI arg to enable empty blocks

* fix: fix docker test

---------

Co-authored-by: Jonas Theis <[email protected]>

Author: varun-doshi

crates/engine/src/driver.rs

@@ -53,6 +53,8 @@ pub struct EngineDriver<EC, CS, P> {
     metrics: EngineDriverMetrics,
     /// The waker to notify when the engine driver should be polled.
     waker: AtomicWaker,
+    /// Whether to allow empty blocks.
+    allow_empty_blocks: bool,
 }
 
 impl<EC, CS, P> EngineDriver<EC, CS, P>
@@ -69,6 +71,7 @@ where
         fcs: ForkchoiceState,
         sync_at_start_up: bool,
         block_building_duration: Duration,
+        allow_empty_blocks: bool,
     ) -> Self {
         Self {
             client,
@@ -85,6 +88,7 @@ where
             engine_future: None,
             metrics: EngineDriverMetrics::default(),
             waker: AtomicWaker::new(),
+            allow_empty_blocks,
         }
     }
 
@@ -312,6 +316,12 @@ where
 
                 match result {
                     Ok(block) => {
+                        // Skip block if no transactions are present in block.
+                        if !self.allow_empty_blocks && block.body.transactions.is_empty() {
+                            tracing::trace!(target: "scroll::engine", "no transactions in block");
+                            return None;
+                        }
+
                         // Update the unsafe block info and return the block
                         let block_info = BlockInfo::new(block.number, block.hash_slow());
                         tracing::trace!(target: "scroll::engine", ?block_info, "updating unsafe block info from new payload");
@@ -531,8 +541,15 @@ mod tests {
             ForkchoiceState::from_block_info(BlockInfo { number: 0, hash: Default::default() });
         let duration = Duration::from_secs(2);
 
-        let mut driver =
-            EngineDriver::new(client, chain_spec, None::<ScrollRootProvider>, fcs, false, duration);
+        let mut driver = EngineDriver::new(
+            client,
+            chain_spec,
+            None::<ScrollRootProvider>,
+            fcs,
+            false,
+            duration,
+            true,
+        );
 
         // Initially, it should be false
         assert!(!driver.is_payload_building_in_progress());
@@ -559,6 +576,7 @@ mod tests {
             fcs,
             false,
             duration,
+            true,
         );
 
         // Initially, it should be false

crates/node/src/args.rs

@@ -261,6 +261,7 @@ impl ScrollRollupNodeConfig {
             fcs,
             self.engine_driver_args.sync_at_startup && !self.test && !chain_spec.is_dev_chain(),
             Duration::from_millis(self.sequencer_args.payload_building_duration),
+            self.sequencer_args.allow_empty_blocks,
         );
 
         // Create the consensus.
@@ -567,6 +568,14 @@ pub struct SequencerArgs {
         help = "L1 message inclusion mode. Use 'finalized' for finalized messages only, or 'depth:{number}' for block depth confirmation (e.g. 'depth:10')"
     )]
     pub l1_message_inclusion_mode: L1MessageInclusionMode,
+    /// Enable empty blocks.
+    #[arg(
+        long = "sequencer.allow-empty-blocks",
+        id = "sequencer_allow_empty_blocks",
+        value_name = "SEQUENCER_ALLOW_EMPTY_BLOCKS",
+        default_value_t = false
+    )]
+    pub allow_empty_blocks: bool,
 }
 
 /// The arguments for the signer.

crates/node/src/test_utils.rs

@@ -149,7 +149,11 @@ pub fn default_test_scroll_rollup_node_config() -> ScrollRollupNodeConfig {
             optimistic_sync_trigger: 100,
             chain_buffer_size: 100,
         },
-        sequencer_args: SequencerArgs { payload_building_duration: 1000, ..Default::default() },
+        sequencer_args: SequencerArgs {
+            payload_building_duration: 1000,
+            allow_empty_blocks: true,
+            ..Default::default()
+        },
         beacon_provider_args: BeaconProviderArgs {
             blob_source: BlobSource::Mock,
             ..Default::default()
@@ -185,6 +189,7 @@ pub fn default_sequencer_test_scroll_rollup_node_config() -> ScrollRollupNodeCon
             payload_building_duration: 40,
             fee_recipient: Default::default(),
             l1_message_inclusion_mode: L1MessageInclusionMode::BlockDepth(0),
+            allow_empty_blocks: true,
         },
         beacon_provider_args: BeaconProviderArgs {
             blob_source: BlobSource::Mock,

crates/node/tests/e2e.rs

@@ -64,6 +64,7 @@ async fn can_bridge_l1_messages() -> eyre::Result<()> {
             sequencer_enabled: true,
             block_time: 0,
             l1_message_inclusion_mode: L1MessageInclusionMode::BlockDepth(0),
+            allow_empty_blocks: true,
             ..SequencerArgs::default()
         },
         beacon_provider_args: BeaconProviderArgs {
@@ -159,6 +160,7 @@ async fn can_sequence_and_gossip_blocks() {
             block_time: 0,
             l1_message_inclusion_mode: L1MessageInclusionMode::BlockDepth(0),
             payload_building_duration: 1000,
+            allow_empty_blocks: true,
             ..SequencerArgs::default()
         },
         beacon_provider_args: BeaconProviderArgs {
@@ -256,6 +258,7 @@ async fn can_penalize_peer_for_invalid_block() {
             block_time: 0,
             l1_message_inclusion_mode: L1MessageInclusionMode::BlockDepth(0),
             payload_building_duration: 1000,
+            allow_empty_blocks: true,
             ..SequencerArgs::default()
         },
         beacon_provider_args: BeaconProviderArgs {

crates/node/tests/sync.rs

@@ -57,7 +57,11 @@ async fn test_should_consolidate_to_block_15k() -> eyre::Result<()> {
             initial_backoff: 100,
         },
         engine_driver_args: EngineDriverArgs { sync_at_startup: false },
-        sequencer_args: SequencerArgs { sequencer_enabled: false, ..Default::default() },
+        sequencer_args: SequencerArgs {
+            sequencer_enabled: false,
+            allow_empty_blocks: true,
+            ..Default::default()
+        },
         beacon_provider_args: BeaconProviderArgs {
             url: Some(Url::parse("https://eth-beacon-chain.drpc.org/rest/")?),
             compute_units_per_second: 100,
@@ -195,6 +199,7 @@ async fn test_should_consolidate_after_optimistic_sync() -> eyre::Result<()> {
             sequencer_enabled: true,
             block_time: 0,
             l1_message_inclusion_mode: L1MessageInclusionMode::BlockDepth(0),
+            allow_empty_blocks: true,
             ..SequencerArgs::default()
         },
         beacon_provider_args: BeaconProviderArgs {
@@ -442,6 +447,7 @@ async fn test_consolidation() -> eyre::Result<()> {
             sequencer_enabled: true,
             block_time: 0,
             l1_message_inclusion_mode: L1MessageInclusionMode::BlockDepth(0),
+            allow_empty_blocks: true,
             ..SequencerArgs::default()
         },
         beacon_provider_args: BeaconProviderArgs {
@@ -614,6 +620,7 @@ async fn test_chain_orchestrator_shallow_reorg_with_gap() -> eyre::Result<()> {
             sequencer_enabled: true,
             block_time: 0,
             l1_message_inclusion_mode: L1MessageInclusionMode::BlockDepth(0),
+            allow_empty_blocks: true,
             ..SequencerArgs::default()
         },
         beacon_provider_args: BeaconProviderArgs {

crates/sequencer/tests/e2e.rs

@@ -37,6 +37,61 @@ use tokio::{
     time::{Duration, Instant},
 };
 
+#[tokio::test]
+async fn skip_block_with_no_transactions() {
+    reth_tracing::init_test_tracing();
+
+    const BLOCK_BUILDING_DURATION: Duration = Duration::from_millis(0);
+
+    // setup a test node
+    let (mut nodes, _tasks, _wallet) = setup(1, false).await.unwrap();
+    let node = nodes.pop().unwrap();
+
+    // create a forkchoice state
+    let genesis_hash = node.inner.chain_spec().genesis_hash();
+    let fcs = ForkchoiceState::new(
+        BlockInfo { hash: genesis_hash, number: 0 },
+        Default::default(),
+        Default::default(),
+    );
+
+    // create the engine driver connected to the node
+    let auth_client = node.inner.engine_http_client();
+    let engine_client = ScrollAuthApiEngineClient::new(auth_client);
+    let mut engine_driver = EngineDriver::new(
+        Arc::new(engine_client),
+        (*SCROLL_DEV).clone(),
+        None::<ScrollRootProvider>,
+        fcs,
+        false,
+        BLOCK_BUILDING_DURATION,
+        false,
+    );
+
+    // create a test database
+    let database = Arc::new(setup_test_db().await);
+    let provider = Arc::new(DatabaseL1MessageProvider::new(database.clone(), 0));
+
+    // create a sequencer
+    let mut sequencer = Sequencer::new(
+        provider,
+        Default::default(),
+        SCROLL_GAS_LIMIT,
+        4,
+        1,
+        L1MessageInclusionMode::BlockDepth(0),
+    );
+
+    // send a new payload attributes request.
+    sequencer.build_payload_attributes();
+    let payload_attributes = sequencer.next().await.unwrap();
+    engine_driver.handle_build_new_payload(payload_attributes);
+
+    // assert that no new payload event is emitted
+    let res = tokio::time::timeout(Duration::from_secs(1), engine_driver.next()).await;
+    assert!(res.is_err(), "expected no new payload, but a block was built: {:?}", res.ok());
+}
+
 #[tokio::test]
 async fn can_build_blocks() {
     reth_tracing::init_test_tracing();
@@ -66,6 +121,7 @@ async fn can_build_blocks() {
         fcs,
         false,
         BLOCK_BUILDING_DURATION,
+        true,
     );
 
     // create a test database
@@ -192,6 +248,7 @@ async fn can_build_blocks_with_delayed_l1_messages() {
         fcs,
         false,
         BLOCK_BUILDING_DURATION,
+        true,
     );
 
     // create a test database
@@ -317,6 +374,7 @@ async fn can_build_blocks_with_finalized_l1_messages() {
         fcs,
         false,
         BLOCK_BUILDING_DURATION,
+        true,
     );
 
     // create a test database
@@ -447,6 +505,7 @@ async fn can_sequence_blocks_with_private_key_file() -> eyre::Result<()> {
             block_time: 0,
             l1_message_inclusion_mode: L1MessageInclusionMode::BlockDepth(0),
             payload_building_duration: 1000,
+            allow_empty_blocks: true,
             ..SequencerArgs::default()
         },
         beacon_provider_args: BeaconProviderArgs {
@@ -538,6 +597,7 @@ async fn can_sequence_blocks_with_hex_key_file_without_prefix() -> eyre::Result<
             block_time: 0,
             l1_message_inclusion_mode: L1MessageInclusionMode::BlockDepth(0),
             payload_building_duration: 1000,
+            allow_empty_blocks: true,
             ..SequencerArgs::default()
         },
         beacon_provider_args: BeaconProviderArgs {
@@ -658,6 +718,7 @@ async fn can_build_blocks_and_exit_at_gas_limit() {
         fcs,
         false,
         BLOCK_BUILDING_DURATION,
+        true,
     );
 
     // issue a new payload to the execution layer.
@@ -743,6 +804,7 @@ async fn can_build_blocks_and_exit_at_time_limit() {
         fcs,
         false,
         BLOCK_BUILDING_DURATION,
+        true,
     );
 
     // start timer.
@@ -809,6 +871,7 @@ async fn should_limit_l1_message_cumulative_gas() {
         fcs,
         false,
         BLOCK_BUILDING_DURATION,
+        true,
     );
 
     // create a test database

tests/launch_rollup_node_sequencer.bash

@@ -9,6 +9,7 @@ exec rollup-node node --chain dev --datadir=/l2reth --metrics=0.0.0.0:6060 --net
   --sequencer.enabled \
   --sequencer.block-time 250 \
   --sequencer.payload-building-duration 230 \
+  --sequencer.allow-empty-blocks \
   --txpool.pending-max-count=1000 \
   --builder.gaslimit=20000000 \
   --rpc.max-connections=5000 \