Our RNG has been replaced with Xoshiro256++, a well-analyzed RNG. Our
unit tests should not be resposible for verifying its statistical
qualities.

After calculating the right-hand side of the elliptic curve equation
(x^3 + 7), the field element `x3` has a magnitude of 2 (1 as result of
`secp256k1_fe_mul`, then increased by 1 due to `secp256k1_fe_add_int`).
This is fine for `secp256k1_fe_equal_var`, as the second parameter only
requires the magnitude to not exceed 31, and the normalize_weak call can
hence be dropped.

See #1001.

The recently merged ellswift PR (#1129) introduced a helper
`secp256k1_ge_x_on_curve_var` to check if a given X coordinate is
valid (i.e. the expression x^3 + 7 is square, see commit
79e5b2a). This can be used for code
deduplication in the `ecmult_const_mult_xonly` test.

…256k1_ge_x_on_curve_var`

7d8d5c8 tests: refactor: take use of `secp256k1_ge_x_on_curve_var` (Sebastian Falbesoner)

Pull request description:

  The recently merged ellswift PR (#1129) introduced a helper `secp256k1_ge_x_on_curve_var` to check if a given X coordinate is on the curve (i.e. the expression x^3 + 7 is square, see commit 79e5b2a). This can be used for code deduplication in the `ecmult_const_mult_xonly` test.

  (Found this instance via `$ git grep add_int.*SECP256K1_B`, I think it's the only one where the helper can be used.)

ACKs for top commit:
  sipa:
    utACK 7d8d5c8
  real-or-random:
    utACK 7d8d5c8

Tree-SHA512: aebff9b5ef2f6f6664ce89e4e1272cb55b6aac81cfb379652c4b7ab30dd1d7fd82a2c3b47c7b7429755ba28f011a3a9e2e6d3aa5c77d3b105d159104c24b89f3

… affecting magnitude are constant

be8ff3a field: Static-assert that int args affecting magnitude are constant (Tim Ruffing)

Pull request description:

  See #1001.

  Try to revert the lines in `tests.c` to see the error message in action.

ACKs for top commit:
  sipa:
    ACK be8ff3a. Verified by introducing some non-constant expressions and seeing compilation fail.
  theStack:
    ACK be8ff3a

Tree-SHA512: 8befec6ee64959cdc7f3e29b4b622410794cfaf69e9df8df17600390a93bc787dba5cf86239de6eb2e99c038b9aca5461e4b3c82f0e0c4cf066ad7c689941b19

There is a function `random_fe_test` which does exactly the
same, so use that instead. Note that it's also moved up before the
`random_group_element_test` function, in order to avoid needing a forward
declaration.

There are several instances in the tests where random non-zero field
elements are generated by calling `random_fe_test` in a do/while-loop.
This commit deduplicates all these by introducing a
`random_fe_non_zero_test` helper. Note that some instances checked the
is-zero condition via `secp256k1_fe_normalizes_to_zero_var`, which is
unnecessary, as the result of `random_fe_test` is already normalized (so
strictly speaking, this is not a pure refactor).

…ro `random_fe_test()` results

5a95a26 tests: introduce helper for non-zero `random_fe_test` results (Sebastian Falbesoner)
304421d tests: refactor: remove duplicate function `random_field_element_test` (Sebastian Falbesoner)

Pull request description:

  There are several instances in the tests where random non-zero field elements are generated by calling `random_fe_test` in a do/while-loop with is-zero condition. This PR deduplicates all these by introducing a `random_fe_non_zero_test` helper. Note that some instances checked the is-zero condition via `secp256k1_fe_normalizes_to_zero_var`, which is unnecessary, as the result of `random_field_element_test` is already normalized (so strictly speaking, this is not a pure refactor, and there could be tiny run-time improvements, though I doubt that's measurable).

  Additionally, the first commit removes the function `random_field_element_test` as it is logically a duplicate of `random_fe_test`.

ACKs for top commit:
  real-or-random:
    ACK 5a95a26

Tree-SHA512: 920404f38ebe8b84bfd52f3354dc17ae6a0fd6355f99b78c9aeb53bf21f7eca5fd4518edc8a422d84f430ae95864661b497de42a3ab7fa9c49515a1df2f1d466

This is no longer necessary as of
cirruslabs/cirrus-ci-docs#791 (comment) .

…commit

98579e2 ci: Drop manual checkout of merge commit (Tim Ruffing)

Pull request description:

ACKs for top commit:
  jonasnick:
    ACK 98579e2

Tree-SHA512: fe5305322e6fa616af4664db7c151acdfb8119feb0255a65190b9c185ae5383eab37debe76085dfc8137c691e0ff55cb20d9e51993f6cc871bc6c5c945ed66bf

This change adds the same functionality to Windows containers that is
already available in Linux containers.

a7bec34 ci: Print commit in Windows container (Hennadii Stepanov)

Pull request description:

  This PR is a follow-up to bitcoin-core/secp256k1#1368 and adds the same functionality to Windows containers that is already available in Linux containers.

  See: bitcoin-core/secp256k1#1368 (comment).

ACKs for top commit:
  real-or-random:
    ACK a7bec34 seems to work: https://cirrus-ci.com/task/4919320090771456?logs=git_show#L2

Tree-SHA512: 0998e0f7231e3057a7e358a27b34071c73ca556973da20494db84fc67f2a72ad2fe582e59647a425ee41e7d9103a0a22fb3cdf0ace6fe0aed1d21f2f75c8ec53

It is a non-Libtool-specific way to explicitly specify the user's
intention to consume a static `libseck256k1`.

This change allows to get rid of MSVC linker warnings LNK4217 and
LNK4286. Also, it makes possible to merge the `SECP256K1_API` and
`SECP256K1_API_VAR` into one.

This change provides a way to build a shared library that is not tired
to the Libtool-specific `DLL_EXPORT` macro.

05873bb tweak_add: fix API doc for tweak=0 (Jonas Nick)

Pull request description:

ACKs for top commit:
  real-or-random:
    ACK 05873bb

Tree-SHA512: ef587a680c3355c6328dd61e0f5fcac80ea995f6045b4392fe35f3ee1c04ee1bd941662c120758ad641588670c1f0f53bfb17a802821f54100f1385b8bb7375a

… compiling with `gcc -O1`

5b9f37f ci: Add `CFLAGS: -O1` to task matrix (Hennadii Stepanov)
a6ca76c Avoid `-Wmaybe-uninitialized` when compiling with `gcc -O1` (Hennadii Stepanov)

Pull request description:

  Fixes bitcoin-core/secp256k1#1361.

  CI tasks have been adjusted to catch similar issues in the future.

ACKs for top commit:
  real-or-random:
    utACK 5b9f37f
  jonasnick:
    tACK 5b9f37f

Tree-SHA512: 8aa5ec22ed88579ecd37681df68d64f8bab93cd14bdbf432a3af41cadc7ab3eba86c33c179db15bf3a3c798c33064bd845ebdedb02ee617ef634e98c596838c2

By requiring that the input group element's X coordinate (`a->x`) has a
magnitude of <= 31, the normalize_weak call and also the field element
variable `r2` are not needed anymore and hence can be dropped.

restoring wycheproof files

restoring wycheproof files2

b6b9834 small fixes (Alejandro)

Pull request description:

  Corrected some typos

ACKs for top commit:
  real-or-random:
    ACK b6b9834

Tree-SHA512: c40c22c66f1067ecca351f08cca07a78b00bb98af2f6cfb08c25d0b1db6845e0e32ace1954c386db7020cf9fc7ae973ff15bd6d9c0144f3d21ea28c15741050f

…bility logic on Windows (attempt 3)

c6cd2b1 ci: Add task for static library on Windows + CMake (Hennadii Stepanov)
020bf69 build: Add extensive docs on visibility issues (Tim Ruffing)
0196e8a build: Introduce `SECP256k1_DLL_EXPORT` macro (Hennadii Stepanov)
9f1b190 refactor: Replace `SECP256K1_API_VAR` with `SECP256K1_API` (Hennadii Stepanov)
ae9db95 build: Introduce `SECP256K1_STATIC` macro for Windows users (Hennadii Stepanov)

Pull request description:

  Previous attempts:
  - bitcoin-core/secp256k1#1346
  - bitcoin-core/secp256k1#1362

  The result is as follows:
  1. Simple, concise and extensively documented code.
  2. Explicitly documented use cases with no ambiguities.
  3. No workarounds for linker warnings.
  4. Solves one item in bitcoin-core/secp256k1#1235.

ACKs for top commit:
  real-or-random:
    utACK c6cd2b1

Tree-SHA512: d58694452d630aefbd047916033249891bc726b7475433aaaa7c3ea2a07ded8f185a598385b67c2ee3440ec5904ff9d9452c97b0961d84dcb2eb2cf46caa171e

…n `secp256k1_ge_is_valid_var`/`secp256k1_gej_eq_x_var`

07c0e8b group: remove unneeded normalize_weak in `secp256k1_gej_eq_x_var` (Sebastian Falbesoner)
efa76c4 group: remove unneeded normalize_weak in `secp256k1_ge_is_valid_var` (Sebastian Falbesoner)

Pull request description:

  This PR removes unneeded normalize_weak calls in two group element functions:
  * `secp256k1_ge_is_valid_var`: After calculating the right-hand side of the elliptic curve equation (x^3 + 7), the field element `x3` has a magnitude of 2 (1 as result of `secp256k1_fe_mul`, then increased by 1 due to `secp256k1_fe_add_int`). This is fine for `secp256k1_fe_equal_var`, as the second parameter only requires the magnitude to not exceed 31, and the normalize_weak call is hence not needed and can be dropped. Note that the interface description for `secp256k1_fe_equal` (which also applies to `secp256k1_fe_equal_var`) once stated that _both_ parameters need to have magnitude 1, but that was corrected in commit 7d7d43c.

  * `secp256k1_gej_eq_x_var`: By requiring that the input group element's X coordinate (`a->x`) has a magnitude of <= 31, the normalize_weak call and also the field element variable `r2` are not needed anymore and hence can be dropped.

ACKs for top commit:
  sipa:
    utACK 07c0e8b
  jonasnick:
    ACK 07c0e8b

Tree-SHA512: 9037e4af881ce7bf3347414d6da06b99e3d318733ba4f70e8b24d2320c2f26d022144e17bd6b95c1a4ef1be3825a4464e56ce2d2b3ae7bbced04257048832b7f

Co-authored-by: Pieter Wuille <[email protected]>
Co-authored-by: Tim Ruffing <[email protected]>

…(with create+decode roundtrip)

2792119 Add exhaustive test for ellswift (create+decode roundtrip) (Sebastian Falbesoner)

Pull request description:

  This PR adds the basic structure for ellswift exhaustive tests. Right now only a `secp256k1_ellswift_create` + `secp256k1_ellswift_decode` indirect roundtrip (exhaustive loop scalar -> ellswift pubkey -> decoded pubkey -> decoded group element, compared with exhaustive precomputed group element) is included.

  The exhaustive tests passes locally with all currently supported orders (n=13 [default] and n=199). Note that for n=7, the test is skipped, as the used curve in this case is even-ordered and ellswift only supports odd-ordered curves.

ACKs for top commit:
  sipa:
    utACK 2792119
  real-or-random:
    utACK 2792119

Tree-SHA512: c51d3d99e9839793b3c15d75b9a29f01080db160ab8819973abd877288f9f0af972ea4264290220ab1cd035fdebcfac7767436aa39154d924ef0bf6a5733a55d

c7d900f doc: minor ellswift.md updates (stratospher)

Pull request description:

ACKs for top commit:
  sipa:
    ACK c7d900f
  real-or-random:
    ACK c7d900f

Tree-SHA512: 161c17d038eb1eed9f5811c3eb92975a821a5274e7f69aa386bfbe5376b3f06f3d0d2887ea3310efbec83424f09ea8e4082e8c02b2fcad3b915625ce5c2007d2

That is, use it also in the definition and not only the declaration.

I believe this was the intention of commit
be82bd8, but it was omitted there.

TODO: Make sure the Docker image is actually rebuild

… to Python3)

Some of the C source files contain contain in-comment Sage code
calculating secp256k1 parameters that are already defined in the file
secp256k1_params.sage.  Replace that by a corresponding load instruction
and access the necessary variables. In ecdsa_impl.h, update the comment
to use a one-line shell command calling sage to get the values.

The remaining code (test `test_add_neg_y_diff_x` in tests.c) is updated
to work with a current version based on Python3 (Sage 9.0+, see
https://wiki.sagemath.org/Python3-Switch).

The latter can be seen as a small follow-up to PR #849 (commit
13c88ef).

…er to secp256k1_params.sage, update to Python3)

600c5ad clean up in-comment Sage code (refer to secp256k1_params.sage, update to Python3) (Sebastian Falbesoner)

Pull request description:

  Some of the C source files contain contain in-comment Sage code calculating secp256k1 parameters that are already defined in the file secp256k1_params.sage.  Replace that by a corresponding load instruction and access the necessary variables. In ecdsa_impl.h, update the comment to use a one-line shell command calling sage to get the values.

  The remaining code (test `test_add_neg_y_diff_x` in tests.c) is updated to work with a current version based on Python3 (Sage 9.0+, see https://wiki.sagemath.org/Python3-Switch).

  The latter can be seen as a small follow-up to PR #849 (commit 13c88ef).

ACKs for top commit:
  sipa:
    ACK 600c5ad
  real-or-random:
    ACK 600c5ad

Tree-SHA512: a9e52f6afbce65edd9ab14203612c3d423639f450fe8f0d269a3dda04bebefa95b607f7aa0faec864cb78b46d49f281632bb1277118749b7d8613e9f5dcc8f3d

… in fe_sqrt

b79ba8a field: Use `restrict` consistently in fe_sqrt (Tim Ruffing)

Pull request description:

  That is, use it also in the definition and not only the declaration.

  I believe this was the intention of commit
  bitcoin-core/secp256k1@be82bd8, but it was omitted there.

  edit: Changed the description. I'm not entirely sure but after looking at the standard, I tend to think this is more than a cosmetic change, and only this change actually makes the parameters `restrict`. Anyway, I believe making them `restrict` was simply forgotten in be82bd8.

ACKs for top commit:
  sipa:
    utACK b79ba8a

Tree-SHA512: eecec7674d8cef7833d50f4041b87241ca8de4839aa8027df1c422b89f5a1bcef3916ac785057a596c459ce1aa9d41e5a21ecb6fed9c5d15a1d9f588c7ee208e

…of GCC and Clang

981e5be ci: Fix typo in comment (Tim Ruffing)
e9e9648 ci: Reduce number of macOS tasks from 28 to 8 (Tim Ruffing)
609093b ci: Add x86_64 Linux tasks for gcc and clang snapshots (Tim Ruffing)
1deecaa ci: Install development snapshots of gcc and clang (Tim Ruffing)

Pull request description:

ACKs for top commit:
  hebasto:
    re-ACK 981e5be
  jonasnick:
    ACK 981e5be

Tree-SHA512: a36ef6f3c30a7f6e09e186e67b8eeb6e16e05de3bd97f21342866e75e33275103d463b6a12603ce235da7e26e4acdef4d811f62f369f18db9ac4e7ff06749136

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.

…ailure when swapping sides

c424e2f ellswift: fix probabilistic test failure when swapping sides (Jonas Nick)

Pull request description:

  Reported by jonatack in bitcoin/bitcoin#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 c424e2f
  real-or-random:
    utACK c424e2f

Tree-SHA512: d1ab61473a77478f9aeffb21ad73e0bba478c90d8573c72ec89d2e0140434cc65c9d5f4d56e5f259931dc68fc1800695c6cd5d63d9cfce4c1c4d6744eeaa2028

6ec3731 Simplify test PRNG implementation (Pieter Wuille)
fb5bfa4 Add static test vector for Xoshiro256++ (Tim Ruffing)
723e8ca Remove randomness tests (Pieter Wuille)

Pull request description:

ACKs for top commit:
  real-or-random:
    utACK 6ec3731
  jonasnick:
    ACK 6ec3731

Tree-SHA512: 4cbbb9c42e31f067b17dd9169ae5d5e68bce77d1253452db9df523d3be2b5d61002d5a4203e5a153f257ec63c5ff2113555743eeb402d4b6c573069ea494d407

4f8c5bd refactor: Drop unused cast (Hennadii Stepanov)

Pull request description:

ACKs for top commit:
  real-or-random:
    utACK 4f8c5bd
  jonasnick:
    ACK 4f8c5bd

Tree-SHA512: cc94b524f53e393bd843383e92bbc5b84dd7557d8121241f2d0461b960a0706236147d02b6f5bfc433272849f517c62eb6f1e0cfae892e1b8054817c27365430

Usage was removed in 6fe5043 .

b097a46 util: remove unused checked_realloc (Cory Fields)

Pull request description:

  Usage was removed in 6fe5043 . 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 b097a46
  real-or-random:
    ACK b097a46

Tree-SHA512: d4249215eddd4035be2b50a8bb48b8a681abdab4ab41ca53f6c2a2507edfbc9ffa39ba22eb48e7da52f978e224198294495ce64f9d571d98c19283b20b82a63a

...like the other modules.

…SECP_CONFIG_DEFINES

78ca880 build: enable ellswift module via SECP_CONFIG_DEFINES (Jonas Nick)

Pull request description:

  ...like the other modules.

ACKs for top commit:
  sipa:
    utACK 78ca880
  real-or-random:
    utACK 78ca880

Tree-SHA512: c157a1ed912b9aa1a318aa0a70859a3ac67cb22303993f08ff00ed601e6ac197380dd503d3b361cbc4e698fc6489b5283b782f570f2703809d23668f3ebe5ba6

4692478 ci: print $ELLSWIFT in cirrus.sh (Jonas Nick)

Pull request description:

ACKs for top commit:
  real-or-random:
    ACK 4692478

Tree-SHA512: 84c6021e2135857541def6ba058d9c9a1c180fd32a625854ff82d51d0561a4dd243623d38d335aeaf40200501581c0678878a9166f4a96ae3fb32717b8d39fbd

0aacf64 9e6d1b0 332af31 afd7eb4 c9ebca9 cc55757 0f7657d 907a672 b40e2d3 c545fdc 2bd5f3e 0e00fc7 c734c64 26392da ' into temp-merge-1386

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 <[email protected]>