diff --git a/crates/engine-api/src/builder.rs b/crates/engine-api/src/builder.rs index 26aa0e3b..90b06bd0 100644 --- a/crates/engine-api/src/builder.rs +++ b/crates/engine-api/src/builder.rs @@ -54,13 +54,6 @@ pub struct RealMorphL2EngineApi { metrics: MorphEngineApiMetrics, } -#[derive(Debug, Clone, Copy, PartialEq)] -struct CanonicalHead { - number: u64, - hash: B256, - timestamp: u64, -} - /// Tracks the L1-derived finalized block hash from `set_block_tags` so that FCU /// calls can forward it to the engine tree. /// @@ -89,6 +82,35 @@ impl BlockTagTracker { } } +#[derive(Debug, Clone, Copy, PartialEq)] +struct CanonicalHead { + number: u64, + hash: B256, + timestamp: u64, +} + +/// Whether a payload build may draw transactions from the local pool. +/// +/// A named policy rather than a bare `bool`, because the build entry points already take +/// several positional `Option` arguments and picking the wrong one here silently forks the +/// chain instead of failing. +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +enum TxPoolPolicy { + /// Sequencer assembly: execute the supplied L1 messages, then pack the best pool + /// transactions. + Include, + /// Derivation import: execute exactly the supplied transactions and nothing else, so the + /// rebuilt block is byte-identical to the one the sequencer committed to L1. + Exclude, +} + +impl TxPoolPolicy { + /// Maps to the `no_tx_pool` flag carried by [`morph_payload_types::MorphPayloadAttributes`]. + const fn no_tx_pool(self) -> bool { + matches!(self, Self::Exclude) + } +} + impl RealMorphL2EngineApi { /// Creates a new [`RealMorphL2EngineApi`]. pub fn new( @@ -152,7 +174,9 @@ where params: AssembleL2BlockParams, ) -> EngineApiResult { let started = Instant::now(); - let result = self.build_l2_payload(params, None, None, None).await; + let result = self + .build_l2_payload(params, None, None, None, TxPoolPolicy::Include) + .await; self.metrics .assemble_l2_block_duration_seconds .record(started.elapsed()); @@ -198,7 +222,13 @@ where }; let result = self - .build_l2_payload(assemble_params, None, None, Some(parent_hash)) + .build_l2_payload( + assemble_params, + None, + None, + Some(parent_hash), + TxPoolPolicy::Include, + ) .await; self.metrics .assemble_l2_block_duration_seconds @@ -491,7 +521,12 @@ where // resolved parent, so callers that pin a non-head parent reorg correctly. let parent_override = data.parent_hash; - // Assemble the block from SafeL2Data inputs. + // Reconstruct the block from SafeL2Data inputs. `SafeL2Data.transactions` is the + // complete ordered transaction list of the L1-committed block, so the build must be + // deterministic: TxPoolPolicy::Exclude keeps the local pool out. Without it a + // follower absorbs gossiped transactions that the sequencer committed to later + // blocks, forking off the sequencer chain (issue #179). SafeL2Data carries no + // expected block hash, so nothing downstream would catch such a divergence. let assemble_params = AssembleL2BlockParams { number: data.number, // Move transactions out of data to avoid cloning the full Vec. @@ -505,6 +540,7 @@ where Some(data.gas_limit), data.base_fee_per_gas, parent_override, + TxPoolPolicy::Exclude, ) .await .inspect_err(|_| { @@ -643,6 +679,7 @@ impl RealMorphL2EngineApi { gas_limit_override: Option, base_fee_override: Option, parent_override: Option, + tx_pool_policy: TxPoolPolicy, ) -> EngineApiResult where Provider: HeaderProvider
@@ -728,6 +765,7 @@ impl RealMorphL2EngineApi { target_gas_limit: None, }, transactions: Some(params.transactions), + no_tx_pool: tx_pool_policy.no_tx_pool(), gas_limit: gas_limit_override, base_fee_per_gas: base_fee_override, }; diff --git a/crates/evm/src/block/mod.rs b/crates/evm/src/block/mod.rs index 42ce3269..7e66deb5 100644 --- a/crates/evm/src/block/mod.rs +++ b/crates/evm/src/block/mod.rs @@ -92,6 +92,11 @@ pub struct MorphBlockExecutor { receipts: Vec, /// Total gas used by executed transactions gas_used: u64, + /// Gas unavailable to later transactions in this block. + /// + /// Unlike receipt gas, L1 messages reserve their full gas limit because Morph geth does not + /// return their unused gas to the block gas pool. + gas_pool_used: u64, /// Cached hardfork for this block (constant across all transactions). /// Set in `apply_pre_execution_changes`, reused in `commit_transaction`. hardfork: MorphHardfork, @@ -119,6 +124,7 @@ where receipt_builder, receipts: Vec::new(), gas_used: 0, + gas_pool_used: 0, hardfork: MorphHardfork::default(), } } @@ -227,8 +233,14 @@ where ) -> Result { let (tx_env, recovered) = tx.into_parts(); - // Validate gas limit fits in remaining block gas. - let block_available_gas = self.evm.block().gas_limit() - self.gas_used; + // Validate gas limit against the geth-compatible block gas pool. Receipt gas cannot be + // used here: L1 messages report their actual execution cost in receipts but reserve their + // full gas limit for block packing. + let block_available_gas = self + .evm + .block() + .gas_limit() + .saturating_sub(self.gas_pool_used); if recovered.tx().gas_limit() > block_available_gas { return Err(BlockExecutionError::msg(format!( "transaction gas limit {} exceeds block available gas {}", @@ -275,6 +287,15 @@ where let gas_used = result.gas().tx_gas_used(); self.gas_used += gas_used; + // Morph geth's L1-message path deducts the full transaction gas limit from GasPool and + // deliberately skips the refund path. Regular transactions return unused gas, so their + // net gas-pool charge is the actual gas used. + self.gas_pool_used += if recovered.tx().is_l1_msg() { + recovered.tx().gas_limit() + } else { + gas_used + }; + // Get MorphTx-specific fields using the recovered transaction. Errors here // are tracing-only — the trait API no longer permits us to surface errors // from `commit_transaction`. diff --git a/crates/node/src/node.rs b/crates/node/src/node.rs index b73f5073..87fee571 100644 --- a/crates/node/src/node.rs +++ b/crates/node/src/node.rs @@ -209,6 +209,8 @@ impl PayloadAttributesBuilder }, // No L1 transactions in local mining mode transactions: None, + // Local mining exists to produce blocks from the pool. + no_tx_pool: false, gas_limit: None, base_fee_per_gas: None, } diff --git a/crates/node/src/test_utils.rs b/crates/node/src/test_utils.rs index dcb2af19..10f94cfb 100644 --- a/crates/node/src/test_utils.rs +++ b/crates/node/src/test_utils.rs @@ -470,6 +470,7 @@ pub async fn advance_empty_block(node: &mut MorphTestNode) -> eyre::Result morph_payload_types::MorphPay target_gas_limit: None, }, transactions: None, + no_tx_pool: false, gas_limit: None, base_fee_per_gas: None, } diff --git a/crates/node/src/validator.rs b/crates/node/src/validator.rs index ab88e2a9..53c378f0 100644 --- a/crates/node/src/validator.rs +++ b/crates/node/src/validator.rs @@ -1029,6 +1029,7 @@ mod tests { target_gas_limit: None, }, transactions: None, + no_tx_pool: false, gas_limit: None, base_fee_per_gas: None, }; @@ -1050,6 +1051,7 @@ mod tests { target_gas_limit: None, }, transactions: None, + no_tx_pool: false, gas_limit: None, base_fee_per_gas: None, }; @@ -1071,6 +1073,7 @@ mod tests { target_gas_limit: None, }, transactions: None, + no_tx_pool: false, gas_limit: None, base_fee_per_gas: None, }; diff --git a/crates/node/tests/it/engine.rs b/crates/node/tests/it/engine.rs index 90be9bb0..aa607b82 100644 --- a/crates/node/tests/it/engine.rs +++ b/crates/node/tests/it/engine.rs @@ -4,11 +4,15 @@ //! enforcement — in particular the state-root validation gating introduced //! by the Jade hardfork. +use alloy_consensus::transaction::TxHashRef; use alloy_consensus::{BlockHeader, Sealable}; +use alloy_eips::eip2718::Decodable2718; use alloy_primitives::{Address, B256}; use alloy_rpc_types_engine::PayloadAttributes; use jsonrpsee::core::client::ClientT; -use morph_node::test_utils::{HardforkSchedule, TestNodeBuilder}; +use morph_node::test_utils::{ + HardforkSchedule, L1MessageBuilder, MorphTxBuilder, TEST_TOKEN_ID, TestNodeBuilder, +}; use morph_payload_types::{ AssembleL2BlockParams, ExecutableL2Data, GenericResponse, MorphPayloadAttributes, MorphPayloadTypes, SafeL2Data, @@ -19,7 +23,11 @@ use reth_payload_builder::BuildNewPayload; use reth_payload_primitives::BuiltPayload; use reth_provider::{BlockIdReader, BlockReaderIdExt}; -use super::helpers::{build_block_no_submit, craft_and_try_import_block}; +use super::helpers::{ + assemble_l2_block, build_block_no_submit, canonical_block, canonical_snapshot, + craft_and_try_import_block, head_timestamp, import_l2_block, transaction_hashes, + wait_until_pooled, +}; /// Pre-Jade: a block with a wrong state root is still accepted. /// @@ -618,6 +626,7 @@ async fn payload_builder_hash_matches_block_hash_with_nonzero_prev_randao() -> e target_gas_limit: None, }, transactions: Some(vec![]), + no_tx_pool: false, gas_limit: None, base_fee_per_gas: None, }; @@ -666,3 +675,367 @@ async fn payload_builder_hash_matches_block_hash_with_nonzero_prev_randao() -> e Ok(()) } + +/// Recast a sequencer-assembled block as the `SafeL2Data` that derivation would reconstruct +/// from an L1 batch: the same inputs, minus every execution output. +/// +/// `parent_hash` is pinned so the follower reconstructs the block on the intended parent +/// rather than on whatever its current head happens to be. +fn safe_data_from(block: &ExecutableL2Data) -> SafeL2Data { + SafeL2Data { + number: block.number, + gas_limit: block.gas_limit, + base_fee_per_gas: block.base_fee_per_gas, + timestamp: block.timestamp, + transactions: block.transactions.clone(), + parent_hash: Some(block.parent_hash), + } +} + +/// Hash of a raw encoded transaction. +fn tx_hash_of(raw: &alloy_primitives::Bytes) -> B256 { + let mut raw = raw.as_ref(); + *morph_primitives::MorphTxEnvelope::decode_2718(&mut raw) + .expect("raw transaction is decodable") + .tx_hash() +} + +/// Transaction hashes of an assembled block, in block order. +fn transaction_hashes_of(block: &ExecutableL2Data) -> Vec { + block.transactions.iter().map(tx_hash_of).collect() +} + +/// Regression test for #179: `engine_newSafeL2Block` must never pull from the local txpool. +/// +/// A derivation follower's pool holds gossiped transactions that the sequencer committed to +/// *later* blocks. Before the fix, the safe path reused the sequencer assembly builder, which +/// unconditionally appended the best pool transactions, so an empty committed block absorbed +/// future transactions. The follower forked off the sequencer at that height and then stalled +/// when those transactions were supplied again at their real heights ("nonce too low"). +/// +/// `SafeL2Data` carries no expected block hash, so nothing downstream catches the divergence — +/// which is why the original failure surfaced 114 blocks past its cause. +/// +/// Behavior contract: +/// - fault: the follower derives block 1 (empty) while its pool already holds the transactions +/// the sequencer committed to blocks 2 and 3; +/// - evidence: derived block 1 stays empty, blocks 2 and 3 accept their committed transactions +/// without a nonce error, and all three follower block hashes equal the sequencer's. +#[tokio::test(flavor = "multi_thread")] +async fn new_safe_l2_block_ignores_txpool() -> eyre::Result<()> { + reth_tracing::init_test_tracing(); + + let (mut nodes, wallet) = TestNodeBuilder::new().with_num_nodes(2).build().await?; + let follower = nodes.pop().expect("two nodes requested"); + let sequencer = nodes.pop().expect("two nodes requested"); + + let genesis_timestamp = head_timestamp(&sequencer)?; + + // Sequencer block 1: empty. This is the block the follower will later derive while its + // pool is already primed with the transactions of blocks 2 and 3. + let mut params = AssembleL2BlockParams::empty(1); + params.timestamp = Some(genesis_timestamp + 1); + let block1 = assemble_l2_block(&sequencer, params).await?; + assert!( + block1.transactions.is_empty(), + "sequencer block 1 must be empty for this scenario" + ); + import_l2_block(&sequencer, block1.clone()).await?; + + // Two transactions from the same sender, committed one per block. Sequential nonces are + // what make a premature inclusion fatal later: replaying nonce 0 at its real height fails + // with "nonce too low" once the follower has already consumed it. + let tx_nonce0 = MorphTxBuilder::new(wallet.chain_id, wallet.inner.clone(), 0) + .with_v1_token_fee(TEST_TOKEN_ID) + .build_signed()?; + let tx_nonce1 = MorphTxBuilder::new(wallet.chain_id, wallet.inner.clone(), 1) + .with_v1_token_fee(TEST_TOKEN_ID) + .build_signed()?; + let tx_nonce0_hash = tx_hash_of(&tx_nonce0); + let tx_nonce1_hash = tx_hash_of(&tx_nonce1); + + // Sequencer block 2 commits nonce 0. Each transaction is injected only when its own block + // is about to be built, so block 2 cannot take both. + sequencer.rpc.inject_tx(tx_nonce0.clone()).await?; + let mut params = AssembleL2BlockParams::empty(2); + params.timestamp = Some(genesis_timestamp + 2); + let block2 = assemble_l2_block(&sequencer, params).await?; + assert_eq!( + transaction_hashes_of(&block2), + vec![tx_nonce0_hash], + "sequencer block 2 must contain exactly nonce 0" + ); + import_l2_block(&sequencer, block2.clone()).await?; + + // Sequencer block 3 commits nonce 1. + sequencer.rpc.inject_tx(tx_nonce1.clone()).await?; + let mut params = AssembleL2BlockParams::empty(3); + params.timestamp = Some(genesis_timestamp + 3); + let block3 = assemble_l2_block(&sequencer, params).await?; + assert_eq!( + transaction_hashes_of(&block3), + vec![tx_nonce1_hash], + "sequencer block 3 must contain exactly nonce 1" + ); + import_l2_block(&sequencer, block3.clone()).await?; + + // Preload the follower's pool with both committed transactions, standing in for the gossip + // a real follower receives from the sequencer. Submitting directly keeps the precondition + // deterministic instead of depending on P2P timing; gossip may already have delivered + // them, so a duplicate rejection here is not a failure — `wait_until_pooled` is the + // assertion that matters. The follower never imported blocks 1-3, so nothing has evicted + // either transaction. This is the precondition that made the original bug fire. + let _ = follower.rpc.inject_tx(tx_nonce0).await; + let _ = follower.rpc.inject_tx(tx_nonce1).await; + wait_until_pooled(&follower, tx_nonce0_hash).await?; + wait_until_pooled(&follower, tx_nonce1_hash).await?; + + // Derive block 1 on the follower. The pool holds nonce 0 and nonce 1; a builder that reads + // the pool would pack both here. + let derived1: MorphHeader = follower + .auth_server_handle() + .http_client() + .request("engine_newSafeL2Block", (safe_data_from(&block1),)) + .await?; + + assert!( + canonical_block(&follower, 1)?.body.transactions.is_empty(), + "derived block 1 must stay empty: the follower pool must not leak into a committed block" + ); + assert_eq!( + derived1.hash_slow(), + block1.hash, + "derived block 1 hash must match the sequencer's" + ); + + // Both transactions must still be replayable at their committed heights. Before the fix + // this failed here with "nonce 0 too low", because block 1 had already consumed them. + let derived2: MorphHeader = follower + .auth_server_handle() + .http_client() + .request("engine_newSafeL2Block", (safe_data_from(&block2),)) + .await?; + assert_eq!( + derived2.hash_slow(), + block2.hash, + "derived block 2 hash must match the sequencer's" + ); + assert_eq!( + transaction_hashes(&canonical_block(&follower, 2)?), + vec![tx_nonce0_hash], + "derived block 2 must contain exactly the committed nonce 0" + ); + + let derived3: MorphHeader = follower + .auth_server_handle() + .http_client() + .request("engine_newSafeL2Block", (safe_data_from(&block3),)) + .await?; + assert_eq!( + derived3.hash_slow(), + block3.hash, + "derived block 3 hash must match the sequencer's" + ); + assert_eq!( + transaction_hashes(&canonical_block(&follower, 3)?), + vec![tx_nonce1_hash], + "derived block 3 must contain exactly the committed nonce 1" + ); + + Ok(()) +} + +/// A derived block reproduces a mixed L1-message + L2 transaction list exactly. +/// +/// This pins the `SafeL2Data.transactions` contract: it is the *complete ordered* transaction +/// list of the committed block, not an L1-message-only list. Before the txpool fix the two +/// readings were conflated — the field was documented as L1-messages-only while the pool +/// silently supplied the L2 half — so a mixed list was never exercised end to end. +/// +/// It also guards the L1-before-L2 ordering check in `execute_supplied_transactions` against +/// inverting: +/// a check that rejected this legal ordering would break every derived block that carries both +/// kinds, which is the common case on a chain with bridge traffic. +#[tokio::test(flavor = "multi_thread")] +async fn new_safe_l2_block_executes_l1_messages_then_l2_transactions() -> eyre::Result<()> { + reth_tracing::init_test_tracing(); + + let (mut nodes, wallet) = TestNodeBuilder::new().build().await?; + let node = nodes.pop().unwrap(); + + let genesis_timestamp = head_timestamp(&node)?; + + let mut params = AssembleL2BlockParams::empty(1); + params.timestamp = Some(genesis_timestamp + 1); + let block1 = assemble_l2_block(&node, params).await?; + let gas_limit = block1.gas_limit; + import_l2_block(&node, block1).await?; + + // Block 1 was empty, so the next expected queue index is 0. + let l1_msg = L1MessageBuilder::new(0).build_encoded(); + let l2_tx = MorphTxBuilder::new(wallet.chain_id, wallet.inner.clone(), 0) + .with_v1_token_fee(TEST_TOKEN_ID) + .build_signed()?; + let l1_msg_hash = tx_hash_of(&l1_msg); + let l2_tx_hash = tx_hash_of(&l2_tx); + + let safe = SafeL2Data { + number: 2, + gas_limit, + base_fee_per_gas: None, + timestamp: genesis_timestamp + 2, + transactions: vec![l1_msg, l2_tx], + parent_hash: None, + }; + + let header: MorphHeader = node + .auth_server_handle() + .http_client() + .request("engine_newSafeL2Block", (safe,)) + .await?; + assert_eq!(header.number(), 2); + + assert_eq!( + transaction_hashes(&canonical_block(&node, 2)?), + vec![l1_msg_hash, l2_tx_hash], + "the derived block must contain exactly the supplied transactions, in the supplied order" + ); + assert_eq!( + header.next_l1_msg_index, 1, + "the L1 message must advance next_l1_msg_index" + ); + + Ok(()) +} + +/// L1 messages reserve their full gas limit from the block gas pool. +/// +/// Morph geth deducts an L1 message's full gas limit and deliberately does not return unused gas +/// to `GasPool`, even though the receipt and block header report only the gas actually consumed. +/// Accounting only the actual gas here would let a follower accept an L2 transaction that geth +/// rejects with `ErrGasLimitReached`. +#[tokio::test(flavor = "multi_thread")] +async fn new_safe_l2_block_applies_l1_message_gas_pool_semantics() -> eyre::Result<()> { + reth_tracing::init_test_tracing(); + + let (mut nodes, wallet) = TestNodeBuilder::new().build().await?; + let node = nodes.pop().unwrap(); + + let genesis_timestamp = head_timestamp(&node)?; + let mut params = AssembleL2BlockParams::empty(1); + params.timestamp = Some(genesis_timestamp + 1); + let block1 = assemble_l2_block(&node, params).await?; + let block_gas_limit = block1.gas_limit; + import_l2_block(&node, block1).await?; + let head_before = canonical_snapshot(&node)?; + + // The simple L1 call consumes far less than its limit, but geth leaves only 1,000,000 gas + // available to later transactions. The L2 transaction therefore cannot fit despite the sum + // of both transactions' actual execution gas being well below the block limit. + let l1_msg = L1MessageBuilder::new(0) + .with_gas_limit( + block_gas_limit + .checked_sub(1_000_000) + .expect("test block gas limit exceeds reserved remainder"), + ) + .build_encoded(); + let l2_tx = MorphTxBuilder::new(wallet.chain_id, wallet.inner.clone(), 0) + .with_v1_token_fee(TEST_TOKEN_ID) + .with_gas_limit(2_000_000) + .build_signed()?; + + let safe = SafeL2Data { + number: 2, + gas_limit: block_gas_limit, + base_fee_per_gas: None, + timestamp: genesis_timestamp + 2, + transactions: vec![l1_msg, l2_tx], + parent_hash: None, + }; + + let result: Result = node + .auth_server_handle() + .http_client() + .request("engine_newSafeL2Block", (safe,)) + .await; + assert!( + result.is_err(), + "the L2 transaction must not use gas reserved by the preceding L1 message" + ); + assert_eq!( + canonical_snapshot(&node)?, + head_before, + "a gas-pool violation must leave the canonical chain untouched" + ); + + Ok(()) +} + +/// A derived block whose transactions exceed its gas limit must be rejected, not truncated. +/// +/// Sequencer assembly stops packing when the next transaction does not fit and seals the block +/// with what already fits — correct when the builder chooses the contents. Derivation does not +/// choose: the transaction list is fixed by what was committed to L1. Truncating it there would +/// seal a *different* block, and since `SafeL2Data` carries no expected hash, nothing +/// downstream would notice. go-ethereum reaches the same outcome structurally, because +/// `NewSafeL2Block` executes the block via `BlockChain.ProcessBlock`, whose gas pool returns +/// `ErrGasLimitReached` and fails the whole block. +/// +/// Behavior contract: +/// - fault: a `SafeL2Data` whose second transaction cannot fit the block gas limit; +/// - evidence: the call fails and the canonical head does not advance. +#[tokio::test(flavor = "multi_thread")] +async fn new_safe_l2_block_rejects_transactions_over_gas_limit() -> eyre::Result<()> { + reth_tracing::init_test_tracing(); + + let (mut nodes, wallet) = TestNodeBuilder::new().build().await?; + let node = nodes.pop().unwrap(); + + let genesis_timestamp = head_timestamp(&node)?; + + let mut params = AssembleL2BlockParams::empty(1); + params.timestamp = Some(genesis_timestamp + 1); + let block1 = assemble_l2_block(&node, params).await?; + let block_gas_limit = block1.gas_limit; + import_l2_block(&node, block1).await?; + let head_before = canonical_snapshot(&node)?; + + // First transaction fits; the second one alone exceeds the whole block gas limit, so the + // pair cannot be executed under it. + let fits = MorphTxBuilder::new(wallet.chain_id, wallet.inner.clone(), 0) + .with_v1_token_fee(TEST_TOKEN_ID) + .with_gas_limit(100_000) + .build_signed()?; + let overflows = MorphTxBuilder::new(wallet.chain_id, wallet.inner.clone(), 1) + .with_v1_token_fee(TEST_TOKEN_ID) + .with_gas_limit(block_gas_limit) + .build_signed()?; + + let safe = SafeL2Data { + number: 2, + gas_limit: block_gas_limit, + base_fee_per_gas: None, + timestamp: genesis_timestamp + 2, + transactions: vec![fits, overflows], + parent_hash: None, + }; + + let result: Result = node + .auth_server_handle() + .http_client() + .request("engine_newSafeL2Block", (safe,)) + .await; + assert!( + result.is_err(), + "a derived block that cannot fit its committed transactions must be rejected, \ + not silently truncated" + ); + + assert_eq!( + canonical_snapshot(&node)?, + head_before, + "a rejected safe block must leave the canonical chain untouched" + ); + + Ok(()) +} diff --git a/crates/node/tests/it/helpers.rs b/crates/node/tests/it/helpers.rs index a0b76bd4..c4d3f4ff 100644 --- a/crates/node/tests/it/helpers.rs +++ b/crates/node/tests/it/helpers.rs @@ -280,6 +280,7 @@ pub(crate) async fn advance_block_with_l1_messages( target_gas_limit: None, }, transactions: Some(l1_messages), + no_tx_pool: false, gas_limit: None, base_fee_per_gas: None, }; @@ -355,6 +356,7 @@ pub(crate) async fn build_block_no_submit( target_gas_limit: None, }, transactions: Some(l1_messages), + no_tx_pool: false, gas_limit: None, base_fee_per_gas: None, }; @@ -481,6 +483,7 @@ pub(crate) async fn expect_payload_build_failure( target_gas_limit: None, }, transactions: Some(l1_messages), + no_tx_pool: false, gas_limit: None, base_fee_per_gas: None, }; diff --git a/crates/node/tests/it/mixed_block_pressure.rs b/crates/node/tests/it/mixed_block_pressure.rs index 583db996..e1de9cee 100644 --- a/crates/node/tests/it/mixed_block_pressure.rs +++ b/crates/node/tests/it/mixed_block_pressure.rs @@ -1,6 +1,6 @@ //! Mixed L1/L2 block construction under independent resource limits. -use alloy_consensus::{BlockHeader, transaction::TxHashRef}; +use alloy_consensus::{BlockHeader, Transaction, TxReceipt, transaction::TxHashRef}; use alloy_primitives::B256; use alloy_primitives::{Bytes, U256}; use alloy_rlp::Encodable; @@ -77,6 +77,39 @@ fn l2_da_bytes(block: &Block) -> u64 { .sum() } +/// Gas removed from the block gas pool by the canonical transactions. +/// +/// L1 messages reserve their full gas limit, while regular transactions return unused gas and +/// therefore consume only the delta reported by their cumulative receipt gas. +fn block_gas_pool_used(node: &MorphTestNode, block: &Block) -> eyre::Result { + let mut previous_cumulative_gas = 0; + let mut gas_pool_used = 0u64; + + for tx in &block.body.transactions { + let receipt = node + .inner + .provider + .receipt_by_hash(*tx.tx_hash())? + .ok_or_else(|| eyre::eyre!("missing receipt for transaction {}", tx.tx_hash()))?; + let cumulative_gas = receipt.cumulative_gas_used(); + let tx_gas_used = cumulative_gas + .checked_sub(previous_cumulative_gas) + .ok_or_else(|| eyre::eyre!("receipt cumulative gas decreased"))?; + previous_cumulative_gas = cumulative_gas; + + let gas_pool_charge = if tx.is_l1_msg() { + tx.gas_limit() + } else { + tx_gas_used + }; + gas_pool_used = gas_pool_used + .checked_add(gas_pool_charge) + .ok_or_else(|| eyre::eyre!("block gas-pool usage overflowed u64"))?; + } + + Ok(gas_pool_used) +} + /// Behavior contract: /// - fault pressure: one L1 message plus three independent pool transactions /// cannot all fit under the block gas limit; @@ -86,8 +119,12 @@ fn l2_da_bytes(block: &Block) -> u64 { async fn mixed_block_respects_gas_limit_without_losing_pool_transactions() -> eyre::Result<()> { reth_tracing::init_test_tracing(); + const BLOCK_GAS_LIMIT: u64 = 110_000; + const L1_MESSAGE_GAS_LIMIT: u64 = 50_000; + const OVERFLOW_TX_GAS_LIMIT: u64 = 30_000; + let (mut nodes, wallet) = TestNodeBuilder::new() - .with_gas_limit(80_000) + .with_gas_limit(BLOCK_GAS_LIMIT) .build() .await?; let node = nodes.pop().expect("one node requested"); @@ -103,7 +140,7 @@ async fn mixed_block_respects_gas_limit_without_losing_pool_transactions() -> ey .inject_tx( MorphTxBuilder::new(wallet.chain_id, signer_b, 0) .with_v1_eth_fee() - .with_gas_limit(30_000) + .with_gas_limit(OVERFLOW_TX_GAS_LIMIT) .with_fees(HIGH_FEE, HIGH_FEE) .build_signed()?, ) @@ -113,7 +150,7 @@ async fn mixed_block_respects_gas_limit_without_losing_pool_transactions() -> ey .inject_tx( MorphTxBuilder::new(wallet.chain_id, signer_c, 0) .with_v1_eth_fee() - .with_gas_limit(30_000) + .with_gas_limit(OVERFLOW_TX_GAS_LIMIT) .with_fees(LOW_FEE, LOW_FEE) .build_signed()?, ) @@ -123,7 +160,7 @@ async fn mixed_block_respects_gas_limit_without_losing_pool_transactions() -> ey &node, vec![ L1MessageBuilder::new(0) - .with_gas_limit(50_000) + .with_gas_limit(L1_MESSAGE_GAS_LIMIT) .build_encoded(), ], ) @@ -133,12 +170,14 @@ async fn mixed_block_respects_gas_limit_without_losing_pool_transactions() -> ey assert_l1_prefix(&block, 1); assert!(block.header.inner.gas_used <= block.header.inner.gas_limit); - // The excluded transaction would not have fit, so the block must genuinely be - // close to full rather than merely under the limit. + let gas_pool_used = block_gas_pool_used(&node, &block)?; + assert!(gas_pool_used <= block.header.inner.gas_limit); + // Header gasUsed intentionally excludes unused gas reserved by the L1 message, so prove the + // packing limit bound against reconstructed gas-pool usage instead. assert!( - block.header.inner.gas_used + 30_000 > block.header.inner.gas_limit, - "gas limit did not actually bind: used {} of {}", - block.header.inner.gas_used, + gas_pool_used + OVERFLOW_TX_GAS_LIMIT > block.header.inner.gas_limit, + "gas limit did not actually bind: gas pool used {} of {}", + gas_pool_used, block.header.inner.gas_limit ); assert!(hashes.contains(®ular_hash)); diff --git a/crates/payload/builder/src/builder.rs b/crates/payload/builder/src/builder.rs index edd42955..4b86137f 100644 --- a/crates/payload/builder/src/builder.rs +++ b/crates/payload/builder/src/builder.rs @@ -71,11 +71,13 @@ impl MorphPayloadTransactions for () { /// Morph's payload builder. /// /// Builds L2 blocks by executing: -/// 1. L1 message transactions from payload attributes -/// 2. Pool transactions (L2 transactions from mempool, always included) +/// 1. the transactions supplied in the payload attributes, in order +/// 2. the best pool transactions — unless the attributes set `no_tx_pool` /// -/// This matches go-ethereum's behavior where txpool transactions are always -/// pulled after L1 messages are executed. +/// Sequencer assembly supplies only L1 messages in step 1 and relies on step 2 for L2 +/// transactions, matching go-ethereum's `AssembleL2Block`. Derivation sets `no_tx_pool` and +/// supplies the complete committed transaction list, matching go-ethereum's `NewSafeL2Block`, +/// which executes the block through `BlockChain.ProcessBlock` rather than the miner. #[derive(Clone, Debug)] pub struct MorphPayloadBuilder { /// The EVM configuration. @@ -326,26 +328,32 @@ impl MorphPayloadBuilderCtx { BestTransactionsAttributes::new(base_fee, None) } - /// Executes L1 message transactions from payload attributes. + /// Executes the transactions supplied in the payload attributes, in order. /// - /// L1 messages are forced transactions from the L1 bridge that must be executed first. - /// They must have sequential queue indices and are never pulled from the transaction pool. + /// Under sequencer assembly these are the L1 messages from the L1 bridge, which must be + /// executed first and carry strictly sequential queue indices. Under a deterministic build + /// (`no_tx_pool`) the list is the complete committed block, so it may also contain L2 + /// transactions after the L1 messages; those are charged fees normally. /// - /// If the next L1 message does not fit in remaining block gas, packing stops and the - /// leftover messages are left for the next block via `next_l1_msg_index`. A single - /// message larger than the whole block gas limit is skipped for this height (not - /// included, index not advanced) so assemble still returns a block. + /// Behaviour when a transaction does not fit the remaining block gas depends on the policy: + /// + /// - assembly: packing stops and the leftover messages roll over to the next block via + /// `next_l1_msg_index`. A single message larger than the whole block gas limit is skipped + /// for this height (not included, index not advanced) so assemble still returns a block. + /// - deterministic build: this is a hard error, mirroring go-ethereum's `ProcessBlock` + /// returning `ErrGasLimitReached` and rejecting the whole block. /// /// Returns the executed transaction bytes for inclusion in ExecutableL2Data. - fn execute_l1_messages( + fn execute_supplied_transactions( &self, builder: &mut impl BlockBuilder, info: &mut ExecutionInfo, ) -> Result, PayloadBuilderError> { let block_gas_limit = builder.evm().block().gas_limit(); let base_fee = builder.evm().block().basefee(); - let l1_tx_count = self.attributes().transactions.len(); - let mut executed_txs: Vec = Vec::with_capacity(l1_tx_count); + let supplied_tx_count = self.attributes().transactions.len(); + let mut executed_txs: Vec = Vec::with_capacity(supplied_tx_count); + let mut saw_l2_transaction = false; for (tx_idx, tx_with_encoded) in self.attributes().transactions.iter().enumerate() { // The transaction is already recovered in `try_new` via `try_into_recovered()`. @@ -361,12 +369,44 @@ impl MorphPayloadBuilderCtx { )); } + // L1 messages must precede every L2 transaction. Consensus rejects a violation + // post-execution (`validate_l1_messages_in_block`), but under a deterministic + // build the supplied list may legitimately contain L2 transactions, so catching + // the bad ordering here fails fast and names the actual problem — the caller's + // transaction list — instead of surfacing it as a block validation error after + // the whole block has been executed. + if recovered_tx.is_l1_msg() { + if saw_l2_transaction { + return Err(PayloadBuilderError::other( + MorphPayloadBuilderError::L1MessageAfterRegularTx, + )); + } + } else { + saw_l2_transaction = true; + } + let tx_gas = recovered_tx.gas_limit(); // Match morph-geth: stop L1 packing when the next message does not fit // remaining gas, and still seal the block with what already fits. // L1 messages are excluded from DA payload size (prepaid on L1). if info.is_tx_over_limits(tx_gas, 0, block_gas_limit) { + // A deterministic build must reproduce the committed block exactly. Dropping + // the tail would silently seal a *different* block, and since SafeL2Data + // carries no expected hash to check against, the divergence would only + // surface many blocks later. Reject instead, as go-ethereum's ProcessBlock + // does via ErrGasLimitReached. + if self.attributes().no_tx_pool { + return Err(PayloadBuilderError::other( + MorphPayloadBuilderError::BlockGasLimitExceeded { + tx_index: tx_idx, + tx_gas, + gas_pool_used: info.gas_pool_used, + block_gas_limit, + }, + )); + } + if info.transaction_count == 0 { tracing::warn!( target: "payload_builder", @@ -381,6 +421,7 @@ impl MorphPayloadBuilderCtx { tx_index = tx_idx, tx_gas, cumulative_gas_used = info.cumulative_gas_used, + gas_pool_used = info.gas_pool_used, block_gas_limit, "L1 message would exceed remaining block gas; stopping L1 packing" ); @@ -404,10 +445,10 @@ impl MorphPayloadBuilderCtx { tx_index = tx_idx, %error, ?recovered_tx, - "invalid L1 message transaction in payload attributes" + "invalid supplied transaction in payload attributes" ); return Err(PayloadBuilderError::other( - MorphPayloadBuilderError::InvalidSequencerTransaction { + MorphPayloadBuilderError::InvalidSuppliedTransaction { error: error.to_string(), }, )); @@ -418,10 +459,10 @@ impl MorphPayloadBuilderCtx { tx_index = tx_idx, %err, ?recovered_tx, - "validation error in L1 message transaction" + "validation error in supplied transaction" ); return Err(PayloadBuilderError::other( - MorphPayloadBuilderError::InvalidSequencerTransaction { + MorphPayloadBuilderError::InvalidSuppliedTransaction { error: err.to_string(), }, )); @@ -433,7 +474,7 @@ impl MorphPayloadBuilderCtx { tx_index = tx_idx, %err, ?recovered_tx, - "fatal EVM execution error on L1 message transaction" + "fatal EVM execution error on supplied transaction" ); return Err(PayloadBuilderError::EvmExecutionError(Box::new(err))); } @@ -444,12 +485,13 @@ impl MorphPayloadBuilderCtx { // For L1 messages, track the next L1 message index. // L1 gas is prepaid on L1, so no fees are collected here. - let gas_used = if recovered_tx.is_l1_msg() { + let is_l1_msg = recovered_tx.is_l1_msg(); + if is_l1_msg { // Ensure the queue index is strictly sequential if let Some(queue_index) = recovered_tx.queue_index() { if queue_index != info.next_l1_message_index { return Err(PayloadBuilderError::other( - MorphPayloadBuilderError::InvalidSequencerTransaction { + MorphPayloadBuilderError::InvalidSuppliedTransaction { error: format!( "invalid L1 message queue index: expected {}, got {}", info.next_l1_message_index, queue_index @@ -459,18 +501,18 @@ impl MorphPayloadBuilderCtx { } info.next_l1_message_index = queue_index + 1; } - // Use actual gas consumed (including intrinsic gas) - gas_used } else { // Calculate fees for L2 transactions: effective_tip * gas_used let effective_tip = recovered_tx .effective_tip_per_gas(base_fee) .unwrap_or_default(); info.total_fees += U256::from(effective_tip) * U256::from(gas_used); - gas_used - }; + } info.cumulative_gas_used += gas_used; + // Morph geth reports actual L1 execution gas in receipts but does not return unused + // L1-message gas to the block gas pool. Regular transactions do return unused gas. + info.gas_pool_used += if is_l1_msg { tx_gas } else { gas_used }; // Increment transaction count info.transaction_count += 1; @@ -507,10 +549,11 @@ impl MorphPayloadBuilderCtx { } // Check if the breaker triggers (time, gas, or DA limits) - if breaker.should_break(info.cumulative_gas_used, info.cumulative_da_bytes_used) { + if breaker.should_break(info.gas_pool_used, info.cumulative_da_bytes_used) { tracing::debug!( target: "payload_builder", cumulative_gas_used = info.cumulative_gas_used, + gas_pool_used = info.gas_pool_used, cumulative_da_bytes_used = info.cumulative_da_bytes_used, transaction_count = info.transaction_count, elapsed = ?breaker.elapsed(), @@ -619,6 +662,7 @@ impl MorphPayloadBuilderCtx { // Update execution info info.cumulative_gas_used += gas_used; + info.gas_pool_used += gas_used; info.cumulative_da_bytes_used += tx_size; info.transaction_count += 1; @@ -639,8 +683,13 @@ impl MorphPayloadBuilderCtx { /// Execution information collected during payload building. #[derive(Debug, Default)] struct ExecutionInfo { - /// Cumulative gas used by all executed transactions. + /// Cumulative gas reported in transaction receipts and the block header. cumulative_gas_used: u64, + /// Gas unavailable to later transactions under Morph geth's block gas-pool rules. + /// + /// L1 messages contribute their full gas limit here, while regular transactions contribute + /// their actual gas used. + gas_pool_used: u64, /// Cumulative encoded L2 transaction bytes counted toward the DA packing cap. /// L1 messages are not included. cumulative_da_bytes_used: u64, @@ -659,6 +708,7 @@ impl ExecutionInfo { const fn new(next_l1_message_index: u64, max_da_block_size: Option) -> Self { Self { cumulative_gas_used: 0, + gas_pool_used: 0, cumulative_da_bytes_used: 0, total_fees: U256::ZERO, next_l1_message_index, @@ -673,7 +723,7 @@ impl ExecutionInfo { /// transaction with an absurd gas limit through and produce an invalid block. fn is_tx_over_limits(&self, tx_gas_limit: u64, tx_size: u64, block_gas_limit: u64) -> bool { if self - .cumulative_gas_used + .gas_pool_used .checked_add(tx_gas_limit) .is_none_or(|total_gas| total_gas > block_gas_limit) { @@ -771,35 +821,47 @@ where // Create breaker for early exit from pool transaction execution let breaker = ctx.builder_config.breaker(block_gas_limit); - // Execute L1 message transactions (must be first, with sequential queue indices) + // Execute the supplied transactions (L1 messages must form a sequential prefix). let txs_all_started = Instant::now(); - let mut executed_txs = ctx.execute_l1_messages(&mut builder, &mut info)?; - - // Always execute pool transactions (L2 transactions from mempool) - // This matches go-ethereum behavior where txpool transactions are always included - let best_txs = best(ctx.best_transaction_attributes(base_fee)); - if ctx - .execute_pool_transactions( - &mut builder, - &mut info, - &mut executed_txs, - best_txs, - &breaker, - )? - .is_some() - { - // Check if it was a cancellation or just breaker triggered - if ctx.cancel.is_cancelled() { - return Ok(BuildOutcomeKind::Cancelled); + let mut executed_txs = ctx.execute_supplied_transactions(&mut builder, &mut info)?; + + // Append the best pool transactions, unless the caller demanded a deterministic build. + // + // Derivation (`engine_newSafeL2Block`) sets `no_tx_pool`: a follower's pool holds + // gossiped transactions that the sequencer committed to *later* blocks, so appending + // them here would fork the follower off the sequencer chain. + if ctx.attributes().include_tx_pool() { + let best_txs = best(ctx.best_transaction_attributes(base_fee)); + if ctx + .execute_pool_transactions( + &mut builder, + &mut info, + &mut executed_txs, + best_txs, + &breaker, + )? + .is_some() + { + // Check if it was a cancellation or just breaker triggered + if ctx.cancel.is_cancelled() { + return Ok(BuildOutcomeKind::Cancelled); + } + // Breaker triggered - continue with current transactions + tracing::debug!( + target: "payload_builder", + elapsed = ?breaker.elapsed(), + cumulative_gas_used = info.cumulative_gas_used, + gas_pool_used = info.gas_pool_used, + cumulative_da_bytes_used = info.cumulative_da_bytes_used, + tx_count = executed_txs.len(), + "breaker stopped pool execution, finalizing payload" + ); } - // Breaker triggered - continue with current transactions + } else { tracing::debug!( target: "payload_builder", - elapsed = ?breaker.elapsed(), - cumulative_gas_used = info.cumulative_gas_used, - cumulative_da_bytes_used = info.cumulative_da_bytes_used, tx_count = executed_txs.len(), - "breaker stopped pool execution, finalizing payload" + "skipping txpool inclusion: deterministic build requested" ); } @@ -944,6 +1006,7 @@ mod tests { fn test_execution_info_default() { let info = ExecutionInfo::default(); assert_eq!(info.cumulative_gas_used, 0); + assert_eq!(info.gas_pool_used, 0); assert_eq!(info.cumulative_da_bytes_used, 0); assert_eq!(info.total_fees, U256::ZERO); assert_eq!(info.next_l1_message_index, 0); @@ -956,6 +1019,7 @@ mod tests { let info = ExecutionInfo::new(42, Some(720 * 1024)); assert_eq!(info.next_l1_message_index, 42); assert_eq!(info.cumulative_gas_used, 0); + assert_eq!(info.gas_pool_used, 0); assert_eq!(info.cumulative_da_bytes_used, 0); assert_eq!(info.total_fees, U256::ZERO); assert_eq!(info.transaction_count, 0); @@ -981,30 +1045,30 @@ mod tests { #[test] fn test_is_tx_over_limits_within_gas() { let info = ExecutionInfo { - cumulative_gas_used: 100_000, + gas_pool_used: 100_000, ..Default::default() }; - // tx_gas + cumulative = 100_000 + 21_000 = 121_000, block limit = 30_000_000 + // tx_gas + gas_pool_used = 100_000 + 21_000 = 121_000, block limit = 30_000_000 assert!(!info.is_tx_over_limits(21_000, 100, 30_000_000)); } #[test] fn test_is_tx_over_limits_exceeds_gas_limit() { let info = ExecutionInfo { - cumulative_gas_used: 29_990_000, + gas_pool_used: 29_990_000, ..Default::default() }; - // tx_gas + cumulative = 29_990_000 + 21_000 = 30_011_000 > 30_000_000 + // tx_gas + gas_pool_used = 29_990_000 + 21_000 = 30_011_000 > 30_000_000 assert!(info.is_tx_over_limits(21_000, 100, 30_000_000)); } #[test] fn test_is_tx_over_limits_exactly_at_gas_limit() { let info = ExecutionInfo { - cumulative_gas_used: 29_979_000, + gas_pool_used: 29_979_000, ..Default::default() }; - // tx_gas + cumulative = 29_979_000 + 21_000 = 30_000_000 == block limit + // tx_gas + gas_pool_used = 29_979_000 + 21_000 = 30_000_000 == block limit // Uses > comparison, so exactly at limit is NOT over assert!(!info.is_tx_over_limits(21_000, 100, 30_000_000)); } @@ -1012,10 +1076,10 @@ mod tests { #[test] fn test_is_tx_over_limits_one_over_gas_limit() { let info = ExecutionInfo { - cumulative_gas_used: 29_979_001, + gas_pool_used: 29_979_001, ..Default::default() }; - // tx_gas + cumulative = 29_979_001 + 21_000 = 30_000_001 > 30_000_000 + // tx_gas + gas_pool_used = 29_979_001 + 21_000 = 30_000_001 > 30_000_000 assert!(info.is_tx_over_limits(21_000, 100, 30_000_000)); } @@ -1036,7 +1100,7 @@ mod tests { #[test] fn test_is_tx_over_limits_gas_sum_overflow() { let info = ExecutionInfo { - cumulative_gas_used: 1, + gas_pool_used: 1, ..Default::default() }; // Wrapping would yield 0 and wrongly report "fits"; overflow must count as over. diff --git a/crates/payload/builder/src/config.rs b/crates/payload/builder/src/config.rs index 6025e414..460c5472 100644 --- a/crates/payload/builder/src/config.rs +++ b/crates/payload/builder/src/config.rs @@ -160,7 +160,7 @@ impl PayloadBuildingBreaker { /// - Time limit has been exceeded /// - Gas limit has been reached (leaving room for at least one minimal transaction) /// - DA size limit has been reached (leaving room for at least one minimal transaction) - pub fn should_break(&self, cumulative_gas_used: u64, cumulative_da_size_used: u64) -> bool { + pub fn should_break(&self, gas_pool_used: u64, cumulative_da_size_used: u64) -> bool { // Check time limit if self.start.elapsed() >= self.time_limit { tracing::trace!( @@ -173,10 +173,10 @@ impl PayloadBuildingBreaker { } // Check gas limit - stop if remaining gas can't fit even the smallest transaction - if cumulative_gas_used > self.gas_limit.saturating_sub(MIN_TRANSACTION_GAS) { + if gas_pool_used > self.gas_limit.saturating_sub(MIN_TRANSACTION_GAS) { tracing::trace!( target: "payload_builder", - cumulative_gas_used, + gas_pool_used, gas_limit = self.gas_limit, "gas limit reached" ); diff --git a/crates/payload/builder/src/error.rs b/crates/payload/builder/src/error.rs index 76a01d8b..6c3d8511 100644 --- a/crates/payload/builder/src/error.rs +++ b/crates/payload/builder/src/error.rs @@ -11,9 +11,9 @@ pub enum MorphPayloadBuilderError { #[error("failed to recover transaction signer")] TransactionEcRecoverFailed, - /// Invalid sequencer transaction in forced transaction list. - #[error("invalid sequencer transaction: {error}")] - InvalidSequencerTransaction { + /// Invalid transaction in the caller-supplied transaction list. + #[error("invalid supplied transaction: {error}")] + InvalidSuppliedTransaction { /// Human-readable validation error. error: String, }, @@ -26,6 +26,25 @@ pub enum MorphPayloadBuilderError { #[error("L1 message appears after regular transaction")] L1MessageAfterRegularTx, + /// A supplied transaction did not fit the block gas limit during a deterministic build. + /// + /// Only raised when `no_tx_pool` is set. Sequencer assembly instead stops packing and + /// seals the block with whatever already fits, leaving the rest for the next block. + #[error( + "transaction {tx_index} exceeds block gas limit during deterministic build: \ + tx_gas={tx_gas}, gas_pool_used={gas_pool_used}, block_gas_limit={block_gas_limit}" + )] + BlockGasLimitExceeded { + /// Index of the offending transaction in the supplied list. + tx_index: usize, + /// Gas limit of the offending transaction. + tx_gas: u64, + /// Gas unavailable to later transactions under the block gas-pool rules. + gas_pool_used: u64, + /// Gas limit of the block being built. + block_gas_limit: u64, + }, + /// Generic storage error (e.g. from revm EvmDatabaseError, ProviderError). #[error("storage error: {0}")] Storage(String), diff --git a/crates/payload/builder/src/lib.rs b/crates/payload/builder/src/lib.rs index 2c2ed35a..096d11f8 100644 --- a/crates/payload/builder/src/lib.rs +++ b/crates/payload/builder/src/lib.rs @@ -4,17 +4,26 @@ //! //! The [`MorphPayloadBuilder`] implements reth's `PayloadBuilder` trait //! to construct L2 blocks with: -//! - L1 message transactions (prioritized, must be at the beginning) -//! - Pool transactions (L2 transactions from mempool, always included) +//! - transactions supplied in the payload attributes (executed first, in order) +//! - pool transactions, unless the caller requested a deterministic build //! -//! # Transaction Ordering +//! # Build Policies //! -//! Transactions are included in the following order: +//! The `no_tx_pool` attribute selects between two behaviours: +//! +//! **Sequencer assembly** (`no_tx_pool == false`, `engine_assembleL2Block`): //! 1. L1 messages from payload attributes (must have sequential queue indices) -//! 2. Pool transactions (L2 transactions from mempool) +//! 2. Best pool transactions (L2 transactions from mempool) +//! +//! This matches go-ethereum's `AssembleL2Block`, where L1 messages arrive via payload +//! attributes and L2 transactions are pulled from the txpool. //! -//! This matches go-ethereum's behavior where L1 messages are provided via -//! payload attributes and L2 transactions are always pulled from the txpool. +//! **Deterministic build** (`no_tx_pool == true`, `engine_newSafeL2Block`): the attributes +//! carry the complete ordered transaction list of an L1-committed block, and nothing is taken +//! from the pool. This mirrors go-ethereum, where `NewSafeL2Block` executes the decoded block +//! through `BlockChain.ProcessBlock` and never involves the miner at all. Reading the pool +//! here would let a follower absorb gossiped transactions belonging to *later* blocks and +//! fork off the sequencer chain. //! //! # L1 Message Rules //! @@ -22,8 +31,9 @@ //! - Queue indices must be strictly sequential //! - Gas is prepaid on L1, so no refunds for unused gas //! - L1 messages are never in the transaction pool -//! - If the next L1 message does not fit remaining block gas, packing stops and -//! leftovers are retried on the next block via `next_l1_msg_index` +//! - If a transaction does not fit remaining block gas: under assembly, packing stops and +//! leftovers are retried on the next block via `next_l1_msg_index`; under a deterministic +//! build it is a hard error, as in go-ethereum's `ErrGasLimitReached` #![cfg_attr(not(test), warn(unused_crate_dependencies))] #![cfg_attr(docsrs, feature(doc_cfg, doc_auto_cfg))] diff --git a/crates/payload/types/src/attributes.rs b/crates/payload/types/src/attributes.rs index 86d72131..6459c11b 100644 --- a/crates/payload/types/src/attributes.rs +++ b/crates/payload/types/src/attributes.rs @@ -8,8 +8,10 @@ use morph_primitives::MorphTxEnvelope; use reth_primitives_traits::{Recovered, SignerRecoverable, WithEncoded}; use sha2::{Digest, Sha256}; -/// Version byte mixed into Morph payload IDs. Bumped only when the payload-attribute -/// hashing scheme materially changes; serves as a domain separator across versions. +/// Engine API version byte stored in Morph payload IDs. +/// +/// Morph's custom payload methods currently use version 1. The complete payload attributes, +/// including the txpool policy, are hashed separately from this protocol version. pub const MORPH_PAYLOAD_BUILDER_VERSION: u8 = 1; /// Morph-specific payload attributes for Engine API. @@ -23,19 +25,37 @@ pub struct MorphPayloadAttributes { #[serde(flatten)] pub inner: PayloadAttributes, - /// L1 message transactions to include at the beginning of the block. + /// Transactions to execute at the beginning of the block, in the given order. + /// + /// The exact meaning depends on [`Self::no_tx_pool`]: /// - /// **IMPORTANT**: This field contains **only L1 messages** (L1→L2 deposit transactions). - /// L2 transactions are always pulled from the transaction pool, matching go-ethereum's behavior. + /// - `no_tx_pool == false` (sequencer assembly): **only L1 messages** (L1→L2 deposits). + /// They are executed first, then the builder appends the best transactions from the + /// local pool. This matches go-ethereum's `AssembleL2Block`. + /// - `no_tx_pool == true` (derivation import): the **complete ordered transaction list** + /// of the block, L1 messages first followed by the committed L2 transactions. Nothing + /// is appended from the pool. This matches go-ethereum's `NewSafeL2Block`, which + /// executes the decoded block via `BlockChain.ProcessBlock` and never touches the miner. /// - /// L1 messages: - /// - Must have sequential queue indices - /// - Are never in the mempool - /// - Must be explicitly provided by the sequencer - /// - Are executed before any L2 transactions + /// In both cases any L1 messages present must carry strictly sequential queue indices and + /// must precede the L2 transactions. L1 messages are never in the mempool and must always + /// be supplied explicitly. #[serde(default, skip_serializing_if = "Option::is_none")] pub transactions: Option>, + /// Disables txpool selection, making block building deterministic. + /// + /// When true, the builder executes exactly [`Self::transactions`] and appends nothing from + /// the local pool. Required by derivation (`engine_newSafeL2Block`): a follower's pool + /// holds gossiped transactions that the sequencer committed to *later* blocks, so + /// appending them to an earlier derived block forks the follower off the sequencer chain. + /// + /// This is deliberately explicit rather than inferred from `transactions.is_some()`: + /// sequencer assembly also supplies `transactions` whenever the block has L1 messages, + /// and inferring the flag there would stop the sequencer from packing the mempool at all. + #[serde(default)] + pub no_tx_pool: bool, + /// Optional gas limit override used by derivation/safe import. #[serde( default, @@ -88,6 +108,7 @@ impl From for MorphPayloadAttributes { Self { inner, transactions: None, + no_tx_pool: false, gas_limit: None, base_fee_per_gas: None, } @@ -97,7 +118,7 @@ impl From for MorphPayloadAttributes { /// Internal payload builder attributes. /// /// This is the internal representation used by the payload builder, -/// with decoded L1 messages and computed payload ID. +/// with decoded supplied transactions and a computed payload ID. /// /// Implements `reth_payload_primitives::PayloadAttributes` so it can serve as the /// `type Attributes` in `PayloadBuilder` (v2.0.0 requires the builder attributes to @@ -125,13 +146,14 @@ pub struct MorphPayloadBuilderAttributes { /// Parent beacon block root. pub parent_beacon_block_root: Option, - /// Decoded L1 message transactions with original encoded bytes. + /// Decoded transactions to execute first, with their original encoded bytes. /// - /// **IMPORTANT**: This contains **only L1 messages**, not L2 transactions. - /// L2 transactions are always pulled from the transaction pool. + /// Holds only L1 messages during sequencer assembly, and the complete ordered block + /// transaction list when [`Self::no_tx_pool`] is set. See + /// [`MorphPayloadAttributes::transactions`] for the full contract. /// - /// L1 messages are decoded and recovered during construction to avoid - /// repeated decoding in the payload builder. + /// Decoded and recovered during construction to avoid repeated decoding in the + /// payload builder. /// /// Skipped for serde: this is purely an internal runtime field derived from /// `MorphPayloadAttributes::transactions` during `try_new`. It is never @@ -139,6 +161,9 @@ pub struct MorphPayloadBuilderAttributes { #[serde(skip)] pub transactions: Vec>>, + /// Disables txpool selection; see [`MorphPayloadAttributes::no_tx_pool`]. + pub no_tx_pool: bool, + /// Optional gas limit override propagated to EVM env construction. pub gas_limit: Option, @@ -147,15 +172,16 @@ pub struct MorphPayloadBuilderAttributes { } impl MorphPayloadBuilderAttributes { - /// Build from parent hash + RPC attributes + version byte, decoding L1 messages. + /// Build from parent hash + RPC attributes + version byte, decoding supplied transactions. pub fn try_new( parent: B256, attributes: MorphPayloadAttributes, version: u8, ) -> Result { let id = payload_id_morph(&parent, &attributes, version); + let no_tx_pool = attributes.no_tx_pool; - // Decode and recover L1 message transactions + // Decode and recover the supplied transactions let transactions = attributes .transactions .unwrap_or_default() @@ -182,6 +208,7 @@ impl MorphPayloadBuilderAttributes { withdrawals: attributes.inner.withdrawals.unwrap_or_default().into(), parent_beacon_block_root: attributes.inner.parent_beacon_block_root, transactions, + no_tx_pool, gas_limit: attributes.gas_limit, base_fee_per_gas: attributes.base_fee_per_gas, }) @@ -224,7 +251,12 @@ impl MorphPayloadBuilderAttributes { /// Returns true if there are L1 messages to execute. pub fn has_l1_messages(&self) -> bool { - !self.transactions.is_empty() + self.transactions.iter().any(|tx| tx.value().is_l1_msg()) + } + + /// Returns true if the builder may append transactions from the local pool. + pub fn include_tx_pool(&self) -> bool { + !self.no_tx_pool } } @@ -283,10 +315,14 @@ fn payload_id_morph(parent: &B256, attributes: &MorphPayloadAttributes, version: hasher.update(root.as_slice()); } - // Hash whether L1 message list was explicitly supplied. + // Hash the txpool policy: an assemble and a derivation import of the same inputs are + // different payloads (one may append pool transactions), so they must not share an id. + hasher.update([u8::from(attributes.no_tx_pool)]); + + // Hash whether the transaction list was explicitly supplied. hasher.update([u8::from(attributes.transactions.is_some())]); - // Hash L1 messages if present. + // Hash the supplied transactions if present. if let Some(txs) = &attributes.transactions { hasher.update(&txs.len().to_be_bytes()[..]); for tx in txs { @@ -323,6 +359,9 @@ fn payload_id_morph(parent: &B256, attributes: &MorphPayloadAttributes, version: #[cfg(test)] mod tests { use super::*; + use alloy_consensus::{Sealed, Signed, TxLegacy}; + use alloy_primitives::{Signature, TxKind, U256}; + use morph_primitives::transaction::TxL1Msg; fn create_test_attributes() -> MorphPayloadAttributes { MorphPayloadAttributes { @@ -337,6 +376,7 @@ mod tests { target_gas_limit: None, }, transactions: None, + no_tx_pool: false, gas_limit: None, base_fee_per_gas: None, } @@ -346,6 +386,66 @@ mod tests { fn test_default_attributes() { let attrs = MorphPayloadAttributes::default(); assert!(attrs.transactions.is_none()); + // Sequencer assembly is the default; only derivation opts out of the pool. + assert!(!attrs.no_tx_pool); + } + + #[test] + fn test_payload_id_distinguishes_tx_pool_policy() { + // An assemble and a derivation import of identical inputs are different payloads: + // one may append pool transactions. They must not collide on the same payload id. + let parent = B256::random(); + let mut with_pool = create_test_attributes(); + with_pool.transactions = Some(vec![Bytes::from(vec![0x01])]); + let mut without_pool = with_pool.clone(); + without_pool.no_tx_pool = true; + + assert_ne!( + payload_id_morph(&parent, &with_pool, 1), + payload_id_morph(&parent, &without_pool, 1), + ); + } + + #[test] + fn test_no_tx_pool_defaults_false_when_absent_from_json() { + // Existing sequencer callers omit the field entirely and must keep packing the pool. + let json = r#"{ + "timestamp": "0x499602d2", + "prevRandao": "0x0000000000000000000000000000000000000000000000000000000000000001", + "suggestedFeeRecipient": "0x0000000000000000000000000000000000000002" + }"#; + + let attrs: MorphPayloadAttributes = serde_json::from_str(json).expect("deserialize"); + assert!(!attrs.no_tx_pool); + } + + #[test] + fn test_no_tx_pool_deserializes_from_camel_case() { + let json = r#"{ + "timestamp": "0x499602d2", + "prevRandao": "0x0000000000000000000000000000000000000000000000000000000000000001", + "suggestedFeeRecipient": "0x0000000000000000000000000000000000000002", + "noTxPool": true + }"#; + + let attrs: MorphPayloadAttributes = serde_json::from_str(json).expect("deserialize"); + assert!(attrs.no_tx_pool); + } + + #[test] + fn test_builder_attributes_carry_tx_pool_policy() { + let parent = B256::random(); + + let mut attrs = create_test_attributes(); + attrs.no_tx_pool = true; + let built = MorphPayloadBuilderAttributes::try_new(parent, attrs, 1).expect("try_new"); + assert!(built.no_tx_pool); + assert!(!built.include_tx_pool()); + + let built = MorphPayloadBuilderAttributes::try_new(parent, create_test_attributes(), 1) + .expect("try_new"); + assert!(!built.no_tx_pool); + assert!(built.include_tx_pool()); } #[test] @@ -575,14 +675,49 @@ mod tests { } #[test] - fn test_builder_attributes_has_l1_messages_empty() { - let attrs = MorphPayloadBuilderAttributes::try_new( + fn test_builder_attributes_detect_only_l1_messages() { + let mut attrs = MorphPayloadBuilderAttributes::try_new( B256::ZERO, create_test_attributes(), MORPH_PAYLOAD_BUILDER_VERSION, ) .unwrap(); assert!(!attrs.has_l1_messages()); + + let l2_tx = MorphTxEnvelope::Legacy(Signed::new_unhashed( + TxLegacy { + chain_id: Some(1), + nonce: 0, + gas_price: 1, + gas_limit: 21_000, + to: TxKind::Call(Address::ZERO), + value: U256::ZERO, + input: Bytes::new(), + }, + Signature::test_signature(), + )); + attrs.transactions.push(WithEncoded::new( + Bytes::new(), + Recovered::new_unchecked(l2_tx, Address::ZERO), + )); + assert!( + !attrs.has_l1_messages(), + "a non-empty L2-only list must not be reported as L1 messages" + ); + + let l1_msg = MorphTxEnvelope::L1Msg(Sealed::new(TxL1Msg { + queue_index: 0, + gas_limit: 21_000, + to: Address::ZERO, + value: U256::ZERO, + sender: Address::ZERO, + input: Bytes::new(), + })); + attrs.transactions.push(WithEncoded::new( + Bytes::new(), + Recovered::new_unchecked(l1_msg, Address::ZERO), + )); + assert!(attrs.has_l1_messages()); } #[test] diff --git a/crates/payload/types/src/safe_l2_data.rs b/crates/payload/types/src/safe_l2_data.rs index 3344887c..aa24736f 100644 --- a/crates/payload/types/src/safe_l2_data.rs +++ b/crates/payload/types/src/safe_l2_data.rs @@ -35,7 +35,10 @@ pub struct SafeL2Data { #[serde(with = "alloy_serde::quantity")] pub timestamp: u64, - /// RLP-encoded transactions. + /// Complete ordered list of RLP-encoded block transactions. + /// + /// L1 messages, when present, form a prefix followed by the committed L2 transactions. + /// Derivation executes exactly this list and never appends transactions from the local pool. #[serde(default)] pub transactions: Vec,