f can never equal -m #1603

pull roconnor-blockstream wants to merge 1 commits into bitcoin-core:master from roconnor-blockstream:f-is-not-neg-modulus_2024-09 changing 2 files +8 −12
  1. roconnor-blockstream commented at 3:25 PM on September 6, 2024: contributor

    In fact, before reaching this particular VERIFY_CHECK, we had already successfully passed through

    VERIFY_CHECK(secp256k1_modinv64_mul_cmp_62(&f, len, &modinfo->modulus, -1) > 0); /* f > -modulus */

    ensuring that f is not -m.

  2. roconnor-blockstream force-pushed on Sep 6, 2024
  3. in src/modinv32_impl.h:646 in e53ed2efeb outdated 
     642 | @@ -643,13 +643,12 @@ static void secp256k1_modinv32_var(secp256k1_modinv32_signed30 *x, const secp256
 643 |  
 644 |      /* g == 0 */
 645 |      VERIFY_CHECK(secp256k1_modinv32_mul_cmp_30(&g, len, &SECP256K1_SIGNED30_ONE, 0) == 0);
 646 | -    /* |f| == 1, or (x == 0 and d == 0 and |f|=modulus) */
 647 | +    /* |f| == 1, or (x == 0 and d == 0 and f=modulus) */
    real-or-random commented at 1:33 PM on September 9, 2024:

    micro-nit:

        /* |f| == 1, or (x == 0 and d == 0 and f == modulus) */
  4. real-or-random approved
  5. real-or-random commented at 1:32 PM on September 25, 2024: contributor

    Okay yes. All VERIFY_CHECKs are redundant in a sense, but this one is really covered by another VERIFY_CHECK already.

    utACK e53ed2efebcca6483a511ac0d9f3162973d66dda

  6. real-or-random added the label refactor/smell on Sep 25, 2024
  7. roconnor-blockstream commented at 2:00 PM on September 25, 2024: contributor

    Just to be clear, this PR isn't removing a redundant check, it is strengthening the existing check by removing a disjunctive clause.

  8. real-or-random commented at 2:46 PM on September 25, 2024: contributor

    Just to be clear, this PR isn't removing a redundant check, it is strengthening the existing check by removing a disjunctive clause.

    Oh, right, I got this wrong. But I still think the change makes sense.

  9. f can never equal -m
    In fact, before reaching this particular VERIFY_CHECK, we had already successfully passed through

    VERIFY_CHECK(secp256k1_modinv64_mul_cmp_62(&f, len, &modinfo->modulus, -1) > 0); /* f > -modulus */

ensuring that f is not -m.
    ef7ff03407
  10. roconnor-blockstream force-pushed on Sep 25, 2024
  11. roconnor-blockstream commented at 3:06 PM on September 25, 2024: contributor

    Done. I've also added the same change to the constant-time versions of these functions.

  12. real-or-random approved
  13. real-or-random commented at 3:16 PM on September 25, 2024: contributor

    utACK ef7ff03407fa79e7eab1b6771b77f72828e63636

    (There seems to be some CI hiccup unrelated to this PR.)

  14. real-or-random added the label assurance on Sep 26, 2024
  15. sipa commented at 3:18 PM on October 7, 2024: contributor

    ACK ef7ff03407fa79e7eab1b6771b77f72828e63636

  16. real-or-random merged this on Oct 8, 2024
  17. real-or-random closed this on Oct 8, 2024
  18. achow101 referenced this in commit 5a0d3f1db5 on Oct 12, 2024
  19. hebasto referenced this in commit 5c425b5703 on Oct 16, 2024
  20. achow101 referenced this in commit c24b343141 on Oct 24, 2024
  21. vmta referenced this in commit 4d1f6d5635 on Oct 29, 2024
  22. achow101 referenced this in commit 378ca17fd1 on Nov 1, 2024
  23. achow101 referenced this in commit 2d46a89386 on Nov 4, 2024
  24. Eunovo referenced this in commit 55a2f7a840 on Nov 12, 2024
  25. vmta referenced this in commit f40affbf6c on Nov 21, 2024
  26. vmta referenced this in commit cc8d145633 on Nov 22, 2024
  27. janus referenced this in commit a4b4239cb4 on Jan 19, 2025
  28. div72 referenced this in commit af627d47c3 on Apr 12, 2025
  29. real-or-random referenced this in commit 8aa05cb351 on Feb 17, 2026
  30. github-actions[bot] referenced this in commit 758d4e90b4 on Mar 1, 2026
  31. github-actions[bot] referenced this in commit 68a2178f22 on Mar 1, 2026
  32. github-actions[bot] referenced this in commit a8bc1a0b2b on Mar 1, 2026
  33. 0x000000000019d6689c085ae165831e934ff76 referenced this in commit 3b9450150d on Mar 2, 2026
  34. csjones referenced this in commit a4d92824ae on Mar 2, 2026
Contributors
roconnor-blockstreamreal-or-randomsipa
Labels
assurancetweak/refactor
github-metadata-mirror

This is a metadata mirror of the GitHub repository bitcoin-core/secp256k1. This site is not affiliated with GitHub. Content is generated from a GitHub metadata backup.
generated: 2026-05-19 06:52 UTC