d78bec7001 ci: Remove Windows MSVC tasks from Cirrus CI (Hennadii Stepanov)
3545dc2b9b ci, gha: Run all MSVC tests on Windows natively (Hennadii Stepanov)
Pull request description:
ACKs for top commit:
real-or-random:
ACK d78bec7001
jonasnick:
ACK d78bec7001
Tree-SHA512: b58162a9f0827dceb1c7eb6fb7c759c0bffcf3e0d24cc7e6628ad71d1faaabaffb9d8de6fcd3d07bfcaca409632a13f711f9ad871a30718139557544cf91b4bf
747ada3587 test: Silent noisy clang warnings about Valgrind code on macOS x86_64 (Hennadii Stepanov)
Pull request description:
Since #1206, on macOS x86_64 with Valgrind installed, clang emits a massive amount of `-Wreserved-identifier` and `-Wreserved-macro-identifier` warnings from the `valgrind/valgrind.h` and `valgrind/memcheck.h` headers.
This PR prevents warnings emitted for the Valgrind code.
ACKs for top commit:
real-or-random:
utACK 747ada3587
Tree-SHA512: dd1b2b9db2d471939fdc30f9d8fd106a12f21ec5008ca98d8ebe3087d7ea352d564e8bbd0cec59a004e084af3a84d4680cb81f2ef6fe13cf164b7691e33f437d
It is not neccessary for the second argument in `secp256k1_fe_equal_var`
(or `secp256k1_fe_equal`) to have magnitude = 1.
Hence, removed the `secp256k1_fe_normalize_weak` call for those argument.
b7c685e74a Save _normalize_weak calls in group add methods (Peter Dettman)
c83afa66e0 Tighten group magnitude limits (Peter Dettman)
173e8d061a Implement current magnitude assumptions (Peter Dettman)
49afd2f5d8 Take use of _fe_verify_magnitude in field_impl.h (Sebastian Falbesoner)
4e9661fc42 Add _fe_verify_magnitude (no-op unless VERIFY is enabled) (Peter Dettman)
690b0fc05a add missing group element invariant checks (Sebastian Falbesoner)
Pull request description:
This PR picks up #1032 by peterdettman. It's essentially a rebase on master; the original first commit (09dbba561f) which introduced group verification methods has mostly been replaced by PR #1299 (commit f20266722a) and what remains now is only adding a few missing checks at some places. The remaining commits are unchanged, though some (easy-to-solve) conflicts appeared through cherry-picking. The last commit which actually removes the `normalize_weak` calls is obviously the critical one and needs the most attention for review.
ACKs for top commit:
sipa:
utACK b7c685e74a
real-or-random:
ACK b7c685e74a
jonasnick:
ACK b7c685e74a
Tree-SHA512: f15167eff7ef6ed971c726a4d738de9a15be95b0c947d7e38329e7b16656202b7113497d36625304e784866349f2293f6f1d8cb97df35393af9ea465a4156da3
175db31149 ci: Drop no longer needed `PATH` variable update on Windows (Hennadii Stepanov)
116d2ab3df cmake: Set `ENVIRONMENT` property for examples on Windows (Hennadii Stepanov)
cef373997c cmake, refactor: Use helper function instead of interface library (Hennadii Stepanov)
Pull request description:
This PR simplifies running examples on Windows, because the DLL must reside either in the same folder where the executable is or somewhere in PATH.
It is an alternative to #1233.
ACKs for top commit:
real-or-random:
utACK 175db31149
Tree-SHA512: 8188018589a5bcf0179647a039cdafcce661dc103a70a5bb9e6b6f680b899332ba30b1e9ef5dad2a8c22c315d7794747e49d8cf2e391eebea21e3d8505ee334b
Also update the operations count comments in each of the affected
functions accordingly and remove a redundant VERIFY_CHECK in
secp256k1_gej_add_ge (the infinity value range check [0,1] is already
covered by secp256k1_gej_verify above).
Co-authored-by: Sebastian Falbesoner <sebastian.falbesoner@gmail.com>
Co-authored-by: Tim Ruffing <crypto@timruffing.de>
Co-authored-by: Jonas Nick <jonasd.nick@gmail.com>
b097a466c1 util: remove unused checked_realloc (Cory Fields)
Pull request description:
Usage was removed in 6fe50439 . This should be a NOOP.
Noticed when analyzing for zenbleed exposure: stdlib calls that aren't optimized away.
In this case realloc isn't making it into the final binary, but as far as I can tell this is completely dead code and should be dropped.
ACKs for top commit:
jonasnick:
ACK b097a466c1
real-or-random:
ACK b097a466c1
Tree-SHA512: d4249215eddd4035be2b50a8bb48b8a681abdab4ab41ca53f6c2a2507edfbc9ffa39ba22eb48e7da52f978e224198294495ce64f9d571d98c19283b20b82a63a
Remove also the explicit magnitude restriction `a->x.magnitude <= 31`
in `secp256k1_gej_eq_x_var` (introduced in commit
07c0e8b82e), as this is implied by the
new limits.
Co-authored-by: Sebastian Falbesoner <sebastian.falbesoner@gmail.com>
The group element checks `secp256k1_{ge,gej}_verify` have first been
implemented and added in commit f20266722a
(PR #1299). This commit adds additional verification calls in group
functions, to match the ones that were originally proposed in commit
09dbba561f of WIP-PR #1032 (which is
obviously not rebased on #1299 yet).
Also, for easier review, all functions handling group elements are
structured in the following wasy for easier review (idea suggested by
Tim Ruffing):
- on entry, verify all input ge, gej (and fe)
- empty line
- actual function body
- empty line
- on exit, verify all output ge, gej
Co-authored-by: Peter Dettman <peter.dettman@gmail.com>
Co-authored-by: Tim Ruffing <crypto@timruffing.de>
c424e2fb43 ellswift: fix probabilistic test failure when swapping sides (Jonas Nick)
Pull request description:
Reported by jonatack in https://github.com/bitcoin/bitcoin/issues/28079.
When configured with `--disable-module-ecdh --enable-module-recovery`, then `./tests 64 81af32fd7ab8c9cbc2e62a689f642106` fails with
```
src/modules/ellswift/tests_impl.h:396: test condition failed: secp256k1_memcmp_var(share32_bad, share32a, 32) != 0
```
This tests verifies that changing the `party` bit of the `secp256k1_ellswift_xdh` function results in a different share. However, that's not the case when the secret keys of both parties are the same and this is actually what happens in the observed test failure. The keys can be equal in this test case because they are created by the `random_scalar_order_test` function whose output is not uniformly random (it's biased towards 0).
This commit restores the assumption that the secret keys differ.
ACKs for top commit:
sipa:
utACK c424e2fb43
real-or-random:
utACK c424e2fb43
Tree-SHA512: d1ab61473a77478f9aeffb21ad73e0bba478c90d8573c72ec89d2e0140434cc65c9d5f4d56e5f259931dc68fc1800695c6cd5d63d9cfce4c1c4d6744eeaa2028
When configured with `--disable-module-ecdh --enable-module-recovery`, then
`./tests 64 81af32fd7ab8c9cbc2e62a689f642106` fails with
```
src/modules/ellswift/tests_impl.h:396: test condition failed: secp256k1_memcmp_var(share32_bad, share32a, 32) != 0
```
This tests verifies that changing the `party` bit of the
`secp256k1_ellswift_xdh` function results in a different share. However, that's
not the case when the secret keys of both parties are the same and this is
actually what happens in the observed test failure. The keys can be equal in
this test case because they are created by the `random_scalar_order_test`
function whose output is not uniformly random (it's biased towards 0).
This commit restores the assummption that the secret keys differ.
This change simplifies running examples on Windows, because the DLL
must reside either in the same folder where the executable is or
somewhere in PATH.
981e5be38c ci: Fix typo in comment (Tim Ruffing)
e9e9648219 ci: Reduce number of macOS tasks from 28 to 8 (Tim Ruffing)
609093b387 ci: Add x86_64 Linux tasks for gcc and clang snapshots (Tim Ruffing)
1deecaaf3b ci: Install development snapshots of gcc and clang (Tim Ruffing)
Pull request description:
ACKs for top commit:
hebasto:
re-ACK 981e5be38c
jonasnick:
ACK 981e5be38c
Tree-SHA512: a36ef6f3c30a7f6e09e186e67b8eeb6e16e05de3bd97f21342866e75e33275103d463b6a12603ce235da7e26e4acdef4d811f62f369f18db9ac4e7ff06749136
