Also add a specific test that fails adaptor sig deserialization because with the
correct size buffer that's not guaranteed anymore with the existing test.
Previously the ECDSA signature had an overflowing s value, which after the sync
with upstream results in a failing VERIFY_CHECK in the inversion function.
However, normally parsed signatures shouldn't contain overflowing s values.
unsigned char foo[4] = "abcd" is not valid C++ because the string
literal "abcd" does not fit into foo due to the terminating NUL
character. This is valid in C, it will just omit the NUL character.
Fixes#962.
5f6ceafcfa schnorrsig: allow setting MSGLEN != 32 in benchmark (Jonas Nick)
fdd06b7967 schnorrsig: add tests for sign_custom and varlen msg verification (Jonas Nick)
d8d806aaf3 schnorrsig: add extra parameter struct for sign_custom (Jonas Nick)
a0c3fc177f schnorrsig: allow signing and verification of variable length msgs (Jonas Nick)
5a8e4991ad Add secp256k1_tagged_sha256 as defined in BIP-340 (Jonas Nick)
b6c0b72fb0 schnorrsig: remove noncefp args from sign; add sign_custom function (Jonas Nick)
442cee5baf schnorrsig: add algolen argument to nonce_function_hardened (Jonas Nick)
df3bfa12c3 schnorrsig: clarify result of calling nonce_function_bip340 without data (Jonas Nick)
99e8614812 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 5f6ceafcfa
Tree-SHA512: cf1716dddf4f29bcacf542ed22622a817d0ec9c20d0592333cb7e6105902c77d819952e776b9407fae1333cbd03d63fded492d3a5df7769dcc5b450d91bb4761
If `secp256k1_ecdsa_sign` fails, the signature which is then loaded by
`secp256k1_ecdsa_signature_load` is garbage. Exit early with an error
when this occurs.
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`).
Varlen message support for the default sign function comes from recommending
tagged_sha256. sign_custom on the other hand gets the ability to directly sign
message of any length. This also implies signing and verification support for
the empty message (NULL) with msglen 0.
Tests for variable lengths follow in a later commit.
This makes the default sign function easier to use while allowing more granular
control through sign_custom.
Tests for sign_custom follow in a later commit.
9570f674cc Avoid passing out-of-bound pointers to 0-size memcpy (Pieter Wuille)
Pull request description:
Doing so could be considered UB in a pedantic interpretation of the standard. Avoid it.
Closes#876.
ACKs for top commit:
practicalswift:
cr ACK 9570f674cc: patch looks correct
real-or-random:
ACK 9570f674cc
Tree-SHA512: f991462d72e39f14e609021b8427c2e6756009bc8cd21efca2da46ec9410250725a4fed662df20fcdcfd10a4dc59038f13e8c166362b2eadde4366586b9ca72b
This reverts commit 20448b8d09.
The removed functions secp256k1_ge_set_xquad and secp256k1_fe_is_quad_var
are required for some modules in secp256k1-zkp.
14c9739a1f tests: Improve secp256k1_ge_set_all_gej_var for some infinity inputs (Tim Ruffing)
4a19668c37 tests: Test secp256k1_ge_set_all_gej_var for all infinity inputs (Tim Ruffing)
45b6468d7e 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)
31c0f6de41 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)
dd6c3de322 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 14c9739a1f
sipa:
utACK 14c9739a1f
real-or-random:
ACK 14c9739a1f
Tree-SHA512: 2e779b767f02e348af4bbc62aa9871c3d1d29e61a6c643c879c49f2de27556a3588850acd2f7c7483790677597d01064025e14befdbf29e783f57996fe4430f9
