Skip to content

Add ECDSA pubkey recovery usage example #1714

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

theStack
Copy link
Contributor

The recovery module is probably not super-relevant these days for newer projects (the primary use-case I'm aware of is message signing in Bitcoin Core and Electrum, likely other wallets; something that is hopefully replaced by BIP-322 one day), but it still seems better to have an example than to have none. It contains all of the five API calls, i.e. for signing, recovering, converting, serializing, parsing. As usual with examples, a lot of code and comments are duplicated (e.g. context creation, keypair generation, cleanup with secret key clearing etc.).

@theStack theStack force-pushed the add_recovery_example branch from a58848d to 314da41 Compare July 30, 2025 22:49
@theStack
Copy link
Contributor Author

I suppose the CI failures for the Valgrind jobs on MacOS Ventura (https://github.com/bitcoin-core/secp256k1/actions/runs/16623024583/job/47031936047 and https://github.com/bitcoin-core/secp256k1/actions/runs/16623024583/job/47031935996) are caused by the same memcmp-issue as observed in the Silent Payments PR (see #1519 (comment) ff.). Stole the my_memcmp_var reimplementation in #1698 from @josibake (thanks!) and force-pushed. Will consider moving it to examples_util.h if it solves the problem.

@josibake
Copy link
Member

The CI failure seems unrelated and probably just needs a restart; its caused by a 503 when installing packages inside the docker container (if I'm reading the logs correctly): https://github.com/bitcoin-core/secp256k1/actions/runs/16635429498/job/47075060509?pr=1714

@fanquake
Copy link
Member

Kicked the failure.

@theStack
Copy link
Contributor Author

The CI failure seems unrelated and probably just needs a restart; its caused by a 503 when installing packages inside the docker container (if I'm reading the logs correctly): https://github.com/bitcoin-core/secp256k1/actions/runs/16635429498/job/47075060509?pr=1714

Yeah indeed. Before the latest force-push (i.e. still using memcmp) the failure looked like this though:
https://github.com/bitcoin-core/secp256k1/actions/runs/16623024583/job/47031936047#step:7:1141
pointing to the line using memcmp, so I guess I have to keep the workaround.

Kicked the failure.

Thanks!

@josibake
Copy link
Member

Yeah indeed. Before the latest force-push

Sorry, my comment was unclear. What I meant to say is your solution works and the current failure is definitely not related.

@real-or-random real-or-random added the user-documentation user-facing documentation label Aug 1, 2025
Comment on lines 117 to 129
/* Successful recovery guarantees a correct signature, but we also do an explicit verification
do demonstrate how to convert a recoverable to a normal ECDSA signature */
return_val = secp256k1_ecdsa_recoverable_signature_convert(ctx, &normal_sig, &recoverable_sig);
assert(return_val);
if (!secp256k1_ecdsa_verify(ctx, &normal_sig, msg, &recovered_pubkey)) {
printf("Signature verification with converted recoverable signature failed\n");
return EXIT_FAILURE;
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is true for signatures created with secp256k1_ecdsa_sign_recoverable, but not necessarily for arbitrarily generated ECDSA signatures (see #1718).

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's interesting, wasn't aware. Added an secp256k1_ecdsa_signature_normalize call before the verification (it's a no-op in this example, but for demonstration purposes) and tried to explain that with a comment.

@theStack theStack force-pushed the add_recovery_example branch from 314da41 to de5b223 Compare August 5, 2025 22:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
user-documentation user-facing documentation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants