Merge #151: MuSig: Add Minimal Compatibility with BIP32 Tweaking

8088edd musig: add test vector for ordinary (non xonly) tweaking (Elliott Jin)
57a1792 musig: add ordinary and xonly tweaking to the example (Jonas Nick)
3710736 musig: allow ordinary, non-xonly tweaking (Jonas Nick)
c519b46 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 8088edd based on #151 (comment) and the following `range-diff`:

Tree-SHA512: a4a0100f0470c870f88a8da27dbcc4684fcc2caabb368d4340e962e08d5ee04634e6289bafa3448dbfd0b5793a3e70de5bd6ddca7a619cc3220ff762d518a8fe

Author: real-or-random

examples/musig.c

@@ -52,14 +52,51 @@ int create_keypair(const secp256k1_context* ctx, struct signer_secrets *signer_s
     return 1;
 }
 
+/* Tweak the pubkey corresponding to the provided keyagg cache, update the cache
+ * and return the tweaked aggregate pk. */
+int tweak(const secp256k1_context* ctx, secp256k1_xonly_pubkey *agg_pk, secp256k1_musig_keyagg_cache *cache) {
+    secp256k1_pubkey output_pk;
+    unsigned char ordinary_tweak[32] = "this could be a BIP32 tweak....";
+    unsigned char xonly_tweak[32] = "this could be a taproot tweak..";
+
+
+    /* Ordinary tweaking which, for example, allows deriving multiple child
+     * public keys from a single aggregate key using BIP32 */
+    if (!secp256k1_musig_pubkey_ec_tweak_add(ctx, NULL, cache, ordinary_tweak)) {
+        return 0;
+    }
+    /* Note that we did not provided an output_pk argument, because the
+     * resulting pk is also saved in the cache and so if one is just interested
+     * in signing the output_pk argument is unnecessary. On the other hand, if
+     * one is not interested in signing, the same output_pk can be obtained by
+     * calling `secp256k1_musig_pubkey_get` right after key aggregation to get
+     * the full pubkey and then call `secp256k1_ec_pubkey_tweak_add`. */
+
+    /* Xonly tweaking which, for example, allows creating taproot commitments */
+    if (!secp256k1_musig_pubkey_xonly_tweak_add(ctx, &output_pk, cache, xonly_tweak)) {
+        return 0;
+    }
+    /* Note that if we wouldn't care about signing, we can arrive at the same
+     * output_pk by providing the untweaked public key to
+     * `secp256k1_xonly_pubkey_tweak_add` (after converting it to an xonly pubkey
+     * if necessary with `secp256k1_xonly_pubkey_from_pubkey`). */
+
+    /* Now we convert the output_pk to an xonly pubkey to allow to later verify
+     * the Schnorr signature against it. For this purpose we can ignore the
+     * `pk_parity` output argument; we would need it if we would have to open
+     * the taproot commitment. */
+    if (!secp256k1_xonly_pubkey_from_pubkey(ctx, agg_pk, NULL, &output_pk)) {
+        return 0;
+    }
+    return 1;
+}
+
 /* Sign a message hash with the given key pairs and store the result in sig */
