- 
                Notifications
    You must be signed in to change notification settings 
- Fork 2
Add F3 light client functionality #27
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from 26 commits
206e385
              1c58608
              91590ae
              6190ad1
              eb93f4a
              a6cf862
              ca8bbc8
              d0cb76b
              36a3327
              f305794
              3857f85
              cf00ac9
              57d5cd0
              6cec2b2
              7a0a39b
              7f9300c
              f2eb43c
              8b05095
              af28ab0
              a44a2f0
              95db86a
              e27943d
              8cffdc8
              70bf5a5
              42cef93
              6bccb28
              3608b91
              51bd363
              4f3b9e8
              b2f6171
              f905f26
              76d73d7
              0c68f0b
              File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | 
|---|---|---|
|  | @@ -33,3 +33,12 @@ calibnet | |
| Filecoin | ||
| merkle | ||
| Keccak256 | ||
| RPC | ||
| BDN | ||
| F3 | ||
| BLS12_381 | ||
| G1 | ||
| G2 | ||
| JSON | ||
| CBOR | ||
| TODO | ||
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| @@ -0,0 +1,75 @@ | ||||||||||||||||||||
| // Copyright 2019-2024 ChainSafe Systems | ||||||||||||||||||||
| // SPDX-License-Identifier: Apache-2.0, MIT | ||||||||||||||||||||
|  | ||||||||||||||||||||
| //! BDN (Boneh-Drijvers-Neven) signature aggregation scheme, for preventing rogue public-key attacks. | ||||||||||||||||||||
| //! | ||||||||||||||||||||
| //! NOTE: currently uses standard BLS aggregation without coefficient weighting, hence returns incorrect values compared to go-f3. | ||||||||||||||||||||
| //! | ||||||||||||||||||||
| 
      Comment on lines
    
      +4
     to 
      +7
    
   There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Critical: Incomplete BDN implementation - returns incorrect results The module acknowledges that it's using standard BLS aggregation instead of proper BDN coefficient weighting. This means the signature verification will fail when compared against go-f3's implementation. The PR objectives note this as a known issue with expected signature verification errors being temporarily silenced. Would you like me to help implement the proper BDN coefficient weighting to match the go-f3 reference implementation? | ||||||||||||||||||||
| use crate::verifier::BLSError; | ||||||||||||||||||||
| use bls_signatures::{PublicKey, Serialize, Signature}; | ||||||||||||||||||||
| use bls12_381::{G1Projective, G2Projective}; | ||||||||||||||||||||
| use filecoin_f3_gpbft::PubKey; | ||||||||||||||||||||
|  | ||||||||||||||||||||
| There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🛠️ Refactor suggestion Prep imports for BDN coefficients Add Scalar and hashing for coefficient derivation.  use crate::verifier::BLSError;
 use bls_signatures::{PublicKey, Serialize, Signature};
-use bls12_381::{G1Projective, G2Projective};
+use bls12_381::{G1Projective, G2Projective, Scalar};
+use sha2::{Digest, Sha256};📝 Committable suggestion
 
        Suggested change
       
 🤖 Prompt for AI Agents | ||||||||||||||||||||
