chore: Signature type unification

[mskrzypkows]

Removed Signature from core types.rs, replaced all usages with Signature from crypto types.rs

[mskrzypkows]

@claude review

[claude]

Claude finished @mskrzypkows's task in 3m 37s —— View job

---

Claude PR Review — Signature type unification

• Read changed files in the PR
• Analyze the Signature type unification
• Check for functional correctness
• Verify Go/Charon parity where applicable
• Post review feedback

---

Summary

Nice consolidation — collapsing pluto_core::types::Signature (a [u8; 96] newtype) into the pluto_crypto::types::Signature alias removes a real source of duplication and the Signature::new / .to_eth2() ceremony at every call site. No circular deps introduced (pluto-crypto does not depend on pluto-core), all removed APIs are no longer referenced, and the wire format is preserved.

What I liked

• Wire-compat snapshot test in crates/core/src/parsigex_codec.rs:482-496 pins the JSON-encoded form to the exact bytes Charon produces (base64 of []byte). The comment "Changing this breaks wire compatibility with Charon" makes the intent unmistakable — this is exactly the kind of guard that should accompany a serialization change.
• Charon's Signature is a []byte whose MarshalJSON does json.Marshal([]byte(s)), which in Go produces a base64 string. The new serialize_signature (crates/core/src/parsigex_codec.rs:75-78) does the same. Round-trip parity looks correct.
• Removing the .clone() calls and the Signature::new(...) wrapper at DKG call sites (aggregate.rs, signing.rs, exchanger.rs, validators.rs) is correct since [u8; 96] is Copy.

Issues / suggestions

1. Lost test coverage for malformed-input error paths  [crates/core/src/signeddata.rs]

The old signature_json_errors test (now removed) verified two failure modes for Signature:

• invalid base64 ("%%%")
• wrong length (95 bytes)

After this PR those paths still exist — but they live in deserialize_signature in parsigex_codec.rs:80-92 and aren't exercised by any test. The snapshot test only covers the happy path. Worth adding two tests at the codec level (calling deserialize_signed_data(&DutyType::Signature, ...) with bad input) so the regression net survives the move. Fix this →

2. sig_from_eth2 / sig_to_eth2 in signeddata.rs are now identity no-ops  [crates/core/src/signeddata.rs:94-101]

With Signature = [u8; 96] = phase0::BLSSignature, both helpers compile to a copy. They are still called from ~20 sites in signeddata.rs. pluto_crypto::tblsconv also has a sig_to_eth2 that is identical (also a no-op). Two reasonable cleanups (not strictly required for this PR, but worth flagging as follow-up): inline the helpers at call sites, or delete the local pair and route through pluto_crypto::tblsconv consistently.

3. impl SignedData for Signature now implements it for any [u8; 96]

Because Signature is a type alias, impl SignedData for Signature (signeddata.rs:103) actually impls SignedData for the primitive [u8; 96]. Any unrelated 96-byte array — a hash, a random buffer, a public-key concatenation — now silently satisfies Box<dyn SignedData>. This is a step away from Charon's behavior (Go uses type Signature []byte — a defined type, not an alias, so type identity is preserved). It's not a bug today, but it weakens type safety and makes accidental misuse harder to catch. Consider whether Signature should remain a newtype in pluto_crypto::types rather than an alias. That's a larger discussion than this PR — flagging for awareness.

4. Error-variant choice in deserialize_signature  [crates/core/src/parsigex_codec.rs:84,86]

The new base64 / length failures are mapped to ParSigExCodecError::SignedData(format!(...)). There is a dedicated InvalidSignature(String) variant defined a few lines up (parsigex_codec.rs:71-72, doc'd as "Failed to extract the signature from signed data") that arguably reads better here. Minor — pick whichever you prefer, but SignedData is currently a generic catch-all and using it for what is clearly a signature-shape error is a small loss of signal.

5. Snapshot string is technically correct but visually misleading  [crates/core/src/parsigex_codec.rs:489-490]

const EXPECTED: &str = "\"q6ur...q6ur\
     q6ur...q6ur\"";

Rust's \<newline> string-continuation escape strips the leading whitespace on the next line, so this evaluates to a 128-char base64 string (correct). A reader scanning the test could mistake the indentation for spaces inside the string. A one-line comment ("Rust \<newline> strips the indentation; the literal is one 128-char base64 string") would save future readers a trip to the Rust reference.

Non-issues I checked

• signature.as_ref().to_vec() in crates/core/src/types.rs:608 still compiles — [u8; 96]: AsRef<[u8]> via blanket impls.
• Cargo.lock change is mechanical (adds pluto-crypto to pluto-core's dep list).
• No call sites of Signature::new / Signature::new_partial / .to_eth2() remain in the workspace.

Verdict

Looks good overall. The wire-compat snapshot test is the most important guard and it is in place. The points above are mostly polish — the only one I'd ask be addressed before merge is (1): the malformed-input tests that disappeared with the old serde impl. The rest can be follow-ups.
• unified_Signature