Conversation
There was a problem hiding this comment.
Pull request overview
Adds a new crypto/bip340 implementation to support BIP-340 (Schnorr over secp256k1) keys and signatures, including official BIP-340 test vectors to validate signing and verification behavior.
Changes:
- Introduces BIP-340 key types (
PublicKey,PrivateKey) with key generation, x-only serialization, signing, and verification. - Embeds and parses the official BIP-340 CSV test vectors and adds a test that runs them end-to-end.
- Integrates the implementation with the existing
crypto/_testsuiteharness for consistency with other key types.
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| crypto/bip340/key.go | Defines sizes/constants (incl. multibase code) and keypair generation with even-Y normalization. |
| crypto/bip340/private.go | Implements BIP-340 signing + tagged hash helper and private key import/serialization. |
| crypto/bip340/public.go | Implements x-only pubkey parsing/serialization and BIP-340 signature verification. |
| crypto/bip340/key_test.go | Hooks BIP-340 into the shared test harness and verifies against official vectors. |
| crypto/bip340/testvectors/vectors.go | Embeds and parses the official test vector CSV into typed structures. |
| crypto/bip340/testvectors/vectors.csv | Adds the official BIP-340 test vectors data file. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 3 potential issues.
Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, have a team admin enable autofix in the Cursor dashboard.
38f66c7 to
68517a8
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 6 out of 6 changed files in this pull request and generated 2 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| var vectors []Vector | ||
| for _, row := range rows[1:] { // skip header |
There was a problem hiding this comment.
Load() assumes the CSV has at least a header row and that every subsequent row has >= 8 columns. If the embedded CSV ever gains a blank line or a malformed row, this will panic with an index-out-of-range instead of returning a parse error. Consider validating len(rows) > 1 before slicing rows[1:], and validating len(row) == 8 (or >= 8) per row and returning a descriptive error when it doesn't match.
| var vectors []Vector | |
| for _, row := range rows[1:] { // skip header | |
| if len(rows) <= 1 { | |
| return nil, fmt.Errorf("vectors.csv: expected header plus at least one data row, got %d row(s)", len(rows)) | |
| } | |
| var vectors []Vector | |
| for i, row := range rows[1:] { // skip header | |
| if len(row) < 8 { | |
| return nil, fmt.Errorf("vectors.csv: row %d: expected at least 8 columns, got %d", i+2, len(row)) | |
| } |
| // Preconditions: r is normalized, r < p, s < n (enforced by caller). | ||
| func bip340Verify(r *secp256k1.FieldVal, s *secp256k1.ModNScalar, pubKey *secp256k1.PublicKey, msgHash []byte) bool { | ||
| // Step 1: let P = lift_x(public key) |
There was a problem hiding this comment.
bip340Verify takes msgHash []byte but callers pass the raw message bytes (BIP-340 does its own tagged hashing internally). Renaming this parameter to msg/message would reduce the chance of future misuse (e.g., someone pre-hashing before calling this helper) and better match how it’s used in VerifyBytes and bip340Sign.

Note
Medium Risk
Introduces new cryptographic signing/verification code paths (nonce derivation, tagged hashing, normalization) where subtle bugs can break security or interoperability, though coverage via official test vectors reduces the risk.
Overview
Adds a new
crypto/bip340package implementing BIP-340 (x-only secp256k1) key generation, byte serialization, signing, and verification, including multibase encoding/decoding support via a newMultibaseCode.Includes comprehensive tests: integration with the shared crypto testsuite plus verification/signing against the official BIP-340 CSV test vectors (embedded and parsed at runtime) to validate correctness and edge cases.
Written by Cursor Bugbot for commit 9c3f63a. This will update automatically on new commits. Configure here.