BIP-374: vendor secp256k1lab and use it for reference implementation #2084

pull theStack wants to merge 6 commits into bitcoin:master from theStack:bip374-vendor-secp256k1lab changing 17 files +442 −127
  1. theStack commented at 1:00 am on January 15, 2026: contributor

    Following up on the recent decision that secp256k1lab should be vendored per-BIP if needed/useful (see #2004 (comment) ff. and #1855 (comment) ff.), this PR exercises this for BIP-374. The changes add secp256k1lab version 1.0.0 as subtree [1] within the bip-0374 folder and take use of it in the reference implementation. In particular, secp256k1.py is removed and the TaggedHash and xor_bytes routines are used from the secp256k1lab utils module, to simplify the code further. Usage of the library is mentioned in the BIP text to be license compliant (see #2004 (comment)).

    Can be tested via:

    0$ ./bip-0374/run_test_vectors.py
    1.....
    2All test vectors passed.
    3$ ./bip-0374/gen_test_vectors.py
    4Generating /home/thestack/bips/bip-0374/test_vectors_generate_proof.csv...
    5Generating /home/thestack/bips/bip-0374/test_vectors_verify_proof.csv...
    6$ git diff
    7<shouldn't show any diffs, as generated .csv files should match the ones in the repository> 
    

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

  2. Squashed 'bip-0374/secp256k1lab/' content from commit 44dc4bd
    git-subtree-dir: bip-0374/secp256k1lab
    git-subtree-split: 44dc4bd893b8f03e621585e3bf255253e0e0fbfb
    3050bb6b25
  3. Merge commit '3050bb6b25c0c20b62e2fc1a23276a09d50d151b' as 'bip-0374/secp256k1lab' d2ceae1dd6
  4. BIP-374: avoid using sys.path[0] to find current working directory
    This approach is incompatible with the sys.path extension approach
    in the next commit which is used to to find the vendored copy of
    secp256k1lab, so use __file__ instead which works as well.
    4e18ee641b
  5. BIP-374: replace secp256k1.py with vendored copy of secp256k1lab 459d977d9b
  6. BIP-374: use `tagged_hash` and `xor_bytes` routines from secp256k1lab 436a3dd1fa
  7. BIP-374: mention secp256k1lab in BIP text 2b7f07986b
  8. nymius commented at 1:38 pm on January 15, 2026: contributor

    ACK 2b7f07986b5f62753486e5fd9db59c189eed946e

    The changes are aligned with the referenced comments. I’m not reviewing the vendor library as the change is not about the qualities of it.

  9. macgyver13 commented at 5:38 pm on January 16, 2026: contributor

    ACK https://github.com/bitcoin/bips/commit/2b7f07986b5f62753486e5fd9db59c189eed946e

    Tested vector generation and confirmed no changes to generated .csv files.

    Additionally tested the same technique in #2046. The next revision of #2046 will depend on the changes in this PR.

  10. murchandamus added the label BIP update by author on Jan 16, 2026
  11. murchandamus commented at 8:14 pm on January 16, 2026: contributor
    cc: @andrewtoth, @RubenSomsen; perhaps also of interest to @real-or-random.
  12. murchandamus commented at 8:19 pm on January 16, 2026: contributor
    Also, while I have you here, BIP 374 has been published for over a year, it appears to have a reference implementation and test vectors, do you still have planned work outstanding? Otherwise, you could perhaps consider whether it is time to advance it from Draft to Complete.
  13. stratospher commented at 12:22 pm on January 17, 2026: contributor
    ACK 2b7f079.
  14. real-or-random commented at 8:54 pm on January 20, 2026: contributor

    Just a minor clarifying comment:

    Usage of the library is mentioned in the BIP text to be license compliant (see #2004 (comment)).

    What makes the usage license compliant is not that it’s mentioned in the BIP but that the COPYING file is included. This is essentially the only condition in the MIT License: “The above copyright notice and this permission notice shall be included in all copies or substantial portions of the Software.” (I recommend this blog post for a deeper understanding of the MIT License.)


    It’s a bit sad that files like the GitHub Actions stuff from secp256k1lab will end up in the BIPs repo, and current the upstream main has even a test directory now. Still, subtreeing the entire repo seems ike the most pragmatic approach. Filtering these additional files will just make things complicated and get in the way of git subtree.

    One thing I could think of is vendoring only the src/secp256k1lab subdir. (And we could update the upstream repo to have the COPYING file there to make this easier.) But as far as I understand, git subtree can’t handle this (without some ugly manual tricks). So we’d need to resort to manual copying of the files, but this wouldn’t be the end of the world either. What do others think?

  15. nymius commented at 4:02 pm on January 22, 2026: contributor

    One thing I could think of is vendoring only the src/secp256k1lab subdir. (And we could update the upstream repo to have the COPYING file there to make this easier.) But as far as I understand, git subtree can’t handle this (without some ugly manual tricks). So we’d need to resort to manual copying of the files, but this wouldn’t be the end of the world either. What do others think?

    What about providing trimmed releases together with the full releases, something like v1.0.0-slim? Here is a diff of what I mean. This is the trimmed v1.0.0-slim version I’m sub-treeing there. Notice in that commit I’m removing the .github directory, and that could be done with the test/ dir too.

    This is adding extra work on secp256k1lab, but I think is easier to automatize on their side and is moving the maintenance of the code away from the bips repository.

    The same could be done to keep the source files and the COPYING file together without README, pyproject and the others. But I thought those files also carry some important info that’s nice to keep around (like authors in the pyproject file for example)

  16. murchandamus commented at 2:50 am on January 27, 2026: contributor
    Just so we don’t have a misunderstanding, I see several ACKs here, but I’m on hold waiting for @theStack to chime in.
  17. real-or-random commented at 7:57 am on January 27, 2026: contributor

    After some more thought, I think it’s not an unreasonable idea to remove some unneeded files, but I still believe it’s most pragmatic to vendor the entire subtree. The tree is still lightweight, even with the tests directory.

    But I’m not the one making the call here. BIP authors are free to do what they want, and it’s okay that different people arrive at different conclusions. Ultimately, each BIP is a separate project, and these just happen to be organized in a monorepo.

    Also, where do you draw the line? README is also good to have because it tells people that this is a somewhat separate thing. tests will be very useful if some BIP authors make changes to secp256k1lab to make it usable for their use case. It’s better to avoid that bikeshedding and let BIP authors decide.

    Should we do anything on the secp256k1lab side? I’m not convinced for the reasons above. Of course, we’re always open to concrete suggestions there, and I think any further discussion should happen in the secp256k1lab repo.

  18. theStack commented at 2:49 pm on January 27, 2026: contributor

    I agree that neither subtreeing the whole secp256k1lab repository (-> lots of unneeded files ending up in the BIPs repo) nor adding a reduced version (-> more decisions needed on what exactly to include, likely more review burden) is fully satisfying. No super-strong opinion, but I’d also lean more towards the most pragmatic approach, and thus leave the PR as-is, if there are no objections from BIP editors.

    But I’m not the one making the call here. BIP authors are free to do what they want, and it’s okay that different people arrive at different conclusions. Ultimately, each BIP is a separate project, and these just happen to be organized in a monorepo.

    Good point. Practically, I think future BIP authors that want to vendor secp256k1lab will very likely look how existing BIPs do it and use the same approach. So in some sense merging this PR would set an example in either direction. If we are feeling uncomfortable imagining e.g. dozens of BIPs all with entire secp256k1lab subtrees in the BIPs repo, it seems to be easier to push back now than later. Other than that consideration, seems fine to me to merge.

  19. murchandamus commented at 9:40 pm on January 27, 2026: contributor
    I don’t think the less than 150 kB per instance would be a major issue.
  20. theStack commented at 11:11 pm on January 27, 2026: contributor

    I don’t think the less than 150 kB per instance would be a major issue.

    Okay, then I’d say its RFM, as far as I’m concerned.

  21. murchandamus commented at 6:31 pm on January 28, 2026: contributor
    Light review ACK 2b7f07986b5f62753486e5fd9db59c189eed946e
  22. murchandamus merged this on Jan 28, 2026
  23. murchandamus closed this on Jan 28, 2026

  24. theStack deleted the branch on Jan 28, 2026

github-metadata-mirror

This is a metadata mirror of the GitHub repository bitcoin/bips. This site is not affiliated with GitHub. Content is generated from a GitHub metadata backup.
generated: 2026-02-10 18:10 UTC

This site is hosted by @0xB10C
More mirrored repositories can be found on mirror.b10c.me