Merge #1518: Add secp256k1_pubkey_sort

7d2591c Add secp256k1_pubkey_sort (Jonas Nick)

Pull request description:

  This PR adds a  `secp256k1_pubkey_sort` function the the public API which was originally part of the musig PR (#1479). However, I opened a separate PR because it adds internal functions that are also used by the WIP silent payments module.

ACKs for top commit:
  sipa:
    ACK 7d2591c
  josibake:
    ACK 7d2591c
  real-or-random:
    ACK 7d2591c

Tree-SHA512: d0e4464dc9cd4bdb35cc5d9bb4c37a7b71233328319165d49bc940d8d3394a2d74a43d2f73ee7bfe8f3f90a466ee8afcdca75cfbbf3969e218d76b89f4af55fb

CHANGELOG.md

@@ -7,6 +7,9 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
 ## [Unreleased]
 
+#### Added
+ - New function `secp256k1_ec_pubkey_sort` that sorts public keys using lexicographic (of compressed serialization) order.
+
 #### Changed
  - The implementation of the point multiplication algorithm used for signing and public key generation was changed, resulting in improved performance for those operations.
    - The related configure option `--ecmult-gen-precision` was replaced with `--ecmult-gen-kb` (`ECMULT_GEN_KB` for CMake).

Makefile.am

@@ -64,6 +64,8 @@ noinst_HEADERS += src/field.h
 noinst_HEADERS += src/field_impl.h
 noinst_HEADERS += src/bench.h
 noinst_HEADERS += src/wycheproof/ecdsa_secp256k1_sha256_bitcoin_test.h
+noinst_HEADERS += src/hsort.h
+noinst_HEADERS += src/hsort_impl.h
 noinst_HEADERS += contrib/lax_der_parsing.h
 noinst_HEADERS += contrib/lax_der_parsing.c
 noinst_HEADERS += contrib/lax_der_privatekey_parsing.h

include/secp256k1.h

@@ -474,6 +474,20 @@ SECP256K1_API SECP256K1_WARN_UNUSED_RESULT int secp256k1_ec_pubkey_cmp(
     const secp256k1_pubkey *pubkey2
 ) SECP256K1_ARG_NONNULL(1) SECP256K1_ARG_NONNULL(2) SECP256K1_ARG_NONNULL(3);
 
+/** Sort public keys using lexicographic (of compressed serialization) order
+ *
+ *  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_ec_pubkey_sort(
+    const secp256k1_context *ctx,
+    const secp256k1_pubkey **pubkeys,
+    size_t n_pubkeys
+) SECP256K1_ARG_NONNULL(1) SECP256K1_ARG_NONNULL(2);
+
 /** Parse an ECDSA signature in compact (64 bytes) format.
  *
  *  Returns: 1 when the signature could be parsed, 0 otherwise.

src/hsort.h

@@ -0,0 +1,33 @@
+/***********************************************************************
+ * 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.
+ * Remember that heapsort is unstable.
+ *
+ * In/Out: ptr: pointer to the array to sort. The contents of the array are
+ *              sorted in ascending order according to the comparison function.
+ * In:   count: number of elements in the array.
+ *        size: size in bytes of each element.
+ *         cmp: pointer to a comparison function that is called with two
+ *              arguments that point to the objects being compared. The cmp_data
+ *              argument of secp256k1_hsort is passed as third argument. The
+ *              function must return an integer less than, equal to, or greater
+ *              than zero if the first argument is considered to be respectively
+ *              less than, equal to, or greater than the second.
+ *    cmp_data: pointer passed as third argument to cmp.
+ */
+static void secp256k1_hsort(void *ptr, size_t count, size_t size,
+                            int (*cmp)(const void *, const void *, void *),
+                            void *cmp_data);
+#endif

src/hsort_impl.h

@@ -0,0 +1,125 @@
+/***********************************************************************
+ * 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 secp256k1_heap_child1(size_t i) {
+    VERIFY_CHECK(i <= (SIZE_MAX - 1)/2);
+    return 2*i + 1;
+}
+
+static SECP256K1_INLINE size_t secp256k1_heap_child2(size_t i) {
+    VERIFY_CHECK(i <= SIZE_MAX/2 - 1);
+    return secp256k1_heap_child1(i)+1;
+}
+
+static SECP256K1_INLINE void secp256k1_heap_swap64(unsigned char *a, unsigned char *b, size_t len) {
+    unsigned char tmp[64];
+    VERIFY_CHECK(len <= 64);
+    memcpy(tmp, a, len);
+    memmove(a, b, len);
+    memcpy(b, tmp, len);
+}
+
+static SECP256K1_INLINE void secp256k1_heap_swap(unsigned char *arr, size_t i, size_t j, size_t stride) {
+    unsigned char *a = arr + i*stride;
+    unsigned char *b = arr + j*stride;
+    size_t len = stride;
+    while (64 < len) {
+        secp256k1_heap_swap64(a + (len - 64), b + (len - 64), 64);
+        len -= 64;
+    }
+    secp256k1_heap_swap64(a, b, len);
+}
+
+/* This function accepts an array arr containing heap_size elements, each of
+ * size stride. The elements in the array at indices >i satisfy the max-heap
+ * property, i.e., for any element at index j (where j > i), all of its children
+ * are smaller than the element itself. The purpose of the function is to update
+ * the array so that all elements at indices >=i satisfy the max-heap
+ * property. */
+static SECP256K1_INLINE void secp256k1_heap_down(unsigned char *arr, 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(secp256k1_heap_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 arr[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 (secp256k1_heap_child2(i) < heap_size
+                && 0 <= cmp(arr + secp256k1_heap_child2(i)*stride, arr + secp256k1_heap_child1(i)*stride, cmp_data)) {
+            if (0 < cmp(arr + secp256k1_heap_child2(i)*stride, arr + i*stride, cmp_data)) {
+                secp256k1_heap_swap(arr, i, secp256k1_heap_child2(i), stride);
+                i = secp256k1_heap_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(arr + secp256k1_heap_child1(i)*stride, arr +         i*stride, cmp_data)) {
+            secp256k1_heap_swap(arr, i, secp256k1_heap_child1(i), stride);
+            i = secp256k1_heap_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) {
+        secp256k1_heap_down(ptr, i-1, count, size, cmp, cmp_data);
+    }
+    for (i = count; 1 < i; --i) {
+        /* Extract the largest value from the heap */
+        secp256k1_heap_swap(ptr, 0, i-1, size);
+
+        /* Repair the heap condition */
+        secp256k1_heap_down(ptr, 0, i-1, size, cmp, cmp_data);
+    }
+}
+
+#endif

src/secp256k1.c

@@ -36,6 +36,7 @@
 #include "int128_impl.h"
 #include "scratch_impl.h"
 #include "selftest.h"
+#include "hsort_impl.h"
 
 #ifdef SECP256K1_NO_BUILD
 # error "secp256k1.h processed without SECP256K1_BUILD defined while building secp256k1.c"
@@ -325,6 +326,34 @@ int secp256k1_ec_pubkey_cmp(const secp256k1_context* ctx, const secp256k1_pubkey
     return secp256k1_memcmp_var(out[0], out[1], sizeof(out[0]));
 }
 
+static int secp256k1_ec_pubkey_sort_cmp(const void* pk1, const void* pk2, void *ctx) {
+    return secp256k1_ec_pubkey_cmp((secp256k1_context *)ctx,
+                                     *(secp256k1_pubkey **)pk1,
+                                     *(secp256k1_pubkey **)pk2);
+}
+
+int secp256k1_ec_pubkey_sort(const secp256k1_context* ctx, const secp256k1_pubkey **pubkeys, size_t n_pubkeys) {
+    VERIFY_CHECK(ctx != NULL);
+    ARG_CHECK(pubkeys != NULL);
+
+    /* Suppress wrong warning (fixed in MSVC 19.33) */
+    #if defined(_MSC_VER) && (_MSC_VER < 1933)
+    #pragma warning(push)
+    #pragma warning(disable: 4090)
+    #endif
+
+    /* Casting away const is fine because neither secp256k1_hsort nor
+     * secp256k1_ec_pubkey_sort_cmp modify the data pointed to by the cmp_data
+     * argument. */
+    secp256k1_hsort(pubkeys, n_pubkeys, sizeof(*pubkeys), secp256k1_ec_pubkey_sort_cmp, (void *)ctx);
+
+    #if defined(_MSC_VER) && (_MSC_VER < 1933)
+    #pragma warning(pop)
+    #endif
+
+    return 1;
+}
+
 static void secp256k1_ecdsa_signature_load(const secp256k1_context* ctx, secp256k1_scalar* r, secp256k1_scalar* s, const secp256k1_ecdsa_signature* sig) {
     (void)ctx;
     if (sizeof(secp256k1_scalar) == 32) {