PROTOTYPE Split headers to make it possible to use only prealloc API #1166

pull real-or-random wants to merge 14 commits into bitcoin-core:master from real-or-random:202212-split-headers changing 18 files +908 −812
  1. real-or-random commented at 7:02 pm on December 1, 2022: contributor

    On top of #1126.

    This prototypes one possible approach to solve #1095 by implementing the approach described in #1095 (comment):

    […] reorganize headers such that most functions are in some header like secp256k1_main.h, which is then included from secp256k1.h (with only malloc functions like secp256k1_context_create()) and secp256k1_preallocated.h (with their prealloc counterparts). Then the user could easily choose between the two interfaces by including either secp256k1.h or secp256k1_preallocated.h

    Now that I’ve been working on this, I don’t like this approach for various reasons:

    • The functions and docs are really cluttered in multiple files.
      • In particular, the context stuff: Creation, cloning, destruction is in secp256k1_(preallocated).h, whereas the static context, flags, randomization, and callbacks are in secp256k1_main.h. This is hard to grasp for new users and also add confusion for users who don’t need the preallocated interface.
      • In general, there’s no canonical “primary” header anymore. Where do you put general docs that belong to the library as a whole (e.g., the comment that explains the rules of thumb for the order of arguments in API calls)?
    • We need to remove the #include "secp256k1.h" from the module headers such as secp256k1_ecdh.h (because otherwise you couldn’t include the module header if you want only the prealloc interface). This means that
      • this PR is not perfectly backwards-compatible: it will break compilation for users who include currently only secp256k1_ecdh.h and not secp256k1_ecdh.h.
      • the dependencies between the header files are not naturally expressed by the #includes
      • you need to include the header files in the right order, i.e., first secp256k1_(preallocated).h and then secp256k1_ecdh.h because the latter needs the declaration of secp256k1_context (and we couldn’t even repeat that declaration somewhere to workaround this problem: Compiler tells us warning: redefinition of typedef 'secp256k1_context' is a C11 feature…)

    So I think this PR would indeed make it possible to use only the prealloc interface, but it makes the life harder for everyone, in particular for normal users who have malloc and don’t even care about prealloc. I think we should switch to an approach with a macro like PREALLOC_INTERFACE_ONLY that you need to set before including secp256k1.h (and when compiling the library itself) and that will remove all the functions that depend on malloc. But please tell me if you disagree.

  2. docs: Never require a verification context ee7341fbac
  3. docs: Change signature "validation" to "verification" 1a553ee8be
  4. contexts: Make contexts returned by _context_create "DEFAULT" contexts cdc46defd7
  5. docs: Improve docs for static context 108ea12c95
  6. contexts: Rename static context dd5369edea
  7. tests: Use new name of static context 8ff22ff9ae
  8. selftest: Rename internal function to make name available for API 2c7c2f5101
  9. selftest: Expose in public API 00448c0aa3
  10. docs: Tidy and improve docs about contexts and randomization 2e9335a68c
  11. docs: Get rid of "initialized for signing" terminology 1fe4142c60
  12. docs: Use doxygen style if and only if comment is user-facing
    and improve phrasing slightly.
    507dcbafd4
  13. examples: Switch to DEFAULT contexts 92e7293d61
  14. WIP Split headers into normal and preallocated adde33199c
  15. DEMO DEMO DEMO Make ECDH example use prealloc API to demo purpose of the previous commit 4340f89a7d
  16. real-or-random force-pushed on Dec 2, 2022
  17. real-or-random cross-referenced this on Mar 26, 2023 from issue Don't #include standard library headers unconditionally by real-or-random
  18. jonasnick added this to the milestone 0.4.1 or 0.5.0 on Sep 20, 2023
  19. real-or-random closed this on Sep 20, 2023

  20. real-or-random removed this from the milestone 0.4.1 or 0.5.0 on Sep 20, 2023
  21. real-or-random commented at 4:10 pm on September 20, 2023: contributor
    Closing to the reasons explained above. It turns out that this is not the best approach to solve #1095 ,


real-or-random


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-10-30 01:15 UTC

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