-
Notifications
You must be signed in to change notification settings - Fork 179
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
BLST-based crypto package #4358
Merged
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
[crypto] add blst source code
…version and multiple cleanups
This reverts commit 90c412f.
[Crypto] new BLST-based scalar and field element type
Merge `master` into BLST feature branch
[WIP] code review changes
durkmurder
approved these changes
Nov 29, 2023
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.
Reviewed .*c
files. Great accomplishment! ⭐
gomisha
approved these changes
Dec 5, 2023
github-merge-queue
bot
removed this pull request from the merge queue due to no response for status checks
Dec 5, 2023
github-merge-queue
bot
removed this pull request from the merge queue due to failed status checks
Dec 6, 2023
github-merge-queue
bot
removed this pull request from the merge queue due to failed status checks
Dec 6, 2023
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Overview
This PR is a major refactor of the Flow crypto library, in particular the the BLS 12-381 curve implementation. It updates the curve implementation from relying on Relic external library to relying on BLST external library.
This has two advantages:
Changes
Flow crypto library implementation provides an implementation of the BLS signature scheme, BLS-based threshold signature, DKG for BLS-based threshold signature, and BLS-based SPoCK. All of these schemes are set up using the BLS 12-381 curve.
The implementation of all these schemes has a common structure into 3 layers:
Layer 3 used to use Relic tools before this PR, and this PR is updating it to use BLST tools. As a consequence, Layer 2 underwent a major refactor to adapt to new BLST tools. Layer 1 also underwent a minor refactor.
The PR also integrates the new Flow crypto library into
onflow/flow-go
. In particular it updates all the test and build tooling (workflows, tests, makefiles)This PR is made of the smaller PRs below, where more details can be found:
flow-go
#4380master
into BLST feature branch #4863Testing
In addition to CI tests on this repo, the new module has been tested on transient Flow networks with up to 150 nodes (using the Benchnet2 framework). Results can be found here.
Benchmark
(on an Intel(R) Core(TM) i5-8259U CPU @ 2.30GHz)
For a quick read, you can focus only on the
BLS_Sign
(3.9x improvement) andBLS_Verify
(3.6x improvement).Risks
Review assignment:
Note: Please do not review files under
onflow/flow-go/crypto/internal/
andonflow/flow-go/crypto/blst_src/
(about 135 files) because these are files copied from the BLST repo without changes. The reasoning behind the copy is explained in this README file.Review is open to everyone interested. In particular, a review from the following members is appreciated:
onflow/flow-go/crypto/
-.c
and.h
files: @durkmurderonflow/flow-go/crypto/
-go
files (tests included): @jordanschalmMakefile
and.yml
andDockerfile
files: @gomisha / @peterargueREADME
files : any member from above is welcome to reviewonflow/flow-go/
-go
files outside/crypto
: @peterargue