Skip to content

Upstream PRs #854 #852 #857 #858 #860 #845 #862 #875 #878 #874 #877 #880 #864 #882 #894 #891 #901#126

Merged
jonasnick merged 51 commits into
masterfrom
temp-merge-901
Mar 12, 2021
Merged

Upstream PRs #854 #852 #857 #858 #860 #845 #862 #875 #878 #874 #877 #880 #864 #882 #894 #891 #901#126
jonasnick merged 51 commits into
masterfrom
temp-merge-901

Conversation

@jonasnick

Copy link
Copy Markdown
Contributor

[upstream PR #854]: Rename msg32 to msghash32 in ecdsa_sign/verify and add explanation
[upstream PR #852]: Add sage script for generating scalar_split_lambda constants
[upstream PR #857]: docs: fix simple typo, dependecy -> dependency
[upstream PR #858]: Fix insecure links
[upstream PR #860]: fixed trivial typo
[upstream PR #845]: Extract the secret key from a keypair
[upstream PR #862]: Autoconf improvements
[upstream PR #875]: Avoid casting (void**) values.
[upstream PR #878]: Remove unused secp256k1_fe_inv_all_var
[upstream PR #874]: Remove underscores from header defs.
[upstream PR #877]: Add missing secp256k1_ge_set_gej_var decl.
[upstream PR #880]: Add parens around ROUND_TO_ALIGN's parameter.
[upstream PR #864]: Add support for Cirrus CI
[upstream PR #882]: Use bit ops instead of int mult for constant-time logic in gej_add_ge
[upstream PR #894]: ctime_test: move context randomization test to the end
[upstream PR #891]: build: Add workaround for automake 1.13 and older
[upstream PR #901]: ci: Switch all Linux builds to Debian and more improvements

This PR was automatically created with ./contrib/sync-upstream.sh .

Also added -zkp modules to .cirrus.yml - with some small simplifications, but I didn't spend to much time on it because we may refactor this anyway upstream.

real-or-random and others added 30 commits November 25, 2020 13:50
 * Move curve parameters to separate file
 * Rename main prover script for clarity
…planation

6e85d67 Rename tweak to tweak32 in public API (Jonas Nick)
f587f04 Rename msg32 to msghash32 in ecdsa_sign/verify and add explanation (Jonas Nick)

Pull request description:

  This fixes #307 if there's nothing else that's confusing.

ACKs for top commit:
  real-or-random:
    ACK 6e85d67 I inspected the diff

Tree-SHA512: 1b0dc9dfffd497058dc39c962a512ed6d7f89218020fef9d2c03aaae1aefbf272b918c4fe6503434b62547714855fe1b8b89f2366f3ae6cde16143207c9e6b86
329a2e0 sage: Add script for generating scalar_split_lambda constants (Tim Ruffing)
f554dfc sage: Reorganize files (Tim Ruffing)

Pull request description:

ACKs for top commit:
  sipa:
    ACK 329a2e0

Tree-SHA512: d41fe5eba332f48af0b800778aa076925c4e8e95ec21c4371a500ddd6088b6d52961bdb93f7ce2b127e18095667dbb966a0d14191177f0d0e78dfaf55271d5e2
There is a small typo in src/group_impl.h.

Should read `dependency` rather than `dependecy`.
18aadf9 docs: fix simple typo, dependecy -> dependency (Tim Gates)

Pull request description:

  There is a small typo in src/group_impl.h.

  Should read `dependency` rather than `dependecy`.

ACKs for top commit:
  real-or-random:
    ACK 18aadf9

Tree-SHA512: 3529f43bcc87ea8940ecf5af765951f61d97d1efa86fd8abc29e32b600fd449165a94a2fa525bc6b3d9a7d8aa6e691cc4d42033537b196ba166a867e6db7f397
07aa4c7 Fix insecure links (Dimitris Apostolou)

Pull request description:

ACKs for top commit:
  sipa:
    ACK 07aa4c7. Verified all the modified links.
  jonasnick:
    ACK 07aa4c7

Tree-SHA512: d1240aab5e40a204c75fca1049b99af9890684df7dbce4167b1904f73424c8a4f84ed85a8cc315501f1b7cf1674d744232b9f2126dff31e3d47e4f3fc65764d4
b7bc3a4 fixed typo (Ferdinando M. Ametrano)

Pull request description:

ACKs for top commit:
  real-or-random:
    ACK b7bc3a4
  elichai:
    ACK b7bc3a4

Tree-SHA512: 6c1889f095607a2f293ffe00359c03e63cfca572b0a17388b83ece54f24ec61ac12d6eb967a47d2dccd54de991383923a07c5cced320c0a96a36a28674cf739c
Valgrind is typically installed using brew on macOS. This commit
makes ./configure detect this case set the appropriate include
directory (in the same way as we already do for openssl and gmp).
No behavioral changes.
This commits simply uses CC as CC_FOR_BUILD and the same for
corresponding flags if we're not cross-compiling. This has a number of
benefits in this common case:
 - It avoids strange cases where very old compilers are used (#768).
 - Flags are consistently set for CC and CC_FOR_BUILD.
 - ./configure is faster.
 - You get compiler x consistently if you set CC=x; we got this wrong
   in CI in the past.

./configure warns if a _FOR_BUILD variable is set but ignored because
we're not cross-compiling.

The change exposed that //-style comments are used in gen_context.c,
which is also fixed by this commit.

This commit also reorganizes code in configure.ac to have a cleaner
separation of sections.
33cb3c2 Add secret key extraction from keypair to constant time tests (Elichai Turkel)
36d9dc1 Add seckey extraction from keypair to the extrakeys tests (Elichai Turkel)
fc96aa7 Add a function to extract the secretkey from a keypair (Elichai Turkel)

Pull request description:

  With schnorrsig if you need to tweak the secret key (for BIP32) you must use the keypair API to get compatible secret/public keys which you do by calling `secp256k1_keypair_xonly_tweak_add()`, but after that there's no currently a way to extract the secret key back for storage.
  so I added a `secp256k1_keypair_seckey` function to extract the key

ACKs for top commit:
  jonasnick:
    ACK 33cb3c2
  real-or-random:
    ACK 33cb3c2 code inspection, tests pass

Tree-SHA512: 11212db38c8b87a87e2dc35c4d6993716867b45215b94b20522b1b3164ca63d4c6bf5192a6bff0e9267b333779cc8164844c56669a94e9be72df9ef025ffcfd4
3c15130 Improve CC_FOR_BUILD detection (Tim Ruffing)
47802a4 Restructure and tidy configure.ac (Tim Ruffing)
252c19d Ask brew for valgrind include path (Tim Ruffing)

Pull request description:

  See individual commit messages. These are improvements in preparation of the switch to Cirrus CI. (Maybe I'll just open a PR on top of this one.)

  The first commit made the difference between successful build https://cirrus-ci.com/task/6740575057608704 and unsuccessful build https://cirrus-ci.com/task/4909571074424832.

  I've tested the second commit without cross-compilation and with cross-compilation for android (bitcoin-core/secp256k1#621 (comment))

  When working on the autoconf stuff, I noticed two things that I just want to write down here:
   - At some point we should update [build-aux/m4/ax_prog_cc_for_build.m4](https://www.gnu.org/software/autoconf-archive/ax_prog_cc_for_build.html). This is outdated, and [there have been a lot of fixes](autoconf-archive/autoconf-archive#207) But the latest version is [broken](https://lists.gnu.org/archive/html/autoconf-archive-maintainers/2020-06/msg00002.html), so now is probably not the time.
   - The latest autoconf 2.70 deprecates `AC_PROG_CC_C89`. It's not needed anymore because `AC_PROG_CC` cares about testing for version support. This makes autoconf 2.70 output a warning that we should probably just ignore. We don't want to force users onto 2.70...

ACKs for top commit:
  sipa:
    utACK 3c15130
  jonasnick:
    utACK 3c15130 makes sense (with my very basic understanding of autoconf)

Tree-SHA512: 595b9de316374c2213f1340cddaa22eb3190b01fa99aa6ae26e77804df41e7ecf96a09e03c28e8f8b9fd04e211e4ee2f78f1e5a7995143c84f99d2e16d4f0260
This makes them consistent with other files and avoids reserved identifiers.
Replaced with an expression that only casts (void*) values.
2730618 Avoid casting (void**) values. Replaced with an expression that only casts (void*) values. (Russell O'Connor)

Pull request description:

ACKs for top commit:
  sipa:
    utACK 2730618
  real-or-random:
    utACK bitcoin-core/secp256k1@2730618
  jonasnick:
    utACK 2730618

Tree-SHA512: bdc1e9eefa10f79b744ef6ae83f379faff7bce9fb428c3bcfcc3f6e4e252e5c6543efbe0f84760709850948cbc8a432772c76a6c5f6b8cd18cb2d862b324912d
75d2ae1 Remove unused secp256k1_fe_inv_all_var (Pieter Wuille)

Pull request description:

ACKs for top commit:
  practicalswift:
    cr ACK 75d2ae1: patch looks correct
  real-or-random:
    utACK bitcoin-core/secp256k1@75d2ae1
  jonasnick:
    utACK 75d2ae1

Tree-SHA512: 6f548a436c6dcb275493e73e6afa23fd1b79392cc3071878f98735732ac9c93971e5c92736c3fe50eaae90a200e1a435e9be9f14d1a69251c83876a6e3c46d41
fb390c5 Remove underscores from header defs. This makes them consistent with other files and avoids reserved identifiers. (Russell O'Connor)

Pull request description:

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

Tree-SHA512: f49da79c0a90d1e82494821e7cf6f61c66bc377a3f37b2d4787ef19d2126e000627bfe4a76aa1c5bfffeb1382054aa824a7e9ab5d73c19d876b0828722c73854
482e4a9 Add missing secp256k1_ge_set_gej_var decl. (Russell O'Connor)

Pull request description:

ACKs for top commit:
  sipa:
    utACK 482e4a9
  real-or-random:
    utACK bitcoin-core/secp256k1@482e4a9
  jonasnick:
    ACK 482e4a9

Tree-SHA512: 02195390fb79f08bcfd655dc56115ea37df42c1ad8f1123b26e7426e387d9658a3bb18fe9951140fc4dd78ce222b84d8b75ce77aec884675e0c26a2005dd2ddc
This makes the macro robust against a hypothetical ROUND_TO_ALIGN(foo ? sizeA : size B) invocation.
b6f6498 Add parens around ROUND_TO_ALIGN's parameter. This makes the macro robust against a hypothetical ROUND_TO_ALIGN(foo ? sizeA : size B) invocation. (Russell O'Connor)

Pull request description:

  This makes the macro robust against a hypothetical `ROUND_TO_ALIGN(foo ? sizeA : size B)` invocation.

  See also <https://wiki.sei.cmu.edu/confluence/display/c/PRE01-C.+Use+parentheses+within+macros+around+parameter+names>.

ACKs for top commit:
  sipa:
    ACK b6f6498. This is the way.
  jonasnick:
    utACK b6f6498
  real-or-random:
    utACK b6f6498

Tree-SHA512: 6a2685f959e8ae472259e5ea75fe12e8e6213f56f5aec7603a896c294e6a8833caae25c412607d9c9a3125370a7765a3e506127b101a1b87203f95e326f6c6c6
jonasnick and others added 13 commits January 30, 2021 10:07
cc2a545 ci: Refactor Nix shell files (Jonas Nick)
2480e55 ci: Remove support for Travis CI (Tim Ruffing)
2b359f1 ci: Enable simple cache for brewing valgrind on macOS (Tim Ruffing)
8c02e46 ci: Add support for Cirrus CI (Tim Ruffing)

Pull request description:

ACKs for top commit:
  sipa:
    ACK cc2a545. Tested by introducing bugs: #883, #884, #885, #886, #887.
  jonasnick:
    ACK cc2a545

Tree-SHA512: c9e8a891c9bda48b3fc307c2a85d2e4aa180531d084edd778d41c034769661627538ab397efac3abfc1a71c2f0730a45350dd212d499fe475c90a2a1b3c61ac8
…n gej_add_ge

e491d06 Use bit ops instead of int mult for constant-time logic in gej_add_ge (Tim Ruffing)

Pull request description:

ACKs for top commit:
  sipa:
    utACK e491d06. Seems obviously better.
  elichai:
    ACK e491d06
  jonasnick:
    ACK e491d06

Tree-SHA512: 65977d3405e3b6c184c736d46898b615689b56f7562165114429dea49c0f9feb81d021cbe196c8a813b6239254b394cc24ac8d278dab37e521843a1bb0f70c47
7d3497c ctime_test: move context randomization test to the end (Jonas Nick)

Pull request description:

ACKs for top commit:
  real-or-random:
    ACK 7d3497c diff looks good

Tree-SHA512: aef006c43df4cab254ee7de79cdd34c4e2f7a463f29d1da6d285006b32bb4e18d0b914a305f371b8b5f5a20594c37ee464eb1e59d1978db9b06bf6b642e651d8
f329bba build: Add workaround for automake 1.13 and older (Tim Ruffing)

Pull request description:

  Fixes #890.

ACKs for top commit:
  michaelfolkson:
    ACK f329bba

Tree-SHA512: 1ae3d1587abb402c2d3bb28d3a48aeff056f061e755d65d482204bb502b8427aad376c7319b4a694a5bf79c193acd3c88cb65928f0bc0e5c7587222e1315b6d0
The experiment of using Nix Shell was not really successful. Most
notably, Nix uses a bunch of wrapper scripts around compilers, which
make the build much less "pure". This may be useful but it's exactly
not what we want for CI. In particular, this resulted in gcc being used
for the "clang" builds because a wrapper script redefined the CC env
variable.

This now builds a single docker image (Debian) for all architectures
that we test in CI on Linux.
This is taken from Bitcoin Core's .cirrus.yml
This should improve compilation times on macOS. Things can certainly
be improved further, e.g., by running the benchmarks in parallel.
9361f36 ci: Select number of parallel make jobs depending on CI environment (Tim Ruffing)
28eccdf ci: Split output of logs into multiple sections (Tim Ruffing)
c7f754f ci: Run PRs on merge result instead of on the source branch (Tim Ruffing)
b994a8b ci: Print information about binaries using "file" (Tim Ruffing)
f24e122 ci: Switch all Linux builds to Debian (Tim Ruffing)

Pull request description:

  Best reviewed commit by commit

ACKs for top commit:
  jonasnick:
    ACK 9361f36
  sipa:
    utACK 9361f36

Tree-SHA512: fc754e8b57dc58058cebbf63a60ca76e08dbaefea1508ea27b7f962bce697c10033da3f57a35f731bc7cf3e210eb00e3b8985ae8b729d7bd83faee085b878b9c
@jonasnick jonasnick closed this Mar 8, 2021
@jonasnick jonasnick reopened this Mar 8, 2021
@real-or-random

Copy link
Copy Markdown
Member

This will fix the following compile error on macOS

In file included from src/num.h:17,
                 from src/num_impl.h:14,
                 from src/bench_whitelist.c:14:
src/num_gmp.h:10:10: fatal error: gmp.h: No such file or directory
@real-or-random real-or-random changed the title Upstream PRs #854 #852 #857 #858 #860 #845 #862 #875 #878 #874 #877 #880 #864 #882 #894 #891 #901 Upstream PRs #854 #852 #857 #858 #860 #845 #862 #875 #878 #874 #877 #880 #864 #882 #894 #891 \#901 Mar 10, 2021
@real-or-random real-or-random changed the title Upstream PRs #854 #852 #857 #858 #860 #845 #862 #875 #878 #874 #877 #880 #864 #882 #894 #891 \#901 Upstream PRs #854 #852 #857 #858 #860 #845 #862 #875 #878 #874 #877 #880 #864 #882 #894 #891 #901 Mar 10, 2021
@real-or-random

Copy link
Copy Markdown
Member

This PR was automatically created with ./contrib/sync-upstream.sh .

Shouldn't the script show the entire command? Also see #127. :)

@jonasnick

Copy link
Copy Markdown
Contributor Author

Yeah that'd be an improvement but it was just ./contrib/sync-upstream.sh range (which isn't great because it can't be easily reproduced when upstream merges new PRs).

@real-or-random

Copy link
Copy Markdown
Member

Yeah that'd be an improvement but it was just ./contrib/sync-upstream.sh range (which isn't great because it can't be easily reproduced when upstream merges new PRs).

Yeah I'm adding a commit to my PR. :)

@real-or-random

Copy link
Copy Markdown
Member

ACK 4091e61 merge commit picks the right parents, merge resolution and additional commit look good

@jonasnick jonasnick merged commit fac477f into master Mar 12, 2021
mllwchrry pushed a commit to mllwchrry/secp256k1-zkp that referenced this pull request Mar 3, 2026
…to improve parallelism

8354618 cmake: Set `LABELS` property for tests (Hennadii Stepanov)
29f26ec cmake: Integrate DiscoverTests and normalize test names (Hennadii Stepanov)
f95b263 cmake: Add DiscoverTests module (Hennadii Stepanov)
4ac6511 cmake, refactor: Deduplicate test-related code (Hennadii Stepanov)

Pull request description:

  This PR implements the idea suggested in bitcoin-core/secp256k1#1734 (review) and is based on the work from bitcoin/bitcoin#33483.

  Here is an example of the `ctest` output:
  ```
  $ ctest --test-dir build -j $(nproc)
  Test project /home/hebasto/dev/secp256k1/secp256k1/build
          Start   1: secp256k1.noverify_tests.selftest_tests
          Start   2: secp256k1.noverify_tests.all_proper_context_tests
          Start   3: secp256k1.noverify_tests.all_static_context_tests
          Start   4: secp256k1.noverify_tests.deprecated_context_flags_test
  <snip>
  193/196 Test  #31: secp256k1.noverify_tests.ecmult_constants .........................   Passed    5.32 sec
  194/196 Test BlockstreamResearch#184: secp256k1.tests.ellswift_xdh_correctness_tests ....................   Passed    5.62 sec
  195/196 Test BlockstreamResearch#191: secp256k1.exhaustive_tests ........................................   Passed    6.97 sec
  196/196 Test BlockstreamResearch#126: secp256k1.tests.ecmult_constants ..................................   Passed    9.60 sec

  100% tests passed, 0 tests failed out of 196

  Label Time Summary:
  secp256k1_example           =   0.02 sec*proc (5 tests)
  secp256k1_exhaustive        =   6.97 sec*proc (1 test)
  secp256k1_noverify_tests    =  23.77 sec*proc (95 tests)
  secp256k1_tests             =  43.67 sec*proc (95 tests)

  Total Test time (real) =  10.21 sec
  ```

  For comparison, here is the output for the master branch on the same machine:
  ```
  $ ctest --test-dir build -j $(nproc)
  Test project /home/hebasto/dev/secp256k1/secp256k1/build
      Start 1: secp256k1_noverify_tests
      Start 2: secp256k1_tests
      Start 3: secp256k1_exhaustive_tests
      Start 4: secp256k1_ecdsa_example
      Start 5: secp256k1_ecdh_example
      Start 6: secp256k1_schnorr_example
      Start 7: secp256k1_ellswift_example
      Start 8: secp256k1_musig_example
  1/8 Test #4: secp256k1_ecdsa_example ..........   Passed    0.00 sec
  2/8 Test #5: secp256k1_ecdh_example ...........   Passed    0.00 sec
  3/8 Test #6: secp256k1_schnorr_example ........   Passed    0.00 sec
  4/8 Test #7: secp256k1_ellswift_example .......   Passed    0.00 sec
  5/8 Test #8: secp256k1_musig_example ..........   Passed    0.00 sec
  6/8 Test #3: secp256k1_exhaustive_tests .......   Passed    6.26 sec
  7/8 Test #1: secp256k1_noverify_tests .........   Passed   14.31 sec
  8/8 Test #2: secp256k1_tests ..................   Passed   31.65 sec

  100% tests passed, 0 tests failed out of 8

  Total Test time (real) =  31.65 sec
  ```

  ---

  **New Feature:** As the number of tests has grown, the _labels_ have been introduced to simplify test management. Now, one can run:
  ```
  $ ctest --test-dir build -j $(nproc) -L example
  Test project /home/hebasto/dev/secp256k1/secp256k1/build
      Start 192: secp256k1.example.ecdsa
      Start 193: secp256k1.example.ecdh
      Start 194: secp256k1.example.schnorr
      Start 195: secp256k1.example.ellswift
      Start 196: secp256k1.example.musig
  1/5 Test BlockstreamResearch#192: secp256k1.example.ecdsa ..........   Passed    0.00 sec
  2/5 Test BlockstreamResearch#193: secp256k1.example.ecdh ...........   Passed    0.00 sec
  3/5 Test BlockstreamResearch#194: secp256k1.example.schnorr ........   Passed    0.00 sec
  4/5 Test BlockstreamResearch#195: secp256k1.example.ellswift .......   Passed    0.00 sec
  5/5 Test BlockstreamResearch#196: secp256k1.example.musig ..........   Passed    0.00 sec

  100% tests passed, 0 tests failed out of 5

  Label Time Summary:
  secp256k1_example    =   0.01 sec*proc (5 tests)

  Total Test time (real) =   0.01 sec
  ```
  or
  ```
  $ ctest --test-dir build -j $(nproc) -LE tests
  Test project /home/hebasto/dev/secp256k1/secp256k1/build
      Start 192: secp256k1.example.ecdsa
      Start 193: secp256k1.example.ecdh
      Start 194: secp256k1.example.schnorr
      Start 195: secp256k1.example.ellswift
      Start 196: secp256k1.example.musig
      Start 191: secp256k1.exhaustive_tests
  1/6 Test BlockstreamResearch#192: secp256k1.example.ecdsa ..........   Passed    0.00 sec
  2/6 Test BlockstreamResearch#193: secp256k1.example.ecdh ...........   Passed    0.00 sec
  3/6 Test BlockstreamResearch#194: secp256k1.example.schnorr ........   Passed    0.00 sec
  4/6 Test BlockstreamResearch#195: secp256k1.example.ellswift .......   Passed    0.00 sec
  5/6 Test BlockstreamResearch#196: secp256k1.example.musig ..........   Passed    0.00 sec
  6/6 Test BlockstreamResearch#191: secp256k1.exhaustive_tests .......   Passed    6.19 sec

  100% tests passed, 0 tests failed out of 6

  Label Time Summary:
  secp256k1_example       =   0.01 sec*proc (5 tests)
  secp256k1_exhaustive    =   6.19 sec*proc (1 test)

  Total Test time (real) =   6.20 sec
  ```

ACKs for top commit:
  purpleKarrot:
    ACK 8354618
  furszy:
    Tested ACK 8354618

Tree-SHA512: 8c506ab08491aba4836b3058a8a09c929c6dd097c11e4e6f4deb20cf602285e73c3fd8a2c2040f7e92a058c7f8fc09752fa9de2ce80f7673adbdd505237ed262
@real-or-random real-or-random deleted the temp-merge-901 branch March 30, 2026 11:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

8 participants