Migrate from Poseidon2 to Poseidon1 signature scheme

[pablodeymo]

Motivation

Poseidon2 is not cryptographically safe. The upstream leanSpec, leansig, and leanMultisig libraries have already migrated to Poseidon1. This PR aligns ethlambda with the new scheme.

Description

Dependency changes

• Bump leansig to devnet4 branch (Poseidon1 internally)
• Bump lean-multisig to devnet4 HEAD (fd88140)
• Add leansig_wrapper for XmssPublicKey/XmssSignature types
• Remove ethereum_ssz from crypto crate (replaced by postcard+lz4 serialization)
• Bump rand from 0.9 to 0.10

Signature scheme change

• Switch from SIGTopLevelTargetSumLifetime32Dim64Base8 (V=64, TopLevelPoseidon) to SchemeAbortingTargetSumLifetime32Dim46Base8 (V=46, AbortingHypercube)
• SIGNATURE_SIZE updated from 3112 to 2536 bytes

API adaptation in ethlambda-crypto

• xmss_aggregate_signatures -> xmss_aggregate (with children + log_inv_rate params)
• xmss_verify_aggregated_signatures -> xmss_verify_aggregation
• Devnet2XmssAggregateSignature -> AggregatedXMSS
• SSZ serialization -> AggregatedXMSS::serialize()/deserialize()

leanSpec bump

• Bump to latest HEAD (includes Poseidon1 migration + tiebreaker fix)
• Remove patches/ directory (no longer needed)
• Adapt test harness for new fixture format:
  • Resolve headRootLabel from block registry when explicit root not provided
  • justifiedSlots changed from Vec<u64> to Vec<bool> (proper bitlist)
  • SignedBlock.message renamed to SignedBlock.block in fixtures
• CI: generate prod keys locally (upstream keys still Poseidon2)

Cross-client tests

• Ream test vectors marked #[ignore] pending Poseidon1 vectors from ream team

How to Test

make fmt
make lint
make test

[github-actions]

🤖 Kimi Code Review

This PR updates ethlambda to align with the devnet4 specification, particularly migrating from Poseidon2-based XMSS signatures to Poseidon1 with aborting hypercube message hashing. The changes are substantial but well-structured.

Critical Consensus Changes

1. Signature Scheme Migration (crates/common/types/src/signature.rs:13-15)

• Changes from instantiations_poseidon_top_level::lifetime_2_to_the_32::hashing_optimized::SIGTopLevelTargetSumLifetime32Dim64Base8 to instantiations_aborting::lifetime_2_to_the_32::SchemeAbortingTargetSumLifetime32Dim46Base8
• Security impact: This reduces signature size from 3112 to 2536 bytes (Poseidon1 is more compact) but changes the underlying hash primitive. Ensure this matches the target devnet4 spec exactly.

2. Signature Size Constant (crates/common/types/src/signature.rs:28-31)

• Updated SIGNATURE_SIZE from 3112 to 2536 bytes
• The comment calculates: path(32*8*4) + rho(7*4) + hashes(46*8*4) + ssz_offsets(3*4) = 2536
• Verify: Cross-check this arithmetic against the actual leansig implementation to prevent deserialization failures.

3. Cross-Client Test Vectors (crates/common/crypto/src/lib.rs:336-357)

• Three tests marked #[ignore] pending Poseidon1 vectors from ream client
• Action required: Ensure tracking issue exists to update these vectors before mainnet; consensus clients must agree on signature format.

API & Dependency Updates

4. lean-multisig API Migration (crates/common/crypto/src/lib.rs:95-99)

• Old: xmss_aggregate_signatures(&lean_pubkeys, &lean_sigs, ...) returning Result
• New: xmss_aggregate(&[], raw_xmss, &message.0, slot, 1) returning tuple with _sorted_pubkeys
• Note: Error handling changed from Result to panic-based (evidenced by AggregationPanicked error type). Consider if this is acceptable for production or if the library should be wrapped to catch panics.

5. Serialization Change (crates/common/crypto/src/lib.rs:105)

• Removed ethereum_ssz dependency for signature serialization
• Now uses aggregate.serialize() and AggregatedXMSS::deserialize()
• Validation: Ensure the new serialization format is canonical and versioned for future compatibility.

Test Infrastructure

6. State Transition Test Fix (crates/blockchain/state_transition/tests/stf_spectests.rs:197-202)

• justified_slots changed from Vec<u64> (indices) to Vec<bool>
• Comparison logic simplified to boolean equality check
• Correctness: This aligns with the spec change where justified slots are represented as a bitfield rather than indices.

7. Fork Choice Label Resolution (crates/blockchain/tests/forkchoice_spectests.rs:191-210)

• Added resolution of *_label fields to actual roots using block_registry
• Previously returned errors for label-only checks; now functional
• Improvement: This enables testing with label-based specifications.

Build & CI

8. CI Cleanup (.github/workflows/ci.yml:81-106)

• Removed production keys download and caching steps
• Verify: Ensure test fixtures in leanSpec at commit 87670883282e631f8abab10fd5f71b46fedc730d no longer require external prod keys.

9. Patch Removal (patches/fix-lexicographic-tiebreaker-test.patch)

• Deleted; fix upstreamed to leanSpec
• Makefile updated to new commit hash without patch application
• Check: Confirm the lexicographic tiebreaker test passes without the patch.

Minor Issues

10. Error Message Formatting (crates/blockchain/tests/signature_types.rs:127-131)

• Good improvement using SIGNATURE_SIZE constant in error message instead of hardcoded 3112.

11. Docstring Accuracy (crates/common/crypto/src/lib.rs:116-121)

• Docstring lists proof_data first but function signature has it first—correct. However, the description "public keys... message... proof data" order in docstring doesn't match the actual parameter order in the function signature (proof_data, public_keys, message, slot). Fix: Align docstring parameter order with function signature.

Summary

The PR correctly implements the devnet4 signature scheme changes. The primary risks are:

1. Consensus compatibility: Ensure all other clients (ream, etc.) have updated to Poseidon1
2. Panic handling: The new aggregation API appears to panic on failure rather than return Result—verify this is intentional and safe
3. Serialization stability: New serialize/deserialize methods need fuzz testing

Recommendation: Add a TODO comment in crates/common/crypto/src/lib.rs near the ignored tests linking to the ream client issue for Poseidon1 test vectors.

---

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

[github-actions]

🤖 Claude Code Review

Now I have enough context. Let me write the review.

---

Review: Migrate from Poseidon2 to Poseidon1 signature scheme

Overall this is a well-scoped migration that tracks upstream leansig/leanSpec changes. The fixture harness adaptations and label-resolution improvements are clean. A few points need attention before merging.

---

Critical

xmss_aggregate is infallible — AggregationPanicked is dead code and panics are uncaught

crates/common/crypto/src/lib.rs

The old API returned Result<_, XmssAggregateError> and failures were mapped into AggregationError::AggregationFailed. The new call is:

let (_sorted_pubkeys, aggregate) = xmss_aggregate(&[], raw_xmss, &message.0, slot, 1);

xmss_aggregate returns a plain tuple — no Result. If the library panics internally (which the new AggregationPanicked variant implicitly acknowledges is a possibility), that panic propagates straight up the call stack and crashes the node.

Two things need addressing:

• Either wrap the call in std::panic::catch_unwind and map to AggregationError::AggregationPanicked, or remove the dead variant if panics genuinely can't happen.
• Clarify in a comment whether xmss_aggregate is documented as panic-free by the upstream library.

---

Significant

Floating branch reference for leansig

Cargo.toml

-leansig = { git = "...", rev = "73bedc26ed961b110df7ac2e234dc11361a4bf25" }
+leansig = { git = "...", branch = "devnet4" }

A branch = reference is non-reproducible — any future push to devnet4 silently changes what gets compiled. The Cargo.lock currently pins the actual resolved commit, but that's easy to accidentally update. For a consensus client where byte-for-byte reproducibility of the cryptographic layer is security-critical, this should be pinned to a specific commit hash (rev =), consistent with how lean-multisig and leansig_wrapper are pinned in crypto/Cargo.toml.

---

Discarded _sorted_pubkeys from xmss_aggregate

crates/common/crypto/src/lib.rs

let (_sorted_pubkeys, aggregate) = xmss_aggregate(&[], raw_xmss, &message.0, slot, 1);

The function returns a sorted permutation of the input pubkeys alongside the aggregate proof. If xmss_verify_aggregation expects keys in the order used during aggregation (i.e., the sorted order, not the call-site order), discarding this could cause verification to fail at peers who receive the proof. The call site for verification passes lean_pubkeys constructed from the original (unsorted) validator ordering.

This needs explicit confirmation from the lean-multisig docs: does xmss_verify_aggregation accept keys in any order, or does it require the sorted order returned by xmss_aggregate? If the latter, both the sorted keys and the original-to-sorted mapping need to be stored alongside the proof.

---

Hardcoded log_inv_rate = 1 without documentation

crates/common/crypto/src/lib.rs

xmss_aggregate(&[], raw_xmss, &message.0, slot, 1)

The last parameter controls proof rate (likely FRI/STARK blowup factor). Its value directly affects proof size and soundness. There's no comment explaining what 1 means or why it was chosen. A named constant and a one-line explanation of the tradeoff (proof size vs. soundness bits) would be appropriate given this is a security-critical parameter.

---

Moderate

stf_spectests.rs justified_slots comparison may miss trailing bits in post state

crates/blockchain/state_transition/tests/stf_spectests.rs

let post_slots: Vec<bool> = (0..justified_slots.data.len())
    .map(|i| post_state.justified_slots.get(i) == Some(true))
    .collect();

The range is bounded by justified_slots.data.len() (the fixture length), not post_state.justified_slots.len(). If the post state has slots set beyond the fixture's specified range, they won't be compared. This is probably intentional (fixture only specifies the slots it cares about), but it differs from the old behavior where both Vec<u64> collections had to match exactly. A comment explaining the truncation-at-fixture-length behavior would prevent future confusion.

---

Cross-client ream test vectors ignored with no tracking issue

crates/common/crypto/src/lib.rs

Three #[ignore] tests cover the cross-client verification path — the highest-risk interoperability scenario. The TODO comment is appropriate, but there's no associated issue or milestone. Given this is a devnet-targeting branch, these should be re-enabled before any mainnet or testnet release. Consider adding a GitHub issue reference in the ignore reason string.

---

Minor

• signature_types.rs: The #[serde(alias = "message")] on block is correct and preserves backward compatibility with older fixtures. Good.
• signature.rs: Using SIGNATURE_SIZE in the deserialization error message instead of the hardcoded 3112 is a clean improvement.
• The label-resolution logic in forkchoice_spectests.rs (resolving headRootLabel via block_registry) correctly replaces the old hard error and is the right pattern for the new fixture format.
• The patches/ directory removal is clean now that the tiebreaker fix has landed upstream.

---

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

[github-actions]

🤖 Codex Code Review

1. leansig is no longer pinned to an immutable revision in Cargo.toml. Moving from rev = ... to branch = "devnet4" makes consensus-critical XMSS behavior depend on whatever that branch points to the next time Cargo.lock is regenerated. For a blockchain client, that is a real supply-chain / reproducibility risk: an upstream force-push or unreviewed branch move can silently change signature semantics.

2. The fork-choice spectest runner now silently drops root assertions when a label is missing from the registry instead of failing fast. In forkchoice_spectests.rs, head_root_label, latest_justified_root_label, and latest_finalized_root_label are resolved with and_then(...); if the label is absent or misspelled, the result becomes None and the corresponding check is skipped. That weakens the test harness and can let incorrect LMD-GHOST / checkpoint behavior pass unnoticed. The previous explicit error on label-without-root was safer; if label resolution is intended, unresolved labels should still return an error.

3. The XMSS test vectors were only partially updated for the new Poseidon1 / 2536-byte signature format. Production now expects 2536-byte signatures in signature.rs and signature.rs, but the ream compatibility tests still embed a 3112-byte Poseidon2 vector in lib.rs, and test_cross_client_decode_ream_signature still hard-expects it to deserialize in lib.rs. The verify tests were ignored, but the decode test was not. As written, that test should fail until the vector is regenerated or the decode test is also gated.

4. The state-transition spectest change weakens justified_slots validation. In stf_spectests.rs, comparison now only checks 0..expected_len, so any extra trailing true bits in post_state.justified_slots are ignored. That can hide stale-justification-window bugs in justification/finalization logic. If the fixture format changed to boolean vectors, the harness should still compare lengths or normalize both sides to the same full window before asserting equality.

I did not find a clear production-path bug in the XMSS wrapper from the local code alone, but I could not run cargo test: the sandbox blocks writes under ~/.cargo/~/.rustup, and network fetches are also unavailable.

---

Automated review by OpenAI Codex · gpt-5.4 · custom prompt

[greptile-apps]

Greptile Summary

This PR migrates ethlambda from Poseidon2 (SIGTopLevelTargetSumLifetime32Dim64Base8, 3112-byte signatures) to Poseidon1 (SchemeAbortingTargetSumLifetime32Dim46Base8, 2536-byte signatures), aligning with upstream leanSpec, leansig, and lean-multisig. The test harness is also updated for new fixture formats (justifiedSlots as Vec<bool>, SignedBlock.block rename, headRootLabel resolution).

• Two non-ignored tests in crates/common/crypto/src/lib.rs (test_cross_client_decode_ream_public_key, test_cross_client_decode_ream_signature) decode 3112-byte Poseidon2 vectors through the new 2536-byte Poseidon1 deserializer and will fail.
• The test helper generate_keypair_and_sign still uses the old Poseidon2 LeanSignatureScheme alias; the ignored aggregation/verification tests are silently broken and will error when un-ignored.

Confidence Score: 4/5

Two non-ignored tests that decode Poseidon2 byte vectors through the new Poseidon1 deserializer will fail in CI; fix required before merge.

The core migration (types, crypto crate API, test harness format changes) is correct and well-structured. However, two active non-ignored tests in crypto/src/lib.rs will fail because they round-trip 3112-byte Poseidon2 data through the new 2536-byte Poseidon1 deserializer, and the ignored aggregation test helper is silently broken.

crates/common/crypto/src/lib.rs — the cross-client decode tests and the test helper LeanSignatureScheme alias both reference the old Poseidon2 scheme.

Important Files Changed

Filename	Overview
crates/common/crypto/src/lib.rs	Core crypto layer migrated to Poseidon1 API (xmss_aggregate, AggregatedXMSS::serialize); two non-ignored tests decode Poseidon2 vectors with the new 2536-byte scheme and will fail, and the test helper still generates keys with the old Poseidon2 type alias.
crates/common/types/src/signature.rs	Signature scheme correctly migrated to SchemeAbortingTargetSumLifetime32Dim46Base8 (Poseidon1) and SIGNATURE_SIZE updated from 3112 to 2536 bytes.
.github/workflows/ci.yml	CI fixture generation command diverges from Makefile: uses --fork=Devnet -n 2 without -o fixtures; the missing output flag may cause fixtures to land in a non-cached directory.
crates/blockchain/tests/forkchoice_spectests.rs	Test harness correctly extended to resolve headRootLabel/latestJustifiedRootLabel/latestFinalizedRootLabel from block_registry when explicit root not provided.
crates/blockchain/tests/signature_types.rs	TestSignedBlock now accepts both 'block' and 'message' field names via serde alias, correctly adapting to renamed fixture field.
crates/blockchain/state_transition/tests/types.rs	justified_slots changed from Vec to Vec correctly, matching the new fixture format.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A[Gossip Signature\nXmssSignature - 2536 bytes] --> B[ethlambda-crypto\naggregate_signatures]
    B --> C[xmss_aggregate\nlean-multisig fd88140]
    C --> D[AggregatedXMSS::serialize\npostcard+lz4]
    D --> E[ByteListMiB\nstored as proof]

    E --> F[verify_aggregated_signature]
    F --> G[AggregatedXMSS::deserialize]
    G --> H[xmss_verify_aggregation\nlean-multisig]
    H --> I{Valid?}
    I -->|Yes| J[Block accepted]
    I -->|No| K[VerificationError]

    subgraph Scheme
        S1["Old: SIGTopLevelTargetSumLifetime32Dim64Base8\nPoseidon2 - 3112 bytes"]
        S2["New: SchemeAbortingTargetSumLifetime32Dim46Base8\nPoseidon1 - 2536 bytes"]
        S1 -->|Replaced by| S2
    end

Loading

Comments Outside Diff (3)

1. crates/common/crypto/src/lib.rs, line 156 (link)

   Test helper uses old Poseidon2 scheme — byte conversion will fail when un-ignored

   The local LeanSignatureScheme alias still points to the old SIGTopLevelTargetSumLifetime32Dim64Base8 (Poseidon2, 3112-byte signatures). generate_keypair_and_sign generates a signature with this type and then feeds the raw bytes to ValidatorSignature::from_bytes(), which now expects 2536 bytes (Poseidon1). The conversion will fail with a parse error.

   The tests that call this helper (test_aggregate_single_signature, test_aggregate_multiple_signatures, etc.) are currently #[ignore]d, so they don't break CI today, but they are silently broken and will all fail when the ignore is removed. Update the local alias to the Poseidon1 type used in ethlambda-types:

   Prompt To Fix With AI

   This is a comment left during a code review.
   Path: crates/common/crypto/src/lib.rs
   Line: 156
   
   Comment:
   **Test helper uses old Poseidon2 scheme — byte conversion will fail when un-ignored**
   
   The local `LeanSignatureScheme` alias still points to the old `SIGTopLevelTargetSumLifetime32Dim64Base8` (Poseidon2, 3112-byte signatures). `generate_keypair_and_sign` generates a signature with this type and then feeds the raw bytes to `ValidatorSignature::from_bytes()`, which now expects 2536 bytes (Poseidon1). The conversion will fail with a parse error.
   
   The tests that call this helper (`test_aggregate_single_signature`, `test_aggregate_multiple_signatures`, etc.) are currently `#[ignore]`d, so they don't break CI today, but they are silently broken and will all fail when the ignore is removed. Update the local alias to the Poseidon1 type used in `ethlambda-types`:
   
   
   
   How can I resolve this? If you propose a fix, please make it concise.

2. crates/common/crypto/src/lib.rs, line 322-332 (link)

   Active decode tests will fail with new signature size

   test_cross_client_decode_ream_signature (line 329) is not marked #[ignore], but REAM_SIGNATURE_HEX encodes a 3112-byte Poseidon2 signature. After this PR, ValidatorSignature::from_bytes() calls the Poseidon1 scheme's deserializer which expects 2536 bytes — the decode will fail and this test will error in CI. The three verify tests are correctly ignored, but these two decode tests need the same #[ignore = "Poseidon2 test vectors - needs Poseidon1 vectors from ream"] annotation, or Poseidon1 test vectors must replace the constants.

   Prompt To Fix With AI

   This is a comment left during a code review.
   Path: crates/common/crypto/src/lib.rs
   Line: 322-332
   
   Comment:
   **Active decode tests will fail with new signature size**
   
   `test_cross_client_decode_ream_signature` (line 329) is not marked `#[ignore]`, but `REAM_SIGNATURE_HEX` encodes a 3112-byte Poseidon2 signature. After this PR, `ValidatorSignature::from_bytes()` calls the Poseidon1 scheme's deserializer which expects 2536 bytes — the decode will fail and this test will error in CI. The three _verify_ tests are correctly ignored, but these two decode tests need the same `#[ignore = "Poseidon2 test vectors - needs Poseidon1 vectors from ream"]` annotation, or Poseidon1 test vectors must replace the constants.
   
   How can I resolve this? If you propose a fix, please make it concise.

