Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion crates/common/consensus/lean/src/attestation.rs
Original file line number Diff line number Diff line change
Expand Up @@ -102,7 +102,7 @@ impl AggregatedAttestation {
}

/// Validator attestation bundled with its signature.
#[derive(Debug, PartialEq, Eq, Clone, Serialize, Deserialize, Encode, Decode, TreeHash)]
#[derive(Debug, PartialEq, Eq, Clone, Serialize, Deserialize, Encode, Decode)]
pub struct SignedAttestation {
pub validator_id: u64,
pub message: AttestationData,
Expand Down
25 changes: 18 additions & 7 deletions crates/common/fork_choice/lean/src/store.rs
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,8 @@ use ream_storage::{
},
};
use ream_sync::rwlock::{Reader, Writer};
#[cfg(feature = "devnet3")]
use ssz::{Decode, Encode};
use ssz_types::{BitList, VariableList, typenum::U4096};
use tokio::sync::Mutex;
use tree_hash::TreeHash;
Expand Down Expand Up @@ -1275,7 +1277,10 @@ impl Store {
proposer_attestation.validator_id,
&proposer_attestation.data,
),
signed_block_with_attestation.signature.proposer_signature,
signed_block_with_attestation
.signature
.proposer_signature
.clone(),
)?;

#[cfg(feature = "devnet3")]
Expand All @@ -1293,7 +1298,10 @@ impl Store {
proposer_attestation.validator_id,
&proposer_attestation.data,
),
signed_block_with_attestation.signature.proposer_signature,
signed_block_with_attestation
.signature
.proposer_signature
.clone(),
)?;
}
}
Expand All @@ -1304,7 +1312,10 @@ impl Store {
SignedAttestation {
validator_id: proposer_attestation.validator_id,
message: proposer_attestation.data.clone(),
signature: signed_block_with_attestation.signature.proposer_signature,
signature: signed_block_with_attestation
.signature
.proposer_signature
.clone(),
},
false,
)
Expand All @@ -1323,8 +1334,7 @@ impl Store {
let signature_bytes = signed_block_with_attestation
.signature
.proposer_signature
.inner
.to_vec();
.as_ssz_bytes();

let proof_data = signature_bytes.try_into().map_err(|err| {
anyhow!("Failed to convert proposer signature to VariableList {err:?}")
Expand Down Expand Up @@ -1505,7 +1515,8 @@ impl Store {
})
.collect::<anyhow::Result<Vec<_>>>()?;

let sig = Signature::from(proof.proof_data.as_ref());
let sig = Signature::from_ssz_bytes(proof.proof_data.as_ref())
.map_err(|err| anyhow!("Failed to decode signature: {err:?}"))?;

for pubkey in &public_keys {
let is_valid = sig.verify(pubkey, attestation_slot as u32, &data_root.0)?;
Expand Down Expand Up @@ -1624,7 +1635,7 @@ impl Store {
) -> anyhow::Result<()> {
let validator_id = signed_attestation.validator_id;
let attestation_data = &signed_attestation.message;
let signature = signed_attestation.signature;
let signature = signed_attestation.signature.clone();

self.validate_attestation(&signed_attestation).await?;

Expand Down
5 changes: 2 additions & 3 deletions crates/crypto/post_quantum/src/lean_multisig/aggregate.rs
Original file line number Diff line number Diff line change
Expand Up @@ -40,9 +40,8 @@ pub fn aggregate_signatures(
.map_err(|err| anyhow!("Failed to convert public keys: {err}"))?,
&signatures
.iter()
.map(|signature| signature.as_lean_sig())
.collect::<Result<Vec<_>, _>>()
.map_err(|err| anyhow!("Failed to convert signatures: {err}"))?,
.map(|signature| signature.as_lean_sig().clone())
.collect::<Vec<_>>(),
message,
epoch,
)
Expand Down
2 changes: 1 addition & 1 deletion crates/crypto/post_quantum/src/leansig/private_key.rs
Original file line number Diff line number Diff line change
Expand Up @@ -76,7 +76,7 @@ impl PrivateKey {
let signature = <LeanSigScheme as SignatureScheme>::sign(&self.inner, epoch, message)
.map_err(LeanSigError::SigningFailed)?;

Signature::from_lean_sig(signature)
Ok(Signature::from_lean_sig(signature))
}

pub fn from_bytes(bytes: &[u8]) -> Result<Self, LeanSigError> {
Expand Down
111 changes: 78 additions & 33 deletions crates/crypto/post_quantum/src/leansig/signature.rs
Original file line number Diff line number Diff line change
@@ -1,40 +1,59 @@
use alloy_primitives::FixedBytes;
use anyhow::anyhow;
use leansig::{MESSAGE_LENGTH, serialization::Serializable, signature::SignatureScheme};
use leansig::{signature::SignatureScheme, MESSAGE_LENGTH};
use serde::{Deserialize, Serialize};
use ssz::{Decode, Encode};
use ssz_derive::{Decode, Encode};
use tree_hash_derive::TreeHash;

use crate::leansig::{LeanSigScheme, errors::LeanSigError, public_key::PublicKey};
use crate::leansig::{public_key::PublicKey, LeanSigScheme};

const SIGNATURE_SIZE: usize = 3112;
/// The inner leansig signature type with built-in SSZ support.
pub type LeanSigSignature = <LeanSigScheme as SignatureScheme>::Signature;

type LeanSigSignature = <LeanSigScheme as SignatureScheme>::Signature;
const BLANK_SIGNATURE_SSZ_BYTES: [u8; 40] = [
36, 0, 0, 0, // offset_path = 36
0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, // rho (28 zeros)
0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, //
40, 0, 0, 0, // offset_hashes = 40
4, 0, 0, 0, // path: empty HashTreeOpening
];

/// Wrapper around a fixed-size serialized hash-based signature.
#[derive(Debug, PartialEq, Eq, Clone, Serialize, Deserialize, Encode, Decode, TreeHash, Copy)]
/// Wrapper around leansig's signature type.
/// Uses leansig's built-in SSZ encoding for interoperability with other clients.
#[derive(Clone, Serialize, Deserialize, Encode, Decode)]
#[ssz(struct_behaviour = "transparent")]
pub struct Signature {
pub inner: FixedBytes<SIGNATURE_SIZE>,
pub inner: LeanSigSignature,
Comment on lines -17 to +24
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not sure if this is the right way to do it, but since FixedBytes is how Ream stores it internally and needs conversion to LeanSigSignature to do any operation anyway, so I'm thinking we could just remove this interim FixedBytes and use LeanSigSignature directly

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Will LeanSigSignature encode to FixedBytes though is the question? I will read your code, but that would be the concern

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In case it's useful this is what I recall. I think FixedBytes was introduced to the spec in devnet0 just because leanSig wasn't stable yet so we just used FixedBytes<SIGNATURE_SIZE> as the mock Signature type in the specs.

And then in devnet1 we started with Bytes3116 in the specs even with the actual signatures.

But towards end of devnet1, the class Signature(Bytes3116) was removed from the specs by leanEthereum/leanSpec#210 and the canonical Signature type becomes this class Signature(Container).

So since then, leanSig and leanSpec uses the Signature container type that's variable length, and the fixed bytes is no longer used. And so that's why I'm thinking we could just use LeanSigSignature directly without conversion to fixed bytes. But I dunno maybe I'm missing something...

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah I did some regression testing and this looks fine 👍

}

impl From<&[u8]> for Signature {
fn from(value: &[u8]) -> Self {
Self {
inner: FixedBytes::from_slice(value),
}
impl std::fmt::Debug for Signature {
fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
f.debug_struct("Signature")
.field("inner", &"<LeanSigSignature>")
.finish()
}
}

impl Signature {
pub fn new(inner: FixedBytes<SIGNATURE_SIZE>) -> Self {
Self { inner }
impl PartialEq for Signature {
fn eq(&self, other: &Self) -> bool {
// Compare by SSZ encoding since LeanSigSignature doesn't implement PartialEq
self.inner.as_ssz_bytes() == other.inner.as_ssz_bytes()
}
}

impl Eq for Signature {}

impl Signature {
/// Create a blank/placeholder signature.
///
/// This decodes from minimal valid SSZ bytes, avoiding expensive key generation.
/// Only use in contexts where the signature won't be validated.
pub fn blank() -> Self {
Self::new(Default::default())
Self::from_ssz_bytes(&BLANK_SIGNATURE_SSZ_BYTES).expect("blank signature bytes are valid")
}

/// Create a mock signature for testing purposes.
///
/// Note: This generates a real signature which is expensive. Prefer `blank()` when
/// you just need a placeholder signature.
pub fn mock() -> Self {
use rand::rng;
Comment on lines 53 to 58
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can you just remove this function as it isn't used


Expand All @@ -48,15 +67,12 @@ impl Signature {
.expect("Mock signature generation failed")
}

pub fn from_lean_sig(signature: LeanSigSignature) -> Result<Self, LeanSigError> {
Ok(Self {
inner: FixedBytes::try_from(signature.to_bytes().as_slice())?,
})
pub fn from_lean_sig(signature: LeanSigSignature) -> Self {
Self { inner: signature }
}

pub fn as_lean_sig(&self) -> anyhow::Result<LeanSigSignature> {
LeanSigSignature::from_bytes(self.inner.as_slice())
.map_err(|err| anyhow!("Failed to decode LeanSigSignature from SSZ: {err:?}"))
pub fn as_lean_sig(&self) -> &LeanSigSignature {
&self.inner
}

pub fn verify(
Expand All @@ -69,17 +85,27 @@ impl Signature {
&public_key.as_lean_sig()?,
epoch,
message,
&self.as_lean_sig()?,
&self.inner,
))
}
}

#[cfg(test)]
mod tests {
use alloy_primitives::FixedBytes;
use leansig::serialization::Serializable;
use rand::rng;
use ssz::{Decode, Encode};

use crate::leansig::{private_key::PrivateKey, signature::Signature};

const LEGACY_SIGNATURE_SIZE: usize = 3112;

#[derive(ssz_derive::Encode)]
struct LegacySignature {
inner: FixedBytes<LEGACY_SIGNATURE_SIZE>,
}

#[test]
fn test_serialization_roundtrip() {
let mut rng = rng();
Expand All @@ -100,13 +126,32 @@ mod tests {
assert!(result.is_ok(), "Signing should succeed");
let signature = result.unwrap();

// convert to leansig signature
let hash_sig_signature = signature.as_lean_sig().unwrap();

// convert back to signature
let signature_returned = Signature::from_lean_sig(hash_sig_signature).unwrap();
// SSZ roundtrip test
let ssz_bytes = signature.as_ssz_bytes();
let signature_decoded = Signature::from_ssz_bytes(&ssz_bytes).unwrap();

// verify roundtrip
assert_eq!(signature, signature_returned);
assert_eq!(signature, signature_decoded);
}

#[test]
fn test_ssz_bytes_match_legacy_signature_wrapper() {
let mut rng = rng();
let activation_epoch = 0;
let num_active_epochs = 10;

let (_, private_key) =
PrivateKey::generate_key_pair(&mut rng, activation_epoch, num_active_epochs);

let epoch = 5;
let message = [0u8; 32];
let signature = private_key.sign(&message, epoch).unwrap();

let legacy_signature = LegacySignature {
inner: FixedBytes::try_from(signature.as_lean_sig().to_bytes().as_slice())
.expect("legacy signature bytes should match fixed size"),
};

assert_eq!(legacy_signature.as_ssz_bytes(), signature.as_ssz_bytes());
}
}
1 change: 1 addition & 0 deletions testing/lean-spec-tests/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ alloy-primitives.workspace = true
anyhow.workspace = true
serde.workspace = true
serde_json.workspace = true
ssz = { package = "ethereum_ssz", version = "0.10" }
ssz_types.workspace = true
tokio.workspace = true
tracing.workspace = true
Expand Down
1 change: 1 addition & 0 deletions testing/lean-spec-tests/src/lib.rs
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
pub mod fork_choice;
pub mod ssz_test;
pub mod state_transition;
pub mod types;
Loading
Loading