Update src/secp256k1 subtree to release v0.3.1 #27445

1. 
   sipa commented at 12:44 AM on April 11, 2023: member

   There is no strict need for any of the changes in v0.3.1 (compared to the v0.3.0 that's currently subtreed) for Bitcoin Core release builds, but if anyone may compile Bitcoin Core from source using Clang v14+, this will prevent known timing leaks in the signing/keygen logic.

   This also includes a CI fix from libsecp256k1 master (on top of 0.3.1) which fixes Wycheproof test vector generation.

   I also had to amend some of the linters to avoid enforcing their rules on the .py files in the secp256k1 subtree.

2. 
   DrahtBot commented at 12:44 AM on April 11, 2023: contributor

   <!--e57a25ab6845829454e8d69fc972939a-->

   The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.

   <!--021abf342d371248e50ceaed478a90ca-->

   Reviews

   See the guideline for information on the review process.

   Type	Reviewers
   ACK	real-or-random, fanquake

   If your review is incorrectly listed, please react with 👎 to this comment and the bot will ignore it on the next update.

   <!--174a7506f384e20aa4161008e828411d-->

   Conflicts

   Reviewers, this pull request conflicts with the following ones:

   • #23432 (BIP324: CKey encode/decode to elligator-swift by dhruv)

   If you consider this pull request important, please also help to review the conflicting pull requests. Ideally, start with the one that should be merged first.

3. in src/secp256k1/Makefile.am:260 in bf2671a10d outdated

    255 | +
    256 | +TESTVECTORS = src/wycheproof/ecdsa_secp256k1_sha256_bitcoin_test.h
    257 | +
    258 | +src/wycheproof/ecdsa_secp256k1_sha256_bitcoin_test.h: src/wycheproof/ecdsa_secp256k1_sha256_bitcoin_test.json
    259 | +	python3 tools/tests_wycheproof_generate.py $< > $@
    260 | +
   

   ---

   ---

   

   real-or-random commented at 8:36 AM on April 11, 2023:

   Shot in the dark: Can you try this and see if it makes CI happy?

   src/wycheproof/ecdsa_secp256k1_sha256_bitcoin_test.h:
   	python3 tools/tests_wycheproof_generate.py src/wycheproof/ecdsa_secp256k1_sha256_bitcoin_test.json > $@
   
   

4. 
   real-or-random commented at 9:01 AM on April 11, 2023: contributor

   Build failure on CI here. This is during make distdir.

    (cd secp256k1 && make  top_distdir=../../bitcoin-i686-pc-linux-gnu distdir=../../bitcoin-i686-pc-linux-gnu/src/secp256k1 \
        am__remove_distdir=: am__skip_length_check=: am__skip_mode_fix=: distdir)
   make[4]: Entering directory '/tmp/cirrus-ci-build/ci/scratch/build/src/secp256k1'
   make  distdir-am
   make[5]: Entering directory '/tmp/cirrus-ci-build/ci/scratch/build/src/secp256k1'
   python3 tools/tests_wycheproof_generate.py /tmp/cirrus-ci-build/src/secp256k1/src/wycheproof/ecdsa_secp256k1_sha256_bitcoin_test.json > src/wycheproof/ecdsa_secp256k1_sha256_bitcoin_test.h
   /bin/dash: 1: cannot create src/wycheproof/ecdsa_secp256k1_sha256_bitcoin_test.h: Directory nonexistent
   

   https://cirrus-ci.com/task/4665749793406976?logs=ci#L986

   So, apparently the wycheproof directory doesn't exist. That's why (re)creating ecdsa_secp256k1_sha256_bitcoin_test.h fails.

   The correct behavior would be that the wycheproof dir just exists (including ecdsa_secp256k1_sha256_bitcoin_test.h), so it wouldn't need to be created in the first place.

   I have no idea why this fails. make distdir works for me locally, even in a out-of-tree build.

5. 
   real-or-random commented at 9:33 AM on April 11, 2023: contributor

   Okay, I think I understand what's going on here:

   1. make distdir is supposed to create a directory with all files to be distributed
   2. the distdir target depends on all files to be distributed
   3. these file include src/wycheproof/secdsa_secp256k1_sha256_bitcoin_test.h because it's listed in EXTRA_DIST (perhaps noinst_HEADERS would be more appropriate, but the result is the same)
   4. src/wycheproof/secdsa_secp256k1_sha256_bitcoin_test.h has a dependency on src/wycheproof/secdsa_secp256k1_sha256_bitcoin_test.json
   5. if the .json file in the src dir appears to be newer than the .h file in the build dir (e.g., due to meaningless timestamps after copying the source dir), make distdir tries to rebuild the .h in the build dir
   6. but this fails because not even the src/wycheproof subdirectory exists in the build dir (in a out-of-tree/VPATH build)

   Puh.

   Item 4 is a bug in libsecp256k1. While this dependency exists in fact, we don't want to list in the Makefile because the .h file is checked in the repo and only meant to be rebuilt by developers/maintainers of the library. (We similarly omit explicit dependencies for other generated source files.) @sipa My suggestion above fixes exactly this. Can you to commit this here to see if it makes the problem disappear, just for a quick and dirty check of CI? If that works, we can create a proper fix in libsecp256k1 and update this PR accordingly.

   Item 7 is another bug in libsecp256k1, which is however only relevant to developers of the library. (Well, and users like Gentoo who insist on rebuilding everything from scratch in an out-of-tree build, see https://github.com/bitcoin-core/secp256k1/pull/1160.) The fix should simply be another mkdir -p call.

6. achow101 added this to the milestone 25.0 on Apr 13, 2023
7. sipa force-pushed on Apr 14, 2023
8. real-or-random referenced this in commit 06c67dea9f on Apr 14, 2023
9. real-or-random referenced this in commit 3111cc2785 on Apr 14, 2023
10. 
    RandomLattice commented at 11:18 AM on April 14, 2023: none

    Okay, I think I understand what's going on here

    Great investigation, thanks! And sorry this broke the build.

    I see you already put hotfixes in https://github.com/bitcoin-core/secp256k1/pull/1276 for the autotools issue. Thanks. I can put a hotfix for the linting issue + open an issue for a longer-term fix for linting.

11. real-or-random referenced this in commit 4258c54f4e on Apr 14, 2023
12. 
    real-or-random commented at 12:19 PM on April 14, 2023: contributor

    @sipa https://github.com/bitcoin-core/secp256k1/pull/1276 has been merged, can you update this PR to include the current secp256k1 master?

13. Squashed 'src/secp256k1/' changes from bdf39000b9..4258c54f4e

    4258c54f4e Merge bitcoin-core/secp256k1#1276: autotools: Don't regenerate Wycheproof header automatically
    06c67dea9f autotools: Don't regenerate Wycheproof header automatically
    3bab71cf05 Merge bitcoin-core/secp256k1#1268: release cleanup: bump version after 0.3.1
    656c6ea8d8 release cleanup: bump version after 0.3.1
    346a053d4c Merge bitcoin-core/secp256k1#1269: changelog: Fix link
    6a37b2a5ea changelog: Fix link
    ec98fcedd5 Merge bitcoin-core/secp256k1#1266: release: Prepare for 0.3.1
    898e1c676e release: Prepare for 0.3.1
    1d9a13fc26 changelog: Remove inconsistent newlines
    0e091669a1 changelog: Catch up in preparation of 0.3.1
    7b7503dac5 Merge bitcoin-core/secp256k1#1245: tests: Add Wycheproof ECDSA vectors
    145078c418 Merge bitcoin-core/secp256k1#1118: Add x-only ecmult_const version with x specified as n/d
    e5de454609 tests: Add Wycheproof ECDSA vectors
    0f8642079b Add exhaustive tests for ecmult_const_xonly
    4485926ace Add x-only ecmult_const version for x=n/d
    a0f4644f7e Merge bitcoin-core/secp256k1#1252: Make position of * in pointer declarations in include/ consistent
    4e682626a3 Merge bitcoin-core/secp256k1#1226: Add CMake instructions to release process
    2d51a454fc Merge bitcoin-core/secp256k1#1257: ct: Use volatile "trick" in all fe/scalar cmov implementations
    4a496a36fb ct: Use volatile "trick" in all fe/scalar cmov implementations
    3d1f430f9f Make position of * in pointer declarations in include/ consistent
    2bca0a5cbf Merge bitcoin-core/secp256k1#1241: build: Improve `SECP_TRY_APPEND_DEFAULT_CFLAGS` macro
    afd8b23b27 Merge bitcoin-core/secp256k1#1244: Suppress `-Wunused-parameter` when building for coverage analysis
    1d8f367515 Merge bitcoin-core/secp256k1#1250: No need to subtract 1 before doing a right shift
    3e43041be6 No need to subtract 1 before doing a right shift
    3addb4c1e8 build: Improve `SECP_TRY_APPEND_DEFAULT_CFLAGS` macro
    0c07c82834 Add CMake instructions to release process
    464a9115b4 Merge bitcoin-core/secp256k1#1242: Set ARM ASM symbol visibility to `hidden`
    f16a709fd6 Merge bitcoin-core/secp256k1#1247: Apply Checks only in VERIFY mode.
    70be3cade5 Merge bitcoin-core/secp256k1#1246: Typo
    4ebd82852d Apply Checks only in VERIFY mode.
    d1e7ca192d Typo
    5bb03c2911 Replace `SECP256K1_ECMULT_TABLE_VERIFY` macro by a function
    9c8c4f443c Merge bitcoin-core/secp256k1#1238: build: bump CMake minimum requirement to 3.13
    0cf2fb91ef Merge bitcoin-core/secp256k1#1243: build: Ensure no optimization when building for coverage analysis
    fd2a408647 Set ARM ASM symbol visibility to `hidden`
    4429a8c218 Suppress `-Wunused-parameter` when building for coverage analysis
    8e79c7ed11 build: Ensure no optimization when building for coverage analysis
    96dd062511 build: bump CMake minimum requirement to 3.13
    427bc3cdcf Merge bitcoin-core/secp256k1#1236: Update comment for secp256k1_modinv32_inv256
    647f0a5cb1 Update comment for secp256k1_modinv32_inv256
    5658209459 Merge bitcoin-core/secp256k1#1228: release cleanup: bump version after 0.3.0
    28e63f7ea7 release cleanup: bump version after 0.3.0
    
    git-subtree-dir: src/secp256k1
    git-subtree-split: 4258c54f4ebfc09390168e8a43306c46b315134b

    c981671e9b
14. Update src/secp256k1 to latest upstream master (v0.3.1 + CI fix) f5fdd4e279
15. Disable Python lint in src/secp256k1 719a74989b
16. sipa force-pushed on Apr 14, 2023
17. Respect and update FILES_ARGS in test/lint/lint-python.py 621c17869d
18. sipa force-pushed on Apr 14, 2023
19. 
    sipa commented at 3:13 PM on April 14, 2023: member

    Updated, and also improved the linter logic to decide which files to lint.

20. in test/lint/lint-python.py:20 in 621c17869d

      14 | @@ -15,7 +15,13 @@
      15 |  
      16 |  DEPS = ['flake8', 'mypy', 'pyzmq']
      17 |  MYPY_CACHE_DIR = f"{os.getenv('BASE_ROOT_DIR', '')}/test/.mypy_cache"
      18 | -FILES_ARGS = ['git', 'ls-files', 'test/functional/*.py', 'contrib/devtools/*.py']
      19 | +
      20 | +# All .py files, except those in src/ (to exclude subtrees there)
      21 | +FLAKE_FILES_ARGS = ['git', 'ls-files', '*.py', ':!:src/*.py']
    

    ---

    ---

    

    sipa commented at 3:15 PM on April 14, 2023:

    Note for reviewers: this should just be excluding the following files:

    • src/crc32c/.ycm_extra_conf.py
    • src/minisketch/tests/pyminisketch.py
    • src/secp256k1/tools/tests_wycheproof_generate.py

    I believe that's the correct course of action, as even if the projects supplying those files would want to comply with Bitcoin Core's linting standards, it shouldn't be up to Bitcoin Core to enforce that.

21. real-or-random approved
22. 
    real-or-random commented at 3:42 PM on April 14, 2023: contributor

    utACK 621c17869d3754559c03e4f2bee73885659e0c68 subtree matches. diff to linter script looks good

23. 
    fanquake commented at 3:46 PM on April 14, 2023: member

    cc @jonasnick too

24. 
    fanquake commented at 11:22 AM on April 15, 2023: member

    Guix Build:

    a750ae4390cbc5386cd263097da9470c1437d798379fe3f5f10a5086e48fc6fb  guix-build-621c17869d37/output/aarch64-linux-gnu/SHA256SUMS.part
    1458929f13ef9d52ebca28948d83f7e1ed9bd2e992fe0334482cdaf3fa32aab6  guix-build-621c17869d37/output/aarch64-linux-gnu/bitcoin-621c17869d37-aarch64-linux-gnu-debug.tar.gz
    74458df1b50213bff9231ed87f0d80f7fe2ab2e6b83d1a959a28b7f1297339b4  guix-build-621c17869d37/output/aarch64-linux-gnu/bitcoin-621c17869d37-aarch64-linux-gnu.tar.gz
    1ed57572087d7b09003aa1fc9c541c0d637be517622a03c69531c82bf9218196  guix-build-621c17869d37/output/arm-linux-gnueabihf/SHA256SUMS.part
    5b535acac7fb477e3891963d08d8eb7c53a613ea9673c8e94f9846a06502b455  guix-build-621c17869d37/output/arm-linux-gnueabihf/bitcoin-621c17869d37-arm-linux-gnueabihf-debug.tar.gz
    e6096969efb61d0777c41f65ca82e0fbddfdbad0a2021e337838d13ee1bb0ced  guix-build-621c17869d37/output/arm-linux-gnueabihf/bitcoin-621c17869d37-arm-linux-gnueabihf.tar.gz
    0dd1cad5f18a053f7db1e46ed74274d58eb5a06031096823847737ea6225c040  guix-build-621c17869d37/output/arm64-apple-darwin/SHA256SUMS.part
    c44f98a1ad6c866d5d20ebbdc0a827c532212740c210a68ba6ffbbfe03d05ff2  guix-build-621c17869d37/output/arm64-apple-darwin/bitcoin-621c17869d37-arm64-apple-darwin-unsigned.dmg
    d4cbf66fb6950570e52edfc6df2b80284e99dcc49e43c1b14cbbb0c3d75f1512  guix-build-621c17869d37/output/arm64-apple-darwin/bitcoin-621c17869d37-arm64-apple-darwin-unsigned.tar.gz
    133bef091207b07b724aa3c28b4b470823fd005c84c5bb27ebda0cab8c79f23f  guix-build-621c17869d37/output/arm64-apple-darwin/bitcoin-621c17869d37-arm64-apple-darwin.tar.gz
    13607ade159c952f2731e283c9a6c1761604a85ac25c81c2f3329d162e846591  guix-build-621c17869d37/output/dist-archive/bitcoin-621c17869d37.tar.gz
    546ba0fab2be1793199ba2421fdcb01c046830d6a2ff015294491c87697cd566  guix-build-621c17869d37/output/powerpc64-linux-gnu/SHA256SUMS.part
    88bbf9156c4000a087f4e692338195255c5a707a04030a2aee2542083fd819db  guix-build-621c17869d37/output/powerpc64-linux-gnu/bitcoin-621c17869d37-powerpc64-linux-gnu-debug.tar.gz
    631f01957a64ca3d690f996a956453e324aa847ce15c53b6067ed68fc81e8131  guix-build-621c17869d37/output/powerpc64-linux-gnu/bitcoin-621c17869d37-powerpc64-linux-gnu.tar.gz
    d82f2c33c2852efd77b35df821a41dc2c7125ad1e3b5a5c7b3d5407bf6de50fc  guix-build-621c17869d37/output/powerpc64le-linux-gnu/SHA256SUMS.part
    c5d9b108f93c5551acd607e0f12c2b9ec9e4ed6aad71ac7d325497d6e1495118  guix-build-621c17869d37/output/powerpc64le-linux-gnu/bitcoin-621c17869d37-powerpc64le-linux-gnu-debug.tar.gz
    4d0b503f9b7db8d7342d3b69b4bff27d151674db7ac697796afab22e33699ba3  guix-build-621c17869d37/output/powerpc64le-linux-gnu/bitcoin-621c17869d37-powerpc64le-linux-gnu.tar.gz
    7380f0a47d68d6c9bdeb61ac60724eb178083828a8d08e945f737165f2c26ac8  guix-build-621c17869d37/output/riscv64-linux-gnu/SHA256SUMS.part
    6c3cc75978ed504b839a471334f43b116e040af6096b9c1f89c4318afe7f0c5f  guix-build-621c17869d37/output/riscv64-linux-gnu/bitcoin-621c17869d37-riscv64-linux-gnu-debug.tar.gz
    832744857f34affaf78cb823155b9cdded71be3796fa53143ddaca9cae85c2c6  guix-build-621c17869d37/output/riscv64-linux-gnu/bitcoin-621c17869d37-riscv64-linux-gnu.tar.gz
    9897e69fa8aed38f49550b6015711968be12826590f5289c85daeb25f526c09a  guix-build-621c17869d37/output/x86_64-apple-darwin/SHA256SUMS.part
    9dee78bd51b4cc1b3b3cb17f5bebd6f8ffc2d51f6f88e3cde989c14eb5a20da9  guix-build-621c17869d37/output/x86_64-apple-darwin/bitcoin-621c17869d37-x86_64-apple-darwin-unsigned.dmg
    a613b69fe28596f8bf53197c4dd9abfd28967ad2e7d409ca5da151d008504675  guix-build-621c17869d37/output/x86_64-apple-darwin/bitcoin-621c17869d37-x86_64-apple-darwin-unsigned.tar.gz
    f94ef742c25463270aaaf2715ce6cd8fa11309c8e0114cd5cbcf275c95687f57  guix-build-621c17869d37/output/x86_64-apple-darwin/bitcoin-621c17869d37-x86_64-apple-darwin.tar.gz
    954163b38343400282d61e2ceaf6ef0802fbab2e0671a898a5bbd21401519c35  guix-build-621c17869d37/output/x86_64-linux-gnu/SHA256SUMS.part
    9c6653a0a4d1fad8acdfc21a52f82f00ba320a9b508a15e52319722990c958c3  guix-build-621c17869d37/output/x86_64-linux-gnu/bitcoin-621c17869d37-x86_64-linux-gnu-debug.tar.gz
    08a5d67947eece9af4c4e1aef4089474e6f3c94169e895b52b0ec2e2c9c09e91  guix-build-621c17869d37/output/x86_64-linux-gnu/bitcoin-621c17869d37-x86_64-linux-gnu.tar.gz
    bce081ba263d970d968c74bd6e87e2ba46a08a3624fbc3c1331580343af7fed7  guix-build-621c17869d37/output/x86_64-w64-mingw32/SHA256SUMS.part
    a38af023d3b6682db933910f535136a5ddab9d7ced513382b239423a24132791  guix-build-621c17869d37/output/x86_64-w64-mingw32/bitcoin-621c17869d37-win64-debug.zip
    3b2dcc19bb75ff990eab621707ecd02e7145b709e47d57805df4e0a4ee8035a2  guix-build-621c17869d37/output/x86_64-w64-mingw32/bitcoin-621c17869d37-win64-setup-unsigned.exe
    00a133efd3c588e66271d67b842b3aa38eb59882c166fca1610a638dce6e9758  guix-build-621c17869d37/output/x86_64-w64-mingw32/bitcoin-621c17869d37-win64-unsigned.tar.gz
    b0a8ede2f34d5d45a63e942e4322b594413fb7989789cdb6b93c3efd7e9f491c  guix-build-621c17869d37/output/x86_64-w64-mingw32/bitcoin-621c17869d37-win64.zip
    

25. fanquake approved
26. 
    fanquake commented at 11:27 AM on April 15, 2023: member

    ACK 621c17869d3754559c03e4f2bee73885659e0c68

27. fanquake merged this on Apr 15, 2023
28. fanquake closed this on Apr 15, 2023

    ---

29. sidhujag referenced this in commit da118866e6 on Apr 17, 2023
30. real-or-random referenced this in commit c0e8c08e23 on Apr 19, 2023
31. real-or-random referenced this in commit 2dd672a287 on Apr 19, 2023
32. real-or-random referenced this in commit 2418d3260a on Apr 25, 2023
33. dderjoel referenced this in commit 81148f6179 on May 23, 2023
34. dderjoel referenced this in commit 190f5745e2 on May 23, 2023
35. kwvg referenced this in commit fb4b28afc7 on Sep 8, 2023
36. kwvg referenced this in commit 4c0d2eb0be on Sep 9, 2023
37. kwvg referenced this in commit 9b6c71e810 on Sep 9, 2023
38. kwvg referenced this in commit 440f93ed66 on Sep 24, 2023
39. kwvg referenced this in commit 9bf3828825 on Sep 28, 2023
40. kwvg referenced this in commit 9a184a79f9 on Sep 28, 2023
41. ogabrielides referenced this in commit 4a60fda3d0 on Nov 15, 2023
42. UdjinM6 referenced this in commit 162fa6eb0d on Nov 15, 2023
43. UdjinM6 referenced this in commit 9cc992b13a on Nov 16, 2023
44. UdjinM6 referenced this in commit 72a4c41eb4 on Nov 20, 2023
45. PastaPastaPasta referenced this in commit 4ac25312cc on Nov 21, 2023
46. PastaPastaPasta referenced this in commit 39ce20cddc on Nov 21, 2023
47. bitcoin locked this on Apr 14, 2024

Milestone
25.0