ecmult: move _ecmult_odd_multiples_table_globalz_windowa #1053

pull siv2r wants to merge 1 commits into bitcoin-core:master from siv2r:move-globalz_windowa-func changing 2 files +16 −19
  1. siv2r commented at 6:07 pm on December 23, 2021: contributor

    Fixes #1035

    Changes: - move secp256k1_ecmult_odd_multiples_table_globalz_windowa function from ecmult to ecmult_const - remove outdated comment

  2. real-or-random commented at 6:36 pm on December 23, 2021: contributor
    Thanks! Can you rebase though? :P
  3. siv2r force-pushed on Dec 23, 2021
  4. siv2r commented at 6:52 pm on December 23, 2021: contributor

    Can you rebase though?

    Done :)

  5. real-or-random approved
  6. real-or-random commented at 7:02 pm on December 23, 2021: contributor
    utACK 6287fa8c43736a9e187862a1ad0f7d159037e3fd
  7. robot-dreams commented at 7:34 pm on December 23, 2021: contributor

    Concept ACK, thanks for addressing this!

    Though should this wait for #899 to merge first to avoid a merge conflict in that one (which is a bit longer and already has two-ish ACKs)?

  8. real-or-random commented at 7:36 pm on December 23, 2021: contributor

    Though should this wait for #899 to merge first to avoid a merge conflict in that one (which is a bit longer and already has two-ish ACKs)?

    Oh, thanks for pointing this out.

  9. in src/ecmult_const_impl.h:18 in 6287fa8c43 outdated
    11@@ -12,6 +12,22 @@
    12 #include "ecmult_const.h"
    13 #include "ecmult_impl.h"
    14 
    15+/** Fill a table 'pre' with precomputed odd multiples of a.
    16+ *
    17+ *  The resulting point set is brought to a single constant Z denominator, stores the X and Y
    18+ *  coordinates as ge_storage points in pre, and stores the global Z in rz.
    


    real-or-random commented at 8:53 pm on December 23, 2021:
    0 *  coordinates as ge_storage points in pre, and stores the global Z in globalz.
    

    Who would have expected that the global Z is stored in globalz?

  10. real-or-random commented at 8:59 pm on December 23, 2021: contributor

    roconnor-blockstream told me that he doesn’t mind rebasing #899 .

    By the way, it’s debatable if this function should be prefixed with ecmult or ecmult_const… But I don’t know, the enitre ecmult_const module is not really separated from the ecmult module (reuses WINDOW_A and the like). I believe this can be made cleaner but let’s maybe do that in a separate PR (see also #1039).

  11. robot-dreams commented at 10:00 pm on December 23, 2021: contributor

    utACK 6287fa8c43736a9e187862a1ad0f7d159037e3fd (aside from @real-or-random ’s suggested comment fixup)

    I agree that:

    • secp256k1_ecmult_odd_multiples_table_globalz_windowa is only called from ecmult_const_impl.h and thus should be moved there
    • The function is no longer used for computation of a*P + b*G, thus the comment should be removed
  12. ecmult: move `_ecmult_odd_multiples_table_globalz_windowa`
    Changes:
        - move `secp256k1_ecmult_odd_multiples_table_globalz_windowa` function from ecmult to ecmult_const
        - remove outdated comment
    05e049b73c
  13. siv2r force-pushed on Dec 24, 2021
  14. siv2r commented at 10:56 am on December 24, 2021: contributor
    Included the suggestion of #1053 (review)
  15. real-or-random approved
  16. real-or-random commented at 12:12 pm on December 24, 2021: contributor
    utACK 05e049b73c69002f498c3c9c21555fd91f95ccac
  17. robot-dreams commented at 1:51 pm on December 24, 2021: contributor
    utACK 05e049b73c69002f498c3c9c21555fd91f95ccac (diff between removed and added lines is exactly as expected)
  18. real-or-random merged this on Dec 24, 2021
  19. real-or-random closed this on Dec 24, 2021

  20. siv2r deleted the branch on Dec 25, 2021
  21. jonasnick cross-referenced this on Jan 2, 2022 from issue Sync Upstream by jonasnick
  22. real-or-random referenced this in commit 21e2d65b79 on Jan 5, 2022
  23. dhruv referenced this in commit 6f7e395acc on Jan 26, 2022
  24. hebasto referenced this in commit 065b6dbf9d on Jan 30, 2022
  25. dhruv referenced this in commit 139d4b881e on Feb 1, 2022
  26. fanquake referenced this in commit 8f8631d826 on Mar 17, 2022
  27. fanquake referenced this in commit 4bb1d7e62a on Mar 17, 2022
  28. fanquake referenced this in commit 465d05253a on Mar 30, 2022
  29. stratospher referenced this in commit 4d5afc9767 on Apr 3, 2022
  30. stratospher referenced this in commit 5b18493cfa on Apr 3, 2022
  31. fanquake referenced this in commit afb7a6fe06 on Apr 6, 2022
  32. gwillen referenced this in commit 35d6112a72 on May 25, 2022
  33. patricklodder referenced this in commit 21badcf9d2 on Jul 25, 2022
  34. patricklodder referenced this in commit 03002a9013 on Jul 28, 2022
  35. janus referenced this in commit 3a0652a777 on Aug 4, 2022
  36. str4d referenced this in commit 522190d5c3 on Apr 21, 2023
  37. vmta referenced this in commit e1120c94a1 on Jun 4, 2023
  38. vmta referenced this in commit 8f03457eed on Jul 1, 2023

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: 2024-11-22 10:15 UTC

This site is hosted by @0xB10C
More mirrored repositories can be found on mirror.b10c.me