aggregate and inc_aggregate: allow NULL if n is 0

BlockstreamResearch · Feb 26, 2024 · 2ed3978 · 2ed3978
1 parent 25bc8f3
commit 2ed3978
Show file tree

Hide file tree

Showing 3 changed files with 29 additions and 15 deletions.
diff --git a/include/secp256k1_schnorrsig_halfagg.h b/include/secp256k1_schnorrsig_halfagg.h
@@ -28,11 +28,13 @@ extern "C" {
  *  In:     all_pubkeys: Array of (n_before + n_new) many x-only public keys,
  *                       including both the ones for the already aggregated signature
  *                       and the ones for the signatures that should be added.
+ *                       Can only be NULL if n_before + n_new is 0.
  *           all_msgs32: Array of (n_before + n_new) many 32-byte messages,
  *                       including both the ones for the already aggregated signature
  *                       and the ones for the signatures that should be added.
+ *                       Can only be NULL if n_before + n_new is 0.
  *           new_sigs64: Array of n_new many 64-byte signatures, containing the new
- *                       signatures that should be added.
+ *                       signatures that should be added. Can only be NULL if n_new is 0.
  *             n_before: Number of signatures that have already been aggregated
  *                       in the input aggregate signature.
  *                n_new: Number of signatures that should now be added
@@ -47,7 +49,7 @@ SECP256K1_API int secp256k1_schnorrsig_inc_aggregate(
     const unsigned char *new_sigs64,
     size_t n_before,
     size_t n_new
-) SECP256K1_ARG_NONNULL(1) SECP256K1_ARG_NONNULL(2) SECP256K1_ARG_NONNULL(3) SECP256K1_ARG_NONNULL(4) SECP256K1_ARG_NONNULL(5) SECP256K1_ARG_NONNULL(6);
+) SECP256K1_ARG_NONNULL(1) SECP256K1_ARG_NONNULL(2) SECP256K1_ARG_NONNULL(3);
 
 /** (Half-)Aggregate a sequence of Schnorr signatures.
  *
@@ -58,8 +60,11 @@ SECP256K1_API int secp256k1_schnorrsig_inc_aggregate(
  *  In/Out:  aggsig_len: size of the aggsig array that is passed in bytes;
  *                       will be overwritten to be the exact size of aggsig.
  *  In:         pubkeys: Array of n many x-only public keys.
+ *                       Can only be NULL if n is 0.
  *               msgs32: Array of n many 32-byte messages.
+ *                       Can only be NULL if n is 0.
  *               sigs64: Array of n many 64-byte signatures.
+ *                       Can only be NULL if n is 0.
  *                    n: number of signatures to be aggregated.
  */
 SECP256K1_API int secp256k1_schnorrsig_aggregate(
@@ -70,7 +75,7 @@ SECP256K1_API int secp256k1_schnorrsig_aggregate(
     const unsigned char *msgs32,
     const unsigned char *sigs64,
     size_t n
-) SECP256K1_ARG_NONNULL(1) SECP256K1_ARG_NONNULL(2) SECP256K1_ARG_NONNULL(3) SECP256K1_ARG_NONNULL(4) SECP256K1_ARG_NONNULL(5) SECP256K1_ARG_NONNULL(6);
+) SECP256K1_ARG_NONNULL(1) SECP256K1_ARG_NONNULL(2) SECP256K1_ARG_NONNULL(3);
 
 /** Verify a (Half-)aggregate Schnorr signature.
  *

diff --git a/src/modules/schnorrsig_halfagg/main_impl.h b/src/modules/schnorrsig_halfagg/main_impl.h
@@ -31,13 +31,13 @@ int secp256k1_schnorrsig_inc_aggregate(const secp256k1_context *ctx, unsigned ch
     VERIFY_CHECK(ctx != NULL);
     ARG_CHECK(aggsig != NULL);
     ARG_CHECK(aggsig_len != NULL);
-    ARG_CHECK(all_pubkeys != NULL);
-    ARG_CHECK(all_msgs32 != NULL);
-    ARG_CHECK(new_sigs64 != NULL);
+    ARG_CHECK(new_sigs64 != NULL || n_new == 0);
 
     /* Check that aggsig_len is large enough, i.e. aggsig_len >= 32*(n+1) */
     n = n_before + n_new;
     ARG_CHECK(n >= n_before);
+    ARG_CHECK(all_pubkeys != NULL || n == 0);
+    ARG_CHECK(all_msgs32 != NULL || n == 0);
     if ((*aggsig_len / 32) <= 0 || ((*aggsig_len / 32) - 1) < n) {
         return 0;
     }

