b43dd83b43 musig: add missing static keyword to function (Jonas Nick)
068e6a036a musig: add test vectors from BIP MuSig (Jonas Nick)
36621d13be musig: update to BIP v1.0.0-rc.2 "Add ''pk'' arg to ''NonceGen''" (Jonas Nick)
d717a4980b musig: update to BIP v0.8 "Switch from X-only to plain pk inputs." (Jonas Nick)
304f1bc96d extrakeys: add pubkey_sort test vectors from BIP MuSig2 (Jonas Nick)
ae89051547 extrakeys: replace xonly_sort with pubkey_sort (Jonas Nick)
98242fcdd9 extrakeys: add secp256k1_pubkey_cmp (Jonas Nick)
73d5b6654d musig: update to BIP v0.7.0 (NonceGen) (Jonas Nick)
060887e9d7 musig: update to BIP v0.5.1 "Rename ordinary tweaking to plain" (Jonas Nick)
cbe2815633 musig: update to BIP v0.4 "Allow the output of NonceAgg to be inf" (Jonas Nick)
206017d67d musig: update to BIP v0.3 (NonceGen) (Jonas Nick)
d800dd55db musig: remove test vectors (Jonas Nick)
Pull request description:
Version 1.0.0-rc.3 of BIP MuSig2 can be found [here](https://github.com/jonasnick/bips/pull/75). This PR does _not_ implement the following optional features that have been added to BIP MuSig2:
- variable length messages
- deterministic signing
- identifiable aborts
The PR also does _not_ yet change the `secnonce` structure to also contain the signer's public key (which would also imply changing the seckey argument in `sign` to a keypair). Additionally, we may want to rename some things in the future to be more consistent with the BIP (e.g. keyagg_cache vs. keyagg_ctx, applytweak vs. tweak_add).
ACKs for top commit:
ariard:
Light Code Review ACK b43dd83b, mostly looks on how the user API will make sense for Lightning, thanks for the answers!
real-or-random:
ACK b43dd83b43
Tree-SHA512: 9b1410951b55a1b0e6590b8c302052996d1fb6d9771765498b4282ff68b44ab0d6add8144c9330217b682ec5a93508b5546099db9a1f2c865f99253010dd76f4
ce3cfc78a6 doc: Describe Jacobi calculation in safegcd_implementation.md (Elliott Jin)
6be01036c8 Add secp256k1_fe_is_square_var function (Pieter Wuille)
1de2a01c2b Native jacobi symbol algorithm (Pieter Wuille)
04c6c1b181 Make secp256k1_modinv64_det_check_pow2 support abs val (Pieter Wuille)
5fffb2c7af Make secp256k1_i128_check_pow2 support -(2^n) (Pieter Wuille)
Pull request description:
This introduces variants of the vartime divsteps-based GCD algorithm used for modular inverses to compute Jacobi symbols. Changes compared to the normal vartime divsteps:
* Only positive matrices are used, guaranteeing that f and g remain positive.
* An additional jac variable is updated to track sign changes during matrix computation.
* There is (so far) no proof that this algorithm terminates within reasonable amount of time for every input, but experimentally it appears to almost always need less than 900 iterations. To account for that, only a bounded number of iterations is performed (1500), after which failure is returned. The field logic then falls back to using square roots to determining the result.
* The algorithm converges to f=g=gcd(f0,g0) rather than g=0. To keep this test simple, the end condition is f=1, which won't be reached if started with g=0. That case is dealt with specially.
This code is currently unused, except for tests. I don't aim for it to be merged until there is a need for it, but this demonstrates its feasibility.
In terms of performance:
```
field_inverse: min 1.76us / avg 1.76us / max 1.78us
field_inverse_var: min 0.991us / avg 0.993us / max 0.996us
field_jacobi_var: min 1.31us / avg 1.31us / max 1.31us
field_sqrt: min 4.36us / avg 4.37us / max 4.40us
```
while with the older (f24e122d13) libgmp based Jacobi code on the same system:
```
num_jacobi: min 1.53us / avg 1.54us / max 1.55us
```
ACKs for top commit:
jonasnick:
ACK ce3cfc78a6
real-or-random:
reACK ce3cfc78a6 diff and writeup is good and I tested every commit
Tree-SHA512: 8a6204a7a108d8802d942a54faca39917f90ea5923130683bbd870f9025f4ec8ef256ffa1d939a793f0b32d4cdfcdcd1d3f8ae5ed74a0193be7ad98362ce027e
The implementation calls the secp256k1_modinvNN_jacobi_var code, falling back
to computing a square root in the (extremely rare) case it failed converge.
This introduces variants of the divsteps-based GCD algorithm used for
modular inverses to compute Jacobi symbols. Changes compared to
the normal vartime divsteps:
* Only positive matrices are used, guaranteeing that f and g remain
positive.
* An additional jac variable is updated to track sign changes during
matrix computation.
* There is (so far) no proof that this algorithm terminates within
reasonable amount of time for every input, but experimentally it
appears to almost always need less than 900 iterations. To account
for that, only a bounded number of iterations is performed (1500),
after which failure is returned. In VERIFY mode a lower iteration
count is used to make sure that callers exercise their fallback.
* The algorithm converges to f=g=gcd(f0,g0) rather than g=0. To keep
this test simple, the end condition is f=1, which won't be reached
if started with non-coprime or g=0 inputs. Because of that we only
support coprime non-zero inputs.
e4330341bd ci: Shutdown wineserver whenever CI script exits (Tim Ruffing)
9a5a611a21 build: Suppress stupid MSVC linker warning (Tim Ruffing)
739c53b19a examples: Extend sig examples by call that uses static context (Tim Ruffing)
914276e4d2 build: Add SECP256K1_API_VAR to fix importing variables from DLLs (Tim Ruffing)
Pull request description:
... and more Windows fixes, please see the individual commits.
The fixed issues were discovered in https://github.com/bitcoin-core/secp256k1/pull/1198.
ACKs for top commit:
sipa:
utACK e4330341bd
hebasto:
ACK e4330341bd, tested on Windows using [CMake](https://github.com/bitcoin-core/secp256k1/pull/1113) (which means that the 3rd commit is reviewed only, but not tested). FWIW, `LNK4217` warnings have been indeed observed.
Tree-SHA512: ce7845b106190cdc517988c30aaf2cc9f1d6da22904dfc5cb6bf4ee05f063929dc8b3038479e703b6cebac79d1c21d0c84560344d2478cb1c1740087383f40e3
e089eecc1e group: Further simply gej_add_ge (Tim Ruffing)
ac71020ebe group: Save a normalize_to_zero in gej_add_ge (Tim Ruffing)
Pull request description:
As discovered by sipa in #1033.
See commit message for reasoning but note that the infinity handling will be replaced in the second commit again.
ACKs for top commit:
sipa:
ACK e089eecc1e
apoelstra:
ACK e089eecc1e
Tree-SHA512: fb1b5742e73dd8b2172b4d3e2852490cfd626e8673b72274d281fa34b04e9368a186895fb9cd232429c22b14011df136f4c09bdc7332beef2b3657f7f2798d66
- 0.7.0: Change ''NonceGen'' such that output when message is not present is different from when message is present but has length 0.
- 0.6.0: Change order of arguments and serialization of the message in the ''NonceGen'' hash function
Silence a compiler warning about an unitialized use of a scalar in case
the user tries to provide a 0-length list of commitments.
Also ensures that commitments have normalized field elements when they
are loaded into ges.
Besides improving the examples, this makes sure that the examples
import a variable (instead of a function), namely the static context,
from the library. This is helpful when testing MSVC builds, because
the MSVC linker tends to be awkward when importing variables.
This fixes a build issue with MSVC. While MSVC imports *functions*
from DLLs automatically when building a consumer of the DLL, it does
not import *variables* automatically. In these cases, we need an
explicit __declspec(dllimport).
This commit simply changes our logic to what the libtool manual
suggests, which has a very comprehensive writeup on the topic. Note
that in particular, this solution is carefully designed not to break
static linking. However, as described in the libtool manual,
statically linking the library with MSVC will output warning LNK4217.
This is still the best solution overall, because the warning is
merely a cosmetic issue.
8c7e0fc1de build: Add -Wreserved-identifier supported by clang (Tim Ruffing)
Pull request description:
This warns on certain identifiers reserved by the C standard, namely
* identifiers that begin with an underscore followed by an uppercase letter, and
* identifiers in the global namespace that begin with an underscore.
We had used such identifiers in the past for macros in include guards, and we should make sure that we don't reintroduce such identifiers going forward.
Note that C reserves more identifiers for "future library directions", e.g., identifiers that begin with "str" followed by a lowercase letter. But even the C standards committee has decided that this is somewhat silly and adopted a proposal [1] for C23 that removes the restriction that programs using these identifiers have UB. Instead, these identifiers are now "potentially reserved", which is not a normative restriction but simply an informative warning that the identifiers may become fully reserved in the future.
[1] https://www.open-std.org/jtc1/sc22/WG14/www/docs/n2625.pdf
ACKs for top commit:
sipa:
utACK 8c7e0fc1de
jonasnick:
tested ACK 8c7e0fc1de
Tree-SHA512: da0c5f1e36cffad2ab2f0b8055c8b3cb56e904d8bfea5a9eed9d6fa984359217b3ef3b9232bfb455cf4071c04a6c2a077e26d2a15b20d1eabc99b1fc61d2025c
This warns on certain identifiers reserved by the C standard, namely
* identifiers that begin with an underscore followed by an uppercase
letter, and
* identifiers in the global namespace that begin with an underscore.
We had used such identifiers in the past for macros in include guards,
and we should make sure that we don't reintroduce such identifiers
going forward.
Note that C reserves more identifiers for "future library directions",
e.g., identifiers that begin with "str" followed by a lowercase letter.
But even the C standards committee has decided that this is somewhat
silly and adopted a proposal [1] for C23 that removes the restriction
that programs using these identifiers have UB. Instead, these
identifiers are now "potentially reserved", which is not a normative
restriction but simply an informative warning that the identifiers
may become fully reserved in the future.
[1] https://www.open-std.org/jtc1/sc22/WG14/www/docs/n2625.pdf
9b60e3148d ci: Do not set git's `user.{email,name}` config options (Hennadii Stepanov)
Pull request description:
A cleanup after https://github.com/bitcoin-core/secp256k1/pull/1199.
git's `user.{email,name}` config options have been no longer required since 0ecf318851.
ACKs for top commit:
real-or-random:
utACK 9b60e3148d
Tree-SHA512: 04f737b0549a91ca992cd1410420e041549a07869eeef068e08971781ea8a4c88a2486e789df36a5ad370ccbbf5d9f7e49ab5f7c1d01faef358ffc4863aaf8e4
ef39721ccc Do not link `bench` and `ctime_tests` to `COMMON_LIB` (Hennadii Stepanov)
Pull request description:
The `bench` and `ctime_tests` binaries are users of the library, they should only be linked to the library, not the objects it was built from.
ACKs for top commit:
sipa:
utACK ef39721ccc
real-or-random:
utACK ef39721ccc
Tree-SHA512: 8bf8330adcce9bf6b21aceacf86e6aff7594762ab68b09257cfe2904fa0ce827377d5a13c0bed5acde74a2b420bb49460657c66d0068ecbe36dc162140876be4
c2415866c7 ci: Don't fetch git history (Tim Ruffing)
0ecf318851 ci: Use remote pull/merge ref instead of local git merge (Tim Ruffing)
Pull request description:
This steals two recent CI improvements from bitcoin/bitcoin. See individual commit messages.
ACKs for top commit:
sipa:
utACK c2415866c7
Tree-SHA512: 966130f45767c6bee8bc041d7e90a3166591a54c7cfccdcf4dff99aa4f6ccc2d02544fa7dca9fd020241349775da3cbd9bdbb041fcdd32de7426efd9dcc9c7f8
