Skip to content

chore: bump leanVM to 2498896#421

Merged
MegaRedHand merged 3 commits into
mainfrom
chore/bump-leanvm-8eec56c
Jun 9, 2026
Merged

chore: bump leanVM to 2498896#421
MegaRedHand merged 3 commits into
mainfrom
chore/bump-leanvm-8eec56c

Conversation

@MegaRedHand

@MegaRedHand MegaRedHand commented Jun 8, 2026

Copy link
Copy Markdown
Collaborator

Motivation

Track upstream leanVM at 2498896 (previously f66d4a9).

Description

No source changes were required: the XMSS aggregation API used by ethlambda-crypto (xmss_aggregate, AggregatedXMSS::compress/decompress) is unchanged at this rev. Only crates/common/crypto/Cargo.toml and Cargo.lock change.

Notes:

  • 2498896 is one commit ahead of the previous candidate a44bca4: a whir prover perf fix (leanVM#250, avoids a serial 256 MiB copy of combined-statement weights).
  • Plonky3 p3-* crates are pinned at bde2e86e, the commit locked by leanVM's own Cargo.lock. leanVM references Plonky3's default branch with no rev, so cargo update floats them forward; newer Plonky3 commits use the unstable maybe_uninit_slice feature, which doesn't build on Rust 1.92.0.

Testing

  • cargo check --workspace and make lint are clean.
  • cargo test --workspace --release: all unit/integration tests pass. The spec fixture suites fail on stale local fixtures (missing field attestationPubkey at JSON parse time); verified via git stash that 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.

Note: previously targeted 8eec56c, then a44bca4; updated to 2498896 (devnet4 HEAD).

@github-actions

github-actions Bot commented Jun 8, 2026

Copy link
Copy Markdown

🤖 Claude Code Review

PR #421 — chore: bump leanVM to 8eec56c

Overview

A dependency-only bump of leanVM from f66d4a98eec56c. Two files change: crates/common/crypto/Cargo.toml (rev strings) and Cargo.lock (cascading updates). No ethlambda source code changes.


What changed inside leanVM (lock file evidence)

The most notable internal change visible in the lock file is the wholesale replacement of rayon with a new first-party parallel crate across all the mt-* math/proving crates, backend, lean-multisig, sub_protocols, zk-alloc, and system-info:

  • rayon drops out of: mt-fiat-shamir, mt-field, mt-koala-bear, mt-poly, mt-sumcheck, mt-symetric, mt-whir, system-info
  • A new parallel crate (git+...leanVM.git?rev=8eec56c, depends only on system-info) is added and wired throughout
  • zk-alloc picks up both parallel and system-info as new deps
  • windows-sys consolidates from three versions (0.52.0, 0.60.2, 0.52.0 in various places) to a single 0.61.2 — minor cleanup

Assessment

Correctness / API stability — Strong evidence provided. The XMSS aggregation API (xmss_aggregate, AggregatedXMSS::compress/decompress) is the only surface ethlambda touches, and the author ran the slow ignored crypto tests (cargo test -p ethlambda-crypto --release -- --ignored) against the new rev. That's the right test to run here.

The rayonparallel substitution — This is an upstream concern, not ethlambda's, but worth noting for awareness: parallel has only system-info as a dependency. If it is a thinner/custom thread pool compared to rayon, proof generation performance could shift. Since this is an internal leanVM refactor and tests pass, it is not a blocker, but worth watching if aggregation latency changes in devnet.

Plonky3 pin — The note about p3-* crates being pinned at 82cfad73 to stay buildable on Rust 1.92.0 (avoiding the maybe_uninit_slice unstable feature) is well-documented and the right call. The pinned entries are not in the diff because they didn't move, which is the expected outcome.

One thing to verify — The PR description says cargo build --workspace and the crypto tests pass, but doesn't mention make lint (clippy). Since there are no source changes this is almost certainly a non-issue, but it would round out the CI evidence.


Summary

This is a clean, low-risk dependency bump. The changed surface in ethlambda is two rev-string lines in Cargo.toml; everything else is the generated lock file. The author has provided the correct build and test evidence. No issues found.


Automated review by Claude (Anthropic) · sonnet · custom prompt

@github-actions

github-actions Bot commented Jun 8, 2026

Copy link
Copy Markdown

🤖 Kimi Code Review

Security & Consensus-Critical Concerns

  1. Custom Parallelism Library Replacing Rayon (High Risk)

    • Location: Cargo.lock diff (e.g., lines 666, 676, 4858, 4909)
    • Issue: The leanVM dependencies have switched from rayon (battle-tested, audited) to a custom parallel crate (defined at line 5624). In a consensus client, thread-scheduling bugs in signature aggregation can lead to non-deterministic state transitions or side-channel leaks.
    • Action: Audit leanVM@8eec56c's parallel crate for:
      • Data races in XMSS signature aggregation paths.
      • Constant-time violations in cryptographic operations (branching on secrets in thread pools).
      • Proper Send/Sync bounds on internal structures.
  2. Custom Allocator (zk-alloc) Introduction

    • Location: Cargo.lock lines 668, 4870, 4918, 9053
    • Issue: The new zk-alloc dependency (used by mt-poly, mt-sumcheck, mt-symetric, backend) implements custom memory allocation. Custom allocators require unsafe Rust and are prone to use-after-free or double-free bugs that could corrupt the beacon state or leak private keys.
    • Action: Verify zk-alloc has:
      • #![forbid(unsafe_code)] OR extensive Miri/Valgrind testing in CI.
      • Fuzzing coverage for allocation edge cases (OOM handling, zero-size types).
  3. XMSS Signature Verification Regression Risk

    • Location: crates/common/crypto/Cargo.toml lines 17, 20
    • Issue: Updating lean-multisig and leansig_wrapper changes post-quantum signature verification logic. A subtle bug here could cause a consensus split (nodes rejecting valid blocks or accepting invalid ones).
    • Action: Before merging, run the full XMSS test suite including:
      • Negative test cases (tampered signatures, wrong pubkeys).
      • Aggregation edge cases (empty sets, single signatures, maximum aggregation size).

Performance Implications

  • Location: Cargo.lock (rayon → parallel migration)
  • Issue: The custom parallel crate may not match rayon's work-stealing performance characteristics. Signature verification is in the hot path for attestation processing.
  • Recommendation: Benchmark process_attestation throughput on a testnet before/after this update. Verify no regression under high load (thousands of attestations per slot).

Maintainability & Supply Chain

  • Location: crates/common/crypto/Cargo.toml lines 17, 20
  • Issue: Git dependencies on external repositories without version tags increase supply chain risk. If leanVM force-pushes or deletes the 8eec56c revision, builds become non-reproducible.
  • Recommendation: Request leanVM maintainers to create signed tags (e.g., v0.2.0) and pin to tags instead of commit hashes when possible.

Minor Observations

  • The comment explaining the lean-multisig name history is helpful and accurate. Keep it.
  • windows-sys minor version bumps (0.60.2 → 0.61.2) appear to be transitive updates and are likely safe, but verify Windows CI passes.

Summary
Do not merge without:

  1. Reviewing the parallel and zk-alloc source in leanVM for unsafe code and thread-safety proofs.
  2. Running full XMSS cryptographic test vectors.
  3. Performance regression testing on signature aggregation.

Automated review by Kimi (Moonshot AI) · kimi-k2.5 · custom prompt

@github-actions

github-actions Bot commented Jun 8, 2026

Copy link
Copy Markdown

🤖 Codex Code Review

Findings

  1. High: This PR swaps the consensus-critical XMSS backend to a different git SHA with no in-repo regression signal that the aggregation and verification semantics are unchanged. The only code that pins the backend is crates/common/crypto/Cargo.toml, while the wrapper still hard-depends on xmss_aggregate(..., 2) and xmss_verify_aggregation(...) at crates/common/crypto/src/lib.rs and crates/common/crypto/src/lib.rs. All meaningful XMSS tests are still ignored as “too slow” at crates/common/crypto/src/lib.rs, lib.rs:333, lib.rs:369, and lib.rs:390. For a consensus client, that means a silent behavior change in the upstream git revision can land without CI catching a fork-inducing incompatibility.

  2. Medium: The lockfile materially expands the trusted code surface in the signature path by replacing rayon with a new internal parallel crate and introducing zk-alloc through backend/lean-multisig, but the PR provides no audit trail or rationale for those additions. See Cargo.lock, Cargo.lock:3664, Cargo.lock:5627, and Cargo.lock:9050. Since this dependency is used for attestation/proposer XMSS proof handling, allocator or scheduler bugs are not just performance bugs; they can become verification failures or DoS vectors. I would not merge this without either vendoring/reviewing the upstream diff or documenting the exact upstream changes being trusted.

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-apps

greptile-apps Bot commented Jun 8, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

This PR bumps the leanVM dependency in ethlambda-crypto from rev f66d4a9 to a newer commit, with no source-level changes required since the XMSS aggregation API surface is unchanged. The lock file reflects leanVM's internal refactor from rayon to a new parallel crate, several new zk-alloc transitive deps, and unification of windows-sys to 0.61.2.

  • crates/common/crypto/Cargo.toml: lean-multisig and leansig_wrapper pinned rev updated from f66d4a9 to 2498896, but the PR title and description claim the target is a44bca4 — these short SHAs refer to different commits and the discrepancy is unresolved.
  • Cargo.lock: All leanVM crates and their transitive graph updated; Plonky3 p3-* crates moved from 82cfad73 to bde2e86e, directly contradicting the PR description's assertion that they remain frozen at 82cfad73 to avoid a Rust 1.92.0 build failure caused by the maybe_uninit_slice nightly feature.

Confidence Score: 3/5

Two 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.

Important Files Changed

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)"]
Loading

Comments Outside Diff (1)

  1. Cargo.lock, line 5306-5310 (link)

    P1 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.

    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

@MegaRedHand MegaRedHand marked this pull request as draft June 8, 2026 19:06
@MegaRedHand

Copy link
Copy Markdown
Collaborator Author

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.
@MegaRedHand MegaRedHand force-pushed the chore/bump-leanvm-8eec56c branch from 87b53f1 to f8f50c9 Compare June 8, 2026 22:04
@MegaRedHand MegaRedHand changed the title chore: bump leanVM to 8eec56c chore: bump leanVM to a44bca4 Jun 8, 2026
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.
@MegaRedHand MegaRedHand marked this pull request as ready for review June 9, 2026 21:16
@MegaRedHand MegaRedHand changed the title chore: bump leanVM to a44bca4 chore: bump leanVM to 2498896 Jun 9, 2026
@github-actions

github-actions Bot commented Jun 9, 2026

Copy link
Copy Markdown

🤖 Claude Code Review

PR Review: chore: bump leanVM to 2498896

Scope: Two files changed — crates/common/crypto/Cargo.toml (two rev updates) and Cargo.lock (transitive churn). No ethlambda source code is modified.


Summary of what changed in the lock file

  • All leanVM-internal crates (backend, lean-multisig, lean_compiler, lean_prover, lean_vm, leansig_wrapper, all mt-*, rec_aggregation, sub_protocols, system-info, utils, zk-alloc) bump from f66d4a92498896.
  • New crate introduced: parallel v0.1.0 (from leanVM), which replaces rayon across the proving sub-system (mt-field, mt-fiat-shamir, mt-poly, mt-sumcheck, mt-symetric, mt-whir, backend, lean-multisig, sub_protocols, zk-alloc).
  • Plonky3 p3-* crates advance from 82cfad73bde2e86e.
  • spin downgrades from 0.11.00.10.0.
  • Several windows-sys consumers move 0.52.0/0.60.20.61.2.
  • p3-poseidon1 loses its p3-mds dependency (Plonky3-internal refactor).

