doc: clarify API doc of `secp256k1_ecdsa

theStack commented at 12:41 pm on September 9, 2025: contributor

This PR aims to fix #1718. I thought it’s nice to provide an usage example, but not sure if we want actual code snippets in the API header (it’s also incomplete, as the declarations are missing), so an alternative might be to drop it, or add a “(see example)” reference in the future in case #1714 gets in.

real-or-random added the label user-documentation on Sep 12, 2025

real-or-random added the label tweak/refactor on Sep 12, 2025

in include/secp256k1_recovery.h:101 in e9c10f7df8 outdated

 97+ *  in a mathematical sense, but it is only guaranteed to also pass verification
 98+ *  with `secp256k1_ecdsa_verify` if it is in lower-S form. For verifying
 99+ *  recoverable signatures that were not created with libsecp256k1 (or might have
100+ *  been malleated), it is thus necessary to normalize the signature using
101+ *  `secp256k1_ecdsa_signature_normalize` between conversion and verification.
102+ *  A typical usage might look like this:

jonasnick commented at 6:45 pm on September 14, 2025:

I think it’d be better if the docs pointed out that this is not typical usage. In typical usage you only care whether the signature is valid for the recovered public key. So you just need run ecdsa_recover and do not need to call ecdsa_verify.

Maybe there are cases where this is necessary (e.g., you have to hand over the signature and public key to a verify that doesn’t support ecdsa_recover, like the Bitcoin consensus protocol).

theStack commented at 0:10 am on September 15, 2025:

Makes sense. My assumption when writing this was that most users would very likely want to enforce lower-S in order to avoid malleability, but that was more based on gut feeling than actual research (seems BOLT11 now explicitly allows low- and high-S signatures if pubkey recovery is used: https://github.com/lightning/bolts/pull/1284).

in include/secp256k1_recovery.h:97 in e9c10f7df8 outdated

91@@ -92,7 +92,22 @@ SECP256K1_API int secp256k1_ecdsa_sign_recoverable(
92 
93 /** Recover an ECDSA public key from a signature.
94  *
95- *  Returns: 1: public key successfully recovered (which guarantees a correct signature).
96+ *  Note that successful public key recovery implies a correct ECDSA signature
97+ *  in a mathematical sense, but it is only guaranteed to also pass verification
98+ *  with `secp256k1_ecdsa_verify` if it is in lower-S form. For verifying

jonasnick commented at 7:18 pm on September 14, 2025:

“correct ECDSA signature in a mathematical sense” sounds a bit imprecise. How about the following wording, which avoids that term entirely?

0Successful public key recovery guarantees that the signature, when normalized,
1passes `secp256k1_ecdsa_verify`. Thus, explicit verification is not necessary.
2
3A recoverable signature converted to a normal signature may not pass
4`secp256k1_ecdsa_verify` if it is not normalized. If a normalized signature is
5required, call `secp256k1_ecdsa_signature_normalize` after
6`secp256k1_ecdsa_recover`.

theStack commented at 0:01 am on September 15, 2025:

Took that suggestion, and set you as commit author accordingly. As tiny-nit, it’s slightly unfortunate that “normal” and “normalized” sound so similar, but not sure how to avoid it (maybe it’s not a big deal).

real-or-random commented at 6:56 pm on September 15, 2025:

Took that suggestion, and set you as commit author accordingly. As tiny-nit, it’s slightly unfortunate that “normal” and “normalized” sound so similar, but not sure how to avoid it (maybe it’s not a big deal).

Stumbled upon this, too. One fix is s/normal/non-recoverable. Another one is to spell out the types explicitly:

Here’s an improved version. This uses a middle way. It says “non-recoverable” but mentions secp256k1_ecdsa_recoverable_signature_convert so the reader can look up the exact types.

0Successful public key recovery guarantees that the signature, after normalization,
1passes `secp256k1_ecdsa_verify`. Thus, explicit verification is not necessary.
2
3However, a non-recoverable signature converted from a recoverable signature
4(using `secp256k1_ecdsa_recoverable_signature_convert`) is not guaranteed
5to be normalized and thus not guaranteed to pass `secp256k1_ecdsa_verify`
6(even if it passes `secp256k1_ecdsa_recover`). If a normalized signature is
7required, call `secp256k1_ecdsa_signature_normalize` after `secp256k1_ecdsa_recoverable_signature_convert`.

theStack commented at 0:26 am on September 16, 2025:

SGTM, taken.

jonasnick commented at 7:14 am on September 16, 2025:

“a non-recoverable signature […] (even if it passes secp256k1_ecdsa_recover)” may be a bit confusing because a non-recoverable signature cannot pass secp256k1_ecdsa_recover.

How about this:

0However, a recoverable signature that successfully passes `secp256k1_ecdsa_recover`,
1when converted to a non-recoverable signature (using
2`secp256k1_ecdsa_recoverable_signature_convert`), is not guaranteed to be
3normalized and thus not guaranteed to pass `secp256k1_ecdsa_verify`.

real-or-random commented at 7:23 am on September 16, 2025:

“a non-recoverable signature […] (even if it passes secp256k1_ecdsa_recover)” may be a bit confusing because a non-recoverable signature cannot pass secp256k1_ecdsa_recover.

Good point.

theStack commented at 7:31 pm on September 16, 2025:

Updated again with the suggestion.

theStack force-pushed on Sep 15, 2025

theStack force-pushed on Sep 16, 2025

real-or-random approved

real-or-random commented at 6:26 am on September 16, 2025: contributor

utACK 131f871469945c3c0da53456b6d74ef72a5fd587

kmk142789 approved

doc: clarify API doc of `secp256k1_ecdsa_recover` return value

Co-authored-by: Tim Ruffing <me@real-or-random.org>

7321bdf27b

theStack force-pushed on Sep 16, 2025

jonasnick approved

jonasnick commented at 8:25 am on September 17, 2025: contributor

ACK 7321bdf27bdc6a68875c61510c1a0d9ec4b0b7e3

Thanks @theStack

jonasnick merged this on Sep 17, 2025

jonasnick closed this on Sep 17, 2025

theStack deleted the branch on Sep 17, 2025

doc: clarify API doc of secp256k1_ecdsa_recover return value #1741

doc: clarify API doc of `secp256k1_ecdsa_recover` return value #1741