f add ARG_CHECK to ensure s2c_opening and s2c_data32 are either both non-NULL or both NULL.

Author: jonasnick

include/secp256k1_schnorrsig.h

@@ -61,7 +61,7 @@ SECP256K1_API int secp256k1_schnorrsig_parse(
  * Returns 1 on success, 0 on failure.
  *  Args:    ctx: pointer to a context object, initialized for signing (cannot be NULL)
  *  Out:     sig: pointer to the returned signature (cannot be NULL)
- *      s2c_data: pointer to an secp256k1_s2c_opening structure which can be
+ *   s2c_opening: pointer to an secp256k1_s2c_opening structure which can be
  *                NULL but is required to be not NULL if this signature creates
  *                a sign-to-contract commitment (i.e. the `s2c_data` argument
  *                is not NULL).

src/modules/schnorrsig/main_impl.h

@@ -76,6 +76,8 @@ int secp256k1_schnorrsig_sign(const secp256k1_context* ctx, secp256k1_schnorrsig
      * because we need to ensure that s2c_data is actually hashed into the nonce and
      * not just ignored. */
     ARG_CHECK(s2c_data32 == NULL || (noncefp == NULL || noncefp == secp256k1_nonce_function_bipschnorr));
+    /* s2c_opening and s2c_data32 should be either both non-NULL or both NULL. */
+    ARG_CHECK((s2c_opening != NULL) == (s2c_data32 != NULL));
 
     if (noncefp == NULL) {
         noncefp = secp256k1_nonce_function_bipschnorr;

src/modules/schnorrsig/tests_impl.h

@@ -72,17 +72,21 @@ void test_schnorrsig_api(secp256k1_scratch_space *scratch) {
     CHECK(ecount == 2);
     CHECK(secp256k1_schnorrsig_sign(sign, NULL, &s2c_opening, msg, sk1, s2c_data32, NULL, NULL) == 0);
     CHECK(ecount == 3);
-    CHECK(secp256k1_schnorrsig_sign(sign, &sig, NULL, msg, sk1, s2c_data32, NULL, NULL) == 1);
-    CHECK(ecount == 3);
-    CHECK(secp256k1_schnorrsig_sign(sign, &sig, &s2c_opening, NULL, sk1, s2c_data32, NULL, NULL) == 0);
+    CHECK(secp256k1_schnorrsig_sign(sign, &sig, NULL, msg, sk1, s2c_data32, NULL, NULL) == 0);
     CHECK(ecount == 4);
-    CHECK(secp256k1_schnorrsig_sign(sign, &sig, &s2c_opening, msg, NULL, s2c_data32, NULL, NULL) == 0);
-    CHECK(ecount == 5);
-    CHECK(secp256k1_schnorrsig_sign(sign, &sig, &s2c_opening, msg, sk1, NULL, NULL, NULL) == 1);
+    CHECK(secp256k1_schnorrsig_sign(sign, &sig, &s2c_opening, NULL, sk1, s2c_data32, NULL, NULL) == 0);
     CHECK(ecount == 5);
+    CHECK(secp256k1_schnorrsig_sign(sign, &sig, &s2c_opening, msg, NULL, s2c_data32, NULL, NULL) == 0);
+    CHECK(ecount == 6);
+    CHECK(secp256k1_schnorrsig_sign(sign, &sig, &s2c_opening, msg, sk1, NULL, NULL, NULL) == 0);
+    CHECK(ecount == 7);
+    /* It's okay if both s2c_opening and s2c_data32 are NULL. It's just not okay if
+     * only a single one of them is NULL. */
+    CHECK(secp256k1_schnorrsig_sign(sign, &sig, NULL, msg, sk1, NULL, NULL, NULL) == 1);
+    CHECK(ecount == 7);
     /* s2c commitments with a different nonce function than bipschnorr are not allowed */
     CHECK(secp256k1_schnorrsig_sign(sign, &sig, &s2c_opening, msg, sk1, s2c_data32, secp256k1_nonce_function_rfc6979, NULL) == 0);
-    CHECK(ecount == 6);
+    CHECK(ecount == 8);
 
     ecount = 0;
     CHECK(secp256k1_schnorrsig_serialize(none, sig64, &sig) == 1);
@@ -170,14 +174,12 @@ void test_schnorrsig_api(secp256k1_scratch_space *scratch) {
 
 /* Helper function for schnorrsig_bip_vectors
  * Signs the message and checks that it's the same as expected_sig. */
-void test_schnorrsig_bip_vectors_check_signing(const unsigned char *sk, const unsigned char *pk_serialized, const unsigned char *msg, const unsigned char *expected_sig, const int expected_nonce_is_negated) {
+void test_schnorrsig_bip_vectors_check_signing(const unsigned char *sk, const unsigned char *pk_serialized, const unsigned char *msg, const unsigned char *expected_sig) {
     secp256k1_schnorrsig sig;
     unsigned char serialized_sig[64];
     secp256k1_pubkey pk;
-    secp256k1_s2c_opening s2c_opening;
 
-    CHECK(secp256k1_schnorrsig_sign(ctx, &sig, &s2c_opening, msg, sk, NULL, NULL, NULL));
-    CHECK(s2c_opening.nonce_is_negated == expected_nonce_is_negated);
+    CHECK(secp256k1_schnorrsig_sign(ctx, &sig, NULL, msg, sk, NULL, NULL, NULL));
     CHECK(secp256k1_schnorrsig_serialize(ctx, serialized_sig, &sig));
     CHECK(memcmp(serialized_sig, expected_sig, 64) == 0);
 
@@ -240,7 +242,7 @@ void test_schnorrsig_bip_vectors(secp256k1_scratch_space *scratch) {
             0x2C, 0xCD, 0x00, 0x79, 0xE1, 0xF9, 0x2A, 0xF1,
             0x77, 0xF7, 0xF2, 0x2C, 0xC1, 0xDC, 0xED, 0x05
         };
-        test_schnorrsig_bip_vectors_check_signing(sk1, pk1, msg1, sig1, 1);
+        test_schnorrsig_bip_vectors_check_signing(sk1, pk1, msg1, sig1);
         test_schnorrsig_bip_vectors_check_verify(scratch, pk1, msg1, sig1, 1);
     }
     {
@@ -274,7 +276,7 @@ void test_schnorrsig_bip_vectors(secp256k1_scratch_space *scratch) {
             0x5F, 0xFC, 0x2D, 0x03, 0x5A, 0x23, 0x04, 0x34,
             0xA1, 0xA6, 0x4D, 0xC5, 0x9F, 0x70, 0x13, 0xFD
         };
-        test_schnorrsig_bip_vectors_check_signing(sk2, pk2, msg2, sig2, 0);
+        test_schnorrsig_bip_vectors_check_signing(sk2, pk2, msg2, sig2);
         test_schnorrsig_bip_vectors_check_verify(scratch, pk2, msg2, sig2, 1);
     }
     {
@@ -308,7 +310,7 @@ void test_schnorrsig_bip_vectors(secp256k1_scratch_space *scratch) {
             0xA5, 0x83, 0x7E, 0xC5, 0x7F, 0xED, 0x76, 0x60,
             0x77, 0x3A, 0x05, 0xF0, 0xDE, 0x14, 0x23, 0x80
         };
-        test_schnorrsig_bip_vectors_check_signing(sk3, pk3, msg3, sig3, 0);
+        test_schnorrsig_bip_vectors_check_signing(sk3, pk3, msg3, sig3);
         test_schnorrsig_bip_vectors_check_verify(scratch, pk3, msg3, sig3, 1);
     }
     {