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.

Sign up for GitHub

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

Jump to bottom

[BLS] Add wrapper around chiabls lib, worker, benchmarks and unit tests #2420

Merged
merged 20 commits into from
Sep 17, 2021
Merged

[BLS] Add wrapper around chiabls lib, worker, benchmarks and unit tests #2420

merged 20 commits into from
Sep 17, 2021

Conversation

random-zebra
Copy link

@random-zebra random-zebra commented Jun 14, 2021 •
edited
Loading

Based on top of:

This reworks #2405, adapting the commits to the new build system (which includes the library already at v1.0.1, as git subtree).

@random-zebra random-zebra added this to the 6.0.0 milestone Jun 14, 2021
@random-zebra random-zebra self-assigned this Jun 14, 2021
@random-zebra random-zebra mentioned this pull request Jun 14, 2021
Merged
3 tasks
@Fuzzbawls Fuzzbawls mentioned this pull request Jun 24, 2021
Merged
@random-zebra random-zebra force-pushed the 202106_bls-wrapper branch 2 times, most recently from 9f087cd to 5a6d14e Compare July 28, 2021 20:52
@random-zebra
Copy link
Author

Rebased on master. Ready for review.

@random-zebra random-zebra force-pushed the 202106_bls-wrapper branch 2 times, most recently from 09edb1a to ad10b65 Compare July 30, 2021 10:59
@furszy furszy modified the milestones: 5.3.0, 6.0.0 Jul 30, 2021
@random-zebra random-zebra force-pushed the 202106_bls-wrapper branch 2 times, most recently from b86ab3a to 1d48ccf Compare September 3, 2021 12:33
furszy
furszy reviewed Sep 6, 2021
View reviewed changes
Copy link

@furszy furszy left a comment

Choose a reason for hiding this comment

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

Code reviewed up to 32121d3.
So far, so good ☕ ☕. Left two comments in the BLS IES area.

Plus added test coverage for the basic encryption and decryption scheme here furszy@8b8c223. If you want to cherry-pick It, all yours.

First stop before jumping into the thread pool workers and DKG bench waters.

src/bls/bls_ies.cpp Outdated Show resolved Hide resolved
src/bls/bls_ies.cpp Show resolved Hide resolved
@random-zebra random-zebra force-pushed the 202106_bls-wrapper branch 3 times, most recently from 100ebc5 to 7acc769 Compare September 7, 2021 11:47
@random-zebra
Copy link
Author

Thanks @furszy . Rebased on master and picked your test, only reworking it using CBLSIESEncryptedObject<std::string> instead of CBLSIESEncryptedBlob directly (in case we want to keep this code).

Inspired by this, have also added encryption/decryption (with CBLSIESMultiRecipientObjects<CBLSSecretKey>) for the contribution shares in the DKG test.

furszy
furszy reviewed Sep 8, 2021
View reviewed changes
Copy link

@furszy furszy left a comment

Choose a reason for hiding this comment

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

Reviewed till a24d9ba.
Few more comments, nothing biggie.

src/bls/bls_worker.cpp Outdated Show resolved Hide resolved
src/bls/bls_worker.cpp Outdated Show resolved Hide resolved
src/bls/bls_worker.cpp Show resolved Hide resolved
furszy
furszy reviewed Sep 13, 2021
View reviewed changes
Copy link

@furszy furszy left a comment

Choose a reason for hiding this comment

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

Review completed, the DKG test made me feel at home ☕ . Code ACK 21d2f24.
Left few more comments, nothing blocking. Can squash the changes and it's ready to go for me.

src/bench/bls.cpp Outdated Show resolved Hide resolved
src/bench/bls.cpp Outdated Show resolved Hide resolved
src/test/bls_tests.cpp Outdated Show resolved Hide resolved
src/test/bls_tests.cpp Outdated Show resolved Hide resolved
random-zebra and others added 13 commits September 14, 2021 13:41
@random-zebra
[Trivial] bls_worker: Fix members initialization order in Aggregators
7bb9f02 
Also fix review nits:

- replace hardware_concurrency() with GetNumCores()
- remove code duplication for (Async)VerifyContributionShare
@codablock @random-zebra
Add BLS and DKG benchmarks
9502597
@random-zebra
[Trivial] Remove unused variables in bls bench test
1c3f2cb
@random-zebra
[BUG] Initialize Random for bench tests
5b16d89
@codablock @random-zebra
Add ECDSA benchmarks
f5c0870
@codablock @random-zebra
Bail out early from secure deallocation
104c4b0
@random-zebra
[Refactor] Initialize BLS allocator in BasicTestingSetup
c4c6efe
@random-zebra
[QA] Add test for BLS sign/verify message and sethexstr
0253ea7
@random-zebra
[QA] Add unit test for DKG using wrapper/worker
9f4ac39
@random-zebra
[Refactoring] Replace std::random_shuffle with Shuffle
aa51751 
std::random_shuffle is deprecated in C++14 and will be removed in C++17
@furszy @random-zebra
Test: Add bls ies basic encryption and decryption test coverage.
0453474
@random-zebra
[QA] Refactor bls_ies_tests using CBLSIESEncryptedObject
db69885
@random-zebra
[QA] Test encryption/decryption of sk shares in dkg unit test
0d126ee
@random-zebra
Copy link
Author

Rebased and squashed.

furszy
furszy approved these changes Sep 14, 2021
View reviewed changes
Copy link

@furszy furszy left a comment

Choose a reason for hiding this comment

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

one step closer, ACK 0d126ee

Fuzzbawls
Fuzzbawls approved these changes Sep 17, 2021
View reviewed changes
Copy link
Collaborator

@Fuzzbawls Fuzzbawls left a comment

Choose a reason for hiding this comment

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

ACK 0d126ee

@random-zebra random-zebra merged commit a589449 into PIVX-Project:master Sep 17, 2021
furszy added a commit that referenced this pull request Oct 8, 2021
@furszy
Merge #2421: [Refactor] MN operator key migration to BLS
295dabf 
d3843cd [Refactor] No need to get the publickey of the active masternode (random-zebra)
483f509 [Refactor] De-duplicate operator BLS key parsing in rpc (random-zebra)
855c094 [Build] Fix CMake builds with chiabls/relic includes (random-zebra)
77ec208 [Tests] Fix TierTwo functional tests with BLS operator key (random-zebra)
dd46f72 [TierTwo] Migration of DMN operator key to BLS (random-zebra)
1dfdcc1 [RPC] Implement generateblskeypair() RPC command (random-zebra)
619c0f8 scripted-diff: Rename keyIDOperator to pubKeyOperator (random-zebra)
b0a7fa5 [QA] Add tests for fbv messages signed with BLS keys (random-zebra)
dd0fb01 [TierTwo] Add functions to sign/verify messages with BLS (random-zebra)

Pull request description:

  This builds on top of:

  - [x] #2363
  - [x] #2419
  - [x] #2420

  As per title, introduce BLS keys/signatures for messages signed by the masternode operator (which is a requirement for [DIP 6](https://github.com/dashpay/dips/blob/master/dip-0006.md), and subsequent upgrades).

  - a new RPC command `generateblskeypair` returns a new publickey/secretkey BLS keypair.
  - `CDeterministicMNState::keyIDOperator` is now a `CBLSLazyPublicKey` (instead of a `CKeyID`), and it's renamed `pubKeyOperator`.
  - `-mnoperatorprivatekey` flag takes a BLS secret key in hex format.
  - `CActiveMasternodeInfo` stores a `CBLSPublicKey`/`CBLSSecretKey`
  - the BLS signature byte vector is serialized for legacy messages (mnw, fbv) in the compatibility code.

ACKs for top commit:
  furszy:
    great, ACK d3843cd
  Fuzzbawls:
    ACK d3843cd

Tree-SHA512: 1caf49615283d10b356caa049b57091dc70805c0bf2c11645d560c81223ce0d9936a46096cc4871c5ec25164518ebdf259d0f23a8c5bca39f2e436cc9e954519
@Fuzzbawls Fuzzbawls modified the milestones: 6.0.0, 5.5.0 Sep 11, 2022
@Fuzzbawls Fuzzbawls modified the milestones: 5.5.0, 6.0.0 Dec 12, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Fuzzbawls Fuzzbawls Fuzzbawls approved these changes

@furszy furszy furszy approved these changes

Assignees

@random-zebra random-zebra

Labels
Tests Upstream Dash Util / log / libs
Projects
None yet
Milestone
6.0.0
Development

Successfully merging this pull request may close these issues.
4 participants
@random-zebra @furszy @Fuzzbawls @codablock