subtree: update libsecp256k1 to latest master #28093

fanquake commented at 4:50 PM on July 17, 2023: member

Includes https://github.com/bitcoin-core/secp256k1/pull/1378. Which fixes #28079. Adapts Windows build for https://github.com/bitcoin-core/secp256k1/pull/1367.

DrahtBot commented at 4:50 PM on July 17, 2023: contributor

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

Reviews

See the guideline for information on the review process.

Type	Reviewers
ACK	hebasto, jonasnick
Concept ACK	real-or-random, jonatack

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

fanquake commented at 4:51 PM on July 17, 2023: member

cc @real-or-random @jonasnick

fanquake commented at 4:57 PM on July 17, 2023: member

Looks like we'll need to adapt to Windows DLL/symbol export related changes. Will fixup.

real-or-random commented at 5:14 PM on July 17, 2023: contributor

I believe that this failure is pretty sporadic. So, it may not be worth updating the subtree.

There's also still (sorry!) the sporadic test condition failed: ((~x[m][shift]) << (64 - (1 << usebits))) == 0 failure (https://github.com/bitcoin-core/secp256k1/pull/367, https://github.com/bitcoin-core/secp256k1/issues/610). That one will be solved by https://github.com/bitcoin-core/secp256k1/pull/1298 by @sipa. Perhaps we should prioritize this PR and wait for its merge first.

real-or-random commented at 5:15 PM on July 17, 2023: contributor

