chore(deps): bump leanMultisig to f66d4a9#400
Conversation
Bump lean-multisig and leansig_wrapper from 5eba3b1 to f66d4a9.
The new rev renames the AggregatedXMSS wire (de)serialization API and
makes xmss_aggregate fallible:
- AggregatedXMSS::serialize/deserialize -> compress/decompress
- xmss_aggregate now returns Result<_, AggregationError> instead of
panicking on cryptographically invalid child proofs
Adapt ethlambda-crypto accordingly: wrap the upstream AggregationError
in our AggregationError via #[from] and propagate with `?`. Public API
of ethlambda-crypto is unchanged.
🤖 Kimi Code ReviewSecurity & Dependencies
Consensus & API Safety
Code Quality
Minor
Recommendation: Before merging, audit the Automated review by Kimi (Moonshot AI) · kimi-k2.5 · custom prompt |
🤖 Claude Code ReviewHere is the full review output: PR #400:
|
🤖 Codex Code ReviewFindings:
The error propagation change from panic-prone aggregation to I couldn’t compile this locally because the new Automated review by OpenAI Codex · gpt-5.4 · custom prompt |
Greptile SummaryBumps the
Confidence Score: 4/5The change is a focused dependency bump with a clean, mechanical API adaptation; the core aggregation and verification paths are structurally unchanged. All three aggregation functions correctly swap the panic path for ?-propagated errors, and the rename of serialize/deserialize to compress/decompress is applied consistently at every call site. The only minor gaps are a doc comment that describes only one of the two conditions under which AggregationError::Aggregation can be returned, and a new macOS Objective-C transitive dependency (objc2-foundation) worth verifying on Linux CI targets. The Cargo.lock entry for rec_aggregation is worth a second look due to the newly added objc2/objc2-foundation dependencies.
|
| Filename | Overview |
|---|---|
| crates/common/crypto/src/lib.rs | API adaptation for new leanMultisig rev: serialize/deserialize → compress/decompress, xmss_aggregate now propagated with ?, and LeanAggregationError wrapped via #[from]. Changes are minimal and correct; aggregate_proofs and aggregate_signatures doc comments lack # Errors sections for the newly possible Aggregation error variant. |
| crates/common/crypto/Cargo.toml | Both lean-multisig and leansig_wrapper git revs bumped from 5eba3b1 to f66d4a9; no other changes. |
| Cargo.lock | All leanMultisig sub-crates updated to the new revision. New transitive dependencies include include_dir, system-info, zk-alloc, objc2-foundation, and serde/tracing additions in several crates. num-bigint is downgraded from 0.4.6 → 0.3.3 in mt-field and mt-koala-bear (both versions can coexist); the new macOS objc2-foundation crate is worth verifying on Linux CI targets. |
Flowchart
%%{init: {'theme': 'neutral'}}%%
flowchart TD
A[aggregate_signatures / aggregate_mixed / aggregate_proofs] --> B[xmss_aggregate]
B -->|Ok| C[serialize_aggregate → compress]
B -->|Err LeanAggregationError| D[AggregationError::Aggregation via #from]
E[deserialize_children] --> F[AggregatedXMSS::decompress]
F -->|None| G[AggregationError::ChildDeserializationFailed]
F -->|Some| H[AggregatedXMSS value]
H --> B
C --> I[ByteListMiB returned to caller]
D --> J[Result::Err returned to caller]
G --> J
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
crates/common/crypto/src/lib.rs:124-127
**Doc comment covers only one of two failure modes for `xmss_aggregate`**
The updated `# Errors` section says the `Aggregation` error fires when a *deserialized child proof* is cryptographically invalid, but `xmss_aggregate` can also return an error when *raw XMSS signatures* are invalid (e.g. signed against a different message or slot). A caller who relies solely on the doc comment to understand what triggers `AggregationError::Aggregation` would have an incomplete picture, which could matter when deciding how to handle or log the error at the call site.
### Issue 2 of 2
Cargo.lock:6378-6396
**New macOS-only transitive dependencies pulled in via `rec_aggregation`**
`objc2-foundation` (v0.3.2) and the `objc2` dependency it wraps are Objective-C runtime bindings for Apple platforms. They are freshly added to `rec_aggregation` in this bump. While the PR confirms `cargo build --workspace` succeeds, it would be worth verifying that these crates compile cleanly on the Linux targets used by CI/CD and deployment (they typically provide stub implementations on non-Apple platforms, but cross-platform correctness is worth a quick check).
Reviews (1): Last reviewed commit: "chore(deps): bump leanMultisig to f66d4a..." | Re-trigger Greptile
| /// # Errors | ||
| /// | ||
| /// Panics if any deserialized child proof is cryptographically invalid (e.g., was | ||
| /// produced for a different message or slot). This is an upstream constraint of | ||
| /// `xmss_aggregate`. | ||
| /// Returns [`AggregationError::Aggregation`] if any deserialized child proof is | ||
| /// cryptographically invalid (e.g., was produced for a different message or slot). |
There was a problem hiding this comment.
Doc comment covers only one of two failure modes for
xmss_aggregate
The updated # Errors section says the Aggregation error fires when a deserialized child proof is cryptographically invalid, but xmss_aggregate can also return an error when raw XMSS signatures are invalid (e.g. signed against a different message or slot). A caller who relies solely on the doc comment to understand what triggers AggregationError::Aggregation would have an incomplete picture, which could matter when deciding how to handle or log the error at the call site.
Prompt To Fix With AI
This is a comment left during a code review.
Path: crates/common/crypto/src/lib.rs
Line: 124-127
Comment:
**Doc comment covers only one of two failure modes for `xmss_aggregate`**
The updated `# Errors` section says the `Aggregation` error fires when a *deserialized child proof* is cryptographically invalid, but `xmss_aggregate` can also return an error when *raw XMSS signatures* are invalid (e.g. signed against a different message or slot). A caller who relies solely on the doc comment to understand what triggers `AggregationError::Aggregation` would have an incomplete picture, which could matter when deciding how to handle or log the error at the call site.
How can I resolve this? If you propose a fix, please make it concise.Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
| [[package]] | ||
| name = "rec_aggregation" | ||
| version = "0.1.0" | ||
| source = "git+https://github.com/leanEthereum/leanMultisig.git?rev=5eba3b1#5eba3b141455349d7cdbf0f5d3ccfb2e640b02aa" | ||
| source = "git+https://github.com/leanEthereum/leanMultisig.git?rev=f66d4a9#f66d4a974eced803574eb0ea43d812e523c8d7ad" | ||
| dependencies = [ | ||
| "backend", | ||
| "include_dir", | ||
| "lean_compiler", | ||
| "lean_prover", | ||
| "lean_vm", | ||
| "leansig_wrapper", | ||
| "lz4_flex", | ||
| "objc2", | ||
| "objc2-foundation", | ||
| "postcard", | ||
| "rand 0.10.1", | ||
| "serde", | ||
| "sha3 0.11.0", | ||
| "sub_protocols", |
There was a problem hiding this comment.
New macOS-only transitive dependencies pulled in via
rec_aggregation
objc2-foundation (v0.3.2) and the objc2 dependency it wraps are Objective-C runtime bindings for Apple platforms. They are freshly added to rec_aggregation in this bump. While the PR confirms cargo build --workspace succeeds, it would be worth verifying that these crates compile cleanly on the Linux targets used by CI/CD and deployment (they typically provide stub implementations on non-Apple platforms, but cross-platform correctness is worth a quick check).
Prompt To Fix With AI
This is a comment left during a code review.
Path: Cargo.lock
Line: 6378-6396
Comment:
**New macOS-only transitive dependencies pulled in via `rec_aggregation`**
`objc2-foundation` (v0.3.2) and the `objc2` dependency it wraps are Objective-C runtime bindings for Apple platforms. They are freshly added to `rec_aggregation` in this bump. While the PR confirms `cargo build --workspace` succeeds, it would be worth verifying that these crates compile cleanly on the Linux targets used by CI/CD and deployment (they typically provide stub implementations on non-Apple platforms, but cross-platform correctness is worth a quick check).
How can I resolve this? If you propose a fix, please make it concise.Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
Address PR review feedback: - Doc comments: aggregate_signatures and aggregate_proofs gained # Errors sections; aggregate_mixed's now notes that *raw* signatures (not only child proofs) can trigger AggregationError::Aggregation. - Add an (ignored, leanVM-backed) test asserting that re-aggregating a valid child proof under a different message returns AggregationError::Aggregation instead of panicking, exercising the new fallible xmss_aggregate path that the bump introduced.
|
Thanks for the reviews. Triaged below; pushed dd5f3ca for the one actionable item plus a test. Addressed (dd5f3ca)
Verified non-issues
All four |
What
Bumps the
lean-multisigandleansig_wrappergit dependencies from5eba3b1tof66d4a9.Why
Keeps ethlambda current with the latest leanMultisig aggregation code.
API changes adapted
The new rev changed the public aggregation API, so
ethlambda-cryptowas updated to match:AggregatedXMSS::serialize()AggregatedXMSS::compress()AggregatedXMSS::deserialize()AggregatedXMSS::decompress()xmss_aggregate(...) -> (Vec, AggregatedXMSS)-> Result<(Vec, AggregatedXMSS), AggregationError>xmss_aggregateno longer panics on a cryptographically invalid child proof; it returns an error instead. The upstreamAggregationErroris now wrapped in ourAggregationErrorvia#[from]and propagated with?. The public API ofethlambda-cryptois unchanged, so no callers were affected.Testing
cargo build --workspace✅cargo clippy -p ethlambda-crypto -- -D warnings✅cargo fmt --all -- --check✅#[ignore]due to slow leanVM ops)