57eda3ba30 musig: ctimetests: fix _declassify range for generated nonce points (Sebastian Falbesoner)
Pull request description:
As noticed in https://github.com/bitcoin-core/secp256k1/pull/1614#discussion_r1796215582, the area marked as non-secret exceeds the nonce_pts array in the second iteration of the for loop. Fix that by passing the correct size to the _declassify call.
ACKs for top commit:
sipa:
utACK 57eda3ba30
real-or-random:
utACK 57eda3ba30
Tree-SHA512: ff8074e3d1078d66a52d08c661997856ff586b3b4564a865a75212b32fafd7906d58885371bd63005007fde554ebcad121ab66125abe4331cf0aac63fc018ed0
447334cb06 include: Avoid visibility("default") on Windows (Tim Ruffing)
Pull request description:
Fixes#1421. See code comments for rationale.
Related meta-bug: #1181. This reminds me that we should move forward with #1359.
ACKs for top commit:
fanquake:
ACK 447334cb06
hebasto:
ACK 447334cb06, tested on Ubuntu 24.04 using the following commands:
theuni:
ACK 447334cb06
Tree-SHA512: aaa47d88fd1b1f85c3e879a2b288c0eb3beebad0cc89e85f05d0b631f83e58d5a324fb441911970865eaa292f6820d03a1b516d6e8de37a87510e2082acc6e28
8be3839fb2 Remove unused scratch space from API (Jonas Nick)
Pull request description:
We had already merged this in #1305, but it was reverted before a release (#1311) because this change is not backwards compatible but at the time we only wanted to make a patch release in order to fix an actual issue.
Due to the musig module, the next release will increment the version number from 0.5.x to 0.6.0, so it would be a good time to remove the scratch space from the API.
ACKs for top commit:
sipa:
utACK 8be3839fb2
real-or-random:
utACK 8be3839fb2
Tree-SHA512: ecd6bc1d925992f9df8e26820388fc436bbb6bc5f250950edf00406f006ca0df52ab8cd56a1b7541e57af0682ddadf6d34bd638b27557d301a5dff6c327a5ebc
The area marked as non-secret exceeds the nonce_pts array in the
second iteration of the for loop. Fix that by passing the correct
size to the _declassify call.
c232486d84 Revert "cmake: Set `ENVIRONMENT` property for examples on Windows" (Hennadii Stepanov)
26e4a7c214 cmake: Set top-level target output locations (Hennadii Stepanov)
Pull request description:
While testing https://github.com/bitcoin-core/secp256k1/pull/1551, I noticed that when cross-compiling a shared library with examples for Windows, the `ctest` fails to run examples with Wine. Adjusting the `PATH` variable in 4af241b320/examples/CMakeLists.txt (L16-L18) does not help because `WINEPATH` is expected.
Another issue with the current implementation is that the examples cannot run individually on Windows.
This PR resolves both issues by reverting the implementation from https://github.com/bitcoin-core/secp256k1/pull/1290 in favour of the reworked and improved implementation from https://github.com/bitcoin-core/secp256k1/pull/1233.
ACKs for top commit:
theuni:
Concept ACK and utACK c232486d84.
real-or-random:
utACK c232486d84
Tree-SHA512: 479b71d15d5d5670f6f69da3da599240c345711003383ca805c821b67065c9baaf269f987792cf1029211cdbfe799aecd401e6940a471539e3929b4a90e0781d
ef7ff03407 f can never equal -m (Russell O'Connor)
Pull request description:
In fact, before reaching this particular VERIFY_CHECK, we had already successfully passed through
VERIFY_CHECK(secp256k1_modinv64_mul_cmp_62(&f, len, &modinfo->modulus, -1) > 0); /* f > -modulus */
ensuring that f is not -m.
ACKs for top commit:
sipa:
ACK ef7ff03407
real-or-random:
utACK ef7ff03407
Tree-SHA512: a8a8dcbad4dff36b9c49e40e07b212312cbf915132aea008eab6ea61b35bddb6d7782229c2cc528fb404d05132482c602cad768414d76153bb425a3d23714fff
168c92011f build: allow enabling the musig module in cmake (Jonas Nick)
f411841a46 Add module "musig" that implements MuSig2 multi-signatures (BIP 327) (Jonas Nick)
0be79660f3 util: add constant-time is_zero_array function (Jonas Nick)
c8fbdb1b97 group: add ge_to_bytes_ext and ge_from_bytes_ext (Jonas Nick)
85e224dd97 group: add ge_to_bytes and ge_from_bytes (Jonas Nick)
Pull request description:
EDIT: based on #1518. Closes#1452. Most of the code is a copy from [libsecp256k1-zkp](https://github.com/BlockstreamResearch/secp256k1-zkp). The API added in this PR is identical with the exception of two modifications:
1. I removed the unused `scratch_space` argument from `secp256k1_musig_pubkey_agg`. This argument was intended to allow using `ecmult_multi` algorithms for key aggregation in the future. But at this point it's unclear whether the `scratch_space` object will remain in its current form (see #1302).
2. Support for adaptor signatures was removed and therefore the `adaptor` argument of `musig_nonce_process` was also removed.
In contrast to the module in libsecp256k1-zkp, the module is non-experimental. I slightly cleaned up parts of the module, adjusted the code to the new definition of the VERIFY_CHECK macro and applied some simplifications that were possible because the module is now in the upstream repo (`ge_from_bytes`, `ge_to_bytes`). You can follow the changes I made to the libsecp256k1-zkp module at https://github.com/jonasnick/secp256k1-zkp/commits/musig2-upstream/.
ACKs for top commit:
sipa:
reACK 168c92011f
real-or-random:
reACK 168c92011f
theStack:
re-ACK 168c92011f
Tree-SHA512: e3a599a8d5a466107b9a86f76582b8fb9dc87ec95416c784c3ef39d1c64686e6c739806ed6ba62c91793eb7fa418a6270cf999027ee7bd3dd85c67bc2c74f677
In fact, before reaching this particular VERIFY_CHECK, we had already successfully passed through
VERIFY_CHECK(secp256k1_modinv64_mul_cmp_62(&f, len, &modinfo->modulus, -1) > 0); /* f > -modulus */
ensuring that f is not -m.
This change:
1. Collects build artifacts in dedicated locations.
2. Allows to run individual examples with a shared library on Windows.
3. Is compatible with Wine when testing cross-compiled Windows binaries
on Linux.
4. Is compatible with integration the project into a larger project
hierarchy.
7c987ec89e cmake: Call `enable_testing()` unconditionally (Hennadii Stepanov)
6aa576515e cmake: Delete `CTest` module (Hennadii Stepanov)
Pull request description:
1. Delete `CTest` module.
The `CTest` module handles `CDash` integration, which we do not use. It is not required for testing functionality.
2. Clean up cases when to invoke `enable_testing()`
The `enable_testing()` command invocation is required for `add_test()` commands, which are used only for `{noverify_}tests`, `exhaustive_tests` and examples.
ACKs for top commit:
real-or-random:
utACK 7c987ec89e
theStack:
ACK 7c987ec89e
Tree-SHA512: f2628a4c4363b86f7c49d705df192e76368997e3f01cb736fcc272330bbf481af023d29d9faec17878d3bf74e4d265870cd93953406ae4612cd717811f22dd1a
292310fbb2 doc: fix typos in `secp256k1_ecdsa_{recoverable_,}signature` API description (Sebastian Falbesoner)
Pull request description:
This small PR fixes two small typos I noticed while looking at the pubkey recovery module (s/structured/structure/).
ACKs for top commit:
real-or-random:
utACK 292310fbb2
Tree-SHA512: 7abda2d5b56d810fd83c7de9b018818a28fd5969f669b53b9dc0e1034428feef63a3157714f45002185b94c6c35617cab21861fa56dba51883d1326b4a1cb033
Downstream projects may include libsecp test executables in their test
suites. In such cases, if a `-fsanitize=...` flag is passed via the
`SECP256K1_APPEND_CFLAGS` variable, the same flag must also be passed to
the linker.
fa67b6752d refactor: Use array initialization for unterminated strings (MarcoFalke)
e34b476730 ci: Bump GCC_SNAPSHOT_MAJOR to 15 (maflcko)
Pull request description:
Follow-up to https://github.com/bitcoin-core/secp256k1/pull/1313
Clang should silently follow the `main` devel branch, but GCC needs to be bumped manually.
ACKs for top commit:
hebasto:
ACK fa67b6752d, I have reviewed the code and it looks OK.
real-or-random:
utACK fa67b6752d
Tree-SHA512: e76371e5b1ff259ec501671872352c0d46d34a96aadae04e6ee37f9457308412e18010e724df667a15c3a85997a16da191f50cd3a01ee3f20d5f16b5893d179a
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";
| ^~~~~~~~~~~~~~~
7057d3c9af ci: Silent Homebrew's noisy reinstall warnings (Hennadii Stepanov)
Pull request description:
Homebrew's warnings are quite noisy on the master branch:
This PR silents them to allow us to focus on any other CI infra warnings once they happen.
ACKs for top commit:
real-or-random:
utACK 7057d3c9af
Tree-SHA512: 4dbdb459e97cb5956219214fd7407ec52fc018dba24ae945337ad5c94df510472cfc708f9f67d4df5b8e983bf4e78b2efd56533f28113b3463a1397d4d7a032a
763d938cf0 ci: only enable extrakeys module when schnorrsig is enabled (Jonas Nick)
af551ab9db tests: do not use functions from extrakeys module (Jonas Nick)
Pull request description:
ACKs for top commit:
sipa:
ACK 763d938cf0
hebasto:
ACK 763d938cf0.
Tree-SHA512: a5d52e69af896e7bb43c6826f269b5f26e8d1b8c0d040d71a369fdaebf894e47dcf9ff1bbe1b62864adb85437d2c93eabd655c4d690d5d894b712145c2bd252f
e2af491263 ci: Switch to the new default value of the precomputed table for signing (Hennadii Stepanov)
d94a9273f8 build: Adjust the default size of the precomputed table for signing (Hennadii Stepanov)
Pull request description:
This PR implements the [outcomes](https://github.com/bitcoin-core/secp256k1/issues/1549#issuecomment-2200559257) from today's IRC meeting:
1. The default size of the precomputed table for signing is now aligned with Bitcoin Core's [default](a057869aa3).
2. The default value in CI has been updated to reflect the new default.
ACKs for top commit:
sipa:
utACK e2af491263
real-or-random:
utACK e2af491263
Tree-SHA512: aa9db5bc2aec29a35a503a80617a4c096e9909648084fe1ce43b5dd7e74dd812e7642305bd5bc13eb581efc23f12904e200e13cb1a35955b773e05ab4f84be4e
