chore: bump leanVM to 2498896#421
Conversation
🤖 Claude Code ReviewPR #421 — chore: bump leanVM to 8eec56cOverviewA dependency-only bump of leanVM from What changed inside leanVM (lock file evidence)The most notable internal change visible in the lock file is the wholesale replacement of
AssessmentCorrectness / API stability — Strong evidence provided. The XMSS aggregation API ( The Plonky3 pin — The note about One thing to verify — The PR description says SummaryThis is a clean, low-risk dependency bump. The changed surface in ethlambda is two rev-string lines in Automated review by Claude (Anthropic) · sonnet · custom prompt |
🤖 Kimi Code ReviewSecurity & Consensus-Critical Concerns
Performance Implications
Maintainability & Supply Chain
Minor Observations
Summary
Automated review by Kimi (Moonshot AI) · kimi-k2.5 · custom prompt |
🤖 Codex Code ReviewFindings
Notes I did not find any repo-local changes to fork choice, attestation processing, justification/finalization, state transition, or SSZ logic in this PR; the risk is entirely in the XMSS dependency bump. I also could not do a clean build in this sandbox because the new git revision is not locally cached and network access is restricted, so I could not independently validate the upstream code path. Automated review by OpenAI Codex · gpt-5.4 · custom prompt |
Greptile SummaryThis PR bumps the leanVM dependency in
Confidence Score: 3/5Two discrepancies need resolution before merging: the pinned rev in Cargo.toml does not match the rev advertised in the PR description, and Plonky3 crates have drifted to a newer commit despite the stated freeze meant to preserve Rust 1.92.0 compatibility. The rev in Cargo.toml (2498896) does not match what the PR title/description claim (a44bca4), and all Plonky3 p3-* crates have moved from 82cfad73 to bde2e86e — contradicting the explicit note that they must stay at 82cfad73 to avoid a build-breaking nightly feature on Rust 1.92.0. Both issues need to be confirmed or corrected before this lands. Both changed files need attention: crates/common/crypto/Cargo.toml for the rev mismatch and Cargo.lock for the Plonky3 drift.
|
| Filename | Overview |
|---|---|
| crates/common/crypto/Cargo.toml | Bumps leanVM rev for lean-multisig and leansig_wrapper from f66d4a9 to 2498896, but PR description claims the target rev is a44bca4 — these two short SHAs do not match. |
| Cargo.lock | All leanVM crates bumped to rev 2498896; rayon replaced by new parallel crate; Plonky3 p3-* crates moved from 82cfad73 to bde2e86e despite stated intention to keep them frozen; windows-sys unified to 0.61.2; spin downgraded to 0.10.0. |
Flowchart
%%{init: {'theme': 'neutral'}}%%
flowchart TD
A["ethlambda-crypto\n(crates/common/crypto)"] --> B["lean-multisig\n(leanVM rev 2498896)"]
A --> C["leansig_wrapper\n(leanVM rev 2498896)"]
B --> D["backend / lean_vm\n(leanVM rev 2498896)"]
B --> E["rec_aggregation\n(leanVM rev 2498896)"]
D --> F["mt-* crates\n(leanVM rev 2498896)"]
F --> G["parallel\n(NEW — replaces rayon)"]
F --> H["zk-alloc\n(NEW dep on parallel)"]
F --> I["p3-* crates\n(Plonky3 bde2e86e)\n⚠️ drifted from stated 82cfad73"]
G --> J["system-info\n(leanVM rev 2498896)"]
Comments Outside Diff (1)
-
Cargo.lock, line 5306-5310 (link)Plonky3 pins have drifted despite stated freeze
The PR description explicitly states Plonky3
p3-*crates are kept pinned at82cfad73to avoid the unstablemaybe_uninit_slicefeature that breaks builds on Rust 1.92.0. However, the lock file shows allp3-*crates moving from82cfad73cd734d37a0d51953094f970c531817ectobde2e86e9a9ad3ed4ec5002bff31f88f23029e40. Ifbde2e86e(or any commit between82cfad73and it) usesmaybe_uninit_slice, CI/builds on Rust 1.92.0 will fail.Prompt To Fix With AI
This is a comment left during a code review. Path: Cargo.lock Line: 5306-5310 Comment: **Plonky3 pins have drifted despite stated freeze** The PR description explicitly states Plonky3 `p3-*` crates are kept pinned at `82cfad73` to avoid the unstable `maybe_uninit_slice` feature that breaks builds on Rust 1.92.0. However, the lock file shows all `p3-*` crates moving from `82cfad73cd734d37a0d51953094f970c531817ec` to `bde2e86e9a9ad3ed4ec5002bff31f88f23029e40`. If `bde2e86e` (or any commit between `82cfad73` and it) uses `maybe_uninit_slice`, CI/builds on Rust 1.92.0 will fail. How can I resolve this? If you propose a fix, please make it concise.
Prompt To Fix All With AI
Fix the following 2 code review issues. Work through them one at a time, proposing concise fixes.
---
### Issue 1 of 2
Cargo.lock:5306-5310
**Plonky3 pins have drifted despite stated freeze**
The PR description explicitly states Plonky3 `p3-*` crates are kept pinned at `82cfad73` to avoid the unstable `maybe_uninit_slice` feature that breaks builds on Rust 1.92.0. However, the lock file shows all `p3-*` crates moving from `82cfad73cd734d37a0d51953094f970c531817ec` to `bde2e86e9a9ad3ed4ec5002bff31f88f23029e40`. If `bde2e86e` (or any commit between `82cfad73` and it) uses `maybe_uninit_slice`, CI/builds on Rust 1.92.0 will fail.
### Issue 2 of 2
crates/common/crypto/Cargo.toml:17-18
**Pinned rev in `Cargo.toml` does not match PR title/description**
The PR title and description state leanVM is being bumped to `a44bca4`, but the actual rev in `Cargo.toml` (and correspondingly in `Cargo.lock`) is `2498896`. These are distinct short SHAs pointing to different commits. The description's note also references a prior target of `8eec56c`. It is unclear whether `2498896` is the intended final rev or a leftover from an intermediate update.
Reviews (2): Last reviewed commit: "Merge branch 'main' into chore/bump-lean..." | Re-trigger Greptile
|
We see lower aggregation performance on this commit. We're pausing merging this while investigating |
Track upstream leanVM at a44bca4 (previously f66d4a9). No source changes were required: the XMSS aggregation API (xmss_aggregate, compress/decompress) is unchanged at this rev. Plonky3 p3-* crates are kept pinned at 82cfad73: cargo update floats them to the default branch, where newer commits use the unstable maybe_uninit_slice feature that doesn't build on Rust 1.92.0.
87b53f1 to
f8f50c9
Compare
Picks up whir perf fix (leanVM#250): avoid serial 256 MiB copy of combined-statement weights. Plonky3 stays pinned to bde2e86e, the commit locked by leanVM itself, since newer Plonky3 requires nightly.
🤖 Claude Code ReviewPR Review: chore: bump leanVM to 2498896Scope: Two files changed — Summary of what changed in the lock file
Findings
The most notable structural change. This is not a correctness concern for consensus (proof generation is off the critical fork-choice path), but it is worth keeping an eye on aggregation throughput in the devnet, since
This is a regression in lock-file terms and worth understanding. Plonky3 The PR description explains the rationale: newer Plonky3 commits require
A Plonky3-internal refactor; ethlambda doesn't use Poseidon1 directly, so this is transparent. API surface unchanged — confirmed by author The two entry points ethlambda uses ( Both
Minor noteThe PR description mentions a previous target of VerdictThe change is mechanically correct, build-verified, API-compatible, and carries a real performance benefit (avoids the 256 MiB serial copy in the WHIR prover). The Approved with no blocking issues. Recommend a devnet run with aggregation enabled (per the Automated review by Claude (Anthropic) · sonnet · custom prompt |
🤖 Kimi Code ReviewGeneral Assessment Security & Consensus Critical Concerns
Performance & Correctness
Code Quality
Requested Changes
Acknowledgment: The dependency update appears to be a necessary upstream sync for ZK proof system improvements, but the threading and allocator changes require rigorous validation before deployment in a consensus context. Automated review by Kimi (Moonshot AI) · kimi-k2.5 · custom prompt |
| lean-multisig = { git = "https://github.com/leanEthereum/leanVM.git", rev = "2498896" } | ||
| # leansig_wrapper provides XmssPublicKey/XmssSignature types used by lean-multisig's public API |
There was a problem hiding this comment.
Pinned rev in
Cargo.toml does not match PR title/description
The PR title and description state leanVM is being bumped to a44bca4, but the actual rev in Cargo.toml (and correspondingly in Cargo.lock) is 2498896. These are distinct short SHAs pointing to different commits. The description's note also references a prior target of 8eec56c. It is unclear whether 2498896 is the intended final rev or a leftover from an intermediate update.
Prompt To Fix With AI
This is a comment left during a code review.
Path: crates/common/crypto/Cargo.toml
Line: 17-18
Comment:
**Pinned rev in `Cargo.toml` does not match PR title/description**
The PR title and description state leanVM is being bumped to `a44bca4`, but the actual rev in `Cargo.toml` (and correspondingly in `Cargo.lock`) is `2498896`. These are distinct short SHAs pointing to different commits. The description's note also references a prior target of `8eec56c`. It is unclear whether `2498896` is the intended final rev or a leftover from an intermediate update.
How can I resolve this? If you propose a fix, please make it concise.
🤖 Codex Code ReviewFindings
Notes I did not find any direct changes to fork choice, attestation processing, justification/finalization, or SSZ/state transition logic in this PR; the risk is entirely in the XMSS dependency bump. I could not complete a local build in this environment because Cargo could not update git/crates sources here due read-only home paths and no network access, so this review is source-based rather than build-verified. Automated review by OpenAI Codex · gpt-5.4 · custom prompt |
Motivation
Track upstream leanVM at
2498896(previouslyf66d4a9).Description
No source changes were required: the XMSS aggregation API used by
ethlambda-crypto(xmss_aggregate,AggregatedXMSS::compress/decompress) is unchanged at this rev. Onlycrates/common/crypto/Cargo.tomlandCargo.lockchange.Notes:
2498896is one commit ahead of the previous candidatea44bca4: a whir prover perf fix (leanVM#250, avoids a serial 256 MiB copy of combined-statement weights).p3-*crates are pinned atbde2e86e, the commit locked by leanVM's ownCargo.lock. leanVM references Plonky3's default branch with no rev, socargo updatefloats them forward; newer Plonky3 commits use the unstablemaybe_uninit_slicefeature, which doesn't build on Rust 1.92.0.Testing
cargo check --workspaceandmake lintare clean.cargo test --workspace --release: all unit/integration tests pass. The spec fixture suites fail on stale local fixtures (missing field attestationPubkeyat JSON parse time); verified viagit stashthat the failure is identical without this change.cargo test -p ethlambda-crypto --release -- --ignored(the slow XMSS aggregation/verification tests) all pass against the new rev.