[API BREAK] Introduce explicit contexts #208

pull sipa wants to merge 1 commits into bitcoin-core:master from sipa:context changing 14 files +342 −277
  1. sipa commented at 1:27 am on February 4, 2015: contributor
  2. DavidEGrayson commented at 2:05 am on February 4, 2015: none
    I looked at the changes to the main header file and they look good. I would be tempted to remove the flags argument from secp256k1_context_create and just let people use the other functions, since it makes the API surface a bit smaller.
  3. gmaxwell commented at 2:09 am on February 4, 2015: contributor
    Need to worry about some combinatorial blowup, e.g. with each mixture of starting options having a new entry point. (We may have more options in the future). Otherwise I’d be agreeing with that.
  4. sipa commented at 2:12 am on February 4, 2015: contributor
    @gmaxwell I’m not sure whether you’re in favor or against the flags argument…
  5. gmaxwell commented at 2:22 am on February 4, 2015: contributor
    For flags. Because if you have six flags or wahtever you might need 2^6 (or more) entriy points otherwise)
  6. DavidEGrayson commented at 2:37 am on February 4, 2015: none
    You just need one secp256k1_context_initialize_* entry point for each high-level feature, because multiple initialization functions can be called on the same context after it is created.
  7. gmaxwell commented at 2:42 am on February 4, 2015: contributor

    Hm. Doesn’t address the case where it’s cheaper to initilize a&&b than a, b or where b means you throw out what A does, without having a weird interface where some interact and some don’t and some are slower unless you combine them.

    Now I don’t know what I prefer. I know I don’t want separate contexts. Since state can potentially be shared (esp in low-mem builds).

  8. DavidEGrayson commented at 2:51 am on February 4, 2015: none
    Yeah, so that would be a good reason to keep the flags argument around.
  9. DavidEGrayson commented at 5:20 pm on February 4, 2015: none

    I think the API for creation/initialization could be simplified to just two functions:

    0secp256k1_context* secp256k1_context_create();
    1void secp256k1_context_initialize(secp256k1_context* ctx, int flags); // can call multiple times
    

    This allows special optimizations of the initialization process, regardless of when the initialization is performed, without combinatorial blowup of entry points. It makes the API simpler because there would be fewer functions and only one way to enable a feature. (Sorry if I’m missing something and this is just another lame idea for some reason!)

  10. sipa commented at 5:41 pm on February 4, 2015: contributor
    So there’s a follow-up plan which is probably not obvious here, namely that some initializations may gain parameters (specifically: the size of the tables), which you probably can’t encode as just flags. The explicit initialization functions can then serve as ‘advanced’ versions, while the flagged create_context does defaulty things.
  11. sipa cross-referenced this on Feb 13, 2015 from issue secp256k1 benchmarking programs can now accept parameters by midnightmagic
  12. sipa cross-referenced this on Feb 13, 2015 from issue Add scalar blinding for ecmult_gen() by gmaxwell
  13. sipa cross-referenced this on Feb 13, 2015 from issue enable libsecp256k1_jni shared lib by theuni
  14. sipa cross-referenced this on Feb 23, 2015 from issue Initial gcov work by cbatson
  15. DavidEGrayson cross-referenced this on Feb 25, 2015 from issue How to use the library? by ShaliDaliHaliDa
  16. sipa force-pushed on Mar 2, 2015
  17. sipa commented at 10:43 am on March 2, 2015: contributor
    Rebased.
  18. sipa force-pushed on Mar 2, 2015
  19. sipa commented at 10:48 am on March 2, 2015: contributor
    And rebased again.
  20. sipa force-pushed on Mar 16, 2015
  21. sipa commented at 10:19 pm on March 16, 2015: contributor
    And again. @gmaxwell I think we’ll see significant API changes still soon (especially if we’d incorporate #179, #190, and similar things). The bulk of this PR is introducing the contexts, and is a bit of a pain to maintain. How to actually build the contexts can be changed later, I think.
  22. sipa force-pushed on Mar 16, 2015
  23. sipa commented at 10:38 pm on March 16, 2015: contributor
    Removed the per-feature initialization functions for now.
  24. sipa force-pushed on Mar 16, 2015
  25. sipa force-pushed on Mar 29, 2015
  26. sipa commented at 11:14 pm on March 29, 2015: contributor
    Rebased.
  27. sipa commented at 1:19 am on April 3, 2015: contributor
    @gmaxwell Review plz? puppy eyes
  28. gmaxwell commented at 3:04 am on April 3, 2015: contributor
    Okay, I think I’ve run out of nits.
  29. sipa force-pushed on Apr 10, 2015
  30. sipa commented at 9:03 am on April 10, 2015: contributor
    Nits addressed.
  31. gmaxwell commented at 2:43 am on April 11, 2015: contributor
    Would be nice to brace that IF; either way: ACK.
  32. [API BREAK] Introduce explicit contexts a9b6595ef8
  33. sipa force-pushed on Apr 11, 2015
  34. sipa merged this on Apr 11, 2015
  35. sipa closed this on Apr 11, 2015

  36. sipa referenced this in commit 3608c7f2f6 on Apr 11, 2015
  37. DavidEGrayson cross-referenced this on Apr 12, 2015 from issue Expose ability to deep-copy a context by apoelstra

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-24 11:15 UTC

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