Remove SECP256K1_WARN_UNUSED_RESULT from secp256k1_keypair_* functions #1379

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?

Contributors
w0xlt real-or-random sipa

Labels
documentation needs-changelog refactor/smell