Go 1.26 support - #44
Merged
Merged
Conversation
tarakby
marked this pull request as ready for review
August 6, 2026 19:42
turbolent
reviewed
Aug 6, 2026
turbolent
left a comment
turbolent
left a comment
Member
There was a problem hiding this comment.
I reviewed the code from a Go-perspective, but I don't have the expertise to review the crypto changes
Member
|
Could you please improve the PR title? |
turbolent
reviewed
Aug 6, 2026
tarakby
commented
Aug 10, 2026
- return untyped nil interfaces on key decoding errors - return false instead of panicking in Equals(nil) - reject hashers whose computed hash is shorter than their declared size - clip the capacity of secp256k1 signatures sliced from the [R||S||V] buffer - make lazy PublicKey() construction safe for concurrent use - derive (N-1)/2 from N and cross-check the secp256k1 constants against go-ethereum at initialization - restore the P-256 public key length check - add tests: RFC 6979 known-answer vectors, concurrent PublicKey, nil inputs, dishonest hasher, compressed edge points
2 tasks
- build the public key eagerly at private key construction instead of lazily in PublicKey(), which removes the concurrency guards - read the secp256k1 curve constants from go-ethereum instead of hardcoding them and cross-checking - check the hasher output in bytes only, based on len(h), and drop the redundant hasher.Size() check - drop the P-256 public key length check, already covered by ParseUncompressedPublicKey - return the untyped nil once after the switch in the decoding functions - move the nil-input checks to sign_test_utils.go so they cover all algos, and extend them to DecodePublicKeyCompressed - document that the RFC 6979 vectors only test the current implementation, not a property the package guarantees - drop the concurrent PublicKey and compressed edge point tests
Contributor
|
I see that pretty much all of my comments a re covered in #46. I wish I saw that sooner :). I'll resolve my comments |
Fix ECDSA edge cases and harden new implementation
Upgrade to blst v0.3.17
turbolent
approved these changes
Aug 11, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
closes #40
CGO_ENABLED=1, in the past version SECp256k1 did not use cgoThis PR also removes the non-cgo build mode, which is used to build the library without cgo (
CGO_ENABLED=0) while disabling BLS. Now that ECDSA secp256k1 also uses cgo, the non-cgo mode has very limited utility and is discontinued.Bench of ECDSA secp256k1 before this update:
Bench of ECDSA secp256k1 after this update: