3c90bdda95aa4e79ff33bfbbbe91872417650ae9 change local lib headers to be relative for those pointing at "include/" dir (William Bright)
Pull request description:
Referencing #924 , this PR splits the two issues brought on to a smaller to digest change. What this does is removes the prefix "include/" when referencing the local library header files.
e.g:
from:
```cpp
#include "include/secp256k1.h"
```
to:
```cpp
#include "secp256k1.h"
```
Rationale besides styling and consistency across other files in the repo, it makes it easier for outside builds to properly locate the headers.
A live example seen here when attempting to build this library within bitcoin repo:
```sh
[ 14%] Building CXX object leveldb/CMakeFiles/leveldb.dir/util/bloom.cc.o
/tmp/bitcoin/src/secp256k1/src/secp256k1.c:7:10: fatal error: include/secp256k1.h: No such file or directory
7 | #include "include/secp256k1.h"
| ^~~~~~~~~~~~~~~~~~~~~
compilation terminated.
make[2]: *** [secp256k1/CMakeFiles/Secp256k1.dir/build.make:76: secp256k1/CMakeFiles/Secp256k1.dir/src/secp256k1.c.o] Error 1
make[1]: *** [CMakeFiles/Makefile2:537: secp256k1/CMakeFiles/Secp256k1.dir/all] Error 2
make[1]: *** Waiting for unfinished jobs....
```
ACKs for top commit:
gmaxwell:
ACK 3c90bdda95aa4e79ff33bfbbbe91872417650ae9
real-or-random:
ACK 3c90bdda95aa4e79ff33bfbbbe91872417650ae9 code looks good and even the tests compile fine now without `-I` args
Tree-SHA512: 94d212718c6f4901f1c310aff504b7afedda91268143ffe1b45e9883cd517c0599e40ac798a51b54d66cd31646fe8cb1a489f1776612cfb5963654f4a1cee757
c8483520c9077905a1dc8b9adb88b6ea2a3bd9ef Makefile.am: Don't pass a variable twice (Tim Ruffing)
2161f31785e66e4e46471208610b5e3e98331849 Makefile.am: Honor config when building gen_context (Tim Ruffing)
99f47c20ec41279075d6b3ae64c9c1a84b40a6f8 gen_context: Don't use external ASM because it complicates the build (Tim Ruffing)
Pull request description:
Obsoletes #935.
ACKs for top commit:
gmaxwell:
ACK c8483520c9077905a1dc8b9adb88b6ea2a3bd9ef looks good and works here. Undefign is kinda yuck, but it is already doing it and it's cleaner than the obvious alternatives.
sipa:
utACK c8483520c9077905a1dc8b9adb88b6ea2a3bd9ef. I verified that building still works on ARM64, but without asm of course.
Tree-SHA512: fc5500688b2aecc4238e21c32f65559bcbfd1e83d1ae4d2c8e15573e94613667731064d8b5f2b9e4209016d88118263802ff4b9a73c1f37c224ccf2a4a1d6536
This passes $(DEFS) (which should literally be "-DHAVE_CONFIG_H") to the
compiler when building gen_context.
This has currently no effect because gen_context.c does not check for
this macro but it's conceivable that it may do so in the future.
99e2d5be0dba938b7701d157cba86252db9eb61c Avoids a missing brace warning in schnorrsig/tests_impl.h on old compilers. (Gregory Maxwell)
Pull request description:
ACKs for top commit:
real-or-random:
ACK 99e2d5be0dba938b7701d157cba86252db9eb61c
jonasnick:
utACK 99e2d5be0dba938b7701d157cba86252db9eb61c
Tree-SHA512: f3f9cfcd62830d7accca74dfce40abb091dec0990a66bad5d2a9599f2533121d8d1422499d511512bfb8d7c57da96e29e012dbc210e2e97ad55ad18de0869735
ae9e648526ceaf7cd97ba4dfe3c105db8e226c35 Define SECP256K1_BUILD in secp256k1.c directly. (Gregory Maxwell)
Pull request description:
This avoids building without it and makes it safer to use a custom
building environment. Test harnesses need to #include secp256k1.c
first now.
Fixes#927
ACKs for top commit:
sipa:
utACK ae9e648526ceaf7cd97ba4dfe3c105db8e226c35
real-or-random:
ACK ae9e648526ceaf7cd97ba4dfe3c105db8e226c35
Tree-SHA512: 65ccc15c18f111ba926db1bb25f06c2beb2997c6f42c6d3ebc371ca84f4b5918379efd25c30556cedfd2e4275758bd79d733e80a11159c6ec013dd4707a683ad
This makes a difference with mingw builds on Wine, where the subsequent
fread() may abort early in the default text mode.
The Microsoft C docs say:
"In text mode, CTRL+Z is interpreted as an EOF character on input."
be0609fd54af95a15b76cea150e6907d581318dd Add unit tests for edge cases with delta=1/2 variant of divsteps (Pieter Wuille)
cd393ce2283e0e7234ea39a15c4931715f4dde1e Optimization: only do 59 hddivsteps per iteration instead of 62 (Pieter Wuille)
277b224b6aba942efbac4a6aae1054035a68d8dd Use modified divsteps with initial delta=1/2 for constant-time (Pieter Wuille)
376ca366db0469f39b93af0af762090986ea75f2 Fix typo in explanation (Pieter Wuille)
Pull request description:
This updates the divsteps-based modular inverse code to use the modified version which starts with delta=1/2. For variable time, the delta=1 variant is still used as it appears to be faster.
See https://github.com/sipa/safegcd-bounds/tree/master/coq and https://medium.com/blockstream/a-formal-proof-of-safegcd-bounds-695e1735a348 for a proof of correctness of this variant.
TODO:
* [x] Update unit tests to include edge cases specific to this variant
I'm still running the Coq proof verification for the 590 bound in non-native mode. It's unclear how long this will take.
ACKs for top commit:
gmaxwell:
ACK be0609fd54af95a15b76cea150e6907d581318dd
sanket1729:
crACK be0609fd54af95a15b76cea150e6907d581318dd
real-or-random:
ACK be0609fd54af95a15b76cea150e6907d581318dd careful code review and some testing
Tree-SHA512: 2f8f400ba3ac8dbd08622d564c3b3e5ff30768bd0eb559f2c4279c6c813e17cdde71b1c16f05742c5657b5238b4d592b48306f9f47d7dbdb57907e58dd99b47a
Before this commit, gen_context.c both included libsecp256k1-config.h
and basic-config.h: The former only to obtain ECMULT_GEN_PREC_BITS
and the latter to obtain a basic working configuration to be able to
use the library.
This was inelegant and confusing: It meant that basic-config.h needs
to #undef all the macros defined in libsecp256k1-config.h. Moreover,
it meant that basic-config.h cannot define ECMULT_GEN_PREC_BITS,
essentially making this file specific for use in gen_context.c.
After this commit, gen_context.c include only libsecp256k1-config.h.
basic-config.h is not necessary anymore for the modules used in
gen_context.c because 79f1f7a made the preprocessor detect all the
relevant config options.
On the way, we remove an unused #define in basic-config.h.
Instead of using eta=-delta, use zeta=-(delta+1/2) to represent
delta. This variant only needs at most 590 iterations for 256-bit
inputs rather than 724 (by convex hull bounds analysis).
4504472269df06b8765b134d41f86619cdcdf8f6 changed import to use brackets <> for openssl as they are not local to the project (William Bright)
Pull request description:
ACKs for top commit:
real-or-random:
ACK 4504472269df06b8765b134d41f86619cdcdf8f6
jonasnick:
ACK 4504472269df06b8765b134d41f86619cdcdf8f6
Tree-SHA512: e35c202835a82dab5fe9f2f75e7752e70b15d5d2ee7485790749f145b35e8e995c4978b4015c726387c24248a7efb636d28791fe882581a144a0ddfb27e14075
24ad04fc064e71abdf973e061c30eb1f3f78db39 Make scalar_inverse{,_var} benchmark scale with SECP256K1_BENCH_ITERS (Pieter Wuille)
ebc1af700f9ec6e96586152b7090a2a6494308c3 Optimization: track f,g limb count and pass to new variable-time update_fg_var (Peter Dettman)
b306935ac12bb24fd931d735b4dfc07f707e7447 Optimization: use formulas instead of lookup tables for cancelling g bits (Peter Dettman)
9164a1b6582e2fc833c760a3403d26b9b0b3b7b3 Optimization: special-case zero modulus limbs in modinv64 (Pieter Wuille)
1f233b3fa05eb29a744487e0682d925055fb0d4c Remove num/gmp support (Pieter Wuille)
20448b8d09a492afcfcae7721033c13a44a776fd Remove unused Jacobi symbol support (Pieter Wuille)
5437e7bdfbffddf69fdf7b4af7e997c78f5dafbf Remove unused scalar_sqr (Pieter Wuille)
aa9cc5218001f14f4312bde1058417d4b755fd11 Improve field/scalar inverse tests (Pieter Wuille)
1e0e885c8ac814c3621d9e43e66d60f25e324e8e Make field/scalar code use the new modinv modules for inverses (Pieter Wuille)
436281afdcb68991395f97338197d208212965e2 Move secp256k1_fe_inverse{_var} to per-impl files (Pieter Wuille)
aa404d53bef21d252a23171381d4bfda6e7e25c6 Move secp256k1_scalar_{inverse{_var},is_even} to per-impl files (Pieter Wuille)
08d54964e51f318ef0cc4ef09d64cfa5ec143c5c Improve bounds checks in modinv modules (Pieter Wuille)
151aac00d31ba5e94800376f6fda4193071168af Add tests for modinv modules (Pieter Wuille)
d8a92fcc4c65cf189ec7bd5298dad8479347c442 Add extensive comments on the safegcd algorithm and implementation (Pieter Wuille)
8e415acba25830da9c23a4dd5531ebfc6b65aae7 Add safegcd based modular inverse modules (Peter Dettman)
de0a643c3dc2c40a447e670cfa1c1683c79c9297 Add secp256k1_ctz{32,64}_var functions (Pieter Wuille)
Pull request description:
This is a rebased and squashed version of #767, adding safegcd-based implementations of constant-time and variable-time modular inverses for scalars and field elements, by Peter Dettman. The PR is organized as follows:
* **Add secp256k1_ctz{32,64}_var functions** Introduction of ctz functions to util.h (which use `__builtin_ctz` on recent GCC and Clang, but fall back to using a software emulation using de Bruijn on other platforms). This isn't used anywhere in this commit, but does include tests.
* **Add safegcd based modular inverse modules** Add Peter Dettman's safegcd code from #767 (without some of his optimizations, which are moved to later commits), turned into separate modules by me.
* **Add extensive comments on the safegcd algorithm and implementation** Add a long description of the algorithm and optimizations to `doc/safegcd_implementation.md`, as well as additional comments to the code itself. It is probably best to review this together with the previous commit (they're separated to keep authorship).
* **Add tests for modinv modules** Adds tests on the modinv interface directly, for arbitrary moduli.
* **Improve bounds checks in modinv modules** Adds a lot of sanity checking to the modinv modules.
* **Move secp256k1_scalar_{inverse{_var},is_even} to per-impl files** A pure refactor to prepare for switching the field and scalar code to modinv.
* **Make field/scalar code use the new modinv modules for inverses** Actually switch over.
* **Add extra modular inverse tests** This adds modular inverse tests through the field/scalar interface, now that those use modinv.
* **Remove unused Jacobi symbol support** No longer needed.
* **Remove num/gmp support** Bye-bye.
* 3 commits with further optimizations.
ACKs for top commit:
gmaxwell:
ACK 24ad04fc064e71abdf973e061c30eb1f3f78db39
sanket1729:
ACK 24ad04fc064e71abdf973e061c30eb1f3f78db39
real-or-random:
ACK 24ad04fc064e71abdf973e061c30eb1f3f78db39 careful code review, some testing
Tree-SHA512: 732fe29315965e43ec9a10ee8c71eceeb983c43fe443da9dc5380a5a11b5e40b06e98d6abf67b773b1de74571fd2014973c6376f3a0caeac85e0cf163ba2144b
The magnitude of the f and g variables generally goes down as the algorithm
progresses. Make use of this by keeping tracking how many limbs are used, and
when the number becomes small enough, make use of this to reduce the complexity
of arithmetic on them.
Refactored by: Pieter Wuille <pieter@wuille.net>
Both the field and scalar modulus can be written in signed{30,62} notation
with one or more zero limbs. Make use of this in the update_de function to
avoid a few wide multiplications when that is the case.
This doesn't appear to be a win in the 32-bit implementation, so only
do it for the 64-bit one.
Add a new run_inverse_tests that replaces all existing field/scalar inverse tests,
and tests a few identities for fixed inputs, small numbers (-999...999), random
inputs (structured and unstructured), as well as comparing with the output of
secp256k1_fe_inv_all_var.
This commit adds functions to verify and compare numbers in signed{30,62} notation,
and uses that to do more extensive bounds checking on various variables in the modinv
code.
This adds tests for the modinv{32,64}_impl.h directly (before the functions are used
inside the field/scalar code). It uses a naive implementation of modular multiplication
and gcds in order to verify the modular inverses themselves.
This adds a long comment explaining the algorithm and implementation choices by building
it up step by step in Python.
Comments in the code are also reworked/added, with references to the long explanation.
9361f360bb04156c7a0fa8f2664680b74d463ed5 ci: Select number of parallel make jobs depending on CI environment (Tim Ruffing)
28eccdf80641f71fada0ee4065c8127468162176 ci: Split output of logs into multiple sections (Tim Ruffing)
c7f754fe4d5e032fd150c4b9b985855e9fcaa521 ci: Run PRs on merge result instead of on the source branch (Tim Ruffing)
b994a8be3cf8ab0fc6a622980a9845bb82cc17db ci: Print information about binaries using "file" (Tim Ruffing)
f24e122d13db7061b1086ddfd21d3a1c5294213b ci: Switch all Linux builds to Debian (Tim Ruffing)
Pull request description:
Best reviewed commit by commit
ACKs for top commit:
jonasnick:
ACK 9361f360bb04156c7a0fa8f2664680b74d463ed5
sipa:
utACK 9361f360bb04156c7a0fa8f2664680b74d463ed5
Tree-SHA512: fc754e8b57dc58058cebbf63a60ca76e08dbaefea1508ea27b7f962bce697c10033da3f57a35f731bc7cf3e210eb00e3b8985ae8b729d7bd83faee085b878b9c
The experiment of using Nix Shell was not really successful. Most
notably, Nix uses a bunch of wrapper scripts around compilers, which
make the build much less "pure". This may be useful but it's exactly
not what we want for CI. In particular, this resulted in gcc being used
for the "clang" builds because a wrapper script redefined the CC env
variable.
This now builds a single docker image (Debian) for all architectures
that we test in CI on Linux.