| /// BDN aggregation context for managing signature and public key aggregation | ||||||||||||||||||||
| pub struct BDNAggregation { | ||||||||||||||||||||
| pub_keys: Vec<PubKey>, | ||||||||||||||||||||
| } | ||||||||||||||||||||
|  | ||||||||||||||||||||
| impl BDNAggregation { | ||||||||||||||||||||
| pub fn new(pub_keys: &[PubKey]) -> Result<Self, BLSError> { | ||||||||||||||||||||
| if pub_keys.is_empty() { | ||||||||||||||||||||
| return Err(BLSError::EmptyPublicKeys); | ||||||||||||||||||||
| } | ||||||||||||||||||||
|  | ||||||||||||||||||||
| Ok(Self { | ||||||||||||||||||||
| pub_keys: pub_keys.to_vec(), | ||||||||||||||||||||
| }) | ||||||||||||||||||||
| } | ||||||||||||||||||||
|         
                  moshababo marked this conversation as resolved.
              Outdated
          
            Show resolved
            Hide resolved | ||||||||||||||||||||
|  | ||||||||||||||||||||
| /// Aggregates signatures using standard BLS aggregation | ||||||||||||||||||||
| /// TODO: Implement BDN aggregation scheme: https://github.com/ChainSafe/rust-f3/issues/29 | ||||||||||||||||||||
| pub fn aggregate_sigs(&self, sigs: &[Vec<u8>]) -> Result<Vec<u8>, BLSError> { | ||||||||||||||||||||
|         
                  moshababo marked this conversation as resolved.
              Outdated
          
            Show resolved
            Hide resolved | ||||||||||||||||||||
| if sigs.len() != self.pub_keys.len() { | ||||||||||||||||||||
| return Err(BLSError::LengthMismatch { | ||||||||||||||||||||
| pub_keys: self.pub_keys.len(), | ||||||||||||||||||||
| sigs: sigs.len(), | ||||||||||||||||||||
| }); | ||||||||||||||||||||
| } | ||||||||||||||||||||
|  | ||||||||||||||||||||
| let mut aggregated_point = G2Projective::identity(); | ||||||||||||||||||||
|  | ||||||||||||||||||||
| for sig_bytes in sigs.iter() { | ||||||||||||||||||||
| // Deserialize signature to G2 point | ||||||||||||||||||||
| let signature = | ||||||||||||||||||||
| Signature::from_bytes(sig_bytes).map_err(|_| BLSError::SignatureDeserialization)?; | ||||||||||||||||||||
|         
                  moshababo marked this conversation as resolved.
              Outdated
          
            Show resolved
            Hide resolved | ||||||||||||||||||||
| let sig_point: G2Projective = signature.into(); | ||||||||||||||||||||
|  | ||||||||||||||||||||
| // Standard BLS aggregation | ||||||||||||||||||||
| aggregated_point += sig_point; | ||||||||||||||||||||
| } | ||||||||||||||||||||
|  | ||||||||||||||||||||
| // Convert back to signature | ||||||||||||||||||||
| let aggregated_sig: Signature = aggregated_point.into(); | ||||||||||||||||||||
| Ok(aggregated_sig.as_bytes().to_vec()) | ||||||||||||||||||||
| } | ||||||||||||||||||||
|  | ||||||||||||||||||||
| /// Aggregates public keys using standard BLS aggregation | ||||||||||||||||||||
| /// TODO: Implement BDN aggregation scheme: https://github.com/ChainSafe/rust-f3/issues/29 | ||||||||||||||||||||
| pub fn aggregate_pub_keys(&self) -> Result<PublicKey, BLSError> { | ||||||||||||||||||||
| let mut aggregated_point = G1Projective::identity(); | ||||||||||||||||||||
|  | ||||||||||||||||||||
| for pub_key_bytes in &self.pub_keys { | ||||||||||||||||||||
| let public_key = PublicKey::from_bytes(&pub_key_bytes.0) | ||||||||||||||||||||
|         
                  moshababo marked this conversation as resolved.
              Outdated
          
            Show resolved
            Hide resolved | ||||||||||||||||||||
| .map_err(|_| BLSError::PublicKeyDeserialization)?; | ||||||||||||||||||||
|  | ||||||||||||||||||||
| // Convert public key to G1Projective for curve operations | ||||||||||||||||||||
| let pub_key_point: G1Projective = public_key.into(); | ||||||||||||||||||||
|  | ||||||||||||||||||||
| // Standard BLS aggregation | ||||||||||||||||||||
| aggregated_point += pub_key_point; | ||||||||||||||||||||
| } | ||||||||||||||||||||
|  | ||||||||||||||||||||
| // Convert back to PublicKey | ||||||||||||||||||||
| Ok(aggregated_point.into()) | ||||||||||||||||||||
| } | ||||||||||||||||||||
| } | ||||||||||||||||||||
| There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 💡 Verification agent 🧩 Analysis chainAdd BDN coefficient derivation helper (deterministic, order-agnostic set digest) 
 // Place near the top of this module
