ef49a11d29 build: allow static or shared but not both (Cory Fields)
36b0adf1b9 build: remove warning until it's reproducible (Cory Fields)
Pull request description:
Continuing from here: https://github.com/bitcoin-core/secp256k1/issues/1224#issuecomment-1460438227
Unfortunately it wasn't really possible to keep a clean diff here because of the nature of the change. I suggest reviewing the lib creation stuff in its entirety, sorry about that :\
Rather than allowing for shared and static libs to be built at the same time like autotools, this PR switches to the CMake convention of allowing only 1.
A new `BUILD_SHARED_LIBS` option is added to match CMake convention, as well as a `SECP256K1_DISABLE_SHARED` option which overrides it. That way even projects which have `BUILD_SHARED_LIBS=1` can opt-into a static libsecp in particular.
Details:
Two object libraries are created: `secp256k1_asm` and `secp256k1_precomputed_objs`. Some tests/benchmarks use the object libraries directly, some link against the real lib: `secp256k1`.
Because the objs don't know what they're going to be linked into, they need to be told how to deal with PIC.
The `DEFINE_SYMBOL` property sets the `DLL_EXPORT` define as necessary (when building a shared lib)
ACKs for top commit:
hebasto:
re-ACK ef49a11d29, only [suggested](https://github.com/bitcoin-core/secp256k1/pull/1230#pullrequestreview-1388191165) changes since my recent [review](https://github.com/bitcoin-core/secp256k1/pull/1230#pullrequestreview-1352125381).
real-or-random:
ACK ef49a11d29
Tree-SHA512: 8870de305176fdb677caff0fdfc6f8c59c0e906489cb72bc9980e551002812685e59e20d731f2a82e33628bdfbb7261eafd6f228038cad3ec83bd74335959600
a575339c02 Remove bits argument from secp256k1_wnaf_const (always 256) (Pieter Wuille)
Pull request description:
There is little reason for having the number of bits in the scalar as a parameter, as I don't think there are any (current) use cases for non-256-bit scalars.
ACKs for top commit:
jonasnick:
ACK a575339c02
real-or-random:
utACK a575339c02
Tree-SHA512: 994b1f19b4c513f6d070ed259a5d6f221a0c2450271ec824c5eba1cd0ecace276de391c170285bfeae96aaf8f1e0f7fe6260966ded0336c75c522ab6c56d182c
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
06c67dea9f autotools: Don't regenerate Wycheproof header automatically (Tim Ruffing)
Pull request description:
This is a hot fix for https://github.com/bitcoin/bitcoin/pull/27445 .
---
Pregenerated files that we distribute should not have dependencies in Makefile.am. For rationale, see the comments about the precomputed table files.
See also https://github.com/bitcoin/bitcoin/pull/27445#issuecomment-1502994264 .
ACKs for top commit:
hebasto:
ACK 06c67dea9f
RandomLattice:
ACK 06c67dea9f
Tree-SHA512: fa7f44eaa1c7e42ecba5829ac1b8ae8b5826d1a1551e01c3caf37af780bd5c102c8f54e88520723937f7016d93c67b62a334c7a28b96c4f422a38fcf8e6a1984
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
6a37b2a5ea changelog: Fix link (Tim Ruffing)
Pull request description:
Top commit has no ACKs.
Tree-SHA512: 70d50c8fe958a197eb527e51c6f8120609e3166d93bfc1bbec75a3cb565c406d5ba0e6d088a724dcfda422b6594abf53f507211946a0533515df371d5d2a91bf
e5de454609 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 e5de454609
real-or-random:
ACK e5de454609
Tree-SHA512: e9684f14ff3f5225a4a4949b490e07527d559c28aa61ed03c03bc52ea64785f0b80b9e1b1628665eacf24006526271ea0fb108629c9c3c1d758e52d214a056f1
0f8642079b Add exhaustive tests for ecmult_const_xonly (Pieter Wuille)
4485926ace 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 #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 0f8642079b
real-or-random:
ACK 0f8642079b
Tree-SHA512: eeedb3045bfabcb4bcaf3a1738067c83a5ea9a79b150b8fd1c00dc3f68505d34c19654885a90e2292ae40ddf40a58dfb27197d98eebcf5d6d9e25897e07ae595
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>
3d1f430f9f Make position of * in pointer declarations in include/ consistent (Jonas Nick)
Pull request description:
ACKs for top commit:
sipa:
utACK 3d1f430f9f. 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 3d1f430f9f
Tree-SHA512: 6361880f4a47e58c83623f094dd121882752fa805e275033cd638d1e8d3477ade9037e5d9e34a57ae46013848648bd7ab764cad326133f2d3435c9a70a0c841b
4a496a36fb 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 https://github.com/bitcoin-core/secp256k1/pull/864#issuecomment-769211867
ACKs for top commit:
jonasnick:
ACK 4a496a36fb
Tree-SHA512: a6010f9d752e45f01f88b804a9b27e77caf5ddf133ddcbc4235b94698bda41c9276bf588c93710e538250d1a96844bcec198ec5459e675f166ceaaa42da921d5
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
5bb03c2911 Replace `SECP256K1_ECMULT_TABLE_VERIFY` macro by a function (Hennadii Stepanov)
4429a8c218 Suppress `-Wunused-parameter` when building for coverage analysis (Hennadii Stepanov)
Pull request description:
ACKs for top commit:
real-or-random:
utACK 5bb03c2911
jonasnick:
ACK 5bb03c2911
Tree-SHA512: 19a395434ecefea201a03fc45b3f0b88f1520908926ac1207bbc6570034b1141b49c3c98e66819dcd9069dfdd28c7c6fbe957f13fb6bd178fd57ce65bfbb8fbd
3e43041be6 No need to subtract 1 before doing a right shift (roconnor-blockstream)
Pull request description:
ACKs for top commit:
real-or-random:
utACK 3e43041be6
jonasnick:
ACK 3e43041be6
Tree-SHA512: bcecda11eae3fb845bef7af88c6171bedcd933872d08a9849c0a250cb6c9e982a88bd45e8a8364a4a348f8be413fc91ee04cf8fa78adae44e584e3ad7ec544cf
fd2a408647 Set ARM ASM symbol visibility to `hidden` (Hennadii Stepanov)
Pull request description:
Solves one item in #1181.
To test on arm-32bit hardware, run:
```
$ ./autogen.sh && ./configure --enable-experimental --with-asm=arm && make
```
On master branch (427bc3cdcf):
```
$ nm -D .libs/libsecp256k1.so | grep secp256k1_fe
0000e2bc T secp256k1_fe_mul_inner
0000e8dc T secp256k1_fe_sqr_inner
```
With this PR:
```
$ nm -D .libs/libsecp256k1.so | grep secp256k1_fe | wc -l
0
```
For reference, see https://sourceware.org/binutils/docs/as/Hidden.html.
ACKs for top commit:
theuni:
ACK fd2a408647.
sipa:
ACK fd2a408647
Tree-SHA512: abf8ad332631672c036844f69c5599917c49e12c4402bf9066f93a692d3007b1914bd3eea8f83f0141c1b09d5c88ebc5e6c8bfbb5444b7b3471749f7b901ca59
4ebd82852d Apply Checks only in VERIFY mode. (roconnor-blockstream)
Pull request description:
This is already done in `field_5x52_impl.h`.
ACKs for top commit:
sipa:
ACK 4ebd82852d
jonasnick:
ACK 4ebd82852d
Tree-SHA512: c24211e5219907e41e2c5792255734bd50ca5866a4863abbb3ec174ed92d1792dd10563a94c08e8fecd6cdf776a9c49ca87e8f9806a023d9081ecc0d55ae3e66
96dd062511 build: bump CMake minimum requirement to 3.13 (Cory Fields)
Pull request description:
As requested here: https://github.com/bitcoin-core/secp256k1/pull/1230#issuecomment-1464730218 . Ping @hebasto
Among other things this allows us to link against object libraries.
3.13 has been mentioned several times as a good overlap between newish features and widespread Linux availability.
ACKs for top commit:
hebasto:
ACK 96dd062511
real-or-random:
utACK 96dd062511
Tree-SHA512: 6c744809aa393b48ef10b3d46c6630370c388a8d375116bfad65c6c907e69c36ed71c1579b9d5c3aa976f70b1cd70e837c1a0226910a43539435125115b32568
