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.
Ubuntu 20.04 LTS has reached the end of standard support. There no
longer appear to be compelling reasons to maintain compatibility with
CMake 3.16.
The new suggested minimum, CMake 3.22, is shipped with Ubuntu 22.04 LTS,
which is supported until April 2027.
This change also introduces new CMake policies, from CMP0098 to CMP0128.
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
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
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
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.
4d9645bee0 cmake: Remove "AUTO" value of `SECP256K1_ECMULT_GEN_KB` option (Hennadii Stepanov)
a06805ee74 cmake: Remove "AUTO" value of `SECP256K1_ECMULT_WINDOW_SIZE` option (Hennadii Stepanov)
26b94ee92a autotools: Remove "auto" value of `--with-ecmult-gen-kb` option (Hennadii Stepanov)
122dbaeb37 autotools: Remove "auto" value of `--with-ecmult-window` option (Hennadii Stepanov)
Pull request description:
"auto" implies that a value is being chosen based on build system introspection or host system capabilities. However, for the `--with-ecmult-window` and `--with-ecmult-gen-kb` options, the values "auto" are hardcoded, which might lead to confusion.
This PR replaces "auto" with more appropriate default values.
If Concept ACKed, I'll add equivalent commits for CMake.
ACKs for top commit:
sipa:
utACK 4d9645bee0
real-or-random:
utACK 4d9645bee0 good from my side, but let's see if we can get more (Concept) ACKs
Tree-SHA512: 9e68f73682c5310c68d2337594f13b99a52bfc365564e39df2e412b576635c90cccd2298406a4281f014916c4a1710e19c7390f05a4b0acbd09869bfb56f36ac
158f9e5eae cmake: Do not modify build types when integrating by downstream project (Hennadii Stepanov)
Pull request description:
The `CMAKE_BUILD_TYPE` and `CMAKE_CONFIGURATION_TYPES` must be managed by the downstream project.
Suggesting to review with `git diff -w`.
Fixes `std::out_of_range` exception from CMake in https://github.com/hebasto/bitcoin/pull/192 when running configuration step using "Ninja Multi-Config" generator:
```
$ cmake -B build -G "Ninja Multi-Config"
...
-- Configuring done (17.1s)
terminate called after throwing an instance of 'std::out_of_range'
what(): map::at
Aborted (core dumped)
```
Here are related discussions:
- https://discourse.cmake.org/t/uncaught-exception-when-trying-to-generate-a-project-using-ninja-multi-config/11051
- https://gitlab.kitware.com/cmake/cmake/-/issues/26064
ACKs for top commit:
real-or-random:
ACK 158f9e5eae
Tree-SHA512: b3040f40438d530f14b7e0f7d523e74b5843d88d250ff7955a99cc8c451feb9471a48134d1a89b3651b3f8195f91c17135c7b8a5d3ab092c8d35275b57743b8c
4706be2cd0 cmake: Reimplement `SECP256K1_APPEND_CFLAGS` using Bitcoin Core approach (Hennadii Stepanov)
c2764dbb99 cmake: Rename `SECP256K1_LATE_CFLAGS` to `SECP256K1_APPEND_CFLAGS` (Hennadii Stepanov)
Pull request description:
This PR address this https://github.com/hebasto/bitcoin/issues/239#issuecomment-2182713690:
> For consistency with libsecp256k1:
>
> > > Is this code block supposed to achieve the same as our `SECP256K1_LATE_CFLAGS` (implemented by a user-defined function `all_targets_add_compile_options`) in libsecp256k1?
> >
> >
> > It is. But this approach guaranties to override even options that are abstracted by CMake, for instance [#157 (comment)](https://github.com/hebasto/bitcoin/pull/157#issuecomment-2090465123).
>
> * If we agree that appending to rule variables is superior, should we also do this in libsecp256k1?
>
> * And/or should we rename the `SECP256K1_LATE_CFLAGS` variable to `APPEND_CFLAGS`?
ACKs for top commit:
real-or-random:
utACK 4706be2cd0
Tree-SHA512: 24603886c4d6ab4e31836a67d5759f9855a60c6c9d34cfc6fc4023bd309cd51c15d986ac0b77a434f9fdc6d5e97dcd3b8484d8f5ef5d8f840f47dc141de18084
ec4c002faa cmake: Simplify `PROJECT_IS_TOP_LEVEL` emulation (Hennadii Stepanov)
cae9a7ad14 cmake: Do not set emulated PROJECT_IS_TOP_LEVEL as cache variable (Hennadii Stepanov)
Pull request description:
As CMake's cache is a global database, modifying it within a project integrated with the `add_subdirectory()` command, which may also include using the `FetchContent` module, could potentially affect downstream projects and sibling ones.
ACKs for top commit:
real-or-random:
utACK ec4c002faa
theuni:
utACK ec4c002faa
Tree-SHA512: de2c8c583367028d06701f79fc5232b351622c8496d196aad8c22a1ec4e450af53e556a4f6526ed47250f818143a69a12f5fc8cc755c864510e67530dacde66e
"AUTO" implies that a value is being chosen based on build system
introspection or host system capabilities. However, for the
`SECP256K1_ECMULT_GEN_KB` option, the value "AUTO" is hardcoded, which
might lead to confusion.
This change replaces "AUTO" with a more appropriate default value.
"AUTO" implies that a value is being chosen based on build system
introspection or host system capabilities. However, for the
`SECP256K1_ECMULT_WINDOW_SIZE` option, the value "AUTO" is hardcoded,
which might lead to confusion.
This change replaces "AUTO" with a more appropriate default value.
Detecting whether it is the top level by comparing the value of
`CMAKE_SOURCE_DIR` with `CMAKE_CURRENT_SOURCE_DIR` is supported by all
versions of CMake and is a very common pattern.
42f8c51402 cmake: Add `SECP256K1_LATE_CFLAGS` configure option (Hennadii Stepanov)
Pull request description:
This PR enables users to override compiler flags that have been set by the CMake-based build system, such as warning flags.
The Autotools-based build system has the same feature out-of-the-box.
See more details [here](https://github.com/bitcoin-core/secp256k1/issues/1235#issuecomment-1465330925).
Here are some examples of the new option usage:
```
cmake -S . -B build -DSECP256K1_LATE_CFLAGS="-Wno-extra -Wlong-long"
```
```
cmake -S . -B build -DSECP256K1_BUILD_EXAMPLES=ON -DSECP256K1_LATE_CFLAGS=-O1
cmake --build build
...
In function ‘secp256k1_ecmult_strauss_wnaf’,
inlined from ‘secp256k1_ecmult’ at /home/hebasto/git/secp256k1/src/ecmult_impl.h:353:5:
/home/hebasto/git/secp256k1/src/ecmult_impl.h:291:5: warning: ‘aux’ may be used uninitialized [-Wmaybe-uninitialized]
291 | secp256k1_ge_table_set_globalz(ECMULT_TABLE_SIZE(WINDOW_A) * no, state->pre_a, state->aux);
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
In file included from /home/hebasto/git/secp256k1/src/secp256k1.c:29:
/home/hebasto/git/secp256k1/src/ecmult_impl.h: In function ‘secp256k1_ecmult’:
/home/hebasto/git/secp256k1/src/group_impl.h:174:13: note: by argument 3 of type ‘const secp256k1_fe *’ to ‘secp256k1_ge_table_set_globalz’ declared here
174 | static void secp256k1_ge_table_set_globalz(size_t len, secp256k1_ge *a, const secp256k1_fe *zr) {
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
In file included from /home/hebasto/git/secp256k1/src/secp256k1.c:30:
/home/hebasto/git/secp256k1/src/ecmult_impl.h:345:18: note: ‘aux’ declared here
345 | secp256k1_fe aux[ECMULT_TABLE_SIZE(WINDOW_A)];
| ^~~
...
```
Please note that in the last case providing `env CFLAGS=-O1` or `-DCMAKE_C_FLAGS=-O1` won't work.
ACKs for top commit:
real-or-random:
ACK 42f8c51402
Tree-SHA512: 2b152e420a4a8ffd5f67857de03ae5ba9f2223e535ac01a867c1025e0619180d8255fdd1e5fb8279b290f0a1c96bcc874043ef968fcd99b1ff4e13041a91b1e1
This also makes the order in which module options are processed
consistent between CMake and autotools (the reverse order of the listing
printed to stdout).
The scheme implemented is described below, and largely follows the paper
"SwiftEC: Shallue–van de Woestijne Indifferentiable Function To Elliptic Curves",
by Chavez-Saab, Rodriguez-Henriquez, and Tibouchi
(https://eprint.iacr.org/2022/759).
A new 64-byte public key format is introduced, with the property that *every*
64-byte array is an encoding for a non-infinite curve point. Each curve point
has roughly 2^256 distinct encodings. This permits disguising public keys as
uniformly random bytes.
The new API functions:
* secp256k1_ellswift_encode: convert a normal public key to an ellswift 64-byte
public key, using additional entropy to pick among the many possible
encodings.
* secp256k1_ellswift_decode: convert an ellswift 64-byte public key to a normal
public key.
* secp256k1_ellswift_create: a faster and safer equivalent to calling
secp256k1_ec_pubkey_create + secp256k1_ellswift_encode.
* secp256k1_ellswift_xdh: x-only ECDH directly on ellswift 64-byte public keys,
where the key encodings are fed to the hash function.
The scheme itself is documented in secp256k1_ellswift.h.
