docs: Clarify that callback can be called more than once

[real-or-random]

The tests in #1698 reminded me that some functions, e.g., secp256k1_ec_pubkey_cmp, may call the illegal callback more than once (see #1390 (comment) for more context). This PR clarifies the API docs to state explicitly that this is possible.

This is the simplest solution. Any production code should crash anyway if it encounters a callback. And in debug code or in our test code, it doesn't really matter whether you see an error message once or twice.

The alternative is to provide a guarantee that the callback is called only once. But that would make our code more complex for no good reason.

The second commit fixes a few typos, wording, and consistency.

[sipa]

concept ack

[real-or-random]

@stratospher Want to review this? :)

[stratospher]

ACK d279379. verified that illegal callback can be called more than once. and the simple route makes sense.

[stratospher]

• When this callback is triggered, the API function called is guaranteed not
• to cause a crash, though its return value and output arguments are
• undefined.

when I initially read the comments on master, I assumed "API not crashing guarantee" is only for debug stage (no abort call) since they were in 1 para. but after seeing this diff just realised that crashing in a callback is acceptable and still a stable API regardless of debug/production build. so I think the comments made it clearer for me!

[real-or-random]

Nice, paragraph breaks matter. :)

Though I think it is relatively clear even on master. When the callback calls abort, then the entire program will crash immediately. And from that point on, any guarantee you get from the API is vacuous.

[stratospher]

ok that makes sense. but now I'm confused. It's just a line break difference but I interpreted the meaning before and after as:

1. before:

*  The philosophy is that these shouldn't be dealt with through a
 *  specific return value, as calling code should not have branches to deal with
 *  the case that this code itself is broken.
 *
 *  On the other hand, during debug stage, one would want to be informed about
 *  such mistakes, and the default (crashing) may be inadvisable.
 *  When this callback is triggered, the API function called is guaranteed not
 *  to cause a crash, though its return value and output arguments are

API function called is guaranteed not to cause a crash in debug stage (in tests where it's mostly the callback function counting_callback_fn that just counts and no abort)

2. Now:

*  The philosophy is that these shouldn't be dealt with through a
 *  specific return value, as calling code should not have branches to deal with
 *  the case that this code itself is broken. On the other hand, during debug
 *  stage, one would want to be informed about such mistakes, and the default
 *  (crashing) may be inadvisable.
 *
 *  When this callback is triggered, the API function called is guaranteed not
 *  to cause a crash, though its return value and output arguments are
 *  undefined. A single API call may trigger the callback more than once.

API function called is guaranteed not to cause a crash in any stage (tests or IRL)
=> because it's the callback doing the crash + handling invalid user inputs + user responsibility and so API is stable?

do we want 1 or 2?