-
Notifications
You must be signed in to change notification settings - Fork 2k
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
Use bls from chia rs #16715
Merged
Merged
Use bls from chia rs #16715
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
arvidn
force-pushed
the
use-bls-from-chia_rs
branch
from
October 28, 2023 08:38
83c524d
to
b26b694
Compare
arvidn
added
the
Changed
Required label for PR that categorizes merge commit message as "Changed" for changelog
label
Oct 28, 2023
arvidn
force-pushed
the
use-bls-from-chia_rs
branch
2 times, most recently
from
October 30, 2023 20:04
f0dcd4e
to
0011ce6
Compare
github-actions
bot
added
the
merge_conflict
Branch has conflicts that prevent merge to main
label
Oct 30, 2023
This comment was marked as outdated.
This comment was marked as outdated.
arvidn
force-pushed
the
use-bls-from-chia_rs
branch
from
October 30, 2023 20:10
0011ce6
to
a30f3b0
Compare
github-actions
bot
removed
the
merge_conflict
Branch has conflicts that prevent merge to main
label
Oct 30, 2023
This comment was marked as outdated.
This comment was marked as outdated.
Pull Request Test Coverage Report for Build 6698187916
💛 - Coveralls |
github-actions
bot
added
the
merge_conflict
Branch has conflicts that prevent merge to main
label
Nov 1, 2023
This comment was marked as outdated.
This comment was marked as outdated.
arvidn
force-pushed
the
use-bls-from-chia_rs
branch
from
November 4, 2023 08:32
a30f3b0
to
2a2e35b
Compare
github-actions
bot
removed
the
merge_conflict
Branch has conflicts that prevent merge to main
label
Nov 4, 2023
This comment was marked as outdated.
This comment was marked as outdated.
|
emlowe
approved these changes
Nov 7, 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.
Looks good to me. Nice work
arvidn
added
ready_to_merge
Submitter and reviewers think this is ready
and removed
coverage-diff
labels
Nov 7, 2023
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Labels
Changed
Required label for PR that categorizes merge commit message as "Changed" for changelog
ready_to_merge
Submitter and reviewers think this is ready
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.
This PR is best reviewed one commit at a time. There are two commits, the first one is the main change. The second one removes a hack to add type-stubs to
blspy
, which is no longer needed.Purpose:
The purpose of moving the the BLS bindings from
blspy
(C++) tochia_rs
(rust) is to move towards being able to perform all block validation in rust. The main benefit of this is to be able to use threads instead of a process pool for parallel block validation.There's also a good chance the BLS cache could be a lot faster by being ported to rust.
Current Behavior:
We currently use BLST for BLS operations, wrapped with some additional features in C++ (
bls-signatures
) then given a python binding inblspy
. This makes it very hard (near impossible) to reach the underlying BLST primitives from rust. And similarly impossible to build rust-native types with BLS elements as members.New Behavior:
We still use BLST but the additional features have been ported to rust (in
chia_rs
) and given a python binding along with all other chia_rs functions and types, in thechia_rs
wheel. This allows rust-native types to have native BLS members, and still be compatible with the python bindings.For example, this enables signature validation to happen in rust (in threads rather than processes) and it allows the BLS cache to be implemented in rust.
One noteworthy discrepancy between
blspy
andchia_rs
is that thePrivateKey
data is not allocated in "secure memory" yet, inchia_rs
. i.e. mlocked and zeroed out.Testing Notes:
How do we know that the BLS types in
chia_rs
behave the same asblspy
?There's a fuzzer that calls into
blspy
to compare the results fromchia-bls
here:https://github.com/Chia-Network/chia_rs/blob/main/chia-bls/fuzz/fuzz_targets/blspy-fidelity.rs
There's a fidelity test exercising the python bindings of both
chia_rs
andblspy
here:https://github.com/Chia-Network/chia_rs/blob/main/tests/test_blspy_fidelity.py