feat: Implement MorphPayloadBuilder for L2 block construction #10
Conversation
- Preserve MorphHeader, MPTFork hardfork - Use block+timestamp based hardforks (Bernoulli/Curie block-based, Morph203/Viridian/Emerald/MPTFork timestamp-based) - Move ConfigureEvm to config.rs with reth-codec feature gate for zkvm compatibility - Fix morph_hardfork_at to take both block_number and timestamp - Use MorphExecutionData for custom payload handling - Add morph field to test genesis configurations
📝 WalkthroughWalkthroughAdds MPTFork hardfork, introduces concrete MorphHeader with L2 fields, implements a full Morph payload builder and its config/errors, refactors L1 message/Tx envelope to sealed 2718 format, adds L1 gas price oracle slots/constants, and updates chainspec/evm integrations and workspace dependencies. Changes
Sequence Diagram(s)sequenceDiagram
participant Builder as MorphPayloadBuilder
participant Breaker as PayloadBuildingBreaker
participant EVM as MorphEvmConfig
participant Pool as TransactionPool
participant State as StateProvider
participant Sequencer as SequencerTxs
Builder->>Breaker: breaker(block_gas_limit)
Builder->>Builder: try_build(attributes)
Builder->>Sequencer: execute_sequencer_transactions()
Sequencer->>State: apply_state_changes
Sequencer-->>Builder: (gas_used, fees, next_l1_index)
Builder->>Breaker: should_break(cumulative_gas, da_bytes)
alt continue
Builder->>Pool: best_transactions(...)
Pool-->>Builder: candidate_txns
Builder->>State: execute_pool_transactions(txns)
Builder->>Breaker: should_break(...)
end
Builder->>EVM: finalize_block(state_root, header)
Builder-->>Builder: MorphBuiltPayload
sequenceDiagram
participant Envelope as MorphTxEnvelope
participant SealedTx as Sealed<TxL1Msg>
participant Codec as EIP2718/Codec
Envelope->>SealedTx: hold sealed L1 message (sender stored)
SealedTx->>Codec: encode() (EIP-2718)
Codec-->>SealedTx: bytes (uses L1_MSG_SIGNATURE placeholder)
Codec->>Envelope: decode_fields(queue, gas, to, value, input, sender)
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
panos-xyz
requested review from
anylots and
chengwenxi
and removed request for
anylots
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
crates/chainspec/src/spec.rs (1)
139-153: Comment on timestamp-based forks should include MPTFork.The inline note still lists only Morph203/Viridian/Emerald, but Line 153 adds MPTFork.
✏️ Suggested comment update
- // while Morph203, Viridian, and Emerald use timestamp-based activation. + // while Morph203, Viridian, Emerald, and MPTFork use timestamp-based activation.
🤖 Fix all issues with AI agents
In `@crates/payload/types/src/lib.rs`:
- Around line 49-60: The serde dependency in this crate needs the "rc" feature
enabled because MorphExecutionData derives Serialize/Deserialize for
Arc<SealedBlock<Block>>; update the serde dependency entry in the crate's
Cargo.toml to include "rc" in its features (so the features include "derive" and
"rc") to allow serialization of Arc types used by MorphExecutionData.
In `@crates/primitives/src/lib.rs`:
- Line 1: The crate documentation incorrectly states that MorphHeader is a
"Header type alias (same as Ethereum)"; update the top-level docs in lib.rs to
reflect that MorphHeader is now a dedicated type defined in the header module
(reference the header module and the MorphHeader type name), remove or reword
any "type alias" / "same as Ethereum" phrasing, and adjust the related doc
paragraphs that mention the alias (the sections referencing MorphHeader and the
header module) to briefly describe the dedicated header type and point readers
to the header module for details.
In `@crates/primitives/src/receipt/mod.rs`:
- Around line 425-429: The doctest imports an unused type; remove the
unnecessary `use alloy_consensus::Receipt;` line from the doctest block that
references `MorphReceipt` and `calculate_receipt_root_no_memo` so the example
only imports/uses symbols that are actually needed and eliminates doctest
warnings.
In `@crates/revm/src/l1block.rs`:
- Around line 92-118: The INITIAL_COMMIT_SCALAR constant is wrong and must be
increased by 1000×; update the value used to construct INITIAL_COMMIT_SCALAR
(the U256::from_limbs call that currently uses 230759955285) to the corrected
literal 230_000_000_000_000 (i.e., 230_000e9) so
CURIE_L1_GAS_PRICE_ORACLE_STORAGE contains the correct commitScalar; modify the
const INITIAL_COMMIT_SCALAR declaration to use the new limbs value and ensure no
other dependent constants are changed.
🧹 Nitpick comments (6)
crates/primitives/src/transaction/envelope.rs (1)
240-244: Consolidate the empty‑signature definition.
There are now two sources of “empty signature.” ReuseTxL1Msg::signature()to keep a single source of truth.♻️ Suggested tweak
- const L1_MSG_SIGNATURE: Signature = Signature::new( - alloy_primitives::U256::ZERO, - alloy_primitives::U256::ZERO, - false, - ); + const L1_MSG_SIGNATURE: Signature = TxL1Msg::signature();Also applies to: 262-273, 282-282
crates/chainspec/src/hardfork.rs (1)
178-199: TheFrom<SpecId>implementation has unreachable branches.Since all
MorphHardforkvariants map toSpecId::OSAKA(lines 167-174), the conversion fromSpecIdback toMorphHardforkwill always returnMPTForkfor any spec>= OSAKA. The subsequentelse ifbranches (Emerald, Viridian, Morph203, Curie) are effectively dead code.The comment at lines 179-183 acknowledges this isn't a strict inverse, but this implementation makes it impossible to recover the original hardfork from a
SpecId. If this is intentional (e.g., defaulting to the latest hardfork), consider simplifying:♻️ Suggested simplification if defaulting to latest is intentional
impl From<SpecId> for MorphHardfork { - /// Maps a [`SpecId`] to the *latest compatible* [`MorphHardfork`]. - /// - /// Note: this is intentionally not a strict inverse of - /// `From<MorphHardfork> for SpecId`, because multiple Morph - /// hardforks may share the same underlying EVM spec. + /// Maps a [`SpecId`] to the *latest* [`MorphHardfork`]. + /// + /// Note: All Morph hardforks currently share the same underlying EVM spec (OSAKA), + /// so this always returns MPTFork for OSAKA-compatible specs. fn from(spec: SpecId) -> Self { - if spec.is_enabled_in(SpecId::from(Self::MPTFork)) { - Self::MPTFork - } else if spec.is_enabled_in(SpecId::from(Self::Emerald)) { - Self::Emerald - } else if spec.is_enabled_in(SpecId::from(Self::Viridian)) { - Self::Viridian - } else if spec.is_enabled_in(SpecId::from(Self::Morph203)) { - Self::Morph203 - } else if spec.is_enabled_in(SpecId::from(Self::Curie)) { - Self::Curie - } else { - Self::Bernoulli + if spec.is_enabled_in(SpecId::OSAKA) { + Self::MPTFork + } else { + Self::Bernoulli } } }Alternatively, if different hardforks will eventually map to different
SpecIdvalues, keep the current structure but add a comment explaining the future plan.crates/revm/src/lib.rs (1)
58-77: Re-exports are comprehensive but could use better organization.The re-exports include all necessary constants. Minor nit: the comment
// Storage slotson line 66 is oddly placed after several storage slot constants have already been listed. Consider reordering so all storage slots are grouped together after the comment.♻️ Suggested reordering
pub use l1block::{ CURIE_L1_GAS_PRICE_ORACLE_STORAGE, + // Storage slots + GPO_OWNER_SLOT, + GPO_L1_BASE_FEE_SLOT, + GPO_OVERHEAD_SLOT, + GPO_SCALAR_SLOT, + GPO_WHITELIST_SLOT, + GPO_L1_BLOB_BASE_FEE_SLOT, + GPO_COMMIT_SCALAR_SLOT, GPO_BLOB_SCALAR_SLOT, - GPO_COMMIT_SCALAR_SLOT, GPO_IS_CURIE_SLOT, - GPO_L1_BASE_FEE_SLOT, - GPO_L1_BLOB_BASE_FEE_SLOT, - GPO_OVERHEAD_SLOT, - // Storage slots - GPO_OWNER_SLOT, - GPO_SCALAR_SLOT, - GPO_WHITELIST_SLOT, + // Curie initial values + INITIAL_L1_BLOB_BASE_FEE, + INITIAL_COMMIT_SCALAR, INITIAL_BLOB_SCALAR, - INITIAL_COMMIT_SCALAR, - // Curie initial values - INITIAL_L1_BLOB_BASE_FEE, IS_CURIE, + // Address and types L1_GAS_PRICE_ORACLE_ADDRESS, L1BlockInfo, };crates/payload/builder/src/config.rs (1)
197-210: Test with thread sleep may be flaky in CI.The test
test_breaker_should_break_on_time_limitusesthread::sleep(Duration::from_millis(150))to exceed a 100ms time limit. On heavily loaded CI systems, this could occasionally fail if sleep undershoots. Consider increasing the margin or using a more deterministic approach if flakiness is observed.crates/primitives/src/header.rs (1)
215-221: Consider validating that all bytes were consumed during decompression.The
from_compactreturns the number of bytes consumed, but it's discarded. If the input contains trailing garbage, this could silently succeed with incomplete data.🔧 Optional: Add validation
impl reth_db_api::table::Decompress for MorphHeader { fn decompress(value: &[u8]) -> Result<Self, reth_db_api::DatabaseError> { let (obj, _) = reth_codecs::Compact::from_compact(value, value.len()); + // Note: Could validate that consumed == value.len() if strict parsing is needed Ok(obj) } }crates/payload/builder/src/builder.rs (1)
678-678: Minor: Comment step number should be 7, not 6.The previous steps are numbered 1-6, so "Build ExecutableL2Data" should be step 7.
- // 6. Build ExecutableL2Data from the sealed block + // 7. Build ExecutableL2Data from the sealed block
panos-xyz
changed the title
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@crates/payload/builder/src/builder.rs`:
- Around line 288-381: execute_sequencer_transactions currently never updates DA
accounting so sequencer (forced) txs are excluded from
ExecutionInfo.cumulative_da_bytes_used; after you obtain tx_bytes (via
tx_with_encoded.encoded_bytes()) increment info.cumulative_da_bytes_used by
tx_bytes.len() (cast to the same integer type as cumulative_da_bytes_used), and
then enforce the DA limit if present by comparing against the DA limit exposed
on ExecutionInfo or the builder/block (e.g., info.max_da_block_size or
builder/evm block DA limit) and return an appropriate PayloadBuilderError when
exceeded; update execute_sequencer_transactions to do this before executing the
transaction so DA usage prevents adding sequencer txs that would overflow the DA
cap.
♻️ Duplicate comments (1)
crates/primitives/src/lib.rs (1)
21-24: Doc still says MorphHeader is a type alias; update to reflect the dedicated header type.This is the same mismatch noted previously; the bullet still reads “type alias” even though MorphHeader now lives in the header module.
✏️ Suggested doc tweak
-//! - [`MorphHeader`]: Morph header type alias +//! - [`MorphHeader`]: Morph-specific header type with L2 fields (see `header` module)
Summary by CodeRabbit
New Features
Chores
✏️ Tip: You can customize this high-level summary in your review settings.