chore(l2): use SP1 patched crate for ecmul precompile by xqft · Pull Request #5133 · lambdaclass/ethrex

xqft · 2025-10-30T21:26:58Z

Motivation

The ecmul precompile represents 10% of the total proving cycles, and we are not using the corresponding SP1 patched crate for this operation that uses a zkVM precompile.

This PR fixes that and reduces 81k cycles (86% of ecmul, 10% of total)

Flamegraphs
before:

after:

Proving times
Mainnet blocks proven in a RTX 4090

Block number	main	l2/ecmul_sp1
23426993	13m 19s	13m 20s
23426994	08m 42s	08m 05s
23426995	06m 39s	06m 35s
23426996	11m 24s	10m 50s

github-actions · 2025-10-30T21:29:35Z

Lines of code report

Total lines added: 45
Total lines removed: 0
Total lines changed: 45

Detailed view

+------------------------------------------+-------+------+
| File                                     | Lines | Diff |
+------------------------------------------+-------+------+
| ethrex/crates/vm/levm/src/precompiles.rs | 1648  | +45  |
+------------------------------------------+-------+------+

github-actions · 2025-10-30T21:47:14Z

Benchmark Results Comparison

No significant difference was registered for any benchmark run.

Detailed Results

Benchmark Results: BubbleSort

Command	Mean [s]	Min [s]	Max [s]	Relative
`main_revm_BubbleSort`	4.745 ± 0.021	4.729	4.798	1.03 ± 0.01
`main_levm_BubbleSort`	4.625 ± 0.024	4.589	4.668	1.00
`pr_revm_BubbleSort`	4.765 ± 0.010	4.753	4.789	1.03 ± 0.01
`pr_levm_BubbleSort`	4.633 ± 0.033	4.598	4.707	1.00 ± 0.01

Benchmark Results: ERC20Approval

Command	Mean [s]	Min [s]	Max [s]	Relative
`main_revm_ERC20Approval`	1.545 ± 0.006	1.535	1.553	1.00
`main_levm_ERC20Approval`	1.683 ± 0.009	1.675	1.708	1.09 ± 0.01
`pr_revm_ERC20Approval`	1.572 ± 0.007	1.554	1.580	1.02 ± 0.01
`pr_levm_ERC20Approval`	1.674 ± 0.011	1.658	1.696	1.08 ± 0.01

Benchmark Results: ERC20Mint

Command	Mean [ms]	Min [ms]	Max [ms]	Relative
`main_revm_ERC20Mint`	190.1 ± 9.2	186.1	215.5	1.00 ± 0.05
`main_levm_ERC20Mint`	208.0 ± 1.1	206.9	210.5	1.10 ± 0.01
`pr_revm_ERC20Mint`	189.6 ± 1.0	188.5	191.2	1.00
`pr_levm_ERC20Mint`	207.6 ± 1.1	206.3	209.9	1.10 ± 0.01

Benchmark Results: ERC20Transfer

Command	Mean [ms]	Min [ms]	Max [ms]	Relative
`main_revm_ERC20Transfer`	353.9 ± 2.2	351.6	358.0	1.00
`main_levm_ERC20Transfer`	399.8 ± 3.1	397.2	408.3	1.13 ± 0.01
`pr_revm_ERC20Transfer`	359.9 ± 1.3	358.6	363.4	1.02 ± 0.01
`pr_levm_ERC20Transfer`	395.8 ± 2.1	392.5	399.2	1.12 ± 0.01

Benchmark Results: Factorial

Command	Mean [ms]	Min [ms]	Max [ms]	Relative
`main_revm_Factorial`	241.4 ± 0.8	240.5	243.0	1.00
`main_levm_Factorial`	291.9 ± 36.7	278.7	396.3	1.21 ± 0.15
`pr_revm_Factorial`	244.1 ± 1.3	241.9	246.6	1.01 ± 0.01
`pr_levm_Factorial`	279.0 ± 1.0	277.2	280.2	1.16 ± 0.01

Benchmark Results: FactorialRecursive

Command	Mean [s]	Min [s]	Max [s]	Relative
`main_revm_FactorialRecursive`	1.708 ± 0.040	1.643	1.760	1.02 ± 0.03
`main_levm_FactorialRecursive`	8.573 ± 0.024	8.539	8.614	5.12 ± 0.10
`pr_revm_FactorialRecursive`	1.673 ± 0.032	1.597	1.706	1.00
`pr_levm_FactorialRecursive`	8.557 ± 0.038	8.482	8.616	5.11 ± 0.10

Benchmark Results: Fibonacci

Command	Mean [ms]	Min [ms]	Max [ms]	Relative
`main_revm_Fibonacci`	215.0 ± 0.7	213.3	216.0	1.00
`main_levm_Fibonacci`	260.0 ± 3.9	256.4	267.8	1.21 ± 0.02
`pr_revm_Fibonacci`	217.8 ± 1.6	216.4	222.0	1.01 ± 0.01
`pr_levm_Fibonacci`	259.4 ± 6.6	255.2	275.9	1.21 ± 0.03

Benchmark Results: FibonacciRecursive

Command	Mean [ms]	Min [ms]	Max [ms]	Relative
`main_revm_FibonacciRecursive`	873.7 ± 9.4	862.3	890.7	1.13 ± 0.03
`main_levm_FibonacciRecursive`	775.0 ± 18.5	724.6	794.6	1.00
`pr_revm_FibonacciRecursive`	892.9 ± 7.0	882.9	905.3	1.15 ± 0.03
`pr_levm_FibonacciRecursive`	781.4 ± 3.2	778.1	788.5	1.01 ± 0.02

Benchmark Results: ManyHashes

Command	Mean [ms]	Min [ms]	Max [ms]	Relative
`main_revm_ManyHashes`	12.5 ± 0.1	12.4	12.6	1.00
`main_levm_ManyHashes`	13.8 ± 0.0	13.7	13.8	1.10 ± 0.01
`pr_revm_ManyHashes`	12.6 ± 0.1	12.5	12.7	1.00 ± 0.01
`pr_levm_ManyHashes`	13.7 ± 0.0	13.7	13.8	1.10 ± 0.01

Benchmark Results: MstoreBench

Command	Mean [ms]	Min [ms]	Max [ms]	Relative
`main_revm_MstoreBench`	268.7 ± 2.1	266.5	273.0	1.06 ± 0.01
`main_levm_MstoreBench`	254.1 ± 1.4	251.4	255.5	1.00 ± 0.01
`pr_revm_MstoreBench`	268.8 ± 2.1	266.9	273.6	1.06 ± 0.01
`pr_levm_MstoreBench`	253.3 ± 2.3	250.9	258.3	1.00

Benchmark Results: Push

Command	Mean [ms]	Min [ms]	Max [ms]	Relative
`main_revm_Push`	299.2 ± 0.6	298.7	300.7	1.00
`main_levm_Push`	307.2 ± 1.6	305.4	310.3	1.03 ± 0.01
`pr_revm_Push`	299.3 ± 2.7	297.4	307.0	1.00 ± 0.01
`pr_levm_Push`	309.2 ± 2.1	307.0	313.1	1.03 ± 0.01

Benchmark Results: SstoreBench_no_opt

Command	Mean [ms]	Min [ms]	Max [ms]	Relative
`main_revm_SstoreBench_no_opt`	220.4 ± 1.6	219.2	224.6	2.45 ± 0.04
`main_levm_SstoreBench_no_opt`	89.9 ± 1.2	88.2	91.1	1.00
`pr_revm_SstoreBench_no_opt`	219.9 ± 0.6	219.3	221.4	2.45 ± 0.03
`pr_levm_SstoreBench_no_opt`	91.3 ± 2.8	87.6	98.4	1.02 ± 0.03

Copilot

Pull Request Overview

This PR refactors BN254 elliptic curve precompile functions (ecmul and ecpairing) by extracting common parsing and validation logic into reusable helper functions. The changes improve code organization and maintainability by separating concerns.

Key Changes:

Extracted BN254 point parsing logic into parse_bn254_g1, parse_bn254_g2, and parse_bn254_scalar helper functions
Split validation logic from validate_bn254_coords into separate validate_bn254_g1_coords and validate_bn254_g2_coords functions
Added SP1 feature-gated implementation of bn254_g1_mul using substrate_bn library

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

ilitteri

The implementation looks good. I'll run some replays to ensure it works properly. Left some comments.

ilitteri · 2025-10-31T17:37:25Z

-        #[expect(unsafe_code, reason = "chunks_exact ensures the conversion is valid")]
-        let input: [u8; 192] = unsafe { input.try_into().unwrap_unchecked() };
-        let (g1, g2) = parse_bn254_coords(&input);
-        if validate_bn254_coords(&g1, &g2)? {
-            batch.push((g1, g2));
-        }


Why do we remove this unsafe? We should double-check with @edg-l, @azteca1998, or @Oppen if this is not counterproductive.

Can't we keep this behaviour when neither sp1 nor risc0 features are enabled?

The function to parse bn254 points was changed to take a slice instead of a reference to a fixed size array (so it can be reused for precompiles whose input have different sizes), so this unsafe is not necessary anymore.

The branching on the error case (which should never happen) could signify a perf. hit, I could change this to an unreachable_unchecked!() and it should be equivalent to before the change, but I'm not sure how impactful that can be (consider that we have other, more evident optimization opportunities, like not cloning and slicing bytes many times while parsing and converting to the crypto primitive types)

