dedis / kyber

Advanced crypto library for the Go language
Other
636 stars 168 forks source link

Optimize BDN Signature/Key Aggregation #546

Closed Stebalien closed 1 week ago

Stebalien commented 3 weeks ago

Originally filed against drand's fork as https://github.com/drand/kyber/pull/60 and https://github.com/drand/kyber/pull/61.

This set of patches:

  1. Removes an N^2 bitfield operation from aggregation. Not a huge deal, but it can become an issue when the set of participants grow.
  2. Introduces a "cached" mask for BDN where coefficients and public key aggregation terms are precomputed. This significantly improves the performance of public key aggregation (88x on my machine) when the different subsets of the same set of public keys are aggregated over and over.

I've also added some test fixtures for BDN at @AnomalRoil's request and added a benchmark for the CachedMask.

CLAassistant commented 3 weeks ago

CLA assistant check
All committers have signed the CLA.

github-actions[bot] commented 3 weeks ago

🔒 Could not start CI tests due to missing safe PR label. Please contact a DEDIS maintainer.

github-actions[bot] commented 3 weeks ago

🔒 Could not start CI tests due to missing safe PR label. Please contact a DEDIS maintainer.

github-actions[bot] commented 3 weeks ago

🔒 Could not start CI tests due to missing safe PR label. Please contact a DEDIS maintainer.

github-actions[bot] commented 3 weeks ago

🔒 Could not start CI tests due to missing safe PR label. Please contact a DEDIS maintainer.

sonarcloud[bot] commented 3 weeks ago

Quality Gate Passed Quality Gate passed

Issues
2 New issues
0 Accepted issues

Measures
0 Security Hotspots
60.3% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarCloud

Stebalien commented 3 weeks ago

Now that we have the notion of Scheme for bdn, I wonder if the masks shouldn't just be embedded in these and just have new exported methods on the Scheme to manage participants and public keys, as well as a method taking an aggregate signature and returning the list of participants or so?

IMO, it's best to keep scheme independent of the participants. But I can make Mask BDN specific and probably simplify it a bit.

sonarcloud[bot] commented 3 weeks ago

Quality Gate Passed Quality Gate passed

Issues
6 New issues
0 Accepted issues

Measures
0 Security Hotspots
88.4% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarCloud

Stebalien commented 3 weeks ago

@AnomalRoil I've moved Mask into BDN and removed the abstraction. This is significantly cleaner.

rjan90 commented 1 week ago

@AnomalRoil could you give this a final review? I think Steb has addressed your last outstanding comments

Stebalien commented 1 week ago

Honestly, no rush; our deadlines aren't your deadlines. Given the interface-based decoupling in kyber, we were able to pretty cleanly vendor just the BDN package so we're not blocked on any of our PRs.

sonarcloud[bot] commented 1 week ago

Quality Gate Passed Quality Gate passed

Issues
6 New issues
0 Accepted issues

Measures
0 Security Hotspots
88.4% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarCloud