To use, invoke `cmake` with argument `--preset dev-mode`.
Solves one item in #1235.
One disadvantage over `./configure --enable-dev-mode` is that CMake
does not provide a way to "hide" presets from users. That is,
`cmake --list-presets` will list dev-mode, and it will also appear
in `cmake-gui`, even though it's not selectable there due to bug
https://gitlab.kitware.com/cmake/cmake/-/issues/23341. (So in our
case, that's probably rather a feature than a bug.)
We curently use version 3 presets which require CMake 3.21+.
Unfortunately, CMake versions before 3.19 may ignore the `--preset`
argument silently. So if the preset is not picked up, make sure you
have a recent enough CMake version.
More unfortunately, we can't even spell this warning out in
CMakePresets.json because CMake does not support officially support
comments in JSON, see
- https://gitlab.kitware.com/cmake/cmake/-/issues/21858
- https://gitlab.kitware.com/cmake/cmake/-/merge_requests/5853 .
We could use a hack hinted at in
https://gitlab.kitware.com/cmake/cmake/-/issues/21858#note_908543
but that's risky, because it could simply break for future versions,
and we probably want to use presets not only for dev mode.
28e63f7ea75af50af0bfde34309993f276bbc0ae release cleanup: bump version after 0.3.0 (Jonas Nick)
Pull request description:
Based on #1223. Should be merged only after tagging the release.
ACKs for top commit:
sipa:
ACK 28e63f7ea75af50af0bfde34309993f276bbc0ae
real-or-random:
ACK 28e63f7ea75af50af0bfde34309993f276bbc0ae
Tree-SHA512: d219f836c9258af52389f62c167adb79a0f83f520ede514e286e84f0540d35234322e67d582409c332662db17114da1681419d5d400ed88ad2be66a0f6a06089
756b61d451d2e00c357a6b55863717519164cdb2 readme: Use correct build type in CMake/Windows build instructions (Tim Ruffing)
Pull request description:
ACKs for top commit:
hebasto:
ACK 756b61d451d2e00c357a6b55863717519164cdb2, it is correct to provide the "RelWithDebInfo" configuration in multi-config setup, as the same build type is the default in single-config setups.
Tree-SHA512: e98a1519fdae4a29c7e06ecd0e68083acaf0f4fc14dfcd12282b89468052bb7c6c2fc7517c8526c9f7555a822a64b2f7c3f1ecc70d17e37a11d831d213f1daef
5d8f53e31293c582fb3fe02157bc67d2eeccea77 Remove redudent checks. (Russell O'Connor)
Pull request description:
These abs checks are implied by the subsequent line, and with the subsequent line written as it is, no underflow is possible with signed integers.
Follows up on https://github.com/bitcoin-core/secp256k1/pull/1218.
ACKs for top commit:
sipa:
utACK 5d8f53e31293c582fb3fe02157bc67d2eeccea77
real-or-random:
ACK 5d8f53e31293c582fb3fe02157bc67d2eeccea77
Tree-SHA512: ddd6758638fe634866fdaf900224372e2e51cb81ef4d024f169fbc39fff38ef1b29e90e0732877e8910158b82bc428ee9c3a4031882c2850b22ad87cc63ee305
2ef1c9b38700b7cca2ee1aace2f020ee834729c0 Update overflow check (Russell O'Connor)
Pull request description:
One does not simply check for integer overlow.
ACKs for top commit:
sipa:
ACK 2ef1c9b38700b7cca2ee1aace2f020ee834729c0
real-or-random:
ACK 2ef1c9b38700b7cca2ee1aace2f020ee834729c0
Tree-SHA512: 61238b7b59b3840aa04c4c3ff768789eba95d8d9cbd16507b86bae585fe8d077ac1ac234f9d8aea7fa342c7278a30d2d888df3a93d7ab24730e73b682b11a7fe
Signed-off-by: Harshil Jani <harshiljani2002@gmail.com>
Add secure_erase function to clear secrets
Signed-off-by: Harshil Jani <harshiljani2002@gmail.com>
Update the function with good practices
Signed-off-by: Harshil Jani <harshiljani2002@gmail.com>
Renaming random.h to examples_util.h
Signed-off-by: Harshil Jani <harshiljani2002@gmail.com>
ce3cfc78a6020d21be299e1e4f22cf8ef089194d doc: Describe Jacobi calculation in safegcd_implementation.md (Elliott Jin)
6be01036c8a6da5043953d055ffb5920728fbff7 Add secp256k1_fe_is_square_var function (Pieter Wuille)
1de2a01c2b22dc8216393ad0471382beaffef525 Native jacobi symbol algorithm (Pieter Wuille)
04c6c1b18162e3dc00d9be5098ee1ccbcb2e78d9 Make secp256k1_modinv64_det_check_pow2 support abs val (Pieter Wuille)
5fffb2c7af5d33223d819283f1a561889a8210d9 Make secp256k1_i128_check_pow2 support -(2^n) (Pieter Wuille)
Pull request description:
This introduces variants of the vartime divsteps-based GCD algorithm used for modular inverses to compute Jacobi symbols. Changes compared to the normal vartime divsteps:
* Only positive matrices are used, guaranteeing that f and g remain positive.
* An additional jac variable is updated to track sign changes during matrix computation.
* There is (so far) no proof that this algorithm terminates within reasonable amount of time for every input, but experimentally it appears to almost always need less than 900 iterations. To account for that, only a bounded number of iterations is performed (1500), after which failure is returned. The field logic then falls back to using square roots to determining the result.
* The algorithm converges to f=g=gcd(f0,g0) rather than g=0. To keep this test simple, the end condition is f=1, which won't be reached if started with g=0. That case is dealt with specially.
This code is currently unused, except for tests. I don't aim for it to be merged until there is a need for it, but this demonstrates its feasibility.
In terms of performance:
```
field_inverse: min 1.76us / avg 1.76us / max 1.78us
field_inverse_var: min 0.991us / avg 0.993us / max 0.996us
field_jacobi_var: min 1.31us / avg 1.31us / max 1.31us
field_sqrt: min 4.36us / avg 4.37us / max 4.40us
```
while with the older (f24e122d13db7061b1086ddfd21d3a1c5294213b) libgmp based Jacobi code on the same system:
```
num_jacobi: min 1.53us / avg 1.54us / max 1.55us
```
ACKs for top commit:
jonasnick:
ACK ce3cfc78a6020d21be299e1e4f22cf8ef089194d
real-or-random:
reACK ce3cfc78a6020d21be299e1e4f22cf8ef089194d diff and writeup is good and I tested every commit
Tree-SHA512: 8a6204a7a108d8802d942a54faca39917f90ea5923130683bbd870f9025f4ec8ef256ffa1d939a793f0b32d4cdfcdcd1d3f8ae5ed74a0193be7ad98362ce027e
The implementation calls the secp256k1_modinvNN_jacobi_var code, falling back
to computing a square root in the (extremely rare) case it failed converge.
This introduces variants of the divsteps-based GCD algorithm used for
modular inverses to compute Jacobi symbols. Changes compared to
the normal vartime divsteps:
* Only positive matrices are used, guaranteeing that f and g remain
positive.
* An additional jac variable is updated to track sign changes during
matrix computation.
* There is (so far) no proof that this algorithm terminates within
reasonable amount of time for every input, but experimentally it
appears to almost always need less than 900 iterations. To account
for that, only a bounded number of iterations is performed (1500),
after which failure is returned. In VERIFY mode a lower iteration
count is used to make sure that callers exercise their fallback.
* The algorithm converges to f=g=gcd(f0,g0) rather than g=0. To keep
this test simple, the end condition is f=1, which won't be reached
if started with non-coprime or g=0 inputs. Because of that we only
support coprime non-zero inputs.
e4330341bd648e93b60fe70c631e311a98bce549 ci: Shutdown wineserver whenever CI script exits (Tim Ruffing)
9a5a611a21fcdf7bf2dab30964cd0208d8cdf444 build: Suppress stupid MSVC linker warning (Tim Ruffing)
739c53b19a22bd8cd251e25ea286089664a2f0eb examples: Extend sig examples by call that uses static context (Tim Ruffing)
914276e4d27a5f21407744d8016b6d0789e676b1 build: Add SECP256K1_API_VAR to fix importing variables from DLLs (Tim Ruffing)
Pull request description:
... and more Windows fixes, please see the individual commits.
The fixed issues were discovered in https://github.com/bitcoin-core/secp256k1/pull/1198.
ACKs for top commit:
sipa:
utACK e4330341bd648e93b60fe70c631e311a98bce549
hebasto:
ACK e4330341bd648e93b60fe70c631e311a98bce549, tested on Windows using [CMake](https://github.com/bitcoin-core/secp256k1/pull/1113) (which means that the 3rd commit is reviewed only, but not tested). FWIW, `LNK4217` warnings have been indeed observed.
Tree-SHA512: ce7845b106190cdc517988c30aaf2cc9f1d6da22904dfc5cb6bf4ee05f063929dc8b3038479e703b6cebac79d1c21d0c84560344d2478cb1c1740087383f40e3
e089eecc1e54551287b12539d2211da631a6ec5c group: Further simply gej_add_ge (Tim Ruffing)
ac71020ebe052901000e5efa7a59aad77ecfc1a0 group: Save a normalize_to_zero in gej_add_ge (Tim Ruffing)
Pull request description:
As discovered by sipa in #1033.
See commit message for reasoning but note that the infinity handling will be replaced in the second commit again.
ACKs for top commit:
sipa:
ACK e089eecc1e54551287b12539d2211da631a6ec5c
apoelstra:
ACK e089eecc1e54551287b12539d2211da631a6ec5c
Tree-SHA512: fb1b5742e73dd8b2172b4d3e2852490cfd626e8673b72274d281fa34b04e9368a186895fb9cd232429c22b14011df136f4c09bdc7332beef2b3657f7f2798d66
Besides improving the examples, this makes sure that the examples
import a variable (instead of a function), namely the static context,
from the library. This is helpful when testing MSVC builds, because
the MSVC linker tends to be awkward when importing variables.
This fixes a build issue with MSVC. While MSVC imports *functions*
from DLLs automatically when building a consumer of the DLL, it does
not import *variables* automatically. In these cases, we need an
explicit __declspec(dllimport).
This commit simply changes our logic to what the libtool manual
suggests, which has a very comprehensive writeup on the topic. Note
that in particular, this solution is carefully designed not to break
static linking. However, as described in the libtool manual,
statically linking the library with MSVC will output warning LNK4217.
This is still the best solution overall, because the warning is
merely a cosmetic issue.
8c7e0fc1de048be98a1f1bc75557671afc14beaa build: Add -Wreserved-identifier supported by clang (Tim Ruffing)
Pull request description:
This warns on certain identifiers reserved by the C standard, namely
* identifiers that begin with an underscore followed by an uppercase letter, and
* identifiers in the global namespace that begin with an underscore.
We had used such identifiers in the past for macros in include guards, and we should make sure that we don't reintroduce such identifiers going forward.
Note that C reserves more identifiers for "future library directions", e.g., identifiers that begin with "str" followed by a lowercase letter. But even the C standards committee has decided that this is somewhat silly and adopted a proposal [1] for C23 that removes the restriction that programs using these identifiers have UB. Instead, these identifiers are now "potentially reserved", which is not a normative restriction but simply an informative warning that the identifiers may become fully reserved in the future.
[1] https://www.open-std.org/jtc1/sc22/WG14/www/docs/n2625.pdf
ACKs for top commit:
sipa:
utACK 8c7e0fc1de048be98a1f1bc75557671afc14beaa
jonasnick:
tested ACK 8c7e0fc1de048be98a1f1bc75557671afc14beaa
Tree-SHA512: da0c5f1e36cffad2ab2f0b8055c8b3cb56e904d8bfea5a9eed9d6fa984359217b3ef3b9232bfb455cf4071c04a6c2a077e26d2a15b20d1eabc99b1fc61d2025c
This warns on certain identifiers reserved by the C standard, namely
* identifiers that begin with an underscore followed by an uppercase
letter, and
* identifiers in the global namespace that begin with an underscore.
We had used such identifiers in the past for macros in include guards,
and we should make sure that we don't reintroduce such identifiers
going forward.
Note that C reserves more identifiers for "future library directions",
e.g., identifiers that begin with "str" followed by a lowercase letter.
But even the C standards committee has decided that this is somewhat
silly and adopted a proposal [1] for C23 that removes the restriction
that programs using these identifiers have UB. Instead, these
identifiers are now "potentially reserved", which is not a normative
restriction but simply an informative warning that the identifiers
may become fully reserved in the future.
[1] https://www.open-std.org/jtc1/sc22/WG14/www/docs/n2625.pdf
9b60e3148d8c19562c8c3805bd0cdc55933e912c ci: Do not set git's `user.{email,name}` config options (Hennadii Stepanov)
Pull request description:
A cleanup after https://github.com/bitcoin-core/secp256k1/pull/1199.
git's `user.{email,name}` config options have been no longer required since 0ecf3188515e46b4da5580b4b9805d2cb927eb91.
ACKs for top commit:
real-or-random:
utACK 9b60e3148d8c19562c8c3805bd0cdc55933e912c
Tree-SHA512: 04f737b0549a91ca992cd1410420e041549a07869eeef068e08971781ea8a4c88a2486e789df36a5ad370ccbbf5d9f7e49ab5f7c1d01faef358ffc4863aaf8e4
ef39721cccec344983f09180bcf9c443d491f7cb Do not link `bench` and `ctime_tests` to `COMMON_LIB` (Hennadii Stepanov)
Pull request description:
The `bench` and `ctime_tests` binaries are users of the library, they should only be linked to the library, not the objects it was built from.
ACKs for top commit:
sipa:
utACK ef39721cccec344983f09180bcf9c443d491f7cb
real-or-random:
utACK ef39721cccec344983f09180bcf9c443d491f7cb
Tree-SHA512: 8bf8330adcce9bf6b21aceacf86e6aff7594762ab68b09257cfe2904fa0ce827377d5a13c0bed5acde74a2b420bb49460657c66d0068ecbe36dc162140876be4
c2415866c7a6769cb29e3db6c5312c1255b37083 ci: Don't fetch git history (Tim Ruffing)
0ecf3188515e46b4da5580b4b9805d2cb927eb91 ci: Use remote pull/merge ref instead of local git merge (Tim Ruffing)
Pull request description:
This steals two recent CI improvements from bitcoin/bitcoin. See individual commit messages.
ACKs for top commit:
sipa:
utACK c2415866c7a6769cb29e3db6c5312c1255b37083
Tree-SHA512: 966130f45767c6bee8bc041d7e90a3166591a54c7cfccdcf4dff99aa4f6ccc2d02544fa7dca9fd020241349775da3cbd9bdbb041fcdd32de7426efd9dcc9c7f8
9b7d18669dc2410bde7690d9b04d90b3dc3e25ce Drop no longer used Autoheader macros (Hennadii Stepanov)
Pull request description:
A cleanup after #1178.
ACKs for top commit:
kevkevinpal:
utACK [9b7d186](9b7d18669d)
sipa:
utACK 9b7d18669dc2410bde7690d9b04d90b3dc3e25ce
real-or-random:
utACK 9b7d18669dc2410bde7690d9b04d90b3dc3e25ce
Tree-SHA512: ce95547683580bde46a55a6adc3dc46aca02fc86b0300ce0598d62ed47f1d77c4fa9ffd38dcda858655cefa6c940260d05f42cca294e7f3e7a46394b117c9ce9
The merge strategy on the remote may be different than the local one.
This may cause local merges to be different or fail completely. Fix this
by using the result of the remote merge.
(copied from bitcoin/bitcoin@fad7281d78)
eb6bebaee3947f8bca46816fa6bf6182085f1b56 scalar: restrict split_lambda args, improve doc and VERIFY_CHECKs (Jonas Nick)
7f49aa7f2dca595ac8b58d0268dc46a9dfff1e38 ci: add test job with -DVERIFY (Jonas Nick)
620ba3d74bed3095cec7cd8877c8ce14cbf5e329 benchmarks: fix bench_scalar_split (Jonas Nick)
Pull request description:
scalar_split_lambda requires that the input pointer is different to both output
pointers. Without this fix, the internal benchmarks crash when compiled with
-DVERIFY.
This was introduced in commit 362bb25608 (which
requires configuring with --enable-endomorphism to exhibit the crash).
I tested that the new CI job would have caught this bug.
ACKs for top commit:
sipa:
utACK eb6bebaee3947f8bca46816fa6bf6182085f1b56
real-or-random:
utACK eb6bebaee3947f8bca46816fa6bf6182085f1b56
Tree-SHA512: c810545aefb01561ddb77b53618fa7acbb156ec13ab809c00523d4758492cafab1dfa01b6ebfb6195a3803bb49b16e63e8b0efcd1abb76ecefdb0476c3e483a3
