Add module "musig" that implements MuSig2 multi-signatures (BIP 327) #1479

jonasnick · 2024-01-06T20:40:28Z

EDIT: based on #1518. Closes #1452. Most of the code is a copy from libsecp256k1-zkp. The API added in this PR is identical with the exception of two modifications:

I removed the unused scratch_space argument from secp256k1_musig_pubkey_agg. This argument was intended to allow using ecmult_multi algorithms for key aggregation in the future. But at this point it's unclear whether the scratch_space object will remain in its current form (see Rework or get rid of scratch space #1302).
Support for adaptor signatures was removed and therefore the adaptor argument of musig_nonce_process was also removed.

In contrast to the module in libsecp256k1-zkp, the module is non-experimental. I slightly cleaned up parts of the module, adjusted the code to the new definition of the VERIFY_CHECK macro and applied some simplifications that were possible because the module is now in the upstream repo (ge_from_bytes, ge_to_bytes). You can follow the changes I made to the libsecp256k1-zkp module at https://github.com/jonasnick/secp256k1-zkp/commits/musig2-upstream/.

include/secp256k1_extrakeys.h

src/secp256k1.c

jonasnick · 2024-01-11T14:01:04Z

Rebased on top of master to get #1480 which allowed dropping a commit. Old state is preserved at https://github.com/jonasnick/secp256k1/tree/musig2-module-backup.

real-or-random

needs rebase :)

include/secp256k1_extrakeys.h

src/modules/musig/keyagg.h

t-bast · 2024-01-23T14:19:46Z

FWIW, we have JVM bindings on top of this branch in ACINQ/secp256k1-kmp#93 and an implementation of swap-in-potentiam (musig2 key-path with alternative delayed script path) in ACINQ/bitcoin-kmp#107 and everything is working fine, and the API is easy enough to use!

jonasnick · 2024-01-23T19:23:30Z

Rebased.

Thanks @t-bast, that's good to hear.

siv2r · 2024-02-01T05:21:26Z

Attaching a visualization for the API flow.

Edit: The above visualization is incorrect. I will update it with the correct one soon.

siv2r · 2024-02-05T10:23:29Z

examples/musig.c

+        pubkeys_ptr[i] = &signers[i].pubkey;
+    }
+    printf("ok\n");
+    printf("Combining public keys...");


Do we recommend that users sort their pubkeys before aggregating them? The musig_pubkey_agg API documentation simply says the user "can" do it.

If we recommend the sorting step, including it in the example file would be helpful.

I think there's no catch-all recommendation. BIP327 says "The aggregate public key produced by KeyAgg (regardless of the type) depends on the order of the individual public keys. If the application does not have a canonical order of the signers, the individual public keys can be sorted with the KeySort algorithm to ensure that the aggregate public key is independent of the order of signers."

In other words: If in your application, the collection of pubkeys (or signers represented by them) is conceptually an (ordered) list, then don't bother with sorting. If in your application, the collection of pubkeys is conceptually an (unordered) set, i.e., the application doesn't want to care about the order of pubkeys, then sort to make sure the set has a canonical serialization.

Perhaps we can explain this somewhere in more detail, either in the API docs or in the example.

nit: I think it would be nice to including sorting in the example for the following reasons:

It provides a practical example on how to use the sorting API

It's a good hook for adding a comment explaining what @real-or-random just explained above. Feels a bit nicer to have that comment in the example, vs the API docs

Added sorting and a comment.

src/modules/musig/session_impl.h

include/secp256k1_musig.h

josibake

I'm also using the heapsort commits in #1471. What do you think about splitting out the sort commits into their own PR? Also fine with cherry-picking for now, but figured I'd mention it since it might simplify both of our PRs.

josibake · 2024-04-08T14:41:15Z

src/modules/extrakeys/main_impl.h

+    /* Suppress wrong warning (fixed in MSVC 19.33) */
+    #if defined(_MSC_VER) && (_MSC_VER < 1933)
+    #pragma warning(push)
+    #pragma warning(disable: 4090)
+    #endif


in 26dde29 ("extrakeys: add secp256k1_pubkey_sort"):

