Skip to content
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

[Crypto] use deterministic randomness in C backend #4253

Merged
merged 17 commits into from
May 3, 2023

Conversation

tarakby
Copy link
Contributor

@tarakby tarakby commented Apr 21, 2023

Relic uses the Kernel internal source of entropy to provide randomness tools that are used by the crypto package. The tools provide random elements in Fp, Fr, E1 and E2. Since Relic is being replaced by BLST, and BLST does not provide randomness tools, this PR removes the reliance on the Relic random tools.
In the crypto library, the random logic in the C layer is replaced by "mapping" functions where seed bytes are mapped to Fp, Fr, E1 and E2 (protocol and test functions). The crypto package C layer does not deal with implementing randomness (to avoid the complexity and possible issues). It relies on randomness created by the upper Go layer (either based on user input seeds or outputs of crypto/rand) depending on the use case:

  • update Fr_rand to use Fr_map instead based on an input seed.
  • Fr[X] polynomial generation is deterministic based on user input seed (used by all Feldman VSS dealers and threshold signature key gen)
  • refactor the polynomial generation above to share further code.
  • BLS batch verification random coefficient generation is deterministic based on randomness provided by the Go layer (using crypto/rand).
  • update logic of testing functions to generate random points in G2 and E2\G2. As a consequence, membership check in G2 is implemented and tested.
  • clean ups and functions renaming.

Not done yet: all random tools related to E1. Since G1 and E1 are still using Relic.

@codecov-commenter
Copy link

codecov-commenter commented Apr 21, 2023

Codecov Report

Merging #4253 (70c3c64) into feature/blst-based-crypto (633f337) will decrease coverage by 3.44%.
The diff coverage is 50.00%.

@@                      Coverage Diff                      @@
##           feature/blst-based-crypto    #4253      +/-   ##
=============================================================
- Coverage                      54.70%   51.27%   -3.44%     
=============================================================
  Files                            701      688      -13     
  Lines                          66415    61562    -4853     
=============================================================
- Hits                           36333    31564    -4769     
- Misses                         27150    27478     +328     
+ Partials                        2932     2520     -412     
Flag Coverage Δ
unittests 51.27% <50.00%> (-3.44%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
cmd/bootstrap/cmd/dkg.go 85.00% <50.00%> (ø)

... and 330 files with indirect coverage changes

@tarakby tarakby marked this pull request as ready for review April 21, 2023 05:12
@github-actions
Copy link
Contributor

github-actions bot commented May 1, 2023

FVM Benchstat comparison

This branch with compared with the base branch onflow:feature/blst-based-crypto commit 2070aa6

The command (for i in {1..7}; do go test ./fvm ./engine/execution/computation --bench . --tags relic -shuffle=on --benchmem --run ^$; done) was used.

Collapsed results for better readability

@tarakby tarakby requested a review from zhangchiqing as a code owner May 3, 2023 00:28
@tarakby tarakby removed the request for review from zhangchiqing May 3, 2023 00:31
@tarakby tarakby merged commit c54e2aa into feature/blst-based-crypto May 3, 2023
@tarakby tarakby deleted the tarak/replace-relic-rand branch May 3, 2023 19:22
This pull request was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants