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.
399b582a5f Split memclear into two versions (John Moffett)
Pull request description:
Replace `memset` which can still be optimized out as `secnonce` isn't read later in this function. The API makes it clear the callee is responsible for it, so we need to assure it's cleared properly.
ACKs for top commit:
real-or-random:
utACK 399b582a5f thanks!
jonasnick:
ACK 399b582a5f
Tree-SHA512: 98cd6c44a45218ad66ca99ceacb9bbccee7ff105957363bcde9c3e17d55ac4a81afc881af8adfce7ff3d6ef5d75f3b111d7220256914ca0bedeb43769b38dd20
Switch msg initialization from a string literal to a brace-enclosed
array to avoid -Wunterminated-string-initialization. Upstream removed
the trailing '!' from the message; this change retains it.
The previous code is correct and harmless to initialize an array with a
non-terminated character sequence using a string literal.
However, it requires exactly specifying the array size, which can be
cumbersome.
Also, GCC-15 may issue the -Wunterminated-string-initialization warning.
[1]
Fix both issues by using array initialization. This refactoring commit
does not change behavior.
[1] Example warning:
src/modules/schnorrsig/main_impl.h:48:46: error: initializer-string for array of 'unsigned char' is too long [-Werror=unterminated-string-initialization]
48 | static const unsigned char bip340_algo[13] = "BIP0340/nonce";
| ^~~~~~~~~~~~~~~
(cherry picked from commit fa67b6752d)
Conflicts:
src/testrand_impl.h (kept local name `secp256k1_testrand_seed`)
7ebaa134a7 check-abi: remove support for obsolete CMake library output location (src/libsecp256k1.so) (Sebastian Falbesoner)
Pull request description:
The CMake library output location was changed from "src/" to "lib/" in PR #1553, supporting the old location (presumably done to avoid having users to reconfigure existing CMake builds for a temporary transition window) shouldn't be necessary anymore.
ACKs for top commit:
real-or-random:
utACK 7ebaa134a7
Tree-SHA512: 7a4e86260d9593ba4616590c927ae8753b007d51426a86eeb6f8fa35338cfa6c9b05d962f32d0d84a3271340edff2cd50f9180ce1357ab49b7850b58fa017192
secp256k1_memclear has the side effect of undefining bytes for
valgrind checks. In some cases, we may want to zero bytes
but allow subsequent reads. So we split memclear into
memclear_explicit, which makes no guarantees about the content
of the buffer on return, and memzero_explicit, which guarantees
zero value on return.
Change the memset in partial_sign to use memzero_explicit.
806de38bfc doc: mention ctx requirement for `_ellswift_create` (not secp256k1_context_static) (Sebastian Falbesoner)
Pull request description:
Public functions that require a context for generator point multiplication (i.e. `ctx->ecmult_gen_ctx` is built) usually denote this in the API header by mentioning to not use `secp256k1_context_static`, so add this for `_ellswift_create` as well. This seems the only instance where this is missing currently, see
```
$ git grep ecmult_gen_context_is_built
$ git grep ctx:.*context_static
```
(note that in the musig and schnorr modules, two public functions for nonce generation / signing map to a single internal function where the context requirement is checked).
Noted while reviewing #1698.
ACKs for top commit:
sipa:
ACK 806de38bfc
jonasnick:
ACK 806de38bfc
real-or-random:
ACK 806de38bfc
josibake:
ACK 806de38bfc
Tree-SHA512: 902e9e21060e09e8e7d72fec8cdc42e0ed18f95824d3220100d7b65720511f934d38e3e556e38bb510d98284bccc12b857f329997640d1c07edd5b55ef6d8e09