Findings

rayon → custom parallel crate (leanVM-internal)

The most notable structural change. rayon is battle-tested with a known safety story. parallel (sourced from leanEthereum/leanVM) is opaque from ethlambda's perspective — its only explicit dependency is system-info (also from leanVM). This replacement touches every hot path in the WHIR/STARK prover: field operations, DFT, sumcheck, summetric, poly commitments.

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 rayon's work-stealing scheduler has different characteristics than whatever parallel implements.

spin downgrade: 0.11.00.10.0

This is a regression in lock-file terms and worth understanding. 0.11.0 is a newer release; 0.10.0 is an older one being pulled in by Plonky3 bde2e86e. Both versions are past the historical RUSTSEC-2019-0031 advisory (fixed in 0.5.3), so there's no known vulnerability here. It is slightly surprising to see a version decrease, but it's entirely driven by Plonky3's own pinned dependency tree, not by anything ethlambda controls.

Plonky3 bde2e86e pin (correctly motivated)

The PR description explains the rationale: newer Plonky3 commits require maybe_uninit_slice (unstable feature, doesn't build on Rust 1.92.0). Pinning to bde2e86e — the commit locked by leanVM's own Cargo.lock — is the correct approach to keep a reproducible build on stable Rust.

p3-poseidon1 loses p3-mds

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 (xmss_aggregate, AggregatedXMSS::compress/decompress) are unchanged. The author ran cargo test -p ethlambda-crypto --release -- --ignored (the slow XMSS tests that are normally skipped) and they pass. This is the right validation step.

Both Cargo.toml and Cargo.lock are consistent

lean-multisig and leansig_wrapper in Cargo.toml both point to rev = "2498896", and the lock file reflects exactly that rev with the full SHA. No mismatch.


Minor note

The PR description mentions a previous target of 8eec56c, then a44bca4, finally landing at 2498896. The commit history and body are consistent with that history; the Cargo.toml is clean. No stale references left behind.


Verdict

The 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 rayonparallel substitution is the only thing worth watching under devnet load, but it doesn't affect consensus correctness.

Approved with no blocking issues. Recommend a devnet run with aggregation enabled (per the --is-aggregator flag) to confirm proof throughput is stable before merging.


Automated review by Claude (Anthropic) · sonnet · custom prompt

@github-actions

github-actions Bot commented Jun 9, 2026

Copy link
Copy Markdown

🤖 Kimi Code Review

General Assessment
This PR updates the leanVM dependency (providing XMSS post-quantum signature aggregation) from revision f66d4a9 to 2498896. The diff is primarily Cargo.lock regeneration with significant upstream changes: migration from rayon to a custom parallel crate, introduction of zk-alloc for ZK-specific memory management, and updates to Plonky3 dependencies.


Security & Consensus Critical Concerns

  1. Custom Threading Implementation (High Risk)

    • Cargo.lock lines 668, 1119, 1130, 1144, 1159, 1173, 1185, 1200, 1212: The upstream migration replaces rayon with a custom parallel crate across all cryptographic components (backend, mt-field, mt-poly, mt-whir, etc.).
    • Risk: Consensus clients require deterministic execution across heterogeneous hardware. Custom thread pools may introduce non-deterministic ordering or race conditions not present in battle-tested rayon.
    • Action: Verify the parallel crate implements deterministic work-stealing or fixed thread scheduling. Test with rayon fallback to ensure state root consistency.
  2. XMSS Signature Verification Changes

    • crates/common/crypto/Cargo.toml lines 17, 20: Updates lean-multisig and leansig_wrapper revisions.
    • Risk: XMSS (post-quantum signatures) verification logic changes could break consensus if validation rules diverge between nodes on different versions during a network upgrade.
    • Action: Request a diff summary of lean-multisig changes between these commits. Ensure signature aggregation batching behavior remains identical.
  3. Custom Allocator Introduction (zk-alloc)

    • Cargo.lock lines 668, 1144, 1159, 1173, 1185, 1200, 9053: New zk-alloc dependency injected into cryptographic pipeline with parallel and system-info dependencies.
    • Risk: Custom allocators in Rust (#[global_allocator] or scoped allocators) can introduce memory leaks, use-after-free in edge cases, or stack overflow handling differences that affect ZK proof verification.
    • Action: Audit zk-alloc for unsafe code blocks. Verify it doesn't override the global allocator in a way that affects non-ZK components.

Performance & Correctness

  1. Spin Lock Downgrade

    • Cargo.lock lines 5370, 5535: spin downgraded from 0.11.0 to 0.10.0 in Plonky3 dependencies.
    • Concern: Version 0.11.0 contained fixes for soundness issues in RwLock implementations. Downgrading may reintroduce these.
    • Action: Confirm this downgrade is intentional (likely forced by Plonky3's Cargo.toml) and not a Cargo resolution bug.
  2. Poseidon1 MDS Removal

    • Cargo.lock lines 5545-5550: p3-poseidon1 no longer depends on p3-mds.
    • Concern: MDS (Maximum Distance Separable) matrices are critical for Poseidon hash security. Removing the dependency suggests a refactor of the permutation logic.
    • Action: Verify this aligns with the latest Plonky3 security audit. Ensure the MDS logic wasn't moved to an inline implementation with potential constant-time violations.

Code Quality

  1. Dependency Transparency

    • Cargo.lock shows 40+ transitive dependency changes. For a consensus client, this is a large blast radius.
    • Suggestion: Pin the leanVM dependencies in Cargo.toml with stricter version constraints or consider vendoring critical cryptographic code to reduce supply chain risk.
  2. Windows-sys Version Uniformity

    • Multiple windows-sys bumps to 0.61.2 (lines 177, 188, 1943, 5107, etc.).
    • Note: This is likely benign, but ensure FFI boundaries in system-info (which uses libc and platform-specific code) handle the new Windows API mappings correctly.

Requested Changes

  1. Block merging until providing:

    • Link to leanVM diff between f66d4a9 and 2498896
    • Benchmark results comparing rayon vs parallel throughput for XMSS aggregation
    • Proof that parallel crate maintains deterministic execution across x86_64 and ARM64
  2. Add to crates/common/crypto/Cargo.toml (line 15-20): Consider adding a comment documenting the cryptographic audit status of the specific leanVM revision:

    # Audit: Revision 2498896 reviewed for XMSS correctness (date: YYYY-MM-DD)
  3. Verify zk-alloc safety: If it uses unsafe, request a miri test run or formal verification of the allocation paths.

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

Comment on lines +17 to 18
lean-multisig = { git = "https://github.com/leanEthereum/leanVM.git", rev = "2498896" }
# leansig_wrapper provides XmssPublicKey/XmssSignature types used by lean-multisig's public API

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.

P1 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.

@github-actions

github-actions Bot commented Jun 9, 2026

Copy link
Copy Markdown

🤖 Codex Code Review

Findings

  1. Medium: this bumps the consensus-critical XMSS verifier/aggregator to a new upstream commit in crates/common/crypto/Cargo.toml, but the repo still has no non-ignored success-path test that exercises real aggregate proofs. The local crypto tests are all ignored in crates/common/crypto/src/lib.rs, and the signature spec harness still constructs SignedBlock through the lossy conversion in crates/blockchain/tests/signature_spectests.rs, which explicitly drops proofData per crates/common/test-fixtures/src/verify_signatures.rs. For a dependency update that can change proof serialization/verification semantics, that is too little coverage. I’d want at least one non-ignored fixture-backed success case using try_into_signed_block_with_proofs() before merging.

  2. Low: the PR updates lean-multisig / leansig_wrapper to leanVM rev 2498896 in crates/common/crypto/Cargo.toml, but the workspace-level leansig dependency remains independently sourced and effectively decoupled in Cargo.toml, with the lockfile still pinned to an older leanSig commit in Cargo.lock. That leaves signer/verifier compatibility depending on lockfile state rather than an explicit version relationship. For crypto code, I’d pin leansig to the exact commit known-good with leanVM 2498896 to avoid silent drift on the next cargo update.

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

@MegaRedHand MegaRedHand merged commit df8020b into main Jun 9, 2026
2 of 3 checks passed
@MegaRedHand MegaRedHand deleted the chore/bump-leanvm-8eec56c branch June 9, 2026 21:35
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.

2 participants