b9ebee1490 fix a couple things to make Elements 22's linter happy (Andrew Poelstra)
Pull request description:
In Elements 22 the linter looks for executable files that don't have a properly-formed shebang. For some reason it wants `/usr/bin/env bash` rather than `/bin/bash`, and also one of our source files was erroneously 755.
ACKs for top commit:
real-or-random:
ACK b9ebee1490
Tree-SHA512: 00da8fefd67c1882c6cec39dc81ce67ae3f52f902ddf72545e902b8f5bc7cd7c1249bf71027c530245c403a99c86ffbb61a89bc18c27c5ec975f6f653200766c
72713872a8 Add missing static to secp256k1_schnorrsig_sign_internal (Elichai Turkel)
adec5a1638 Add missing null check for ctx and input keys in the public API (Elichai Turkel)
f4edfc7581 Improve consistency for NULL arguments in the public interface (Elichai Turkel)
20abd52c2e Add tests for pre_g tables. (Russell O'Connor)
6815761cf5 Remove ecmult_context. (Russell O'Connor)
f20dcbbad1 Correct typo. (Russell O'Connor)
16a3cc07e8 Generate ecmult_static_pre_g.h (Russell O'Connor)
8de2d86a06 Bump memory limits in advance of making the ecmult context static. (Russell O'Connor)
5d5c74a057 tests: Rewrite code to circument potential bug in clang (Tim Ruffing)
3d2f492ceb ci: Install libasan6 (instead of 5) after Debian upgrade (Tim Ruffing)
Pull request description:
[bitcoin-core/secp256k1#969]: ci: Fixes after Debian release
[bitcoin-core/secp256k1#956]: Replace ecmult_context with a generated static array.
[bitcoin-core/secp256k1#783]: Make the public API docs more consistent and explicit
[bitcoin-core/secp256k1#976]: `secp256k1_schnorrsig_sign_internal` should be static
This PR can be recreated with `./sync-upstream.sh range 2a3a97c665475bc00d5d60f2f04830202983a631`.
ACKs for top commit:
real-or-random:
ACK 938725c1c9 inspected the diff between the pure output of running the sync script and this PR
Tree-SHA512: 6dd5964563497ced6afe533e4deaa82df76c071b5146a9eb7a5a998187210b5fbf19195d34320b7b2193f6b40d778cf258ad22033d7bc33479e0dc4791aceff9
72713872a8 Add missing static to secp256k1_schnorrsig_sign_internal (Elichai Turkel)
Pull request description:
This function isn't used outside of this module so it should be declared static
ACKs for top commit:
real-or-random:
ACK 72713872a8
jonasnick:
ACK 72713872a8
Tree-SHA512: 6107a2c84c3e11ffd68de22a5288d989a3c71c2ec1ee4827c88f6165fc27ef8339d0f6740928540e8ccd03aff49a2a96149bf698ccebe6d6d8ad6e23e38e8838
adec5a1638 Add missing null check for ctx and input keys in the public API (Elichai Turkel)
f4edfc7581 Improve consistency for NULL arguments in the public interface (Elichai Turkel)
Pull request description:
I went over the public API and added missing explanations on when a pointer can be null and when it cannot,
and added some missing checks for null ctx and null pubkey pointers.
Open questions IMHO:
1. Can `secp256k1_context_create` return NULL? right now it could return null if you replaced the callbacks at compile time to ones that do return(unlike the default ones which never return).
2. Related to the first, should we document that the callbacks should never return? (in the tests we use returning callbacks but we can violate our own API) right now we say the following:
> After this callback returns, anything may happen, including crashing.
Is this enough to document answer `no` for the first question and just saying that if the callback returned then you violated the API so `secp256k1_context_create` can return NULL even though it is promised not to?
Right now we AFAICT we never check if it returns null
Another nit I'm not sure about is wording `(does nothing if NULL)`/`(ignored if NULL)`/`(can be NULL)`
More missing docs:
1. Documenting the `data` argument to the default nonce functions
ACKs for top commit:
ariard:
ACK adec5a16
jonasnick:
ACK adec5a1638
Tree-SHA512: 6fe785776b7e451e9e8cae944987f927b1eb2e2d404dfcb1b0ceb0a30bda4ce16469708920269417e5ada09739723a430e270dea1868fe7d12ccd5699dde5976
This header contains a static array that replaces the ecmult_context pre_g and pre_g_128 tables.
The gen_ecmult_static_pre_g program generates this header file.
clang 7 to 11 (and maybe earlier versions) warn about recid being
potentially unitiliazed in "CHECK(recid >= 0 [...]", which was mitigated
in commit 3d2cf6c5bd by initializing recid
to make clang happy but VG_UNDEF'ing the variable after initializiation
in order to ensure valgrind's memcheck analysis will still be sound and
complain if recid is not actually written to when creating a signature.
However, it turns out that at least for binaries produced by clang 11
(but not clang 7), valgrind complains about a branch on unitialized data
in the recid variable in that line before *and* after the aforementioned
commit. While the complaint after the commit could be spurious (clang
knows that recid is initialized, so it's fine to access it even though
the access is stupid), the complaint before the commit indicates a real
problem: it might be the case that clang is performing a wrong
optimization that leads to a situation where recid is really not
guaranteed to be initialized when it's accessed. As a result, clang
warns about this and generates code that just accesses the variable.
I'm not going to bother with this further because this is fixed in
clang 12 and the problem is just in our test code, not in the tested
code.
This commit rewrites the code in a way that groups the signing together
with the CHECK such that it's very easy to figure out for clang that
recid will be initialized properly. This seems to circument the issue.
8f093be374 musig: use tagged hash for the list of pubkeys to aggregate (Jonas Nick)
a6a768a4bf musig: make key agg test vector more precise (Jonas Nick)
Pull request description:
Top commit has no ACKs.
Tree-SHA512: 5369dc5b4039dd4cda2c50282db2882c088b96e1daa5801240f92be1832ed8f29317fdbfc3cab211707155c284a68dc593967f3141703e2544f6b8dc1553e44d
aeece44599 gen_context: Don't use any ASM (Tim Ruffing)
Pull request description:
See https://github.com/bitcoin/bitcoin/issues/22441 , we need to wait for the testing results there.
ACKs for top commit:
sipa:
utACK aeece44599
jonasnick:
ACK aeece44599
Tree-SHA512: 52ff90f3dedda90124140de1c2c1c065a2f9374930d6b988d35c37f5eeae97f7d557b7ab0cf99d22add5a76ff8a3e06226572e43949e12d1048cb323d1b3d92b
f09497ea3e CI: tweak cirrus.yml to prevent OOM and timeout w sanitizer/valgrind (Jonas Nick)
7226cf215a ecdsa_adaptor: fix too small buffer in tests (Jonas Nick)
b053e853d4 ecdsa_adaptor: fix test case with invalid signature (Jonas Nick)
d27e459861 Revert "Remove unused Jacobi symbol support" (Jonas Nick)
c58c4ea470 ci: Add ppc64le build (Tim Ruffing)
8f879c2887 Fix array size in bench_ecmult (Jonas Nick)
2fe1b50df1 Add ecmult_gen, ecmult_const and ecmult to benchmark (Jonas Nick)
593e6bad9c Clean up ecmult_bench to make space for more benchmarks (Jonas Nick)
a35fdd3478 ci: Run PRs on merge result even for i686 (Tim Ruffing)
02dcea1ad9 ci: Make test iterations configurable and tweak for sanitizer builds (Tim Ruffing)
489ff5c20a tests: Treat empty SECP2561_TEST_ITERS as if it was unset (Tim Ruffing)
fcfcb97e74 ci: Simplify to use generic wrapper for QEMU, Valgrind, etc (Tim Ruffing)
de4157f13a ci: Run ASan/LSan and reorganize sanitizer and Valgrind jobs (Tim Ruffing)
09b3bb8648 Clean up git tree (Tim Ruffing)
8bbad7a18e Add asm build to ARM32 CI (Pieter Wuille)
7d65ed5214 Add ARM32/ARM64 CI (Pieter Wuille)
6eceec6d56 add `secp256k1_xonly_pubkey_cmp` method (Andrew Poelstra)
0d9561ae87 add `secp256k1_ec_pubkey_cmp` method (Andrew Poelstra)
22a9ea154a contrib: Explain explicit header guards (Tim Ruffing)
0881633dfd secp256k1.h: clarify that by default arguments must be != NULL (Jonas Nick)
14c9739a1f tests: Improve secp256k1_ge_set_all_gej_var for some infinity inputs (Tim Ruffing)
4a19668c37 tests: Test secp256k1_ge_set_all_gej_var for all infinity inputs (Tim Ruffing)
45b6468d7e 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)
31c0f6de41 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)
dd6c3de322 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)
3c90bdda95 change local lib headers to be relative for those pointing at "include/" dir (William Bright)
c8483520c9 Makefile.am: Don't pass a variable twice (Tim Ruffing)
2161f31785 Makefile.am: Honor config when building gen_context (Tim Ruffing)
99f47c20ec gen_context: Don't use external ASM because it complicates the build (Tim Ruffing)
99e2d5be0d Avoids a missing brace warning in schnorrsig/tests_impl.h on old compilers. (Gregory Maxwell)
ed5a199bed tests: fopen /dev/urandom in binary mode (Tim Ruffing)
4dc37bf81b Add mingw32-w64/wine CI build (Pieter Wuille)
ae9e648526 Define SECP256K1_BUILD in secp256k1.c directly. (Gregory Maxwell)
be0609fd54 Add unit tests for edge cases with delta=1/2 variant of divsteps (Pieter Wuille)
cd393ce228 Optimization: only do 59 hddivsteps per iteration instead of 62 (Pieter Wuille)
277b224b6a Use modified divsteps with initial delta=1/2 for constant-time (Pieter Wuille)
376ca366db Fix typo in explanation (Pieter Wuille)
07067967ee add ECMULT_GEN_PREC_BITS to basic_config.h (Aaron Voisine)
a3aa2628c7 gen_context: Don't include basic-config.h (Tim Ruffing)
99a1cfec17 print warnings for conditional-uninitialized (PiRK)
3d2cf6c5bd initialize variable in tests (PiRK)
23c3fb629b Make argument of fe_normalizes_to_zero{_var} const (Pieter Wuille)
4504472269 changed import to use brackets <> for openssl as they are not local to the project (William Bright)
24ad04fc06 Make scalar_inverse{,_var} benchmark scale with SECP256K1_BENCH_ITERS (Pieter Wuille)
ebc1af700f Optimization: track f,g limb count and pass to new variable-time update_fg_var (Peter Dettman)
b306935ac1 Optimization: use formulas instead of lookup tables for cancelling g bits (Peter Dettman)
9164a1b658 Optimization: special-case zero modulus limbs in modinv64 (Pieter Wuille)
1f233b3fa0 Remove num/gmp support (Pieter Wuille)
20448b8d09 Remove unused Jacobi symbol support (Pieter Wuille)
5437e7bdfb Remove unused scalar_sqr (Pieter Wuille)
aa9cc52180 Improve field/scalar inverse tests (Pieter Wuille)
1e0e885c8a Make field/scalar code use the new modinv modules for inverses (Pieter Wuille)
436281afdc Move secp256k1_fe_inverse{_var} to per-impl files (Pieter Wuille)
aa404d53be Move secp256k1_scalar_{inverse{_var},is_even} to per-impl files (Pieter Wuille)
08d54964e5 Improve bounds checks in modinv modules (Pieter Wuille)
151aac00d3 Add tests for modinv modules (Pieter Wuille)
d8a92fcc4c Add extensive comments on the safegcd algorithm and implementation (Pieter Wuille)
8e415acba2 Add safegcd based modular inverse modules (Peter Dettman)
de0a643c3d Add secp256k1_ctz{32,64}_var functions (Pieter Wuille)
Pull request description:
ACKs for top commit:
real-or-random:
ACK f09497ea3e
Tree-SHA512: 6cf3e96c5974e9aa17bd649fa7fdd738090ec3ab8c99e144fec397c086a24adc2ace9a5218a3c527989fc07e1d5c669027e4c895caf92d22771c8414b2a9bf35
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.
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.