3. .github/workflows/ci.yml, line 87 (link)

   CI fixture command diverges from Makefile

   The CI fixture generation command is uv run fill --fork=Devnet --scheme prod -n 2 while the Makefile uses uv run fill --fork devnet --scheme=prod -o fixtures. The CI command omits -o fixtures and adds -n 2. If the fill tool doesn't default output to fixtures/, the cache path leanSpec/fixtures will be empty and subsequent test runs will fail to find fixtures. Confirm the default output path matches leanSpec/fixtures, or add -o fixtures explicitly for consistency.

   Prompt To Fix With AI

   This is a comment left during a code review.
   Path: .github/workflows/ci.yml
   Line: 87
   
   Comment:
   **CI fixture command diverges from Makefile**
   
   The CI fixture generation command is `uv run fill --fork=Devnet --scheme prod -n 2` while the Makefile uses `uv run fill --fork devnet --scheme=prod -o fixtures`. The CI command omits `-o fixtures` and adds `-n 2`. If the `fill` tool doesn't default output to `fixtures/`, the cache path `leanSpec/fixtures` will be empty and subsequent test runs will fail to find fixtures. Confirm the default output path matches `leanSpec/fixtures`, or add `-o fixtures` explicitly for consistency.
   
   How can I resolve this? If you propose a fix, please make it concise.

Prompt To Fix All With AI

This is a comment left during a code review.
Path: crates/common/crypto/src/lib.rs
Line: 156

Comment:
**Test helper uses old Poseidon2 scheme — byte conversion will fail when un-ignored**

The local `LeanSignatureScheme` alias still points to the old `SIGTopLevelTargetSumLifetime32Dim64Base8` (Poseidon2, 3112-byte signatures). `generate_keypair_and_sign` generates a signature with this type and then feeds the raw bytes to `ValidatorSignature::from_bytes()`, which now expects 2536 bytes (Poseidon1). The conversion will fail with a parse error.

The tests that call this helper (`test_aggregate_single_signature`, `test_aggregate_multiple_signatures`, etc.) are currently `#[ignore]`d, so they don't break CI today, but they are silently broken and will all fail when the ignore is removed. Update the local alias to the Poseidon1 type used in `ethlambda-types`:

```suggestion
    type LeanSignatureScheme = leansig::signature::generalized_xmss::instantiations_aborting::lifetime_2_to_the_32::SchemeAbortingTargetSumLifetime32Dim46Base8;
```

How can I resolve this? If you propose a fix, please make it concise.

---

This is a comment left during a code review.
Path: crates/common/crypto/src/lib.rs
Line: 322-332

Comment:
**Active decode tests will fail with new signature size**

`test_cross_client_decode_ream_signature` (line 329) is not marked `#[ignore]`, but `REAM_SIGNATURE_HEX` encodes a 3112-byte Poseidon2 signature. After this PR, `ValidatorSignature::from_bytes()` calls the Poseidon1 scheme's deserializer which expects 2536 bytes — the decode will fail and this test will error in CI. The three _verify_ tests are correctly ignored, but these two decode tests need the same `#[ignore = "Poseidon2 test vectors - needs Poseidon1 vectors from ream"]` annotation, or Poseidon1 test vectors must replace the constants.

How can I resolve this? If you propose a fix, please make it concise.

---

This is a comment left during a code review.
Path: .github/workflows/ci.yml
Line: 87

Comment:
**CI fixture command diverges from Makefile**

The CI fixture generation command is `uv run fill --fork=Devnet --scheme prod -n 2` while the Makefile uses `uv run fill --fork devnet --scheme=prod -o fixtures`. The CI command omits `-o fixtures` and adds `-n 2`. If the `fill` tool doesn't default output to `fixtures/`, the cache path `leanSpec/fixtures` will be empty and subsequent test runs will fail to find fixtures. Confirm the default output path matches `leanSpec/fixtures`, or add `-o fixtures` explicitly for consistency.

How can I resolve this? If you propose a fix, please make it concise.

_{Reviews (1): Last reviewed commit: "Bump leanSpec to HEAD (8767088), remove ..." | Re-trigger Greptile}