Add gnark as bls-12-381 backend #551
Conversation
Signed-off-by: Jakub Sztandera <[email protected]>
|
🔒 Could not start CI tests due to missing safe PR label. Please contact a DEDIS maintainer. |
Errcheck was already not checking other impls Signed-off-by: Jakub Sztandera <[email protected]>
|
It's probably going to take a while for upstream to merge the changes so we're importing just the changed package (BDN) and the new package (Gnark) into this repo. That way we avoid forking the entire repo but can still import our changes. Any changes to these pacakges should be submitted as PRs to upstream _first_, then backported to this repo. Includes: - dedis/kyber#546 - dedis/kyber#551 - dedis/kyber#553
It's probably going to take a while for upstream to merge the changes so we're importing just the changed package (BDN) and the new package (Gnark) into this repo. That way we avoid forking the entire repo but can still import our changes. Any changes to these pacakges should be submitted as PRs to upstream _first_, then backported to this repo. Includes: - dedis/kyber#546 - dedis/kyber#551 - dedis/kyber#553
It's probably going to take a while for upstream to merge the changes so we're importing just the changed package (BDN) and the new package (Gnark) into this repo. That way we avoid forking the entire repo but can still import our changes. Any changes to these pacakges should be submitted as PRs to upstream _first_, then backported to this repo. Includes: - dedis/kyber#546 - dedis/kyber#551 - dedis/kyber#553
Signed-off-by: Jakub Sztandera <[email protected]>
|
|
Please retry analysis of this Pull-Request directly on SonarQube Cloud |
| p.inner.Set(&aa.inner) | ||
| p.inner.AddAssign(&bb.inner) | ||
| return p | ||
| } | ||
|
|
||
| func (p *G1Elt) Sub(a, b kyber.Point) kyber.Point { | ||
| aa, bb := a.(*G1Elt), b.(*G1Elt) | ||
| p.inner.Set(&aa.inner) | ||
| p.inner.SubAssign(&bb.inner) | ||
| return p | ||
| } | ||
|
|
||
| func (p *G1Elt) Neg(a kyber.Point) kyber.Point { | ||
| p.inner.Neg(&a.(*G1Elt).inner) |
There was a problem hiding this comment.
All of these are not safe for concurrent use since they aren't cloning p but setting it / overwriting it, right?
So I guess this might warrant a comment / require to open an issue about it.
Either that or p.inner should be cloned prior to modification if this wasn't the intention.
There was a problem hiding this comment.
None of them are thread-safe.
I wouldn't expect any thread safety, and the bigint semantics (which this resembles) in Go are that the struct will be clobbered.
The straightforward reason for this is that it gives options to either clone before or reuse and save on allocations.
I can add a note.
|
|
||
| var _ kyber.Point = >Elt{} | ||
|
|
||
| // GTElt is a wrapper around the Circl Gt point type. |
There was a problem hiding this comment.
| // GTElt is a wrapper around the Circl Gt point type. | |
| // GTElt is a wrapper around the gnark GT point type. |
Given that kilic is deprecated (and caused us some issues recently), I'm opening this PR with BLS-12-381 backend using https://github.com/Consensys/gnark-crypto/
The Gnark's BLS-12-381 code was audited in the past (but there have been changes to it since), and it is actively maintained.
Signing performance on G2 is comparable to kilic, while the signing on G1 and public key aggregation performance is better than both circl and kilic.
https://gist.github.com/Stebalien/d6268b5c1abc4aa29857cf7787b205e8