b6f649889a Add parens around ROUND_TO_ALIGN's parameter. This makes the macro robust against a hypothetical ROUND_TO_ALIGN(foo ? sizeA : size B) invocation. (Russell O'Connor)
Pull request description:
This makes the macro robust against a hypothetical `ROUND_TO_ALIGN(foo ? sizeA : size B)` invocation.
See also <https://wiki.sei.cmu.edu/confluence/display/c/PRE01-C.+Use+parentheses+within+macros+around+parameter+names>.
ACKs for top commit:
sipa:
ACK b6f649889a. This is the way.
jonasnick:
utACK b6f649889a
real-or-random:
utACK b6f649889a
Tree-SHA512: 6a2685f959e8ae472259e5ea75fe12e8e6213f56f5aec7603a896c294e6a8833caae25c412607d9c9a3125370a7765a3e506127b101a1b87203f95e326f6c6c6
fb390c5299 Remove underscores from header defs. This makes them consistent with other files and avoids reserved identifiers. (Russell O'Connor)
Pull request description:
ACKs for top commit:
real-or-random:
utACK fb390c5299
jonasnick:
ACK fb390c5299
Tree-SHA512: f49da79c0a90d1e82494821e7cf6f61c66bc377a3f37b2d4787ef19d2126e000627bfe4a76aa1c5bfffeb1382054aa824a7e9ab5d73c19d876b0828722c73854
3c15130709 Improve CC_FOR_BUILD detection (Tim Ruffing)
47802a4762 Restructure and tidy configure.ac (Tim Ruffing)
252c19dfc6 Ask brew for valgrind include path (Tim Ruffing)
Pull request description:
See individual commit messages. These are improvements in preparation of the switch to Cirrus CI. (Maybe I'll just open a PR on top of this one.)
The first commit made the difference between successful build https://cirrus-ci.com/task/6740575057608704 and unsuccessful build https://cirrus-ci.com/task/4909571074424832.
I've tested the second commit without cross-compilation and with cross-compilation for android (https://github.com/bitcoin-core/secp256k1/issues/621#issuecomment-495703399)
When working on the autoconf stuff, I noticed two things that I just want to write down here:
- At some point we should update [build-aux/m4/ax_prog_cc_for_build.m4](https://www.gnu.org/software/autoconf-archive/ax_prog_cc_for_build.html). This is outdated, and [there have been a lot of fixes](https://github.com/autoconf-archive/autoconf-archive/pull/207) But the latest version is [broken](https://lists.gnu.org/archive/html/autoconf-archive-maintainers/2020-06/msg00002.html), so now is probably not the time.
- The latest autoconf 2.70 deprecates `AC_PROG_CC_C89`. It's not needed anymore because `AC_PROG_CC` cares about testing for version support. This makes autoconf 2.70 output a warning that we should probably just ignore. We don't want to force users onto 2.70...
ACKs for top commit:
sipa:
utACK 3c15130709
jonasnick:
utACK 3c15130 makes sense (with my very basic understanding of autoconf)
Tree-SHA512: 595b9de316374c2213f1340cddaa22eb3190b01fa99aa6ae26e77804df41e7ecf96a09e03c28e8f8b9fd04e211e4ee2f78f1e5a7995143c84f99d2e16d4f0260
33cb3c2b1f Add secret key extraction from keypair to constant time tests (Elichai Turkel)
36d9dc1e8e Add seckey extraction from keypair to the extrakeys tests (Elichai Turkel)
fc96aa73f5 Add a function to extract the secretkey from a keypair (Elichai Turkel)
Pull request description:
With schnorrsig if you need to tweak the secret key (for BIP32) you must use the keypair API to get compatible secret/public keys which you do by calling `secp256k1_keypair_xonly_tweak_add()`, but after that there's no currently a way to extract the secret key back for storage.
so I added a `secp256k1_keypair_seckey` function to extract the key
ACKs for top commit:
jonasnick:
ACK 33cb3c2b1f
real-or-random:
ACK 33cb3c2b1f code inspection, tests pass
Tree-SHA512: 11212db38c8b87a87e2dc35c4d6993716867b45215b94b20522b1b3164ca63d4c6bf5192a6bff0e9267b333779cc8164844c56669a94e9be72df9ef025ffcfd4
This commits simply uses CC as CC_FOR_BUILD and the same for
corresponding flags if we're not cross-compiling. This has a number of
benefits in this common case:
- It avoids strange cases where very old compilers are used (#768).
- Flags are consistently set for CC and CC_FOR_BUILD.
- ./configure is faster.
- You get compiler x consistently if you set CC=x; we got this wrong
in CI in the past.
./configure warns if a _FOR_BUILD variable is set but ignored because
we're not cross-compiling.
The change exposed that //-style comments are used in gen_context.c,
which is also fixed by this commit.
This commit also reorganizes code in configure.ac to have a cleaner
separation of sections.
Valgrind is typically installed using brew on macOS. This commit
makes ./configure detect this case set the appropriate include
directory (in the same way as we already do for openssl and gmp).
18aadf9d28 docs: fix simple typo, dependecy -> dependency (Tim Gates)
Pull request description:
There is a small typo in src/group_impl.h.
Should read `dependency` rather than `dependecy`.
ACKs for top commit:
real-or-random:
ACK 18aadf9d28
Tree-SHA512: 3529f43bcc87ea8940ecf5af765951f61d97d1efa86fd8abc29e32b600fd449165a94a2fa525bc6b3d9a7d8aa6e691cc4d42033537b196ba166a867e6db7f397
6e85d675aa Rename tweak to tweak32 in public API (Jonas Nick)
f587f04e35 Rename msg32 to msghash32 in ecdsa_sign/verify and add explanation (Jonas Nick)
Pull request description:
This fixes#307 if there's nothing else that's confusing.
ACKs for top commit:
real-or-random:
ACK 6e85d675aa I inspected the diff
Tree-SHA512: 1b0dc9dfffd497058dc39c962a512ed6d7f89218020fef9d2c03aaae1aefbf272b918c4fe6503434b62547714855fe1b8b89f2366f3ae6cde16143207c9e6b86
f4fa8d226a forbid a test iteration of 0 or less (Andrew Poelstra)
0ce4554881 make test count iteration configurable by environment variable (Andrew Poelstra)
Pull request description:
ACKs for top commit:
jonasnick:
ACK f4fa8d226a
real-or-random:
ACK f4fa8d226a
Tree-SHA512: 087771402c8e9536c07446baa7d02da5104d2b691f40c1dd04737329534422d895d3b692f485990d5791af8ccc124305b4f8b19be75e27b6b04cfb2337b28beb
1f4dd03838 Typedef (u)int128_t only when they're not provided by the compiler (Tim Ruffing)
e89278f211 Don't use reserved identifiers memczero and benchmark_verify_t (Tim Ruffing)
Pull request description:
As identified in #829 and #833. Fixes#829.
Since we touch this anyway, this commit additionally makes the
identifiers in the benchmark files a little bit more consistent.
This is necessary before we can merge #833. I preferred a separate PR because it makes it easier to see the results of Travis in #833.
ACKs for top commit:
sipa:
utACK 1f4dd03838
jonasnick:
ACK 1f4dd03838
Tree-SHA512: c0ec92798f3c94f3ef6ac69b3f0f39a39257a32be9d9a068832cece1ebe64c89848b70e44652fc397004b8b240883ac4bc0c8f95abbe4ba4b028de120e6734bf
29a299e373 Run the undefined behaviour sanitizer on Travis (Fabien)
7506e064d7 Prevent arithmetic on NULL pointer if the scratch space is too small (Fabien)
Pull request description:
ACKs for top commit:
sipa:
ACK 29a299e373. Reviewed the code changes and verified that building with these sanitizer flags catches the existing error, as well as a signed integer overflow if introduced.
real-or-random:
ACK 29a299e373 code inspection
jonasnick:
utACK 29a299e373
Tree-SHA512: 4d788f12f3d7b48018e884910adb9b530a05d88f504de83dadeab8a22d75da83c05a1518f7317de5f536c4dd243ea7b347b1eaddb2ca1d804c663e41b85db69d
3734b68200 Configure echo if openssl tests are enabled (Elichai Turkel)
e6692778d3 Modify bitcoin_secp.m4's openssl check to call all the functions that we use in the tests/benchmarks. That way linking will fail if those symbols are missing (Elichai Turkel)
Pull request description:
I added all the openssl functions that we call in `tests.c` and in `bench_verify.c` to the m4 check, that way if any of them are missing it won't enable openssl.
I also modified it a little to prevent a segmentation fault when running that program (not that it really matters for autotools)
This should fix#836
ACKs for top commit:
sipa:
ACK 3734b68200
real-or-random:
ACK 3734b68200
Tree-SHA512: c82aa96a4176061284dfa5fdb87ca874a25aa2e11f75c4ec6d1edebcc8a19e2bc940990f8a5cfa64776fd295b6fd3a140fa2afede29326564504bc8d1a3a6b69
If the user passes invalid flags to _context_create, and the default
illegal callback does not abort the program (which is possible), then we
work with the result of malloc(0), which may be undefined behavior. This
violates the promise that a library function won't crash after the
illegal callback has been called.
This commit fixes this issue by returning NULL early in _context_create
in that case.
8893f42438 Avoids a potentially shortening size_t to int cast in strauss_wnaf_ (Tim Ruffing)
Pull request description:
ACKs for top commit:
sipa:
ACK 8893f42438. `np` and `no` shouldn't ever take on negative values.
jonasnick:
ACK 8893f42438
elichai:
ACK 8893f42438
Tree-SHA512: 431a6b88c8db8c8883b35c9bc03c90e37ecd0b06c7ee01c5d83cca4a7f6fc1f3cfbbaa871a4a23374ce4cc5bcfb9502c7f2e2540f9f9db9535e47e48827b6af6
Run UBSAN with both GCC and Clang, on Linux and macOS.
The `halt_on_error=1` option is required to make the build fail if the
sanitizer finds an issue.
Tim Ruffing
Jonas Nick
Russell O'Connor
Pieter Wuille
Ferdinando M. Ametrano
Elichai Turkel
Dimitris Apostolou
Tim Gates
Andrew Poelstra
Frédéric Chapoton
Fabien