Alter BEEFY primitives to prepare for potential BLS integration #10466
Conversation
tomusdrw
added
A0-please_review
primitives/beefy/src/commitment.rs
Outdated
| let mut signatures_compact: Vec<Signature> = vec![]; | ||
| let mut signatures_compact: Vec<&'a TSignature> = vec![]; | ||
|
|
||
| let mut bits: Vec<u8> = |
There was a problem hiding this comment.
can't we populate this and signatures_compact in one iteration?
There was a problem hiding this comment.
Yes, but actually I think I'd rather use iterators for both:
let mut bits = signatures.iter().map(|x| if x.is_some() { 1 } else { 0 }).collect();
let mut signatures_compact = signatures.iter().filter_map(|x| x).collect();wdyt?
…tytech#10466) * Generalize signature. * Fix tests. * Introduce VersionedFinalityProof. * cargo +nightly fmt --all * Rework packing a tad.
…tytech#10466) * Generalize signature. * Fix tests. * Introduce VersionedFinalityProof. * cargo +nightly fmt --all * Rework packing a tad.
|
@andresilva, @acatangiu: The current structure results in BEEFY messages of around 80K for 1K validators vs if we just keep one aggregated BLS signature it would 32K + 48Bytes + 1000/8 = 32172 that is about 2.5 time bulkier messages for no obvious advantage. I suggest to have a vector of ECDSA signatures and a single aggregated BLS signature field in the BEEFY message instead of having a vector of (ECDSA, single BLS) signatures. |
Agreed! I believe the ideal end result is to have single aggregated signature, resulting in slim messages. |
|
So aggregation before gossip unfortunately is not possible with unorganized gossip that we have: w3f/bls#51 |
…tytech#10466) * Generalize signature. * Fix tests. * Introduce VersionedFinalityProof. * cargo +nightly fmt --all * Rework packing a tad.
This PR makes
SignedCommitmentcontain a generic parameterSignature(instead of harcodedecsdasignature), so that in the future it can be used as:SignedCommitment<BlockNumber, ecsda::Signature>orSignedCommitment<BlockNumber, (ecdsa::Signature, (bls::Signature, ecsda::Signature))>).Also I've renamed
VersionedCommitmenttoVersionedFinalityProof. Currently BEEFY finality proof is simply theSignedCommitmentwith sufficient number of valid signatures, but in the future it will include not only theecdsaSignatures, but also an aggregated BLS signature.Related: paritytech/grandpa-bridge-gadget#2
CC @drskalman @andresilva