-
Notifications
You must be signed in to change notification settings - Fork 20.3k
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
crypto: use btcec/v2 for no-cgo #24533
Conversation
Thanks! I will review tomorrow! |
8e1026c
to
2f96c2b
Compare
Pushed some minor changes to make it a bit more defensive: checking expected lengths, using package-level consts, and zeroing the temporary |
This updates the no-cgo implementations in the crypto package to use the github.com/btcsuite/btcd/btcec/v2 module instead of the older btcec package that was part of the main github.com/btcsuite/btcd module. name old time/op new time/op delta EcrecoverSignature-32 198µs ± 0% 144µs ± 0% -27.11% VerifySignature-32 177µs ± 0% 128µs ± 0% -27.44% DecompressPubkey-32 20.9µs ± 0% 10.1µs ± 0% -51.51% Use (*ModNScalar).IsOverHalfOrder instead of math/big.Int when checking for malleable signatures.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've checked this some more and it works as expected! While performance of btcec still lags behind C libsecp256k1 quite a bit, I can confirm 20% speedup of the non-cgo path with this update.
Thanks again for this work!
This updates the no-cgo implementations in the crypto package to use the github.com/btcsuite/btcd/btcec/v2 module instead of the older btcec package that was part of the main github.com/btcsuite/btcd module. name old time/op new time/op delta EcrecoverSignature-32 198µs ± 0% 144µs ± 0% -27.11% VerifySignature-32 177µs ± 0% 128µs ± 0% -27.44% DecompressPubkey-32 20.9µs ± 0% 10.1µs ± 0% -51.51% Use (*ModNScalar).IsOverHalfOrder instead of math/big.Int when checking for malleable signatures.
This updates the no-cgo implementations in the crypto package to use the github.com/btcsuite/btcd/btcec/v2 module instead of the older btcec package that was part of the main github.com/btcsuite/btcd module. name old time/op new time/op delta EcrecoverSignature-32 198µs ± 0% 144µs ± 0% -27.11% VerifySignature-32 177µs ± 0% 128µs ± 0% -27.44% DecompressPubkey-32 20.9µs ± 0% 10.1µs ± 0% -51.51% Use (*ModNScalar).IsOverHalfOrder instead of math/big.Int when checking for malleable signatures.
This updates the no-cgo code in the
crypto
package to use the new github.com/btcsuite/btcd/btcec/v2 module instead of the older btcec package that was part of the main github.com/btcsuite/btcd module.This is nice as it removes the dependency on the legacy
btcd
mega-module that does not yet follow semantic import versioning (has breaking Go API changes between versions like v0.21 and v0.22) with the newbtcec/v2
module that does follow SIV, plus it brings performance improvements:The tests that passed previously with
CGO_ENABLED=0
still pass, and the secp256k1 fuzzing results show no discrepancies with the cgo version.tests/fuzzers/secp256k1:
The most notable recent breaking change in the main
btcd
module that was previously required is that with the introduction of thebtcec/v2
module, there is no longer a btcec package in the mainbtcd
module, which creates build errors when the latest revisions of btcd are used by either go-ethereum or transitive dependencies. Moving to thebtcec/v2
module resolves this issue.CC @fjl