-int sign(const secp256k1_context* ctx, struct signer_secrets *signer_secrets, struct signer *signer, const unsigned char* msg32, unsigned char *sig64) {
+int sign(const secp256k1_context* ctx, struct signer_secrets *signer_secrets, struct signer *signer, const secp256k1_musig_keyagg_cache *cache, const unsigned char *msg32, unsigned char *sig64) {
     int i;
-    const secp256k1_xonly_pubkey *pubkeys[N_SIGNERS];
     const secp256k1_musig_pubnonce *pubnonces[N_SIGNERS];
     const secp256k1_musig_partial_sig *partial_sigs[N_SIGNERS];
     /* The same for all signers */
-    secp256k1_musig_keyagg_cache cache;
     secp256k1_musig_session session;
 
     for (i = 0; i < N_SIGNERS; i++) {
@@ -86,29 +123,25 @@ int sign(const secp256k1_context* ctx, struct signer_secrets *signer_secrets, st
         if (!secp256k1_musig_nonce_gen(ctx, &signer_secrets[i].secnonce, &signer[i].pubnonce, session_id, seckey, msg32, NULL, NULL)) {
             return 0;
         }
-        pubkeys[i] = &signer[i].pubkey;
         pubnonces[i] = &signer[i].pubnonce;
     }
     /* Communication round 1: A production system would exchange public nonces
      * here before moving on. */
     for (i = 0; i < N_SIGNERS; i++) {
         secp256k1_musig_aggnonce agg_pubnonce;
 
-        /* Create aggregate pubkey, aggregate nonce and initialize signer data */
-        if (!secp256k1_musig_pubkey_agg(ctx, NULL, NULL, &cache, pubkeys, N_SIGNERS)) {
-            return 0;
-        }
+        /* Create aggregate nonce and initialize the session */
         if (!secp256k1_musig_nonce_agg(ctx, &agg_pubnonce, pubnonces, N_SIGNERS)) {
             return 0;
         }
-        if (!secp256k1_musig_nonce_process(ctx, &session, &agg_pubnonce, msg32, &cache, NULL)) {
+        if (!secp256k1_musig_nonce_process(ctx, &session, &agg_pubnonce, msg32, cache, NULL)) {
             return 0;
         }
         /* partial_sign will clear the secnonce by setting it to 0. That's because
          * you must _never_ reuse the secnonce (or use the same session_id to
          * create a secnonce). If you do, you effectively reuse the nonce and
          * leak the secret key. */
-        if (!secp256k1_musig_partial_sign(ctx, &signer[i].partial_sig, &signer_secrets[i].secnonce, &signer_secrets[i].keypair, &cache, &session)) {
+        if (!secp256k1_musig_partial_sign(ctx, &signer[i].partial_sig, &signer_secrets[i].secnonce, &signer_secrets[i].keypair, cache, &session)) {
             return 0;
         }
         partial_sigs[i] = &signer[i].partial_sig;
@@ -127,7 +160,7 @@ int sign(const secp256k1_context* ctx, struct signer_secrets *signer_secrets, st
          * fine to first verify the aggregate sig, and only verify the individual
          * sigs if it does not work.
          */
-        if (!secp256k1_musig_partial_sig_verify(ctx, &signer[i].partial_sig, &signer[i].pubnonce, &signer[i].pubkey, &cache, &session)) {
+        if (!secp256k1_musig_partial_sig_verify(ctx, &signer[i].partial_sig, &signer[i].pubnonce, &signer[i].pubkey, cache, &session)) {
             return 0;
         }
     }
@@ -141,6 +174,7 @@ int sign(const secp256k1_context* ctx, struct signer_secrets *signer_secrets, st
     struct signer signers[N_SIGNERS];
     const secp256k1_xonly_pubkey *pubkeys_ptr[N_SIGNERS];
     secp256k1_xonly_pubkey agg_pk;
+    secp256k1_musig_keyagg_cache cache;
     unsigned char msg[32] = "this_could_be_the_hash_of_a_msg!";
     unsigned char sig[64];
 
@@ -156,13 +190,21 @@ int sign(const secp256k1_context* ctx, struct signer_secrets *signer_secrets, st
     }
     printf("ok\n");
     printf("Combining public keys...");
-    if (!secp256k1_musig_pubkey_agg(ctx, NULL, &agg_pk, NULL, pubkeys_ptr, N_SIGNERS)) {
+    /* If you just want to aggregate and not sign the cache can be NULL */
+    if (!secp256k1_musig_pubkey_agg(ctx, NULL, &agg_pk, &cache, pubkeys_ptr, N_SIGNERS)) {
+        printf("FAILED\n");
+        return 1;
+    }
+    printf("ok\n");
+    printf("Tweaking................");
+    /* Optionally tweak the aggregate key */
+    if (!tweak(ctx, &agg_pk, &cache)) {
         printf("FAILED\n");
         return 1;
     }
     printf("ok\n");
     printf("Signing message.........");
-    if (!sign(ctx, signer_secrets, signers, msg, sig)) {
+    if (!sign(ctx, signer_secrets, signers, &cache, msg, sig)) {
         printf("FAILED\n");
         return 1;
     }

include/secp256k1_musig.h

@@ -223,16 +223,77 @@ SECP256K1_API int secp256k1_musig_pubkey_agg(
     size_t n_pubkeys
 ) SECP256K1_ARG_NONNULL(1) SECP256K1_ARG_NONNULL(5);
 
-/** Tweak an x-only public key in a given keyagg_cache by adding
- *  the generator multiplied with `tweak32` to it.
+/** Obtain the aggregate public key from a keyagg_cache.
+ *
+ *  This is only useful if you need the non-xonly public key, in particular for
+ *  ordinary (non-xonly) tweaking or batch-verifying multiple key aggregations
+ *  (not implemented).
+ *
+ *  Returns: 0 if the arguments are invalid, 1 otherwise
+ *  Args:        ctx: pointer to a context object
+ *  Out:      agg_pk: the MuSig-aggregated public key.
+ *  In: keyagg_cache: pointer to a `musig_keyagg_cache` struct initialized by
+ *                    `musig_pubkey_agg`
+ */
+SECP256K1_API SECP256K1_WARN_UNUSED_RESULT int secp256k1_musig_pubkey_get(
+    const secp256k1_context* ctx,
+    secp256k1_pubkey *agg_pk,
+    secp256k1_musig_keyagg_cache *keyagg_cache
+) SECP256K1_ARG_NONNULL(1) SECP256K1_ARG_NONNULL(2) SECP256K1_ARG_NONNULL(3);
+
+/** Apply ordinary "EC" tweaking to a public key in a given keyagg_cache by
+ *  adding the generator multiplied with `tweak32` to it. This is useful for
+ *  deriving child keys from an aggregate public key via BIP32.
+ *
+ *  The tweaking method is the same as `secp256k1_ec_pubkey_tweak_add`. So after
+ *  the following pseudocode buf and buf2 have identical contents (absent
+ *  earlier failures).
+ *
+ *  secp256k1_musig_pubkey_agg(..., keyagg_cache, pubkeys, ...)
+ *  secp256k1_musig_pubkey_get(..., agg_pk, keyagg_cache)
+ *  secp256k1_musig_pubkey_ec_tweak_add(..., output_pk, tweak32, keyagg_cache)
+ *  secp256k1_ec_pubkey_serialize(..., buf, output_pk)
+ *  secp256k1_ec_pubkey_tweak_add(..., agg_pk, tweak32)
+ *  secp256k1_ec_pubkey_serialize(..., buf2, agg_pk)
+ *
+ *  This function is required if you want to _sign_ for a tweaked aggregate key.
+ *  On the other hand, if you are only computing a public key, but not intending
+ *  to create a signature for it, you can just use
+ *  `secp256k1_ec_pubkey_tweak_add`.
+ *
+ *  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
+ *           secret key). 1 otherwise.
+ *  Args:            ctx: pointer to a context object initialized for verification
+ *  Out:   output_pubkey: pointer to a public key to store the result. Will be set
+ *                        to an invalid value if this function returns 0. If you
+ *                        do not need it, this arg can be NULL.
+ *  In/Out: keyagg_cache: pointer to a `musig_keyagg_cache` struct initialized by
+ *                       `musig_pubkey_agg`
+ *  In:          tweak32: pointer to a 32-byte tweak. If the tweak is invalid
+ *                        according to `secp256k1_ec_seckey_verify`, this function
+ *                        returns 0. For uniformly random 32-byte arrays the
+ *                        chance of being invalid is negligible (around 1 in
+ *                        2^128).
+ */
+SECP256K1_API SECP256K1_WARN_UNUSED_RESULT int secp256k1_musig_pubkey_ec_tweak_add(
+    const secp256k1_context* ctx,
+    secp256k1_pubkey *output_pubkey,
+    secp256k1_musig_keyagg_cache *keyagg_cache,
+    const unsigned char *tweak32
+) SECP256K1_ARG_NONNULL(1) SECP256K1_ARG_NONNULL(3) SECP256K1_ARG_NONNULL(4);
+
+/** Apply x-only tweaking to a public key in a given keyagg_cache by adding the
+ *  generator multiplied with `tweak32` to it. This is useful for creating
+ *  Taproot outputs.
  *
  *  The tweaking method is the same as `secp256k1_xonly_pubkey_tweak_add`. So in
  *  the following pseudocode xonly_pubkey_tweak_add_check (absent earlier
  *  failures) returns 1.
  *
  *  secp256k1_musig_pubkey_agg(..., agg_pk, keyagg_cache, pubkeys, ...)
- *  secp256k1_musig_pubkey_tweak_add(..., output_pubkey, tweak32, keyagg_cache)
- *  secp256k1_xonly_pubkey_serialize(..., buf, output_pubkey)
+ *  secp256k1_musig_pubkey_xonly_tweak_add(..., output_pk, tweak32, keyagg_cache)
+ *  secp256k1_xonly_pubkey_serialize(..., buf, output_pk)
  *  secp256k1_xonly_pubkey_tweak_add_check(..., buf, ..., agg_pk, tweak32)
  *
  *  This function is required if you want to _sign_ for a tweaked aggregate key.
@@ -255,7 +316,7 @@ SECP256K1_API int secp256k1_musig_pubkey_agg(
  *                        chance of being invalid is negligible (around 1 in
  *                        2^128).
  */
-SECP256K1_API SECP256K1_WARN_UNUSED_RESULT int secp256k1_musig_pubkey_tweak_add(
+SECP256K1_API SECP256K1_WARN_UNUSED_RESULT int secp256k1_musig_pubkey_xonly_tweak_add(
     const secp256k1_context* ctx,
     secp256k1_pubkey *output_pubkey,
     secp256k1_musig_keyagg_cache *keyagg_cache,

src/modules/musig/keyagg_impl.h

@@ -244,7 +244,21 @@ int secp256k1_musig_pubkey_agg(const secp256k1_context* ctx, secp256k1_scratch_s
     return 1;
 }
 
-int secp256k1_musig_pubkey_tweak_add(const secp256k1_context* ctx, secp256k1_pubkey *output_pubkey, secp256k1_musig_keyagg_cache *keyagg_cache, const unsigned char *tweak32) {
+int secp256k1_musig_pubkey_get(const secp256k1_context* ctx, secp256k1_pubkey *agg_pk, secp256k1_musig_keyagg_cache *keyagg_cache) {
+    secp256k1_keyagg_cache_internal cache_i;
+    VERIFY_CHECK(ctx != NULL);
+    ARG_CHECK(agg_pk != NULL);
+    memset(agg_pk, 0, sizeof(*agg_pk));
+    ARG_CHECK(keyagg_cache != NULL);
+
+    if(!secp256k1_keyagg_cache_load(ctx, &cache_i, keyagg_cache)) {
+        return 0;
+    }
+    secp256k1_pubkey_save(agg_pk, &cache_i.pk);
+    return 1;
+}
+
+static int secp256k1_musig_pubkey_tweak_add_internal(const secp256k1_context* ctx, secp256k1_pubkey *output_pubkey, secp256k1_musig_keyagg_cache *keyagg_cache, const unsigned char *tweak32, int xonly) {
     secp256k1_keyagg_cache_internal cache_i;
     int overflow = 0;
     secp256k1_scalar tweak;
@@ -263,7 +277,7 @@ int secp256k1_musig_pubkey_tweak_add(const secp256k1_context* ctx, secp256k1_pub
     if (overflow) {
         return 0;
     }
-    if (secp256k1_extrakeys_ge_even_y(&cache_i.pk)) {
+    if (xonly && secp256k1_extrakeys_ge_even_y(&cache_i.pk)) {
         cache_i.internal_key_parity ^= 1;
         secp256k1_scalar_negate(&cache_i.tweak, &cache_i.tweak);
     }
@@ -280,4 +294,12 @@ int secp256k1_musig_pubkey_tweak_add(const secp256k1_context* ctx, secp256k1_pub
     return 1;
 }
 
+int secp256k1_musig_pubkey_ec_tweak_add(const secp256k1_context* ctx, secp256k1_pubkey *output_pubkey, secp256k1_musig_keyagg_cache *keyagg_cache, const unsigned char *tweak32) {
+    return secp256k1_musig_pubkey_tweak_add_internal(ctx, output_pubkey, keyagg_cache, tweak32, 0);
+}
+
+int secp256k1_musig_pubkey_xonly_tweak_add(const secp256k1_context* ctx, secp256k1_pubkey *output_pubkey, secp256k1_musig_keyagg_cache *keyagg_cache, const unsigned char *tweak32) {
+    return secp256k1_musig_pubkey_tweak_add_internal(ctx, output_pubkey, keyagg_cache, tweak32, 1);
+}
+
 #endif