Concept ACK (meaning there's nothing wrong with updating the subtree right now)

hebasto commented at 6:51 PM on July 17, 2023: member

Looks like we'll need to adapt to Windows DLL/symbol export related changes. Will fixup.

For MSVC build suggesting the diff as follows:

--- a/build_msvc/common.init.vcxproj.in
+++ b/build_msvc/common.init.vcxproj.in
@@ -90,7 +90,7 @@
       <AdditionalOptions>/utf-8 /Zc:__cplusplus /std:c++20 %(AdditionalOptions)</AdditionalOptions>
       <DisableSpecificWarnings>4018;4244;4267;4715;4805</DisableSpecificWarnings>
       <TreatWarningAsError>true</TreatWarningAsError>
-      <PreprocessorDefinitions>_SILENCE_CXX17_CODECVT_HEADER_DEPRECATION_WARNING;ZMQ_STATIC;NOMINMAX;WIN32;HAVE_CONFIG_H;_CRT_SECURE_NO_WARNINGS;_SCL_SECURE_NO_WARNINGS;_CONSOLE;_WIN32_WINNT=0x0601;_WIN32_IE=0x0501;WIN32_LEAN_AND_MEAN;%(PreprocessorDefinitions)</PreprocessorDefinitions>
+      <PreprocessorDefinitions>_SILENCE_CXX17_CODECVT_HEADER_DEPRECATION_WARNING;SECP256K1_STATIC;ZMQ_STATIC;NOMINMAX;WIN32;HAVE_CONFIG_H;_CRT_SECURE_NO_WARNINGS;_SCL_SECURE_NO_WARNINGS;_CONSOLE;_WIN32_WINNT=0x0601;_WIN32_IE=0x0501;WIN32_LEAN_AND_MEAN;%(PreprocessorDefinitions)</PreprocessorDefinitions>
       <AdditionalIncludeDirectories>..\..\src;..\..\src\minisketch\include;..\..\src\univalue\include;..\..\src\secp256k1\include;..\..\src\leveldb\include;..\..\src\leveldb\helpers\memenv;%(AdditionalIncludeDirectories)</AdditionalIncludeDirectories>
     </ClCompile>
     <Link>

hebasto commented at 6:52 PM on July 17, 2023: member

Concept ACK.

DrahtBot added the label CI failed on Jul 17, 2023

jonatack commented at 1:02 AM on July 18, 2023: contributor

Concept ACK

fanquake commented at 9:42 AM on July 18, 2023: member

Added a commit to fix the Windows builds.

DrahtBot removed the label CI failed on Jul 18, 2023

jonasnick commented at 2:07 PM on July 18, 2023: contributor

There's also still (sorry!) the sporadic test condition failed: ((~x[m][shift]) << (64 - (1 << usebits))) == 0 failure (https://github.com/bitcoin-core/secp256k1/pull/367, https://github.com/bitcoin-core/secp256k1/issues/610). That one will be solved by https://github.com/bitcoin-core/secp256k1/pull/1298 by @sipa. Perhaps we should prioritize this PR and wait for its merge first.

This is fixed in libsecp256k1 master now.

Squashed 'src/secp256k1/' changes from 705ce7ed8c..c545fdc374

c545fdc374 Merge bitcoin-core/secp256k1#1298: Remove randomness tests
b40e2d30b7 Merge bitcoin-core/secp256k1#1378: ellswift: fix probabilistic test failure when swapping sides
c424e2fb43 ellswift: fix probabilistic test failure when swapping sides
907a67212e Merge bitcoin-core/secp256k1#1313: ci: Test on development snapshots of GCC and Clang
0f7657d59c Merge bitcoin-core/secp256k1#1366: field: Use `restrict` consistently in fe_sqrt
cc55757552 Merge bitcoin-core/secp256k1#1340: clean up in-comment Sage code (refer to secp256k1_params.sage, update to Python3)
600c5adcd5 clean up in-comment Sage code (refer to secp256k1_params.sage, update to Python3)
981e5be38c ci: Fix typo in comment
e9e9648219 ci: Reduce number of macOS tasks from 28 to 8
609093b387 ci: Add x86_64 Linux tasks for gcc and clang snapshots
1deecaaf3b ci: Install development snapshots of gcc and clang
b79ba8aa4c field: Use `restrict` consistently in fe_sqrt
c9ebca95f9 Merge bitcoin-core/secp256k1#1363: doc: minor ellswift.md updates
afd7eb4a55 Merge bitcoin-core/secp256k1#1371: Add exhaustive tests for ellswift (with create+decode roundtrip)
2792119278 Add exhaustive test for ellswift (create+decode roundtrip)
c7d900ffd1 doc: minor ellswift.md updates
332af315fc Merge bitcoin-core/secp256k1#1344: group: save normalize_weak calls in `secp256k1_ge_is_valid_var`/`secp256k1_gej_eq_x_var`
9e6d1b0e9b Merge bitcoin-core/secp256k1#1367: build: Improvements to symbol visibility logic on Windows (attempt 3)
0aacf64352 Merge bitcoin-core/secp256k1#1370: Corrected some typos
b6b9834e8d small fixes
07c0e8b82e group: remove unneeded normalize_weak in `secp256k1_gej_eq_x_var`
3fc1de5c55 Merge bitcoin-core/secp256k1#1364: Avoid `-Wmaybe-uninitialized` when compiling with `gcc -O1`
fb758fe8d6 Merge bitcoin-core/secp256k1#1323: tweak_add: fix API doc for tweak=0
c6cd2b15a0 ci: Add task for static library on Windows + CMake
020bf69a44 build: Add extensive docs on visibility issues
0196e8ade1 build: Introduce `SECP256k1_DLL_EXPORT` macro
9f1b1904a3 refactor: Replace `SECP256K1_API_VAR` with `SECP256K1_API`
ae9db95cea build: Introduce `SECP256K1_STATIC` macro for Windows users
7966aee31d Merge bitcoin-core/secp256k1#1369: ci: Print commit in Windows container
a7bec34231 ci: Print commit in Windows container
249c81eaa3 Merge bitcoin-core/secp256k1#1368: ci: Drop manual checkout of merge commit
98579e297b ci: Drop manual checkout of merge commit
5b9f37f136 ci: Add `CFLAGS: -O1` to task matrix
a6ca76cdf2 Avoid `-Wmaybe-uninitialized` when compiling with `gcc -O1`
0fa84f869d Merge bitcoin-core/secp256k1#1358: tests: introduce helper for non-zero `random_fe_test()` results
5a95a268b9 tests: introduce helper for non-zero `random_fe_test` results
304421d57b tests: refactor: remove duplicate function `random_field_element_test`
3aef6ab8e1 Merge bitcoin-core/secp256k1#1345: field: Static-assert that int args affecting magnitude are constant
4494a369b6 Merge bitcoin-core/secp256k1#1357: tests: refactor: take use of `secp256k1_ge_x_on_curve_var`
799f4eec27 Merge bitcoin-core/secp256k1#1356: ci: Adjust Docker image to Debian 12 "bookworm"
c862a9fb49 ci: Adjust Docker image to Debian 12 "bookworm"
a1782098a9 ci: Force DWARF v4 for Clang when Valgrind tests are expected
7d8d5c86df tests: refactor: take use of `secp256k1_ge_x_on_curve_var`
8a7273465b Help the compiler prove that a loop is entered
fd491ea1bb Merge bitcoin-core/secp256k1#1355: Fix a typo in the error message
ac43613d25 Merge bitcoin-core/secp256k1#1354: Add ellswift to CHANGELOG
67887ae65c Fix a typo in the error message
926dd3e962 Merge bitcoin-core/secp256k1#1295: abi: Use dllexport for mingw builds
10836832e7 Merge bitcoin-core/secp256k1#1336: Use `__shiftright128` intrinsic in `secp256k1_u128_rshift` on MSVC
7c7467ab7f Refer to ellswift.md in API docs
c32ffd8d8c Add ellswift to CHANGELOG
3c1a0fd37f Merge bitcoin-core/secp256k1#1347: field: Document return value of fe_sqrt()
5779137457 field: Document return value of fe_sqrt()
be8ff3a02a field: Static-assert that int args affecting magnitude are constant
efa76c4bf7 group: remove unneeded normalize_weak in `secp256k1_ge_is_valid_var`
5b7bf2e9d4 Use `__shiftright128` intrinsic in `secp256k1_u128_rshift` on MSVC
05873bb6b1 tweak_add: fix API doc for tweak=0
6ec3731e8c Simplify test PRNG implementation
fb5bfa4eed Add static test vector for Xoshiro256++
723e8ca8f7 Remove randomness tests
bc7c8db179 abi: Use dllexport for mingw builds

git-subtree-dir: src/secp256k1
git-subtree-split: c545fdc374964424683d9dac31a828adedabe860

ff061fde18

Update secp256k1 subtree to latest upstream master 8ee662984f

build: adapt Windows builds for libsecp256k1 build changes

See https://github.com/bitcoin-core/secp256k1/pull/1367.

5080c9c25f

fanquake force-pushed on Jul 18, 2023

fanquake commented at 2:27 PM on July 18, 2023: member

This is fixed in libsecp256k1 master now.

Thanks. Updated to pull that change in here as well.

hebasto approved

hebasto commented at 2:34 PM on July 18, 2023: member

ACK 5080c9c25f44ae9d16a69d41f0da1d1e06483bf7, I've made the src/secp256k1 subtree update locally and got zero diff with this PR branch.

jonasnick commented at 9:07 AM on July 19, 2023: contributor

ACK 5080c9c25f44ae9d16a69d41f0da1d1e06483bf7

fanquake merged this on Jul 19, 2023

fanquake closed this on Jul 19, 2023

fanquake deleted the branch on Jul 19, 2023

sidhujag referenced this in commit 381fa93615 on Jul 19, 2023

bitcoin locked this on Jul 18, 2024