#1570 improve examples: remove key generation loop

[Cheapshot003]

Hi! As discussed in #1570 I propose this as a change to the examples.c

If the key is either zero or out of range we just return 1 and end the example instead of looping until a good key is found. This is all very unlikely but could indicate a faulty/manipulated RNG.
Anybody knows where I can find the docs for this? First commit to this library, hope I don't break anything!

[jonasnick]

Hey @Cheapshot003, thanks for the contribution. What sort of docs are you looking for exactly?

[Cheapshot003]

Hey @Cheapshot003, thanks for the contribution. What sort of docs are you looking for exactly?

If you look here someone talks about API docs that should be changed. I don't find any docs related to the examples.

[jonasnick]

Ah. I don't know which API docs @real-or-random meant.

[Cheapshot003]

ok @jonasnick should be all fixed now :)

[theStack]

@Cheapshot003: can you squash your commits please, see e.g. https://github.com/bitcoin/bitcoin/blob/9cb9651d92ddb5d92724f6a52440601c7a0bbcf8/CONTRIBUTING.md?plain=1#L202-L230? You may also want to ensure that the resulting commit has the correct username and e-mail address set, so it will be attributed to you (the last few commits have just the user name "root" :-))

[Cheapshot003]

Ah I pushed from another machine that wasn't properly configured.
Squashed commits, should be alright now. Thx for the help!

[real-or-random]

Ah. I don't know which API docs @real-or-random meant.

My thinking was that the docs of secp256k1_ec_seckey_verify could not only say that the probability of getting an invalid key is negligible, but also spell out the truth that if this occurs on a randomly generated key, then the caller should assume that the randomness source is severely broken and not retry.

secp256k1/include/secp256k1.h

Lines 682 to 697 in 1988855

	/** Verify an ECDSA secret key.
	*
	* A secret key is valid if it is not 0 and less than the secp256k1 curve order
	* when interpreted as an integer (most significant byte first). The
	* probability of choosing a 32-byte string uniformly at random which is an
	* invalid secret key is negligible.
	*
	* Returns: 1: secret key is valid
	* 0: secret key is invalid
	* Args: ctx: pointer to a context object.
	* In: seckey: pointer to a 32-byte secret key.
	*/
	SECP256K1_API SECP256K1_WARN_UNUSED_RESULT int secp256k1_ec_seckey_verify(
	const secp256k1_context *ctx,
	const unsigned char *seckey
	) SECP256K1_ARG_NONNULL(1) SECP256K1_ARG_NONNULL(2);

Please don't hesitate to add a commit, but we can also do this in a different PR instead.

(Now that I see this, we should also replace "ECDSA secret key" by "elliptic curve secret key" or just "secret key". This dates back to the early days of the library, when it supported only ECDSA.)

[Cheapshot003]

Oh thx, I'll adjust these when I come home in the afternoon. I think it would make sense to do it within this pull request

[real-or-random]

Looks good from my side except minor corrections.

Can you squash your commits again after addressing the comments? I recommend either keeping it two logical commits (one for the examples and one for the docs), or just squashing everything in a single commit if that's easier. (It's a bit a matter of taste in the end. The example changes and the doc changes are not exactly of the same nature, but certainly closely related.)

[real-or-random]

Two final minor remarks from my side (applies to all the example files):

• I think the code comment should be moved right above the second if because it applies to that one.
• Perhaps change "we return 1" (which is obvious from the code itself and doesn't deserve a comment) to "we fail".

[real-or-random]

friendly ping :) @Cheapshot003

[Cheapshot003]

Heyo! Back from vacation! The force push looked a bit scary at first but I think I didn't break anything. Should be alright now!
Greetings

[real-or-random]

utACK cd4f84f

[jonasnick]

ACK cd4f84f