const BDN_COEFF_DST: &[u8] = b"F3-BDN-agg-v1";
// Canonical set digest: length-prefix + lexicographic sort to make it order-agnostic.
fn canonical_set_digest(pub_keys: &[PubKey]) -> [u8; 32] {
    let mut keys: Vec<&[u8]> = pub_keys.iter().map(|k| k.0.as_slice()).collect();
    keys.sort_unstable();
    let mut h = Sha256::new();
    h.update(BDN_COEFF_DST);
    h.update((keys.len() as u64).to_be_bytes());
    for k in keys {
        h.update((k.len() as u64).to_be_bytes());
        h.update(k);
    }
    let out = h.finalize();
    let mut digest = [0u8; 32];
    digest.copy_from_slice(&out);
    digest
}
// Derive per-key coefficients as field elements using a wide reduction.
fn compute_bdn_coefficients(pub_keys: &[PubKey]) -> Result<Vec<Scalar>, BLSError> {
    if pub_keys.is_empty() {
        return Err(BLSError::EmptyPublicKeys);
    }
    let set_digest = canonical_set_digest(pub_keys);
    let mut coeffs = Vec::with_capacity(pub_keys.len());
    for pk in pub_keys {
        // Two domain-separated hashes -> 64-byte wide input
        let mut h1 = Sha256::new();
        h1.update(BDN_COEFF_DST);
        h1.update(set_digest);
        h1.update((pk.0.len() as u64).to_be_bytes());
        h1.update(&pk.0);
        h1.update([0u8]);
        let r1 = h1.finalize();
        let mut h2 = Sha256::new();
        h2.update(BDN_COEFF_DST);
        h2.update(set_digest);
        h2.update((pk.0.len() as u64).to_be_bytes());
        h2.update(&pk.0);
        h2.update([1u8]);
        let r2 = h2.finalize();
        let mut wide = [0u8; 64];
        wide[..32].copy_from_slice(&r1);
        wide[32..].copy_from_slice(&r2);
        let c = Scalar::from_bytes_wide(&wide);
        coeffs.push(c);
    }
    Ok(coeffs)
}If go-f3 uses a different DST/derivation, please paste it and I’ll align the helper for byte-for-byte compatibility. 🏁 Script executed: #!/bin/bash
# Verify if `sha2` crate is declared for SHA-256 functionality
grep -R "sha2" -n Cargo.toml || echo "sha2 NOT found in Cargo.toml"
# Verify bls12_381 dependency (for Scalar) is present
grep -R "bls12_381" -n Cargo.toml || echo "bls12_381 NOT found in Cargo.toml"
# Search for existing Scalar usage in Rust sources
rg -n "Scalar" -g "*.rs" || echo "No Scalar references in code"Length of output: 242 🌐 Web query: 💡 Result: I can't find that constant in the repo with my current search — do you want me to search the go-f3 repository code (and specific files under blssig) now and return the exact DST string? Add Sha256 and Scalar dependencies & imports 
 🤖 Prompt for AI Agents | ||||||||||||||||||||
