Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Upstream PRs 1228, 1236, 1243, 1238, 1246, 1247, 1242, 1250, 1244, 1241, 1257, 1226, 1252, 1118, 1245, 1266, 1269 #253

Closed
wants to merge 274 commits into from

Conversation

jonasnick
Copy link
Contributor

[bitcoin-core/secp256k1#1228]: release cleanup: bump version after 0.3.0
[bitcoin-core/secp256k1#1236]: Update comment for secp256k1_modinv32_inv256
[bitcoin-core/secp256k1#1243]: build: Ensure no optimization when building for coverage analysis
[bitcoin-core/secp256k1#1238]: build: bump CMake minimum requirement to 3.13
[bitcoin-core/secp256k1#1246]: Typo
[bitcoin-core/secp256k1#1247]: Apply Checks only in VERIFY mode.
[bitcoin-core/secp256k1#1242]: Set ARM ASM symbol visibility to hidden
[bitcoin-core/secp256k1#1250]: No need to subtract 1 before doing a right shift
[bitcoin-core/secp256k1#1244]: Suppress -Wunused-parameter when building for coverage analysis
[bitcoin-core/secp256k1#1241]: build: Improve SECP_TRY_APPEND_DEFAULT_CFLAGS macro
[bitcoin-core/secp256k1#1257]: ct: Use volatile "trick" in all fe/scalar cmov implementations
[bitcoin-core/secp256k1#1226]: Add CMake instructions to release process
[bitcoin-core/secp256k1#1252]: Make position of * in pointer declarations in include/ consistent
[bitcoin-core/secp256k1#1118]: Add x-only ecmult_const version with x specified as n/d
[bitcoin-core/secp256k1#1245]: tests: Add Wycheproof ECDSA vectors
[bitcoin-core/secp256k1#1266]: release: Prepare for 0.3.1
[bitcoin-core/secp256k1#1269]: changelog: Fix link

This PR can be recreated with ./contrib/sync-upstream.sh -b sync-upstream range v0.3.1.
Tip: Use git show --remerge-diff to show the changes manually added to the merge commit.

  • Made position of * consistent in -zkp include/ files.

real-or-random and others added 30 commits June 30, 2022 12:17
… group.h

069aba8 Fix sepc256k1 -> secp256k1 typo in group.h (henopied)

Pull request description:

ACKs for top commit:
  real-or-random:
    ACK 069aba8

Tree-SHA512: 0fcb7d042f201737870da99f5425c8449e9ec3f5f8e9bbe5eb719e46cdf230db057509fb9102d4ce50a94d616015233c29249665c754e726899174fea3ea9f40
This simplifies building without a build system.

This is in line with #925; the paths fixed here were either forgotten
there or only introduced later. This commit also makes the Makefile
stricter so that further "wrong" #include paths will lead to build
errors even in autotools builds.

This belongs to #929.

Co-authored-by: Hennadii Stepanov <32963518+hebasto@users.noreply.github.com>
… get rid of further -I arguments

40a3473 build: Fix #include "..." paths to get rid of further -I arguments (Tim Ruffing)

Pull request description:

  This simplifies building without a build system.

  This is in line with #925; the paths fixed here were either forgotten
  there or only introduced later. This commit also makes the Makefile
  stricter so that further "wrong" #include paths will lead to build
  errors even in autotools builds.

  This belongs to #929.

ACKs for top commit:
  hebasto:
    ACK 40a3473

Tree-SHA512: 6f4d825ea3cf86b13f294e2ec19fafc29660fa99450e6b579157d7a6e9bdb3404d761edf89c1135fa89b984d6431a527beeb97031dc90f2fae9761528f4d06d1
Running the RNG is pointless if no seed is available because the key
will be fixed. The computation just wastes time.

Previously, users could avoid this computation at least by asking for
a context without signing capabilities. But since 3b0c218 we always
build an ecmult_gen context, ignoring the context flags. Moreover,
users could never avoid this pointless computation when asking for
the creation of a signing context.
Whenever I read this code, I first think that rescaling ctx->initial is
a dead store because we overwrite it later with gb. But that's wrong.
The rescaling blinds the computation of gb and affects its result.
This simplifies manual builds and solves one item in #929.
… blinding if no seed is available

55f8bc9 ecmult_gen: Improve comments about projective blinding (Tim Ruffing)
7a86955 ecmult_gen: Simplify code (no observable change) (Tim Ruffing)
4cc0b1b ecmult_gen: Skip RNG when creating blinding if no seed is available (Tim Ruffing)

Pull request description:

  Running the RNG is pointless if no seed is available because the key
  will be fixed. The computation just wastes time.

  Previously, users could avoid this computation at least by asking for
  a context without signing capabilities. But since 3b0c218 we always
  build an ecmult_gen context, ignoring the context flags. Moreover,
  users could never avoid this pointless computation when asking for
  the creation of a signing context.

  This fixes one item in #1065.

ACKs for top commit:
  sipa:
    ACK 55f8bc9
  apoelstra:
    ACK 55f8bc9

Tree-SHA512: 5ccba56041f94fa8f40a8a56ce505369ff2e0ed20cd7f0bfc3fdfffa5fa7bf826a93602b9b2455a352865a9548ab4928e858c19bb5af7ec221594a3bf25c4f3d
It's unused and thus potentially confusing.
…probability 15/16 instead of 1/4

17065f4 tests: Randomize the context with probability 15/16 instead of 1/4 (Tim Ruffing)

Pull request description:

ACKs for top commit:
  sipa:
    ACK 17065f4
  jonasnick:
    ACK 17065f4

Tree-SHA512: 3b7005770007b922a294be610f23da60b0dde74dfd7585d64a2cb04eaa6ec879de8d21a0ade31c1857019a8dd97260fa3aa167ae16fc55027ef280a3e3feaa6d
…for ECMULT_* config values

c27ae45 config: Remove basic-config.h (Tim Ruffing)
da6514a config: Introduce DEBUG_CONFIG macro for debug output of config (Tim Ruffing)
d0cf55e config: Set preprocessor defaults for ECMULT_* config values (Tim Ruffing)

Pull request description:

ACKs for top commit:
  sipa:
    ACK c27ae45
  hebasto:
    ACK c27ae45, I have reviewed the code and it looks correct.
  jonasnick:
    ACK c27ae45

Tree-SHA512: 56b0f384bd9f42cf7c903bec08f4807db1415ddf9a06676dfe1e638e4d02431c522ef0422585e85429074e0dbb51da4f400cf53e8f883d6e07122731c57be1e3
We had removed `PKG_PROG_PKG_CONFIG` in 21b2eba
(#1090). But then then the not rebased (!) merge of 2be6ba0
(#1084) brought that macro back at another location, without git
complaining about a conflict.

Fixes #1127.
… again (reintroduced by mismerge)

cabe085 configure: Remove pkgconfig macros again (reintroduced by mismerge) (Tim Ruffing)

Pull request description:

  We had removed `PKG_PROG_PKG_CONFIG` in 21b2eba
  (#1090). But then then the not rebased (!) merge of 2be6ba0
  (#1084) brought that macro back at another location, without git
  complaining about a conflict.

  Fixes #1127.

ACKs for top commit:
  fanquake:
    ACK cabe085
  hebasto:
    ACK cabe085
  jonasnick:
    ACK cabe085

Tree-SHA512: ba497503db3a11e631b15c4fe875e62d892971c2c708d90b2f6be684e85d164043ea97c13af0452831eef41f3cf8230cd8a9eafa332dc5b5ae18e118b87c3828
88b0089 readme: Fix line break (Tim Ruffing)
78f5296 readme: Sell "no runtime dependencies" (Tim Ruffing)
ef48f08 readme: Add IRC channel (Tim Ruffing)

Pull request description:

ACKs for top commit:
  apoelstra:
    utACK 88b0089
  sipa:
    ACK 88b0089

Tree-SHA512: 174f1596406f98a19059a18cd4fb993102e5ffb8ec29fcc6d03e27f135fcb526b37204b64055b5e4f0a273daab05d395cf335f26241cf3a29a060041c9ef109b
We don't enable the ECDSA recovery module, because we don't recommend
ECDSA recovery for new protocols. In particular, the recovery API is
prone to misuse: It invites the caller to forget to check the public
key (and the verification function always returns 1).

In general, we also don't recommend ordinary ECDSA for new protocols.
But disabling the ECDSA functions is not possible because they're not
in a module, and let's be honest: disabling ECDSA would mean to ignore
reality blatantly.
The removed line was introduced for `obj/.gitignore` file. Since the
`obj` directory has been removed, it is not longer required.
f5039cb Cleanup `.gitignore` file (Hennadii Stepanov)
798727a Revert "Add test logs to gitignore" (Hennadii Stepanov)

Pull request description:

ACKs for top commit:
  jonasnick:
    ACK f5039cb
  real-or-random:
    ACK f5039cb

Tree-SHA512: 3586329e77958a9bfa06dd84e5b121cd456e93332670d5afc1a6691e165cdfa5a6fd6a61f82be12ec33f2a58b26a13adfedeb177ae1056202e53a530949fc549
$CC, $WRAPPER_CMD and valgrind are not necessarily defined
Currently CHECK is used only in test and bench mark files except for one
usage in `ecmult_impl.h`.

We would like to move the definition of CHECK out of `util.h` so that
`util.h` no longer has a hard dependency on `stdio.h`.

Done in preparation for moving the definition of `CHECK` as part of an
effort to allow secp256k1 to be compiled to WASM as part of
`rust-secp256k1`.
After this commit, int128.h and int128_impl.h are included as follows:
 - .c files which use int128 include int128_impl.h (after util.h)
 - .h files which use int128 include int128.h (after util.h)

This list is exhaustive. util.h needs to included first because it sets
up necessary #defines.
jonasnick and others added 28 commits March 28, 2023 19:39
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 bitcoin-core/secp256k1#864 (comment)
…/scalar cmov implementations

4a496a3 ct: Use volatile "trick" in all fe/scalar cmov implementations (Tim Ruffing)

Pull request description:

  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 bitcoin-core/secp256k1#864 (comment)

ACKs for top commit:
  jonasnick:
    ACK 4a496a3

Tree-SHA512: a6010f9d752e45f01f88b804a9b27e77caf5ddf133ddcbc4235b94698bda41c9276bf588c93710e538250d1a96844bcec198ec5459e675f166ceaaa42da921d5
…process

0c07c82 Add CMake instructions to release process (Tim Ruffing)

Pull request description:

ACKs for top commit:
  hebasto:
    ACK 0c07c82
  jonasnick:
    ACK 0c07c82

Tree-SHA512: a2c38f71cc96766f833f6ed79af1b560501f2d9516843b789de06c9cbffd7a1d9e8709a2f4d08bea8c1c3616301e51942cfa9f11e25e903ee4146c7733a8cb8c
…arations in include/ consistent

3d1f430 Make position of * in pointer declarations in include/ consistent (Jonas Nick)

Pull request description:

ACKs for top commit:
  sipa:
    utACK 3d1f430. I have not verified these are the only instances where changes would need to be made.
  apoelstra:
    utACK 3d1f430 from me too. I also value consistency more than either specific choice.'
  real-or-random:
    utACK bitcoin-core/secp256k1@3d1f430

Tree-SHA512: 6361880f4a47e58c83623f094dd121882752fa805e275033cd638d1e8d3477ade9037e5d9e34a57ae46013848648bd7ab764cad326133f2d3435c9a70a0c841b
Adds a test using the Wycheproof vectors as outlined in #1106. The
vectors are taken from the Wycheproof repo. We use a python script
to convert the JSON-formatted vectors into C code.

Co-authored-by: Sean Andersen <6730974+andozw@users.noreply.github.com>
…th x specified as n/d

0f86420 Add exhaustive tests for ecmult_const_xonly (Pieter Wuille)
4485926 Add x-only ecmult_const version for x=n/d (Pieter Wuille)

Pull request description:

  This implements a generalization of Peter Dettman's sqrt-less x-only random-base multiplication algorithm from BlockstreamResearch#262, using the Jacobi symbol algorithm from #979. The generalization is to permit the X coordinate of the base point to be specified as a fraction $n/d$:

  To compute $x(q \cdot P)$, where $x(P) = n/d$:
  * Compute $g=n^3 + 7d^3$.
  * Let $P' = (ng, g^2, 1)$ (the Jacobian coordinates of $P$ mapped to the isomorphic curve $y^2 = x^3 + 7(dg)^3$).
  * Compute the Jacobian coordinates $(X',Y',Z') = q \cdot P'$ on the isomorphic curve.
  * Return $X'/(dgZ'^2)$, which is the affine x coordinate on the isomorphic curve $X/Z'^2$ mapped back to secp256k1.

  This ability to specify the X coordinate as a fraction is useful in the context of x-only [Elligator Swift](https://eprint.iacr.org/2022/759), which can decode to X coordinates on the curve without inversions this way.

ACKs for top commit:
  jonasnick:
    ACK 0f86420
  real-or-random:
    ACK 0f86420

Tree-SHA512: eeedb3045bfabcb4bcaf3a1738067c83a5ea9a79b150b8fd1c00dc3f68505d34c19654885a90e2292ae40ddf40a58dfb27197d98eebcf5d6d9e25897e07ae595
e5de454 tests: Add Wycheproof ECDSA vectors (RandomLattice)

Pull request description:

  This PR adds a test using the Wycheproof vectors as outlined in #1106. We add all 463 ECDSA test vectors. These vectors cover:
  - edge cases in arithmetic operations
  - signatures with special values for (r,s) that should be rejected
  - special cases of public keys

  The vectors are pulled from the Wycheproof project using a python script to emit C code.

  All the new ECDSA Wycheproof vectors pass.

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

Tree-SHA512: e9684f14ff3f5225a4a4949b490e07527d559c28aa61ed03c03bc52ea64785f0b80b9e1b1628665eacf24006526271ea0fb108629c9c3c1d758e52d214a056f1
Co-authored-by: Pieter Wuille <pieter@wuille.net>
898e1c6 release: Prepare for 0.3.1 (Tim Ruffing)
1d9a13f changelog: Remove inconsistent newlines (Tim Ruffing)
0e09166 changelog: Catch up in preparation of 0.3.1 (Tim Ruffing)

Pull request description:

ACKs for top commit:
  sipa:
    ACK 898e1c6
  jonasnick:
    ACK 898e1c6

Tree-SHA512: 941b1524f8b34ca803a2ede55a7baf54d2faa69a4c5e47254297e96cc4ac2121094ed90e7cd64a708f3e9af830b0de0ef3c755dfee1b01ce958cc998fc1a1311
6a37b2a changelog: Fix link (Tim Ruffing)

Pull request description:

Top commit has no ACKs.

Tree-SHA512: 70d50c8fe958a197eb527e51c6f8120609e3166d93bfc1bbec75a3cb565c406d5ba0e6d088a724dcfda422b6594abf53f507211946a0533515df371d5d2a91bf
…1187

Upstream PRs 1174, 1154, 1178, 1177, 1171, 1158, 1183, 1185, 1186, 1188, 1187
7e91936 ci: Always define EXPERIMENTAL variable (Tim Ruffing)
0a99156 sync-upstream.sh: Add "git show --remerge-diff" tip (Tim Ruffing)
9b6a1c3 sync-upstream.sh: Fix position of "-b" option in reproduce command (Tim Ruffing)
05b207e sync-upstream: allows providing the local branch via cli (Jonas Nick)

Pull request description:

ACKs for top commit:
  real-or-random:
    utACK 7e91936

Tree-SHA512: 4527cb6a2493d210eb7ba6d8f6e717b2acbc07aebdc1c4011cffe23490876a4e795d656a69df2cd50e4e3fe8742c123d9ea493914c148c8fbc93d7d3799e7447
…192, 1194, 1196, 1195, 1170, 1172, 1200, 1199, 1203, 1201, 1206, 1078, 1209, 979, 1212, 1218, 1217, 1221, 1222

5d8f53e Remove redudent checks. (Russell O'Connor)
d232112 Update Changelog (Tim Ruffing)
b081f7e Add secp256k1_fe_add_int function (Pieter Wuille)
2ef1c9b Update overflow check (Russell O'Connor)
5660c13 prevent optimization in algorithms (Harshil Jani)
ce3cfc7 doc: Describe Jacobi calculation in safegcd_implementation.md (Elliott Jin)
6be0103 Add secp256k1_fe_is_square_var function (Pieter Wuille)
1de2a01 Native jacobi symbol algorithm (Pieter Wuille)
04c6c1b Make secp256k1_modinv64_det_check_pow2 support abs val (Pieter Wuille)
5fffb2c Make secp256k1_i128_check_pow2 support -(2^n) (Pieter Wuille)
e433034 ci: Shutdown wineserver whenever CI script exits (Tim Ruffing)
9a5a611 build: Suppress stupid MSVC linker warning (Tim Ruffing)
739c53b examples: Extend sig examples by call that uses static context (Tim Ruffing)
914276e build: Add SECP256K1_API_VAR to fix importing variables from DLLs (Tim Ruffing)
e089eec group: Further simply gej_add_ge (Tim Ruffing)
ac71020 group: Save a normalize_to_zero in gej_add_ge (Tim Ruffing)
8c7e0fc build: Add -Wreserved-identifier supported by clang (Tim Ruffing)
9b60e31 ci: Do not set git's `user.{email,name}` config options (Hennadii Stepanov)
ef39721 Do not link `bench` and `ctime_tests` to `COMMON_LIB` (Hennadii Stepanov)
c241586 ci: Don't fetch git history (Tim Ruffing)
0ecf318 ci: Use remote pull/merge ref instead of local git merge (Tim Ruffing)
9b7d186 Drop no longer used Autoheader macros (Hennadii Stepanov)
eb6beba scalar: restrict split_lambda args, improve doc and VERIFY_CHECKs (Jonas Nick)
7f49aa7 ci: add test job with -DVERIFY (Jonas Nick)
620ba3d benchmarks: fix bench_scalar_split (Jonas Nick)
e39d954 tests: Add CHECK_ILLEGAL(_VOID) macros and use in static ctx tests (Tim Ruffing)
61841fc contexts: Forbid randomizing secp256k1_context_static (Tim Ruffing)
4b6df5e contexts: Forbid cloning/destroying secp256k1_context_static (Tim Ruffing)
8f51229 ctime_tests: improve output when CHECKMEM_RUNNING is not defined (Jonas Nick)
2cd4e3c Drop no longer used `SECP_{LIBS,INCLUDE}` variables (Hennadii Stepanov)
613626f Drop no longer used `SECP_TEST_{LIBS,INCLUDE}` variables (Hennadii Stepanov)
d6ff738 Ensure safety of ctz_debruijn implementation. (Russell O'Connor)
ce60785 Introduce SECP256K1_B macro for curve b coefficient (Pieter Wuille)
4934aa7 Switch to exhaustive groups with small B coefficient (Pieter Wuille)
e03ef86 Make all non-API functions (except main) static (Pieter Wuille)
0f088ec Rename CTIMETEST -> CTIMETESTS (Pieter Wuille)
74b026f Add runtime checking for DECLASSIFY flag (Pieter Wuille)
5e2e6fc Run ctime test in Linux MSan CI job (Pieter Wuille)
1897406 Make ctime tests building configurable (Pieter Wuille)
5048be1 Rename valgrind_ctime_test -> ctime_tests (Pieter Wuille)
6eed6c1 Update error messages to suggest msan as well (Pieter Wuille)
8e11f89 Add support for msan integration to checkmem.h (Pieter Wuille)
8dc6407 Add compile-time error to valgrind_ctime_test (Pieter Wuille)
0db05a7 Abstract interactions with valgrind behind new checkmem.h (Pieter Wuille)
4f1a54e Move valgrind CPPFLAGS into SECP_CONFIG_DEFINES (Pieter Wuille)
d4a6b58 Add `noverify_tests` to `.gitignore` (Hennadii Stepanov)
e862c4a Makefile: add -I$(top_srcdir)/src to CPPFLAGS for precomputed (Matt Whitlock)

Pull request description:

ACKs for top commit:
  real-or-random:
    tACK  BlockstreamResearch@0d540ec

Tree-SHA512: bc54ccf752163ab6e1a12bb8c4e1f9339f4421d2e4f7716c408549514b3c902f2e9f727655799f1eecb085b0026761b04735b17be3c95c6cf54e07fbf7e86477
b40adf2 release: prepare for 0.3.0 (Jonas Nick)
8be82d4 cmake: Rename project to "libsecp256k1" (Hennadii Stepanov)
756b61d readme: Use correct build type in CMake/Windows build instructions (Tim Ruffing)
92098d8 changelog: Add entry for CMake (Tim Ruffing)
e1eb337 ci: Add "x86_64: Windows (VS 2022)" task (Hennadii Stepanov)
10602b0 cmake: Export config files (Hennadii Stepanov)
5468d70 build: Add CMake-based build system (Hennadii Stepanov)

Pull request description:

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

Tree-SHA512: ded76837ee78d3a99daf5e9dbdb3912a1f7efb8b9ea329535e5b5452f8bf6d02bc290dd2378b17a20e1d33b4811c1d88482bf46a57d6c414855b64cf55e38e99
@jonasnick
Copy link
Contributor Author

Opened against wrong branch.

@jonasnick jonasnick closed this Jul 21, 2023
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.