bitcoin-core
Constness of pointers in nonce_function, callbacks
The context callbacks and nonce_function take non-const data pointers but the functions calling them take const pointers.
I think this is somewhat confusing, in particular for the user-exposed ecsda_sign() that takes a const pointer but also a nonce_function with a non-const pointer.
This has been introduced by https://github.com/bitcoin-core/secp256k1/pull/299/commits/05732c5a5f781d49972659d8f59e5262ff026ce8 but I don't really understand the benefits of this method.
cc @luke-jr
You can pass any pointer into const void*, but you can't pass const pointers into void*
The callback uses void* so the API user doesn't need to cast away the constness itself
A const void pointer is a subtype of a void pointer, meaning that a void pointer can be converted to a cont void pointer as needed.
Function parameters are contravariant, so that means a function signature taking a void pointer is morally an subtype of the same function signature but taking a const void pointer, meaning that a function taking a const void pointer argument can be morally be "converted" to a the same function taking a void pointer (though this conversion of function doesn't actually happen; rather the effect of it happens when the caller can converts their void pointer to a const void pointer during the call).
Because function arguments are contravariant, the variance of the arguments to a function pointer are also reversed. That means that a function signature f with an argument that is a function pointer with a signature g, the variance of the parameters of g are flipped to being covariant. So a function signature f taking a function pointer signature g taking a const void pointer is a subtype of the same function signature f taking the same function pointer signature g but taking a void pointer. Thus a function f taking a function pointer g taking a void pointer can be morally "converted" to the same function taking a function pointer taking a const void pointer.
The implication is that arguments to function pointer arguments in the libsecp256k1 will be more general if they are void pointers rather than const void pointers. And indeed https://github.com/bitcoin-core/secp256k1/commit/05732c5a5f781d49972659d8f59e5262ff026ce8 is a change that made the libsecp256k1 API more general in this way.
With that said, just because the API is as general as it can be, doesn't mean the API is correct, because having a more general API doesn't make it const-correct, and the current API doesn't appear to me to be const-correct (for some definition of const-correctness). In particular, if the users allocates a const nonce_data argument and calls ecdsa_sign with it along with a nonce_function that modifies the nonce_data, then UB will happen.
I suspect the problem is that the two possible API's for ecdsa_sign, one taking const nonce_data and a nonce_function operating on const nonce_data, versus the other API for ecdsa_sign, taking non-const nonce_data and a nonce_function operating on non-const nonce_data are invariant with respect to each other, meaning neither one is a subtype of each other.
If this is true, the above line of reasoning suggests that you need to provide both the const and non-const version of the ecdsa_sign function to be const-correct (thank you lack-of-polymorphism-in-C).
However, another line of argument says, "look, we already have to use void pointers to get around the lack-of-polymorphism-in-C, so users already have to be careful to cast to appropriate data types in their nonce_functions. It is not really worse for them to be careful regarding the constness of their casting while they are at it." That is to say, in their nonce_function, while casting from their void pointer, they had better not only only cast to the correct datatype but also cast to the correct const qualifier too.
That line of argument makes sense to me, so long as there is no UB caused by C during the intermediate use of non-const void pointers that may actually be pointing to const data. AFAIU, there are no such problems in C, as long as the const qualifiers (and datatypes) are properly cast back in the nonce_function.
My judgement is that the API is good as is, given the lack-of-polymorphism-in-C. If their nonce_data is const, their nonce_function needs to restore the const qualifier. If their nonce_data is non-const, then AFAIU, it is safe and legal in C for that const qualifier to be cast away and not be restored in their nonce_function.
