🤖 Kimi Code Review

⚠️ Warning: Diff was truncated from 16151 to 10000 lines. Some changes were not reviewed.

This is a large PR introducing EIP-8025 (Execution Layer Triggerable Proofs) and Native Rollups support. The changes span the CLI, blockchain engine, L2 sequencer, RPC layer, and zkVM guest programs.

Critical Issues

1. SSH Git Dependency Blocks External Builds

File: Cargo.toml (multiple crates)

The SSZ library is referenced via SSH:

ssz = { git = "ssh://git@github.com/lambdaclass/libssz", optional = true }

Problem: This requires SSH authentication and will fail for external contributors or CI without configured SSH keys. If libssz is private, this is a complete blocker for open-source builds.

Fix: Change to HTTPS:

ssz = { git = "https://github.com/lambdaclass/libssz", optional = true }

Or publish the crate to crates.io.

2. Hardcoded Private Keys in Source

File: cmd/ethrex/l2/deployer.rs, lines 1780–1790

let test_private_keys = [
    "0x59c6995e998f97a5a0044966f0945389dc9e86dae88c7a8412f4603b6b78690d",
    // ...
];

Problem: Hardcoded test keys are a security anti-pattern. Even for testnets, these should be loaded from fixture files or environment variables.

Fix: Move to fixtures/keys/ and load at runtime, or generate deterministically from a seed phrase.

3. Unwrap/Expect in Critical Path

File: crates/l2/sequencer/native_rollup/l1_advancer.rs, line 394

let original_hash = block_header.compute_block_hash(&NativeCrypto);
// ...
let reconstructed_hash = reconstructed_block.hash();
assert_eq!(original_hash, reconstructed_hash); // In debug block

Problem: While this specific instance is in a debug-only block (lines 156–181), similar patterns exist in production code:

File: crates/guest-program/src/l1/program.rs, line 76

let base_fee_bytes = payload.base_fee_per_gas[..8]
    .try_into()
    .map_err(|_| "base_fee_per_gas conversion")?; // Should handle error properly

Fix: Use proper error propagation instead of expect/unwrap in guest program code.

Security & Correctness

4. Integer Overflow Risk in Gas Calculations

File: crates/l2/sequencer/native_rollup/block_producer.rs, line 320

let next_gas = cumulative_gas.saturating_add(msg.gas_limit);
if next_gas > self.config.block_gas_limit { ... }

Issue: saturating_add prevents overflow, but the comparison logic should ensure msg.gas_limit alone doesn't exceed the block limit before adding.

Fix: Add check:

if msg.gas_limit > self.config.block_gas_limit {
    warn!("L1 message gas limit exceeds block limit, skipping");
    continue;
}

5. TCP Server Without Connection Limits

File: crates/blockchain/proof_engine/coordinator.rs, lines 95–110

The L1 ProofCoordinator accepts TCP connections in an infinite loop without rate limiting or connection timeouts.

Fix: Add connection limits and timeouts:

tokio::time::timeout(Duration::from_secs(30), listener.accept())

6. SSZ Round-trip Verification in Production

File: crates/l2/sequencer/native_rollup/l1_advancer.rs, lines 156–181

The advance_next_block function performs full SSZ serialization, deserialization, and block reconstruction for debugging before submitting to L1.

Issue: This adds significant overhead (merkle tree operations, RLP encoding) to every block submission.

Fix: Gate behind #[cfg(debug_assertions)] or a verbose-validation feature flag.

Architecture & Maintainability

7. Feature Flag Inconsistency

The stateless-validation feature is added to many crates, but some dependencies aren't properly gated:

File: crates/l2/Cargo.toml

[dependencies]
ssz = { git = "...", optional = true }
# ...
k256 = { workspace = true, optional = true }

But in crates/l2/sequencer/native_rollup/types.rs, the k256 usage isn't feature-gated properly (though it compiles due to re-exports).

8. Duplicate Merkle Tree Implementation

File: crates/common/merkle_tree.rs (new)
File: crates/l2/common/src/merkle_tree.rs (now re-exports)

Good refactoring to move the merkle tree to common, but ensure the commutative hash property is documented:

/// Commutative Keccak256 hash: H(a, b) == H(b, a).
/// Sorts inputs so the smaller value comes first, matching OpenZeppelin's
/// `_hashPair` in MerkleProof.sol.

9. Guest Program Input Format Breaking Change

File: crates/guest-program/bin/*/src/main.rs

The guest programs now branch on stateless-validation feature to choose between rkyv and SSZ deserialization. This is a breaking change for existing proof infrastructure.

Suggestion: Document the input format change in the PR description and ensure the coordinator sends the correct format based on feature flags.

Minor Issues

10. Typo in Log Message

File: cmd/ethrex/l2/command.rs, line 300

"Succesfully unpaused contract"

Should be "Successfully".

11. Unused Import

File: crates/l2/sequencer/native_rollup/l1_watcher.rs, line 10

use ethrex_crypto::keccak::keccak_hash;

Imported but not used (the keccak function from ethrex_common is used instead).

12. Missing Documentation on SSZ Limits

File: crates/common/types/stateless_ssz.rs

The constants like MAX_TRANSACTIONS = 1_048_576 should reference the specific EIP or consensus spec section (Electra/Amsterdam).

Positive Notes

1. Good separation of concerns: The native rollup logic is cleanly separated into block_producer, l1_watcher, and l1_advancer modules following the GenServer pattern.
2. Comprehensive SSZ types: The stateless_ssz.rs module properly implements the consensus spec containers with correct hash tree root implementations.
3. Round-trip testing: The inclusion of SSZ round-trip tests in l1_advancer.rs (even if it should be debug-only) shows attention to serialization correctness.
4. Refactoring: Moving ethrex-prover to a shared crate and creating ethrex-l2-prover is a good architectural improvement.

Summary

Must fix before merge:

1. Change SSH dependency to HTTPS
2. Remove hardcoded private keys
3. Gate expensive debug verification behind debug_assertions

Should fix:
4. Add connection limits to TCP coordinator
5. Fix unwrap/expect in guest programs
6. Add overflow checks for gas limit math

The stateless validation logic appears correct and follows the EIP-8025 specification, but the infrastructure changes (SSH deps, hardcoded keys) are blockers.

Automated review by Kimi (Moonshot AI) · kimi-k2.5 · custom prompt

Lines of code report

Total lines added: 2728
Total lines removed: 0
Total lines changed: 2728

+---------------------------------------------------------------+-------+------+
| File                                                          | Lines | Diff |
+---------------------------------------------------------------+-------+------+
| ethrex/cmd/ethrex/build_l2.rs                                 | 470   | +23  |
+---------------------------------------------------------------+-------+------+
| ethrex/cmd/ethrex/l2/command.rs                               | 666   | +22  |
+---------------------------------------------------------------+-------+------+
| ethrex/cmd/ethrex/l2/deployer.rs                              | 1794  | +262 |
+---------------------------------------------------------------+-------+------+
| ethrex/cmd/ethrex/l2/initializers.rs                          | 452   | +75  |
+---------------------------------------------------------------+-------+------+
| ethrex/cmd/ethrex/l2/mod.rs                                   | 12    | +2   |
+---------------------------------------------------------------+-------+------+
| ethrex/cmd/ethrex/l2/options.rs                               | 1232  | +128 |
+---------------------------------------------------------------+-------+------+
| ethrex/crates/blockchain/blockchain.rs                        | 2506  | +16  |
+---------------------------------------------------------------+-------+------+
| ethrex/crates/blockchain/stateless.rs                         | 103   | +103 |
+---------------------------------------------------------------+-------+------+
| ethrex/crates/common/types/block_execution_witness.rs         | 641   | +71  |
+---------------------------------------------------------------+-------+------+
| ethrex/crates/common/types/stateless_ssz.rs                   | 374   | +374 |
+---------------------------------------------------------------+-------+------+
| ethrex/crates/guest-program/src/l1/mod.rs                     | 12    | +2   |
+---------------------------------------------------------------+-------+------+
| ethrex/crates/l2/common/src/messages.rs                       | 208   | +4   |
+---------------------------------------------------------------+-------+------+
| ethrex/crates/l2/l2.rs                                        | 13    | +2   |
+---------------------------------------------------------------+-------+------+
| ethrex/crates/l2/networking/rpc/l2/mod.rs                     | 7     | +2   |
+---------------------------------------------------------------+-------+------+
| ethrex/crates/l2/networking/rpc/l2/native_withdrawal_proof.rs | 129   | +129 |
+---------------------------------------------------------------+-------+------+
| ethrex/crates/l2/networking/rpc/rpc.rs                        | 260   | +4   |
+---------------------------------------------------------------+-------+------+
| ethrex/crates/l2/sdk/src/sdk.rs                               | 1240  | +6   |
+---------------------------------------------------------------+-------+------+
| ethrex/crates/l2/sequencer/mod.rs                             | 279   | +2   |
+---------------------------------------------------------------+-------+------+
| ethrex/crates/l2/sequencer/native_rollup/block_producer.rs    | 364   | +364 |
+---------------------------------------------------------------+-------+------+
| ethrex/crates/l2/sequencer/native_rollup/l1_advancer.rs       | 520   | +520 |
+---------------------------------------------------------------+-------+------+
| ethrex/crates/l2/sequencer/native_rollup/l1_watcher.rs        | 239   | +239 |
+---------------------------------------------------------------+-------+------+
| ethrex/crates/l2/sequencer/native_rollup/mod.rs               | 79    | +79  |
+---------------------------------------------------------------+-------+------+
| ethrex/crates/l2/sequencer/native_rollup/types.rs             | 25    | +25  |
+---------------------------------------------------------------+-------+------+
| ethrex/crates/vm/backends/levm/mod.rs                         | 2264  | +54  |
+---------------------------------------------------------------+-------+------+
| ethrex/crates/vm/backends/levm/tracing.rs                     | 78    | +13  |
+---------------------------------------------------------------+-------+------+
| ethrex/crates/vm/backends/mod.rs                              | 260   | +21  |
+---------------------------------------------------------------+-------+------+
| ethrex/crates/vm/levm/runner/src/main.rs                      | 280   | +1   |
+---------------------------------------------------------------+-------+------+
| ethrex/crates/vm/levm/src/db/guest_program_state_db.rs        | 67    | +67  |
+---------------------------------------------------------------+-------+------+
| ethrex/crates/vm/levm/src/db/mod.rs                           | 147   | +2   |
+---------------------------------------------------------------+-------+------+
| ethrex/crates/vm/levm/src/execute_precompile.rs               | 74    | +74  |
+---------------------------------------------------------------+-------+------+
| ethrex/crates/vm/levm/src/hooks/l2_hook.rs                    | 676   | +1   |
+---------------------------------------------------------------+-------+------+
| ethrex/crates/vm/levm/src/lib.rs                              | 25    | +5   |
+---------------------------------------------------------------+-------+------+
| ethrex/crates/vm/levm/src/opcode_handlers/system.rs           | 950   | +1   |
+---------------------------------------------------------------+-------+------+
| ethrex/crates/vm/levm/src/precompiles.rs                      | 1204  | +24  |
+---------------------------------------------------------------+-------+------+
| ethrex/crates/vm/levm/src/vm.rs                               | 565   | +7   |
+---------------------------------------------------------------+-------+------+
| ethrex/crates/vm/lib.rs                                       | 15    | +2   |
+---------------------------------------------------------------+-------+------+
| ethrex/crates/vm/tracing.rs                                   | 45    | +2   |
+---------------------------------------------------------------+-------+------+

🤖 Codex Code Review

1. High: the zkevm fixture detection is too broad and will also rewrite the normal Amsterdam EF tests. The check in test_runner.rs:42 keys off path.contains("for_amsterdam"), but both the standard Amsterdam target and the zkevm target unpack into that same directory in Makefile:47 and Makefile:53. That means ordinary Amsterdam fixtures will run under ZKEVM_AMSTERDAM_CONFIG, skip run_two_pass_parallel, and stop exercising the actual Amsterdam rules this harness is supposed to cover.

2. High: zkevm_supplementary_validation() can turn false accepts into passing tests. In test_runner.rs:176, an accepted block with expectException is treated as “correctly rejected” if test_runner.rs:216 returns any error. But that helper currently rejects any non-empty block_access_list_hash, without checking the expected exception or the actual computed BAL. So if the Osaka pipeline wrongly accepts an invalid zkevm Amsterdam block for some unrelated reason, the harness can still pass the test just because the header carries a non-empty BAL hash.

3. High: the stateless zkevm path re-enables Amsterdam rules even though the stateful path explicitly downgrades these fixtures to Osaka. test_runner.rs:666 passes test.network.chain_config() into witness conversion; that config is stored in the witness at execution_witness.rs:94 and block_execution_witness.rs:226, and then used for pre-execution validation in execution.rs:62. So test-stateless-zkevm is not validating under the same fork rules as the preceding run() path, which defeats the workaround this PR is adding.

4. High: the new SSZ stateless validator claims to run at the “latest fork (Amsterdam)”, but the synthesized ChainConfig only activates through Prague. See stateless.rs:297 and stateless.rs:314. ChainConfig does have explicit Osaka/Amsterdam activation checks and fork selection in genesis.rs:356 and genesis.rs:439, so this path will silently skip Osaka/Amsterdam-era validation in EXECUTE/EIP-8025/guest mode.

Static review only; I did not run the test suite.

Automated review by OpenAI Codex · gpt-5.4 · custom prompt

Benchmark Results Comparison

Benchmark Results: Factorial

🤖 Claude Code Review

PR #6418 Review — Unify stateless validation: EIP-8025 + native rollups

This is a large, well-scoped PR with solid architectural thinking (the StatelessValidator trait for cycle-breaking is a clean design). The review focuses on correctness and security issues. The PR is marked as DRAFT, which is appropriate given the open items below.

Critical Issues

1. chain_id verification disabled in NativeRollup.sol

advance() (line 151) has the chain ID check commented out with a TODO:

// TODO: proper SSZ uint64 little-endian decoding
// require(provenChainId == chainId, "chain_id mismatch");

This means the contract accepts EXECUTE precompile output from any chain's execution — a replay attack vector. If the SSZ decoding is wrong (wrong offset or endianness), fix the decoding; don't skip the check. The assembly at line 144 is also reading from offset 65 (add(result, 65)) which loads bytes starting at position 33 in the result — correct for little-endian uint64 at bytes [33..41], but mload returns 32 bytes and the high 24 bytes won't be zero for typical chain IDs, making the comparison to chainId (which is a uint256) always fail. The fix requires masking: provenChainId := and(shr(192, mload(add(result, 65))), 0xffffffffffffffff).

2. FINALITY_DELAY = 0 hardcoded as immutable

NativeRollup.sol constructor (line 78):

FINALITY_DELAY = 0;

Since FINALITY_DELAY is immutable, this can never be changed after deployment. Withdrawals are claimable immediately after advance() commits a state root. There is zero fraud-proof window. For a production system this is a critical vulnerability — a buggy or malicious advancer can drain the bridge instantly. Even for a PoC this should be either configurable via constructor parameter or set to a non-zero value with a comment explaining the intentional tradeoff.

3. advance() has no access control

Anyone can submit arbitrary SSZ input to advance(). If there is any precompile edge case that returns successful_validation = false but the overall call still succeeds (e.g., the precompile is not available at a given fork), the state is still updated via _newBlockHash/_newStateRoot parameters that are passed in untrusted by the caller. The contract trusts the precompile output only — but _newBlockHash and _newStateRoot come from the caller and are not verified against the precompile output. An attacker who gets require(uint8(result[32]) == 1) to pass can submit any block hash and state root.

High Issues

4. Double SSZ decode in execute_precompile.rs

run_execute calls extract_gas_used(calldata) and then validate_l2_constraints(calldata), each independently deserializing the full SSZ input. At ~tens of kilobytes for a real witness, this doubles deserialization cost. Decode once, pass the struct to both functions.

5. processL1Message silently swallows subcall failures

L2Bridge.sol processL1Message() does:

(bool innerSuccess, ) = _to.call{value: _value, gas: _gasLimit}(_data);
// (no check on innerSuccess)

If the subcall fails, l1MessageNonce is still incremented and the ETH (which was sent from L1 bridge escrow) is consumed but not delivered. The message cannot be replayed. Users lose their deposited ETH. At minimum, emit a failure event so off-chain indexers can detect it. The design doc should explicitly justify this behavior — it may be intentional for DOS-resistance, but it needs documentation.

6. EXECUTE precompile is_precompile ignores fork

precompiles.rs line 271:

#[cfg(feature = "stateless-validation")]
if *address == EXECUTE.address {
    return true;
}

The active_since_fork: Paris constant is set but never checked in the dispatch path. Standard precompiles use precompiles_for_fork(fork).any(...) which does filter by active_since_fork. The EXECUTE precompile skips this filter entirely, making it active on all forks whenever the feature flag is set. This should check fork >= EXECUTE.active_since_fork.

Medium Issues

7. Gas model: charging L2 gas_used as L1 gas

The EXECUTE precompile charges gas_used from the L2 execution payload directly against the L1 call's gas_remaining. A 30M-gas L2 block would require a caller to supply at least 30M gas for the L1 transaction. This might be intentional by the spec, but there's no cap or minimum — there's also no intrinsic cost for the precompile itself (e.g., calldata cost, decoding cost). Document this explicitly.

8. _burnGas uses a busy loop

function _burnGas(uint256 amount) private view {
    uint256 startingGas = gasleft();
    while (startingGas - gasleft() < amount) {}
}

Gas costs of individual EVM opcodes change with hard forks. This loop's cost per iteration is not stable. If a future EIP changes the cost of GAS/SUB/LT, the effective burn amount changes. Use a well-known assembly idiom or simply require that callers pass sufficient ETH payment; burning gas is an unusual cost model.

9. Inline extension nodes unsupported in MPTProof.sol

processInlineNode() (line ~118):

require(isLeaf, "MPT: inline ext not supported");

Inline extension nodes are valid in Ethereum's MPT and can appear in small tries. On a fresh L2 with few accounts (e.g., genesis state), this will cause proof verification to revert for valid state proofs. This needs to be implemented before the withdrawal path can be considered production-ready.

10. ssz_witness_to_internal forces all forks to timestamp/block 0

chain_config: ChainConfig {
    chain_id: chain_config.chain_id,
    homestead_block: Some(0),
    // ... all forks at 0

The comment says "stateless validation always runs at the latest fork." But the chain_config in the SSZ input only carries chain_id. This means the actual fork activation rules are silently ignored. If a block is validated that was produced under different fork rules (e.g., pre-Prague), the validation will run Prague rules regardless and may produce incorrect results. At minimum, this assumption should be validated against the block's timestamp/number.

Low Issues / Nitpicks

11. chainId: 1 in native_l2.json

fixtures/genesis/native_l2.json uses chainId: 1 (Ethereum mainnet). This will cause signed transactions with v-field EIP-155 protection to be replayable on mainnet if private keys are reused. Use a clearly test-only chain ID (e.g., 31337 or a value in the EIP-2294 reserved range).

12. StatelessValidator trait not feature-gated

The trait is defined unconditionally in levm/src/lib.rs but the execute_precompile module using it is gated on stateless-validation. This causes the vm.rs field stateless_validator: Option<&'a dyn StatelessValidator> to always exist, adding an extra pointer to VM even in non-rollup builds. Gate the trait and the field behind the same feature flag.

13. GuestProgramStateDb Mutex contention not needed

The EXECUTE precompile runs inside a staticcall — it's single-threaded by nature. Mutex<GuestProgramState> adds unnecessary overhead. Use RefCell if interior mutability is needed, or restructure GuestProgramState methods to take &mut self with a unique borrow.

14. validate_l2_constraints checks only the first byte for blob transactions

if let Some(&0x03) = tx_bytes.iter().next() {

This is correct for blob transactions (type byte 0x03) but doesn't guard against other future typed transactions that may also be forbidden on L2. More importantly, this check only looks at the first byte — an empty tx_bytes would pass silently. This should be defensive: require tx_bytes.len() > 0 or use tx_bytes.first().

15. SSZ round-trip test doesn't cover the witness conversion

ssz_round_trip.rs tests BlockHeader field preservation but not ssz_witness_to_internal / internal_witness_to_ssz. This is the most complex conversion path and most likely to have field-level bugs (e.g., base_fee_per_gas 8-of-32-byte encoding). A unit test round-tripping an ExecutionWitness with non-trivial account and storage state would provide much higher confidence.

16. _newBlockHash / _newStateRoot are caller-supplied in advance() but never verified against precompile output

The precompile returns new_payload_request_root (the SSZ hash tree root of the input, not the new block hash or state root). The contract uses _newBlockHash and _newStateRoot as passed by the caller, not as derived from the precompile response. An honest advancer supplies the correct values, but nothing enforces this on-chain. Either include the expected hash/root in the precompile output, or verify them independently.

Summary

The unification architecture and trait design are solid. The implementation needs hardening before this merges:

• The disabled chain_id check (Point 1) and zero finality delay (Point 2) are blockers for any real deployment.
• Points 3, 5, and 6 represent correctness gaps.
• The double SSZ decode (Point 4) is a straightforward fix.
• Points 9 and 16 are functional limitations that affect correctness of the withdrawal path.

Automated review by Claude (Anthropic) · sonnet · custom prompt

🤖 Kimi Code Review

Overall Assessment: This is a substantial PR introducing Native Rollup (EIP-8079) support as an experimental feature. The code is generally well-structured, properly feature-gated, and includes comprehensive documentation and integration tests. However, there are a few security and correctness concerns that should be addressed before production use.

Critical Security Concerns

1. L1 Contract Missing State Root Verification (NativeRollup.sol)
The advance() function accepts _newBlockHash and _newStateRoot as separate parameters from the SSZ input, but does not verify they match the values inside the _sszStatelessInput payload:

// crates/l2/contracts/src/nativeRollup/l1/NativeRollup.sol:194-210
blockHash = _newBlockHash;  // Arbitrary value accepted!
stateRoot = _newStateRoot;  // Arbitrary value accepted!

While the EXECUTE precompile validates that the SSZ input produces a valid state transition, the contract does not constrain the claimed state root to match the one in the SSZ payload. An attacker could submit a valid proof for block N but claim the state root of block N+1.

Recommendation: Either extract and verify the state root from the SSZ input within the contract, or include the resulting state root in StatelessValidationResult and verify it on-chain.

2. MPT Proof Verification Edge Cases (MPTProof.sol)
The processInlineNode function uses inline assembly for memory operations:

// crates/l2/contracts/src/nativeRollup/l1/MPTProof.sol:130-132
assembly ("memory-safe") { 
    prefix := shr(252, mload(add(add(content, 32), epStart))) 
}

While marked memory-safe, the subsequent nibble extraction logic assumes epStart is within bounds. Ensure epLen validation occurs before this assembly block to prevent out-of-bounds reads.

3. Gas Charging Precondition
In the EXECUTE precompile, gas is charged based on the claimed gas_used before execution validates the witness:

// crates/vm/levm/src/execute_precompile.rs:62-65
increase_precompile_consumed_gas(
    input.new_payload_request.execution_payload.gas_used,
    gas_remaining,
)?;

This is acceptable since execution follows immediately and will fail if the claim is fraudulent, but ensure the gas charging cannot overflow gas_remaining (the helper appears to handle this).

Code Correctness & Rust Best Practices

4. SSZ Limits Enforcement
The SSZ types define limits (e.g., MAX_WITNESS_NODES = 1_048_576), but ensure these are enforced during deserialization:

// crates/common/types/stateless_ssz.rs:259-260
const MAX_W


---
*Automated review by Kimi (Moonshot AI) · kimi-k2.5 · custom prompt*

🤖 Claude Code Review

Now I have a thorough picture of the entire PR. Let me produce the review.

PR #6418 Review: feat(l2): add EIP-8079 native rollups

Overview

This PR implements EIP-8079 (native rollups) on top of the EIP-8025 stateless-validation infrastructure. It adds:

• The EXECUTE precompile (0x0101) in LEVM behind a experimental-devnet feature flag
• NativeRollup.sol, L2Bridge.sol, MPTProof.sol contracts
• Three L2 actors: NativeBlockProducer, NativeL1Watcher, NativeL1Advancer
• A StatelessValidator trait in ethrex-levm with StatelessExecutor implemented in ethrex-blockchain to break the dependency cycle
• SSZ-based witness encoding/decoding in ethrex-common

The dependency-inversion approach for the cycle is well-documented in the PR and in code comments. Overall the design is architecturally sound for a PoC, but there are several correctness and security issues.

Critical Issues

1. advance() stores caller-supplied hashes without validating them against the EXECUTE result

File: crates/l2/contracts/src/nativeRollup/l1/NativeRollup.sol:2789-2825

The EXECUTE precompile validates the SSZ input and returns:

new_payload_request_root (32 B) | successful_validation (1 B) | chain_config (8 B)

The contract checks successful_validation == 1 and chain_id matches — but then stores the caller-supplied _newBlockHash and _newStateRoot verbatim:

blockHash = _newBlockHash;   // ← taken from caller, never cross-validated
stateRoot = _newStateRoot;   // ← same
stateRootHistory[newBlockNumber] = _newStateRoot;

An attacker can submit a valid SSZ proof for a real block but supply an arbitrary _newStateRoot, which is then used to verify future withdrawal claims via stateRootHistory. This allows fake withdrawal proofs against a malicious state root.

Fix: The StatelessValidationResult must include the validated block_hash and state_root from the execution_payload, and the contract must cross-validate them:

pub struct SszStatelessValidationResult {
    pub new_payload_request_root: [u8; 32],
    pub block_hash: [u8; 32],   // ← add
    pub state_root: [u8; 32],   // ← add
    pub successful_validation: bool,
    pub chain_config: SszChainConfig,
}

2. advance() does not verify the SSZ block number or parent hash match on-chain state

File: NativeRollup.sol:2789-2825

The contract advances blockNumber → blockNumber + 1 and stores blockHash = _newBlockHash, but never checks:

• That the SSZ execution_payload.block_number == blockNumber + 1
• That the SSZ execution_payload.parent_hash == blockHash

A valid SSZ proof for a different block position (e.g., a block built on a stale parent, or a different chain fork) would be accepted. Combined with issue 1, this means a valid SSZ for block N can be submitted when the contract is at block M ≠ N−1, provided the _newBlockHash/_newStateRoot values satisfy later checks.

Fix: Include parent_hash and block_number in the result (or verify them from the SSZ input inline in the contract), and require them to match:

require(result_block_number == blockNumber + 1, "wrong block number");
require(result_parent_hash == blockHash, "wrong parent hash");

High Severity

3. Double SSZ decoding

Files: crates/vm/levm/src/execute_precompile.rs:run_execute, crates/blockchain/stateless.rs:StatelessExecutor::verify

run_execute decodes SszStatelessInput from calldata to charge gas and validate L2 constraints, then passes the original raw calldata bytes to validator.verify(calldata), which decodes the same SSZ a second time inside StatelessExecutor::verify. For large blocks with many transactions and witness nodes, this doubles the deserialization overhead.

Fix: Either thread the already-decoded struct through the trait, or restructure so decoding happens exactly once (e.g., split the trait method into decode + verify_decoded).

4. _burnGas has no cap — trivial DoS vector

File: NativeRollup.sol:2754-2278

function sendL1Message(address _to, uint256 _gasLimit, bytes calldata _data) external payable {
    _burnGas(_gasLimit);
    ...
}

There is no maximum on _gasLimit. A caller who passes _gasLimit = 25_000_000 burns 25M L1 gas in a single call. There is no economic disincentive beyond the ETH gas fee. This allows griefing of L1 block capacity or artificial inflation of relayer transaction costs. At minimum, add:

require(_gasLimit <= l2GasLimit, "gasLimit exceeds L2 block limit");

5. is_precompile returns true for EXECUTE regardless of VMType or fork

File: crates/vm/levm/src/precompiles.rs:5724-5728

#[cfg(feature = "experimental-devnet")]
if *address == EXECUTE.address {
    return true;  // ignores vm_type and fork
}

With experimental-devnet enabled, address 0x0101 is treated as a precompile in all VMs — including L2 VMs and the inner VM created inside verify_inner for stateless block execution. When the inner VM encounters 0x0101, it has stateless_validator = None and returns VMError::Internal(...). Depending on how the CALL opcode handler propagates VMError::Internal (non-recoverable vs. sub-call failure), this could cause the entire L2 block validation to abort with an internal error rather than gracefully reverting the transaction. If VMError::Internal is non-recoverable, any L2 transaction that calls 0x0101 makes the L2 block impossible to validate.

Fix: Restrict the EXECUTE precompile to VMType::L1 only:

if *address == EXECUTE.address && matches!(vm_type, VMType::L1) {
    return true;
}

Medium Severity

6. L1 watcher scans from block 0 on every restart

File: crates/l2/sequencer/native_rollup/l1_watcher.rs:4549

last_block_fetched: U256::zero(),

On every process restart, the watcher re-scans all L1 history from genesis looking for L1MessageRecorded events. For a long-running deployment this becomes increasingly expensive. The last_block_fetched cursor must be persisted (to the Store or a config file), or a configurable start_block parameter must be provided (e.g., the contract deployment block number).

7. FINALITY_DELAY = 0 — no reorg protection on withdrawals

File: NativeRollup.sol:2749

FINALITY_DELAY = 0;

Withdrawals can be claimed immediately with no finality buffer. In an L1 reorg scenario, a block that was used to generate a withdrawal proof could be reverted, while the L1 withdrawal has already succeeded. For a PoC this may be acceptable, but it should be explicitly documented as a TODO in the contract (not just the PR description) since the field infrastructure exists.

8. ABI offset in parse_l1_message_recorded is silently skipped

File: crates/l2/sequencer/native_rollup/l1_watcher.rs:4692-4698

// data[64..96] = ABI offset for `bytes data` (skip — we know it points to 96)
// data[96..128] = byte length of `data`

The ABI offset at bytes 64-96 is ignored without verification. If the event signature ever gains an additional non-indexed field (or if an RPC relay encodes the data differently), the offset could be non-96 and the watcher would silently parse garbage. Add at minimum a debug assertion:

let offset = U256::from_big_endian(&data[64..96]);
debug_assert_eq!(offset, U256::from(96), "unexpected ABI offset in L1MessageRecorded");

9. verify_stateless_new_payload swallows errors silently

File: crates/blockchain/stateless.rs:1122-1127

let successful = match verify_inner(...) {
    Ok(()) => true,
    Err(e) => {
        tracing::error!("stateless validation failed: {e}");
        false
    }
};

Internal errors (OOM, thread panics, serialization bugs) produce successful = false identically to genuine validation failures. A persistent internal bug would produce a stream of silent failures with no signal that they are software errors rather than genuinely invalid blocks. Consider adding a separate error variant for infrastructure failures vs. consensus failures.

10. MPTProof.sol does not support inline extension nodes

File: crates/l2/contracts/src/nativeRollup/l1/MPTProof.sol:2528

require(isLeaf, "MPT: inline ext not supported");

The Ethereum MPT specification allows inline extension nodes (nodes whose RLP encoding is ≤32 bytes, embedded directly in a parent branch). If a withdrawal proof passes through an inline extension node in the L2 state trie, claimWithdrawal will unconditionally revert with "MPT: inline ext not supported." This is a correctness limitation that could make specific withdrawals permanently unclaimable.

Low / Style

11. Duplicate step 7 comment in block_producer.rs

File: crates/l2/sequencer/native_rollup/block_producer.rs:3509, 3512

// 7. Finalize payload (compute state root, receipts root, etc.)
self.blockchain.finalize_payload(&mut context)?;

// 7. Store block      ← should be 8
let block = context.payload;

12. Hardcoded elasticity_multiplier: 2 should come from chain config

File: block_producer.rs:3489

elasticity_multiplier: 2, // EIP-1559 default

ELASTICITY_MULTIPLIER is already a constant in ethrex-common. Use it or derive it from chain_config for consistency with the rest of the codebase.

13. StatelessValidator trait is unconditional but its only use is feature-gated

File: crates/vm/levm/src/lib.rs:5679

The StatelessValidator trait declaration is outside any #[cfg(feature = "experimental-devnet")] gate, so it exists on all builds. The stateless_validator: Option<&'a dyn StatelessValidator> field also exists on every VM even in builds where no implementation exists. This adds a little unnecessary size/complexity. Unless you intend for downstream crates to implement the trait without the feature flag, move both behind the feature gate.

14. Gas charged before L2 constraint validation

File: crates/vm/levm/src/execute_precompile.rs:5599-5606

increase_precompile_consumed_gas(
    input.new_payload_request.execution_payload.gas_used,
    gas_remaining,
)?;
// Validate L2-specific constraints
validate_l2_constraints(&input)?;

Gas equal to the L2 gas_used is charged before constraint validation. If an L2 block fails the constraint check (e.g., blob transactions present), the L1 caller still pays gas_used gas units. This is likely intentional, but should be documented as a design decision.

Positive Notes

• The StatelessValidator dependency-inversion approach is well-designed and clearly documented, with the trade-offs explicitly called out in the PR description and code comments.
• The actor/GenServer model for NativeL1Watcher, NativeBlockProducer, and NativeL1Advancer is clean and consistent with the existing L2 sequencer architecture.
• The internal_witness_to_ssz conversion and the build_ssz_stateless_input function are thorough in their handling of the trie-to-flat-bytes translation.
• Feature-gating via experimental-devnet and the single-precompile early-dispatch in precompiles.rs is clean and avoids disrupting the existing constant precompile table.
• parse_l1_message_recorded has thorough bounds checking and correct ABI head/tail decoding for the dynamic bytes field.

Summary

Issues 1 and 2 are the most critical: advance() accepts caller-supplied block hash and state root without binding them to what the EXECUTE precompile actually validated. These must be fixed before deploying to any shared test environment, as they allow arbitrary state root substitution. Issues 3–5 are important for correctness and safety. Issues 6–10 are worth fixing before production use.

Automated review by Claude (Anthropic) · sonnet · custom prompt

🤖 Codex Code Review

1. NativeRollup.sol lines 120-156 do not anchor the proven block to the contract’s current head, and then write caller-supplied _newBlockHash / _newStateRoot directly into storage. Because advance() never checks that the validated payload extends the current blockHash and claimWithdrawal() later trusts stateRootHistory (same file), a caller can submit any self-consistent stateless proof from another parent and corrupt the withdrawal root on L1.

2. NativeRollup.sol only range-checks _l1MessagesCount; it never binds that count to the payload’s parent_beacon_block_root. You already have _computeMerkleRoot() available (same file), but advance() does not compare it against the queued pendingL1Messages[startIdx..], so a valid block with zero or different bridge messages can still skip pending L1 messages by passing a larger count.

3. block_producer.rs and block_producer.rs reuse msg.gas_limit as both the outer relayer transaction gas limit and the block-packing budget. But L2Bridge.processL1Message() documents that field as the forwarded subcall gas (L2Bridge.sol), so proof verification, nonce/storage writes, and log emission are unbudgeted; honest L1->L2 messages will start reverting once proofs or calldata are non-trivial.

4. block_producer.rs still includes reverted relayer txs in the block, while l1_advancer.rs counts consumed L1 messages by tx.sender == relayer_address. That means an under-gassed / bad-proof / duplicate relayer tx still increments _l1MessagesCount on L1 and permanently advances l1MessageIndex past messages that were never processed on L2.

5. The experimental-devnet path looks source-incompatible as written: l1_advancer.rs still initializes removed ExecutionPayload fields and treats execution_requests as SszList, execute_precompile.rs calls .is_empty() on it, but stateless_ssz.rs / stateless_ssz.rs define ExecutionPayload without those fields and NewPayloadRequest.execution_requests as an ExecutionRequests struct. This should be fixed before the feature can build.

Beyond those blockers, the new RPC proof helper and the stateless-validator plumbing look directionally reasonable.

Automated review by OpenAI Codex · gpt-5.4 · custom prompt

Greptile Summary

This PR adds EIP-8079 native rollup support behind the experimental-devnet feature flag, introducing NativeRollup.sol, L2Bridge.sol, three new L2 actors (NativeBlockProducer, NativeL1Watcher, NativeL1Advancer), and an EXECUTE precompile in LEVM that delegates to verify_stateless_new_payload via a StatelessValidator trait to break a crate dependency cycle.

Two security issues in NativeRollup.advance() need attention before this is used beyond an isolated devnet:

• _newBlockHash, _newStateRoot, and _l1MessagesCount are stored directly from caller arguments without being verified against the EXECUTE precompile result. The returned new_payload_request_root commits to the actual block hash and state root but is never cross-checked.
• The submitted block's parent_hash (inside the SSZ input) is not compared against the on-chain blockHash, allowing non-sequential or forked block commitments.

Confidence Score: 3/5

Safe to merge as an experimental devnet PoC, but two structural security issues in NativeRollup.advance() must be resolved before the on-chain state commitments can be trusted.

Two P1 security findings in NativeRollup.sol — caller-supplied state root/block hash stored without verification against the EXECUTE proof, and no parent-hash continuity check — mean the on-chain L2 state can be set arbitrarily. Feature is gated behind experimental-devnet which limits blast radius, but the issues are structural.

crates/l2/contracts/src/nativeRollup/l1/NativeRollup.sol — the two P1 findings in advance() are the primary concern.

Sequence Diagram

sequenceDiagram
    participant L1W as NativeL1Watcher
    participant BP as NativeBlockProducer
    participant Store as L2 Store
    participant ADV as NativeL1Advancer
    participant L1 as NativeRollup.sol (L1)
    participant EXEC as EXECUTE Precompile

    L1W->>L1: poll L1MessageRecorded events
    L1W->>BP: EnqueueL1Messages(msgs)
    BP->>BP: build relayer txs (Merkle proofs)
    BP->>Store: produce & store L2 block
    ADV->>Store: fetch next unadvanced block + witness
    ADV->>ADV: build_ssz_stateless_input()
    ADV->>L1: advance(l1MsgCount, sszInput, newBlockHash, newStateRoot)
    L1->>EXEC: staticcall(sszInput)
    EXEC->>EXEC: decode SSZ, validate L2 constraints, charge gas_used
    EXEC->>EXEC: StatelessValidator::verify()
    EXEC-->>L1: SszStatelessValidationResult
    L1->>L1: store blockHash/stateRoot from caller args (not bound to proof)

This is a comment left during a code review.
Path: crates/l2/contracts/src/nativeRollup/l1/NativeRollup.sol
Line: 153-179

Comment:
**Caller-supplied block hash, state root, and message count are not verified against the EXECUTE proof**

`advance()` accepts `_newBlockHash`, `_newStateRoot`, and `_l1MessagesCount` as untrusted caller arguments but stores them directly after only checking `successful_validation == 1` and `chain_id`. A caller who has a valid SSZ witness for any block can submit that witness but pass arbitrary `_newBlockHash`, `_newStateRoot`, or `_l1MessagesCount` values — the EXECUTE precompile validates execution correctness but the resulting values stored on-chain are never bound to the proof.

Concretely: the `result` returned by `EXECUTE_PRECOMPILE.staticcall` contains `new_payload_request_root` (a hash-tree-root that commits to `execution_payload.block_hash`, `execution_payload.state_root`, and the parent hash), but the contract never verifies that `_newBlockHash` or `_newStateRoot` match those committed values. Similarly, `_l1MessagesCount` has no cryptographic tie to the messages actually consumed inside the proven block.

The fix is to either extract `block_hash`/`state_root` from the SSZ input bytes inside the contract, or extend `SszStatelessValidationResult` to return them so the contract can assert `_newBlockHash == result.block_hash`.

How can I resolve this? If you propose a fix, please make it concise.

---

This is a comment left during a code review.
Path: crates/l2/contracts/src/nativeRollup/l1/NativeRollup.sol
Line: 151-153

Comment:
**No check that the submitted block's parent hash matches the current on-chain `blockHash`**

`advance()` increments `blockNumber` and stores the new state, but nothing verifies that the block inside `_sszStatelessInput` descends from the current on-chain tip. The `execution_payload.parent_hash` in the SSZ input is never compared to `blockHash`. This allows a sequencer to commit an execution proof for an arbitrary block (any parent chain), forking the on-chain L2 view. Combined with the unverified `_newBlockHash` finding above, the on-chain chain state can be set to any value as long as some valid SSZ witness exists.

How can I resolve this? If you propose a fix, please make it concise.

---

This is a comment left during a code review.
Path: crates/vm/levm/src/execute_precompile.rs
Line: 57-73

Comment:
**Double SSZ decode of the same input**

`run_execute` decodes `calldata` into `SszStatelessInput` to charge gas and validate L2 constraints, then passes the original `calldata` bytes to `validator.verify(calldata)`. Inside `StatelessExecutor::verify`, the bytes are decoded a second time from scratch. Both decodes operate on the same bytes so there's no correctness issue, but the gas-charged `gas_used` and the gas-executed `gas_used` are computed from independent decode instances. Consider passing the already-decoded `input` (or its `gas_used` / constraint data) into the validator to avoid the redundant work and make the coupling explicit.

How can I resolve this? If you propose a fix, please make it concise.

---

This is a comment left during a code review.
Path: crates/l2/sequencer/native_rollup/l1_advancer.rs
Line: 153-161

Comment:
**`l1_messages_count` computed by recovering tx sender — silent undercount on recovery failure**

The count of L1 messages is derived by ECDSA-recovering the sender of every transaction in the block and comparing to `relayer_address`. If any relayer transaction's sender recovery fails (e.g. a malformed signature edge case), that transaction is silently excluded and the count passed to `advance()` will be lower than the number of messages actually processed in the block. Because `advance()` trusts this count to advance `l1MessageIndex`, the on-chain index would fall behind, making those messages re-eligible for future inclusion.

Consider deriving the count from the block's execution receipts or storing it as metadata during block production, rather than re-recovering senders at advancer time.

How can I resolve this? If you propose a fix, please make it concise.

_{Reviews (1): Last reviewed commit: "Restore the doc comment first paragraph ..." | Re-trigger Greptile}