diff --git a/src/modules/schnorrsig_halfagg/tests_impl.h b/src/modules/schnorrsig_halfagg/tests_impl.h
@@ -179,28 +179,37 @@ static void test_schnorrsig_aggregate_api(void) {
     unsigned char aggsig[32*(N_MAX + 1)];
     test_schnorrsig_aggregate_input_helper(pubkeys, msgs32, sigs64, n);
 
-    /* Test body 1: Check API of function aggregate.
-     * Should not accept NULL for any pointer input. */
+    /* Test body 1: Check API of function aggregate. */
     {
+        /* Should not accept NULL for aggsig or aggsig length */
         size_t aggsig_len = sizeof(aggsig);
         CHECK_ILLEGAL(CTX, secp256k1_schnorrsig_aggregate(CTX, NULL, &aggsig_len, pubkeys, msgs32, sigs64, n_initial));
         CHECK_ILLEGAL(CTX, secp256k1_schnorrsig_aggregate(CTX, aggsig, NULL, pubkeys, msgs32, sigs64, n_initial));
-        CHECK_ILLEGAL(CTX, secp256k1_schnorrsig_aggregate(CTX, aggsig, &aggsig_len, NULL, msgs32, sigs64, n_initial));
-        CHECK_ILLEGAL(CTX, secp256k1_schnorrsig_aggregate(CTX, aggsig, &aggsig_len, pubkeys, NULL, sigs64, n_initial));
-        CHECK_ILLEGAL(CTX, secp256k1_schnorrsig_aggregate(CTX, aggsig, &aggsig_len, pubkeys, msgs32, NULL, n_initial));
+        /* Should not accept NULL for keys, messages, or signatures if n_initial is not 0 */
+        if (n_initial != 0) {
+            CHECK_ILLEGAL(CTX, secp256k1_schnorrsig_aggregate(CTX, aggsig, &aggsig_len, NULL, msgs32, sigs64, n_initial));
+            CHECK_ILLEGAL(CTX, secp256k1_schnorrsig_aggregate(CTX, aggsig, &aggsig_len, pubkeys, NULL, sigs64, n_initial));
+            CHECK_ILLEGAL(CTX, secp256k1_schnorrsig_aggregate(CTX, aggsig, &aggsig_len, pubkeys, msgs32, NULL, n_initial));
+        }
     }
 
     /* Test body 2: Check API of function inc_aggregate. */
     {
         size_t aggsig_len = sizeof(aggsig);
         CHECK(secp256k1_schnorrsig_aggregate(CTX, aggsig, &aggsig_len, pubkeys, msgs32, sigs64, n_initial));
         aggsig_len = 32*(n+1);
-        /* Should not accept NULL for any pointer input. */
+        /* Should not accept NULL for aggsig or aggsig length */
         CHECK_ILLEGAL(CTX, secp256k1_schnorrsig_inc_aggregate(CTX, NULL, &aggsig_len, pubkeys, msgs32, &sigs64[n_initial*64], n_initial, n_new));
         CHECK_ILLEGAL(CTX, secp256k1_schnorrsig_inc_aggregate(CTX, aggsig, NULL, pubkeys, msgs32, &sigs64[n_initial*64], n_initial, n_new));
-        CHECK_ILLEGAL(CTX, secp256k1_schnorrsig_inc_aggregate(CTX, aggsig, &aggsig_len, NULL, msgs32, &sigs64[n_initial*64], n_initial, n_new));
-        CHECK_ILLEGAL(CTX, secp256k1_schnorrsig_inc_aggregate(CTX, aggsig, &aggsig_len, pubkeys, NULL, &sigs64[n_initial*64], n_initial, n_new));
-        CHECK_ILLEGAL(CTX, secp256k1_schnorrsig_inc_aggregate(CTX, aggsig, &aggsig_len, pubkeys, msgs32, NULL, n_initial, n_new));
+        /* Should not accept NULL for keys or messages if n is not 0 */
+        if (n != 0) {
+            CHECK_ILLEGAL(CTX, secp256k1_schnorrsig_inc_aggregate(CTX, aggsig, &aggsig_len, NULL, msgs32, &sigs64[n_initial*64], n_initial, n_new));
+            CHECK_ILLEGAL(CTX, secp256k1_schnorrsig_inc_aggregate(CTX, aggsig, &aggsig_len, pubkeys, NULL, &sigs64[n_initial*64], n_initial, n_new));
+        }
+        /* Should not accept NULL for new_sigs64 if n_new is not 0 */
+        if (n_new != 0) {
+            CHECK_ILLEGAL(CTX, secp256k1_schnorrsig_inc_aggregate(CTX, aggsig, &aggsig_len, pubkeys, msgs32, NULL, n_initial, n_new));
+        }
         /* Should not accept overflowing number of sigs. */
         CHECK_ILLEGAL(CTX, secp256k1_schnorrsig_inc_aggregate(CTX, aggsig, &aggsig_len, pubkeys, msgs32, &sigs64[n_initial*64], SIZE_MAX, SIZE_MAX));
         if (n_initial > 0) {