musig: make keyagg_cache.tweak identical to t in spec

src/modules/musig/keyagg.h

@@ -18,6 +18,7 @@ typedef struct {
     secp256k1_ge pk;
     secp256k1_fe second_pk_x;
     unsigned char pk_hash[32];
+    /* tweak is identical to value t in the in the specification. */
     secp256k1_scalar tweak;
     /* parity_factor is identical to g in the specification and therefore either
      * 1 or -1. */

src/modules/musig/keyagg_impl.h

@@ -292,9 +292,37 @@ static int secp256k1_musig_pubkey_tweak_add_internal(const secp256k1_context* ct
         return 0;
     }
     internal_parity = secp256k1_fe_is_odd(&cache_i.pk.y);
-    /* Negate internal key as per the definition of x-only tweaking. */
-    if (xonly && secp256k1_extrakeys_ge_even_y(&cache_i.pk)) {
-        secp256k1_scalar_negate(&cache_i.tweak, &cache_i.tweak);
+    if (internal_parity) {
+        if (xonly) {
+            /* Negate internal key as per the definition of x-only tweaking. */
+            secp256k1_fe_negate(&cache_i.pk.y, &cache_i.pk.y, 1);
+        } else {
+            /* To explain how cache_i.tweak is updated, let us refer to the
+             * number of tweaks that have happened before this call by v. The
+             * variable cache_i.tweak is equal to value t of the specification
+             * when calling KeyAgg with v tweaks. We want to set cache_i.tweak
+             * to a new value t' that is equal to the t value of KeyAgg with v+1
+             * tweaks. It holds that
+             *   t  = g[1]*...*g[v]*t[1] + ... + g[v]*t[v].
+             *   t' = g'[1]*...*g'[v]*g'[v+1]*t[1] + ... + g'[v]*g'[v+1]*t[v] + g'[v+1]*t[v+1]
+             *      = g'[v+1]*(t'' + t[v+1]).
+             * where
+             *   t'' = g'[1]*...*g'[v]*t[1] + ... + g'[v]*t[v]
+             * This code block sets cache_i.tweak to t''.
+             *
+             * According to the specification, we have
+             *     g'[i] = g[i] for 0 <= i <= v-1
+             * and therefore
+             *   t'' = g'[v](g'[1]*...*g'[v-1]*t[1] + ... + *t[v])
+             *       = g'[v]/g[v]*t.
+             * Moreover, the specification says that
+             *     g[v] = par_to_fac((Q[v].y & 1))
+             *     g'[v] = par_to_fac(xonly && (Q[v].y & 1))
+             * which implies t'' = -t if not xonly and variable internal_parity
+             * (= Q[v].y & 1) is 1. Otherwise, t'' = t.
+             */
+            secp256k1_scalar_negate(&cache_i.tweak, &cache_i.tweak);
+        }
     }
     secp256k1_scalar_add(&cache_i.tweak, &cache_i.tweak, &tweak);
     if (!secp256k1_eckey_pubkey_tweak_add(&cache_i.pk, &tweak)) {
@@ -326,6 +354,12 @@ static int secp256k1_musig_pubkey_tweak_add_internal(const secp256k1_context* ct
      */
     output_parfac = par_to_fac(secp256k1_fe_is_odd(&cache_i.pk.y));
     cache_i.parity_factor *= par_to_fac(!xonly && internal_parity) * output_parfac;
+    /* At this point, cache_i.tweak is equal to t'' + t[v+1] as defined above.
+     * Therefore, in order to set it to t', we multiply by g'[v+1].
+     */
+    if (output_parfac < 0) {
+        secp256k1_scalar_negate(&cache_i.tweak, &cache_i.tweak);
+    }
     secp256k1_keyagg_cache_save(keyagg_cache, &cache_i);
     if (output_pubkey != NULL) {
         secp256k1_pubkey_save(output_pubkey, &cache_i.pk);

src/modules/musig/session_impl.h

@@ -461,9 +461,6 @@ int secp256k1_musig_nonce_process(const secp256k1_context* ctx, secp256k1_musig_
     if (!secp256k1_scalar_is_zero(&cache_i.tweak)) {
         secp256k1_scalar e_tmp;
         secp256k1_scalar_mul(&e_tmp, &session_i.challenge, &cache_i.tweak);
-        if (secp256k1_fe_is_odd(&cache_i.pk.y)) {
-            secp256k1_scalar_negate(&e_tmp, &e_tmp);
-        }
         secp256k1_scalar_add(&session_i.s_part, &session_i.s_part, &e_tmp);
     }
     memcpy(session_i.fin_nonce, fin_nonce, sizeof(session_i.fin_nonce));