Skip to content

feat(openvm): add precompile patches#74

Merged
jsign merged 2 commits into
eth-act:masterfrom
axiom-crypto:openvm/update-patches
Sep 7, 2025
Merged

feat(openvm): add precompile patches#74
jsign merged 2 commits into
eth-act:masterfrom
axiom-crypto:openvm/update-patches

Conversation

@jonathanpwang

@jonathanpwang jonathanpwang commented Jun 12, 2025

Copy link
Copy Markdown
Contributor

This PR adds the necessary guest setup and patches to trigger all OpenVM custom instructions and patches for ETH block validation. Updates to openvm v1.4.0.

We currently patch revm to handle pre-compiles, but in a subsequent PR we will switch to using the new CryptoProvider trait to be able to directly use upstream revm.

I was not able to include the openvm ruint patch because it requires version to be fixed to v1.14.0 but some other risc0 packages seem to require v1.15^. From previous profiling, the ruint patch is not one of the most significant patches, so it's okay to skip it for now (although performance is still best with it).

We note that currently the stateless_validation function has a significant performance degradation because the conversion from execution witness to MPT in the guest program is very unoptimized, compared to our current deserialization approach:
Flamegraphs to be updated but we believe the performance degradation remains

  • deserialization approach (faster) benchmark:
    https://axiom-public-data-sandbox-us-east-1.s3.us-east-1.amazonaws.com/benchmark/github/flamegraphs/reth-357041628ad40f5bc3a02b0835e7bc72e716b069-f06666e653e1195fec07add609aac2114f20b1e7865e542fd3a8ddd137d1cd2b/metrics-reth.prove_evm.block_21000000.dsl_ir.opcode.air_name.cells_used.svg
    The flamegraph is weighted by number of AIR trace cells, not instructions, per call stack. source

  • stateless_validation approach (slower) benchmark run from this branch:
    https://axiom-public-data-sandbox-us-east-1.s3.us-east-1.amazonaws.com/benchmark/github/flamegraphs/reth-631ea41abb37e5ad1a7973fb40bd16055535d491-02ca23cf16f16dfe48b1ef00b28bf17f91f0042dc5ed0beda9fdc22150344c98/metrics-reth.prove_evm.block_21000000.dsl_ir.opcode.air_name.cells_used.svg
    Observe the large portion spent on StatelessTrie::new(). source

@jsign jsign mentioned this pull request Jun 20, 2025
@jsign

jsign commented Aug 14, 2025

Copy link
Copy Markdown
Collaborator

Will move out of draft after openvm v1.2.1 is released (it includes the k256 patch and other devex features to make patching easier).

@jonathanpwang, I see you released v1.3.0 a while ago -- could you rebase so we can consider merging this? I'm quite interested in giving it a spin for benchmarks.

Also, we changed to a new MPT from Risc0 and more optimized inputs (here and later here), so I'm curious how this now looks in your flamegraphs if you can re-run them 🙏

@jonathanpwang

Copy link
Copy Markdown
Contributor Author

@jonathanpwang, I see you released v1.3.0 a while ago -- could you rebase so we can consider merging this? I'm quite interested in giving it a spin for benchmarks.

Also, we changed to a new MPT from Risc0 and more optimized inputs (here and later here), so I'm curious how this now looks in your flamegraphs if you can re-run them 🙏

Will do. Yes I saw the MPT and was interested to try.

@jonathanpwang
jonathanpwang marked this pull request as ready for review September 3, 2025 23:07
@jonathanpwang

Copy link
Copy Markdown
Contributor Author

@jsign I've updated to OpenVM v1.4.0.

I wasn't able to test it directly in the repo yet because the readme instructions:

cd crates/witness-generator-cli
cargo run --release -- tests --include Prague --include cold

gave me error

Error: Failed to build EEST generator

Caused by:
    0: Failed to execute download script
    1: No such file or directory (os error 2)

By the way, with OpenVM v1.4.0, the CLI and SDK both come with GPU support if feature cuda is enabled: https://docs.openvm.dev/book/getting-started/install#installation-with-nvidia-gpu-support
I haven't looked into how this is handled in ere-host yet, so just mentioning it's an option. cc @han0110

@jsign

jsign commented Sep 3, 2025

Copy link
Copy Markdown
Collaborator

@jonathanpwang, thanks very much!

Ah, regarding witness-generator-cli we should fix the docs -- should be ran at the workspace level, not crate level. Standing in root of the repo, running RAYON_NUM_THREADS=10 RUST_LOG=info cargo run --release -p witness-generator-cli -- tests --include 10M- --include Prague will generate all 10M gas tests (as an example).

Reg OpenVM v1.4.0, we just merged the support for ere today in eth-act/ere#113 :) We have to update the ere dep in this repo, and should be good to go.

I'm OOO until Monday, so I can do this as soon as I come back.

@han0110

han0110 commented Sep 4, 2025

Copy link
Copy Markdown
Collaborator

@jonathanpwang Thanks for upgrading the patching, just ran locally and the performance looks nice!

But I found that the ruint patch is not used, because the alloy-trie@0.9.0 requires nybbles@0.4.0 that requires ruint@1.15.0, and in OpenVM's guest lib the ruint is 1.14.0, so the patching is ignored.

Not sure how much it affects the performance, but I think we could integrate the current patching first, and upgrade in future PR to get the fully patched performance.

@han0110 han0110 mentioned this pull request Sep 4, 2025
@jonathanpwang

Copy link
Copy Markdown
Contributor Author

@han0110 yes I noted it in the PR description. I tried to patch to 1.16 but there was some change that requires const fn and it was not the most straightforward. I believe it doesn't affect performance too much because it's all in revm_interpreter::arithmetic and even there most of the instructions are associated with handling the stack

@han0110

han0110 commented Sep 4, 2025

Copy link
Copy Markdown
Collaborator

Sorry I missed that part about ruint patching in the PR description 🙏

I believe it doesn't affect performance too much because it's all in revm_interpreter::arithmetic and even there most of the instructions are associated with handling the stack

Got it, sounds good!

@jsign

jsign commented Sep 7, 2025

Copy link
Copy Markdown
Collaborator

The CI job failing is expected since this is an external branch PR, so the GH repo secrets are guarded. Thanks very much again @jonathanpwang.

@jsign
jsign merged commit b84b1b8 into eth-act:master Sep 7, 2025
25 of 26 checks passed
@kevaundray

Copy link
Copy Markdown
Contributor

We note that currently the stateless_validation function has a significant performance degradation because the conversion from execution witness to MPT in the guest program is very unoptimized, compared to our current deserialization approach:

@jonathanpwang do you have a link to where this is done in an optimized way?

@jonathanpwang

Copy link
Copy Markdown
Contributor Author

@jonathanpwang do you have a link to where this is done in an optimized way?

The latest is https://github.com/axiom-crypto/openvm-reth-benchmark/tree/main/crates/executor/client which uses a new MPT.

Let me update everything to reth 1.7.0 to do a fair comparison.

BTW (not really the right place to mention) - if this repo could update reth to 1.7.0, then I will prepare another PR to switch to CryptoProvider.

@jsign

jsign commented Sep 10, 2025

Copy link
Copy Markdown
Collaborator

@jonathanpwang, I'm updating our fork to latest Reth in #182 -- but found a problem with OpenVM compiler, more see comment https://github.com/eth-act/zkevm-benchmark-workload/pull/182/files#r2337888495.

WiseMrMusa pushed a commit to NethermindEth/zkevm-benchmark-workload that referenced this pull request Jan 8, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants