Remove `SECP256K1_WARN_UNUSED_RESULT` from `secp256k1_keypair_*` functions #1379

issue w0xlt opened this issue on July 17, 2023
  1. w0xlt commented at 3:01 PM on July 17, 2023: none

    Why do secp256k1_keypair_* functions have SECP256K1_WARN_UNUSED_RESULT if they always return 1 ? The only exception is secp256k1_keypair_create.

  2. real-or-random commented at 3:32 PM on July 17, 2023: contributor

    Why do secp256k1_keypair_* functions have SECP256K1_WARN_UNUSED_RESULT if they always return 1 ?

    Because they are not guaranteed to always return 1 in future (but backwards-compatible) API versions, I suppose.

  3. sipa commented at 3:39 PM on July 17, 2023: contributor

    I don't think that's a concern. If a backward-compatible API change causes 0 to become a permitted return value, it shouldn't occur for code written against the old header file (e.g. it'd require API calls, or constants, or magics, ... that don't exist in the old header). Thus, to any user of the old header file, no warning should be needed, and the WARN_UNUSED_RESULT can just be added to the header whenever such an API change is introduced.

    TL;DR: I think we should drop SECP256K1_WARN_UNUSED_RESULT for functions that are defined to only return 1.

  4. real-or-random added the label documentation on Jul 17, 2023
  5. real-or-random added the label needs-changelog on Jul 17, 2023
  6. real-or-random added the label refactor/smell on Jul 17, 2023
  7. real-or-random commented at 8:07 AM on July 18, 2023: contributor

    These functions can actually return 0, but this requires a violation of the API: https://github.com/bitcoin-core/secp256k1/blob/b40e2d30b7a6f71e7d82340c1cf82150c5cf0540/src/modules/extrakeys/main_impl.h#L44-L54

    Wouldn't it make more sense to call the illegal callback in that case?

  8. jonasnick commented at 9:29 AM on March 13, 2025: contributor

    @real-or-random secp256k1_xonly_pubkey_load calls the illegal callback if it fails.

  9. real-or-random closed this on Mar 13, 2025

  10. pull[bot] referenced this in commit 3f54ed8c1b on Mar 13, 2025
  11. fanquake removed the label needs-changelog on Jul 29, 2025
  12. fanquake commented at 10:30 AM on July 29, 2025: member

    Changelog was added in #1702. Dropping "needs-changelog".


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

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