You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
Is this the right thing to do? AFAIK this is the only place in our API where a parsing function can abort your program on invalid input.
This means that whoever uses the API must check the recid before passing it to the parsing function.
I think this makes sense on opaque structs which we require to go through the parsing functions first and not manually initialize them because that will be "library UB"(sometimes abort, others unexpected results, see #668#701) but I think it makes less sense on an int used as the input to a parsing function.
The text was updated successfully, but these errors were encountered:
Yeah, this somehow depends on what "parsing" is. The parse function is supposed to parse the signature (as in "read a byte array"), whereas it assumes that recid is an int already. But that's weird because the normal usage of recovery is that the recid is transferred along with the signature. You may even consider it part of the signature, and if you look at the caller in Core, that's pretty much what it's doing: https://github.com/bitcoin/bitcoin/blob/99813a9745fe10a58bedd7a4cb721faf14f907a4/src/pubkey.cpp#L187
I'm not sure to be honest. I think the ideal API would define a serialized recoverable signature to be a 65-byte string and simply take care of parsing the first byte. Maybe we can simply add a function that does this but also the keep the current function as it is?
If people don't think that's a good idea, I think you're right. Then the pragmatic thing is to return 0. (And we could do this change without breaking the API, we're just defining undefined cases.)
Hi,
Currently if you pass a recid that is
recid < 0 || recid > 3
intosecp256k1_ecdsa_recoverable_signature_parse_compact
it will fail onARG_CHECK
which will callsecp256k1_callback_call
which will by default abort the program.https://github.com/bitcoin-core/secp256k1/blob/master/src/modules/recovery/main_impl.h#L46
Is this the right thing to do? AFAIK this is the only place in our API where a parsing function can abort your program on invalid input.
This means that whoever uses the API must check the recid before passing it to the parsing function.
I think this makes sense on opaque structs which we require to go through the parsing functions first and not manually initialize them because that will be "library UB"(sometimes abort, others unexpected results, see #668 #701) but I think it makes less sense on an int used as the input to a parsing function.
The text was updated successfully, but these errors were encountered: