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

from crypto types.rs

Author: mskrzypkows

Cargo.lock

@@ -16,6 +16,7 @@ dyn-eq.workspace = true
 hex.workspace = true
 libp2p.workspace = true
 vise.workspace = true
+pluto-crypto.workspace = true
 pluto-eth2api.workspace = true
 prost.workspace = true
 prost-types.workspace = true

crates/core/Cargo.toml

@@ -7,6 +7,8 @@
 
 use std::any::Any;
 
+use base64::Engine as _;
+
 use crate::{
     signeddata::{
         Attestation, BeaconCommitteeSelection, SignedAggregateAndProof, SignedRandao,
@@ -70,6 +72,25 @@ pub enum ParSigExCodecError {
     InvalidSignature(String),
 }
 
+fn serialize_signature(sig: &Signature) -> Result<Vec<u8>, ParSigExCodecError> {
+    let encoded = base64::engine::general_purpose::STANDARD.encode(sig);
+    Ok(serde_json::to_vec(&encoded)?)
+}
+
+fn deserialize_signature(bytes: &[u8]) -> Result<Box<dyn SignedData>, ParSigExCodecError> {
+    let encoded: String = serde_json::from_slice(bytes)?;
+    let raw = base64::engine::general_purpose::STANDARD
+        .decode(encoded)
+        .map_err(|e| ParSigExCodecError::SignedData(format!("invalid base64: {e}")))?;
+    let sig: Signature = raw.try_into().map_err(|v: Vec<u8>| {
+        ParSigExCodecError::SignedData(format!(
+            "invalid signature length: got {}, want 96",
+            v.len()
+        ))
+    })?;
+    Ok(Box::new(sig))
+}
+
 pub(crate) fn serialize_signed_data(data: &dyn SignedData) -> Result<Vec<u8>, ParSigExCodecError> {
     let any = data as &dyn Any;
 
@@ -131,7 +152,9 @@ pub(crate) fn serialize_signed_data(data: &dyn SignedData) -> Result<Vec<u8>, Pa
     serialize_json!(VersionedSignedValidatorRegistration);
     serialize_json!(SignedVoluntaryExit);
     serialize_json!(SignedRandao);
-    serialize_json!(Signature);
+    if let Some(value) = any.downcast_ref::<Signature>() {
+        return serialize_signature(value);
+    }
     serialize_json!(BeaconCommitteeSelection);
     serialize_json!(SyncCommitteeSelection);
 
@@ -206,7 +229,7 @@ pub(crate) fn deserialize_signed_data(
         DutyType::Randao => deserialize_json!(SignedRandao),
 
         // -- Signature: JSON-only --
-        DutyType::Signature => deserialize_json!(Signature),
+        DutyType::Signature => deserialize_signature(bytes),
 
         // -- PrepareAggregator: JSON-only --
         DutyType::PrepareAggregator => deserialize_json!(BeaconCommitteeSelection),
@@ -455,4 +478,20 @@ mod tests {
             downcast(deserialize_signed_data(&DutyType::Aggregator, &json_bytes).unwrap());
         assert_eq!(sap, decoded);
     }
+
+    #[test]
+    fn marshal_unmarshal_signature() {
+        let sig: Signature = [0xab; 96];
+        let bytes = serialize_signed_data(&sig).unwrap();
+
+        // Snapshot: Signature serializes as a base64-encoded JSON string.
+        // Changing this breaks wire compatibility with Charon.
+        const EXPECTED: &str = "\"q6urq6urq6urq6urq6urq6urq6urq6urq6urq6urq6urq6urq6urq6urq6urq6ur\
+             q6urq6urq6urq6urq6urq6urq6urq6urq6urq6urq6urq6urq6urq6urq6urq6ur\"";
+        assert_eq!(bytes, EXPECTED.as_bytes());
+
+        let decoded: Signature =
+            downcast(deserialize_signed_data(&DutyType::Signature, &bytes).unwrap());
+        assert_eq!(sig, decoded);
+    }
 }

crates/core/src/parsigex_codec.rs

@@ -3,7 +3,6 @@
 use serde::{Deserialize, Deserializer, Serialize, Serializer};
 use tree_hash::TreeHash;
 
-use base64::Engine as _;
 use pluto_eth2api::{
     spec::{
         altair, bellatrix, capella, deneb, electra, phase0, serde_legacy_builder_version,
@@ -94,57 +93,16 @@ struct VersionedRawAggregateAndProofJson<T> {
 
 /// Converts an ETH2 signature to a core signature.
 pub fn sig_from_eth2(sig: phase0::BLSSignature) -> Signature {
-    Signature::new(sig)
+    sig
 }
 
 fn sig_to_eth2(sig: &Signature) -> phase0::BLSSignature {
-    *sig.as_ref()
-}
-
-impl serde::Serialize for Signature {
-    fn serialize<S>(&self, serializer: S) -> Result<S::Ok, S::Error>
-    where
-        S: serde::Serializer,
-    {
-        let encoded = base64::engine::general_purpose::STANDARD.encode(self.as_ref());
-        serializer.serialize_str(&encoded)
-    }
-}
-
-impl<'de> serde::Deserialize<'de> for Signature {
-    fn deserialize<D>(deserializer: D) -> Result<Self, D::Error>
-    where
-        D: serde::Deserializer<'de>,
-    {
-        let encoded = String::deserialize(deserializer)?;
-        let bytes = base64::engine::general_purpose::STANDARD
-            .decode(encoded)
-            .map_err(|err| serde::de::Error::custom(format!("invalid base64 signature: {err}")))?;
-        let sig: [u8; 96] = bytes.try_into().map_err(|bytes: Vec<u8>| {
-            serde::de::Error::custom(format!(
-                "invalid signature length: got {}, want 96",
-                bytes.len()
-            ))
-        })?;
-        Ok(Signature::new(sig))
-    }
-}
-
-impl Signature {
-    /// Converts the signature to an ETH2 signature.
-    pub fn to_eth2(&self) -> phase0::BLSSignature {
-        sig_to_eth2(self)
-    }
-
-    /// Creates a partially signed signature wrapper.
-    pub fn new_partial(sig: Self, share_idx: u64) -> ParSignedData {
-        ParSignedData::new(sig, share_idx)
-    }
+    *sig
 }
 
 impl SignedData for Signature {
     fn signature(&self) -> Result<Signature, SignedDataError> {
-        Ok(self.clone())
+        Ok(*self)
     }
 
     fn set_signature(&self, signature: Signature) -> Result<Self, SignedDataError> {
@@ -1361,7 +1319,7 @@ mod tests {
     }
 
     fn sample_signature(byte: u8) -> Signature {
-        Signature::new([byte; 96])
+        [byte; 96]
     }
 
     fn sample_root(byte: u8) -> phase0::Root {
@@ -2563,37 +2521,17 @@ mod tests {
     #[test]
     fn signature() {
         let sig1 = sample_signature(0x22);
-        let sig2 = sig1.clone();
+        let sig2 = sig1;
 
         assert!(matches!(
             sig1.message_root(),
             Err(SignedDataError::UnsupportedSignatureMessageRoot)
         ));
         assert_eq!(sig1, sig1.signature().unwrap());
-        assert_eq!(sig1.to_eth2(), sig2.signature().unwrap().to_eth2());
+        assert_eq!(sig1, sig2.signature().unwrap());
 
         let ss = sig1.set_signature(sig2.signature().unwrap()).unwrap();
         assert_eq!(sig2, ss);
-
-        let js = serde_json::to_vec(&sig1).unwrap();
-        let sig3: Signature = serde_json::from_slice(&js).unwrap();
-        assert_eq!(sig1, sig3);
-    }
-
-    #[test]
-    fn signature_json_errors() {
-        let invalid_base64 = serde_json::from_slice::<Signature>(br#""%%%""#);
-        assert!(matches!(
-            invalid_base64,
-            Err(err) if matches!(err.classify(), serde_json::error::Category::Data)
-        ));
-
-        let short = base64::engine::general_purpose::STANDARD.encode([0x11_u8; 95]);
-        let wrong_len = serde_json::from_slice::<Signature>(format!("\"{short}\"").as_bytes());
-        assert!(matches!(
-            wrong_len,
-            Err(err) if matches!(err.classify(), serde_json::error::Category::Data)
-        ));
     }
 
     #[test_case(false ; "unblinded")]
@@ -2751,7 +2689,7 @@ mod tests {
         assert_ne!(msg_root, [0_u8; 32]);
 
         let signature = sample_signature(0x99);
-        let updated = wrapped.set_signature(signature.clone()).unwrap();
+        let updated = wrapped.set_signature(signature).unwrap();
         assert_eq!(signature, updated.signature().unwrap());
 
         let js = serde_json::to_vec(&wrapped).unwrap();

crates/core/src/signeddata.rs

@@ -300,7 +300,8 @@ pub enum ProposalType {
 // the pub key as [u8; 48] instead of string.
 // [original implementation](https://github.com/ObolNetwork/charon/blob/b3008103c5429b031b63518195f4c49db4e9a68d/core/types.go#L264)
 const PK_LEN: usize = 48;
-const SIG_LEN: usize = 96;
+
+pub use pluto_crypto::types::Signature;
 
 /// Public key struct
 #[derive(Debug, Clone, Copy, PartialEq, Eq, Hash)]
@@ -524,24 +525,6 @@ where
     }
 }
 
-// todo: add proper signature type
-/// Signature type
-#[derive(Debug, Clone, PartialEq, Eq)]
-pub struct Signature(pub(crate) [u8; SIG_LEN]);
-
-impl Signature {
-    /// Create a new signature.
-    pub fn new(signature: [u8; SIG_LEN]) -> Self {
-        Signature(signature)
-    }
-}
-
-impl AsRef<[u8; SIG_LEN]> for Signature {
-    fn as_ref(&self) -> &[u8; SIG_LEN] {
-        &self.0
-    }
-}
-
 /// Signed data type
 pub trait SignedData: Any + DynClone + DynEq + StdDebug + Send + Sync {
     /// signature returns the signed duty data's signature.
@@ -1027,7 +1010,7 @@ mod tests {
 
     impl SignedData for MockSignedData {
         fn signature(&self) -> Result<Signature, SignedDataError> {
-            Ok(Signature::new([42u8; SIG_LEN]))
+            Ok([42u8; 96])
         }
 
         fn set_signature(&self, _signature: Signature) -> Result<Self, SignedDataError> {
@@ -1048,10 +1031,7 @@ mod tests {
         assert!(retrieved.is_some());
         let retrieved = retrieved.unwrap();
         assert_eq!(retrieved.share_idx, 0);
-        assert_eq!(
-            retrieved.signed_data.signature().unwrap(),
-            Signature::new([42u8; SIG_LEN])
-        );
+        assert_eq!(retrieved.signed_data.signature().unwrap(), [42u8; 96]);
     }
 
     #[test]

crates/core/src/types.rs

@@ -246,7 +246,7 @@ pub fn agg_validator_registrations(
             .verify(&share.pub_key, &sig_root, &agg_sig)
             .map_err(AggregateError::InvalidValidatorRegistrationAggregatedSignature)?;
 
-        res.push(msg.set_signature(pluto_core::types::Signature::new(agg_sig))?);
+        res.push(msg.set_signature(agg_sig)?);
     }
 
     Ok(res)
@@ -355,7 +355,7 @@ mod tests {
     }
 
     fn partial_signature(sig: Signature, share_idx: u64) -> ParSignedData {
-        ParSignedData::new(pluto_core::types::Signature::new(sig), share_idx)
+        ParSignedData::new(sig, share_idx)
     }
 
     #[test]

crates/dkg/src/aggregate.rs

@@ -332,7 +332,7 @@ mod tests {
     use anyhow::Context as _;
     use futures::StreamExt as _;
     use libp2p::{Multiaddr, swarm::SwarmEvent};
-    use pluto_core::types::{DutyType, ParSignedData, ParSignedDataSet, PubKey, Signature};
+    use pluto_core::types::{DutyType, ParSignedData, ParSignedDataSet, PubKey};
     use pluto_p2p::{
         config::P2PConfig,
         p2p::{Node, NodeType},
@@ -380,7 +380,7 @@ mod tests {
         let mut set = ParSignedDataSet::new();
         set.insert(
             PubKey::from(pk_bytes),
-            Signature::new_partial(Signature::new(sig_bytes), share_idx),
+            ParSignedData::new(sig_bytes, share_idx),
         );
         set
     }
@@ -493,7 +493,7 @@ mod tests {
                 .map(|j| {
                     let mut bytes = [0u8; 96];
                     rand::thread_rng().fill(&mut bytes[..]);
-                    Signature::new_partial(Signature::new(bytes), (j + 1) as u64)
+                    ParSignedData::new(bytes, (j + 1) as u64)
                 })
                 .collect();
             expected_data.insert(*pk, psigs);

crates/dkg/src/exchanger.rs

@@ -114,10 +114,7 @@ pub fn sign_lock_hash(share_idx: u64, shares: &[Share], hash: &[u8]) -> Result<P
         let pub_key = share_pubkey(share, "signing lock hash")?;
         let sig = BlstImpl.sign(&share.secret_share, hash)?;
 
-        set.insert(
-            pub_key,
-            ParSignedData::new(pluto_core::types::Signature::new(sig), share_idx),
-        );
+        set.insert(pub_key, ParSignedData::new(sig, share_idx));
     }
 
     Ok(set)
@@ -149,10 +146,7 @@ pub fn sign_deposit_msgs(
         let sig_root = deposit::get_message_signing_root(&msg, network_name)?;
         let sig = BlstImpl.sign(&share.secret_share, &sig_root)?;
 
-        set.insert(
-            pub_key,
-            ParSignedData::new(pluto_core::types::Signature::new(sig), share_idx),
-        );
+        set.insert(pub_key, ParSignedData::new(sig, share_idx));
         msgs.insert(pub_key, msg);
     }
 
@@ -205,10 +199,7 @@ pub fn sign_validator_registrations(
             },
         )?;
 
-        set.insert(
-            pub_key,
-            ParSignedData::new(pluto_core::types::Signature::new(sig), share_idx),
-        );
+        set.insert(pub_key, ParSignedData::new(sig, share_idx));
         msgs.insert(pub_key, signed_reg);
     }
 
@@ -486,7 +477,7 @@ mod tests {
                 .signature()
                 .expect("signature should exist");
             BlstImpl
-                .verify(&share.public_shares[&2], &hash, sig.as_ref())
+                .verify(&share.public_shares[&2], &hash, &sig)
                 .expect("partial signature should verify against share public key");
         }
     }

crates/dkg/src/signing.rs

@@ -214,9 +214,7 @@ mod tests {
     fn set_registration_signature_updates_v1_signature() {
         let reg =
             make_core_registration([0x11; 48], [0x22; 20], 30_000_000, 1_746_843_400, [0; 96]);
-        let updated =
-            set_registration_signature(&reg, pluto_core::types::Signature::new([0x44; 96]))
-                .expect("should work");
+        let updated = set_registration_signature(&reg, [0x44u8; 96]).expect("should work");
 
         let builder_registration =
             builder_registration_from_eth2(&updated).expect("conversion should succeed");