Does it make sense to move this block into the secp256k1_sort function? I ended up copying these lines while writing a secp256k1_silentpayments_recipient_sort function, which made me realize anyone else would also need to copy these lines when writing a sort function for heapsort.

You mean into secp256k1_hsort? I'd guess the wrong warning is emitted when secp256k1_hsort is called and therefore it would be to late when the warning was disabled in secp256k1_hsort.

real-or-random · 2024-09-01T00:06:11Z

From my understanding the reason we have this "toxic data" of making sure nonces are not reused is that the counterparties could send nonces that are computed as sum of their nonce and inverse of our nonce which would allow them to compute private key.

I think you're mixing up two different issues. What you describe doesn't lead to an attack. The attacker can choose nonces that cancel out a honest signer's nonces, but the only effect this will have is that the attacker won't be able to come up with a valid partial signature and thus the protocol will fail.

This cancellation attack you describe is a concern not with nonces, but with the individual public keys in the key aggregation. Indeed, cancelling a key of an honest signer would work if the aggregate public key was just the sum of the individual keys. (This is called a "rouge key attack" or literally "a key cancellation attack"). But MuSig2 eliminates this attack vector by using a key aggregation function that is not just the sum of the keys, but instead a sum with random coefficients.

But yes, letting everyone prove knowledge of the discrete logarithm of their public individual public key using a Schnorr signature is a different way to eliminate rogue key attacks. This is used in SpeedyMuSig: https://eprint.iacr.org/2021/1375.pdf The drawback of this method is that it doesn't suffice to send these additional Schnorr signatures around during the signing protocol. For this to be secure, the signatures need to be there already at the key aggregation stage, which is possibly performed by a different party. In MuSig2, anyone (i.e., not just the involved signers) can combine some keys A and B into an aggregate key and use it, e.g., send money to it, and it will be ensured that A and B can only spend it if they work together. In SpeedyMuSig, the party who performs the key aggregation needs to see the Schnorr signatures valid for A and B before they can securely send money to the aggregate key A+B. This complicates key management and is a potential footgun (people could send to A+B without checking the additional Schnorr sigs).

edit: For the actual problem with reusing nonces, see either the paper or https://delvingbitcoin.org/t/how-many-nonce-reuse-before-exposing-your-musig2-private-key/217, but both are rather involved...

Kixunil · 2024-09-01T08:18:08Z

Ah, OK I think I understand better now. Thanks for explaining! What I was really thinking was if all nonces are deterministic then for the same key and same message they produce the same signature so how could an attacker possibly extract additional information from that? What I was missing is there's no way for a signer to know if the pubnonce sent by another party was generated deterministically except for having some kind of ZKP that would be likely expensive to construct and verify.

Still it would be nice to not have to store the secnonce on a permanent storage so maybe a viable solution for cases when there's one "unreliable" signer (random user with a smartphone that could kill the app at any time) and other parties are "reliable" (run on server 24/7, can hold the secnonces in RAM) would be for the "unreliable" signer to collect all nonces first and compute his own as an HMAC(secret_key, message || aggregate_pubkey || all_nonces_except_signers) and be the last one who sends pubnonce. The other signers hold their secnonces in RAM in the meantime. Does that make sense?

jonasnick · 2024-09-02T16:54:28Z

@Kixunil

if all nonces are deterministic then for the same key and same message they produce the same signature so how could an attacker possibly extract additional information from that? What I was missing is there's no way for a signer to know if the pubnonce sent by another party was generated deterministically except for having some kind of ZKP that would be likely expensive to construct and verify.

Proving that the nonces were generated deterministically from the session parameters is the idea behind deterministic signing in MuSig-DN and Exponent-VRFs and Their Applications.

Does that make sense?

It sounds like you're describing a variant of the "Deterministic and Stateless Signing for a Single Signer"-mode of BIP MuSig. In this mode, one signer is able to derive the nonces deterministically from the session parameters and the pubnonces of all other signers.

Kixunil · 2024-09-03T08:55:38Z

@jonasnick thanks for references!

It sounds like you're describing a variant of the "Deterministic and Stateless Signing for a Single Signer"-mode of BIP MuSig.

