Clear output in more cases for pubkey_combine, adds tests.

Also corrects an outdated comment and adds an additional
 secp256k1_ecdsa_signature_parse_compact test.

include/secp256k1.h

@@ -562,12 +562,10 @@ SECP256K1_API SECP256K1_WARN_UNUSED_RESULT int secp256k1_context_randomize(
  *  Returns: 1: the sum of the public keys is valid.
  *           0: the sum of the public keys is not valid.
  *  Args:   ctx:        pointer to a context object
- *  Out:    out:        pointer to pubkey for placing the resulting public key
+ *  Out:    out:        pointer to a public key object for placing the resulting public key
  *                      (cannot be NULL)
  *  In:     ins:        pointer to array of pointers to public keys (cannot be NULL)
  *          n:          the number of public keys to add together (must be at least 1)
- *  Use secp256k1_ec_pubkey_compress and secp256k1_ec_pubkey_decompress if the
- *  uncompressed format is needed.
  */
 SECP256K1_API SECP256K1_WARN_UNUSED_RESULT int secp256k1_ec_pubkey_combine(
     const secp256k1_context* ctx,

src/secp256k1.c

@@ -536,6 +536,7 @@ int secp256k1_ec_pubkey_combine(const secp256k1_context* ctx, secp256k1_pubkey *
     secp256k1_ge Q;
 
     ARG_CHECK(pubnonce != NULL);
+    memset(pubnonce, 0, sizeof(*pubnonce));
     ARG_CHECK(n >= 1);
     ARG_CHECK(pubnonces != NULL);
 
@@ -546,7 +547,6 @@ int secp256k1_ec_pubkey_combine(const secp256k1_context* ctx, secp256k1_pubkey *
         secp256k1_gej_add_ge(&Qj, &Qj, &Q);
     }
     if (secp256k1_gej_is_infinity(&Qj)) {
-        memset(pubnonce, 0, sizeof(*pubnonce));
         return 0;
     }
     secp256k1_ge_set_gej(&Q, &Qj);

src/tests.c

@@ -2902,10 +2902,14 @@ void run_eckey_edge_case_test(void) {
         0xbf, 0xd2, 0x5e, 0x8c, 0xd0, 0x36, 0x41, 0x41
     };
     const unsigned char zeros[sizeof(secp256k1_pubkey)] = {0x00};
-    unsigned char ctmp[32];
-    unsigned char ctmp2[32];
+    unsigned char ctmp[33];
+    unsigned char ctmp2[33];
     secp256k1_pubkey pubkey;
     secp256k1_pubkey pubkey2;
+    secp256k1_pubkey pubkey_one;
+    secp256k1_pubkey pubkey_negone;
+    const secp256k1_pubkey *pubkeys[3];
+    size_t len;
     int32_t ecount;
     /* Group order is too large, reject. */
     CHECK(secp256k1_ec_seckey_verify(ctx, orderc) == 0);
@@ -2937,6 +2941,7 @@ void run_eckey_edge_case_test(void) {
     CHECK(secp256k1_ec_pubkey_create(ctx, &pubkey, ctmp) == 1);
     VG_CHECK(&pubkey, sizeof(pubkey));
     CHECK(memcmp(&pubkey, zeros, sizeof(secp256k1_pubkey)) > 0);
+    pubkey_one = pubkey;
     /* Group order + 1 is too large, reject. */
     memcpy(ctmp, orderc, 32);
     ctmp[31] = 0x42;
@@ -2954,6 +2959,7 @@ void run_eckey_edge_case_test(void) {
     CHECK(secp256k1_ec_pubkey_create(ctx, &pubkey, ctmp) == 1);
     VG_CHECK(&pubkey, sizeof(pubkey));
     CHECK(memcmp(&pubkey, zeros, sizeof(secp256k1_pubkey)) > 0);
+    pubkey_negone = pubkey;
     /* Tweak of zero leaves the value changed. */
     memset(ctmp2, 0, 32);
     CHECK(secp256k1_ec_privkey_tweak_add(ctx, ctmp, ctmp2) == 1);
@@ -3060,6 +3066,73 @@ void run_eckey_edge_case_test(void) {
     CHECK(secp256k1_ec_pubkey_create(ctx, &pubkey, NULL) == 0);
     CHECK(ecount == 2);
     CHECK(memcmp(&pubkey, zeros, sizeof(secp256k1_pubkey)) == 0);
+    /* secp256k1_ec_pubkey_combine tests. */
+    ecount = 0;
+    pubkeys[0] = &pubkey_one;
+    VG_UNDEF(&pubkeys[0], sizeof(secp256k1_pubkey *));
+    VG_UNDEF(&pubkeys[1], sizeof(secp256k1_pubkey *));
+    VG_UNDEF(&pubkeys[2], sizeof(secp256k1_pubkey *));
+    memset(&pubkey, 255, sizeof(secp256k1_pubkey));
+    VG_UNDEF(&pubkey, sizeof(secp256k1_pubkey));
+    CHECK(secp256k1_ec_pubkey_combine(ctx, &pubkey, pubkeys, 0) == 0);
+    VG_CHECK(&pubkey, sizeof(secp256k1_pubkey));
+    CHECK(memcmp(&pubkey, zeros, sizeof(secp256k1_pubkey)) == 0);
+    CHECK(ecount == 1);
+    memset(&pubkey, 255, sizeof(secp256k1_pubkey));
+    VG_UNDEF(&pubkey, sizeof(secp256k1_pubkey));
+    CHECK(secp256k1_ec_pubkey_combine(ctx, &pubkey, pubkeys, -1) == 0);
+    VG_CHECK(&pubkey, sizeof(secp256k1_pubkey));
+    CHECK(memcmp(&pubkey, zeros, sizeof(secp256k1_pubkey)) == 0);
+    CHECK(ecount == 2);
+    CHECK(secp256k1_ec_pubkey_combine(ctx, NULL, pubkeys, 1) == 0);
+    CHECK(memcmp(&pubkey, zeros, sizeof(secp256k1_pubkey)) == 0);
+    CHECK(ecount == 3);
+    memset(&pubkey, 255, sizeof(secp256k1_pubkey));
+    VG_UNDEF(&pubkey, sizeof(secp256k1_pubkey));
+    CHECK(secp256k1_ec_pubkey_combine(ctx, &pubkey, NULL, 1) == 0);
+    VG_CHECK(&pubkey, sizeof(secp256k1_pubkey));
+    CHECK(memcmp(&pubkey, zeros, sizeof(secp256k1_pubkey)) == 0);
+    CHECK(ecount == 4);
+    pubkeys[0] = &pubkey_negone;
+    memset(&pubkey, 255, sizeof(secp256k1_pubkey));
+    VG_UNDEF(&pubkey, sizeof(secp256k1_pubkey));
+    CHECK(secp256k1_ec_pubkey_combine(ctx, &pubkey, pubkeys, 1) == 1);
+    VG_CHECK(&pubkey, sizeof(secp256k1_pubkey));
+    CHECK(memcmp(&pubkey, zeros, sizeof(secp256k1_pubkey)) > 0);
+    CHECK(ecount == 4);
+    len = 33;
+    CHECK(secp256k1_ec_pubkey_serialize(ctx, ctmp, &len, &pubkey, SECP256K1_EC_COMPRESSED) == 1);
+    CHECK(secp256k1_ec_pubkey_serialize(ctx, ctmp2, &len, &pubkey_negone, SECP256K1_EC_COMPRESSED) == 1);
+    CHECK(memcmp(ctmp, ctmp2, 33) == 0);
+    /* Result is infinity. */
+    pubkeys[0] = &pubkey_one;
+    pubkeys[1] = &pubkey_negone;
+    memset(&pubkey, 255, sizeof(secp256k1_pubkey));
+    VG_UNDEF(&pubkey, sizeof(secp256k1_pubkey));
+    CHECK(secp256k1_ec_pubkey_combine(ctx, &pubkey, pubkeys, 2) == 0);
+    VG_CHECK(&pubkey, sizeof(secp256k1_pubkey));
+    CHECK(memcmp(&pubkey, zeros, sizeof(secp256k1_pubkey)) == 0);
+    CHECK(ecount == 4);
+    /* Passes through infinity but comes out one. */
+    pubkeys[2] = &pubkey_one;
+    memset(&pubkey, 255, sizeof(secp256k1_pubkey));
+    VG_UNDEF(&pubkey, sizeof(secp256k1_pubkey));
+    CHECK(secp256k1_ec_pubkey_combine(ctx, &pubkey, pubkeys, 3) == 1);
+    VG_CHECK(&pubkey, sizeof(secp256k1_pubkey));
+    CHECK(memcmp(&pubkey, zeros, sizeof(secp256k1_pubkey)) > 0);
+    CHECK(ecount == 4);
+    len = 33;
+    CHECK(secp256k1_ec_pubkey_serialize(ctx, ctmp, &len, &pubkey, SECP256K1_EC_COMPRESSED) == 1);
+    CHECK(secp256k1_ec_pubkey_serialize(ctx, ctmp2, &len, &pubkey_one, SECP256K1_EC_COMPRESSED) == 1);
+    CHECK(memcmp(ctmp, ctmp2, 33) == 0);
+    /* Adds to two. */
+    pubkeys[1] = &pubkey_one;
+    memset(&pubkey, 255, sizeof(secp256k1_pubkey));
+    VG_UNDEF(&pubkey, sizeof(secp256k1_pubkey));
+    CHECK(secp256k1_ec_pubkey_combine(ctx, &pubkey, pubkeys, 2) == 1);
+    VG_CHECK(&pubkey, sizeof(secp256k1_pubkey));
+    CHECK(memcmp(&pubkey, zeros, sizeof(secp256k1_pubkey)) > 0);
+    CHECK(ecount == 4);
     secp256k1_context_set_illegal_callback(ctx, NULL, NULL);
 }
 
@@ -3933,6 +4006,9 @@ void test_ecdsa_edge_cases(void) {
         CHECK(ecount == 5);
         CHECK(secp256k1_ecdsa_signature_parse_compact(ctx, &sig, signature) == 1);
         CHECK(ecount == 5);
+        memset(signature, 255, 64);
+        CHECK(secp256k1_ecdsa_signature_parse_compact(ctx, &sig, signature) == 0);
+        CHECK(ecount == 5);
         secp256k1_context_set_illegal_callback(ctx, NULL, NULL);
     }