refactor: Move common nonce helpers from musig to shared module #1938

pull fjahr wants to merge 1 commits into bitcoin-core:master from fjahr:2026-09-musig-nonce-common changing 5 files +193 −83

fjahr commented at 9:49 PM on September 15, 2026: contributor

Factor the secret and public nonce (de)serialization, partial signature (de)serialization, effective nonce computation and secret clearing out of the musig module into a new nonce_common module. These helpers are not specific to musig and can be reused by other modules implementing interactive signing protocols with two-point nonces.

This came out of my work on the FullAgg/Dahlias PR which requires exactly these same helpers but since this refactoring idea can be discussed independently of the new module I am putting it here separately, as suggested by theStack.

refactor: Move common nonce helpers from musig to shared module

Factor the secret and public nonce (de)serialization, partial signature
(de)serialization, effective nonce computation and secret clearing out of
the musig module into a new nonce_common module. These helpers are not
specific to musig and can be reused by other modules implementing
interactive signing protocols with two-point nonces.

This is a pure refactor without a change in behavior.

e210f51657

theStack added the label tweak/refactor on Sep 22, 2026

in src/modules/nonce_common.h:23 in e210f51657

  18 | + * (de)serialized so that passing an object to the wrong API fails. */
  19 | +
  20 | +/* Saves a secret nonce into a 132 byte buffer: magic (4 bytes) || k[0] (32
  21 | + * bytes) || k[1] (32 bytes) || pk (64 bytes). */
  22 | +static void secp256k1_secnonce_save(unsigned char *data, const unsigned char *magic, const secp256k1_scalar *k, const secp256k1_ge *pk);
  23 | +static int secp256k1_secnonce_load(const secp256k1_context *ctx, secp256k1_scalar *k, secp256k1_ge *pk, const unsigned char *data, const unsigned char *magic);

IsaqueFranklin commented at 1:56 PM on September 29, 2026:

nit: I think that a comment here for the _load, similar to the one for _save, would be nice to be consistent.

in src/modules/nonce_common.h:45 in e210f51657

  40 | +static void secp256k1_nonce_points_serialize(unsigned char *out66, secp256k1_ge *ges);
  41 | +
  42 | +/* Saves a partial signature into a 36 byte buffer: magic (4 bytes) || s (32
  43 | + * bytes). */
  44 | +static void secp256k1_partial_sig_save(unsigned char *data, const unsigned char *magic, const secp256k1_scalar *s);
  45 | +static int secp256k1_partial_sig_load(const secp256k1_context *ctx, secp256k1_scalar *s, const unsigned char *data, const unsigned char *magic);

IsaqueFranklin commented at 1:56 PM on September 29, 2026:

nit: A comment for the _load here would be nice too.

in src/modules/nonce_common_impl.h:93 in e210f51657
```
  88 | +        secp256k1_ge_serialize33(&ges[i], &out66[33*i]);
  89 | +    }
  90 | +}
  91 | +
  92 | +static void secp256k1_partial_sig_save(unsigned char *data, const unsigned char *magic, const secp256k1_scalar *s) {
  93 | +    memcpy(&data[0], magic, 4);
```
IsaqueFranklin commented at 2:05 PM on September 29, 2026:

Are magic values expected to always be 4 bytes? Would future modules using these helpers also be expected to use 4 bytes for them? If so, would it make sense to define a constant for the magic size instead of hardcoding 4, in order to make that assumption explicit?

Maybe I'm missing something here.
IsaqueFranklin commented at 2:13 PM on September 29, 2026: none

Concept ACK. I like the idea, and that it makes it easier for other possible future modules (e.g. Nested Musig 2, if it ever becomes a module) to use these helpers without having to duplicate code.
IsaqueFranklin commented at 10:38 AM on October 1, 2026: none

It also seems a bit weird to me to leave these nonce helper files "hanging" directly inside the modules directory. A helpers subdirectory under modules for files like these might be an idea, however, this also feels a bit weird. I'm not sure which direction is better.

Contributors

fjahr

IsaqueFranklin

Labels

tweak/refactor

Linked (view graph)

#1754 BIP459: Add FullAgg (DahLIAS) module