Enable non-experimental modules by default #993

pull real-or-random wants to merge 1 commits into bitcoin-core:master from real-or-random:202110-default-modules changing 2 files +9 −6
  1. real-or-random commented at 12:09 pm on October 19, 2021: contributor

    This has been discussed in #817 (comment) and I agree with the arguments brought up there.

    Alternatively, we could not enable them and add a discussion to the readme why we discourage people from using the modules. I believe enabling ECDH is not very controversial. But what about recovery? Do we want to leave it off and instead give a reason?

  2. real-or-random force-pushed on Oct 19, 2021
  3. gmaxwell commented at 6:04 pm on October 19, 2021: contributor
    Convince bitcoin to drop the use of recovery, then it can just be removed.
  4. sipa commented at 6:36 pm on October 19, 2021: contributor

    @gmaxwell I think it’s used by a number of things, including BOLT11 in Lightning (where it helps avoiding encoding the payee ID), so I don’t think that’s a good idea, even if the P2PKH message signing feature was removed.

    As far as I know, the concern with it is that it’s an ad-hoc design we personally probably wouldn’t use again in the context it was first used (message signing with addresses).

    Any other reasons why we’d want to “discourage” the use?

  5. t-bast commented at 8:36 am on October 21, 2021: none

    I think it’s used by a number of things, including BOLT11 in Lightning (where it helps avoiding encoding the payee ID)

    That’s right, we do depend on pubkey recovery for lightning:

    • to avoid wasting 33 bytes in Bolt 11 invoices because they’re already bigger than we’d like
    • in our message signing scheme (that one could arguably include the public key directly, it’s not that useful to do pubkey recovery for it)

    What would be the rationale to remove it?

  6. real-or-random commented at 4:15 pm on July 29, 2022: contributor

    I propose doing this before the release. A release would hopefully trigger updates in a few packages and distributions (which currently use dates or commit id as version numbers), and if we believe that standard distributions should enable non-experimental modules (as discussed in the linked issue), then now is a good time.

    Note: This PR needs an update, now that more (i.e., all) modules have been made non-experimental.

  7. jonasnick commented at 5:45 pm on July 31, 2022: contributor

    I propose doing this before the release.

    Concept ACK. Do you also want to add an entry to the ChangeLog, similar to #1126?

  8. jonasnick added this to the milestone initial release (0.x) on Jul 31, 2022
  9. build: Enable some modules by default
    We don't enable the ECDSA recovery module, because we don't recommend
    ECDSA recovery for new protocols. In particular, the recovery API is
    prone to misuse: It invites the caller to forget to check the public
    key (and the verification function always returns 1).
    
    In general, we also don't recommend ordinary ECDSA for new protocols.
    But disabling the ECDSA functions is not possible because they're not
    in a module, and let's be honest: disabling ECDSA would mean to ignore
    reality blatantly.
    41e8704b48
  10. real-or-random force-pushed on Aug 3, 2022
  11. real-or-random commented at 3:12 pm on August 3, 2022: contributor
    Updated. This now doesn’t enable the ECDSA recovery module if this is controversial. Rationale is in the commit message.
  12. jonasnick commented at 1:13 pm on August 8, 2022: contributor
    ACK 41e8704b484652cf5bbb2b7ecc27feedc3cf0ae1
  13. sipa commented at 9:55 pm on November 21, 2022: contributor
    ACK 41e8704b484652cf5bbb2b7ecc27feedc3cf0ae1
  14. real-or-random merged this on Nov 22, 2022
  15. real-or-random closed this on Nov 22, 2022

  16. hebasto cross-referenced this on Nov 30, 2022 from issue build: Add CMake-based build system by hebasto
  17. sipa referenced this in commit 9d47e7b71b on Dec 13, 2022
  18. dhruv referenced this in commit 55ffd47cc6 on Dec 14, 2022
  19. dhruv referenced this in commit 967c65b158 on Dec 14, 2022
  20. dhruv referenced this in commit 78b5ddf28b on Jan 11, 2023
  21. dhruv referenced this in commit 215394a1d5 on Jan 11, 2023
  22. div72 referenced this in commit 945b094575 on Mar 14, 2023
  23. str4d referenced this in commit 0df7b459f6 on Apr 21, 2023
  24. vmta referenced this in commit e1120c94a1 on Jun 4, 2023
  25. vmta referenced this in commit 8f03457eed on Jul 1, 2023
  26. jonasnick cross-referenced this on Jul 18, 2023 from issue Upstream PRs 993, 1152, 1165, 1126, 1168, 1173, 1055 by jonasnick


real-or-random gmaxwell sipa t-bast jonasnick

Milestone
initial release (0.x)


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-21 18:15 UTC

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