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
1. using xonly_pubkeys in MuSig for input public keys and the combined
pk. For that to work we need to store whether the MuSig aggregated point
has an even y in the session, may need to negate each signers secret
key and may need to negate each signers public key in
musig_partial_sig_verify.
2. using a tagged hash for the message hash.
3. use !fe_is_odd in place of fe_is_quad_var
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.
If the scratch space is too small when calling
`secp256k1_ecmult_strauss_batch()`, the `state.pre_a` allocation will
fail and the pointer will be `NULL`. This causes `state.pre_a_lam` to be
computed from the `NULL` pointer.
It is also possible that the first allocation to fail is for `state.ps`,
which will cause the failure to occur when in
`secp256k1_ecmult_strauss_wnaf()`.
The issue has been detected by UBSAN using Clang 10:
```
CC=clang \
CFLAGS="-fsanitize=undefined -fno-omit-frame-pointer" \
LDFLAGS="-fsanitize=undefined -fno-omit-frame-pointer" \
../configure
UBSAN_OPTIONS=print_stacktrace=1:halt_on_error=1 make check
```
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.
c582abade1 Consistency improvements to the comments (Pieter Wuille)
63c6b71616 Reorder comments/function around scalar_split_lambda (Pieter Wuille)
2edc514c90 WNAF of lambda_split output has max size 129 (Pieter Wuille)
4232e5b7da Rip out non-endomorphism code (Pieter Wuille)
ebad8414b0 Check correctness of lambda split without -DVERIFY (Gregory Maxwell)
fe7fc1fda8 Make lambda constant accessible (Pieter Wuille)
9d2f2b44d8 Add tests to exercise lambda split near bounds (Pieter Wuille)
9aca2f7f07 Add secp256k1_split_lambda_verify (Russell O'Connor)
acab934d24 Detailed comments for secp256k1_scalar_split_lambda (Russell O'Connor)
76ed922a5f Increase precision of g1 and g2 (Russell O'Connor)
6173839c90 Switch to our own memcmp function (Tim Ruffing)
Pull request description:
This is a rebased/combined version of the following pull requests/commits with minor changes:
* #825 Switch to our own memcmp function
* Modification: `secp256k1_memcmp_var` is marked static inline
* Modification: also replace `memcmp` with `secp256k1_memcmp_var` in exhaustive tests
* Modification: add reference to GCC bug 95189
* #822 Increase precision of g1 and g2
* Modification: use the new `secp256k1_memcmp_var` function instead of `memcmp` (see https://github.com/bitcoin-core/secp256k1/pull/822#issuecomment-706610361)
* Modification: drop the " Allow secp256k1_split_lambda_verify to pass even in the presence of GCC bug https://gcc.gnu.org/bugzilla/show_bug.cgi?id=95189." commit, as it's dealt with using `secp256k1_memcmp_var`.
* Modification: rename secp256k1_gej_mul_lambda -> secp256k1_ge_mul_lambda
* A new commit that moves the `lambda` constant out of `secp256k1_scalar_split_lambda` and (`_verify`).
* The test commit suggested here: https://github.com/bitcoin-core/secp256k1/pull/822#issuecomment-706610276
* Modification: use the new accessible `secp256k1_const_lambda` instead of duplicating it.
* #826 Rip out non-endomorphism code
* A new commit that reduces the size of the WNAF output to 129, as we now have proof that the split output is always 128 bits or less.
* A new commit to more consistently use input:`k`, integer outputs:`k1`,`k2`, modulo n outputs:`r1`,`r2`
ACKs for top commit:
real-or-random:
ACK c582abade1 code inspection, some tests, verified the new g1/g2 constants
jonasnick:
ACK c582abade1 didn't verify the proof
Tree-SHA512: 323a3ee3884b7ac4fa85c8e7b785111b5c0638d718bc1c805a38963c87411e81a746c98e9a42a3e2197ab34a874544de5cc51326955d1c4d0ea45afd418e819f
The VERIFY macro turns on various paranoid consistency checks, but
the complete functionality should still be tested without it.
This also adds a couple of static test points for extremely small
split inputs/outputs. The existing bounds vectors already check
extremely large outputs.
