28e63f7ea7 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 28e63f7ea7
real-or-random:
ACK 28e63f7ea7
Tree-SHA512: d219f836c9258af52389f62c167adb79a0f83f520ede514e286e84f0540d35234322e67d582409c332662db17114da1681419d5d400ed88ad2be66a0f6a06089
756b61d451 readme: Use correct build type in CMake/Windows build instructions (Tim Ruffing)
Pull request description:
ACKs for top commit:
hebasto:
ACK 756b61d451, 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
5d8f53e312 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 5d8f53e312
real-or-random:
ACK 5d8f53e312
Tree-SHA512: ddd6758638fe634866fdaf900224372e2e51cb81ef4d024f169fbc39fff38ef1b29e90e0732877e8910158b82bc428ee9c3a4031882c2850b22ad87cc63ee305
2ef1c9b387 Update overflow check (Russell O'Connor)
Pull request description:
One does not simply check for integer overlow.
ACKs for top commit:
sipa:
ACK 2ef1c9b387
real-or-random:
ACK 2ef1c9b387
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>
ce3cfc78a6 doc: Describe Jacobi calculation in safegcd_implementation.md (Elliott Jin)
6be01036c8 Add secp256k1_fe_is_square_var function (Pieter Wuille)
1de2a01c2b Native jacobi symbol algorithm (Pieter Wuille)
04c6c1b181 Make secp256k1_modinv64_det_check_pow2 support abs val (Pieter Wuille)
5fffb2c7af 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 (f24e122d13) 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 ce3cfc78a6
real-or-random:
reACK ce3cfc78a6 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.
e4330341bd ci: Shutdown wineserver whenever CI script exits (Tim Ruffing)
9a5a611a21 build: Suppress stupid MSVC linker warning (Tim Ruffing)
739c53b19a examples: Extend sig examples by call that uses static context (Tim Ruffing)
914276e4d2 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 e4330341bd
hebasto:
ACK e4330341bd, 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
e089eecc1e group: Further simply gej_add_ge (Tim Ruffing)
ac71020ebe 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 e089eecc1e
apoelstra:
ACK e089eecc1e
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.
8c7e0fc1de 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 8c7e0fc1de
jonasnick:
tested ACK 8c7e0fc1de
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
9b60e3148d 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 0ecf318851.
ACKs for top commit:
real-or-random:
utACK 9b60e3148d
Tree-SHA512: 04f737b0549a91ca992cd1410420e041549a07869eeef068e08971781ea8a4c88a2486e789df36a5ad370ccbbf5d9f7e49ab5f7c1d01faef358ffc4863aaf8e4
ef39721ccc 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 ef39721ccc
real-or-random:
utACK ef39721ccc
Tree-SHA512: 8bf8330adcce9bf6b21aceacf86e6aff7594762ab68b09257cfe2904fa0ce827377d5a13c0bed5acde74a2b420bb49460657c66d0068ecbe36dc162140876be4
c2415866c7 ci: Don't fetch git history (Tim Ruffing)
0ecf318851 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 c2415866c7
Tree-SHA512: 966130f45767c6bee8bc041d7e90a3166591a54c7cfccdcf4dff99aa4f6ccc2d02544fa7dca9fd020241349775da3cbd9bdbb041fcdd32de7426efd9dcc9c7f8
9b7d18669d 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 9b7d18669d
real-or-random:
utACK 9b7d18669d
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)
eb6bebaee3 scalar: restrict split_lambda args, improve doc and VERIFY_CHECKs (Jonas Nick)
7f49aa7f2d ci: add test job with -DVERIFY (Jonas Nick)
620ba3d74b 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 eb6bebaee3
real-or-random:
utACK eb6bebaee3
Tree-SHA512: c810545aefb01561ddb77b53618fa7acbb156ec13ab809c00523d4758492cafab1dfa01b6ebfb6195a3803bb49b16e63e8b0efcd1abb76ecefdb0476c3e483a3
Pieter Wuille
roconnor-blockstream
Tim Ruffing
Jonas Nick
Hennadii Stepanov
Russell O'Connor
Harshil Jani
Elliott Jin
