Merge #120: Add MuSig Key Aggregation spec

fc26ca8 musig: remove unnecessary constant time normalize in combine (Jonas Nick)
48f63ef musig: remove unnecessary branch in pubkey_tweak_add (Jonas Nick)
5860b5e musig: do not also require schnorrsig module config flag (Jonas Nick)
f27fd1d musig: improve test coverage of pubkey_combine (Jonas Nick)
56014e8 musig: change pubkey_combine arg to array of pointers to pks (Jonas Nick)
08fa02d musig: add key aggregation spec draft (Jonas Nick)
4a9b059 musig: rename Musig coefficient to KeyAgg coefficient (Jonas Nick)
4bc46d8 musig: optimize key aggregation using const 1 for 2nd key (Jonas Nick)
2310849 musig: compute musig coefficient by hashing key instead of index (Jonas Nick)
9683c8a musig: add static test vectors for key aggregation (Jonas Nick)
9b3d7bf extrakeys: add xonly_sort function (Jonas Nick)
f31affd extrakeys: add hsort, in-place, iterative heapsort (Jonas Nick)

Pull request description:

ACKs for top commit:
  real-or-random:
    ACK fc26ca8

Tree-SHA512: fa29fe259d0e98d634782c0fb36308716dc3ffa6e35fe47b87fc25b2e5dd0a9859a72da0b9d669f03d379bc3ed972c5961995762b2f7e4ac16b9c6b5d8c4721d

configure.ac

@@ -492,6 +492,7 @@ fi
 
 if test x"$enable_module_musig" = x"yes"; then
   AC_DEFINE(ENABLE_MODULE_MUSIG, 1, [Define this symbol to enable the MuSig module])
+  enable_module_schnorrsig=yes
 fi
 
 if test x"$enable_module_recovery" = x"yes"; then
@@ -513,7 +514,8 @@ fi
 if test x"$enable_module_surjectionproof" = x"yes"; then
   AC_DEFINE(ENABLE_MODULE_SURJECTIONPROOF, 1, [Define this symbol to enable the surjection proof module])
 fi
-
+# Test if extrakeys is set _after_ the MuSig module to allow the MuSig
+# module to set enable_module_schnorrsig=yes
 if test x"$enable_module_schnorrsig" = x"yes"; then
   AC_DEFINE(ENABLE_MODULE_SCHNORRSIG, 1, [Define this symbol to enable the schnorrsig module])
   enable_module_extrakeys=yes
@@ -663,6 +665,7 @@ echo "  module ecdh             = $enable_module_ecdh"
 echo "  module recovery         = $enable_module_recovery"
 echo "  module extrakeys        = $enable_module_extrakeys"
 echo "  module schnorrsig       = $enable_module_schnorrsig"
+echo "  module musig            = $enable_module_musig"
 echo "  module ecdsa-s2c        = $enable_module_ecdsa_s2c"
 echo "  module ecdsa-adaptor    = $enable_module_ecdsa_adaptor"
 echo

include/secp256k1_extrakeys.h

@@ -166,6 +166,20 @@ SECP256K1_API SECP256K1_WARN_UNUSED_RESULT int secp256k1_xonly_pubkey_tweak_add_
     const unsigned char *tweak32
 ) SECP256K1_ARG_NONNULL(1) SECP256K1_ARG_NONNULL(2) SECP256K1_ARG_NONNULL(4) SECP256K1_ARG_NONNULL(5);
 