| Original file line number | Diff line number | Diff line change | 
|---|---|---|
| @@ -1,17 +1,14 @@ | ||
| // Copyright 2019-2024 ChainSafe Systems | ||
| // SPDX-License-Identifier: Apache-2.0, MIT | ||
|  | ||
| pub fn add(left: usize, right: usize) -> usize { | ||
| left + right | ||
| } | ||
| //! BLS signature implementation using BDN aggregation scheme. | ||
| //! | ||
| //! This module implements the BLS signature scheme used in the Filecoin F3 protocol. | ||
| //! It uses the BLS12_381 curve with G1 for public keys and G2 for signatures. | ||
| //! The BDN (Boneh-Drijvers-Neven) scheme is used for signature and public key aggregation | ||
| //! to prevent rogue public-key attacks. | ||
|  | ||
| #[cfg(test)] | ||
| mod tests { | ||
| use super::*; | ||
| mod bdn; | ||
| mod verifier; | ||
|  | ||
| #[test] | ||
| fn it_works() { | ||
| let result = add(2, 2); | ||
| assert_eq!(result, 4); | ||
| } | ||
| } | ||
| pub use verifier::{BLSError, BLSVerifier}; | 
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| @@ -0,0 +1,169 @@ | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| // Copyright 2019-2024 ChainSafe Systems | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| // SPDX-License-Identifier: Apache-2.0, MIT | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|  | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| use bls_signatures::{PublicKey, Serialize, Signature, verify_messages}; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| use filecoin_f3_gpbft::PubKey; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| use filecoin_f3_gpbft::api::Verifier; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| use hashlink::LruCache; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| use parking_lot::RwLock; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| use thiserror::Error; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|  | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| use crate::bdn::BDNAggregation; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|  | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| #[cfg(test)] | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| mod tests; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|  | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| #[derive(Error, Debug, PartialEq)] | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|         
                  moshababo marked this conversation as resolved.
              Outdated
          
            Show resolved
            Hide resolved | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| pub enum BLSError { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| #[error("empty public keys provided")] | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| EmptyPublicKeys, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| #[error("invalid public key length: expected {BLS_PUBLIC_KEY_LENGTH} bytes, got {0}")] | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| InvalidPublicKeyLength(usize), | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| #[error("failed to deserialize public key")] | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| PublicKeyDeserialization, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| #[error("invalid signature length: expected {BLS_SIGNATURE_LENGTH} bytes, got {0}")] | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| InvalidSignatureLength(usize), | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| #[error("failed to deserialize signature")] | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| SignatureDeserialization, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|         
                  moshababo marked this conversation as resolved.
              Outdated
          
            Show resolved
            Hide resolved | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| #[error("BLS signature verification failed")] | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| SignatureVerificationFailed, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| #[error("mismatched number of public keys and signatures: {pub_keys} != {sigs}")] | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| LengthMismatch { pub_keys: usize, sigs: usize }, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|  | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| /// BLS signature verifier using BDN aggregation scheme | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| /// | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| /// This verifier implements the same scheme used by `go-f3/blssig`, with: | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| /// - BLS12_381 curve | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| /// - G1 for public keys, G2 for signatures | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| /// - BDN aggregation for rogue-key attack prevention | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| /// - Point cache for performance optimization | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| pub struct BLSVerifier { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| /// Cache for deserialized public key points to avoid expensive repeated operations | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| point_cache: RwLock<LruCache<Vec<u8>, PublicKey>>, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'd change the signatures of the  There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I followed the existing  However, I agree that the  I'm not convinced though that  | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|  | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| impl Default for BLSVerifier { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| fn default() -> Self { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| Self::new() | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|  | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| /// BLS12-381 public key length in bytes | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| const BLS_PUBLIC_KEY_LENGTH: usize = 48; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|  | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| /// BLS12-381 signature length in bytes | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| const BLS_SIGNATURE_LENGTH: usize = 96; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|  | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| /// Maximum number of cached public key points to prevent excessive memory usage | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| const MAX_POINT_CACHE_SIZE: usize = 10_000; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|  | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| impl BLSVerifier { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| pub fn new() -> Self { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| Self { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| point_cache: RwLock::new(LruCache::new(MAX_POINT_CACHE_SIZE)), | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|         
                  moshababo marked this conversation as resolved.
              Show resolved
            Hide resolved | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|  | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| /// Verifies a single BLS signature | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| fn verify_single(&self, pub_key: &PubKey, msg: &[u8], sig: &[u8]) -> Result<(), BLSError> { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| // Validate input lengths | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| if pub_key.0.len() != BLS_PUBLIC_KEY_LENGTH { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| return Err(BLSError::InvalidPublicKeyLength(pub_key.0.len())); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| if sig.len() != BLS_SIGNATURE_LENGTH { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| return Err(BLSError::InvalidSignatureLength(sig.len())); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|  | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| // Get cached public key | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| let pub_key = self.get_or_cache_public_key(&pub_key.0)?; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|  | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| // Deserialize signature | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| let signature = | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| Signature::from_bytes(sig).map_err(|_| BLSError::SignatureDeserialization)?; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|         
                  moshababo marked this conversation as resolved.
              Outdated
          
            Show resolved
            Hide resolved | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|  | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| // Verify using bls-signatures | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| let msgs = [msg]; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| let pub_keys = [pub_key]; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| match verify_messages(&signature, &msgs, &pub_keys) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| true => Ok(()), | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| false => Err(BLSError::SignatureVerificationFailed), | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|  | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| /// Gets a cached public key or deserialize and caches it | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| fn get_or_cache_public_key(&self, pub_key_bytes: &[u8]) -> Result<PublicKey, BLSError> { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| // Check cache first | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| let mut cache = self.point_cache.write(); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| if let Some(cached_key) = cache.get(pub_key_bytes) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| return Ok(*cached_key); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|         
                  moshababo marked this conversation as resolved.
              Show resolved
            Hide resolved | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| drop(cache); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|         
                  moshababo marked this conversation as resolved.
              Outdated
          
            Show resolved
            Hide resolved | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|  | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| // Deserialize the public key | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| let pub_key = | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| PublicKey::from_bytes(pub_key_bytes).map_err(|_| BLSError::PublicKeyDeserialization)?; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|  | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| // Cache it | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| let mut cache = self.point_cache.write(); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| cache.insert(pub_key_bytes.to_vec(), pub_key); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| drop(cache); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|         
                  moshababo marked this conversation as resolved.
              Outdated
          
            Show resolved
            Hide resolved | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|  | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| Ok(pub_key) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🛠️ Refactor suggestion Fix Copy/move bugs in cache path (PublicKey is not guaranteed Copy). Returning  Apply this diff: -    fn get_or_cache_public_key(&self, pub_key_bytes: &[u8]) -> Result<PublicKey, BLSError> {
-        // Check cache first
-        let mut cache = self.point_cache.write();
-        if let Some(cached_key) = cache.get(pub_key_bytes) {
-            return Ok(*cached_key);
-        }
-        drop(cache);
-
-        // Deserialize the public key
-        let pub_key =
-            PublicKey::from_bytes(pub_key_bytes).map_err(|_| BLSError::PublicKeyDeserialization)?;
-
-        // Cache it
-        let mut cache = self.point_cache.write();
-        cache.insert(pub_key_bytes.to_vec(), pub_key);
-        drop(cache);
-
-        Ok(pub_key)
-    }
+    fn get_or_cache_public_key(&self, pub_key_bytes: &[u8]) -> Result<PublicKey, BLSError> {
+        // Fast path: refresh LRU on hit and clone for the caller
+        if let Some(pk) = self.point_cache.write().get(pub_key_bytes).cloned() {
+            return Ok(pk);
+        }
+        // Miss: deserialize outside of locks
+        let pk =
+            PublicKey::from_bytes(pub_key_bytes).map_err(|_| BLSError::PublicKeyDeserialization)?;
+        // Insert and return a clone
+        self.point_cache.write().insert(pub_key_bytes.to_vec(), pk.clone());
+        Ok(pk)
+    }📝 Committable suggestion
 
        Suggested change
       
 🤖 Prompt for AI Agents | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|  | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| impl Verifier for BLSVerifier { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| type Error = BLSError; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|  | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| fn verify(&self, pub_key: &PubKey, msg: &[u8], sig: &[u8]) -> Result<(), Self::Error> { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| self.verify_single(pub_key, msg, sig) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|  | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| fn aggregate(&self, pub_keys: &[PubKey], sigs: &[Vec<u8>]) -> Result<Vec<u8>, Self::Error> { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| if pub_keys.is_empty() || sigs.is_empty() { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|         
                  moshababo marked this conversation as resolved.
              Outdated
          
            Show resolved
            Hide resolved | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| return Err(BLSError::EmptyPublicKeys); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| if pub_keys.len() != sigs.len() { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| return Err(BLSError::LengthMismatch { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| pub_keys: pub_keys.len(), | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| sigs: sigs.len(), | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| }); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|  | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| // Validate all input lengths | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| for (i, pub_key) in pub_keys.iter().enumerate() { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| if pub_key.0.len() != BLS_PUBLIC_KEY_LENGTH { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| return Err(BLSError::InvalidPublicKeyLength(pub_key.0.len())); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| if sigs[i].len() != BLS_SIGNATURE_LENGTH { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| return Err(BLSError::InvalidSignatureLength(sigs[i].len())); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|  | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| let bdn = BDNAggregation::new(pub_keys)?; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| bdn.aggregate_sigs(sigs) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|  | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| fn verify_aggregate( | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| &self, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| payload: &[u8], | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| agg_sig: &[u8], | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| signers: &[PubKey], | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| ) -> Result<(), Self::Error> { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| if signers.is_empty() { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| return Err(BLSError::EmptyPublicKeys); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|  | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| for pub_key in signers { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| if pub_key.0.len() != BLS_PUBLIC_KEY_LENGTH { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| return Err(BLSError::InvalidPublicKeyLength(pub_key.0.len())); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|  | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| let bdn = BDNAggregation::new(signers)?; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| let agg_pub_key = bdn.aggregate_pub_keys()?; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| let agg_pub_key_bytes = PubKey(agg_pub_key.as_bytes().to_vec()); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| self.verify_single(&agg_pub_key_bytes, payload, agg_sig) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
💡 Verification agent
🧩 Analysis chain
Workspace members added: ensure per-crate inheritance of workspace package fields
Nice addition of
lightclient,merkle, andrpcto the workspace. Please verify each new crate’s Cargo.toml inherits the workspace’s package metadata (edition, rust-version, license, repository) to keep CI/tooling consistent. If not already present, add:Apply the same in
merkle/Cargo.tomlandrpc/Cargo.toml. This avoids accidental drift (e.g., building with a different edition/MSRV locally vs CI).Run this to confirm inheritance is set across all new crates:
🏁 Script executed:
Length of output: 1089
Add missing
rust-version.workspaceinheritance in new cratesThe verification script shows that while
lightclient/Cargo.toml,merkle/Cargo.toml, andrpc/Cargo.tomleach include:they are missing the
rust-version.workspace = trueline. Please add this under the[package]section for each crate to ensure the workspace MSRV is consistently applied.Affected files:
Example diff for lightclient/Cargo.toml:
[package] name = "lightclient" version = "0.1.0" repository.workspace = true license.workspace = true edition.workspace = true +rust-version.workspace = trueRepeat the same insertion in
merkle/Cargo.tomlandrpc/Cargo.toml.🤖 Prompt for AI Agents