7dfceceea6 build: Remove #undef hack for ASM in the precomputation programs (Tim Ruffing)
bb36fe9be0 ci: Test `make precomp` (Tim Ruffing)
d94a37a20c build: Remove CC_FOR_BUILD stuff (Tim Ruffing)
ad63bb4c29 build: Prebuild and distribute ecmult_gen table (Tim Ruffing)
ac49361ed0 prealloc: Get rid of manual memory management for prealloc contexts (Tim Ruffing)
6573c08f65 ecmult_gen: Tidy precomputed file and save space (Tim Ruffing)
5eba83f17c ecmult_gen: Precompute tables for all values of ECMULT_GEN_PREC_BITS (Tim Ruffing)
fdb33dd122 refactor: Make PREC_BITS a parameter of ecmult_gen_build_prec_table (Tim Ruffing)
a4875e30a6 refactor: Move default callbacks to util.h (Tim Ruffing)
4c94c55bce doc: Remove obsolete hint for valgrind stack size (Tim Ruffing)
5106226991 exhaustive_tests: Fix with ecmult_gen table with custom generator (Tim Ruffing)
e1a76530db refactor: Make generator a parameter of ecmult_gen_create_prec_table (Tim Ruffing)
9ad09f6911 refactor: Rename program that generates static ecmult_gen table (Tim Ruffing)
8ae18f1ab3 refactor: Rename file that contains static ecmult_gen table (Tim Ruffing)
00d2fa116e ecmult_gen: Make code consistent with comment (Tim Ruffing)
3b0c2185ea ecmult_gen: Simplify ecmult_gen context after making table static (Tim Ruffing)
e43ba02cfc refactor: Decouple table generation and ecmult_gen context (Tim Ruffing)
22dc2c0a0d ecmult_gen: Move table creation to new file and force static prec (Tim Ruffing)
Pull request description:
This resolves#893, resolves#692 (and also resolvesbitcoin/bitcoin#22854).
- [x] Extract table generation to separate function in separate file (to be used by generation script and exhaustive tests)
- [x] Tidy up
- [x] Remove code that deals with non-static tables
- [x] Make functions that need ecmult_gen not depend on signing context
- [x] Rename stuff to make it fit the new structure and consistent with how we hande verification tables (#956)
- [x] Fix exhaustive tests
- [x] Make table generation function take generator as input
- [x] Overwrite the static tables with a table with custom generator in exhaustive tests
- [x] Overhaul script that generates table files
- [x] Make table generation function take PREC_BITS as input (I have some code already, just not yet in this branch)
- [x] Change generation script to generate three tables (for all three values of ECMULT_GEN_PREC_BITS)
- [x] Ship pre-built tables
- [x] Add pregenerated table file to repo
- [x] Remove generation of table file from build process (like in #956)
- [x] Remove left-over stuff (e.g., detecting a compiler running on the build machine) from build system
- [x] Final cleanups (copyright headers, commit, messages, etc.)
- [ ] (separate PR:) Make sure link-time optimization remove corresponding static tables (and code) when no signing/verifcation function is called
- [ ] (separate PR:) Compile precomputation as a separate object file and link it (https://github.com/bitcoin-core/secp256k1/pull/988#issuecomment-977813538)
- [ ] (separate PR:) Document the backwards-compatible API changes made in this PR and in #956.
- [ ] Maybe deprecate the static context
ACKs for top commit:
sipa:
ACK 7dfceceea6
robot-dreams:
ACK 7dfceceea6 (based on range-diff between 56284c7d44c0ed46e636588bfbf6c403b7dfa6c1 and 7dfceceea6)
Tree-SHA512: 6efb3f36f05efe3b79bbd877881fe1409f71fd6488d24c811b2e77d9f053bed78670dd1dcbb42ad780458a51c4ffa36de9cd6567271b22041dc7a122ceb677c5
This was necessary because we used to cross-compile the library but
compile the precomputation programs for the build host. Now it's no
longer necessary and we can cleanly link even the external ASM
(which was the intent of #935).
On the way, remove an obsolete "-I" parameter.
- Improve Makefile.am for both prebuilt tables files
- On the way, tidy EXTRA_DIST: Move the header files to noinst_HEADERS,
where they conceptually belong, and add missing SECURITY.md to EXTRA_DIST
099bad945e Comment and check a parameter for inf in secp256k1_ecmult_const. (Russell O'Connor)
6c0be857f8 Verify that secp256k1_ge_set_gej_zinv does not operate on infinity. a->x and a->y should not be used if the infinity flag is set. (Russell O'Connor)
Pull request description:
a->x and a->y should not be used if the infinity flag is set.
ACKs for top commit:
robot-dreams:
ACK 099bad945e
real-or-random:
ACK 099bad945e I inspected all call sites, they all ensure that a is not infinity
Tree-SHA512: 495fcfe4ec4cacb3fc64bd5d04ecc67ab34f6b63666c6169d473abfd63c2041bc501a9a60d817566517435b986406ea2b7db3f5806043cecf30e214eba9892e9
592661c22f ci: move test environment variable declaration to .cirrus.yml (siv2r)
dcbe84b841 bench: add --help option to bench. (siv2r)
Pull request description:
Fixes#1005
**Changes:**
- added `--help` option to `bench.c`
- `help()` function prints the help to command-line
- `have_invalid_args()` checks if the user has entered an invalid argument
- moved `secp256k1_bench_iters` and `secp256k1_test_iters` environment variables declaration to `.cirrus.yml`
ACKs for top commit:
sipa:
utACK 592661c22f
real-or-random:
ACK 592661c22f
Tree-SHA512: ebc6a2e6e47b529212efa1c9b75cc79649fca7f42aa75ce46502db24ac94f46b6cef59c828d13265d1fa69187a81c140d1951e7daeb7c8e008a6c1ad75259741
The following functions were created:
1. bench.c: help()
- prints the help to the command-line
2. bench.h: have_invalid_args()
- takes a list of arguments that the user is allowed to enter on the command-line
- returns 1 if the user entered an invalid argument
- returns 0 if all the user entered arguments are valid
This is a backwards-compatible API change: Before this commit, a context
initialized for signing was required to call functions that rely on
ecmult_gen. After this commit, this is no longer necessary because the
static ecmult_gen table is always present. In practice this means that
the corresponding functions will just work instead of calling the
illegal callback when given a context which is not (officially)
initialized for signing.
This is in line with 6815761, which made the analogous change with
respect to ecmult and contexts initialized for signing. But as opposed
to 681571, which removed the ecmult context entirely, we cannot remove
the ecmult_gen context entirely because it is still used for random
blinding. Moreover, since the secp256k1_context_no_precomp context is
const and cannot meaningfully support random blinding, we refrain (for
now) from changing its API, i.e., the illegal callback will still be
called when trying to use ecmult_gen operations with the static
secp256k1_context_no_precomp context.
60bf8890df ecmult: fix definition of STRAUSS_SCRATCH_OBJECTS (Jonas Nick)
Pull request description:
This bug was introduced in 7506e064d7 by adding
an allocation but not updating the constant.
ACKs for top commit:
robot-dreams:
ACK 60bf8890df
real-or-random:
ACK 60bf8890df
Tree-SHA512: d7782fe9bf09fea8cf22304ab13679223a48f4d8b09081e662ea162a68c4e35f6b5820fbe4c6030fabad02a48dfdd02eb9eef22262c1dbbf02955bb92b75aef8
This fixes a compiler warning:
./src/ecdsa_impl.h:312:12: warning: use of bitwise '&' with boolean operands [-Wbitwise-instead-of-logical]
return !secp256k1_scalar_is_zero(sigr) & !secp256k1_scalar_is_zero(sigs);
^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
BIP340's default signing algorithm always requires an aux_rnd argument,
but permits using an all-zero one when no randomness is available.
Make secp256k1_schnorrsig_sign follow this even when aux_rnd32==NULL,
by treating the same as if an all-zero byte array was provided as
input.
2888640132 VERIFY_CHECK precondition for secp256k1_fe_set_int. (Russell O'Connor)
d49011f54c Make _set_fe_int( . , 0 ) set magnitude to 0 (Tim Ruffing)
Pull request description:
Also set the magnitude to 0 when setting the value to 0.
ACKs for top commit:
real-or-random:
ACK 2888640132
jonasnick:
ACK 2888640132
Tree-SHA512: 6ec9b3485380503b11c00f30bfa79f92ba3facb93ee4f3df582b881c4e19fb8ae8b5acd5aeb6326497c290cd0904230d0356f33bd136ca577d2f25616279e090
This makes the semantic of have_flag more clear and fixes a bug
that was introduced in
2fe1b50df1
Add ecmult_gen, ecmult_const and ecmult to benchmark
where the behavior introduced by this commit was already assumed. If
bench_ecmult was called without arguments, have_flag("simple") returned 1 and no
scratch space was allocated which led to very wrong output.
Previously "ecmult{,_multi} xg" meant multiplication with (x - 1) random points
and base point G. Now
- ecmult_{,multi_}xp means multiplication with x random points and
- ecmult_{,multi_}xp_g means multiplication with x random points and G
b4b130678d create csv file from the benchmark output (siv2r)
26a255beb6 Shared benchmark format for command line and CSV outputs (siv2r)
Pull request description:
ACKs for top commit:
real-or-random:
ACK b4b130678d
jonasnick:
ACK b4b130678d
Tree-SHA512: 1eebbdd7701ad21d9647434ff05f23827be217d47870bb05a2fdb12447abc365fc6e56306f344e05d8d2ec1ff5532562131b3876261733e4412117357c5c65f8
1. add `print_output_table_header_row` func to print the table header for benchmark output
2. modify the following benchmarks to include the table header
- bench_ecdh.c
- bench_ecmult.c
- bench_internal.c
- bench_recover.c
- bench_schnorrsig.c
- bench_sign.c
- bench_verify.c
b53e0cd61f Avoid overly-wide multiplications (Peter Dettman)
Pull request description:
Speeds up bench_ecdh, bench_sign, bench_verify relative to master by 5+% at -O3, haswell.
ACKs for top commit:
sipa:
ACK b53e0cd61f
real-or-random:
ACK b53e0cd61f I've inspected the diff and run the tests without asm for a CPU day
Tree-SHA512: 4f79c98371a3dc9da013632210c8db979f910b222291999dfaa0c31849a77eb427361e4ab9206cbfee73c30a8933178784d6cb8e747e8dca6b227eb77fbea2a2
9be7b0f083 Avoid computing out-of-bounds pointer. (Tim Ruffing)
Pull request description:
This is a pedantic case of UB.
Spotted in #879.
ACKs for top commit:
elichai:
ACK 9be7b0f083
practicalswift:
cr ACK 9be7b0f083
sipa:
ACK 9be7b0f083
Tree-SHA512: a9d028c4cdb37ad0d5fcf0d2f678eef732a653d37155a69a20272c6b283c28e083172485d7a37dc4a7c6100b22a6f5b6a92e729239031be228cc511842ee35e8
bc08599e77 Remove OpenSSL testing support (Pieter Wuille)
Pull request description:
This removes the ability to test against OpenSSL, as well as the OpenSSL verification benchmark.
The motivation is that OpenSSL 3 is deprecating part of the API used here (see #869), and I'm not sure it's worth maintaining. We do lose the fact that this is the only test that verifies randomly-generated cases against an independent implementation. On the other hand, there are tons of existing fixed tests now that test all kinds of edge cases already.
ACKs for top commit:
elichai:
tACK bc08599
real-or-random:
ACK bc08599e77
jonasnick:
ACK bc08599e77
Tree-SHA512: 632e6d3cf7bbc5828f5ca1f0f2a92c80bcb681bbcd4320c352b4a86fd521e410c852ccebcfc30fadc8fbf86649267a9e521f53e0f78072a8cd74d8726da28973
1. secp256k1_fe_verify is removed from tests since, it throws an error if VERIFY is not defined during compilation.
(Ex: ./configure --enable-coverage)
2. `secp256k1_fe_from_storage` calls `secp256k1_fe_verify` in the VERIFY build to check for invalid field element.
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
