5f6ceafcfa46a69e901bed87e2c5f323b03b1e8c schnorrsig: allow setting MSGLEN != 32 in benchmark (Jonas Nick)
fdd06b7967196a3b34f73a5b19632637b4bde90a schnorrsig: add tests for sign_custom and varlen msg verification (Jonas Nick)
d8d806aaf386c7ead9431649f899ff82b0185aae schnorrsig: add extra parameter struct for sign_custom (Jonas Nick)
a0c3fc177f7f435e593962504182c3861c47d1be schnorrsig: allow signing and verification of variable length msgs (Jonas Nick)
5a8e4991ad443cc0cc613d80380a2db802a4cbce Add secp256k1_tagged_sha256 as defined in BIP-340 (Jonas Nick)
b6c0b72fb06e3c31121f1ef4403d2a229a31ec1c schnorrsig: remove noncefp args from sign; add sign_custom function (Jonas Nick)
442cee5bafbd7419acadf203ca11569e371f1f85 schnorrsig: add algolen argument to nonce_function_hardened (Jonas Nick)
df3bfa12c3b728241d3e61d13f8c976719a3de41 schnorrsig: clarify result of calling nonce_function_bip340 without data (Jonas Nick)
99e8614812bf23798a48c53649957e26e5b12f4a README: mention schnorrsig module (Jonas Nick)
Pull request description:
This is a work in progress because I wanted to put this up for discussion before writing tests. It addresses the TODOs that didn't make it in the schnorrsig PR and changes the APIs of `schnorrsig_sign`, `schnorrsig_verify` and `hardened_nonce_function`.
- Ideally, the new `aux_rand32` argument for `sign` would be const, but didn't find a solution I was happy with.
- Support for variable length message signing and verification supports the [suggested BIP amendment](https://github.com/sipa/bips/issues/207#issuecomment-673681901) for such messages.
- ~~`sign_custom` with its opaque config object allows adding more arguments later without having to change the API again. Perhaps there are other sensible customization options, but I'm thinking of [sign-to-contract/covert-channel](https://github.com/bitcoin-core/secp256k1/pull/590) in particular. It would require adding the fields `unsigned char *s2c_data32` and `secp256k1_s2c_opening *s2c_opening` to the config struct. The former is the data to commit to and the latter is written to by `sign_custom`.~~ (EDIT: see below)
ACKs for top commit:
ariard:
utACK 5f6ceaf
LLFourn:
utACK 5f6ceafcfa46a69e901bed87e2c5f323b03b1e8c
Tree-SHA512: cf1716dddf4f29bcacf542ed22622a817d0ec9c20d0592333cb7e6105902c77d819952e776b9407fae1333cbd03d63fded492d3a5df7769dcc5b450d91bb4761
Function `test_inverse_scalar` contains:
(var ? secp256k1_scalar_inverse_var : secp256k1_scalar_inverse_var)(&l, x); /* l = 1/x */
The two sides of the condition are the same function. This seems to be
an error, as there also exists a non-var function, named
`secp256k1_scalar_inverse`.
Make `test_inverse_scalar` use this other function when `var` is false.
This issue was found using clang's static analyzer, which reported a
"Logic error: Identical expressions in conditional expression" (with
checker `alpha.core.IdenticalExpr`).
This reverts commit 20448b8d09a492afcfcae7721033c13a44a776fd.
The removed functions secp256k1_ge_set_xquad and secp256k1_fe_is_quad_var
are required for some modules in secp256k1-zkp.
14c9739a1fb485bb56dbe3447132a37bcbef4e22 tests: Improve secp256k1_ge_set_all_gej_var for some infinity inputs (Tim Ruffing)
4a19668c37bc77d0165f4a1c0e626e321e9c4a09 tests: Test secp256k1_ge_set_all_gej_var for all infinity inputs (Tim Ruffing)
45b6468d7e3ed9849ed474c71e9a9479de1a77db Have secp256k1_ge_set_all_gej_var initialize all fields. Previous behaviour would not initialize r->y values in the case where infinity is passed in. Furthermore, the previous behaviour wouldn't initialize anything in the case where all inputs were infinity. (Russell O'Connor)
31c0f6de413e521731ad0e63424431b3dd49cec8 Have secp256k1_gej_double_var initialize all fields. Previous behaviour would not initialize r->x and r->y values in the case where infinity is passed in. (Russell O'Connor)
dd6c3de322740a3054cf6a1994a38dc8f201b473 Have secp256k1_ge_set_gej_var initialize all fields. Previous behaviour would not initialize r->x and r->y values in the case where infinity is passed in. (Russell O'Connor)
Pull request description:
Previous behaviour would not initialize `r->x` and `r->y` values in the case where infinity is passed in.
ACKs for top commit:
gmaxwell:
ACK 14c9739a1fb485bb56dbe3447132a37bcbef4e22
sipa:
utACK 14c9739a1fb485bb56dbe3447132a37bcbef4e22
real-or-random:
ACK 14c9739a1fb485bb56dbe3447132a37bcbef4e22
Tree-SHA512: 2e779b767f02e348af4bbc62aa9871c3d1d29e61a6c643c879c49f2de27556a3588850acd2f7c7483790677597d01064025e14befdbf29e783f57996fe4430f9
This commit adds test coverage including Cirrus scripts, Valgrind
constant time tests for secret data, API tests, nonce function tests,
and test vectors from the spec.
Add a new run_inverse_tests that replaces all existing field/scalar inverse tests,
and tests a few identities for fixed inputs, small numbers (-999...999), random
inputs (structured and unstructured), as well as comparing with the output of
secp256k1_fe_inv_all_var.
This adds tests for the modinv{32,64}_impl.h directly (before the functions are used
inside the field/scalar code). It uses a naive implementation of modular multiplication
and gcds in order to verify the modular inverses themselves.
This was detected while running the tests with the `-Wconditional-uninitialized` flag
```
./autogen.sh
CC=clang CFLAGS="-Wconditional-uninitialized" ./configure
make check
```
The resulting warning is a false positive, but setting the value to -1
ensures that the CHECK below will fail if recid is never written to.
As identified in #829 and #833. Fixes#829.
Since we touch this anyway, this commit additionally makes the
identifiers in the benchmark files a little bit more consistent.
The VERIFY macro turns on various paranoid consistency checks, but
the complete functionality should still be tested without it.
This also adds a couple of static test points for extremely small
split inputs/outputs. The existing bounds vectors already check
extremely large outputs.
37dba329c6cb0f7a4228a11dc26aa3a342a3a5d0 Remove unnecessary sign variable from wnaf_const (Jonas Nick)
6bb0b77e158fc2f9e56e4b65b08bcb660d4c588b Fix test_constant_wnaf for -1 and add a test for it. (Jonas Nick)
Pull request description:
ACKs for top commit:
jonasnick:
ACK 37dba329c6cb0f7a4228a11dc26aa3a342a3a5d0
Tree-SHA512: 4a529579f04dfa8d5abded16cbc0ad2747ccdbef41501c984c348ffd154afdd28333d739db60c869410211187850e3c05dfe91dfb184ad8dca8d5c59b3a158ed