**Motivation** The ecmul precompile represents 10% of the total proving cycles, and we are not using the corresponding SP1 patched crate for this operation that uses a zkVM precompile. This PR fixes that and reduces 81k cycles (86% of ecmul, 10% of total) **Flamegraphs** before: <img width="2936" height="1134" alt="image" src="https://github.com/user-attachments/assets/42212408-8f2e-4dd4-80ea-c9b87a8383ef" /> after: <img width="2716" height="1220" alt="image" src="https://github.com/user-attachments/assets/f82e25f0-a7cb-43e9-ae4d-307a39bd9d0a" /> **Proving times** Mainnet blocks proven in a RTX 4090 | Block number | main | l2/ecmul_sp1 | |---------------|---------|----------------| | 23426993 | 13m 19s | 13m 20s | | 23426994 | 08m 42s | 08m 05s | | 23426995 | 06m 39s | 06m 35s | | 23426996 | 11m 24s | 10m 50s | --------- Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> Co-authored-by: Ivan Litteri <67517699+ilitteri@users.noreply.github.com> Co-authored-by: ilitteri <ilitteri@fi.uba.ar>

implement ecmul with substrate for sp1

ec0601e

github-actions Bot assigned xqft Oct 30, 2025

github-actions Bot added the L2 Rollup client label Oct 30, 2025

github-project-automation Bot added this to ethrex_l2 Oct 30, 2025

xqft added 3 commits October 31, 2025 10:35

fixes

be9dc69

use slicing

2e394b3

use offset

effb630

xqft marked this pull request as ready for review October 31, 2025 14:22

xqft requested a review from a team as a code owner October 31, 2025 14:22

Merge branch 'main' into l2/ecmul_sp1

4d6c045

Copilot AI review requested due to automatic review settings October 31, 2025 14:22

Copilot AI reviewed Oct 31, 2025

View reviewed changes

Comment thread crates/vm/levm/src/precompiles.rs Outdated

xqft and others added 6 commits October 31, 2025 12:02

Update crates/vm/levm/src/precompiles.rs

718e183

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

use safer add and slicing

e406e46

change name

a9aba91

clippy

cd4b392

Merge branch 'main' into l2/ecmul_sp1

7e011e1

fix slicing

a6df6f9

ilitteri requested changes Oct 31, 2025

View reviewed changes

github-project-automation Bot moved this to Requires Changes in ethrex_l2 Oct 31, 2025

ilitteri requested review from MauroToscano, Oppen, azteca1998 and edg-l October 31, 2025 17:50

xqft added 2 commits October 31, 2025 16:08

Update cfg attributes for bn254_g1_mul function

63c0a65

Merge branch 'main' into l2/ecmul_sp1

f1fe328

ilitteri approved these changes Oct 31, 2025

View reviewed changes

Merge branch 'main' into l2/ecmul_sp1

af37b76

Fix RISC0's compilation

38f3aaf

pablodeymo approved these changes Nov 4, 2025

View reviewed changes

Merge branch 'main' into l2/ecmul_sp1

54c8f2a

ilitteri enabled auto-merge November 4, 2025 20:15

ilitteri added this pull request to the merge queue Nov 4, 2025

Merged via the queue into main with commit 2f3a86f Nov 4, 2025
50 checks passed

ilitteri deleted the l2/ecmul_sp1 branch November 4, 2025 20:53

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

Uh oh!

Conversation

xqft commented Oct 30, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

github-actions Bot commented Oct 30, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Lines of code report

Uh oh!

github-actions Bot commented Oct 30, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Benchmark Results Comparison

Benchmark Results: BubbleSort

Benchmark Results: ERC20Approval

Benchmark Results: ERC20Mint

Benchmark Results: ERC20Transfer

Benchmark Results: Factorial

Benchmark Results: FactorialRecursive

Benchmark Results: Fibonacci

Benchmark Results: FibonacciRecursive

Benchmark Results: ManyHashes

Benchmark Results: MstoreBench

Benchmark Results: Push

Benchmark Results: SstoreBench_no_opt

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Key Changes:

Uh oh!

Uh oh!

ilitteri left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

ilitteri Oct 31, 2025

Choose a reason for hiding this comment

Uh oh!

ilitteri Oct 31, 2025

Choose a reason for hiding this comment

Uh oh!

xqft Oct 31, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

xqft commented Oct 30, 2025 •

edited

Loading

github-actions Bot commented Oct 30, 2025 •

edited

Loading

github-actions Bot commented Oct 30, 2025 •

edited

Loading

xqft Oct 31, 2025 •

edited

Loading