diff --git a/crates/scroll/payload/src/builder.rs b/crates/scroll/payload/src/builder.rs index 1022ab80aad..9497bbec33e 100644 --- a/crates/scroll/payload/src/builder.rs +++ b/crates/scroll/payload/src/builder.rs @@ -1,9 +1,9 @@ //! Scroll's payload builder implementation. use super::ScrollPayloadBuilderError; -use crate::config::{PayloadBuildingBreaker, ScrollBuilderConfig}; +use crate::config::{calculate_block_gas_limit, PayloadBuildingBreaker, ScrollBuilderConfig}; -use alloy_consensus::{Transaction, Typed2718}; +use alloy_consensus::{BlockHeader, Transaction, Typed2718}; use alloy_primitives::U256; use alloy_rlp::Encodable; use core::fmt::Debug; @@ -239,12 +239,18 @@ impl 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(); let mut builder = ctx.block_builder(&mut db, builder_config)?; + // Create the breaker with the actual block gas limit (after clamping based on parent). + // The configured gas_limit may differ from the actual block gas limit because it gets + // clamped to not exceed parent_gas_limit ± parent_gas_limit/1024. Using the actual + // block gas limit ensures the breaker exits the transaction loop at the right time, + // avoiding wasted work trying to add transactions that won't fit. + let block_gas_limit = builder.evm().block().gas_limit(); + let breaker = builder_config.breaker_with_gas_limit(block_gas_limit); + // 1. apply pre-execution changes builder.apply_pre_execution_changes().map_err(|err| { tracing::warn!(target: "payload_builder", %err, "failed to apply pre-execution changes"); @@ -394,6 +400,9 @@ where } /// Prepares a [`BlockBuilder`] for the next block. + /// + /// The gas limit is clamped based on the parent block's gas limit to ensure it does not + /// increase or decrease by more than `parent_gas_limit / 1024` per block. pub fn block_builder<'a, DB: Database>( &'a self, db: &'a mut State, @@ -405,6 +414,17 @@ where .next_block_base_fee(db, self.parent().header(), self.attributes().timestamp()) .map_err(|err| PayloadBuilderError::Other(Box::new(err)))?; + // Get the desired gas limit from attributes or config + let desired_gas_limit = self + .attributes() + .gas_limit + .unwrap_or_else(|| builder_config.gas_limit.unwrap_or_default()); + + // Clamp the gas limit based on parent's gas limit. + // The gas limit can only change by at most `parent_gas_limit / 1024` per block. + let parent_gas_limit = self.parent().gas_limit(); + let gas_limit = calculate_block_gas_limit(parent_gas_limit, desired_gas_limit); + self.evm_config .builder_for_next_block( db, @@ -412,10 +432,7 @@ where ScrollNextBlockEnvAttributes { timestamp: self.attributes().timestamp(), suggested_fee_recipient: self.attributes().suggested_fee_recipient(), - gas_limit: self - .attributes() - .gas_limit - .unwrap_or_else(|| builder_config.gas_limit.unwrap_or_default()), + gas_limit, base_fee, }, ) diff --git a/crates/scroll/payload/src/config.rs b/crates/scroll/payload/src/config.rs index 63993b60b9b..87c43870b3a 100644 --- a/crates/scroll/payload/src/config.rs +++ b/crates/scroll/payload/src/config.rs @@ -2,6 +2,7 @@ use core::time::Duration; use reth_chainspec::MIN_TRANSACTION_GAS; +use reth_primitives_traits::constants::GAS_LIMIT_BOUND_DIVISOR; use std::{fmt::Debug, time::Instant}; /// Settings for the Scroll builder. @@ -28,9 +29,11 @@ impl ScrollBuilderConfig { 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, self.max_da_block_size) + /// Returns the [`PayloadBuildingBreaker`] for the config with the actual gas limit used. + /// + /// The `actual_gas_limit` should be the gas limit after clamping based on parent's gas limit. + pub(super) fn breaker_with_gas_limit(&self, actual_gas_limit: u64) -> PayloadBuildingBreaker { + PayloadBuildingBreaker::new(self.time_limit, Some(actual_gas_limit), self.max_da_block_size) } } @@ -78,6 +81,17 @@ impl PayloadBuildingBreaker { } } +/// Calculate the gas limit for the next block based on parent and desired gas limits. +/// +/// The gas limit can only change by at most `parent_gas_limit / 1024` per block. +/// Ref: +pub fn calculate_block_gas_limit(parent_gas_limit: u64, desired_gas_limit: u64) -> u64 { + let delta = (parent_gas_limit / GAS_LIMIT_BOUND_DIVISOR).saturating_sub(1); + let min_gas_limit = parent_gas_limit.saturating_sub(delta); + let max_gas_limit = parent_gas_limit.saturating_add(delta); + desired_gas_limit.clamp(min_gas_limit, max_gas_limit) +} + #[cfg(test)] mod tests { use super::*; @@ -128,4 +142,72 @@ mod tests { // But should still break on gas limit assert!(breaker.should_break(MIN_TRANSACTION_GAS + 1, u64::MAX)); } + + #[test] + fn test_calculate_block_gas_limit_within_bounds() { + let parent_gas_limit = GAS_LIMIT_BOUND_DIVISOR * 10; // 10240 + let delta = parent_gas_limit / GAS_LIMIT_BOUND_DIVISOR - 1; // 9 + + // Desired equals parent - should return parent + assert_eq!(calculate_block_gas_limit(parent_gas_limit, parent_gas_limit), parent_gas_limit); + + // Small increase within bounds + assert_eq!( + calculate_block_gas_limit(parent_gas_limit, parent_gas_limit + 5), + parent_gas_limit + 5 + ); + + // Small decrease within bounds + assert_eq!( + calculate_block_gas_limit(parent_gas_limit, parent_gas_limit - 5), + parent_gas_limit - 5 + ); + + // Exactly at max allowed increase + assert_eq!( + calculate_block_gas_limit(parent_gas_limit, parent_gas_limit + delta), + parent_gas_limit + delta + ); + + // Exactly at max allowed decrease + assert_eq!( + calculate_block_gas_limit(parent_gas_limit, parent_gas_limit - delta), + parent_gas_limit - delta + ); + } + + #[test] + fn test_calculate_block_gas_limit_clamped_increase() { + let parent_gas_limit = GAS_LIMIT_BOUND_DIVISOR * 10; // 10240 + let delta = parent_gas_limit / GAS_LIMIT_BOUND_DIVISOR - 1; // 9 + let max_gas_limit = parent_gas_limit + delta; + + // Desired exceeds max - should clamp to max + assert_eq!( + calculate_block_gas_limit(parent_gas_limit, parent_gas_limit + delta + 1), + max_gas_limit + ); + + // Large increase - should clamp + assert_eq!( + calculate_block_gas_limit(parent_gas_limit, parent_gas_limit * 2), + max_gas_limit + ); + } + + #[test] + fn test_calculate_block_gas_limit_clamped_decrease() { + let parent_gas_limit = GAS_LIMIT_BOUND_DIVISOR * 10; // 10240 + let delta = parent_gas_limit / GAS_LIMIT_BOUND_DIVISOR - 1; // 9 + let min_gas_limit = parent_gas_limit - delta; + + // Desired below min - should clamp to min + assert_eq!( + calculate_block_gas_limit(parent_gas_limit, parent_gas_limit - delta - 1), + min_gas_limit + ); + + // Much lower than allowed - should clamp + assert_eq!(calculate_block_gas_limit(parent_gas_limit, 0), min_gas_limit); + } }