From 8c5001f13f7df3cf69685188016b5b42f85f1293 Mon Sep 17 00:00:00 2001 From: Maciej Skrzypkowski Date: Fri, 22 May 2026 19:28:16 +0200 Subject: [PATCH 1/3] Removed Signature from core types.rs, replaced all usages with Signature from crypto types.rs --- Cargo.lock | 1 + crates/core/Cargo.toml | 1 + crates/core/src/parsigex_codec.rs | 43 ++++++++++++++++- crates/core/src/signeddata.rs | 76 +++---------------------------- crates/core/src/types.rs | 28 ++---------- crates/dkg/src/aggregate.rs | 4 +- crates/dkg/src/exchanger.rs | 6 +-- crates/dkg/src/signing.rs | 17 ++----- crates/dkg/src/validators.rs | 4 +- 9 files changed, 64 insertions(+), 116 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 5b6a5a2a..0357b94b 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -5604,6 +5604,7 @@ dependencies = [ "libp2p", "pluto-build-proto", "pluto-cluster", + "pluto-crypto", "pluto-eth2api", "pluto-eth2util", "pluto-p2p", diff --git a/crates/core/Cargo.toml b/crates/core/Cargo.toml index 594b2d18..f19067f5 100644 --- a/crates/core/Cargo.toml +++ b/crates/core/Cargo.toml @@ -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 diff --git a/crates/core/src/parsigex_codec.rs b/crates/core/src/parsigex_codec.rs index 02cd56c1..acf56b88 100644 --- a/crates/core/src/parsigex_codec.rs +++ b/crates/core/src/parsigex_codec.rs @@ -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, ParSigExCodecError> { + let encoded = base64::engine::general_purpose::STANDARD.encode(sig); + Ok(serde_json::to_vec(&encoded)?) +} + +fn deserialize_signature(bytes: &[u8]) -> Result, 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| { + 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, ParSigExCodecError> { let any = data as &dyn Any; @@ -131,7 +152,9 @@ pub(crate) fn serialize_signed_data(data: &dyn SignedData) -> Result, Pa serialize_json!(VersionedSignedValidatorRegistration); serialize_json!(SignedVoluntaryExit); serialize_json!(SignedRandao); - serialize_json!(Signature); + if let Some(value) = any.downcast_ref::() { + 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); + } } diff --git a/crates/core/src/signeddata.rs b/crates/core/src/signeddata.rs index f08b2145..894d6da2 100644 --- a/crates/core/src/signeddata.rs +++ b/crates/core/src/signeddata.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 { /// 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(&self, serializer: S) -> Result - 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(deserializer: D) -> Result - 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| { - 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 { - Ok(self.clone()) + Ok(*self) } fn set_signature(&self, signature: Signature) -> Result { @@ -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::(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::(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(); diff --git a/crates/core/src/types.rs b/crates/core/src/types.rs index 0ecfd477..e7739b53 100644 --- a/crates/core/src/types.rs +++ b/crates/core/src/types.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 { - Ok(Signature::new([42u8; SIG_LEN])) + Ok([42u8; 96]) } fn set_signature(&self, _signature: Signature) -> Result { @@ -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] diff --git a/crates/dkg/src/aggregate.rs b/crates/dkg/src/aggregate.rs index 92823f44..2e6e8401 100644 --- a/crates/dkg/src/aggregate.rs +++ b/crates/dkg/src/aggregate.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] diff --git a/crates/dkg/src/exchanger.rs b/crates/dkg/src/exchanger.rs index 26a33beb..a6d335e2 100644 --- a/crates/dkg/src/exchanger.rs +++ b/crates/dkg/src/exchanger.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); diff --git a/crates/dkg/src/signing.rs b/crates/dkg/src/signing.rs index 2fa49472..070a845d 100644 --- a/crates/dkg/src/signing.rs +++ b/crates/dkg/src/signing.rs @@ -114,10 +114,7 @@ pub fn sign_lock_hash(share_idx: u64, shares: &[Share], hash: &[u8]) -> Result

Date: Fri, 22 May 2026 19:40:04 +0200 Subject: [PATCH 2/3] SIGNATURE_LENGTH used instead of 96 --- crates/core/src/parsigex_codec.rs | 6 +++--- crates/core/src/signeddata.rs | 2 +- crates/core/src/types.rs | 9 ++++++--- 3 files changed, 10 insertions(+), 7 deletions(-) diff --git a/crates/core/src/parsigex_codec.rs b/crates/core/src/parsigex_codec.rs index acf56b88..dc2186f7 100644 --- a/crates/core/src/parsigex_codec.rs +++ b/crates/core/src/parsigex_codec.rs @@ -17,7 +17,7 @@ use crate::{ VersionedSignedProposal, VersionedSignedValidatorRegistration, }, ssz_codec, - types::{DutyType, Signature, SignedData}, + types::{DutyType, SIGNATURE_LENGTH, Signature, SignedData}, }; /// Error type for partial signature exchange codec operations. @@ -84,7 +84,7 @@ fn deserialize_signature(bytes: &[u8]) -> Result, ParSigExCo .map_err(|e| ParSigExCodecError::SignedData(format!("invalid base64: {e}")))?; let sig: Signature = raw.try_into().map_err(|v: Vec| { ParSigExCodecError::SignedData(format!( - "invalid signature length: got {}, want 96", + "invalid signature length: got {}, want {SIGNATURE_LENGTH}", v.len() )) })?; @@ -481,7 +481,7 @@ mod tests { #[test] fn marshal_unmarshal_signature() { - let sig: Signature = [0xab; 96]; + let sig: Signature = [0xab; SIGNATURE_LENGTH]; let bytes = serialize_signed_data(&sig).unwrap(); // Snapshot: Signature serializes as a base64-encoded JSON string. diff --git a/crates/core/src/signeddata.rs b/crates/core/src/signeddata.rs index 894d6da2..590fe8e7 100644 --- a/crates/core/src/signeddata.rs +++ b/crates/core/src/signeddata.rs @@ -1319,7 +1319,7 @@ mod tests { } fn sample_signature(byte: u8) -> Signature { - [byte; 96] + [byte; crate::types::SIGNATURE_LENGTH] } fn sample_root(byte: u8) -> phase0::Root { diff --git a/crates/core/src/types.rs b/crates/core/src/types.rs index e7739b53..1c0b9f0b 100644 --- a/crates/core/src/types.rs +++ b/crates/core/src/types.rs @@ -301,7 +301,7 @@ pub enum ProposalType { // [original implementation](https://github.com/ObolNetwork/charon/blob/b3008103c5429b031b63518195f4c49db4e9a68d/core/types.go#L264) const PK_LEN: usize = 48; -pub use pluto_crypto::types::Signature; +pub use pluto_crypto::types::{SIGNATURE_LENGTH, Signature}; /// Public key struct #[derive(Debug, Clone, Copy, PartialEq, Eq, Hash)] @@ -1010,7 +1010,7 @@ mod tests { impl SignedData for MockSignedData { fn signature(&self) -> Result { - Ok([42u8; 96]) + Ok([42u8; SIGNATURE_LENGTH]) } fn set_signature(&self, _signature: Signature) -> Result { @@ -1031,7 +1031,10 @@ mod tests { assert!(retrieved.is_some()); let retrieved = retrieved.unwrap(); assert_eq!(retrieved.share_idx, 0); - assert_eq!(retrieved.signed_data.signature().unwrap(), [42u8; 96]); + assert_eq!( + retrieved.signed_data.signature().unwrap(), + [42u8; SIGNATURE_LENGTH] + ); } #[test] From d13d1a0c8ef43e4028a67bfe42bcffb8052569a3 Mon Sep 17 00:00:00 2001 From: Maciej Skrzypkowski Date: Fri, 22 May 2026 19:43:07 +0200 Subject: [PATCH 3/3] missing tests --- crates/core/src/parsigex_codec.rs | 21 +++++++++++++++++++++ 1 file changed, 21 insertions(+) diff --git a/crates/core/src/parsigex_codec.rs b/crates/core/src/parsigex_codec.rs index dc2186f7..fc65ab9c 100644 --- a/crates/core/src/parsigex_codec.rs +++ b/crates/core/src/parsigex_codec.rs @@ -494,4 +494,25 @@ mod tests { downcast(deserialize_signed_data(&DutyType::Signature, &bytes).unwrap()); assert_eq!(sig, decoded); } + + #[test] + fn deserialize_signature_invalid_base64() { + let err = deserialize_signed_data(&DutyType::Signature, br#""%%%""#).unwrap_err(); + assert!( + matches!(err, ParSigExCodecError::SignedData(_)), + "expected SignedData error, got {err:?}" + ); + } + + #[test] + fn deserialize_signature_wrong_length() { + let short = + base64::engine::general_purpose::STANDARD.encode([0x11_u8; SIGNATURE_LENGTH - 1]); + let input = format!("\"{short}\""); + let err = deserialize_signed_data(&DutyType::Signature, input.as_bytes()).unwrap_err(); + assert!( + matches!(err, ParSigExCodecError::SignedData(_)), + "expected SignedData error, got {err:?}" + ); + } }