20b05c9d3f configure: Show exhaustive tests in summary (Tim Ruffing)
Pull request description:
ACKs for top commit:
hebasto:
ACK 20b05c9d3f, it aligns now with the CMake script: e56716a3bc/CMakeLists.txt (L348-L350)
sipa:
utACK 20b05c9d3f
jonasnick:
ACK 20b05c9d3f
Tree-SHA512: 30744ea5e5b7441ad252868c83cebfec2b02b75786b9ea55d4b0b0a696f1c7df39c48c243b29b13839a9f3a7757c192da8be0dd95412678a7583b123db6e99ac
1b6e081538 include: remove WARN_UNUSED_RESULT for functions always returning 1 (Jonas Nick)
Pull request description:
This makes the usage of the atribute consistent. In the musig and ellswift module, functions that return 1 always already don't have the WARN_UNUSED_RESULT attribute. In secp256k1.h and the extrakeys module, this has only been the case partially.
In all cases where this was removed, the function only returns 0 if the illegal callback has been called.
Fixes#1379
ACKs for top commit:
real-or-random:
utACK 1b6e081538
sipa:
utACK 1b6e081538
Tree-SHA512: 5d1f2563ddde34fb721dd0b96622e0888a9c72f95af6f1c8a94f7f1f72ca934b6af98a3357c1e922d8611a9869264bb0f3ceb7bed0164c6c3a6f92f9950d4f19
d87c3bc58f ci: Fix exiting from ci.sh on error (Tim Ruffing)
Pull request description:
Fixes the following bash error when make fails:
./ci/ci.sh: line 100: return: can only `return' from a function or
sourced script
ACKs for top commit:
hebasto:
ACK d87c3bc58f
Tree-SHA512: 5ecd0f550f7659cc41b403fdb7d5d3d37d1a167d585cca02b0aca209c8b9592bb3067cf11aeb80775666e7232f31bf05cf1bb97fec8c67f3bc5fe2243ddbbcfa
This makes the usage of the atribute consistent. In the musig and ellswift
module, functions that return 1 always already don't have the WARN_UNUSED_RESULT
attribute. In secp256k1.h and the extrakeys module, this has only been the case
partially.
In all cases where this was removed, the function only returns 0 if the illegal
callback has been called.
59860bcc24 gha: Print all *.log files, in a separate action (Tim Ruffing)
Pull request description:
Before this commit, we didn't print *_example.log files and
test_suite.log.
Printing is now handled in a separate action, which avoids code
duplication and makes the ci.yml file more readable. This changes the
folding/grouping of the log output in the GitHub Actions CI, but I
think the new variant is as good as the old one.
Furthermore, the condition for printing the logs is changed from
"always()" to "!cancelled()". This ensures that logs will still be
printed if previous steps such as the CI script failed, but that they
won't be printed if the entire run is cancelled (e.g., by clicking a
button in the UI or through a force-push to the PR). This is in line
with a recommendation in the GHA docs:
https://docs.github.com/en/actions/writing-workflows/choosing-what-your-workflow-does/evaluate-expressions-in-workflows-and-actions#always
ACKs for top commit:
hebasto:
ACK 59860bcc24.
sipa:
ACK 59860bcc24
Tree-SHA512: ca11f5e5f01660964276b9c2e11c22caeed8492e9c7ffaa2078aaaa733005c63242fc93a1056124fb8f1f83019d46818c12b10142fb10f43270a8562fd10885a
Before this commit, we didn't print *_example.log files and
test_suite.log.
Printing is now handled in a separate action, which avoids code
duplication and makes the ci.yml file more readable. This changes the
folding/grouping of the log output in the GitHub Actions CI, but I
think the new variant is as good as the old one.
Furthermore, the condition for printing the logs is changed from
"always()" to "!cancelled()". This ensures that logs will still be
printed if previous steps such as the CI script failed, but that they
won't be printed if the entire run is cancelled (e.g., by clicking a
button in the UI or through a force-push to the PR). This is in line
with a recommendation in the GHA docs:
https://docs.github.com/en/actions/writing-workflows/choosing-what-your-workflow-does/evaluate-expressions-in-workflows-and-actions#always
4c50d73dd9 ci: Add new "Windows (clang-cl)" job (Hennadii Stepanov)
84c0bd1f72 cmake: Adjust diagnostic flags for clang-cl (Hennadii Stepanov)
Pull request description:
When building with `clang-cl` on Windows, the output is cluttered with warning messages because compiler diagnostic flags are not applied correctly:
```
> cmake -B build -G Ninja -DCMAKE_C_COMPILER="C:\Users\hebasto\Downloads\clang+llvm-18.1.8-x86_64-pc-windows-msvc\bin\clang-cl.exe"
> cmake --build build
[1/16] Building C object src\CMakeFiles\bench.dir\bench.c.obj
In file included from C:\Users\hebasto\secp256k1\src\bench.c:11:
C:\Users\hebasto\secp256k1\src\util.h(34,13): warning: unused function 'print_buf_plain' [-Wunused-function]
34 | static void print_buf_plain(const unsigned char *buf, size_t len) {
| ^~~~~~~~~~~~~~~
1 warning generated.
[2/16] Building C object src\CMakeFiles\secp256k1_precomputed.dir\precomputed_ecmult_gen.c.obj
In file included from C:\Users\hebasto\secp256k1\src\precomputed_ecmult_gen.c:3:
In file included from C:\Users\hebasto\secp256k1\src\group.h:10:
In file included from C:\Users\hebasto\secp256k1\src\field.h:10:
C:\Users\hebasto\secp256k1\src\util.h(34,13): warning: unused function 'print_buf_plain' [-Wunused-function]
34 | static void print_buf_plain(const unsigned char *buf, size_t len) {
| ^~~~~~~~~~~~~~~
```
This PR resolves this issue.
---
**Additional note for reviewers:** The VS builtin clang can also be used assuming that the following VS components are installed:
The user can generate a build system on Windows as follows:
- Using the default "Visual Studio" generator:
```
cmake -B build -T ClangCL
```
- Using the "Ninja" generator:
```
cmake -B build -G Ninja -DCMAKE_C_COMPILER=clang-cl
```
---
Required for downstream projects which aim to build with `clang-cl` (see https://github.com/bitcoin/bitcoin/issues/31456).
ACKs for top commit:
real-or-random:
utACK 4c50d73dd9
Tree-SHA512: 439eb53afd7be65d538cd569f3d095f58325bd26ffc5014ca5f94320689a45b20c9a5a963170578214a20fd3233ec15ef6ab75ab96ce3a4314c282b1b6229ca1
These function aliases have been described as DEPRECATED in the public
API docs already many years ago (see #701, commit 41fc7856), and in
addition explicit deprecation warnings are shown by the compiler at
least since the first official release 0.2.0 (see PR #1089, commit
fc94a2da), so it should be fine to just remove them by now.
Co-authored-by: Tim Ruffing <crypto@timruffing.de>
1823594761 Verify `compressed` argument in `secp256k1_eckey_pubkey_serialize` (Sebastian Falbesoner)
Pull request description:
Due to similarity to the public API function `secp256k1_ec_pubkey_serialize`, public API flags like `SECP256K1_EC_COMPRESSED` are sometimes mistakingly passed to `secp256k1_eckey_pubkey_serialize` in newly proposed code (this is currently the case for several modules in secp256k1-zkp, see https://github.com/BlockstreamResearch/secp256k1-zkp/pull/300), which is currently not detected. To avoid this in the future, a VERIFY_CHECK is added to check that the `compressed` argument is either 0 or 1.
ACKs for top commit:
real-or-random:
utACK 1823594761
stratospher:
ACK 1823594. Got tests failures when passing public API flags to `secp256k1_eckey_pubkey_serialize`.
Tree-SHA512: ca542afc87f33e436ba33dc55b285dfe3759007c446ef94503bc1044c7a0a7f7b2208ae82e2c9743fc5fa38cf386127f3fbfa02d2c242f28fab3041ee46f153b
13d389629a CONTRIBUTING: mention that `EXIT_` codes should be used (Sebastian Falbesoner)
c855581728 test, bench, precompute_ecmult: use `EXIT_...` constants for `main` return values (Sebastian Falbesoner)
965393fcea examples: use `EXIT_...` constants for `main` return values (Sebastian Falbesoner)
Pull request description:
This simple PR addresses #1609 for all example and internal binaries. Alternative to #1618, which is stale (the author confirmed to me that they are not working on that PR anymore). The last commits adds a suggestion to CONTRIBUTING.md, not sure though if we want to go that far.
ACKs for top commit:
jonasnick:
ACK 13d389629a
real-or-random:
utACK 13d389629a
Tree-SHA512: 513eba4b712ba3d5f23a5fdc51cb27c5347b29bcaba39501345913c220be6f093a41186911032d2ddc898b848de84f05f374b3554ffcf92610728b2a23c0bb36
2ac9f558c4 doc: Improve cmake instructions in README (Fabian Jahr)
Pull request description:
Minor improvement suggestion for the readme. I find this alternative way of using cmake a bit more comfortable because I don't like to change the directory.
It's just a suggestion based on personal preference, if this is too minor of an improvement feel free to close.
ACKs for top commit:
hebasto:
ACK 2ac9f558c4.
real-or-random:
utACK 2ac9f558c4
Tree-SHA512: 5f7bc8b5ff91fb7a115a0e57224c66b018cfc824784e0def1064d07f9be66efe55e1a71e034f6a3d6489e063995c1ae17a9e91c990a0944d600cc957c038909d
Due to similarity to the public API function `secp256k1_ec_pubkey_serialize`,
public API flags like `SECP256K1_EC_COMPRESSED` are sometimes mistakingly
passed to newly proposed code (this is currently the case for several modules in
secp256k1-zkp, see https://github.com/BlockstreamResearch/secp256k1-zkp/pull/300).
which is currently not detected. To avoid this in the future, a VERIFY_CHECK
is added to check that the `compressed` argument is either 0 or 1.
39705450eb Fix some misspellings (Nicolas Iooss)
Pull request description:
Hello,
Some files contained English misspellings or math issues (`lamba` instead of `lambda`), mainly in comments. Fixing them helps readability.
By the way, the misspellings found in the Wycheproof test vector file were also reported upstream: https://github.com/C2SP/wycheproof/issues/124
ACKs for top commit:
real-or-random:
utACK 39705450eb
Tree-SHA512: 36327e8bb58ef3c0408cf4966bb33f51c84b1614809d8711d86eaf3d4e5336ae8c663593cb5f0e9c56adbb2d7f2ca62a9b84cae1b76b9811c110f87f1defa624
