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.
This is only needed for the very oldest of non-POSIX-compatible shells.
Note that this code will also only be executed on macOS, where it'd be
very unlikely to run into such a shell.
Followup to #1019.
See:
https://github.com/koalaman/shellcheck/wiki/SC2006
Co-authored-by: Hennadii Stepanov <32963518+hebasto@users.noreply.github.com>
60bf8890df5360148df921f26d8dc4d667dd5926 ecmult: fix definition of STRAUSS_SCRATCH_OBJECTS (Jonas Nick)
Pull request description:
This bug was introduced in 7506e064d791e529d2e57bb52c156deb33b897ef by adding
an allocation but not updating the constant.
ACKs for top commit:
robot-dreams:
ACK 60bf8890df5360148df921f26d8dc4d667dd5926
real-or-random:
ACK 60bf8890df5360148df921f26d8dc4d667dd5926
Tree-SHA512: d7782fe9bf09fea8cf22304ab13679223a48f4d8b09081e662ea162a68c4e35f6b5820fbe4c6030fabad02a48dfdd02eb9eef22262c1dbbf02955bb92b75aef8
214042a170c860523b7aad2a251b0dbfbaefb235 build: don't append valgrind CPPFLAGS if not installed (fanquake)
Pull request description:
Valgrinds CPPFLAGS, i.e `-I/usr/local/opt/valgrind/include`, are currently added to CPPFLAGS, regardless of whether valgrind is installed. This changes configure so that they are only added if valgrind is available. i.e the output of `brew list --versions valgrind` is non-null.
ACKs for top commit:
real-or-random:
ACK 214042a170c860523b7aad2a251b0dbfbaefb235
hebasto:
ACK 214042a170c860523b7aad2a251b0dbfbaefb235, tested on macOS Big Sur 11.6.1 (20G224, Intel).
Tree-SHA512: 5101636a0a12f1941b01967ca8eab7aa20f44db0d1ef4571a5ad6026bb89494b983465d34d93c8b17a260b695116792991da53d135bc19a3c9e974f5266a90af
812ff5c74745e451f1a9de83b5bd0d0c18c75e5f doc: remove use of 0xa0 "no break space" (fanquake)
Pull request description:
This is miscellaneous, but I don't think these were being used on purpose?
ACKs for top commit:
siv2r:
ACK 812ff5c. The non-breaking space character is replaced with whitespace. Tested with [NBSP highlighter extension](https://marketplace.visualstudio.com/items?itemName=viktorzetterstrom.non-breaking-space-highlighter) on vscode.
real-or-random:
ACK 812ff5c74745e451f1a9de83b5bd0d0c18c75e5f
Tree-SHA512: ccfcc64798f5a5eb0c669eb00f4408ab713e6710d67fd15ee2a4dca0d052e27636d7f0ad312aa94be0cd068c7e7874441aa2e114c4118322d0c764398a4ff695
72de1359e953390dc2f1ab5a59dd1a4057000acb ci: Enable -g if we set CFLAGS manually (Tim Ruffing)
Pull request description:
ACKs for top commit:
jonasnick:
ACK 72de1359e953390dc2f1ab5a59dd1a4057000acb
Tree-SHA512: 0456db0ba53410640653e5d11ee4f328be0657e1e9077aa982ed4fd3eb6e326cfc022ec7ab71fc5c62d7942a20bbc7a5e8000cf5b62201fa1c183853d899ea77
16d132215cab68e57798927301268518bc1c3bf8 refactor: Use (int)&(int) in boolean context to avoid compiler warning (MarcoFalke)
Pull request description:
This one should *really* be only a refactor with the goal to silence static analysis warnings. clang-14 (trunk) recently added one in commit f62d18ff14 and I expect other tools will offer similar warnings.
Follow up to #1006, which was not a refactor.
ACKs for top commit:
real-or-random:
ACK 16d132215cab68e57798927301268518bc1c3bf8
jonasnick:
ACK 16d132215cab68e57798927301268518bc1c3bf8
Tree-SHA512: c465522ea4ddb58b5974c95bc36423c453e6fcf5948cb32114172113b5244209ceaa9418ec86ebe210390ae5509c2f24a42c41a7353de4cfb8fd063b0d5c0e79
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);
^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
3b157c48edb8ab080683232125dc7ec058bbd24c doc: Suggest keys.openpgp.org as keyserver in SECURITY.md (Tim Ruffing)
73a7472cd0335f2aa7eaf2c514e909ce36aba291 doc: Replace apoelstra's GPG key by jonasnick's GPG key (Tim Ruffing)
Pull request description:
I have verified the new key via other secure channels.
This closes#1003 .
We can skip the second commit but I expect https://github.com/bitcoin/bitcoin/pull/23466/ to be merged. If it won't be merged, we could still revert.
ACKs for top commit:
sipa:
ACK 3b157c48edb8ab080683232125dc7ec058bbd24c. I've also verified the key out of band.
jonasnick:
ACK 3b157c48edb8ab080683232125dc7ec058bbd24c
Tree-SHA512: 496f98121f14031bc693aa83bf208b253f79b700b4bca0b629deadc8852f76ef6d69ad90109baa771d7b9f6e4b983e4ed8dca404cf5aceffe9d520d3362b533a
af6abcb3d0097a7f7892fb8b54a4c6363e5c2c7f Make bench support selecting which benchmarks to run (Pieter Wuille)
9f56bdf5b9ba2e22e77c6adaaeb8302398732df3 Merge bench_schnorrsig into bench (Pieter Wuille)
3208557ae1062f7fcce25d5f2c5a29dc34a18895 Merge bench_recover into bench (Pieter Wuille)
855e18d8a809b98a622ab55765792aca132ea640 Merge bench_ecdh into bench (Pieter Wuille)
2a7be678a660d58742b1e767444c0fe75fa22592 Combine bench_sign and bench_verify into single bench (Pieter Wuille)
Pull request description:
This combines `bench_verify`, `bench_sign`, `bench_ecdh`, `bench_recovery`, and `bench_schnorrsig` into a single `bench` binary.
I don't think there is a good reason to have this many binaries, and it complicates build config and CI.
ACKs for top commit:
real-or-random:
ACK af6abcb3d0097a7f7892fb8b54a4c6363e5c2c7f diff looks good, command line options work, valgrind is happy
siv2r:
tACK af6abcb, the command-line options work as expected on my ubuntu machine. The diff looks good.
Tree-SHA512: 39c099b508c70136eaac8a429591b4250a8c22e423fa922d83928ea9273d8f2c1136317427563b28f249c02cf83d1c73ea787c6d26aa88545236241641965705
5324f8942dd322448fae6c9b225ecac2854fa7e2 Make aux_rnd32==NULL behave identical to 0x0000..00. (Pieter Wuille)
Pull request description:
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.
ACKs for top commit:
junderw:
ACK 5324f89
elichai:
ACK 5324f8942dd322448fae6c9b225ecac2854fa7e2
jonasnick:
ACK 5324f8942dd322448fae6c9b225ecac2854fa7e2
real-or-random:
utACK 5324f8942dd322448fae6c9b225ecac2854fa7e2
Tree-SHA512: caa1d5a0eacea3239d8aaace5284eedcd850058bbe759768e626233a010199db6c637618aedccfb51fe94ec8d28f45bc0c441be77e2e12fa2a393b9cc3a5d3ae
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.
2888640132eb64ed30a8a208931f27447c3e0366 VERIFY_CHECK precondition for secp256k1_fe_set_int. (Russell O'Connor)
d49011f54c2b31807158bdf06364f331558cccc7 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 2888640132eb64ed30a8a208931f27447c3e0366
jonasnick:
ACK 2888640132eb64ed30a8a208931f27447c3e0366
Tree-SHA512: 6ec9b3485380503b11c00f30bfa79f92ba3facb93ee4f3df582b881c4e19fb8ae8b5acd5aeb6326497c290cd0904230d0356f33bd136ca577d2f25616279e090
23e2f66726f930ac01d5075106aa16a4073442b4 bench: don't return 1 in have_flag() if argc = 1 (Jonas Nick)
96b1ad2ea9f9d9419e566b95162487c48902c3eb bench_ecmult: improve clarity of output (Jonas Nick)
Pull request description:
Previously "ecmult{,_multi} xg" meant multiplication with (x - 1) random points
and base point G. Now
- xP means multiplication with x random points and
- xP & G means multiplication with x random points and G
ACKs for top commit:
siv2r:
tACK 23e2f66
real-or-random:
ACK 23e2f66726f930ac01d5075106aa16a4073442b4
Tree-SHA512: 0218aaa0baa4c2f92a7b98c97b8cc3b596e3da44d7f38ab4bdd707a4bdb96bb071b953fc6106cd34977a562278e4eaa860a3a7fa64c323c5117945e7a3107162
This makes the semantic of have_flag more clear and fixes a bug
that was introduced in
2fe1b50df16c9f41ea77b151634d734b930eeddd
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
b4b130678db31a7cabc2cde091bc4acbca92b7a3 create csv file from the benchmark output (siv2r)
26a255beb673217c839dcc51790d9a484f9a292d Shared benchmark format for command line and CSV outputs (siv2r)
Pull request description:
ACKs for top commit:
real-or-random:
ACK b4b130678db31a7cabc2cde091bc4acbca92b7a3
jonasnick:
ACK b4b130678db31a7cabc2cde091bc4acbca92b7a3
Tree-SHA512: 1eebbdd7701ad21d9647434ff05f23827be217d47870bb05a2fdb12447abc365fc6e56306f344e05d8d2ec1ff5532562131b3876261733e4412117357c5c65f8
044d95630556dda5492a70af056bc277f0b79ebc Fix G.y parity in sage code (Pieter Wuille)
Pull request description:
I'm not sure if `EllipticCurve.lift_x` has well-defined Y coordinate or not, but at least my current version of Sage computes the wrong G. Fix this.
ACKs for top commit:
real-or-random:
ACK 044d95630556dda5492a70af056bc277f0b79ebc
Tree-SHA512: afb919af29027da2bb3c58628924f9740672d3c347ad39cc663c9c399b1aa8536256fd3fd4e1e54457e38344704d47f281d82488da413f4e6e67e191decc960f
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
b53e0cd61fce0bcef178f317537c91efc9afd04d 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 b53e0cd61fce0bcef178f317537c91efc9afd04d
real-or-random:
ACK b53e0cd61fce0bcef178f317537c91efc9afd04d I've inspected the diff and run the tests without asm for a CPU day
Tree-SHA512: 4f79c98371a3dc9da013632210c8db979f910b222291999dfaa0c31849a77eb427361e4ab9206cbfee73c30a8933178784d6cb8e747e8dca6b227eb77fbea2a2
9be7b0f08340a063d961547b5d2663405f3fc162 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 9be7b0f08340a063d961547b5d2663405f3fc162
practicalswift:
cr ACK 9be7b0f08340a063d961547b5d2663405f3fc162
sipa:
ACK 9be7b0f08340a063d961547b5d2663405f3fc162
Tree-SHA512: a9d028c4cdb37ad0d5fcf0d2f678eef732a653d37155a69a20272c6b283c28e083172485d7a37dc4a7c6100b22a6f5b6a92e729239031be228cc511842ee35e8
bc08599e776aff33c834ef829843ec5f629d1f39 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 bc08599e776aff33c834ef829843ec5f629d1f39
jonasnick:
ACK bc08599e776aff33c834ef829843ec5f629d1f39
Tree-SHA512: 632e6d3cf7bbc5828f5ca1f0f2a92c80bcb681bbcd4320c352b4a86fd521e410c852ccebcfc30fadc8fbf86649267a9e521f53e0f78072a8cd74d8726da28973
189f6bcfef6578b89e21f937b24060f74bd18f00 Fix unused parameter warnings when building without VERIFY (Jonas Nick)
Pull request description:
This commit makes `./configure --enable-coverage && make check` free of warnings.
ACKs for top commit:
practicalswift:
cr ACK 189f6bcfef6578b89e21f937b24060f74bd18f00
elichai:
utACK 189f6bcfef6578b89e21f937b24060f74bd18f00
siv2r:
Tested ACK 189f6bc
Tree-SHA512: 727fe0e40ff61f404780b32dfa4102a58bed9d922e61bd17ddaaf1243b0c06edd9697ff4763b5e92d033e7db3778193bee07d85cfa3b9c46d45e5fec3f568009