8088eddc534cbbb89dd5f892828c4013416c4f2b musig: add test vector for ordinary (non xonly) tweaking (Elliott Jin)
57a17929fc0056efb5436a6001597d656591e1ad musig: add ordinary and xonly tweaking to the example (Jonas Nick)
37107361a0ff3b8764903e3b384cfc12ed484e7a musig: allow ordinary, non-xonly tweaking (Jonas Nick)
c519b468791670654f8b66368a675655cd337ae8 musig: add pubkey_get to obtain a full pubkey from a keyagg_cache (Jonas Nick)
Pull request description:
In short, `musig_pubkey_tweak_add` now allows for xonly _and_ "ordinary" tweaking. Also, in order to allow using `ec_pubkey_tweak_add` on the non-xonly aggregate public key, there's a new function `musig_pubkey_get` that allows obtaining it from the `keyagg_cache`.
One alternative would be that instead of adding `musig_pubkey_get`, we could change `pubkey_agg` to output an ordinary (non-xonly) pubkey. Then users of the API who do not need ordinary (BIP32) tweaking would be forced to call `xonly_pubkey_from_pubkey`. And we'd probably want to change the spec. And it would be a bit weird to output a pubkey that can't be directly schnorrsig_verify'd.
Based on #131
ACKs for top commit:
robot-dreams:
ACK 8088eddc534cbbb89dd5f892828c4013416c4f2b based on https://github.com/ElementsProject/secp256k1-zkp/pull/151#issuecomment-1005198409 and the following `range-diff`:
Tree-SHA512: a4a0100f0470c870f88a8da27dbcc4684fcc2caabb368d4340e962e08d5ee04634e6289bafa3448dbfd0b5793a3e70de5bd6ddca7a619cc3220ff762d518a8fe
070e772211b3fcd297577b90b56bbf7a5cfbd0a3 Faster fixed-input ecmult tests (Pieter Wuille)
Pull request description:
Given how much #920 slowed down the tests with low iteration count, replace it with 3 different similar test:
* count >= 1: a test with 1024 multiplies that tests any pattern of 6 bits in windows not more than 20 bits wide
* count >= 3: a test with 2048 multiplies that tests any pattern of 8 consecutive bits
* count >= 35: the old test (which effectively tests all 2-bit patterns)
ACKs for top commit:
robot-dreams:
ACK 070e772211b3fcd297577b90b56bbf7a5cfbd0a3, the addition of the `CONDITIONAL_TEST` macro is nice.
real-or-random:
ACK 070e772211b3fcd297577b90b56bbf7a5cfbd0a3
Tree-SHA512: b4ccca42c71fcd1baa7143f73d1c3ac9d012c296485164a03341dbeee02e4ba9f7c7ad6b441923a5fe0286c97eff60815033adb4e1d30b3ef08bcb79590327ff
3d7cbafb5fd7f152fc47dc907af5df03150accc0 tests: Fix test whose result is implementation-defined (Tim Ruffing)
Pull request description:
A compiler may add struct padding and fe_cmov is not guaranteed to
preserve it.
On the way, we restore the name of the function. It was mistakenly
renamed in 6173839c90553385171d560be8a17cbe167e3bef using
"search and replace".
ACKs for top commit:
robot-dreams:
ACK 3d7cbafb5fd7f152fc47dc907af5df03150accc0
sipa:
utACK 3d7cbafb5fd7f152fc47dc907af5df03150accc0
Tree-SHA512: f8bb643d4915e9ce9c4fe45b48a2878f6cf1f29e654be1c150cdf65c6959cf65f8491928cf098da5a01f1d488ba475914905ca96b232abed499eb6ed65e53fb8
77a19750b46916b93bb6a08837c26f585bd940fa Use xoshiro256++ PRNG instead of RFC6979 in tests (Pieter Wuille)
5f2efe684ecca8f767f98ee0ace813103cc88ade secp256k1_testrand_int(2**N) -> secp256k1_testrand_bits(N) (Pieter Wuille)
Pull request description:
Just some easy low-hanging fruit. It's complete overkill to use the RFC6979 RNG for our test randomness. Replace it with a modern non-cryptographic RNG with good properties. It's a few % speedup for me.
Given the internal naming of all these functions to be "testrand", I'm not concerned about the risk of someone using this for something that needs actual cryptographic randomness.
ACKs for top commit:
robot-dreams:
ACK 77a19750b46916b93bb6a08837c26f585bd940fa
real-or-random:
utACK 77a19750b46916b93bb6a08837c26f585bd940fa
Tree-SHA512: 2706f37689e037e84b5df25c98af924c0756e6d59f5f822b23aec5ba381b2d536e0848f134026e2568396427218f1c770f1bb07613d702efb23a84015dc9271d
A compiler may add struct padding and fe_cmov is not guaranteed to
preserve it.
On the way, we improve the identity check such that it covers the
VERIFY struct members.
b4ac1a1d5f4d51b9836ac310b78bc9d4256580c2 ci: Run valgrind/memcheck tasks with 2 CPUs (Tim Ruffing)
e70acab601aecf3c5a8affb5a4dce5612b298964 ci: Use Cirrus "greedy" flag to use idle CPU time when available (Tim Ruffing)
d07e30176e084334081fa53be81e75c064375f36 ci: Update brew on macOS (Tim Ruffing)
22382f0ea0e234242e248720b9d1d171cb2de0f8 ci: Test different ecmult window sizes (Tim Ruffing)
26a022a3a0e3fceb1cd2e882e1476c950cabc2e8 ci: Remove STATICPRECOMPUTATION (Tim Ruffing)
10461d8bd3ce3ee8ca443ccad20915217ee74397 precompute_ecmult: Always compute all tables up to default WINDOW_G (Tim Ruffing)
Pull request description:
ACKs for top commit:
elichai:
utACK b4ac1a1d5f4d51b9836ac310b78bc9d4256580c2
jonasnick:
ACK b4ac1a1d5f4d51b9836ac310b78bc9d4256580c2
Tree-SHA512: b283d7b1c72cf87484de1fe98318298698fe9982dc33389eaca62e92318ab0074c183b9799add274f46358032491fee875e5ffb2a76a47f3b07520b850f4c85e
1287786c7a97eff520ffbd6b0d8b2f99dbfc6371 doc: Add comment to top of field_10x26_impl.h (Elliott Jin)
58da5bd589f61b0e0e9b58388ee3e0da8a2c3c3a doc: Fix upper bounds + cleanup in field_5x52_impl.h comment (Elliott Jin)
Pull request description:
When reviewing #816 I noticed the upper bounds in the comment at the top of `field_5x52_impl.h` were off by 1 (see `fe_verify`). This PR fixes the upper bounds and also cleans up the comment along the way.
ACKs for top commit:
real-or-random:
ACK 1287786c7a97eff520ffbd6b0d8b2f99dbfc6371
Tree-SHA512: 4b7dadc92451ab1ceb5a547a3101ff37f3ffd0645490563f1f3442ea8d6219f100ed914289d22435c4172d190fa1ff52e37e4464132bb3f9bbcc338488227f7b
22d25c8e0ab1d24f0f4a80fe016cbd71cd889866 Add another ecmult_multi test (Pieter Wuille)
Pull request description:
ACKs for top commit:
jonasnick:
ACK 22d25c8e0ab1d24f0f4a80fe016cbd71cd889866
Tree-SHA512: e1394fa1708e65a66d4b324cca60dd49c67e37b23b7da2a3ff0db7a2a25c23976cb03b96a8c8584ee81aaec559feb84fb113dff2e2ebf89110ed466a4a6b158b
ac1e36769dda3964f7294319ecb06fb5c414938d musig: turn off multiexponentiation for now (Jonas Nick)
3c79d97bd92ec22cc204ff5a08c9b0e5adda12e6 ci: increase timeout for macOS tasks (Jonas Nick)
22c88815c76e6edb23baf9401f820e1a944c3ecf musig: replace MuSig(1) with MuSig2 (Jonas Nick)
Pull request description:
The main commit comprises `905 insertions(+), 1253 deletions(-)`. The diff isn't as small as I had hoped, but that's mostly because it was possible to simplify the API quite substantially which required rewriting large parts. Sorry, almost all of the changes are in one big commit which makes the diff very hard to read. Perhaps best to re-review most parts from scratch.
A few key changes:
- Obviously no commitment round. No big session struct and no `verifier` sessions. No `signer` struct.
- There's a new `secnonce` struct that is the output of musig_nonce_gen and derived from a uniformly random session_id32. The derivation can be strengthened by adding whatever session parameters (combined_pk, msg) are available. The nonce function is my ad-hoc construction that allows for these optional inputs. Please have a look at that.
- The secnonce is made invalid after being used in partial_sign.
- Adaptor signatures basically work as before, according to https://github.com/ElementsProject/scriptless-scripts/pull/24 (with the exception that they operate on aggregate instead of partial sigs)
- To avoid making this PR overly complex I did not consider how this implementation interacts with nested-MuSig, sign-to-contract, and antiklepto.
- Testing should be close to complete. There's no reachable line or branch that isn't exercised by the tests.
- [x] ~In the current implementation when a signer sends an invalid nonce (i.e. some garbage that can't be mapped to a group element), it is ignored when combining nonces. Only after receiving the signers partial signature and running `partial_sig_verify` will we notice that the signer misbehaved. The reason for this is that 1) this makes the API simpler and 2) malicious peers don't gain any additional powers because they can always interrupt the protocol by refusing to sign. However, this is up for discussion.~ EDIT: this is not the case anymore since invalid nonces are rejected when they're parsed.
- [x] ~For every partial signature we verify we have to parse the pubnonce (two compressed points), despite having parsed it in `process_nonces` already. This is not great. `process_nonces` could optionally output the array of parsed pubnonces.~ EDIT: fixed by having a dedicated type for nonces.
- [x] ~I left `src/modules/musig/musig.md` unchanged for now. Perhaps we should merge it with the `musig-spec`~ EDIT: musig.md is updated
- [x] partial verification should use multiexp to compute `R1 + b*R2 + c*P`, but this can be done in a separate PR
- [x] renaming wishlist
- pre_session -> keyagg_cache (because there is no session anymore)
- pubkey_combine, nonce_combine, partial_sig_combine -> pubkey_agg, nonce_agg, partial_sig_agg (shorter, matches terminology in musig2)
- musig_session_init -> musig_start (shorter, simpler) or [musig_generate_nonce](https://github.com/ElementsProject/secp256k1-zkp/pull/131#discussion_r654190890) or musig_prepare
- musig_partial_signature to musig_partial_sig (shorter)
- [x] perhaps remove pubnonces and n_pubnonces argument from process_nonces (and then also add a opaque type for the combined nonce?)
- [x] write the `combined_pubkey` into the `pre_session` struct (as suggested [below](https://github.com/ElementsProject/secp256k1-zkp/pull/131#issuecomment-866904975): then 1) session_init and process_nonces don't need a combined_pk argument (and there can't be mix up between tweaked and untweaked keys) and 2) pubkey_tweak doesn't need an input_pubkey and the output_pubkey can be written directly into the pre_session (reducing frustration such as Replace MuSig(1) module with MuSig2 #131 (comment))
- [x] perhaps allow adapting both partial sigs (`partial_sig` struct) and aggregate partial sigs (64 raw bytes) as suggested [below](https://github.com/ElementsProject/secp256k1-zkp/pull/131#issuecomment-867281531).
Based on #120.
ACKs for top commit:
robot-dreams:
ACK ac1e36769dda3964f7294319ecb06fb5c414938d
real-or-random:
ACK ac1e36769dda3964f7294319ecb06fb5c414938d
Tree-SHA512: 916b42811aa5c00649cfb923d2002422c338106a6936a01253ba693015a242f21f7f7b4cce60d5ab5764a129926c6fd6676977c69c9e6e0aedc51b308ac6578d