Yes, that's what I want! From what I can see it's not currently supported by the API. Is this planned or can it be implemented some other way?

Kixunil · 2024-09-03T08:59:10Z

include/secp256k1_musig.h

+SECP256K1_API int secp256k1_musig_pubnonce_parse(
+    const secp256k1_context *ctx,
+    secp256k1_musig_pubnonce *nonce,
+    const unsigned char *in66


Wouldn't it be better to declare this as const unsigned char (*in66)[66]? Aside from clarity it could help static analysis tools or perhaps bindings generators to generate correct bindings.

That's an interesting point. Right now, we consistently use declarations of the form const unsigned char *in66 consistently in the library. I prefer to have a standard across the whole API and would avoid just changing this in the musig module.

That makes sense. Might be nice to change all of them at some point unless there's a problem I'm missing.

You cannot pass arrays as arguments in C; they collapse into pointers. The syntax

int some_func(char c[32]) { ... }

is for all intents and purposes equivalent to int some_func(char *c) { ...}, including the fact that sizeof(c) will be 4 or 8 bytes (same as sizeof(char*)). Because it is so confusing, it's generally considered misleading to use arrays as arguments in C code.

EDIT: Oh, you're not suggesting this, but int some_func(char (*c)[32]) instead. That does work, and equivalent after compilation, but it does mean an additional & at the caller, and an additional * inside the callee. Can you elaborate on the (potential) advantages for static analysis or binding generators?

For instance in Rust we have bindgen, a tool that generates bindings automatically. I've tried to pass it this header file:

void foo(const unsigned char *arg); void bar(const unsigned char arg[32]); void baz(const unsigned char (*arg)[32]);

And it generated this code:

extern "C" { pub fn foo(arg: *const ::std::os::raw::c_uchar); } extern "C" { pub fn bar(arg: *const ::std::os::raw::c_uchar); } extern "C" { pub fn baz(arg: *const [::std::os::raw::c_uchar; 32usize]); }

As you can see, the last one has a different type. And while it's the same from ABI perspective, there's a real difference between them in Rust since you can directly pass in &array without any casts because the types match making the code more obviously correct.

Currently, we aren't using bidgen in rust-secp256k1 but we might in the future and I'd be surprised if similar tools don't exist in other languages.

Also I'm not familiar with the current state of static analysis in C but I think a reasonable static analyzer should be able to see that statements like out[33] = 42 or baz(&twentyone_item_array) are obviously wrong. gcc already complains about the these when compiling with -O2 -W -Wall -Warray-bounds.

Interesting!

It's unfortunate that it's an API break to introduce this (but indeed not an ABI break, I think).

Oh, I found that bindgen has an option to generate *const [c_uchar; 32] instead of *const c_uchar for arguments defined as const unsigned char arg[32], so maybe even that is already valuable? (IIUC not API-breaking) I think it has documentation value too.

It looks like cppcheck is also able to detect mistakes when you use these (I haven't tested but I found a PR that supposedly does it.)

sipa

Mostly looked through the interface and documentation.

sipa · 2024-09-06T15:34:22Z

doc/musig.md

+2. Call `secp256k1_musig_pubkey_agg` with the pubkeys of all participants.
+3. Optionally add a (Taproot) tweak with `secp256k1_musig_pubkey_xonly_tweak_add` and a plain tweak with `secp256k1_musig_pubkey_ec_tweak_add`.
+4. Generate a pair of secret and public nonce with `secp256k1_musig_nonce_gen` and send the public nonce to the other signers.
+5. Someone (not necessarily the signer) aggregates the public nonce with `secp256k1_musig_nonce_agg` and sends it to the signers.


Nit: the public nonces.

sipa · 2024-09-06T15:40:24Z

doc/musig.md

+
+The aggregate signature can be verified with `secp256k1_schnorrsig_verify`.
+
+Note that steps 1 to 5 can happen before the message to be signed is known to the signers.


I think it makes sense to stress that the normal (safer) mode is to construct the nonces after the message is known, rather than only pointing out the alternative here.

Suggested language:

Steps 1 through 5 above can happen before or after the message to be signed is known to the signers. Wherever possible, it is recommended to only generate the nonces after the message is known, as it has more defense-in-depth measures, but requires two communication rounds at signing time. The alternative, generating the nonces in a pre-processing step before the message is known, removes those measures, but means signing can happen non-interatively.

Agree, improved the wording.

sipa · 2024-09-06T17:52:44Z

include/secp256k1_musig.h

+ *     misuse-resistance. The extra_input32 argument can be used to provide
+ *     additional data that does not repeat in normal scenarios, such as the
+ *     current time.
+ *  3. Avoid copying (or serializing) the secnonce. This reduces the possibility


In case someone has no way around storing session data on disk somewhere, would the recommendation be:

To store/load the secnonce to/from disk (violating rule 3 here, plus risking platform dependence).

To store/load the session_secrand32 to/from disk and re-run secp256k1_musig_nonce_gen to obtain the same secnonce (violating rule 1 here, and duplicating computation).

I suspect some users will have not have the ability to leave the secnonce in volatile memory, so it may be useful to give advice on how to do that (plus reiterate the dangers of not wiping the stored data after signing).

We don't provide the ability to serialize the secnonce right now, which prevents users from storing/loading from disk. Maybe worth a separate PR?

Well they can just store the raw bytes on disk, which works, but risks platform-dependency issues.

If we provide the on store/load the secnonce from disk, we should also provide de/serialization functions because our API docs instruct users to not read the raw bytes. I had considered this out of scope for this initial PR because it seems dangerous. Especially since so far no one requested this feature, just "don't store to disk" seems to be a sensible answer.

Actually one developer asked me about the missing de/serialization functions for the secnonce but further discussion on their motivation for requesting this revealed that they planned to build a very insecure protocol because they had entirely misunderstood what nonce reuse actually is. In the end they agreed that they should not store the secnonce to disk and changed their implementation accordingly.

Also I had hoped that someone would eventually come up with a more clever API that can protect from misuse better than simple de/serialization functions (at least in some cases). But maybe that just doesn't exist.

That's totally reasonable, but I don't think you have answered my question. If someone is somehow not able to keep everything in volatile memory, they have two (bad) options (store secnonce, or store session_secrand32), and both options violate a rule. Which of the two would you, as API and protocol designer, recommend people take?

If someone really needs to do this right now, then I would probably recommend option 2. In contrast to option 1, if you reuse session_secrand but you're signing for a different message or aggregate public key, you're protected.

One downside of that method is that session_secrand has no "in-memory protection", in the sense that the session_secrand buffer is not cleared after nonce_generation. We do wipe the secnonce after signing. Maybe we should change nonce generation to overwrite the session_secrand buffer?

I agree that option 2 is better for the reasons mentioned.

We do wipe the secnonce after signing. Maybe we should change nonce generation to overwrite the session_secrand buffer?

That sounds reasonable to me.

Seems like a good idea, yes.

Done. The secrand buffer is invalidated whenever nonce_gen returns 1. The alternative is to invalidate whenever a session_secrand argument is given, which is a tiny bit more tedious to test and document. Anyway, if nonce_gen returns 0, then the secnonce is invalid as well, so reusing the secrand buffer in that case is not an issue.

sipa · 2024-09-06T17:54:42Z

include/secp256k1_musig.h

+ *  In:
+ *  session_secrand32: a 32-byte session_secrand32 as explained above. Must be unique to this
+ *                     call to secp256k1_musig_nonce_gen and must be uniformly random.
+ *             seckey: the 32-byte secret key that will later be used for signing, if


If provided, what happens if the seckey does not match the pubkey argument? (also applies to secp256k1_musig_nonce_gen_counter).

The musig_nonce_gen functions do not check if the seckey matches the given pubkey.

But will for example the resulting signature be invalid, or does doing this risk leaking private keys?

If the wrong public key is provided, signing will fail. This works because the public key is appended to the secnonce output of the nonce_gen functions and partial_sign will compare this public key with the public key the signer is trying to sign for.

Providing the optional seckey argument is a defense-in-depth measure to strengthen nonce generation against a low entropy session_secrand argument. If provided, the seckey is simply hashed into the nonce and does not have an effect on whether the resulting signature is valid.

So, the idea behind the defense-in-depth measure is that if the seckey does correspond to the pubkey argument and has high entropy, but the session_secrand argument has low entropy (but does not repeat), the generated nonce is secure. On the other hand, if the seckey does not match the pubkey, nonce generation is only insecure if both the provided seckey and secrand have low entropy. Providing a wrong seckey cannot negatively affect nonce generation as long as secrand has enough entropy.

If the wrong public key is provided, signing will fail. This works because the public key is appended to the secnonce output of the nonce_gen functions and partial_sign will compare this public key with the public key the signer is trying to sign for.

Let me add that the reason for this (and for the need for a pubkey argument in the first place) is that it prevents a vulnerability that could occur when people sign with tweaked individual keys, see https://lists.linuxfoundation.org/pipermail/bitcoin-dev/2022-October/021000.html (you should remember the story).

The docs currently just say "Returns: 0 if the arguments are invalid and 1 otherwise" Perhaps we should note that a pubkey/seckey mismatch does not count as invalid, but that it will be caught later during signing.

Perhaps we should note that a pubkey/seckey mismatch does not count as invalid, but that it will be caught later during signing.

I added a sentence, but it may be confusing. A pubkey/seckey mismatch in nonce generation may not be caught during signing, because the signer can still provide the correct seckey for signing.

include/secp256k1_musig.h

src/modules/musig/keyagg.h

src/modules/musig/keyagg_impl.h

CHANGELOG.md

theStack

Code-review ACK fdc0960

Reviewed once again that the code matches the specification in BIP327 and also took a closer look on the tests. Regarding the interface discussion #1479 (comment), I wonder if it's acceptable for now to not support (and not even mention) the use-case where users can't store the secnonce in volatile memory by now, and leave that for a follow-up? If a suggested way to work around that is described in the API docs ("if you really need to do this..."), that could be seen by users as invitation to break the rules. On the other hand, if nothing is mentioned, users might break the rules anyway and do it in an even worse way, so not sure what is better. Anyways, happy to re-review on changes in that area (e.g. suggestions in #1479 (comment)).

src/modules/musig/keyagg_impl.h

theStack · 2024-09-19T12:36:35Z

src/modules/musig/keyagg_impl.h

+    for (i = 0; i < np; i++) {
+        unsigned char ser[33];
+        size_t ser_len = sizeof(ser);
+        if (!secp256k1_ec_pubkey_serialize(ctx, ser, &ser_len, pks[i], SECP256K1_EC_COMPRESSED)) {


note for other reviewers: it's intentional to use the public API function for serializing a pubkey here (as opposed to the internal one called _eckey_pubkey_serialize, which is faster and used at all other places in the musig module) , as we want to check the validity of the input pubkeys of secp256k1_musig_pubkey_agg.

real-or-random

ACK fdc0960

I left a bunch of nits, mostly noticed when I looked at @sipa's comments in the include file, but none of these are blockers.

README.md

include/secp256k1_musig.h

real-or-random · 2024-09-24T12:30:19Z

include/secp256k1_musig.h

+ *     misuse-resistance. The extra_input32 argument can be used to provide
+ *     additional data that does not repeat in normal scenarios, such as the
+ *     current time.
+ *  3. Avoid copying (or serializing) the secnonce. This reduces the possibility


I agree that option 2 is better for the reasons mentioned.

We do wipe the secnonce after signing. Maybe we should change nonce generation to overwrite the session_secrand buffer?

That sounds reasonable to me.

include/secp256k1_musig.h

src/modules/musig/keyagg_impl.h

src/modules/musig/session_impl.h

src/group_impl.h

doc/musig.md

include/secp256k1_musig.h

sipa · 2024-09-24T15:05:15Z

include/secp256k1_musig.h

+ *     misuse-resistance. The extra_input32 argument can be used to provide
+ *     additional data that does not repeat in normal scenarios, such as the
+ *     current time.
+ *  3. Avoid copying (or serializing) the secnonce. This reduces the possibility


Seems like a good idea, yes.

include/secp256k1_musig.h

theStack · 2024-09-26T10:43:13Z

examples/musig.c

+        pubnonces[i] = &signer[i].pubnonce;
+
+        secure_erase(seckey, sizeof(seckey));
+        secure_erase(session_secrand, sizeof(session_secrand));


with the latest change to secp256k1_musig_nonce_gen, this line wouldn't be needed anymore (doesn't hurt to keep it though for demonstrating good practices, I presume)

Good catch. I removed that line. Imo explicitly erasaing secrand is more overkill than good practice.

real-or-random · 2024-09-26T11:43:00Z

src/modules/musig/session_impl.h

+    is_zero = secp256k1_is_zero_array(&secnonce->data[4], 32);
+    is_zero |= secp256k1_is_zero_array(&secnonce->data[36], 32);


This is what I meant. :) Yeah, it's not so much simpler, but I think it's an improvement overall.

If you want to make it one line simpler, you could do this (not sure if it's better):

Suggested change

is_zero = secp256k1_is_zero_array(&secnonce->data[4], 32);

is_zero |= secp256k1_is_zero_array(&secnonce->data[36], 32);

is_zero = secp256k1_is_zero_array(&secnonce->data[4], 2 * 32);

Oh, yes! Replaced the two lines with the suggestion.

theStack

Minor API question: are there any musig API functions where the SECP256K1_WARN_UNUSED_RESULT attribute should be added? (Probably for the _{pubnonce,aggnonce,partial_sig}_parse functions, given that external data is passed in, and the nonce generation functions?)

theStack · 2024-09-29T16:45:44Z

src/modules/musig/session_impl.h

+        if (!secp256k1_ge_is_in_correct_subgroup(&ges[i])) {
+            return 0;
+        }


curiosity nit: is this check needed, and if yes, does it make sense to also add it in the aggregate nonce parsing function _musig_aggnonce_parse? IIUC, this is only becomes relevant once exhaustive tests are added, and returns always 1 in production (so no big deal if it stays as-is now).

sipa

Code review ACK 5e55f09

I have not done a detailed comparison with the BIP spec.

sipa · 2024-09-30T14:12:08Z

include/secp256k1_musig.h

+ *  signature for it, use `secp256k1_xonly_pubkey_tweak_add` instead.
+ *
+ *  Returns: 0 if the arguments are invalid or the resulting public key would be
+ *           invalid (only when the tweak is the negation of the corresponding


Depending on the sign of the previous aggregated public key, I believe it is also possible this returns 0 if the tweak equals the corresponding private key?

sipa · 2024-09-30T16:30:13Z

src/modules/musig/session_impl.h

+    secp256k1_nonce_function_musig(k, input_nonce, msg32, seckey, pk_ser, aggpk_ser_ptr, extra_input32);
+    VERIFY_CHECK(!secp256k1_scalar_is_zero(&k[0]));
+    VERIFY_CHECK(!secp256k1_scalar_is_zero(&k[1]));
+    VERIFY_CHECK(!secp256k1_scalar_eq(&k[0], &k[1]));


Any given individual linear relation between k0 and k1 is of course practically impossible to reach, but what is the reason for this specific one? BIP327 only seems to suggest tests for k0=0 and k1=0 (in NonceGen()).

sipa · 2024-09-30T16:59:34Z

src/modules/musig/session_impl.h

+    for (i = 0; i < 2; i++) {
+        secp256k1_gej nonce_ptj;
+        secp256k1_ecmult_gen(&ctx->ecmult_gen_ctx, &nonce_ptj, &k[i]);
+        secp256k1_ge_set_gej(&nonce_pts[i], &nonce_ptj);


It would be nice if this could use batch inversion, but secp256k1_ge_set_all_gej_var is variable time (which I don't think is acceptable here?), and a constant-time version would either need a guarantee that neither point is infinity, or some annoying cmov logic to deal with infinities. Perhaps for a follow-up.

sipa · 2024-09-30T17:00:07Z

src/modules/musig/session_impl.h

+        return 0;
+    }
+    for (i = 0; i < 2; i++) {
+        secp256k1_ge_set_gej(&aggnonce_pts[i], &aggnonce_ptsj[i]);


I think secp256k1_ge_set_all_gej_var is usable here, as the public key inputs are not secret?

sipa · 2024-09-30T17:08:02Z

src/modules/musig/session_impl.h

+    }
+
+    /* Multiply KeyAgg coefficient */
+    /* TODO Cache mu */


Is there space in the cache data structure to do this caching? Would it make sense to reserve space for it?

sipa · 2024-09-30T18:11:59Z

src/modules/musig/tests_impl.h

+    CHECK(secp256k1_schnorrsig_verify(CTX, final_sig, msg, sizeof(msg), &agg_pk) == 1);
+}
+
+static void pubnonce_summing_to_inf(secp256k1_musig_pubnonce *pubnonce) {


Can you add a comment what this function does?

sipa · 2024-09-30T18:30:02Z

src/modules/musig/tests_impl.h

+#include "../../util.h"
+
+#include "vectors.h"
+#include <inttypes.h>


What is inttypes.h needed for?

yes, it seems that stdint.h suffices here

theStack

re-ACK 5e55f09

(as per $ git diff fdc09608036822afc1cebbe0c5b56cebf8ba508d 5e55f093118c2517f068841f5414f767fb35c7fe)

As far as I can tell, none of the unadressed questions/comments are critical.

jonasnick force-pushed the musig2-module branch from f5a74fd to fb60ae9 Compare January 6, 2024 21:56

real-or-random reviewed Jan 7, 2024

View reviewed changes

include/secp256k1_extrakeys.h Outdated Show resolved Hide resolved

real-or-random reviewed Jan 7, 2024

View reviewed changes

src/secp256k1.c Outdated Show resolved Hide resolved

jonasnick force-pushed the musig2-module branch 5 times, most recently from ab7fc1e to eaf1e78 Compare January 8, 2024 08:06

Davidson-Souza mentioned this pull request Jan 8, 2024

Topicos Janeiro saopaulobitdevs/saopaulobitdevs.org#17

Closed

jonasnick force-pushed the musig2-module branch from eaf1e78 to 70bb685 Compare January 11, 2024 13:58

real-or-random added the feature label Jan 11, 2024

real-or-random mentioned this pull request Jan 18, 2024

Upstream PRs 1426, 1430, 1184, 1437, 1442, 1441, 1445, 1438, 1393, 1446, 1450, 1451, 1431, 990, 1455, 1380, 1465, 1466, 1473, 1474, 1476, 1480, 1468, 1482, 1249 BlockstreamResearch/secp256k1-zkp#285

Merged

sstone mentioned this pull request Jan 18, 2024

Add support for musig2 ACINQ/secp256k1-kmp#93

Merged

real-or-random reviewed Jan 18, 2024

View reviewed changes

include/secp256k1_extrakeys.h Outdated Show resolved Hide resolved

src/modules/musig/keyagg.h Outdated Show resolved Hide resolved

narcelio mentioned this pull request Jan 23, 2024

Topicos Janeiro plebemineira/bhbitdevs.org#1

Closed

jonasnick force-pushed the musig2-module branch from 70bb685 to dd4932b Compare January 23, 2024 19:21

theStack mentioned this pull request Jan 30, 2024

Add silentpayments (BIP352) module #1471

Closed

siv2r reviewed Feb 5, 2024

View reviewed changes

siv2r reviewed Feb 6, 2024

View reviewed changes

src/modules/musig/session_impl.h Show resolved Hide resolved

Roasbeef mentioned this pull request Feb 12, 2024

Lightning Specification Meeting 2024/02/12 lightning/bolts#1134

Closed

22 tasks

real-or-random reviewed Mar 7, 2024

View reviewed changes

include/secp256k1_musig.h Outdated Show resolved Hide resolved

jonasnick force-pushed the musig2-module branch from 2017bbe to 4619706 Compare March 10, 2024 14:57

real-or-random reviewed Mar 14, 2024

View reviewed changes

include/secp256k1_musig.h Outdated Show resolved Hide resolved

include/secp256k1_musig.h Outdated Show resolved Hide resolved

include/secp256k1_musig.h Outdated Show resolved Hide resolved

achow101 mentioned this pull request Mar 18, 2024

wallet: Be able to receive and spend inputs involving MuSig2 aggregate keys bitcoin/bitcoin#29675

Draft

josibake reviewed Apr 8, 2024

View reviewed changes

jonasnick mentioned this pull request Apr 16, 2024

Add secp256k1_pubkey_sort #1518

Merged

Bigheem approved these changes Aug 31, 2024

View reviewed changes

jonasnick added 2 commits September 2, 2024 19:05

group: add ge_to_bytes and ge_from_bytes

85e224d

group: add ge_to_bytes_ext and ge_from_bytes_ext

12835f8

jonasnick force-pushed the musig2-module branch 2 times, most recently from 265297b to 5bd0891 Compare September 2, 2024 20:06

Kixunil reviewed Sep 3, 2024

View reviewed changes

sipa reviewed Sep 6, 2024

View reviewed changes

jonasnick force-pushed the musig2-module branch 2 times, most recently from 753cf07 to 49f69cb Compare September 12, 2024 17:08

theStack reviewed Sep 13, 2024

View reviewed changes

CHANGELOG.md Outdated Show resolved Hide resolved

jonasnick force-pushed the musig2-module branch from 49f69cb to fdc0960 Compare September 13, 2024 09:47

theStack approved these changes Sep 19, 2024

View reviewed changes

real-or-random approved these changes Sep 24, 2024

View reviewed changes

sipa reviewed Sep 24, 2024

View reviewed changes

util: add constant-time is_zero_array function

2210def

jonasnick force-pushed the musig2-module branch from fdc0960 to 55d3dc0 Compare September 25, 2024 19:23

theStack reviewed Sep 26, 2024

View reviewed changes

real-or-random reviewed Sep 26, 2024

View reviewed changes

real-or-random mentioned this pull request Sep 26, 2024

Fix: GenerateNonce(ECPubKey) wasn't random as expected MetacoSA/NBitcoin#1228

Merged

jonasnick added 2 commits September 26, 2024 15:08

Add module "musig" that implements MuSig2 multi-signatures (BIP 327)

6f496a0

build: allow enabling the musig module in cmake

5e55f09

jonasnick force-pushed the musig2-module branch from 55d3dc0 to 5e55f09 Compare September 26, 2024 15:09

theStack mentioned this pull request Sep 27, 2024

use EXIT_ codes instead of magic numbers for exit(...) and main return values #1609

Open

theStack reviewed Sep 29, 2024

View reviewed changes

sipa reviewed Sep 30, 2024

View reviewed changes

theStack approved these changes Oct 1, 2024

View reviewed changes


		The aggregate signature can be verified with `secp256k1_schnorrsig_verify`.

		Note that steps 1 to 5 can happen before the message to be signed is known to the signers.

		is_zero = secp256k1_is_zero_array(&secnonce->data[4], 32);
		is_zero \|= secp256k1_is_zero_array(&secnonce->data[36], 32);

Add module "musig" that implements MuSig2 multi-signatures (BIP 327) #1479

Are you sure you want to change the base?

Add module "musig" that implements MuSig2 multi-signatures (BIP 327) #1479

Conversation

jonasnick commented Jan 6, 2024 • edited Loading

jonasnick commented Jan 11, 2024

real-or-random left a comment

Choose a reason for hiding this comment

t-bast commented Jan 23, 2024

jonasnick commented Jan 23, 2024

siv2r commented Feb 1, 2024 • edited Loading

siv2r Feb 5, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

josibake left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

real-or-random commented Sep 1, 2024 • edited Loading

Kixunil commented Sep 1, 2024

jonasnick commented Sep 2, 2024

Kixunil commented Sep 3, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sipa Sep 4, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Kixunil Sep 6, 2024 • edited Loading

Choose a reason for hiding this comment

sipa left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sipa Sep 13, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

theStack left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

real-or-random left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

theStack left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sipa left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jonasnick commented Jan 6, 2024 •

edited

Loading

siv2r commented Feb 1, 2024 •

edited

Loading

siv2r Feb 5, 2024 •

edited

Loading

real-or-random commented Sep 1, 2024 •

edited

Loading

sipa Sep 4, 2024 •

edited

Loading

Kixunil Sep 6, 2024 •

edited

Loading

sipa Sep 13, 2024 •

edited

Loading