-
Notifications
You must be signed in to change notification settings - Fork 761
Simplex QuorumCertificate and BLS aggregator #4004
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
base: master
Are you sure you want to change the base?
Conversation
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.
Pull Request Overview
Implements BLS-based quorum certificates and aggregation for the Simplex consensus engine, along with test utilities and configuration.
- Adds
testValidatorInfo
andnewEngineConfig
insimplex/test_util.go
to bootstrap BLS-based engine tests. - Introduces
QC
,QCDeserializer
, andSignatureAggregator
insimplex/quorum.go
for creating, serializing, verifying, and aggregating quorum certificates. - Implements BLS signing/verifying in
simplex/bls.go
and corresponding unit tests insimplex/bls_test.go
.
Reviewed Changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated 2 comments.
Show a summary per file
File | Description |
---|---|
simplex/test_util.go | Mock ValidatorInfo and newEngineConfig helper for tests |
simplex/quorum.go | QC implementation, bytes/deserialize logic, and signature aggregator |
simplex/config.go | Defines Config and SimplexChainContext |
simplex/bls.go | BLSSigner /BLSVerifier and message encoding for BLS auth |
simplex/bls_test.go | Unit tests for signing, verification, and QC aggregation |
go.mod | Adds github.com/ava-labs/simplex dependency |
Comments suppressed due to low confidence (4)
simplex/bls_test.go:97
- This test is empty; implement the scenario for when a QC has a signer not in the membership set or remove the placeholder.
func TestQCSignerNotInMembershipSet(t *testing.T) {
simplex/bls_test.go:101
- This test is empty; implement the scenario for when deserialization or verification is attempted on a certificate outside the membership set or remove the placeholder.
func TestQCNotInMembershipSet(t *testing.T) {
simplex/bls.go:75
- The comment refers to
networkID
and a length prefix that don't match the ASN.1 encoding ofencodedSimplexSignedPayload
. Update the docstring to reflect the actual fields and format produced byasn1.Marshal
.
// encodesMessageToSign returns a byte slice [simplexLabel][chainID][networkID][message length][message].
simplex/quorum.go:90
- Iterating with
for range quorumSize
over an integer is invalid. Use a standard index loop:for i := 0; i < int(quorumSize); i++ {
.
for range quorumSize {
Signed-off-by: Sam Liokumovich <[email protected]>
Signed-off-by: Sam Liokumovich <[email protected]>
Signed-off-by: Sam Liokumovich <[email protected]>
@@ -0,0 +1,92 @@ | |||
// Copyright (C) 2019-2024, Ava Labs, Inc. All rights reserved. |
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.
can we try renaming via git mv
?
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.
generally LGTM, but can we use git mv
?
Signed-off-by: Sam Liokumovich <[email protected]>
Why this should be merged
Implements the simplex
QuorumCertificate
,QCDeserializer
andSignatureAggregator
interfaces. This allows simplex to parse, aggregate and verify quorum certificates(ex. finalizations and notarizations) during execution.How this works
asn1
How this was tested
Added unit tests to
bls_test.go
.Need to be documented in RELEASES.md?
no