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

[Test] Complete test for Aggregated Shamir's Secret Sharing Scheme and DKG protocol. #8

Merged
merged 4 commits into from
Jul 22, 2021

Conversation

furszy
Copy link

@furszy furszy commented Jul 18, 2021

Going one step further over the Threshold Signatures tests, validating that the correlation and aggregation BLS properties are working properly on top of the Shamir's Secret Sharing Scheme, verifying the DKG protocol and each participant share/sig validation step described in DIP6 BLS M-of-N Threshold Scheme and Distributed Key Generation

Plus corrected a duplicated HexStr function and few invalid function's args names.

Copy link

@random-zebra random-zebra left a comment

Choose a reason for hiding this comment

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

Really nice test coverage. ACK 9b5a07f

There's few minor nits fixed in random-zebra@0fa37a4. Can be picked here, or addressed in another pull request.

src/test.cpp Outdated Show resolved Hide resolved
src/test.cpp Outdated Show resolved Hide resolved
src/test.cpp Outdated Show resolved Hide resolved
src/test.cpp Outdated Show resolved Hide resolved
src/test.cpp Outdated Show resolved Hide resolved
src/test.cpp Show resolved Hide resolved
@furszy
Copy link
Author

furszy commented Jul 20, 2021

Made most of them a bit differently (realized late that you did them in your commit). As said in one of the comments, i didn't remove the sigs vector and use it to verify the recovered SIGa(0) result at the end.

Side from that, cherry-picked the last check for a failed verification against the aggregated threshold signature and gave you authorship for it 👍.

And.. only TODO that have left is the protocol naming standardizations that we spoke via pm. Will add them later today.

@furszy
Copy link
Author

furszy commented Jul 20, 2021

done, updated. We can move forward with this now.

Copy link

@random-zebra random-zebra left a comment

Choose a reason for hiding this comment

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

ACK d074717

@furszy furszy merged commit 676ea45 into PIVX-Project:main Jul 22, 2021
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