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

Update BLST And Herumi #7632

Merged
merged 33 commits into from
Oct 30, 2020
Merged

Update BLST And Herumi #7632

merged 33 commits into from
Oct 30, 2020

Conversation

nisdas
Copy link
Member

@nisdas nisdas commented Oct 24, 2020

What type of PR is this?

Bug Fix

What does this PR do? Why is it needed?

  • In Update Ethdo Keystore Deps #7537, our bls go wrapper was updated but neither of the source libraries were updated which caused a mismatch
    when compiling from source. This PR fixes that by updating all the relevant dependencies and cleaning up mcl builds.
  • Update Blst and Herumi deps
  • Clean Up BLS methods to allow for sharing common libraries.

Which issues(s) does this PR fix?

N.A

Other notes for review

@nisdas nisdas requested review from prestonvanloon and a team as code owners October 24, 2020 11:12
@nisdas nisdas added the Ready For Review A pull request ready for code review label Oct 24, 2020
@nisdas nisdas changed the title Fix Build From Source Update Herumi To Correct Tag Oct 26, 2020
@nisdas nisdas changed the title Update Herumi To Correct Tag Update Herumi To Correct Version Oct 26, 2020
Copy link
Contributor

@shayzluf shayzluf left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

need to update the comment as we don't use the random any more

// VerifyMultipleSignatures verifies a non-singular set of signatures and its respective pubkeys and messages.
// This method provides a safe way to verify multiple signatures at once. We pick a number randomly from 1 to max
// uint64 and then multiply the signature by it. We continue doing this for all signatures and its respective pubkeys.
// S* = S_1 * r_1 + S_2 * r_2 + ... + S_n * r_n
// P'{i,j} = P{i,j} * r_i
// e(S*, G) = \prod_{i=1}^n \prod_{j=1}^{m_i} e(P'{i,j}, M{i,j})
// Using this we can verify multiple signatures safely.

@nisdas nisdas marked this pull request as draft October 26, 2020 09:01
@nisdas nisdas changed the title Update Herumi To Correct Version Update BLST And Herumi Oct 28, 2020
@nisdas nisdas marked this pull request as ready for review October 28, 2020 16:13
@codecov
Copy link

codecov bot commented Oct 28, 2020

Codecov Report

Merging #7632 into master will not change coverage.
The diff coverage is n/a.

@@           Coverage Diff           @@
##           master    #7632   +/-   ##
=======================================
  Coverage   61.82%   61.82%           
=======================================
  Files         425      425           
  Lines       29903    29903           
=======================================
  Hits        18487    18487           
  Misses       8502     8502           
  Partials     2914     2914           

shared/bls/bls_test.go Outdated Show resolved Hide resolved
shared/bls/bls_test.go Outdated Show resolved Hide resolved
shared/bls/bls_test.go Outdated Show resolved Hide resolved
shared/bls/blst/secret_key.go Outdated Show resolved Hide resolved
shared/bls/blst/secret_key.go Outdated Show resolved Hide resolved
Comment on lines 54 to 58
msg := test.Input.Message
if msg == "" {
msg = test.Input.Messages
}
msgBytes, err := hex.DecodeString(msg[2:])
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please add a comment or TODO why we have to do this. It should be fixed soon with ethereum/consensus-spec-tests#22

shared/bls/spectest/sign_test.go Outdated Show resolved Hide resolved
shared/bls/spectest/verify_test.go Outdated Show resolved Hide resolved
third_party/herumi/mcl.BUILD Outdated Show resolved Hide resolved
@@ -7,6 +7,7 @@ type FastAggregateVerifyTest struct {
Input struct {
Pubkeys []string `json:"pubkeys"`
Message string `json:"message"`
Messages string `json:"messages"`
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same comment here about a TODO or comment. This should be removed after ethereum/consensus-spec-tests#22 is resolved.

shared/bls/herumi/public_key.go Outdated Show resolved Hide resolved
shared/bls/blst/signature_test.go Outdated Show resolved Hide resolved
shared/bls/blst/signature.go Outdated Show resolved Hide resolved
@prylabs-bulldozer prylabs-bulldozer bot merged commit 211d9bc into master Oct 30, 2020
@delete-merged-branch delete-merged-branch bot deleted the fixBuildFromSource branch October 30, 2020 19:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Ready For Review A pull request ready for code review
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants