make code consistent with Schnorrsig & more tests

BlockstreamResearch · Nov 13, 2023 · 799444b · 799444b
1 parent c46def1
commit 799444b
Show file tree

Hide file tree

Showing 2 changed files with 55 additions and 15 deletions.
diff --git a/src/modules/schnorrsig_halfagg/main_impl.h b/src/modules/schnorrsig_halfagg/main_impl.h
@@ -158,9 +158,10 @@ int secp256k1_schnorrsig_aggverify(const secp256k1_context *ctx, const secp256k1
 
         /* Step 0: Serialize pk_i into pk_ser   */
         /* We need that in Step 1 and in Step 2 */
-        if (!secp256k1_xonly_pubkey_serialize(ctx, pk_ser, &pubkeys[i])) {
+        if (!secp256k1_xonly_pubkey_load(ctx, &pp, &pubkeys[i])) {
             return 0;
         }
+        secp256k1_fe_get_b32(pk_ser, &pp.x);
 
         /* Step 1: z_i = TaggedHash(...) */
         /* 1.a) Write into hash r_i, pk_i, m_i, r_i */
@@ -181,14 +182,11 @@ int secp256k1_schnorrsig_aggverify(const secp256k1_context *ctx, const secp256k1
         if (!secp256k1_ge_set_xo_var(&rp, &rx, 0)) {
             return 0;
         }
-        /* 2.b) P_i = lift_x(int(pk_i)); fail if that fails */
-        if (!secp256k1_xonly_pubkey_load(ctx, &pp, &pubkeys[i])) {
-            return 0;
-        }
-        /* 2.c) e_i = int(hash_{BIP0340/challenge}(bytes(r_i) || pk_i || m_i)) mod n */
+
+        /* 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.d) acc = R_i + e_i⋅P_i */
+        /* 2.c) acc = R_i + e_i⋅P_i */
         secp256k1_ecmult(&acc, &ppj, &ei, NULL);
         secp256k1_gej_add_ge_var(&acc, &acc, &rp, NULL);
 

diff --git a/src/modules/schnorrsig_halfagg/tests_impl.h b/src/modules/schnorrsig_halfagg/tests_impl.h
@@ -320,16 +320,58 @@ In this test, we make sure that the algorithms properly reject
 for overflowing and non parseable values.
 */
 static void test_schnorrsig_aggregate_overflow(void) {
-    /* In general: algorithms can reject if         */
-    /* (1) some pk can not be serialized            */
-    /* (2) the s in signatures overflows            */
-    /* (3) R can not be decoded                     */
-    /* TODO */
     /* Test 1: We check that aggregation            */
-    /* returns 0 if inputs violate (1) or (2)       */
-    /* TODO */
+    /* returns 0 if s overflows                     */
+    {
+        int i;
+        secp256k1_xonly_pubkey pubkeys[N];
+        unsigned char msgs32[N*32];
+        unsigned char sigs64[N*64];
+        unsigned char aggsig[32*(N + 1)];
+        size_t aggsig_size = 32*(N + 1);
+
+        /* create N many Schnorr keys and sigs for random messages */
+        for (i = 0; i < N; ++i) {
+            unsigned char sk[32];
+            secp256k1_keypair keypair;
+            secp256k1_testrand256(sk);
+            secp256k1_testrand256(&msgs32[i*32]);
+            CHECK(secp256k1_keypair_create(CTX, &keypair, sk) == 1);
+            CHECK(secp256k1_keypair_xonly_pub(CTX, &pubkeys[i], NULL, &keypair));
+            CHECK(secp256k1_schnorrsig_sign(CTX, &sigs64[i*64], &msgs32[i*32], &keypair, NULL));
+        }
+        /* make one s (say the first one) overflow */
+        memset(&sigs64[32], 0xFF, 32);
+        /* check that aggregating fails */
+        CHECK(secp256k1_schnorrsig_aggregate(CTX, aggsig, &aggsig_size, pubkeys, msgs32, sigs64, N) == 0);
+    }
     /* Test 2: We check that verification returns 0 */
-    /* if inputs violate (1), (2), or (3)           */
+    /* if s overflows                               */
+    {
+        int i;
+        secp256k1_xonly_pubkey pubkeys[N];
+        unsigned char msgs32[N*32];
+        unsigned char sigs64[N*64];
+        unsigned char aggsig[32*(N + 1)];
+        size_t aggsig_size = 32*(N + 1);
+
+        /* create N many Schnorr keys and sigs for random messages */
+        for (i = 0; i < N; ++i) {
+            unsigned char sk[32];
+            secp256k1_keypair keypair;
+            secp256k1_testrand256(sk);
+            secp256k1_testrand256(&msgs32[i*32]);
+            CHECK(secp256k1_keypair_create(CTX, &keypair, sk) == 1);
+            CHECK(secp256k1_keypair_xonly_pub(CTX, &pubkeys[i], NULL, &keypair));
+            CHECK(secp256k1_schnorrsig_sign(CTX, &sigs64[i*64], &msgs32[i*32], &keypair, NULL));
+        }
+        /* aggregate */
+        CHECK(secp256k1_schnorrsig_aggregate(CTX, aggsig, &aggsig_size, pubkeys, msgs32, sigs64, N));
+        /* make s in the aggsig overflow */
+        memset(&aggsig[N*32], 0xFF, 32);
+        /* should not verify */
+        CHECK(secp256k1_schnorrsig_aggverify(CTX, pubkeys, msgs32, N, aggsig, aggsig_size) == 0);
+    }
 }
 
 static void run_schnorrsig_halfagg_tests(void) {