This was detected while running the tests with the `-Wconditional-uninitialized` flag

```
./autogen.sh
CC=clang CFLAGS="-Wconditional-uninitialized" ./configure
make check
```

The resulting warning is a false positive, but setting the value to -1
ensures that the CHECK below will fail if recid is never written to.

This compiler flag is available for clang but not gcc.

Test plan:

```
autogen.sh
./configure
make check
CC=clang ./configure
make check
```

If a variable is used uninitialized, the warning should look something
like:
```
  CC       src/tests-tests.o
src/tests.c:4336:15: warning: variable 'recid' may be uninitialized when used here [-Wconditional-uninitialized]
        CHECK(recid >= 0 && recid < 4);
              ^~~~~
./src/util.h:54:18: note: expanded from macro 'CHECK'
    if (EXPECT(!(cond), 0)) { \
                 ^~~~
./src/util.h:41:39: note: expanded from macro 'EXPECT'
                                      ^
src/tests.c:4327:14: note: initialize the variable 'recid' to silence this warning
    int recid;
             ^
              = 0
1 warning generated.
```

These functions count the number of trailing zeroes in non-zero integers.

Refactored by: Pieter Wuille <[email protected]>

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.

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 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 temporarily duplicates the inversion code across the 4x64 and 8x32
implementations. Those implementations will be replaced in a later commit.

This temporarily duplicates the inversion code across the 5x52 and 10x26
implementations. Those implementations will be replaced in a next commit.

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.

No exposed functions rely on Jacobi symbol computation anymore. Remove it; it can always
be brough back later if needed.

The whole "num" API and its libgmp-based implementation are now unused. Remove them.

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.

…bits

This only seems to be a win on 64-bit platforms, so only do it there.

Refactored by: Pieter Wuille <[email protected]>

…te_fg_var

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 <[email protected]>

24ad04f Make scalar_inverse{,_var} benchmark scale with SECP256K1_BENCH_ITERS (Pieter Wuille)
ebc1af7 Optimization: track f,g limb count and pass to new variable-time update_fg_var (Peter Dettman)
b306935 Optimization: use formulas instead of lookup tables for cancelling g bits (Peter Dettman)
9164a1b Optimization: special-case zero modulus limbs in modinv64 (Pieter Wuille)
1f233b3 Remove num/gmp support (Pieter Wuille)
20448b8 Remove unused Jacobi symbol support (Pieter Wuille)
5437e7b Remove unused scalar_sqr (Pieter Wuille)
aa9cc52 Improve field/scalar inverse tests (Pieter Wuille)
1e0e885 Make field/scalar code use the new modinv modules for inverses (Pieter Wuille)
436281a Move secp256k1_fe_inverse{_var} to per-impl files (Pieter Wuille)
aa404d5 Move secp256k1_scalar_{inverse{_var},is_even} to per-impl files (Pieter Wuille)
08d5496 Improve bounds checks in modinv modules (Pieter Wuille)
151aac0 Add tests for modinv modules (Pieter Wuille)
d8a92fc Add extensive comments on the safegcd algorithm and implementation (Pieter Wuille)
8e415ac Add safegcd based modular inverse modules (Peter Dettman)
de0a643 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 24ad04f
  sanket1729:
    ACK 24ad04f
  real-or-random:
    ACK 24ad04f careful code review, some testing

Tree-SHA512: 732fe29315965e43ec9a10ee8c71eceeb983c43fe443da9dc5380a5a11b5e40b06e98d6abf67b773b1de74571fd2014973c6376f3a0caeac85e0cf163ba2144b

…o the project

4504472 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 4504472
  jonasnick:
    ACK 4504472

Tree-SHA512: e35c202835a82dab5fe9f2f75e7752e70b15d5d2ee7485790749f145b35e8e995c4978b4015c726387c24248a7efb636d28791fe882581a144a0ddfb27e14075

23c3fb6 Make argument of fe_normalizes_to_zero{_var} const (Pieter Wuille)

Pull request description:

ACKs for top commit:
  real-or-random:
    ACK 23c3fb6 diff looks good
  jonasnick:
    ACK 23c3fb6

Tree-SHA512: a51894a9e59851dc4854e92e4200ef6d12a11f6785b903c23585cfff5ef8d369216f4121260fe8789d46d3e215f3c2baa42decae99ab9328e8081f5274e67fab

99a1cfe print warnings for conditional-uninitialized (PiRK)
3d2cf6c initialize variable in tests (PiRK)

Pull request description:

ACKs for top commit:
  real-or-random:
    ACK 99a1cfe code inspection
  jonasnick:
    ACK 99a1cfe

Tree-SHA512: 72f92f51c44210ab54f166920f540525db0e3d1f19a2fa56e4a6d157a38a582f9dc649d919cf3278482c9fd723021b07759284a8fccbc574b62a22aac0facf51

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).

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.

set ECMULT_GEN_PREC_BITS to the "auto" value of 4 in basic_config.h, so libsecp can be used without autoconf

0706796 add ECMULT_GEN_PREC_BITS to basic_config.h (Aaron Voisine)
a3aa262 gen_context: Don't include basic-config.h (Tim Ruffing)

Pull request description:

ACKs for top commit:
  sipa:
    utACK 0706796
  jonasnick:
    ACK 0706796

Tree-SHA512: 4889b483a33ac54f6038a5a5db1ccd225b03e752c5724243db7345389372ecf043433fd5441199043fc8b74c963f13cbf6a7c8068367f9a105e2be93392f24e9

…-time

be0609f Add unit tests for edge cases with delta=1/2 variant of divsteps (Pieter Wuille)
cd393ce Optimization: only do 59 hddivsteps per iteration instead of 62 (Pieter Wuille)
277b224 Use modified divsteps with initial delta=1/2 for constant-time (Pieter Wuille)
376ca36 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 be0609f
  sanket1729:
    crACK be0609f
  real-or-random:
    ACK be0609f careful code review and some testing

Tree-SHA512: 2f8f400ba3ac8dbd08622d564c3b3e5ff30768bd0eb559f2c4279c6c813e17cdde71b1c16f05742c5657b5238b4d592b48306f9f47d7dbdb57907e58dd99b47a

The same file says that the illegal callback will only triger for violations
explicitly mentioned, which is not true without this commit because we often
don't mention that an argument is not allowed to be NULL.

This avoids building without it and makes it safer to use a custom
 building environment.  Test harnesses need to #include secp256k1.c
 first now.

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."

ae9e648 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 ae9e648
  real-or-random:
    ACK ae9e648

Tree-SHA512: 65ccc15c18f111ba926db1bb25f06c2beb2997c6f42c6d3ebc371ca84f4b5918379efd25c30556cedfd2e4275758bd79d733e80a11159c6ec013dd4707a683ad

ed5a199 tests: fopen /dev/urandom in binary mode (Tim Ruffing)
4dc37bf Add mingw32-w64/wine CI build (Pieter Wuille)

Pull request description:

ACKs for top commit:
  real-or-random:
    ACK ed5a199
  jonasnick:
    utACK ed5a199

Tree-SHA512: 45afc394e3a200f7c020426a66f78df8d12827b9dc91bb04dc1708c3ad5cdc4e7d20554d6d5c046d288552f4e722d4fe8a0f3234b662e7351a4d27aaaeb0d5c0

…ilers.

GCC 4.9.2, at least, emits "warning: missing braces around initializer"
 without this.

… on old compilers

99e2d5b 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 99e2d5b
  jonasnick:
    utACK 99e2d5b

Tree-SHA512: f3f9cfcd62830d7accca74dfce40abb091dec0990a66bad5d2a9599f2533121d8d1422499d511512bfb8d7c57da96e29e012dbc210e2e97ad55ad18de0869735

Fixes #931.

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.

c848352 Makefile.am: Don't pass a variable twice (Tim Ruffing)
2161f31 Makefile.am: Honor config when building gen_context (Tim Ruffing)
99f47c2 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 c848352   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 c848352. I verified that building still works on ARM64, but without asm of course.

Tree-SHA512: fc5500688b2aecc4238e21c32f65559bcbfd1e83d1ae4d2c8e15573e94613667731064d8b5f2b9e4209016d88118263802ff4b9a73c1f37c224ccf2a4a1d6536

Previous behaviour would not initialize r->x and r->y values in the case where infinity is passed in.

Previous behaviour would not initialize r->x and r->y values in the case where infinity is passed in.

Previous behaviour would not initialize r->y values in the case where infinity is passed in.
Furthermore, the previous behaviour wouldn't initialize anything in the case where all inputs were infinity.

…e/" dir

added relative paths to header files imported from src directory

added include guards for contrib/ files when referring to secp256k1.h

3c90bdd 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 3c90bdd
  real-or-random:
    ACK 3c90bdd code looks good and even the tests compile fine now without `-I` args

