-
Couldn't load subscription status.
- 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 all 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,62 @@ | ||
| // 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, Signature}; | ||
| use bls12_381::{G1Projective, G2Affine, G2Projective}; | ||
|
|
||
|
Comment on lines
+8
to
+11
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 ❓ Verification inconclusiveVerify cross-crate conversions (bls_signatures <-> bls12_381) You’re converting bls_signatures::{Signature,PublicKey} into bls12_381::{G2Affine,G1Projective}. These From/Into impls may not exist; mixing curve types across crates often fails. Run to confirm conversions or existing adapters: 🏁 Script executed: #!/bin/bash
# Look for any local adapters/impls bridging bls_signatures <-> bls12_381
rg -nP -C2 'impl\s+(From|Into)<\s*bls_signatures::(Signature|PublicKey)\s*>\s+for\s+bls12_381::(G[12](Affine|Projective))'
rg -nP -C2 'impl\s+(From|Into)<\s*bls12_381::(G[12](Affine|Projective))\s*>\s+for\s+bls_signatures::(Signature|PublicKey)'
# Scan for helper conversion fns
rg -nP -C2 '(to|into|as)_(g1|g2)_(proj|affine)|from_(g1|g2)'Length of output: 0 Add missing conversions between |
||
| /// BDN aggregation context for managing signature and public key aggregation | ||
| pub struct BDNAggregation { | ||
| pub_keys: Vec<PublicKey>, | ||
| } | ||
|
|
||
| impl BDNAggregation { | ||
| pub fn new(pub_keys: Vec<PublicKey>) -> Result<Self, BLSError> { | ||
| if pub_keys.is_empty() { | ||
| return Err(BLSError::EmptyPublicKeys); | ||
| } | ||
|
|
||
| Ok(Self { pub_keys }) | ||
| } | ||
|
|
||
| /// 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<Signature>) -> Result<Signature, BLSError> { | ||
| if sigs.len() != self.pub_keys.len() { | ||
| return Err(BLSError::LengthMismatch { | ||
| pub_keys: self.pub_keys.len(), | ||
| sigs: sigs.len(), | ||
| }); | ||
| } | ||
|
|
||
| // Standard BLS aggregation | ||
| let mut agg_point = G2Projective::identity(); | ||
| for sig in sigs { | ||
| let sig: G2Affine = sig.into(); | ||
| agg_point += sig; | ||
| } | ||
|
|
||
| // Convert back to Signature | ||
| let agg_sig: Signature = agg_point.into(); | ||
| Ok(agg_sig) | ||
| } | ||
|
|
||
| /// 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> { | ||
| // Standard BLS aggregation | ||
| let mut agg_point = G1Projective::identity(); | ||
| for pub_key in &self.pub_keys { | ||
| let pub_key_point: G1Projective = (*pub_key).into(); | ||
| agg_point += pub_key_point; | ||
| } | ||
|
|
||
| // Convert back to PublicKey | ||
| let agg_pub_key: PublicKey = agg_point.into(); | ||
| Ok(agg_pub_key) | ||
| } | ||
| } | ||
| 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,185 @@ | ||
| // 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)] | ||
| pub enum BLSError { | ||
| #[error("empty public keys provided")] | ||
| EmptyPublicKeys, | ||
| #[error("empty signatures provided")] | ||
| EmptySignatures, | ||
| #[error("invalid public key length: expected {BLS_PUBLIC_KEY_LENGTH} bytes, got {0}")] | ||
| InvalidPublicKeyLength(usize), | ||
| #[error("failed to deserialize public key: {0}")] | ||
| PublicKeyDeserialization(bls_signatures::Error), | ||
| #[error("invalid signature length: expected {BLS_SIGNATURE_LENGTH} bytes, got {0}")] | ||
| InvalidSignatureLength(usize), | ||
| #[error("failed to deserialize signature: {0}")] | ||
| SignatureDeserialization(bls_signatures::Error), | ||
| #[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 | ||
| 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 { | ||
| // key size: 48, value size: 196, total estimated: 1.83 MiB | ||
| 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 = self.deserialize_signature(sig)?; | ||
|
|
||
| // 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: &[u8]) -> Result<PublicKey, BLSError> { | ||
| // Check cache first | ||
| if let Some(cached) = self.point_cache.write().get(pub_key) { | ||
| return Ok(*cached); | ||
| } | ||
moshababo marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
|
||
| // Deserialize and cache | ||
| let typed_pub_key = self.deserialize_public_key(pub_key)?; | ||
| self.point_cache | ||
| .write() | ||
| .insert(pub_key.to_vec(), typed_pub_key); | ||
| Ok(typed_pub_key) | ||
| } | ||
|
|
||
| fn deserialize_public_key(&self, pub_key: &[u8]) -> Result<PublicKey, BLSError> { | ||
| PublicKey::from_bytes(pub_key).map_err(BLSError::PublicKeyDeserialization) | ||
| } | ||
|
|
||
| fn deserialize_signature(&self, sig: &[u8]) -> Result<Signature, BLSError> { | ||
| Signature::from_bytes(sig).map_err(BLSError::SignatureDeserialization) | ||
| } | ||
| } | ||
|
|
||
| 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() { | ||
| return Err(BLSError::EmptyPublicKeys); | ||
| } | ||
| if sigs.is_empty() { | ||
| return Err(BLSError::EmptySignatures); | ||
| } | ||
|
|
||
| if pub_keys.len() != sigs.len() { | ||
| return Err(BLSError::LengthMismatch { | ||
| pub_keys: pub_keys.len(), | ||
| sigs: sigs.len(), | ||
| }); | ||
| } | ||
|
|
||
| // Validate all input lengths | ||
| let mut typed_pub_keys = vec![]; | ||
| let mut typed_sigs = vec![]; | ||
| 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())); | ||
| } | ||
|
|
||
| typed_pub_keys.push(self.get_or_cache_public_key(&pub_key.0)?); | ||
| typed_sigs.push(self.deserialize_signature(&sigs[i])?); | ||
| } | ||
|
|
||
| let bdn = BDNAggregation::new(typed_pub_keys)?; | ||
| let agg_sig = bdn.aggregate_sigs(typed_sigs)?; | ||
| Ok(agg_sig.as_bytes()) | ||
| } | ||
|
|
||
| fn verify_aggregate( | ||
| &self, | ||
| payload: &[u8], | ||
| agg_sig: &[u8], | ||
| signers: &[PubKey], | ||
| ) -> Result<(), Self::Error> { | ||
| if signers.is_empty() { | ||
| return Err(BLSError::EmptyPublicKeys); | ||
| } | ||
|
|
||
| let mut typed_pub_keys = vec![]; | ||
| for pub_key in signers { | ||
| if pub_key.0.len() != BLS_PUBLIC_KEY_LENGTH { | ||
| return Err(BLSError::InvalidPublicKeyLength(pub_key.0.len())); | ||
| } | ||
|
|
||
| typed_pub_keys.push(self.get_or_cache_public_key(&pub_key.0)?); | ||
| } | ||
|
|
||
| let bdn = BDNAggregation::new(typed_pub_keys)?; | ||
| 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) | ||
| } | ||
|
Comment on lines
+180
to
+184
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 chainBDN weighting likely incomplete — verify_aggregate may mismatch go-f3 If Run to check for TODOs in BDN: 🏁 Script executed: #!/bin/bash
rg -n -C2 -i 'BDN|coefficient|weight' blssig/src/bdn
rg -n -C2 -i 'TODO|FIXME' blssig/src/bdnLength of output: 2677 Gate aggregate verification until BDN weighting is implemented 🤖 Prompt for AI Agents |
||
| } | ||
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