The test is supposed to create an invalid sign byte. Before this PR,
the generated sign byte could in fact be valid due to an overflow.
Co-authored-by: Jonas Nick <jonasd.nick@gmail.com>
9b6a1c384d sync-upstream.sh: Fix position of "-b" option in reproduce command (Tim Ruffing)
Pull request description:
ACKs for top commit:
jonasnick:
utACK 9b6a1c384d
Tree-SHA512: 27e4a41bc9c8f10715623f669c97a511520753b23d24ae91d6d2144e54588da0769f97b1de78c87b7471b39e556b682b1c2910b2bf71f124fb77cbc9e446d5f8
05b207e969 sync-upstream: allows providing the local branch via cli (Jonas Nick)
Pull request description:
ACKs for top commit:
real-or-random:
ACK 05b207e969
Tree-SHA512: b02f3fdf6565943cea2b93a0b2b0a38c30bb3c94873d0c4ed2ad276c75f3dc610911d1c9c076c8b7fd3a5baf83aa1ab66ec86415333cf58fe8f07c64fa74656f
39407c3f59 Mark stack variables as early clobber for technical correctness (Pieter Wuille)
56a5d41429 Bugfix: mark outputs as early clobber in scalar x86_64 asm (Pieter Wuille)
c8c0f55a11 ct: Be cautious and use volatile trick in more "conditional" paths (Tim Ruffing)
3e94289966 ct: Use volatile trick in scalar_cond_negate (Tim Ruffing)
Pull request description:
ACKs for top commit:
real-or-random:
ACK 39407c3f59 I also verified that the ct time tests pass on GCC 13.1.1 and Clang 15.0.7.
Tree-SHA512: b7e695527ea58cc7b94a5f2fff6473b6779a469bc5c38baf92624b655cbdf303fbd204e6c1395fa02b98db3bc47bab32afe64bae4ab4fab18da856b621aab070
In the field 5x52 asm for x86_64, stack variables are provided as outputs.
The existing inputs are all forcibly allocated to registers, so cannot
coincide, but mark them as early clobber anyway to make this clearer.
In the existing code, the compiler is allowed to allocate the RSI register
for outputs m0, m1, or m2, which are written to before the input in RSI is
read from. Fix this by marking them as early clobber.
Reported by ehoffman2 in https://github.com/bitcoin-core/secp256k1/issues/766
- secp256k1_scalar_cadd_bit
- secp256k1_modinvXX_normalize_YY
- secp256k1_modinvXX_divsteps_ZZ
- ECMULT_CONST_TABLE_GET_GE
Even though those code loations are not problematic right now
(with current compilers).
4ab4ec38a0 musig: add note about missing verification to partial_sign to doc (Jonas Nick)
f50ad76004 musig: update version number of BIP (Jonas Nick)
Pull request description:
ACKs for top commit:
real-or-random:
ACK 4ab4ec38a0
Tree-SHA512: 1e35d83dd97bac96dfbc02d58841582afe67c38562c728bc3c593a797e1316dfdd550c4988dc78557f25a1633711ec13b35f6c5bae0f7dd29c9f1c994ad5c82c
f3126fdfec norm arg: remove prove edge tests which are now covered by vectors (Jonas Nick)
847ed9ecb2 norm arg: add verification to prove vectors (Jonas Nick)
cf797ed2a4 norm arg: add prove test vectors (Jonas Nick)
095c1e749c norm arg: add prove_const to tests (Jonas Nick)
bf7bf8a64f norm arg: split norm_arg_zero into prove_edge and verify_zero_len (Jonas Nick)
a70c4d4a8a norm arg: add test vector for |n| = 0 (Jonas Nick)
f5e4b16f0f norm arg: add test vector for sign bit malleability (Jonas Nick)
c0de361fc5 norm arg: allow X and R to be the point at infinity (Jonas Nick)
f22834f202 norm arg: add verify vector for n = [0], l = [0] (Jonas Nick)
d8e7f3763b musig: move ge_{serialize,parse}_ext to module-independent file (Jonas Nick)
Pull request description:
ACKs for top commit:
Liam-Eagen:
ACK f3126fd
Tree-SHA512: 1aad86521fce74435beabe7690c7fcc38ad9ae7a884ddcab69ef825b573433f700723a7672d29df1b4465bc33d5957b6a46f657f988cfd2cc73fa94a3472357d
e444d24bca Fix include guards: No _ prefix/suffix but _H suffix (as in upstream) (Tim Ruffing)
0eea7d97ab Use relative #include paths in tests (as in upstream) (Tim Ruffing)
c690d6df70 Use relative #include paths in benchmarks (as in upstream) (Tim Ruffing)
c565827c1a Use relative #include paths in ctime_test (as in upstream) (Tim Ruffing)
4eca406f4c Use relative #include paths in library (as in upstream) (Tim Ruffing)
Pull request description:
ACKs for top commit:
apoelstra:
ACK e444d24bca
jonasnick:
ACK e444d24bca
Tree-SHA512: 4d125cf75748f4a921b70ca933ee59c3cf5c0845c6960e6915a322e53840cb3a0955fe5952e654d133ad36991f3268aeee44430cbd6f7d83e333a554c436f39b
13c438cdee sync-upstream: Use --autostash to handle uncommitted changes (Tim Ruffing)
Pull request description:
This makes it possible to use sync-upstream with uncommitted changes. (This is in particular helpful when working on the script itself.)
Without this commit, git pull will fail due to the uncommitted changes.
ACKs for top commit:
apoelstra:
utACK 13c438cdee
Tree-SHA512: c3a2fce68382bf4e769c64bbdc5666a8f4d9cf6f387e7d8af408e9c3e07b4a875205b7cdae9f647b7127128c13ee58effc0045ac5faf5fba2851b38af40439e8
This makes it possible to use sync-upstream with uncommitted changes. (This
is in particular helpful when working on the script itself.)
Without this commit, git pull will fail due to the uncommitted changes.
96f4853850 ct: Use volatile "trick" in all fe/scalar cmov implementations (Tim Ruffing)
Pull request description:
ACKs for top commit:
jonasnick:
ACK 96f4853850
Tree-SHA512: b3524a817ad8787a19dd28fc38523ab0ee2ddb72c5d88dfef566a9baa849b8d6a12df93030ecf97251e078128ec8203478bf98f3e8d9b28cc595ea5e8579c762
Apparently clang 15 is able to compile our cmov code into a branch,
at least for fe_cmov and fe_storage_cmov. This commit makes the
condition volatile in all cmov implementations (except ge but that
one only calls into the fe impls).
This is just a quick fix. We should still look into other methods,
e.g., asm and #457. We should also consider not caring about
constant-time in scalar_low_impl.h
We should also consider testing on very new compilers in nightly CI,
see https://github.com/bitcoin-core/secp256k1/pull/864#issuecomment-769211867
