Add _ge_set_all_gej and use it in musig for own public nonces #1614

1. 
   real-or-random commented at 12:03 pm on October 8, 2024: contributor

   As suggested in #1479 (review)
2. real-or-random added the label performance on Oct 8, 2024
3. in src/group.h:83 in 2089a73a2c outdated

   79@@ -80,7 +80,10 @@ static void secp256k1_ge_set_gej(secp256k1_ge *r, secp256k1_gej *a);
   80 /** Set a group element equal to another which is given in jacobian coordinates. */
   81 static void secp256k1_ge_set_gej_var(secp256k1_ge *r, secp256k1_gej *a);
   82 
   83-/** Set a batch of group elements equal to the inputs given in jacobian coordinates */
   84+/** Set a batch of group elements equal to the inputs given in jacobian coordinates (affine). Constant time. */
   

   ---

   ---

   

   sipa commented at 12:08 pm on October 8, 2024:

   Explicitly say that the inputs are not allowed to be infinity?

   ---

   

   real-or-random commented at 2:44 pm on October 8, 2024:

   rephrased the entire docstring
4. 
   sipa commented at 12:47 pm on October 8, 2024: contributor

   utACK b4d602806bbd3830e0c56421daaafe647e2ab000
5. real-or-random force-pushed on Oct 8, 2024
6. real-or-random force-pushed on Oct 8, 2024
7. in src/group.h:88 in e49d8c19a8 outdated

   84+/** Set group elements r[0:len] (affine) equal to group elements a[0:len] (jacobian).
   85+ * None of the group elements in a[0:len] may be infinity. Constant time. */
   86+static void secp256k1_ge_set_all_gej(secp256k1_ge *r, const secp256k1_gej *a, size_t len);
   87+
   88+/** Set group elements r[0:len] (affine) equal to group elements a[0:len] (jacobian).
   89+ * None of the group elements in a[0:len] may be infinity. */
   

   ---

   ---

   

   theStack commented at 12:39 pm on October 9, 2024:

   Shouldn’t this description differ from the constant-time variant, as infinity group elements are allowed here? (e.g. “can be infinity”)

   ---

   

   real-or-random commented at 11:44 am on October 10, 2024:

   Oh, copy and paste… Should be fixed, I simply dropped the sentence.
8. real-or-random force-pushed on Oct 10, 2024
9. in src/modules/musig/session_impl.h:454 in f24e3e669e outdated

   453+        secp256k1_ecmult_gen(&ctx->ecmult_gen_ctx, &nonce_ptj[i], &k[i]);
   454         secp256k1_scalar_clear(&k[i]);
   455     }
   456+    secp256k1_ge_set_all_gej(nonce_pts, nonce_ptj, 2);
   457+    for (i = 0; i < 2; i++) {
   458+        secp256k1_declassify(ctx, &nonce_pts[i], sizeof(nonce_pts));
   

   ---

   ---

   

   theStack commented at 11:25 pm on October 10, 2024:

   nit: not directly related to this PR, as the _declassify call is only moved but still called with the same values, but the size passed seems too large, if I’m not missing anything (should be sizeof(nonce_pts[i]), rather than the full array size, otherwise we mark beyond the array on the second iteration, and the second half of the array twice).
10. theStack approved
11. 
    theStack commented at 11:29 pm on October 10, 2024: contributor

    ACK f24e3e669e54cc3e80e35c8f7e1e0a8152dbf47e
12. real-or-random referenced this in commit 68b55209f1 on Oct 22, 2024
13. group: Add constant-time secp256k1_ge_set_all_gej

    This is a dump mechanical translation of secp256k1_ge_set_all_gej_var
    that assumes that inputs are not infinity.

    d3082ddead
14. group: Simplify secp256k1_ge_set_all_gej

    No semantic changes.

    365f274ce3
15. tests: Improve _ge_set_all_gej(_var) tests 300aab1c05
16. musig: Use _ge_set_all_gej for own public nonces 64228a648f
17. real-or-random force-pushed on Nov 1, 2024
18. 
    real-or-random commented at 10:36 pm on November 1, 2024: contributor

    rebased, ready for review again
19. theStack approved
20. 
    theStack commented at 1:15 pm on November 3, 2024: contributor

    re-ACK 64228a648fa137723e73c6e019378f58add18a1a

Contributors
real-or-random sipa theStack

Labels
performance