CI / x86_64: Linux (Debian stable) (clang, map[env_vars:map[CFLAGS:-O1 ECDH:yes ELLSWIFT:yes EXTRAKEYS:yes MUSIG:yes RECOVERY:yes SCHNORRSIG:yes]]) (push) Has been cancelled
CI / x86_64: Linux (Debian stable) (clang, map[env_vars:map[CPPFLAGS:-DVERIFY CTIMETESTS:no ECDH:yes EXTRAKEYS:yes MUSIG:yes RECOVERY:yes SCHNORRSIG:yes]]) (push) Has been cancelled
CI / x86_64: Linux (Debian stable) (clang, map[env_vars:map[ECDH:yes ELLSWIFT:yes EXTRAKEYS:yes MUSIG:yes SCHNORRSIG:yes WIDEMUL:int64]]) (push) Has been cancelled
CI / x86_64: Linux (Debian stable) (clang, map[env_vars:map[ECDH:yes EXTRAKEYS:yes MUSIG:yes SCHNORRSIG:yes WIDEMUL:int128]]) (push) Has been cancelled
CI / x86_64: Linux (Debian stable) (clang, map[env_vars:map[ELLSWIFT:yes EXTRAKEYS:yes MUSIG:yes RECOVERY:yes SCHNORRSIG:yes WIDEMUL:int128]]) (push) Has been cancelled
CI / x86_64: Linux (Debian stable) (clang-snapshot, map[env_vars:map[BENCH:no BUILD:distcheck CTIMETESTS:no WITH_VALGRIND:no]]) (push) Has been cancelled
CI / x86_64: Linux (Debian stable) (clang-snapshot, map[env_vars:map[CFLAGS:-O1 ECDH:yes ELLSWIFT:yes EXTRAKEYS:yes MUSIG:yes RECOVERY:yes SCHNORRSIG:yes]]) (push) Has been cancelled
CI / x86_64: Linux (Debian stable) (clang-snapshot, map[env_vars:map[CPPFLAGS:-DVERIFY CTIMETESTS:no ECDH:yes EXTRAKEYS:yes MUSIG:yes RECOVERY:yes SCHNORRSIG:yes]]) (push) Has been cancelled
CI / x86_64: Linux (Debian stable) (clang-snapshot, map[env_vars:map[ECDH:yes ELLSWIFT:yes EXTRAKEYS:yes MUSIG:yes SCHNORRSIG:yes WIDEMUL:int64]]) (push) Has been cancelled
CI / x86_64: Linux (Debian stable) (clang-snapshot, map[env_vars:map[ECDH:yes EXTRAKEYS:yes MUSIG:yes SCHNORRSIG:yes WIDEMUL:int128]]) (push) Has been cancelled
CI / x86_64: Linux (Debian stable) (clang-snapshot, map[env_vars:map[ELLSWIFT:yes EXTRAKEYS:yes MUSIG:yes RECOVERY:yes SCHNORRSIG:yes WIDEMUL:int128]]) (push) Has been cancelled
CI / x86_64: Linux (Debian stable) (gcc, map[env_vars:map[CFLAGS:-O1 ECDH:yes ELLSWIFT:yes EXTRAKEYS:yes MUSIG:yes RECOVERY:yes SCHNORRSIG:yes]]) (push) Has been cancelled
CI / x86_64: Linux (Debian stable) (gcc, map[env_vars:map[CPPFLAGS:-DVERIFY CTIMETESTS:no ECDH:yes EXTRAKEYS:yes MUSIG:yes RECOVERY:yes SCHNORRSIG:yes]]) (push) Has been cancelled
CI / x86_64: Linux (Debian stable) (gcc, map[env_vars:map[ECDH:yes ELLSWIFT:yes EXTRAKEYS:yes MUSIG:yes SCHNORRSIG:yes WIDEMUL:int64]]) (push) Has been cancelled
CI / x86_64: Linux (Debian stable) (gcc, map[env_vars:map[ELLSWIFT:yes EXTRAKEYS:yes MUSIG:yes RECOVERY:yes SCHNORRSIG:yes WIDEMUL:int128]]) (push) Has been cancelled
CI / x86_64: Linux (Debian stable) (gcc-snapshot, map[env_vars:map[BENCH:no BUILD:distcheck CTIMETESTS:no WITH_VALGRIND:no]]) (push) Has been cancelled
CI / x86_64: Linux (Debian stable) (gcc-snapshot, map[env_vars:map[CFLAGS:-O1 ECDH:yes ELLSWIFT:yes EXTRAKEYS:yes MUSIG:yes RECOVERY:yes SCHNORRSIG:yes]]) (push) Has been cancelled
CI / x86_64: Linux (Debian stable) (gcc-snapshot, map[env_vars:map[CPPFLAGS:-DVERIFY CTIMETESTS:no ECDH:yes EXTRAKEYS:yes MUSIG:yes RECOVERY:yes SCHNORRSIG:yes]]) (push) Has been cancelled
CI / x86_64: Linux (Debian stable) (gcc-snapshot, map[env_vars:map[ECDH:yes ELLSWIFT:yes EXTRAKEYS:yes MUSIG:yes SCHNORRSIG:yes WIDEMUL:int64]]) (push) Has been cancelled
CI / x86_64: Linux (Debian stable) (gcc-snapshot, map[env_vars:map[ECDH:yes EXTRAKEYS:yes MUSIG:yes SCHNORRSIG:yes WIDEMUL:int128]]) (push) Has been cancelled
CI / x86_64: Linux (Debian stable) (gcc-snapshot, map[env_vars:map[ELLSWIFT:yes EXTRAKEYS:yes MUSIG:yes RECOVERY:yes SCHNORRSIG:yes WIDEMUL:int128]]) (push) Has been cancelled
CI / i686: Linux (Debian stable) (clang --target=i686-pc-linux-gnu -isystem /usr/i686-linux-gnu/include, map[env_vars:map[]]) (push) Has been cancelled
CI / MSan (clang, map[env_vars:map[CFLAGS:-fsanitize=memory -fsanitize-recover=memory -fsanitize-memory-param-retval -g CTIMETESTS:no]]) (push) Has been cancelled
CI / MSan (clang, map[env_vars:map[CFLAGS:-fsanitize=memory -fsanitize-recover=memory -g -O3 CTIMETESTS:yes ECMULTGENKB:2 ECMULTWINDOW:2]]) (push) Has been cancelled
CI / MSan (clang-snapshot, map[env_vars:map[CFLAGS:-fsanitize=memory -fsanitize-recover=memory -fsanitize-memory-param-retval -g CTIMETESTS:no]]) (push) Has been cancelled
CI / MSan (clang-snapshot, map[env_vars:map[CFLAGS:-fsanitize=memory -fsanitize-recover=memory -g -O3 CTIMETESTS:yes ECMULTGENKB:2 ECMULTWINDOW:2]]) (push) Has been cancelled
f5e815f430 remove secp256k1_eckey_pubkey_serialize function (Sebastian Falbesoner)
0d3659c547 use new `_eckey_pubkey_serialize{33,65}` functions in modules (ellswift,musig) (Sebastian Falbesoner)
adb76f82ea use new `_eckey_pubkey_serialize{33,65}` functions in public API (Sebastian Falbesoner)
fc7458ca3e introduce `secp256k1_eckey_pubkey_serialize{33,65}` functions (Sebastian Falbesoner)
Pull request description:
This PR splits up the pubkey serialization function `secp256k1_eckey_pubkey_serialize` into two variants for the compressed (33 bytes) and uncompressed (65 bytes) public key output format each, where only non-infinity group elements as input are allowed. The motivation is to simplify call-sites significantly, as they currently need to introduce two variables and a VERIFY_CHECKs on the return value and the in/out size parameter within a pre-processor block, typically leading to 8 lines of code. By using the new functions, the code is reduced to a single line of code that just calls the function (see #1773). This is helpful for already existing modules on master (ellswift, musig) and upcoming ones (silentpayments, see #1765).
One drawback is that the public API function `secp256k1_ec_pubkey_serialize` is now slightly more complex (we now call one of two functions instead of a single one, depending on whether the compressed flag is set or not), but that should hopefully not be a problem.
The commits are intentionally kept small to ease review, happy to squash them if that is preferred.
(Kudos to w0xlt for the initial idea (https://github.com/bitcoin-core/secp256k1/pull/1765#pullrequestreview-3462461331) and to real-or-random for the suggestion to split the already existing function (https://github.com/bitcoin-core/secp256k1/issues/1773#issuecomment-3540461718).)
ACKs for top commit:
real-or-random:
utACK f5e815f430
w0xlt:
ACK f5e815f430
Tree-SHA512: da576bbeae477f31ba76c0001f8df08b51fe5e31d67b422a238348ead3341bf37f0c1509ad9d0a93b63e6d61c152707c85beabd02f4eac3b3bdcff129e0ea750
26166c4f5f ecmult_multi: reduce strauss memory usage by 30% (Jonas Nick)
Pull request description:
This is a draft because I'm not sure about the cleanest way to implement it.
ACKs for top commit:
real-or-random:
ACK 26166c4f5f benchmarks show no significant difference (only tried low point counts)
siv2r:
tACK 26166c4
hebasto:
ACK 26166c4f5f, I have reviewed the code and it looks OK.
Tree-SHA512: f289daee0b0b51451331eefdd99200a78bd83539365d38465c038dc0e6ad940daf821119f7161b08a2390cf046e3859a8f950f2fe881a427aba16353031def7d
f252da7e6e ci: Use Python virtual environment in "x86_64-macos-native" job (Hennadii Stepanov)
Pull request description:
Fixes https://github.com/bitcoin-core/secp256k1/issues/1768.
This PR explicitly sets up a virtual environment instead of using `uv`, as suggested [here](https://github.com/bitcoin-core/secp256k1/issues/1768#issue-3599176194), for the following reasons:
1. It does not require granting a third-party action access to the repository.
2. This approach is already used in other parts of the CI framework (it’s unclear why it was missed in https://github.com/bitcoin-core/secp256k1/pull/1359 in the first place).
ACKs for top commit:
real-or-random:
ACK f252da7e6e
Tree-SHA512: df167db9b69ea4a055565c64ea0daa8c0d7b6a12510c35c22309f00a192a8cc4fbbdb0920bacd25547ea8c8f6a7853c2603be35234bd2bac88f610ea848f6120
153eea20c2 bench: Use `ALIGNMENT` macro instead of hardcoded value (Hennadii Stepanov)
Pull request description:
This PR brings consistency with the rest of the code and appears more correct.
ACKs for top commit:
real-or-random:
utACK 153eea20c2
Tree-SHA512: 0fc61a390205ef29b99cfce9cb0d365930b7a93aa0f2aaaa00bfd9e0fc34ac928a3fe13b096f394b7d35e95e9ede7c47ecb054de2846be1e1aa33a84d14942cf
c09519f0e3 ci: Drop workaround for Valgrind older than 3.20.0 (Hennadii Stepanov)
Pull request description:
This is no longer needed in the current CI framework.
If someone runs the CI scripts locally, it is reasonable to expect that they are using up-to-date tools, including Valgrind.
ACKs for top commit:
real-or-random:
ACK c09519f0e3
Tree-SHA512: ef840dc8fd6e29110e194e35139050d9720bf8b38e89497790dcf5f2f2ce603805b572fb7408b4ae47bda31289a05548671ac41923f6ab3d1fbf288855f058d0
8bc50b72ff ci: Switch to macOS 15 Sequoia Intel-based image (Hennadii Stepanov)
Pull request description:
This is an alternative to https://github.com/bitcoin-core/secp256k1/pull/1755.
More details from the GHA developers:
> Apple has discontinued support for the x86_64 (Intel) architecture going forward. GitHub will no longer support this architecture on macOS after the macOS 15 runner image is retired in Fall 2027.
ACKs for top commit:
real-or-random:
ACK 8bc50b72ff
Tree-SHA512: 2c3de907b8910193b77bb201e1c7aad3f70f89804efe4b3ead23181d45a912f677cd533e819914cf1b346aa01d2f273fd5f1e63d7b8d284320d1e4e6198d8578
2f4546ce56 test: add --log option to display tests execution (furszy)
95b9953ea4 test: Add option to display all available tests (furszy)
953f7b0088 test: support running specific tests/modules targets (furszy)
0302c1a3d7 test: add --help for command-line options (furszy)
9ec3bfe22d test: adapt modules to the new test infrastructure (furszy)
48789dafc2 test: introduce (mini) unit test framework (furszy)
9cce703863 refactor: move 'gettime_i64()' to tests_common.h (furszy)
Pull request description:
Early Note:
Don’t be scared by the PR’s line changes count — most of it’s just doc or part of the test framework API.
Context:
Currently, all tests run single-threaded sequentially and the library lacks the ability to specify which test (or group of tests) you would like to run. This is not only inconvenient as more tests are added but also time consuming during development and affects downstream projects that may want to parallelize the workload (such as Bitcoin-Core CI).
PR Goal:
Introduce a lightweight, extensible C89 unit test framework with no dynamic memory allocations, providing a structured way to register, execute, and report tests. The framework supports named command-line arguments in `-key=value` form, parallel test execution across multiple worker processes, granular test selection (selecting tests either by name or by module name), and time accumulation reports.
The introduced framework supports:
* `-help` or `-h`: display list of available commands along with their descriptions.
* `-jobs=<num>`: distribute tests across multiple worker processes (default: sequential if 0).
* `-target=<name>` or `-t=<name>`: run only specific tests by name; can be repeated to select multiple tests.
* `-target=<module name>`, `-t=<module>` Run all tests within a specific module (can be provided multiple times)
* `-seed=<hex>`: set a specific RNG seed (defaults to random if unspecified).
* `-iterations=<n>`: specify the number of iterations.
* `-list_tests`: display list of available tests and modules you can run.
* `-log=<0|1>`: enable or disable test execution logging (default: 0 = disabled).
Beyond these features, the idea is to also make future developments smoother, as adding new tests require only a single entry in the central test registry, and new command-line options can be introduced easily by extending the framework’s `parse_arg()` function.
Compatibility Note:
The framework continues accepting the two positional arguments previously supported (iterations and seed), ensuring existing workflows remain intact.
Testing Notes:
Have fun. You can quickly try it through `./tests -j=<workers_num>` for parallel execution or `./tests -t=<test_name>` to run a specific test (call `./tests -print_tests` to display all available tests and modules).
Extra Note:
I haven't checked the exhaustive tests file so far, but I will soon. For now, this only runs all tests declared in the `tests` binary.
Testing Results: (Current master branch vs PR in seconds)
* Raspberry Pi 5: master \~100 s → PR \~38 s (5 jobs)
* MacBook Pro M1: master \~30 s → PR \~10 s (6 jobs)
ACKs for top commit:
theStack:
re-ACK 2f4546ce56
real-or-random:
ACK 2f4546ce56
hebasto:
ACK 2f4546ce56.
Tree-SHA512: 85ca2cbb620b84b35b353d5d4cf093c388fc3851ca405eeb0e458f8fa72b60534bccd357c7edabf8fc9aa93d9ad0a6fbac3dd5c4d5f9dfdf4d8701a9834755b9
15d014804e ci: Drop default for `inputs.command` in `run-in-docker-action` (Hennadii Stepanov)
1decc49a1f ci: Use YAML anchor and aliases for repeated "CI script" steps (Hennadii Stepanov)
dff1bc107d ci, refactor: Generalize use of `matrix.configuration.env_vars` (Hennadii Stepanov)
4b644da199 ci: Use YAML anchor and aliases for repeated "Print logs" steps (Hennadii Stepanov)
a889cd93df ci: Bump `actions/checkout` version (Hennadii Stepanov)
574c2f3080 ci: Use YAML anchor and aliases for repeated "Checkout" steps (Hennadii Stepanov)
Pull request description:
GHA YAML parser now [supports](https://github.com/actions/runner/issues/1182#issuecomment-3156285802) anchors.
This PR makes use of that support to [DRY](https://en.wikipedia.org/wiki/Don%27t_repeat_yourself) the workflow code.
ACKs for top commit:
real-or-random:
utACK 15d014804e
Tree-SHA512: a25a226fec0053242bc46b8c9815067a35af632cfbffefcc5cd4c96a67c0535dde447753099cbc74ecc64072d36aef2aa78c105b66f43cb3134ffa1ae60dca1e
53585f93b7 ci: Use clang-snapshot in "MSan" job (Hennadii Stepanov)
6894c964f3 Fix Clang 21+ `-Wuninitialized-const-pointer` warning when using MSan (Hennadii Stepanov)
Pull request description:
In Bitcoin Core, the "MSan" CI jobs use the latest tagged Clang available from http://apt.llvm.org.
This PR applies similar changes and switches the "MSan" CI jobs to clang-snapshot.
This exposes problematic code that was reported in https://github.com/bitcoin/bitcoin/issues/33284.
ACKs for top commit:
real-or-random:
utACK 53585f93b7
Tree-SHA512: 79bc10f1d0a60ed67b518eb8fab9a48146a4ef1fff95c8775717be3a950b323ae89a12999504ed8446f164da426894abe02f9fb61b5ca19453d549a34873b73b
f163c35897 ci: Set `DEBIAN_FRONTEND=noninteractive` (Hennadii Stepanov)
70ae177ca0 ci: Bump `docker/build-push-action` version (Hennadii Stepanov)
b2a95a420f ci: Drop `tags` input for `docker/build-push-action` (Hennadii Stepanov)
122014edb3 ci: Add `scope` parameter to `cache-{to,from}` options (Hennadii Stepanov)
Pull request description:
This PR fixes an issue where only the latest image cache was available.
For other minor improvements, see the individual commit messages.
ACKs for top commit:
real-or-random:
utACK f163c35897
Tree-SHA512: 7178c447d32e5c06e42d33ed32c9088fc19ca6a67369f2a8f6672b0ec010a516d4bb3a70a1847eec76e034ec22d6df778f6d421a04ba603ae18526a6f4104e65
Add support for specifying single tests or modules to run via the
"--target" or "-t" command-line option. Multiple targets can be
provided; only the specified tests or all tests in the specified
module/s will run instead of the full suite.
Examples:
-t=<test name> runs an specific test.
-t=<module name> runs all tests within the specified module.
Both options can be provided multiple times.
Lightweight unit testing framework, providing a structured way to define,
execute, and report tests. It includes a central test registry, a flexible
command-line argument parser of the form "--key=value" / "-k=value" /
"-key=value" (facilitating future framework extensions), ability to run
tests in parallel and accumulated test time logging reports.
So far the supported command-line args are:
- "--jobs=<num>" or "-j=<num>" to specify the number of parallel workers.
- "--seed=<hex>" to specify the RNG seed (random if not set).
- "--iterations=<num>" or "-i=<num>" to specify the number of iterations.
Compatibility Note:
To stay compatible with previous versions, the framework also supports
the two original positional arguments: the iterations count and the
RNG seed (in that order).
4d90585fea docs: Improve API docs of _context_set_illegal_callback (Tim Ruffing)
895f53d1cf docs: Clarify that callback can be called more than once (Tim Ruffing)
Pull request description:
The tests in #1698 reminded me that some functions, e.g., `secp256k1_ec_pubkey_cmp`, may call the illegal callback more than once (see https://github.com/bitcoin-core/secp256k1/pull/1390#discussion_r1279194655 for more context). This PR clarifies the API docs to state explicitly that this is possible.
This is the simplest solution. Any production code should crash anyway if it encounters a callback. And in debug code or in our test code, it doesn't really matter whether you see an error message once or twice.
The alternative is to provide a guarantee that the callback is called only once. But that would make our code more complex for no good reason.
The second commit fixes a few typos, wording, and consistency.
ACKs for top commit:
stratospher:
ACK 4d90585.
theStack:
re-ACK 4d90585fea
Tree-SHA512: 97c31d68851e845b21e9ec2530432603917c019580feba98b62014b538f61be94ba963bf619217720d8f7331ac830e97e62c76c02e7297d3cf73dd085e6f4ca2
dfe284ed2d bench: improve context creation in ECDH benchmark (Sebastian Falbesoner)
Pull request description:
Calling `secp256k1_context_create` with `SECP256K1_FLAGS_TYPE_CONTEXT` seems to be confusing and not strictly API-compliant, as the only allowed (non-deprecated) value is `SECP256K1_CONTEXT_NONE`, even if the former happens to map to the latter currently.
Fix this by not dynamically creating a context in the first place and switch to using the static context, as it is sufficient for this benchmark and presumably matches what the "no capabilities" comment intended back then.
Alternatives are:
* keep the signing context and only fix the name, i.e. s/_FLAGS_TYPE_CONTEXT/_CONTEXT_NONE/
* use `secp256k1_context_static` everywhere directly and get rid of the `ctx` field in the `bench_ecdh_data` struct (less flexible for future changes, deviates from other bench structures)
Found while reviewing #1698, see https://github.com/bitcoin-core/secp256k1/pull/1698#discussion_r2350395913.
ACKs for top commit:
sipa:
utACK dfe284ed2d
stratospher:
ACK dfe284e. not sure whether the alternative is preferred though.
real-or-random:
utACK dfe284ed2d
furszy:
ACK dfe284ed2d
Tree-SHA512: 106d115ec11577fcd66e93293289545aa0fa78480f4bdc8c440963e4e6b050c81d4775268cfa0e1ab44db4c08a5768a0ff80f74b866e550ddd22a4e17ccb9014
ab560078aa build: Fix warnings in x86_64 assembly check (Hennadii Stepanov)
Pull request description:
On the master branch @ 10dab907e7, the x86_64 assembly check in both the Autotools and CMake build systems can fail depending on externally provided flags. For example:
```
$ env CFLAGS="-Wall -Werror" ./configure --with-asm=x86_64
<snip>
checking for x86_64 assembly availability... no
configure: error: x86_64 assembly requested but not available
```
or
```
$ env CFLAGS="-Wall -Werror" cmake -B build -DSECP256K1_ASM=x86_64
<snip>
-- Performing Test HAVE_X86_64_ASM
-- Performing Test HAVE_X86_64_ASM - Failed
CMake Error at CMakeLists.txt:111 (message):
x86_64 assembly requested but not available.
-- Configuring incomplete, errors occurred!
```
The same issue occurs in CI jobs that build on Windows using clang-cl.
This PR fixes both build systems.
ACKs for top commit:
real-or-random:
utACK ab560078aa
furszy:
ACK ab560078aa
Tree-SHA512: d556b642d58c601e7f027ac54975249e05a8b3927c5efd229be43d264b024d00eab9973193adb52f2f60075fca0571644662d61150a19098820091ced2d56fa0
7321bdf27b doc: clarify API doc of `secp256k1_ecdsa_recover` return value (Jonas Nick)
Pull request description:
ACKs for top commit:
jonasnick:
ACK 7321bdf27b
Tree-SHA512: a7bac228cf504f6c8a53fdc15c7f3241ac27c214e10b9aabdf962e8d7c4c5107d0c0898467bd3f491cecf0ca2851c604da1ec5520a353b27c8aa02ce3daf2334
Calling `secp256k1_context_create` with `SECP256K1_FLAGS_TYPE_CONTEXT`
seems to be not strictly API-compliant, as the only allowed
(non-deprecated) value is `SECP256K1_CONTEXT_NONE`, even if the
former happens to map to the latter currently.
Fix this by not dynamically creating a context in the first place and
switch to using the static context, as it is sufficient for this
benchmark and presumably matches what the "no capabilities" comment
intended back then.
Relocate the clock time getter to tests_common.h to
make it easily reusable across test programs. This
will be useful for the upcoming unit test framework.
Context - why not placing it inside testutil.h?:
The bench program links against the production-compiled library,
not its own compiled version. Therefore, `gettime_i64()` cannot
be moved to testutil.h, because testutil.h calls
`secp256k1_pubkey_save()`, which exists only in the internal
secp256k1.c and not in the public API.
