fix all four api issues (didn't look at pr comments)

Author: siv2r

src/modules/schnorr_adaptor/main_impl.h

@@ -9,7 +9,9 @@
 
 #include "../../../include/secp256k1.h"
 #include "../../../include/secp256k1_schnorr_adaptor.h"
+
 #include "../../hash.h"
+#include "../../scalar.h"
 
 /* Initializes SHA256 with fixed midstate. This midstate was computed by applying
  * SHA256 to SHA256("SchnorrAdaptor/nonce")||SHA256("SchnorrAdaptor/nonce"). */
@@ -145,11 +147,11 @@ static int secp256k1_schnorr_adaptor_presign_internal(const secp256k1_context *c
     /* R = k*G */
     secp256k1_ecmult_gen(&ctx->ecmult_gen_ctx, &rj, &k);
     secp256k1_ge_set_gej(&r, &rj);
+
     /* We declassify the non-secret values R and T to allow using them
      * as branch points. */
     secp256k1_declassify(ctx, &r, sizeof(rp));
     secp256k1_declassify(ctx, &adaptor_ge, sizeof(rp));
-
     /* R' = R + T */
     secp256k1_gej_add_ge_var(&rpj, &rj, &adaptor_ge, NULL);
     secp256k1_ge_set_gej(&rp, &rpj);
@@ -158,11 +160,19 @@ static int secp256k1_schnorr_adaptor_presign_internal(const secp256k1_context *c
     secp256k1_declassify(ctx, &rp, sizeof(rp));
     secp256k1_fe_normalize_var(&rp.y);
 
-    /* s =  k + e * d when R'.y is even
-     *   = -k + e * d when R'.y is odd
-     * TODO: explain the negation here?
-     * just say here we negate based on R' rather than R because R'
-     * will be in the final bip340 signature */
+    /* Determine if the secret nonce should be negated.
+    *
+    * pre_sig65[0:33] contains the compressed 33-byte encoding of the public
+    * nonce R' = k*G + T, where k is the secret nonce and T is the adaptor point.
+    *
+    * Since a BIP340 signature requires an x-only public nonce, in the case where
+    * R' = k*G + T has odd Y-coordinate, the x-only public nonce corresponding to
+    * the signature is actually -k*G - T. Therefore, we negate k to ensure that the
+    * adapted pre-signature will result in a valid BIP340 signature, with an even R'.y
+    *
+    *  pre_sig65[33:65] =  k + e * d if R'.y is even
+    *                   = -k + e * d if R'.y is odd
+    */
     if (secp256k1_fe_is_odd(&rp.y)) {
         secp256k1_scalar_negate(&k, &k);
     }
@@ -208,7 +218,7 @@ int secp256k1_schnorr_adaptor_extract(const secp256k1_context *ctx, secp256k1_pu
     if (!secp256k1_eckey_pubkey_parse(&rp, &pre_sig65[0], 33)) {
         return 0;
     }
-
+    /* s := pre_sig65[33:65] */
     secp256k1_scalar_set_b32(&s, &pre_sig65[33], &overflow);
     if (overflow) {
         return 0;
@@ -230,10 +240,15 @@ int secp256k1_schnorr_adaptor_extract(const secp256k1_context *ctx, secp256k1_pu
         return 0;
     }
 
-    /* T =  R' - R when R'.y is even
-     *   =  R' + R when R'.y is odd 
-     * TODO: explain the negation here?
-     * just say here presign negates like this */
+    /* Determine if R needs to be negated
+     *
+     * `adaptor_presign` negates the secret nonce k when R’.y is odd, during
+     * the computation of the s value (i.e., presig[33:65]). Therefore, we need
+     * to negate R = k*G (if R'.y is odd) before subtracting it from R'.
+     *
+     *  T =  R' - R if R'.y is even
+     *    =  R' + R if R'.y is odd
+     */
     if (secp256k1_fe_is_odd(&rp.y)) {
         secp256k1_gej_neg(&rj, &rj);
     }
@@ -247,13 +262,11 @@ int secp256k1_schnorr_adaptor_extract(const secp256k1_context *ctx, secp256k1_pu
     return 1;
 }
 
-/* TODO: musig2 consideres `t` (secadaptor) to be secret. And `s` (pre-signature) to be public*/
-/* should we consider bip340 signature to be secret? or public?
- * I think it should be secret since, bip340 sig is computed by adding a secret t to presig */
 int secp256k1_schnorr_adaptor_adapt(const secp256k1_context *ctx, unsigned char *sig64, const unsigned char *pre_sig65, const unsigned char *sec_adaptor32) {
     secp256k1_scalar s;
     secp256k1_scalar t;
     int overflow;
+    int ret = 1;
 
     VERIFY_CHECK(ctx != NULL);
     ARG_CHECK(sig64 != NULL);
@@ -266,59 +279,69 @@ int secp256k1_schnorr_adaptor_adapt(const secp256k1_context *ctx, unsigned char
         return 0;
     }
     secp256k1_scalar_set_b32(&t, sec_adaptor32, &overflow);
-    if (overflow) {
-        return 0;
-    }
-
-    /* TODO: add a comment about why we negate here*/
+    ret &= !overflow;
+
+    /* Determine if the secret adaptor should be negated.
+     *
+     * pre_sig65[0:33] contains the compressed 33-byte encoding of the public
+     * nonce R' = (k + t)*G, where r is the secret nonce generated by
+     * `adaptor_presign` and t is the secret adaptor.
+     *
+     * Since a BIP340 signature requires an x-only public nonce, in the case where
+     * (k + t)*G has odd Y-coordinate, the x-only public nonce corresponding to the
+     * signature is actually (-k - t)*G. Thus adapting a pre-signature requires
+     * negating t in this case.
+     *
+     * sig64[32:64]  =  s + t if R'.y is even
+     *               =  s - t if R'.y is odd
+     */
     if (pre_sig65[0] == SECP256K1_TAG_PUBKEY_ODD) {
         secp256k1_scalar_negate(&t, &t);
     }
 
     secp256k1_scalar_add(&s, &s, &t);
     secp256k1_scalar_get_b32(&sig64[32], &s);
     memmove(sig64, &pre_sig65[1], 32);
-    secp256k1_scalar_clear(&s);
-    secp256k1_scalar_clear(&t); /* remove this? */
-    return 1;
+    secp256k1_scalar_clear(&t);
+    return ret;
 }
 
-int secp256k1_schnorr_adaptor_extract_sec(const secp256k1_context *ctx, unsigned char *secadaptor, const unsigned char *pre_sig65, const unsigned char *sig64) {
-    secp256k1_scalar s0;
-    secp256k1_scalar s;
+int secp256k1_schnorr_adaptor_extract_sec(const secp256k1_context *ctx, unsigned char *sec_adaptor32, const unsigned char *pre_sig65, const unsigned char *sig64) {
     secp256k1_scalar t;
+    secp256k1_scalar s;
     int overflow;
+    int ret = 1;
 
     VERIFY_CHECK(ctx != NULL);
-    ARG_CHECK(secadaptor != NULL);
+    ARG_CHECK(sec_adaptor32 != NULL);
     ARG_CHECK(pre_sig65 != NULL);
     ARG_CHECK(pre_sig65[0] == SECP256K1_TAG_PUBKEY_EVEN || pre_sig65[0] == SECP256K1_TAG_PUBKEY_ODD);
     ARG_CHECK(sig64 != NULL);
 
-    /* s0 */
-    secp256k1_scalar_set_b32(&s0, &pre_sig65[33], &overflow);
-    if (overflow) {
-        return 0;
-    }
+    secp256k1_scalar_set_b32(&t, &sig64[32], &overflow);
+    ret &= !overflow;
 
-    /* s */
-    secp256k1_scalar_set_b32(&s, &sig64[32], &overflow);
+    secp256k1_scalar_set_b32(&s, &pre_sig65[33], &overflow);
     if (overflow) {
         return 0;
     }
 
-    if (pre_sig65[0] == SECP256K1_TAG_PUBKEY_EVEN) {
-        secp256k1_scalar_negate(&s0, &s0);
-    } else if (pre_sig65[0] == SECP256K1_TAG_PUBKEY_ODD) {
-        secp256k1_scalar_negate(&s, &s);
+    secp256k1_scalar_negate(&s, &s);
+    secp256k1_scalar_add(&t, &t, &s);
+    /* `adaptor_adapt` negates the secret adaptor t when R’.y is odd, during
+     * the computation of the BIP340 signature. Therefore, we need negate
+     * (sig[32:64] - pre_sig65[33:65]) in this case.
+     *
+     *  t =  (sig[32:64] - pre_sig65[33:65])  if R'.y is even
+     *    = -(sig[32:64] - pre_sig65[33:65])  if R'.y is odd
+     */
+    if (pre_sig65[0] == SECP256K1_TAG_PUBKEY_ODD) {
+        secp256k1_scalar_negate(&t, &t);
     }
-    secp256k1_scalar_add(&t, &s0, &s);
 
-    secp256k1_scalar_get_b32(secadaptor, &t);
-    secp256k1_scalar_clear(&s);
+    secp256k1_scalar_get_b32(sec_adaptor32, &t);
     secp256k1_scalar_clear(&t);
-
-    return 1;
+    return ret;
 }
 
 #endif