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
📝 WalkthroughWalkthroughThis PR adds MPTFork hardfork support, introduces a Morph L2 payload builder and its config/breaker/error types, refactors L1 messages (Signed→Sealed) and TxL1Msg fields with EIP-2718 support, introduces MorphHeader, applies Curie L1 gas-oracle initialization, updates many public APIs to use MorphHeader, and adjusts workspace Cargo manifests. Changes
Sequence Diagram(s)sequenceDiagram
participant Builder as MorphPayloadBuilder
participant Pool as TransactionPool
participant EVM as MorphEvm
participant State as State<DB>
participant Breaker as PayloadBuildingBreaker
Builder->>Breaker: create breaker(block_gas_limit)
Builder->>State: prepare_state() / pre-execution
State-->>Builder: state ready
Builder->>Builder: execute_sequencer_transactions()
Builder->>EVM: execute forced sequencer txs
EVM->>State: apply changes
State-->>EVM: result
EVM-->>Builder: gas/fees
loop pool execution until Breaker.should_break()
Builder->>Pool: best_transactions()
Pool-->>Builder: next_tx
Builder->>EVM: execute pool tx
EVM->>State: update
State-->>EVM: result
EVM-->>Builder: gas_used, bytes
Builder->>Breaker: should_break(cumulative_gas, da_bytes)
end
Builder->>State: read_withdraw_trie_root()
Builder->>EVM: finish_block(mpt_root)
EVM-->>Builder: MorphBuiltPayload
sequenceDiagram
participant Executor as MorphBlockExecutor
participant State as State<DB>
participant Oracle as L1GasPriceOracle
participant Curie as apply_curie_hard_fork
Executor->>State: pre_execute(block)
State->>Oracle: ensure_cached(L1_GAS_PRICE_ORACLE_ADDRESS)
Oracle-->>State: contract loaded
alt at Curie transition
State->>Curie: apply_curie_hard_fork()
Curie->>State: read oracle storage
Curie->>State: queue storage transitions (blob_base_fee, commit_scalar, blob_scalar, is_curie)
State-->>Curie: transitions applied
end
Executor->>State: execute_transactions()
State-->>Executor: execution result
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)
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 Fix all issues with AI agents
In `@crates/evm/src/block/curie.rs`:
- Around line 1-67: The file fails rustfmt checks; run rustfmt/cargo fmt to fix
formatting inconsistencies in the Curie hard fork module—format the file
containing apply_curie_hard_fork and related symbols
(CURIE_L1_GAS_PRICE_ORACLE_STORAGE, L1_GAS_PRICE_ORACLE_ADDRESS, StorageSlot) so
the code adheres to cargo fmt rules, then commit the formatted changes;
alternatively run `cargo fmt --all` locally or in CI to automatically apply the
correct formatting.
In `@crates/evm/src/block/mod.rs`:
- Around line 1-11: The file's formatting doesn't meet rustfmt checks; run cargo
fmt --all (or rustfmt this file) to fix whitespace/indentation and comment
formatting in this module (ensure the module header docs and items like the
curie mod declaration and the pub(crate) use receipt::MorphReceiptBuilder line
are properly formatted) so CI `cargo fmt --all -- --check` passes.
In `@crates/evm/src/block/receipt.rs`:
- Around line 1-52: The file fails rustfmt; run cargo fmt --all and commit the
formatting changes so the code matches rustfmt rules. Specifically reformat the
impl for MorphReceiptBuilder and its build_receipt function (see symbols
MorphReceiptBuilder, build_receipt, and ReceiptBuilderCtx) so whitespace,
imports, and indentation conform to rustfmt, then re-run the CI check.
In `@crates/revm/src/lib.rs`:
- Around line 60-79: Remove inline comments from inside the pub use l1block {
... } block (they break rustfmt ordering) and let cargo fmt reorder the items;
if you need grouping semantics, move the comments to a module-level doc comment
above the pub use or add separate commented sections outside the use block.
Ensure the exported symbols (e.g., CURIE_L1_GAS_PRICE_ORACLE_STORAGE,
GPO_OWNER_SLOT, INITIAL_L1_BLOB_BASE_FEE, L1BlockInfo) remain listed but without
inline // Storage slots or // Curie initial values comments inside the braces,
then run cargo fmt to verify.
🧹 Nitpick comments (2)
crates/evm/src/block/receipt.rs (1)
41-50: Receipt construction logic looks correct.The mapping from
MorphTxTypetoMorphReceiptvariants is well-structured. The distinction betweenL1Msg(using bareReceipt) and other types (usingMorphTransactionReceipt::new(inner)) correctly reflects their different receipt structures.The TODO for L1 fee calculation is noted—this will be important for accurate fee tracking in receipts.
Would you like me to open an issue to track the L1 fee calculation implementation?
crates/evm/src/block/curie.rs (1)
62-64: Add logging or assertion to detect if transitions are silently discarded during Curie fork application.The transitions are silently discarded if
state.transition_stateisNone, which could mask configuration issues where the fork appears to apply successfully but doesn't persist state changes. While the test explicitly uses.with_bundle_update()to enable transition tracking, production State initialization should be verified to ensure transitions are always captured.Consider adding a debug log or documenting this requirement:
Suggested debug logging
// Add transition to state if let Some(s) = state.transition_state.as_mut() { s.add_transitions(vec![(L1_GAS_PRICE_ORACLE_ADDRESS, transition)]); + } else { + tracing::debug!(target: "morph::evm", "Curie fork applied but transition_state is None; changes may not persist"); }Alternatively, document that
apply_curie_hard_forkrequires a State initialized withwith_bundle_update()to function correctly.
Summary by CodeRabbit
New Features
Refactor
✏️ Tip: You can customize this high-level summary in your review settings.