feat: break payload building (#238)

* feat: break payload building

Signed-off-by: Gregory Edison <gregory.edison1993@gmail.com>

* fix: lints

Signed-off-by: Gregory Edison <gregory.edison1993@gmail.com>

* feat: breaker provider

Signed-off-by: Gregory Edison <gregory.edison1993@gmail.com>

* feat: update payload building

Signed-off-by: Gregory Edison <gregory.edison1993@gmail.com>

* fix: lint

Signed-off-by: Gregory Edison <gregory.edison1993@gmail.com>

---------

Signed-off-by: Gregory Edison <gregory.edison1993@gmail.com>

Author: greged93

Cargo.lock

@@ -13,7 +13,6 @@ workspace = true
 
 [dependencies]
 # alloy
-alloy-json-rpc.workspace = true
 alloy-provider.workspace = true
 alloy-primitives.workspace = true
 alloy-rpc-types-engine = { workspace = true, features = ["serde"] }
@@ -40,7 +39,6 @@ reqwest.workspace = true
 tower.workspace = true
 thiserror.workspace = true
 jsonrpsee.workspace = true
-jsonrpsee-types.workspace = true
 
 [dev-dependencies]
 reth-payload-builder = { workspace = true, features = ["test-utils"] }
@@ -72,7 +70,6 @@ std = [
     "reth-engine-primitives/std",
     "reth-primitives/std",
     "reth-primitives-traits/std",
-    "reth-scroll-payload/std",
     "futures-util/std",
     "reth-scroll-chainspec/std",
     "thiserror/std",

crates/scroll/alloy/provider/Cargo.toml

@@ -10,17 +10,30 @@ use reth_scroll_evm::ScrollNextBlockEnvAttributes;
 use reth_scroll_payload::{ScrollBuilderConfig, ScrollPayloadTransactions};
 use reth_scroll_primitives::{ScrollPrimitives, ScrollTransactionSigned};
 use reth_transaction_pool::{PoolTransaction, TransactionPool};
+use std::time::Duration;
 
 /// Payload builder for Scroll.
-#[derive(Debug, Clone, Default, Copy)]
-pub struct ScrollPayloadBuilder<Txs = ()> {
+#[derive(Debug, Clone, Copy)]
+pub struct ScrollPayloadBuilderBuilder<Txs = ()> {
     /// Returns the current best transactions from the mempool.
     pub best_transactions: Txs,
+    /// The payload building time limit.
+    pub payload_building_time_limit: Duration,
+}
+
+impl Default for ScrollPayloadBuilderBuilder {
+    fn default() -> Self {
+        Self {
+            best_transactions: (),
+            payload_building_time_limit: SCROLL_PAYLOAD_BUILDING_DURATION,
+        }
+    }
 }
 
 const SCROLL_GAS_LIMIT: u64 = 20_000_000;
+const SCROLL_PAYLOAD_BUILDING_DURATION: Duration = Duration::from_secs(1);
 
-impl<Txs> ScrollPayloadBuilder<Txs> {
+impl<Txs> ScrollPayloadBuilderBuilder<Txs> {
     /// A helper method to initialize [`reth_scroll_payload::ScrollPayloadBuilder`] with the
     /// given EVM config.
     pub fn build<Node, Evm, Pool>(
@@ -43,26 +56,25 @@ impl<Txs> ScrollPayloadBuilder<Txs> {
         Evm: ConfigureEvm<Primitives = PrimitivesTy<Node::Types>>,
         Txs: ScrollPayloadTransactions<Pool::Transaction>,
     {
-        let gas_limit = if let Some(gas) = ctx.payload_builder_config().gas_limit() {
-            gas
-        } else {
+        let gas_limit = ctx.payload_builder_config().gas_limit().unwrap_or_else (|| {
             tracing::warn!(target: "reth::cli", "Using {SCROLL_GAS_LIMIT} gas limit for ScrollPayloadBuilder. Configure with --builder.gaslimit");
             SCROLL_GAS_LIMIT
-        };
+        });
 
         let payload_builder = reth_scroll_payload::ScrollPayloadBuilder::new(
             pool,
             evm_config,
             ctx.provider().clone(),
-            ScrollBuilderConfig::new(gas_limit),
+            ScrollBuilderConfig::new(gas_limit, self.payload_building_time_limit),
         )
         .with_transactions(self.best_transactions);
 
         Ok(payload_builder)
     }
 }
 
-impl<Node, Pool, Txs, Evm> PayloadBuilderBuilder<Node, Pool, Evm> for ScrollPayloadBuilder<Txs>
+impl<Node, Pool, Txs, Evm> PayloadBuilderBuilder<Node, Pool, Evm>
+    for ScrollPayloadBuilderBuilder<Txs>
 where
     Node: FullNodeTypes<
         Types: NodeTypes<

crates/scroll/node/src/builder/payload.rs

@@ -5,7 +5,7 @@ pub use builder::{
     engine::{ScrollEngineValidator, ScrollEngineValidatorBuilder},
     execution::ScrollExecutorBuilder,
     network::{ScrollHeaderTransform, ScrollNetworkBuilder, ScrollNetworkPrimitives},
-    payload::ScrollPayloadBuilder,
+    payload::ScrollPayloadBuilderBuilder,
     pool::ScrollPoolBuilder,
 };
 

crates/scroll/node/src/lib.rs

@@ -2,7 +2,7 @@
 
 use crate::{
     ScrollAddOns, ScrollConsensusBuilder, ScrollExecutorBuilder, ScrollNetworkBuilder,
-    ScrollPayloadBuilder, ScrollPoolBuilder, ScrollStorage,
+    ScrollPayloadBuilderBuilder, ScrollPoolBuilder, ScrollStorage,
 };
 use reth_node_builder::{
     components::{BasicPayloadServiceBuilder, ComponentsBuilder},
@@ -23,7 +23,7 @@ impl ScrollNode {
     pub fn components<Node>() -> ComponentsBuilder<
         Node,
         ScrollPoolBuilder,
-        BasicPayloadServiceBuilder<ScrollPayloadBuilder>,
+        BasicPayloadServiceBuilder<ScrollPayloadBuilderBuilder>,
         ScrollNetworkBuilder,
         ScrollExecutorBuilder,
         ScrollConsensusBuilder,
@@ -41,7 +41,7 @@ impl ScrollNode {
             .node_types::<Node>()
             .pool(ScrollPoolBuilder::default())
             .executor(ScrollExecutorBuilder::default())
-            .payload(BasicPayloadServiceBuilder::new(ScrollPayloadBuilder::default()))
+            .payload(BasicPayloadServiceBuilder::new(ScrollPayloadBuilderBuilder::default()))
             .network(ScrollNetworkBuilder)
             .executor(ScrollExecutorBuilder)
             .consensus(ScrollConsensusBuilder)
@@ -55,7 +55,7 @@ where
     type ComponentsBuilder = ComponentsBuilder<
         N,
         ScrollPoolBuilder,
-        BasicPayloadServiceBuilder<ScrollPayloadBuilder>,
+        BasicPayloadServiceBuilder<ScrollPayloadBuilderBuilder>,
         ScrollNetworkBuilder,
         ScrollExecutorBuilder,
         ScrollConsensusBuilder,

crates/scroll/node/src/node.rs

@@ -21,7 +21,6 @@ alloy-primitives.workspace = true
 reth-scroll-evm.workspace = true
 scroll-alloy-hardforks.workspace = true
 scroll-alloy-evm.workspace = true
-scroll-alloy-consensus.workspace = true
 
 # revm
 revm.workspace = true
@@ -36,8 +35,6 @@ reth-payload-builder.workspace = true
 reth-payload-primitives.workspace = true
 reth-primitives-traits.workspace = true
 reth-revm.workspace = true
-reth-scroll-chainspec.workspace = true
-reth-scroll-forks.workspace = true
 reth-storage-api.workspace = true
 reth-transaction-pool.workspace = true
 reth-payload-util.workspace = true
@@ -52,28 +49,6 @@ thiserror.workspace = true
 tracing.workspace = true
 
 [features]
-std = [
-    "futures-util/std",
-    "reth-payload-primitives/std",
-    "reth-primitives-traits/std",
-    "reth-scroll-engine-primitives/std",
-    "alloy-consensus/std",
-    "alloy-primitives/std",
-    "alloy-rlp/std",
-    "reth-chainspec/std",
-    "reth-evm/std",
-    "reth-execution-types/std",
-    "reth-revm/std",
-    "reth-storage-api/std",
-    "revm/std",
-    "thiserror/std",
-    "tracing/std",
-    "reth-scroll-chainspec/std",
-    "reth-scroll-forks/std",
-    "scroll-alloy-consensus/std",
-    "scroll-alloy-hardforks/std",
-    "scroll-alloy-evm/std",
-]
 test-utils = [
     "dep:futures-util",
     "reth-payload-builder/test-utils",

crates/scroll/payload/Cargo.toml

@@ -1,7 +1,7 @@
 //! Scroll's payload builder implementation.
 
 use super::{PayloadBuildingBaseFeeProvider, ScrollPayloadBuilderError};
-use crate::config::ScrollBuilderConfig;
+use crate::config::{PayloadBuildingBreaker, ScrollBuilderConfig};
 
 use alloy_consensus::{Transaction, Typed2718};
 use alloy_primitives::{B256, U256};
@@ -57,7 +57,7 @@ impl<T: PoolTransaction> ScrollPayloadTransactions<T> for () {
 }
 
 /// Scroll's payload builder.
-#[derive(Debug, Clone)]
+#[derive(Clone, Debug)]
 pub struct ScrollPayloadBuilder<Pool, Client, Evm, Txs = ()> {
     /// The type responsible for creating the evm.
     pub evm_config: Evm,
@@ -134,15 +134,10 @@ where
         let state = StateProviderDatabase::new(&state_provider);
 
         if ctx.attributes().no_tx_pool {
-            builder.build(state, &state_provider, ctx, self.builder_config.clone())
+            builder.build(state, &state_provider, ctx, &self.builder_config)
         } else {
             // sequencer mode we can reuse cachedreads from previous runs
-            builder.build(
-                cached_reads.as_db_mut(state),
-                &state_provider,
-                ctx,
-                self.builder_config.clone(),
-            )
+            builder.build(cached_reads.as_db_mut(state), &state_provider, ctx, &self.builder_config)
         }
         .map(|out| out.with_cached_reads(cached_reads))
     }
@@ -222,7 +217,7 @@ impl<Txs> ScrollBuilder<'_, Txs> {
         db: impl Database<Error = ProviderError>,
         state_provider: impl StateProvider,
         ctx: ScrollPayloadBuilderCtx<EvmConfig, ChainSpec>,
-        builder_config: ScrollBuilderConfig,
+        builder_config: &ScrollBuilderConfig,
     ) -> Result<BuildOutcomeKind<ScrollBuiltPayload>, PayloadBuilderError>
     where
         EvmConfig: ConfigureEvm<
@@ -234,6 +229,7 @@ impl<Txs> ScrollBuilder<'_, Txs> {
     {
         let Self { best } = self;
         tracing::debug!(target: "payload_builder", id=%ctx.payload_id(), parent_header = ?ctx.parent().hash(), parent_number = ctx.parent().number, "building new payload");
+        let breaker = builder_config.breaker();
 
         let mut db = State::builder().with_database(db).with_bundle_update().build();
 
@@ -251,8 +247,9 @@ impl<Txs> ScrollBuilder<'_, Txs> {
         // 3. if mem pool transactions are requested we execute them
         if !ctx.attributes().no_tx_pool {
             let best_txs = best(ctx.best_transaction_attributes(builder.evm_mut().block()));
-            if ctx.execute_best_transactions(&mut info, &mut builder, best_txs)?.is_some() {
-                return Ok(BuildOutcomeKind::Cancelled)
+            if ctx.execute_best_transactions(&mut info, &mut builder, best_txs, breaker)?.is_some()
+            {
+                return Ok(BuildOutcomeKind::Cancelled);
             }
 
             // check if the new payload is even more valuable
@@ -371,7 +368,7 @@ where
     pub fn block_builder<'a, DB: Database>(
         &'a self,
         db: &'a mut State<DB>,
-        builder_config: ScrollBuilderConfig,
+        builder_config: &ScrollBuilderConfig,
     ) -> Result<impl BlockBuilder<Primitives = Evm::Primitives> + 'a, PayloadBuilderError> {
         // get the base fee for the attributes.
         let base_fee: u64 = if self.chain_spec.is_curie_active_at_block(self.parent().number + 1) {
@@ -390,7 +387,7 @@ where
                 ScrollNextBlockEnvAttributes {
                     timestamp: self.attributes().timestamp(),
                     suggested_fee_recipient: self.attributes().suggested_fee_recipient(),
-                    gas_limit: builder_config.desired_gas_limit,
+                    gas_limit: builder_config.gas_limit,
                     base_fee,
                 },
             )
@@ -459,6 +456,7 @@ where
         mut best_txs: impl PayloadTransactions<
             Transaction: PoolTransaction<Consensus = TxTy<Evm::Primitives>>,
         >,
+        breaker: PayloadBuildingBreaker,
     ) -> Result<Option<()>, PayloadBuilderError> {
         let block_gas_limit = builder.evm_mut().block().gas_limit;
         let base_fee = builder.evm_mut().block().basefee;
@@ -484,6 +482,12 @@ where
                 return Ok(Some(()))
             }
 
+            // check if the execution needs to be halted.
+            if breaker.should_break(info.cumulative_gas_used) {
+                tracing::trace!(target: "scroll::payload_builder", ?info, "breaking execution loop");
+                return Ok(None);
+            }
+
             let gas_used = match builder.execute_transaction(tx.clone()) {
                 Ok(gas_used) => gas_used,
                 Err(BlockExecutionError::Validation(BlockValidationError::InvalidTx {

crates/scroll/payload/src/builder.rs

@@ -1,15 +1,68 @@
 //! Configuration for the payload builder.
 
+use core::time::Duration;
+use reth_chainspec::MIN_TRANSACTION_GAS;
+use std::{fmt::Debug, time::Instant};
+
 /// Settings for the Scroll builder.
-#[derive(PartialEq, Eq, Clone, Debug)]
+#[derive(Debug, PartialEq, Eq, Clone)]
 pub struct ScrollBuilderConfig {
-    /// Desired gas limit.
-    pub desired_gas_limit: u64,
+    /// Gas limit.
+    pub gas_limit: u64,
+    /// Time limit for payload building.
+    pub time_limit: Duration,
 }
 
 impl ScrollBuilderConfig {
     /// Returns a new instance of [`ScrollBuilderConfig`].
-    pub const fn new(desired_gas_limit: u64) -> Self {
-        Self { desired_gas_limit }
+    pub const fn new(gas_limit: u64, time_limit: Duration) -> Self {
+        Self { gas_limit, time_limit }
+    }
+
+    /// Returns the [`PayloadBuildingBreaker`] for the config.
+    pub(super) fn breaker(&self) -> PayloadBuildingBreaker {
+        PayloadBuildingBreaker::new(self.time_limit, self.gas_limit)
+    }
+}
+
+/// Used in the [`super::ScrollPayloadBuilder`] to exit the transactions execution loop early.
+#[derive(Debug, Clone)]
+pub struct PayloadBuildingBreaker {
+    start: Instant,
+    time_limit: Duration,
+    gas_limit: u64,
+}
+
+impl PayloadBuildingBreaker {
+    /// Returns a new instance of the [`PayloadBuildingBreaker`].
+    fn new(time_limit: Duration, gas_limit: u64) -> Self {
+        Self { start: Instant::now(), time_limit, gas_limit }
+    }
+
+    /// Returns whether the payload building should stop.
+    pub(super) fn should_break(&self, cumulative_gas_used: u64) -> bool {
+        self.start.elapsed() >= self.time_limit ||
+            cumulative_gas_used > self.gas_limit.saturating_sub(MIN_TRANSACTION_GAS)
+    }
+}
+
+#[cfg(test)]
+mod tests {
+    use super::*;
+
+    #[test]
+    fn test_should_break_on_time_limit() {
+        let breaker =
+            PayloadBuildingBreaker::new(Duration::from_millis(200), 2 * MIN_TRANSACTION_GAS);
+        assert!(!breaker.should_break(MIN_TRANSACTION_GAS));
+        std::thread::sleep(Duration::from_millis(201));
+        assert!(breaker.should_break(MIN_TRANSACTION_GAS));
+    }
+
+    #[test]
+    fn test_should_break_on_gas_limit() {
+        let breaker = PayloadBuildingBreaker::new(Duration::from_secs(1), 2 * MIN_TRANSACTION_GAS);
+        assert!(!breaker.should_break(MIN_TRANSACTION_GAS));
+        assert!(breaker.should_break(MIN_TRANSACTION_GAS + 1));
     }
 }