Add cross-client XMSS compatibility tests (ream)

[pablodeymo]

Motivation

Multi-client interoperability is critical for the lean consensus ecosystem. leanSpec PR #433 introduces cross-client XMSS compatibility tests that verify ream-produced signatures can be decoded and verified by leanSpec's Python implementation. This PR adds the equivalent tests to ethlambda, ensuring our Rust client can also verify ream-generated XMSS signatures.

Description

Adds 5 cross-client XMSS compatibility tests to crates/common/crypto/src/lib.rs using test vectors from ream (epoch 5, all-zeros message):

Test	What it verifies
test_cross_client_decode_ream_public_key	52-byte pubkey roundtrip (decode → encode)
test_cross_client_decode_ream_signature	3112-byte signature roundtrip (decode → encode)
test_cross_client_verify_ream_signature	Signature verifies with correct epoch and message
test_cross_client_ream_signature_rejects_wrong_message	Rejects verification with wrong message (0xff...ff)
test_cross_client_ream_signature_rejects_wrong_epoch	Rejects verification with wrong epoch (6 instead of 5)

These tests only do deserialization + verification (no keygen, no aggregation), so they run fast (~50ms) and don't need #[ignore].

Also adds hex as a dev-dependency to ethlambda-crypto for decoding the test vectors.

How to test

cargo test -p ethlambda-crypto -- cross_client

Related

• leanSpec cross-client tests: xmss: add cross test with ream signature leanEthereum/leanSpec#433

[github-actions]

🤖 Kimi Code Review

Review Summary

The PR adds cross-client XMSS compatibility tests to verify that signatures produced by the ream client can be decoded and verified by ethlambda. Overall, the implementation is solid with good test coverage.

Issues Found

1. Line 326: The comment mentions "3112-byte XMSS signature" but the actual signature is 3114 bytes (1557 hex chars = 3114 bytes). Update the comment to reflect the actual size.

2. Lines 334-335: The ream_pubkey() and ream_signature() functions use expect() which will panic on failure. While acceptable for tests, consider using unwrap_or_else() with better error messages or returning Result for clearer test failures.

3. Missing test: Consider adding a test for epoch boundary conditions (epoch 0 and maximum valid epoch) to ensure robustness.

4. Lines 344-345, 350-351: The roundtrip tests in test_cross_client_decode_ream_public_key and test_cross_client_decode_ream_signature are redundant since the conversion functions are already tested elsewhere. These could be removed to reduce test bloat.

Positive Aspects

• Good separation of test data from test logic
• Comprehensive test coverage for the cross-client compatibility scenario
• Proper use of hex decoding for test vectors
• Clear naming conventions for test functions
• Appropriate use of constants for test data

Suggested Changes

// Line 326: Update comment
/// 3114-byte XMSS signature produced by ream (epoch 5, message all-zeros).

// Lines 334-335: Better error handling
fn ream_pubkey() -> ValidatorPublicKey {
    let bytes = hex::decode(REAM_PUBLIC_KEY_HEX)
        .unwrap_or_else(|_| panic!("invalid public key hex: {}", REAM_PUBLIC_KEY_HEX));
    ValidatorPublicKey::from_bytes(&bytes)
        .unwrap_or_else(|_| panic!("failed to decode ream public key"))
}

The PR is ready to merge after addressing the minor issues above.

---

Automated review by Kimi (Moonshot AI) · custom prompt

[github-actions]

🤖 Codex Code Review

No blocking findings in the diff.

• The PR only adds cross-client XMSS test vectors and test coverage in crates/common/crypto/src/lib.rs:315 plus a test-only dependency in crates/common/crypto/Cargo.toml:24.
• No production-path logic changed (fork choice, attestation handling, justification/finalization, state transition, SSZ runtime behavior are untouched).
• Security/correctness impact is positive: tests now check decode roundtrips and is_valid acceptance/rejection paths for epoch/message mismatches (crates/common/crypto/src/lib.rs:339, crates/common/crypto/src/lib.rs:353, crates/common/crypto/src/lib.rs:363, crates/common/crypto/src/lib.rs:374).

Residual gap:

• I could not execute tests in this environment due sandboxed rustup temp-file permission errors, so runtime verification wasn’t possible here.

---

Automated review by OpenAI Codex · custom prompt

[greptile-apps]

Greptile Summary

This PR adds five cross-client XMSS compatibility tests to crates/common/crypto/src/lib.rs using hardcoded test vectors (public key + signature) produced by the ream Rust client signing an all-zeros message at epoch 5. A hex dev-dependency is also added to Cargo.toml to decode the vector constants.

Key changes:

• Two helper functions (ream_pubkey, ream_signature) decode the hex-encoded test vectors into ValidatorPublicKey / ValidatorSignature.
• test_cross_client_decode_ream_public_key / test_cross_client_decode_ream_signature verify serialisation roundtrips.
• test_cross_client_verify_ream_signature exercises the full verification path with the correct epoch and message.
• test_cross_client_ream_signature_rejects_wrong_message and test_cross_client_ream_signature_rejects_wrong_epoch confirm negative cases.
• hex is correctly placed under [dev-dependencies], so it does not affect production builds.

Two minor style issues were found: the wrong-epoch constant is a magic literal rather than REAM_EPOCH + 1, and the block comment misattributes the test vectors to leanSpec PR #433 rather than ream.

Confidence Score: 5/5

• This PR is safe to merge — all changes are test-only with no impact on production code paths.
• The only production-facing change is adding hex as a dev-dependency, which is correctly scoped and does not affect release builds. All five new tests are well-structured, cover both positive and negative verification paths, and align with the pattern established by the existing tests in the file.
• No files require special attention.

Important Files Changed

Filename	Overview
crates/common/crypto/src/lib.rs	Adds 5 cross-client XMSS compatibility tests (decode roundtrip for pubkey and signature, verify with correct params, reject wrong message, reject wrong epoch) plus two helper functions; only minor style issues (magic epoch number, source attribution comment).
crates/common/crypto/Cargo.toml	Correctly adds hex as a [dev-dependencies] entry (workspace-pinned), scoping it to test builds only.
Cargo.lock	Lock file updated to include hex in the ethlambda-crypto dependency set; no other changes.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A[Hex-encoded test vectors\nfrom ream client] -->|hex::decode| B[Raw byte slices]
    B -->|from_bytes| C[ValidatorPublicKey\nValidatorSignature]
    C -->|to_bytes| D{Roundtrip\nmatch?}
    D -->|yes| T1[test_decode_public_key PASS]
    D -->|yes| T2[test_decode_signature PASS]
    C --> E{is_valid\ncorrect epoch+msg}
    E -->|true| T3[test_verify PASS]
    C --> F{is_valid\nwrong message}
    F -->|false| T4[test_rejects_wrong_message PASS]
    C --> G{is_valid\nwrong epoch}
    G -->|false| T5[test_rejects_wrong_epoch PASS]

Loading

_{Last reviewed commit: bac7004}

[greptile-apps]

_{3 files reviewed, 2 comments}

_{Edit Code Review Agent Settings | Greptile}

[github-actions]

🤖 Claude Code Review

Here is the review:

---

PR Review: Add cross-client XMSS compatibility tests (ream)

Summary

Clean, focused PR. The test vectors have correct sizes (52-byte pubkey, 3112-byte signature matching Diff<U3600, U488>), hex is properly scoped to [dev-dependencies], and omitting #[ignore] is justified since these tests only exercise deserialization + LeanSignatureScheme::verify (no keygen, no aggregated-proof path, no ensure_verifier_ready() needed).

---

Observations

Test vector source attribution is ambiguous

crates/common/crypto/src/lib.rs:315–319

The comment says "Test vectors come from leanSpec PR #433" but the vectors are ream-produced; leanSpec PR #433 references them. A future maintainer can't easily audit these constants without knowing which ream commit or test file generated them. Consider adding a direct reference:

// Vectors generated by ream — see:
// https://github.com/ReamLabs/ream/.../<specific file or commit>
// Cross-verified by leanSpec PR #433.

No negative test for wrong public key

The suite covers wrong message (0xff…ff) and wrong epoch (6). A third negative case — verifying with a different (but structurally valid) public key — would complete the matrix. Without it, a broken is_valid that ignores the key entirely could pass all five tests.

Redundant hex decode in ream_pubkey() / ream_signature() helpers vs. roundtrip tests

The roundtrip tests decode inline (good — they're self-contained). The verification tests call the helpers, which parse the raw hex on every invocation. For five fast tests this is inconsequential, but if more tests are added later, consider std::sync::OnceLock to parse once. Not a blocker.

Minor: wrong_epoch named variable used once

lib.rs:378

let wrong_epoch = 6;
assert!(!sig.is_valid(&pk, wrong_epoch, &H256::ZERO), ...);

The local variable is a matter of taste; REAM_EPOCH + 1 inline would self-document the intent without a declaration. Either is fine.

---

Verdict

No correctness or security issues. The two substantive points are the source attribution (important for auditability of cross-client test vectors) and the missing wrong-key negative test (increases confidence in the is_valid implementation). Both are low-severity. Approvable with the attribution comment improved.

---

Automated review by Claude (Anthropic) · custom prompt