Skip to content

BIP-352: vendor secp256k1lab and use it for reference implementation#2087

Merged
murchandamus merged 4 commits intobitcoin:masterfrom
theStack:bip352-vendor-secp256k1lab
Mar 6, 2026
Merged

BIP-352: vendor secp256k1lab and use it for reference implementation#2087
murchandamus merged 4 commits intobitcoin:masterfrom
theStack:bip352-vendor-secp256k1lab

Conversation

@theStack
Copy link
Contributor

This PR adds secp256k1lab version 1.0.0 as subtree within the bip-0352 folder [1] and takes use of it in the reference implementation. In particular, the file secp256k1.py is removed and the GE and Scalar classes are used from the secp256k1lab.secp256k1, replacing ECPubKey and ECKey, respectively. See the main commit message for a detailed table of replacement patterns for easier review. Usage of the library is mentioned in the BIP text to be license compliant (see #2004 (comment)).

Can be tested via:

$ ./bip-0352/reference.py ./bip-0352/send_and_receive_test_vectors.json
[ ..... ]
All tests passed

[1] added via the command $ git subtree add --prefix=bip-0352/secp256k1lab --squash https://github.com/secp256k1lab/secp256k1lab v1.0.0

@murchandamus murchandamus added Proposed BIP modification Pending acceptance This BIP modification requires sign-off by the champion of the BIP being modified labels Jan 16, 2026
@murchandamus
Copy link
Member

cc: @josibake, @RubenSomsen

@murchandamus murchandamus added BIP update by author and removed Proposed BIP modification Pending acceptance This BIP modification requires sign-off by the champion of the BIP being modified labels Feb 27, 2026
@murchandamus
Copy link
Member

@theStack: Since you are an author of BIP 352, if this is ready to go, please let me know, so I can just merge it.

@theStack
Copy link
Contributor Author

theStack commented Mar 2, 2026

@theStack: Since you are an author of BIP 352, if this is ready to go, please let me know, so I can just merge it.

Okay, I will give this another self-review within the next few days and then let you know. Also pinging some contributors who have previously reviewed a similar secp256k1lab vendoring PR for BIP-374, in case anyone wants to take a look: @nymius, @macgyver13, @stratospher, @real-or-random.
This PR is not a requirement for #2106, I see it more as long-term 'nice to have'.

@murchandamus
Copy link
Member

Okay, marking it as in your court, please let me know when it’s back in mine. :)

@murchandamus murchandamus added the PR Author action required Needs updates, has unaddressed review comments, or is otherwise waiting for PR author label Mar 2, 2026
@macgyver13
Copy link
Contributor

ACK 345f762

Tested:

python reference.py send_and_receive_test_vectors.json
...
All tests passed

theStack added 3 commits March 2, 2026 19:16
git-subtree-dir: bip-0352/secp256k1lab
git-subtree-split: 44dc4bd893b8f03e621585e3bf255253e0e0fbfb
This allows to remove secp256k1.py and replace the secp256k1-specific
parts in the reference implementation. Replacement guide:

    * ECKey -> Scalar
    * ECKey.set(seckey_bytes) -> Scalar.from_bytes_checked(seckey_bytes)
    * seckey.get_pubkey() -> seckey * G
    * seckey.get_bytes() -> seckey.to_bytes()
    * seckey.add(tweak_bytes) -> seckey + Scalar.from_bytes_checked(tweak_bytes)
    * seckey.negate() -> seckey = -seckey
    * seckey.sign_schnorr -> schnorr_sign(..., seckey.to_bytes(), ...)

    * ECPubKey -> GE
    * ECPubKey.set(pubkey_bytes) -> GE.from_bytes_{xonly,compressed}(pubkey_bytes)
    * pubkey.get_y() % 2 == 0 -> pubkey.has_even_y()
    * pubkey.get_bytes(False) -> pubkey.to_bytes_compressed()
    * pubkey.get_bytes() -> pubkey.to_bytes_xonly()
    * not pubkey.valid -> pubkey.infinity
    * pubkey.verify_schnorr -> schnorr_verify(..., pubkey.to_bytes_xonly(), ...)

    * TaggedHash -> tagged_hash
    * hashlib.sha256(preimage).digest() -> hash_sha256(preimage)
@theStack theStack force-pushed the bip352-vendor-secp256k1lab branch from 345f762 to c27b0a3 Compare March 5, 2026 01:30
@theStack
Copy link
Contributor Author

theStack commented Mar 5, 2026

Rebased on master, due to a conflict after #2106 was merged.

Copy link
Contributor

@nymius nymius left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ACK c27b0a3

All tests passed.

@murchandamus
Copy link
Member

Rebased on master, due to a conflict after #2106 was merged.

Did you also finish your self-review, or was this prep for the self-review?

also fix a small grammar nit (s/are provided/is provided/)
@theStack theStack force-pushed the bip352-vendor-secp256k1lab branch from c27b0a3 to 249bdef Compare March 6, 2026 14:21
@theStack
Copy link
Contributor Author

theStack commented Mar 6, 2026

Rebased on master, due to a conflict after #2106 was merged.

Did you also finish your self-review, or was this prep for the self-review?

Took another look today, and I consider it ready for merge.

@murchandamus murchandamus merged commit b382728 into bitcoin:master Mar 6, 2026
4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

BIP update by author PR Author action required Needs updates, has unaddressed review comments, or is otherwise waiting for PR author

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants