feat(l2): remove public inputs from verify() function in OnChainProposer by pedrobergamini · Pull Request #2836 · lambdaclass/ethrex

pedrobergamini · 2025-05-18T23:26:57Z

Motivation

This PR implements the removal of public inputs from the verifyBatch() function in the OnChainProposer contract. Public inputs are now retrieved from the committed batch data instead of being passed as parameters.

Description

This PR removes public inputs from the verifyBatch function signature and instead reconstructs them from already-committed batch data. This simplifies the interface and reduces calldata size.

OnChainProposer.sol

Modified verifyBatch function signature: Removed all public input parameters
- Removed sp1PublicValues parameter (SP1 now only needs sp1ProofBytes)
- Removed tdxPublicValues parameter (TDX now only needs tdxSignature)
- Note: Pico has been removed from the codebase in main branch
Added _getPublicInputsFromCommitment function: Internal function that reconstructs 160-byte public inputs from committed batch data
- bytes 0-32: Initial state root (from last verified batch)
- bytes 32-64: Final state root (from current batch)
- bytes 64-96: Withdrawals merkle root
- bytes 96-128: Deposits log hash
- bytes 128-160: Last block hash
Updated all verifiers to use reconstructed public inputs:
- RISC0: Uses sha256(publicInputs)
- SP1: Uses publicInputs directly
- TDX: Uses publicInputs directly

zkVM Interface Updates

SP1: Updated to use commit_slice for output
RISC0: Updated to use commit_slice for output
Both interfaces now output only the encoded result without public inputs prefix

l1_proof_sender.rs

Updated VERIFY_FUNCTION_SIGNATURE constant to match new contract signature
Removed extraction logic for proof parts (no longer needed)
Simplified calldata construction

proving_systems.rs

Updated empty_calldata() method for proving systems
SP1 now only includes vkey and proof bytes
TDX now only includes signature
Removed Pico as it was removed from main branch

Closes #2804

LeanSerra

Hi! Congratulations for your first PR on Ethrex. The code is looking great however there are a few things that need fixing that cause the integration test to fail.

To fix them please address all the review comments + this change at the very end of the file crates/l2/prover/zkvm/interface/sp1/src/main.rs

    // Calculate final state root hash and check
    let final_state_hash = state_trie.hash_no_commit();
    if final_state_hash != last_block_state_root {
        panic!("invalid final state trie");
    }

    sp1_zkvm::io::commit_slice(
        &ProgramOutput {
            initial_state_hash,
            final_state_hash,
            #[cfg(feature = "l2")]
            withdrawals_merkle_root,
            #[cfg(feature = "l2")]
            deposit_logs_hash,
        }
        .encode(),
    );

We have to remove the gas_used as a public value as we don't want to store it in the contract and we need to use commit_slice as the previous commit function was adding a few bytes of padding leading to a mismatch when verifying the proof.

With these changes the integration test will pass however with the recent merged PRs TDX was added as a prover which implies a few changes on the OnChainProposer contract that you'll need to address.

Feel free to ask for help if you need any

…nProposer

…rom-verify-function Feat/remove public inputs from verify function

jrchatruc · 2025-06-19T15:48:25Z

@pedrobergamini overall code looks good but the integration test is not passing, seems like there's a few things to fix on the contract and the proof sender code

klaus993 · 2025-06-25T17:41:27Z

Hey @pedrobergamini!

The CI is not running because we're using a GitHub Variable in all workflows, which isn't accessible from PRs created from forks. This is a problem we had not anticipated. Due to this, I'm currently working on a fix which removes the use of this variable completely, which would allow the CI to run in this PR (and all future PRs from forks).

Sorry for the inconvenience, I'll let you know when you can merge changes from main to your branch so the CI can run.

cc @jrchatruc

klaus993 · 2025-06-25T18:46:26Z

@pedrobergamini @jrchatruc

#3318: Fix in progress. I'll merge it as soon as all its CI jobs succeed.

iovoid · 2025-07-03T18:57:33Z

        if (SP1VERIFIER != DEV_MODE) {
            // If the verification fails, it will revert.
-            _verifyPublicData(batchNumber, sp1PublicValues[8:]);
            ISP1Verifier(SP1VERIFIER).verifyProof(
                SP1_VERIFICATION_KEY,
-                sp1PublicValues,
+                publicInputs,
                sp1ProofBytes
            );
        }


SP1 public inputs have an 8 byte header containing the length of the data.

mpaulucci · 2025-07-15T14:04:40Z

Closing since this hasn't been worked on in some time. Feel free to reopen.

xqft · 2025-07-22T14:40:27Z

The CI was fixed in #3591. @pedrobergamini do you want to work on this still?

pedrobergamini · 2025-08-04T23:42:48Z

The CI was fixed in #3591. @pedrobergamini do you want to work on this still?

Yep! I will get back to it this week then, thanks for the ping.

…ation Mirrors EELS PR #2836 (merged 2026-05-12, devnets/bal/7): ethereum/execution-specs#2836 When the authority's pre-state code slot already holds a 7702 delegation indicator (overwrite or clear), refill the 23-byte AUTH_BASE portion of intrinsic state gas via both the reservoir and the state_refund channel. Keys off the pre-state code, not the auth target, so it applies whether auth.address is a new delegation target or zero (clear). Breaks ef-tests until upstream re-cuts fixtures past tests-bal@v7.0.0; no skips added per the bal-devnet-7 alignment policy.

Mirrors EELS PR #2848 (tests-bal@v7.1.1, devnets/bal/7): ethereum/execution-specs#2848 PR #2836 already refunded the 23-byte AUTH_BASE portion when the authority's pre-state code slot held a delegation indicator. #2848 broadens the condition: when the auth is a clear (`auth.address == 0x00`), no new indicator bytes are written regardless of pre-state code, so the AUTH_BASE refill applies even against an authority with no prior code. Equivalent to EELS: if authority_account.code_hash != EMPTY_CODE_HASH or auth.address == NULL_ADDRESS: refund = STATE_BYTES_PER_AUTH_BASE * COST_PER_STATE_BYTE

**Motivation** Bring ethrex up to bal-devnet-7 (BAL fixtures `bal@v7.1.1`). Stacked on top of #bal-devnet-6-pr (now in main). **Description** Aligns EIP-8037 state-gas accounting with bal-devnet-7 spec progression (EELS PRs lambdaclass#2815 / lambdaclass#2816 / lambdaclass#2823 / lambdaclass#2827 / lambdaclass#2828 / lambdaclass#2836 / lambdaclass#2845 / lambdaclass#2848), bumps Amsterdam fixtures from `snobal-devnet-6@v1.1.0` to `bal@v7.1.1`, and bumps the pinned hive version past the ethrex `--http.api` fix. Main changes: - EIP-8037 state-gas alignment with bal-devnet-7: - System-call state-gas reservoir. - Halt refunds spilled state gas (Policy A). - Tx-level CREATE failure refunds intrinsic `NEW_ACCOUNT`; `intrinsic_state_gas_charged` preserved across the failure path. - Tx-CREATE collision refund with regular-gas burn; billing matches EELS. - Cross-frame revert leaks inline credits. - Cross-frame revert reservoir formula fix. - Block-level `state_gas_used` subtracts `state_refund`. - Remove same-tx SELFDESTRUCT state-gas refund (EELS PR lambdaclass#2845, v7.1.0). - EIP-7702: - `set_delegation` refund via dedicated `state_refund` channel. - `set_delegation` refunds `AUTH_BASE` on existing delegation (EELS PR lambdaclass#2836). - `set_delegation` refunds `AUTH_BASE` on delegation clear (EELS PR lambdaclass#2848, v7.1.1). - levm fixes pulled from main: - `revert` doesn't unmark the account as existing (lambdaclass#6592). - Account erroneously considered as existing after zero-value transfer (lambdaclass#6591). - Tooling / tests: - Per-tx gas-dimension dump on block `gas_used` mismatch. - Bump Amsterdam fixtures to `bal@v7.1.1`. - Annotate BAL balance-mismatch errors with gas-equivalent diff and recognised state-gas constant multiples. - Unskip 74 bal-devnet-6 Amsterdam fixtures now passing. - Skip 21 stale EIP-8025 fixtures pinned at `bal@v5.7.0` (zkevm@v0.3.3 bundle, pre-bal-7). - Drop stale bal-devnet-6 known-issues entries from `docs/known_issues.md` and hive `KNOWN_EXCLUDED_TESTS`. - CI: - Bump pinned hive version past the ethrex `--http.api` flag feature-detect fix (`c4d839b3`, hive lambdaclass#1485). Without this, hive starts ethrex with the default HTTP namespace allowlist (`eth,net,web3`) and tests touching `admin_*`/`debug_*`/`txpool_*` fail. **Local test run** `./run_test.sh` against `tests-bal@v7.1.1`: 2,145 / 2,145 pass. `cargo test -p ethrex-test --tests`: 453 / 453 pass. **Checklist** - [ ] Updated `STORE_SCHEMA_VERSION` (crates/storage/lib.rs) if the PR includes breaking changes to the `Store` requiring a re-sync. --------- Co-authored-by: Lucas Fiegl <iovoid@users.noreply.github.com> Co-authored-by: Ivan Litteri <67517699+ilitteri@users.noreply.github.com>

pedrobergamini requested a review from a team as a code owner May 18, 2025 23:26

ManuelBilbao added the L2 Rollup client label May 20, 2025

LeanSerra suggested changes May 23, 2025

View reviewed changes

Comment thread crates/l2/contracts/src/l1/OnChainProposer.sol Outdated

Comment thread crates/l2/sequencer/l1_proof_sender.rs Outdated

Comment thread crates/l2/sequencer/l1_proof_sender.rs Outdated

LeanSerra reviewed May 23, 2025

View reviewed changes

Comment thread crates/l2/contracts/src/l1/OnChainProposer.sol Outdated

pedrobergamini added 7 commits June 8, 2025 14:46

feat(l2): remove public inputs from verify() function in OnChainProposer

f1d9662

chore(l2): document _getPublicInputsFromCommitment function in OnChai…

197fe66

…nProposer

feat(l2): remove public inputs from verifyBatch

f0e9a94

feat(l2): update zkVM interfaces to use commit_slice

944009a

fix(l2): update prover backends for new contract interface

473d536

chore(l2): update verify function signature and TDX calldata

6ab288b

chore: resolve merge conflicts

97cb2d4

pedrobergamini changed the title ~~chore(l2): remove public inputs from verify() function in OnChainProposer~~ feat(l2): remove public inputs from verify() function in OnChainProposer Jun 15, 2025

pedrobergamini closed this Jun 15, 2025

pedrobergamini force-pushed the main branch from fdf7507 to fdfbcc2 Compare June 15, 2025 23:44

Merge pull request #1 from pedrobergamini/feat/remove-public-inputs-f…

7ece6a1

…rom-verify-function Feat/remove public inputs from verify function

pedrobergamini reopened this Jun 15, 2025

pedrobergamini requested review from ManuelBilbao and jrchatruc as code owners June 15, 2025 23:45

pedrobergamini requested a review from LeanSerra June 15, 2025 23:47

Merge branch 'main' into main

15f83f9

pedrobergamini and others added 6 commits June 20, 2025 19:54

chore: fix missing _getPublicInputsFromCommitment call

e678ccd

chore: remove typo from natspec

550e9c6

chore: resolve OnChainProposer conflicts

b2042fd

chore: fix merge conflicts issue

0d8382e

Merge branch 'main' into main

b084f6b

Merge branch 'main' into main

e22311f

Merge branch 'main' into main

387b7a6

MegaRedHand added this to ethrex_l2 Jul 1, 2025

MegaRedHand moved this to In Progress in ethrex_l2 Jul 1, 2025

iovoid requested changes Jul 3, 2025

View reviewed changes

ilitteri moved this from In Progress to Requires Changes in ethrex_l2 Jul 3, 2025

ilitteri assigned pedrobergamini Jul 4, 2025

mpaulucci closed this Jul 15, 2025

github-project-automation Bot moved this from Requires Changes to Done in ethrex_l2 Jul 15, 2025

pedrobergamini mentioned this pull request Aug 25, 2025

feat(l2): remove public inputs from verify() function in OnChainProposer #4147

Closed

edg-l mentioned this pull request May 12, 2026

tracking: glamsterdam-devnet-6 alignment #6583

Open

edg-l mentioned this pull request May 13, 2026

feat(l1): support bal-devnet-7 (bal@v7.1.1) #6653

Merged

1 task

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(l2): remove public inputs from verify() function in OnChainProposer#2836

feat(l2): remove public inputs from verify() function in OnChainProposer#2836
pedrobergamini wants to merge 16 commits into
lambdaclass:mainfrom
pedrobergamini:main

pedrobergamini commented May 18, 2025 •

edited

Loading

Uh oh!

LeanSerra left a comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

jrchatruc commented Jun 19, 2025

Uh oh!

klaus993 commented Jun 25, 2025

Uh oh!

klaus993 commented Jun 25, 2025 •

edited

Loading

Uh oh!

iovoid Jul 3, 2025

Uh oh!

mpaulucci commented Jul 15, 2025

Uh oh!

xqft commented Jul 22, 2025

Uh oh!

pedrobergamini commented Aug 4, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

10 participants

Conversation

pedrobergamini commented May 18, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

OnChainProposer.sol

zkVM Interface Updates

l1_proof_sender.rs

proving_systems.rs

Uh oh!

LeanSerra left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

jrchatruc commented Jun 19, 2025

Uh oh!

klaus993 commented Jun 25, 2025

Uh oh!

klaus993 commented Jun 25, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

iovoid Jul 3, 2025

Choose a reason for hiding this comment

Uh oh!

mpaulucci commented Jul 15, 2025

Uh oh!

xqft commented Jul 22, 2025

Uh oh!

pedrobergamini commented Aug 4, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

10 participants

pedrobergamini commented May 18, 2025 •

edited

Loading

klaus993 commented Jun 25, 2025 •

edited

Loading