+/** Sorts xonly public keys according to secp256k1_xonly_pubkey_cmp
+ *
+ *  Returns: 0 if the arguments are invalid. 1 otherwise.
+ *
+ *  Args:     ctx: pointer to a context object
+ *  In:   pubkeys: array of pointers to pubkeys to sort
+ *      n_pubkeys: number of elements in the pubkeys array
+ */
+SECP256K1_API int secp256k1_xonly_sort(
+    const secp256k1_context* ctx,
+    const secp256k1_xonly_pubkey **pubkeys,
+    size_t n_pubkeys
+) SECP256K1_ARG_NONNULL(1) SECP256K1_ARG_NONNULL(2);
+
 /** Compute the keypair for a secret key.
  *
  *  Returns: 1: secret was valid, keypair is ready to use

include/secp256k1_musig.h

@@ -25,6 +25,8 @@ extern "C" {
  *        magic: Set during initialization in `pubkey_combine` to allow
  *               detecting an uninitialized object.
  *      pk_hash: The 32-byte hash of the original public keys
+ *    second_pk: Serialized x-coordinate of the second public key in the list.
+ *               Filled with zeros if there is none.
  *    pk_parity: Whether the MuSig-aggregated point was negated when
  *               converting it to the combined xonly pubkey.
  *     is_tweaked: Whether the combined pubkey was tweaked
@@ -35,6 +37,7 @@ extern "C" {
 typedef struct {
     uint64_t magic;
     unsigned char pk_hash[32];
+    unsigned char second_pk[32];
     int pk_parity;
     int is_tweaked;
     unsigned char tweak[32];
@@ -94,7 +97,7 @@ typedef struct {
  * The workflow for this structure is as follows:
  *
  * 1. This structure is initialized with `musig_session_init` or
- *    `musig_session_init_verifier`, which set the `index` field, and zero out
+ *    `musig_session_init_verifier`, which initializes
  *    all other fields. The public session is initialized with the signers'
  *    nonce_commitments.
  *
@@ -111,14 +114,12 @@ typedef struct {
  *
  * Fields:
  *   present: indicates whether the signer's nonce is set
- *     index: index of the signer in the MuSig key aggregation
  *     nonce: public nonce, must be a valid curvepoint if the signer is `present`
  * nonce_commitment: commitment to the nonce, or all-bits zero if a commitment
  *                   has not yet been set
  */
 typedef struct {
     int present;
-    uint32_t index;
     secp256k1_xonly_pubkey nonce;
     unsigned char nonce_commitment[32];
 } secp256k1_musig_session_signer_data;
@@ -137,8 +138,14 @@ typedef struct {
 } secp256k1_musig_partial_signature;
 
 /** Computes a combined public key and the hash of the given public keys.
+ *
  *  Different orders of `pubkeys` result in different `combined_pk`s.
  *
+ *  The pubkeys can be sorted before combining with `secp256k1_xonly_sort` which
+ *  ensures the same resulting `combined_pk` for the same multiset of pubkeys.
+ *  This is useful to do before pubkey_combine, such that the order of pubkeys
+ *  does not affect the combined public key.
+ *
  *  Returns: 1 if the public keys were successfully combined, 0 otherwise
  *  Args:        ctx: pointer to a context object initialized for verification
  *                    (cannot be NULL)
@@ -147,17 +154,17 @@ typedef struct {
  *  Out: combined_pk: the MuSig-combined xonly public key (cannot be NULL)
  *       pre_session: if non-NULL, pointer to a musig_pre_session struct to be used in
  *                    `musig_session_init` or `musig_pubkey_tweak_add`.
- *   In:     pubkeys: input array of public keys to combine. The order is important;
- *                    a different order will result in a different combined public
- *                    key (cannot be NULL)
+ *   In:     pubkeys: input array of pointers to public keys to combine. The order
+ *                    is important; a different order will result in a different
+ *                    combined public key (cannot be NULL)
  *         n_pubkeys: length of pubkeys array. Must be greater than 0.
  */
 SECP256K1_API int secp256k1_musig_pubkey_combine(
     const secp256k1_context* ctx,
     secp256k1_scratch_space *scratch,
     secp256k1_xonly_pubkey *combined_pk,
     secp256k1_musig_pre_session *pre_session,
-    const secp256k1_xonly_pubkey *pubkeys,
+    const secp256k1_xonly_pubkey * const* pubkeys,
     size_t n_pubkeys
 ) SECP256K1_ARG_NONNULL(1) SECP256K1_ARG_NONNULL(3) SECP256K1_ARG_NONNULL(5);
 
@@ -221,8 +228,6 @@ SECP256K1_API SECP256K1_WARN_UNUSED_RESULT int secp256k1_musig_pubkey_tweak_add(
  *                     `musig_pubkey_tweak_add` (cannot be NULL).
  *          n_signers: length of signers array. Number of signers participating in
  *                     the MuSig. Must be greater than 0 and at most 2^32 - 1.
- *           my_index: index of this signer in the signers array. Must be less
- *                     than `n_signers`.
  *             seckey: the signer's 32-byte secret key (cannot be NULL)
  */
 SECP256K1_API int secp256k1_musig_session_init(
@@ -235,7 +240,6 @@ SECP256K1_API int secp256k1_musig_session_init(
     const secp256k1_xonly_pubkey *combined_pk,
     const secp256k1_musig_pre_session *pre_session,
     size_t n_signers,
-    size_t my_index,
     const unsigned char *seckey
 ) SECP256K1_ARG_NONNULL(1) SECP256K1_ARG_NONNULL(2) SECP256K1_ARG_NONNULL(3) SECP256K1_ARG_NONNULL(4) SECP256K1_ARG_NONNULL(5) SECP256K1_ARG_NONNULL(7) SECP256K1_ARG_NONNULL(8) SECP256K1_ARG_NONNULL(11);
 

src/modules/extrakeys/Makefile.am.include

@@ -2,3 +2,5 @@ include_HEADERS += include/secp256k1_extrakeys.h
 noinst_HEADERS += src/modules/extrakeys/tests_impl.h
 noinst_HEADERS += src/modules/extrakeys/tests_exhaustive_impl.h
 noinst_HEADERS += src/modules/extrakeys/main_impl.h
+noinst_HEADERS += src/modules/extrakeys/hsort.h
+noinst_HEADERS += src/modules/extrakeys/hsort_impl.h

src/modules/extrakeys/hsort.h

@@ -0,0 +1,22 @@
+/***********************************************************************
+ * Copyright (c) 2021 Russell O'Connor, Jonas Nick                     *
+ * Distributed under the MIT software license, see the accompanying    *
+ * file COPYING or https://www.opensource.org/licenses/mit-license.php.*
+ ***********************************************************************/
+
+#ifndef SECP256K1_HSORT_H_
+#define SECP256K1_HSORT_H_
+
+#include <stddef.h>
+#include <string.h>
+
+/* In-place, iterative heapsort with an interface matching glibc's qsort_r. This
+ * is preferred over standard library implementations because they generally
+ * make no guarantee about being fast for malicious inputs.
+ *
+ * See the qsort_r manpage for a description of the interface.
+ */
+static void secp256k1_hsort(void *ptr, size_t count, size_t size,
+                            int (*cmp)(const void *, const void *, void *),
+                            void *cmp_data);
+#endif

src/modules/extrakeys/hsort_impl.h

@@ -0,0 +1,116 @@
+/***********************************************************************
+ * Copyright (c) 2021 Russell O'Connor, Jonas Nick                     *
+ * Distributed under the MIT software license, see the accompanying    *
+ * file COPYING or https://www.opensource.org/licenses/mit-license.php.*
+ ***********************************************************************/
+
+#ifndef SECP256K1_HSORT_IMPL_H_
+#define SECP256K1_HSORT_IMPL_H_
+
+#include "hsort.h"
+
+/* An array is a heap when, for all non-zero indexes i, the element at index i
+ * compares as less than or equal to the element at index parent(i) = (i-1)/2.
+ */
+
+static SECP256K1_INLINE size_t child1(size_t i) {
+    VERIFY_CHECK(i <= (SIZE_MAX - 1)/2);
+    return 2*i + 1;
+}
+
+static SECP256K1_INLINE size_t child2(size_t i) {
+    VERIFY_CHECK(i <= SIZE_MAX/2 - 1);
+    return child1(i)+1;
+}
+
+static SECP256K1_INLINE void swap64(unsigned char *a, size_t i, size_t j, size_t stride) {
+    unsigned char tmp[64];
+    VERIFY_CHECK(stride <= 64);
+    memcpy(tmp, a + i*stride, stride);
+    memmove(a + i*stride, a + j*stride, stride);
+    memcpy(a + j*stride, tmp, stride);
+}
+
+static SECP256K1_INLINE void swap(unsigned char *a, size_t i, size_t j, size_t stride) {
+    while (64 < stride) {
+        swap64(a + (stride - 64), i, j, 64);
+        stride -= 64;
+    }
+    swap64(a, i, j, stride);
+}
+
+static SECP256K1_INLINE void heap_down(unsigned char *a, size_t i, size_t heap_size, size_t stride,
+                            int (*cmp)(const void *, const void *, void *), void *cmp_data) {
+    while (i < heap_size/2) {
+        VERIFY_CHECK(i <= SIZE_MAX/2 - 1);
+        /* Proof:
+         * i < heap_size/2
+         * i + 1 <= heap_size/2
+         * 2*i + 2 <= heap_size <= SIZE_MAX
+         * 2*i <= SIZE_MAX - 2
+         */
+
+        VERIFY_CHECK(child1(i) < heap_size);
+        /* Proof:
+         * i < heap_size/2
+         * i + 1 <= heap_size/2
+         * 2*i + 2 <= heap_size
+         * 2*i + 1 < heap_size
+         * child1(i) < heap_size
+         */
+
+        /* Let [x] be notation for the contents at a[x*stride].
+         *
+         * If [child1(i)] > [i] and [child2(i)] > [i],
+         *   swap [i] with the larger child to ensure the new parent is larger
+         *   than both children. When [child1(i)] == [child2(i)], swap [i] with
+         *   [child2(i)].
+         * Else if [child1(i)] > [i], swap [i] with [child1(i)].
+         * Else if [child2(i)] > [i], swap [i] with [child2(i)].
+         */
+        if (child2(i) < heap_size
+                && 0 <= cmp(a + child2(i)*stride, a + child1(i)*stride, cmp_data)) {
+            if (0 < cmp(a + child2(i)*stride, a +         i*stride, cmp_data)) {
+                swap(a, i, child2(i), stride);
+                i = child2(i);
+            } else {
+                /* At this point we have [child2(i)] >= [child1(i)] and we have
+                 * [child2(i)] <= [i], and thus [child1(i)] <= [i] which means
+                 * that the next comparison can be skipped. */
+                return;
+            }
+        } else if (0 < cmp(a + child1(i)*stride, a +         i*stride, cmp_data)) {
+            swap(a, i, child1(i), stride);
+            i = child1(i);
+        } else {
+            return;
+        }
+    }
+    /* heap_size/2 <= i
+     * heap_size/2 < i + 1
+     * heap_size < 2*i + 2
+     * heap_size <= 2*i + 1
+     * heap_size <= child1(i)
+     * Thus child1(i) and child2(i) are now out of bounds and we are at a leaf.
+     */
+}
+
+/* In-place heap sort. */
+static void secp256k1_hsort(void *ptr, size_t count, size_t size,
+                            int (*cmp)(const void *, const void *, void *),
+                            void *cmp_data ) {
+    size_t i;
+
+    for(i = count/2; 0 < i; --i) {
+        heap_down(ptr, i-1, count, size, cmp, cmp_data);
+    }
+    for(i = count; 1 < i; --i) {
+        /* Extract the largest value from the heap */
+        swap(ptr, 0, i-1, size);
+
+        /* Repair the heap condition */
+        heap_down(ptr, 0, i-1, size, cmp, cmp_data);
+    }
+}
+
+#endif

src/modules/extrakeys/main_impl.h

@@ -9,6 +9,7 @@
 
 #include "../../../include/secp256k1.h"
 #include "../../../include/secp256k1_extrakeys.h"
+#include "hsort_impl.h"
 
 static SECP256K1_INLINE int secp256k1_xonly_pubkey_load(const secp256k1_context* ctx, secp256k1_ge *ge, const secp256k1_xonly_pubkey *pubkey) {
     return secp256k1_pubkey_load(ctx, ge, (const secp256k1_pubkey *) pubkey);
@@ -154,6 +155,28 @@ int secp256k1_xonly_pubkey_tweak_add_check(const secp256k1_context* ctx, const u
             && secp256k1_fe_is_odd(&pk.y) == tweaked_pk_parity;
 }
 
+/* This struct wraps a const context pointer to satisfy the secp256k1_hsort api
+ * which expects a non-const cmp_data pointer. */
+typedef struct {
+    const secp256k1_context *ctx;
+} secp256k1_xonly_sort_cmp_data;
+
+static int secp256k1_xonly_sort_cmp(const void* pk1, const void* pk2, void *cmp_data) {
+    return secp256k1_xonly_pubkey_cmp(((secp256k1_xonly_sort_cmp_data*)cmp_data)->ctx,
+                                      *(secp256k1_xonly_pubkey **)pk1,
+                                      *(secp256k1_xonly_pubkey **)pk2);
+}
+
+int secp256k1_xonly_sort(const secp256k1_context* ctx, const secp256k1_xonly_pubkey **pubkeys, size_t n_pubkeys) {
+    secp256k1_xonly_sort_cmp_data cmp_data;
+    VERIFY_CHECK(ctx != NULL);
+    ARG_CHECK(pubkeys != NULL);
+
+    cmp_data.ctx = ctx;
+    secp256k1_hsort(pubkeys, n_pubkeys, sizeof(*pubkeys), secp256k1_xonly_sort_cmp, &cmp_data);
+    return 1;
+}
+
 static void secp256k1_keypair_save(secp256k1_keypair *keypair, const secp256k1_scalar *sk, secp256k1_ge *pk) {
     secp256k1_scalar_get_b32(&keypair->data[0], sk);
     secp256k1_pubkey_save((secp256k1_pubkey *)&keypair->data[32], pk);