Skip to content

OpenVMGuest: Bump OpenVM runtime to 2.1.0.49 - #14630

Closed
maximmenshikov wants to merge 2 commits into
masterfrom
build/openvm-runtime-2.1.0.49
Closed

maximmenshikov wants to merge 2 commits into
masterfrom
build/openvm-runtime-2.1.0.49

Conversation

@maximmenshikov

@maximmenshikov maximmenshikov commented Oct 13, 2026 •

Copy link
Copy Markdown
Contributor

User description

Update Nethermind.OpenVM.Runtime from 1.0.0-preview.1 to 2.1.0.49 (release), the first release that bflat-openvm versions after the OpenVM line it targets (v2.1.0).

Changes

Types of changes

What types of changes does your code introduce?

  • Bugfix (a non-breaking change that fixes an issue)
  • New feature (a non-breaking change that adds functionality)
  • Breaking change (a change that causes existing functionality not to work as expected)
  • Optimization
  • Refactoring
  • Documentation update
  • Build-related changes
  • Other: version bump

Testing

Requires testing

  • Yes
  • No

If yes, did you write tests?

  • Yes
  • No

Documentation

Requires documentation update

  • Yes
  • No

Requires explanation in Release Notes

  • Yes
  • No

🤖 Generated with Claude Code


PR Type

Other


Description

  • Bump Nethermind.OpenVM.Runtime to 2.1.0.49

  • Adds GLV/wNAF secp256k1 ecrecover and u256 intrinsics

  • Update version reference comment in openvm.toml


File Walkthrough

Relevant files
Dependencies
Directory.Packages.props
Bump OpenVM runtime package version                                           

Directory.Packages.props

  • Bump Nethermind.OpenVM.Runtime package version pin from
    1.0.0-preview.1 to 2.1.0.49
+1/-1     
Documentation
openvm.toml
Update version reference comment                                                 

src/Nethermind/Nethermind.Stateless.OpenVMGuest/openvm.toml

  • Update version reference in comment from 1.0.0-preview.1 to 2.1.0.49
  • No changes to accelerator moduli or curve configuration
+1/-1     

Update Nethermind.OpenVM.Runtime from 1.0.0-preview.1 to 2.1.0.49, the
first release versioned after the OpenVM line it targets. It brings the
GLV/wNAF secp256k1 ecrecover and the u256 intrinsics. The accelerator
moduli and curves in accel-rs init.rs are unchanged, so openvm.toml only
gets its version reference updated.

Signed-off-by: Maxim Menshikov <maksim.menshikov@nethermind.io>
@maximmenshikov maximmenshikov added the dependencies Pull requests that update a dependency file label Oct 13, 2026
@claude

claude Bot commented Oct 13, 2026 •

Copy link
Copy Markdown
Contributor

Claude finished @maximmenshikov's task in 1m 25s —— View job


PR review: OpenVM runtime bump to 2.1.0.49

  • Read the PR diff (Directory.Packages.props and openvm.toml)
  • Checked the upstream changes v1.0.0-preview.1...v2.1.0.49 in NethermindEth/bflat-openvm
  • Checked whether the accelerator ABI coupling in openvm.toml still holds
  • Checked the pinned runner image and how the guest calls the runtime's symbols
  • Checked CI

Verdict: Critical 0, High 0, Medium 0, Low 2. Nothing blocks the merge. I'd still wait for the OpenVM leg of Stateless execution tests to pass, because it is still running.

What I verified

  • The moduli and curve order still match. src/accel-rs/src/init.rs is not in the upstream diff between the two tags, so its moduli and curve order are unchanged. The new runner/openvm.toml upstream lists the same moduli, fp2 and curves, in the same order, as src/Nethermind/Nethermind.Stateless.OpenVMGuest/openvm.toml. The PR's claim is correct, and changing only the version in the comment is right.
  • The exact pin is kept. [2.1.0.49] still uses the exact-version bracket, which the ABI comment in openvm.toml relies on. This is the only reference to the package in the repo.
  • The pinned runner still works. The new ecrecover.rs uses only the standard ecc and algebra extensions: SW_ADD_NE, SW_DOUBLE, IS_EQ, the constrained division and the verified Sqrt hint. The OPENVM_IMAGE digest pinned in Makefile:19 does not need to change for this bump.
  • The CI failure is unrelated. Stateless benchmarks failed at docker/login-action on the ZisK leg (unauthorized: incorrect username or password). That is a credentials problem, not this change. The PR labeler failure is also unrelated.

Findings

Low: zkvm_secp256k1_ecrecover is now a new implementation. Before this bump, OpenVM guests recovered through openvm-eth's k256 msm. From 2.1.0.49 they use bflat-openvm's own GLV/wNAF code (src/accel-rs/src/ecrecover.rs, about 520 lines). build.sh renames symbols with objcopy to swap it in. This only affects the OpenVM guest, not the main client, but ecrecover results go into the state root, and a wrong recovery produces a valid proof of the wrong result. Upstream has good coverage: a host differential test against a Python reference with adversarial vectors, plus a guest run on the OpenVM runner. It would still be worth confirming that the OpenVM leg of stateless-tests.yml passes on fixtures with many signatures (block transactions plus the 0x01 precompile) before merging. A short note in the PR body that ecrecover itself changed would also help, because the description reads like a toolchain-only bump.

Low: the runtime now ships a symbol that only the custom runner can execute. zkvm_u512_divrem_hint is a phantom instruction (custom-0, imm 0x4d1) that only nethermindeth/openvm-runner registers. On a standard OpenVM SDK executor, such as an external prover, it transpiles to unimp and traps. Nethermind does not reference it today: its only zkvm_* P/Invokes are zkvm_uint256_mulmod (SP1) and zkvm_sha256_extend / zkvm_sha256_compress. So nothing changes now. Anyone who later binds to the new zkvm_u256_* / zkvm_u512_* intrinsics from managed code should know that the guest would then run only on the custom runner. No action is needed in this PR.

Not checked

  • I did not build the OpenVM guest or run it locally. That needs the bflat toolchain and Docker, and CI covers it.
  • I did not review the upstream code itself, including the GLV math in ecrecover.rs.

@github-actions

github-actions Bot commented Oct 13, 2026 •

Copy link
Copy Markdown
Contributor

Stateless guest cost

block steps Δ steps prover cost Δ cost
25526356 115,029,241 +0.000% 18,040,516,277 +0.000%
25532382 122,465,543 +0.000% 20,207,017,012 +0.000%
25532442 160,658,046 +0.000% 23,089,350,581 +0.000%
25532454 144,923,764 +0.000% 22,698,864,835 +0.000%
25532471 142,348,816 +0.000% 22,171,103,923 +0.000%
25532480 94,391,793 +0.000% 15,658,156,311 +0.000%
25532553 135,156,594 +0.000% 20,632,665,619 +0.000%
25532642 133,432,077 +0.000% 20,847,894,029 +0.000%
25532711 131,183,530 +0.000% 19,634,154,457 +0.000%
all 9 blocks 1,179,589,404 +0.000% 182,979,723,044 +0.000%
Variable cost by bucket, summed over shared blocks (excludes BASE)
bucket before after Δ
MAIN 80,212,079,472 80,212,079,472 +0.000%
OPCODES 16,518,149,121 16,518,149,121 +0.000%
PRECOMPILES 63,846,123,132 63,846,123,132 +0.000%
MEMORY 19,817,582,903 19,817,582,903 +0.000%

56b7eceb0ba0 (merge commit) · 9 pinned blocks · ziskemu is deterministic, so these figures are exact and any non-zero delta is real.

Compared against dd27c6db4049.

@github-actions

github-actions Bot commented Oct 13, 2026 •

Copy link
Copy Markdown
Contributor

PR Reviewer Guide 🔍

(Review updated until commit 8770130)

Here are some key observations to aid the review process:

⏱️ Estimated effort to review: 1 🔵⚪⚪⚪⚪
🏅 Score: 85
🧪 No relevant tests
🔒 No security concerns identified
⚡ Recommended focus areas for review

Unverified Crypto Change

This bump pulls in a rewritten GLV/wNAF secp256k1 ecrecover implementation (per the PR description), which is consensus-critical: an incorrect recovery would silently produce wrong signer addresses rather than fail loudly. The PR adds no tests exercising ecrecover via the new runtime to confirm behavioral equivalence with the previous implementation.

<PackageVersion Include="Nethermind.OpenVM.Runtime" Version="[2.1.0.49]" />

@github-actions

github-actions Bot commented Oct 13, 2026 •

Copy link
Copy Markdown
Contributor

EVM Opcode Benchmark Diff

Aggregated runs: base=3, pr=3
Noisy rerun opcodes: ADDMOD, CALLDEST, DUP4, MULMOD, ORIGIN, RETURNSUB, SELFDESTRUCT, SWAP5, TXDIFF

Regressions (2)

Opcode Base Median (ns) PR Median (ns) Delta Abs Δ (ns) Base CV PR CV Threshold Uncertainty Effective
MULMOD 285.950 760.598 +165.99% 474.648 37.9% 3.1% ±5.0% ±32.0% ±75.9%
SELFDESTRUCT 889.260 1319.961 +48.43% 430.701 19.8% 17.0% ±5.0% ±34.7% ±39.6%

@github-actions

Copy link
Copy Markdown
Contributor

Persistent review updated to latest commit 8770130

# fail to load - it computes the right operation on the wrong field. Transcribed
# from accel-rs init.rs in NethermindEth/bflat-openvm, as shipped in
# Nethermind.OpenVM.Runtime 1.0.0-preview.1, which Directory.Packages.props
# Nethermind.OpenVM.Runtime 2.1.0.49, which Directory.Packages.props

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Did you re-check the extension list against init.rs for 2.1.0.49? The comment says to do it when the pin moves

@svlachakis svlachakis left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

master already has this change: Directory.Packages.props pins Nethermind.OpenVM.Runtime [2.1.0.49], and the effective diff of this branch against current master is empty. I think this PR can be closed. If anything is still missing, please rebase so the diff shows it. Daniil's question about checking the extension list in openvm.toml against init.rs for 2.1.0.49 is still open.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

dependencies Pull requests that update a dependency file Review effort 1/5

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants