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).
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.
325d65a8cf Rename and clear var containing k or -k (John Moffett)
Pull request description:
Follow-up to #1579. `buf` still holds the nonce or its negation, so ought to be cleared.
ACKs for top commit:
theStack:
Code review re-ACK 325d65a8cf
real-or-random:
utACK 325d65a8cf
Tree-SHA512: a2fe39d7c44cebc0abe712828d521c2a7aba1db2d9dc5fc811dcaf96f1e45494dba5d7f016b6d9200ab523641ff62083686dbc942284e0f548183aaf60d8bfa2
960ba5f9c6 Use size_t instead of int for RFC6979 outlen copy (John Moffett)
Pull request description:
If `outlen > INT_MAX` it results in segfault or hang (when `outlen` is a multiple of 2^32) on most implementations due to conversion in: `int now = outlen` producing negative values or zero. Unreachable in current code and highly improbable in future practice, but fits contract better and fixes a couple of compiler warnings.
ACKs for top commit:
real-or-random:
utACK 960ba5f9c6
theStack:
Code-review ACK 960ba5f9c6
Tree-SHA512: b91ee2fd3e962000f1b98a42e6f3c70cb3738c639fef8c2ce0cf53f49fe55da3e5d332eabbd8cbe9cdccb4e9c0ae70d3390a41f9468fd23ded3318596548c68f
buf currently holds k or -k and isn't cleared, so clear it and rename to
nonce32 to clarify its sensitivity and match how it is named in the
corresponding ECDSA sign_inner.
5153cf1c91 tests: refactor tagged hash tests (josibake)
Pull request description:
Opened in response to https://github.com/bitcoin-core/secp256k1/pull/1698#discussion_r2269449070
---
We use tagged hashes in `modules/musig`, `modules/schnorrsig`, `modules/ellswift`, and the proposed `modules/silentpayments`. In looking for inspiration on how to add tagged hash midstate verification for https://github.com/bitcoin-core/secp256k1/pull/1698, it seemed like a good opportunity to DRY up the code across all of the modules.
I chose the convention used in the ellswift module as this seems the most idiomatic C. Since the tags are normally specified as strings in the BIPs, I also added a comment above each char array for convenience.
If its deemed too invasive to refactor the existing modules in this PR, I'm happy to drop the refactor commits for the ellswift and schnorrsig modules. All I need for https://github.com/bitcoin-core/secp256k1/pull/1698 is the first commit which moves the utility function out of the musig module to make it available to use in the silent payments module.
ACKs for top commit:
real-or-random:
utACK 5153cf1c91 assuming CI passes
theStack:
Code-review ACK 5153cf1c91
Tree-SHA512: 335ec3ee6a265e13cc379968f8fa1624534bef2389e4e21b85e6a9572ce1bd9dee4eabd2cb6d187ac974db3ab8246c2626d309ccfbee5744c30cf7560d1e261c
Move the sha256_tag_test_internal function out of the musig module
into tests.c. This makes it available to other modules wishing to verify tagged
hashes without needing to duplicate the function.
Change the function signature to expect a const unsigned char and update
the tagged hash tests to use static const unsigned char character
arrays (where necessary).
Add a comment for each tag. This is done as a convenience for checking
the strings against the protocol specifications, where the tags are
normally specified as strings.
Update tests in the ellswift and schnorrsig modules to use the
sha256_tag_test_internal helper function.
Otherwise, commands fail with the error:
```
Stderr of gcov was >><< End of stderr
Exception was >>Got function secp256k1_scalar_split_lambda on multiple lines: 67, 142.
You can run gcovr with --merge-mode-functions=MERGE_MODE.
The available values for MERGE_MODE are described in the documentation.<< End of stderr
```
24ba8ff168 chore(ci): Fix typo in Dockerfile comment (Maximilian Hubert)
Pull request description:
Corrects the spelling of "dpkg-dev" in a comment within the Dockerfile.
The comment explains the reason for installing the `dpkg-dev` package..
ACKs for top commit:
real-or-random:
ACK 24ba8ff168
Tree-SHA512: 79742bcd8018f6435d1113a08e44bacca6027faa5171e564b6c6f9cc8a5630aac32f8f611d276ef50b7f03d842bc9228d368c99bdc816decf947d39956caed2c
c25c3c8a88 test: update wycheproof test vectors (josibake)
Pull request description:
Pull in updated test vectors. This update is done as a follow-up to #1711.
ACKs for top commit:
real-or-random:
ACK c25c3c8a88
Tree-SHA512: 6f3b41460155a9f78d23711e471e73f732d3f6cdc2442f2f024540c04c3d4ab2ca376d4db020e99b093cd6a95c11f7dd3220f18727d62c4995a9a069d362406d
7b07b22957 cmake: Avoid contaminating parent project's cache with BUILD_SHARED_LIBS (Hennadii Stepanov)
Pull request description:
The CMake cache is global in scope. Therefore, setting the standard cache variable `BUILD_SHARED_LIBS` can inadvertently affect the behavior of a parent project.
Consider configuring Bitcoin Core without explicit setting `BUILD_SHARED_LIBS`:
```
$ cmake -B build -DBUILD_KERNEL_LIB=ON
```
According to CMake’s documentation, this should configure `libbitcoinkernel` as `STATIC`.
However, that's not the case:
```
$ cmake --build build -t libbitcoinkernel
[143/143] Linking CXX shared library lib/libbitcoinkernel.so
```
This PR:
1. Sets the `BUILD_SHARED_LIBS` cache variable only when `libsecp256k1` is the top-level project.
2. Removes the `SECP256K1_DISABLE_SHARED` cache variable. This enables parent projects that include libsecp256k1 as a subproject to rely solely on standard CMake variables for configuring the library type. During integration into a parent project, the static library can be forced as demonstrated [here](https://github.com/bitcoin-core/minisketch/pull/75#issuecomment-2984025132).
ACKs for top commit:
purpleKarrot:
ACK 7b07b22957
theuni:
utACK 7b07b22957
Tree-SHA512: 399a02e86093656ce70d9a0292a1f30834bcc5b9cf0f77d6465adc5e8a4d8e779684adedc40942eb931fed857399e4176a23fdbdf45f277184b28159fbc932d2
The CMake cache is global in scope. Therefore, setting the standard
cache variable `BUILD_SHARED_LIBS` can inadvertently affect the behavior
of a parent project.
This change:
1. Sets the `BUILD_SHARED_LIBS` cache variable only when libsecp256k1 is
the top-level project.
2. Removes the `SECP256K1_DISABLE_SHARED` cache variable. This enables
parent projects that include libsecp256k1 as a subproject to rely
solely on standard CMake variables for configuring the library type.
7ab8b0cc01 release cleanup: bump version after 0.7.0 (Jonas Nick)
Pull request description:
Based on #1707.
ACKs for top commit:
real-or-random:
utACK 7ab8b0cc01
Tree-SHA512: 2eea54938ffa2a2c0503607566cb2900b76d5ff6d6285c65afe6fc09fa8b961f210ad915276618c5f43e3ade55252d6533c4c72a5e12c4db4c30fb20235a88df
cde4130898 musig/tests: initialize keypair (Jonas Nick)
Pull request description:
The keypair is unused in musig_partial_sign, but clang-snapshot gives a compiler warning anyway.
ACKs for top commit:
real-or-random:
ACK cde4130898
Tree-SHA512: 2a6c2b31c3ef8b5353c5dbdc156d052ff802d1f62d22214d952ee46223a9f87bc36b3ecdd49788381db379a01be83ded9def752747769b3751f3f058d57403e6
40b4a06520 changelog: update (Jonas Nick)
Pull request description:
This update includes all PRs that have a [`needs-changelog` tag](https://github.com/bitcoin-core/secp256k1/pulls?q=is%3Apr+is%3Aclosed+label%3Aneeds-changelog):
- Make static context const #1639
- include: remove WARN_UNUSED_RESULT for functions always returning 1 #1659
- cmake: Bump minimum required CMake version to 3.22 #1675#1675
- added: cmake: add a helper for linking into static libs #1678
- doc: Promote "Building with CMake" to standard procedure #1680
- also added cmake: Emulate Libtool's behavior on FreeBSD #1685
I had added the `needs-changelog` tag to #1685 because it seems noteworthy, but otherwise have not found any additional merged PR that would require a changelog entry.
I'm not sure if the changelog entry for #1678 makes sense (CC theuni)
ACKs for top commit:
real-or-random:
ACK 40b4a06520
Tree-SHA512: 678c409aea13ea9e9adaddd92f3d8da4fc98aeb9d661df9ff0f5093ad788c5ca0f194831c134675474cf9540679f3314086ee894f781afa3ea327d8001f53cd9
c82d84bb86 build: add CMake option for disabling symbol visibility attributes (Cory Fields)
ce7923874f build: Add SECP256K1_NO_API_VISIBILITY_ATTRIBUTES (Tim Ruffing)
e5297f6d79 build: Refactor visibility logic (Tim Ruffing)
Pull request description:
This is less invasive than #1695. The latter might be the right thing in a new library (and then we'd probably not support autotools in the first place), but any semantic change to this code has the potential to create news bug, or at least breakages for downstream users.
This is different from #1677 in that it does not set `hidden` explicitly. I agree with the comment in #1677 that setting `hidden` violates the principle of least surprise.
So this similar in spirit to #1674. So I wonder if this should also include
3eef7362c4. I'd say no, `fvisibility` should then set by the user. But can you, in CMake, set `CMAKE_C_VISIBILITY_PRESET` from a parent project?
ACKs for top commit:
hebasto:
ACK c82d84bb86, I have reviewed the code and it looks OK.
Tree-SHA512: dad36c32a108d813e8d4e1849260af43f79a9aa8fbfb9a42b07d737e0467924a511110df0a2c6761539a1587b617a1b11123610a3db9d4cdf2b985dfb3eb21da
bf082221ff cmake: Make `secp256k1_objs` inherit interface defines from `secp256k1` (Hennadii Stepanov)
Pull request description:
This PR effectively adds `-DSECP256K1_STATIC` to usage requirements of `secp256k1_objs` on Windows, preventing LNK4217 linker warnings.
I overlooked this issue while reviewing https://github.com/bitcoin-core/secp256k1/pull/1678.
ACKs for top commit:
real-or-random:
utACK bf082221ff
Tree-SHA512: d31e963378b795fd2be88f26574561f1e16b4c2fe10f348f6bfcf1bcc7e68da81ff93475548e9513514ded8eef5a0bd5e2eac2f4a9c5b879e0ab171e49dfcad5
