feat: add block payload size validation (#287)

yiweichi · web-flow · commit 357655f0646c · 2025-07-28T22:12:22.000+08:00
* feat: payloadBuilder add block da size validation

* fix: ci

* fix: test

* add SCROLL_DEFAULT_PAYLOAD_SIZE_LIMIT

* change block_da_size_limit to Option&lt;u64&gt;
diff --git a/crates/scroll/node/src/builder/payload.rs b/crates/scroll/node/src/builder/payload.rs
@@ -19,19 +19,23 @@ pub struct ScrollPayloadBuilderBuilder<Txs = ()> {
     pub best_transactions: Txs,
     /// The payload building time limit.
     pub payload_building_time_limit: Duration,
+    /// The block DA size limit.
+    pub block_da_size_limit: Option<u64>,
 }
 
 impl Default for ScrollPayloadBuilderBuilder {
     fn default() -> Self {
         Self {
             best_transactions: (),
             payload_building_time_limit: SCROLL_PAYLOAD_BUILDING_DURATION,
+            block_da_size_limit: Some(SCROLL_DEFAULT_PAYLOAD_SIZE_LIMIT),
         }
     }
 }
 
 const SCROLL_GAS_LIMIT: u64 = 20_000_000;
 const SCROLL_PAYLOAD_BUILDING_DURATION: Duration = Duration::from_secs(1);
+const SCROLL_DEFAULT_PAYLOAD_SIZE_LIMIT: u64 = 122_880;
 
 impl<Txs> ScrollPayloadBuilderBuilder<Txs> {
     /// A helper method to initialize [`reth_scroll_payload::ScrollPayloadBuilder`] with the
@@ -65,7 +69,11 @@ impl<Txs> ScrollPayloadBuilderBuilder<Txs> {
             pool,
             evm_config,
             ctx.provider().clone(),
-            ScrollBuilderConfig::new(gas_limit, self.payload_building_time_limit),
+            ScrollBuilderConfig::new(
+                gas_limit,
+                self.payload_building_time_limit,
+                self.block_da_size_limit,
+            ),
         )
         .with_transactions(self.best_transactions);
 
diff --git a/crates/scroll/payload/src/builder.rs b/crates/scroll/payload/src/builder.rs
@@ -259,7 +259,15 @@ 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, breaker)?.is_some()
+            if ctx
+                .execute_best_transactions(
+                    &mut info,
+                    &mut builder,
+                    best_txs,
+                    builder_config,
+                    breaker,
+                )?
+                .is_some()
             {
                 return Ok(BuildOutcomeKind::Cancelled);
             }
@@ -475,14 +483,16 @@ where
         mut best_txs: impl PayloadTransactions<
             Transaction: PoolTransaction<Consensus = TxTy<Evm::Primitives>>,
         >,
+        builder_config: &ScrollBuilderConfig,
         breaker: PayloadBuildingBreaker,
     ) -> Result<Option<()>, PayloadBuilderError> {
         let block_gas_limit = builder.evm_mut().block().gas_limit;
         let base_fee = builder.evm_mut().block().basefee;
 
         while let Some(tx) = best_txs.next(()) {
             let tx = tx.into_consensus();
-            if info.is_tx_over_limits(tx.inner(), block_gas_limit) {
+            if info.is_tx_over_limits(tx.inner(), block_gas_limit, builder_config.max_da_block_size)
+            {
                 // we can't fit this transaction into the block, so we need to mark it as
                 // invalid which also removes all dependent transaction from
                 // the iterator before we can continue
@@ -502,7 +512,7 @@ where
             }
 
             // check if the execution needs to be halted.
-            if breaker.should_break(info.cumulative_gas_used) {
+            if breaker.should_break(info.cumulative_gas_used, info.cumulative_da_bytes_used) {
                 tracing::trace!(target: "scroll::payload_builder", ?info, "breaking execution loop");
                 return Ok(None);
             }
@@ -582,7 +592,14 @@ impl ExecutionInfo {
         &self,
         tx: &(impl Encodable + Transaction),
         block_gas_limit: u64,
+        block_data_limit: Option<u64>,
     ) -> bool {
+        if block_data_limit
+            .is_some_and(|da_limit| self.cumulative_da_bytes_used + tx.length() as u64 > da_limit)
+        {
+            return true;
+        }
+
         self.cumulative_gas_used + tx.gas_limit() > block_gas_limit
     }
 }
diff --git a/crates/scroll/payload/src/config.rs b/crates/scroll/payload/src/config.rs
@@ -11,17 +11,22 @@ pub struct ScrollBuilderConfig {
     pub gas_limit: u64,
     /// Time limit for payload building.
     pub time_limit: Duration,
+    /// Maximum total data availability size for a block.
+    pub max_da_block_size: Option<u64>,
 }
 
+/// Minimal data bytes size per transaction.
+pub const MIN_TRANSACTION_DATA_SIZE: u64 = 115u64;
+
 impl ScrollBuilderConfig {
     /// Returns a new instance of [`ScrollBuilderConfig`].
-    pub const fn new(gas_limit: u64, time_limit: Duration) -> Self {
-        Self { gas_limit, time_limit }
+    pub const fn new(gas_limit: u64, time_limit: Duration, max_da_block_size: Option<u64>) -> Self {
+        Self { gas_limit, time_limit, max_da_block_size }
     }
 
     /// Returns the [`PayloadBuildingBreaker`] for the config.
     pub(super) fn breaker(&self) -> PayloadBuildingBreaker {
-        PayloadBuildingBreaker::new(self.time_limit, self.gas_limit)
+        PayloadBuildingBreaker::new(self.time_limit, self.gas_limit, self.max_da_block_size)
     }
 }
 
@@ -31,18 +36,39 @@ pub struct PayloadBuildingBreaker {
     start: Instant,
     time_limit: Duration,
     gas_limit: u64,
+    max_da_block_size: Option<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 }
+    fn new(time_limit: Duration, gas_limit: u64, max_da_block_size: Option<u64>) -> Self {
+        Self { start: Instant::now(), time_limit, gas_limit, max_da_block_size }
     }
 
     /// 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)
+    pub(super) fn should_break(
+        &self,
+        cumulative_gas_used: u64,
+        cumulative_da_size_used: u64,
+    ) -> bool {
+        // Check time limit
+        if self.start.elapsed() >= self.time_limit {
+            return true;
+        }
+
+        // Check gas limit
+        if cumulative_gas_used > self.gas_limit.saturating_sub(MIN_TRANSACTION_GAS) {
+            return true;
+        }
+
+        // Check data availability size limit if configured
+        if let Some(max_size) = self.max_da_block_size {
+            if cumulative_da_size_used > max_size.saturating_sub(MIN_TRANSACTION_DATA_SIZE) {
+                return true;
+            }
+        }
+
+        false
     }
 }
 
@@ -52,17 +78,48 @@ mod tests {
 
     #[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));
+        let breaker = PayloadBuildingBreaker::new(
+            Duration::from_millis(200),
+            2 * MIN_TRANSACTION_GAS,
+            Some(2 * MIN_TRANSACTION_DATA_SIZE),
+        );
+        assert!(!breaker.should_break(MIN_TRANSACTION_GAS, MIN_TRANSACTION_DATA_SIZE));
         std::thread::sleep(Duration::from_millis(201));
-        assert!(breaker.should_break(MIN_TRANSACTION_GAS));
+        assert!(breaker.should_break(MIN_TRANSACTION_GAS, MIN_TRANSACTION_DATA_SIZE));
     }
 
     #[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));
+        let breaker = PayloadBuildingBreaker::new(
+            Duration::from_secs(1),
+            2 * MIN_TRANSACTION_GAS,
+            Some(2 * MIN_TRANSACTION_DATA_SIZE),
+        );
+        assert!(!breaker.should_break(MIN_TRANSACTION_GAS, MIN_TRANSACTION_DATA_SIZE));
+        assert!(breaker.should_break(MIN_TRANSACTION_GAS + 1, MIN_TRANSACTION_DATA_SIZE));
+    }
+
+    #[test]
+    fn test_should_break_on_data_size_limit() {
+        let breaker = PayloadBuildingBreaker::new(
+            Duration::from_secs(1),
+            2 * MIN_TRANSACTION_GAS,
+            Some(2 * MIN_TRANSACTION_DATA_SIZE),
+        );
+        assert!(!breaker.should_break(MIN_TRANSACTION_GAS, MIN_TRANSACTION_DATA_SIZE));
+        assert!(breaker.should_break(MIN_TRANSACTION_GAS, MIN_TRANSACTION_DATA_SIZE + 1));
+    }
+
+    #[test]
+    fn test_should_break_with_no_da_limit() {
+        let breaker = PayloadBuildingBreaker::new(
+            Duration::from_secs(1),
+            2 * MIN_TRANSACTION_GAS,
+            None, // No DA limit
+        );
+        // Should not break on large DA size when no limit is set
+        assert!(!breaker.should_break(MIN_TRANSACTION_GAS, u64::MAX));
+        // But should still break on gas limit
+        assert!(breaker.should_break(MIN_TRANSACTION_GAS + 1, u64::MAX));
     }
 }
