address comments from code review

include/secp256k1_schnorrsig_halfagg.h

@@ -15,26 +15,26 @@ extern "C" {
  *  Returns 1 on success, 0 on failure.
  *  Args:           ctx: a secp256k1 context object.
  *  In/Out:      aggsig: pointer to the serialized aggregate signature
- *                       that is input. Will be overwritten by the new
- *                       serialized aggregate signature.
- *          aggsig_size: size of the memory allocated in aggsig.
+ *                       that is input. The first 32*(n_before+1) of this
+ *                       array should hold the input aggsig. It will be
+ *                       overwritten by the new serialized aggregate signature.
+ *                       It should be large enough for that, see aggsig_size.
+ *          aggsig_size: size of aggsig array in bytes.
  *                       Should be large enough to hold the new
- *                       serialized aggregate signature.
- *                       I.e., should be a multiple of 32 and should
- *                       satisfy aggsig_size >= 32*(n_before+n_new+1)
- *  In:     all_pubkeys: Array of x-only public keys, including both
- *                       the ones for the already aggregated signature
+ *                       serialized aggregate signature, i.e.,
+ *                       should satisfy aggsig_size >= 32*(n_before+n_new+1).
+ *                       It will be overwritten to be the exact size of the
+ *                       resulting aggsig.
+ *  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.
- *                       Assumed to contain n = n_before + n_new many public keys.
- *           all_msgs32: Array of 32-byte messages, including both
- *                       the ones for the already aggregated signature
+ *           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.
- *                       Assumed to contain n = n_before + n_new many messages.
- *           new_sigs64: Array of 64-byte signatures, containing the new
+ *           new_sigs64: Array of n_new many 64-byte signatures, containing the new
  *                       signatures that should be added.
- *                       Assumed to contain n_new many signatures.
- *             n_before: Number of signatures that are already "contained"
- *                       in the aggregate signature.
+ *             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
  *                       to the aggregate signature.
  */
@@ -57,12 +57,9 @@ SECP256K1_API int secp256k1_schnorrsig_inc_aggregate(
  *                       store the serialized aggregate signature.
  *  In/Out: aggsig_size: size of the aggsig array that is passed;
  *                       will be overwritten to be the exact size of aggsig.
- *  In:         pubkeys: Array of x-only public keys.
- *                       Assumed to contain n many public keys.
- *               msgs32: Array of 32-byte messages.
- *                       Assumed to contain n many messages.
- *               sigs64: Array of 64-byte signatures.
- *                       Assumed to contain n many signatures.
+ *  In:         pubkeys: Array of n many x-only public keys.
+ *               msgs32: Array of n many 32-byte messages.
+ *               sigs64: Array of n many 64-byte signatures.
  *                    n: number of signatures to be aggregated.
  */
 SECP256K1_API int secp256k1_schnorrsig_aggregate(
@@ -80,10 +77,8 @@ SECP256K1_API int secp256k1_schnorrsig_aggregate(
  *  Returns:          1: correct signature.
  *                    0: incorrect signature.
  *  Args:           ctx: a secp256k1 context object.
- *  In:         pubkeys: Array of x-only public keys.
- *                       Assumed to contain n many public keys.
- *               msgs32: Array of 32-byte messages.
- *                       Assumed to contain n many messages.
+ *  In:         pubkeys: Array of n many x-only public keys.
+ *               msgs32: Array of n many 32-byte messages.
  *                    n: number of signatures to that have been aggregated.
  *               aggsig: Pointer to an array of aggsig_size many bytes
  *                       containing the serialized aggregate

src/modules/schnorrsig_halfagg/main_impl.h

@@ -39,7 +39,7 @@ int secp256k1_schnorrsig_inc_aggregate(const secp256k1_context *ctx, unsigned ch
     /* Check that aggsig_size is large enough, i.e. aggsig_size >= 32*(n+1) */
     n = n_before + n_new;
     ARG_CHECK(n >= n_before);
-    if ((*aggsig_size / 32) <= 0 || (*aggsig_size & 31) != 0 || ((*aggsig_size / 32)-1) < n) {
+    if ((*aggsig_size / 32) <= 0 || ((*aggsig_size / 32) - 1) < n) {
         return 0;
     }
 
@@ -114,10 +114,7 @@ int secp256k1_schnorrsig_inc_aggregate(const secp256k1_context *ctx, unsigned ch
 }
 
 int secp256k1_schnorrsig_aggregate(const secp256k1_context *ctx, unsigned char *aggsig, size_t *aggsig_size, const secp256k1_xonly_pubkey *pubkeys, const unsigned char *msgs32, const unsigned char *sigs64, size_t n) {
-    if (!secp256k1_schnorrsig_inc_aggregate(ctx, aggsig, aggsig_size, pubkeys, msgs32, sigs64, 0, n)) {
-        return 0;
-    }
-    return 1;
+    return secp256k1_schnorrsig_inc_aggregate(ctx, aggsig, aggsig_size, pubkeys, msgs32, sigs64, 0, n);
 }
 
 int secp256k1_schnorrsig_aggverify(const secp256k1_context *ctx, const secp256k1_xonly_pubkey *pubkeys, const unsigned char *msgs32, size_t n, const unsigned char *aggsig, size_t aggsig_size) {
@@ -131,9 +128,10 @@ int secp256k1_schnorrsig_aggverify(const secp256k1_context *ctx, const secp256k1
     ARG_CHECK(pubkeys != NULL);
     ARG_CHECK(msgs32 != NULL);
     ARG_CHECK(aggsig != NULL);
+    ARG_CHECK(secp256k1_ecmult_gen_context_is_built(&ctx->ecmult_gen_ctx));
 
     /* Check that aggsig_size is correct, i.e. aggsig_size = 32*(n+1) */
-    if ((aggsig_size / 32) <= 0 || (aggsig_size & 31) != 0 || ((aggsig_size / 32)-1) != n) {
+    if ((aggsig_size / 32) <= 0 || ((aggsig_size / 32)-1) != n || (aggsig_size % 32) != 0) {
         return 0;
     }
 
@@ -149,7 +147,7 @@ int secp256k1_schnorrsig_aggverify(const secp256k1_context *ctx, const secp256k1
         secp256k1_fe rx;
         secp256k1_ge rp, pp;
         secp256k1_scalar ei;
-        secp256k1_gej ppj, acc;
+        secp256k1_gej ppj, ti;
 
         unsigned char pk_ser[32];
         unsigned char hashoutput[32];
@@ -186,22 +184,21 @@ int secp256k1_schnorrsig_aggverify(const secp256k1_context *ctx, const secp256k1
         /* 2.b) e_i = int(hash_{BIP0340/challenge}(bytes(r_i) || pk_i || m_i)) mod n */
         secp256k1_schnorrsig_challenge(&ei, &aggsig[i*32], &msgs32[i*32], 32, pk_ser);
         secp256k1_gej_set_ge(&ppj, &pp);
-        /* 2.c) acc = R_i + e_i⋅P_i */
-        secp256k1_ecmult(&acc, &ppj, &ei, NULL);
-        secp256k1_gej_add_ge_var(&acc, &acc, &rp, NULL);
-
+        /* 2.c) T_i = R_i + e_i*P_i */
+        secp256k1_ecmult(&ti, &ppj, &ei, NULL);
+        secp256k1_gej_add_ge_var(&ti, &ti, &rp, NULL);
 
-        /* Step 3: rhs = rhs + zi*acc  */
-        secp256k1_ecmult(&acc, &acc, &zi, NULL);
-        secp256k1_gej_add_var(&rhs,&rhs,&acc,NULL);
+        /* Step 3: rhs = rhs + zi*T_i  */
+        secp256k1_ecmult(&ti, &ti, &zi, NULL);
+        secp256k1_gej_add_var(&rhs, &rhs, &ti, NULL);
     }
 
     /* Compute the lhs as lhs = s*G */
     secp256k1_scalar_set_b32(&s, &aggsig[n*32], &overflow);
     if (overflow) {
         return 0;
     }
-    secp256k1_ecmult(&lhs, NULL, &secp256k1_scalar_zero, &s);
+    secp256k1_ecmult_gen(&ctx->ecmult_gen_ctx, &lhs, &s);
 
     /* Check that lhs == rhs */
     secp256k1_gej_neg(&lhs, &lhs);