Tree-SHA512: 94d212718c6f4901f1c310aff504b7afedda91268143ffe1b45e9883cd517c0599e40ac798a51b54d66cd31646fe8cb1a489f1776612cfb5963654f4a1cee757

…r initialize all fields of their outputs.

14c9739 tests: Improve secp256k1_ge_set_all_gej_var for some infinity inputs (Tim Ruffing)
4a19668 tests: Test secp256k1_ge_set_all_gej_var for all infinity inputs (Tim Ruffing)
45b6468 Have secp256k1_ge_set_all_gej_var initialize all fields. Previous behaviour would not initialize r->y values in the case where infinity is passed in. Furthermore, the previous behaviour wouldn't initialize anything in the case where all inputs were infinity. (Russell O'Connor)
31c0f6d Have secp256k1_gej_double_var initialize all fields. Previous behaviour would not initialize r->x and r->y values in the case where infinity is passed in. (Russell O'Connor)
dd6c3de Have secp256k1_ge_set_gej_var initialize all fields. Previous behaviour would not initialize r->x and r->y values in the case where infinity is passed in. (Russell O'Connor)

Pull request description:

  Previous behaviour would not initialize `r->x` and `r->y` values in the case where infinity is passed in.

ACKs for top commit:
  gmaxwell:
    ACK 14c9739
  sipa:
    utACK 14c9739
  real-or-random:
    ACK 14c9739

Tree-SHA512: 2e779b767f02e348af4bbc62aa9871c3d1d29e61a6c643c879c49f2de27556a3588850acd2f7c7483790677597d01064025e14befdbf29e783f57996fe4430f9

They were added in #925 and deserve a comment.

… NULL

0881633 secp256k1.h: clarify that by default arguments must be != NULL (Jonas Nick)

Pull request description:

  The same file says that the illegal callback will only triger for violations
  explicitly mentioned, which is not true without this commit because we often
  don't mention that an argument is not allowed to be NULL.

  This line is extracted from #783 in the hope that it gets merged faster because other PRs depend on it.

ACKs for top commit:
  gmaxwell:
    ACK 0881633
  real-or-random:
    ACK 0881633

Tree-SHA512: ecdc6954a1c21c333da5b03db51f50a0e53984aaef69cc697adaddc96b276da23e342037f476d21742632f6ec02bfa0574f837a5b5791f5985f4c355037176fa

…ards

22a9ea1 contrib: Explain explicit header guards (Tim Ruffing)

Pull request description:

  They were added in #925 and deserve a comment.

ACKs for top commit:
  gmaxwell:
    ACK 22a9ea1
  sipa:
    ACK 22a9ea1

Tree-SHA512: 832e28d71857d52912dae7e6c0e08a3183bb788996bb2470616c6fbbac6ba601cc74bb51a4c908aec7df9ae4f4cbf2cbb1b451cefde1b5a7359dc93299840278

6eceec6 add `secp256k1_xonly_pubkey_cmp` method (Andrew Poelstra)
0d9561a add `secp256k1_ec_pubkey_cmp` method (Andrew Poelstra)

Pull request description:

ACKs for top commit:
  elichai:
    Code review ACK 6eceec6
  jonasnick:
    ACK 6eceec6
  real-or-random:
    ACK 6eceec6

Tree-SHA512: f95cbf65f16c88a4adfa1ea7cc6ddabab14baa3b68fa069e78e6faad4852cdbfaea42ee72590d2e0b8f3159cf9b37969511550eb6b2d256b101e2147711cc817

8bbad7a Add asm build to ARM32 CI (Pieter Wuille)
7d65ed5 Add ARM32/ARM64 CI (Pieter Wuille)

Pull request description:

ACKs for top commit:
  real-or-random:
    ACK 8bbad7a CI output looks fine
  jonasnick:
    ACK 8bbad7a

Tree-SHA512: 090a52af6914cf9fb659f9626a8224d82c8da81f6e628b7300e34851e198d8299dfd25789c0f1d6f2c79f58b5413be498f9fba43bc50238480fe6524b640538a

This removes the ununsed `obj` directory. It also suggests in the README
to create the "coverage" files in a separate directory and adds the
coverage files to .gitignore.

readme: Improve instructions for coverage reports

09b3bb8 Clean up git tree (Tim Ruffing)

Pull request description:

ACKs for top commit:
  jonasnick:
    ACK 09b3bb8

Tree-SHA512: 70db146f4475e9618ecd68cf678d09a351e8da6c4fd4aa937c3f2fa30e3f6a9480ff24ac6301785fc2463bb5f8ff974091f8e9292ae7674ca9632b449a7034d5

02dcea1 ci: Make test iterations configurable and tweak for sanitizer builds (Tim Ruffing)
489ff5c tests: Treat empty SECP2561_TEST_ITERS as if it was unset (Tim Ruffing)
fcfcb97 ci: Simplify to use generic wrapper for QEMU, Valgrind, etc (Tim Ruffing)
de4157f ci: Run ASan/LSan and reorganize sanitizer and Valgrind jobs (Tim Ruffing)

Pull request description:

ACKs for top commit:
  sipa:
    utACK 02dcea1
  jonasnick:
    ACK 02dcea1 spot-checked ci output, checked that when `valgrind ./tests` crashes then `LOG_COMPILER=valgrind make check` also crashes.

Tree-SHA512: 5f4a2fe186eca0b4ca29190eb18e20d0804934df614cdc8eb8cf0145ff36ded43194325572bb77eaaeba85c369f6effe69b7bdf7df97ba418d72cf36c9749a8c

This line should have been added in c7f754f.

This mistake caused some i686 builds to fail when the PR was not
rebased, see https://cirrus-ci.com/build/5156197872435200.

a35fdd3 ci: Run PRs on merge result even for i686 (Tim Ruffing)

Pull request description:

ACKs for top commit:
  jonasnick:
    ACK a35fdd3

Tree-SHA512: 9b800b1136da2ecdaff7fcffaac92d91623c682abed1fa5c2a1fe4384f20d2ff1079786f7216c39f58f5dd025e4ed32237e7aff29f7658a74554f0c298e9148e

8f879c2 Fix array size in bench_ecmult (Jonas Nick)
2fe1b50 Add ecmult_gen, ecmult_const and ecmult to benchmark (Jonas Nick)
593e6ba Clean up ecmult_bench to make space for more benchmarks (Jonas Nick)

Pull request description:

  I was trying to determine the impact of ecmult_gen in schnorrsig signing and noticed that there is no way to bench this right now. The new benchmarks look like this:
  ```
  $ ./bench_ecmult
  ecmult_gen: min 20.9us / avg 21.2us / max 21.7us
  ecmult_const: min 63.9us / avg 64.3us / max 64.8us
  ecmult 1: min 49.4us / avg 49.7us / max 50.3us
  ecmult 1g: min 39.8us / avg 40.0us / max 40.3us
  ecmult 2g: min 27.2us / avg 27.3us / max 27.8us
  ecmult_multi 1g: min 39.8us / avg 40.0us / max 40.2us
  ecmult_multi 2g: min 27.2us / avg 27.4us / max 27.7us
  ecmult_multi 3g: min 22.8us / avg 22.9us / max 23.1us
  ecmult_multi 4g: min 20.6us / avg 20.8us / max 21.1us
  ecmult_multi 5g: min 19.3us / avg 19.5us / max 19.7us
  ```

  (Turns out ecmult_gen is 37% of the 55.8us that schnorrsig sign takes)

ACKs for top commit:
  real-or-random:
    ACK 8f879c2
  elichai:
    tACK 8f879c2

Tree-SHA512: 8a739f5de1e2c0467c8d1c3ceeaf453b396a470ea0e8e5bef15fe1b32f3f9633b6b1c7e2ce1d94d736cf3e9adecd8f4f983ad4ba37450cd5991767f1a95db85c

c58c4ea ci: Add ppc64le build (Tim Ruffing)

Pull request description:

ACKs for top commit:
  sipa:
    ACK c58c4ea
  jonasnick:
    ACK c58c4ea

Tree-SHA512: 8f58783d07b34241619051c8375749699b1bd447de56541b3aea3d2e9546c6eb22fbcae55ad57bff614b8c3455933d74031162d00e5eabe6d1d55d56b4aaca16

34388af 98e0358 d0bd269 185a6af 6c52ae8 6939487 1e78c18 202a030 bf0ac46 399722a 3dc8c07 50f3367 7973576 1758a92 ' into temp-merge-950

This reverts commit 20448b8.

The removed functions secp256k1_ge_set_xquad and secp256k1_fe_is_quad_var
are required for some modules in secp256k1-zkp.

Previously the ECDSA signature had an overflowing s value, which after the sync
with upstream results in a failing VERIFY_CHECK in the inversion function.
However, normally parsed signatures shouldn't contain overflowing s values.

Also add a specific test that fails adaptor sig deserialization because with the
correct size buffer that's not guaranteed anymore with the existing test.