Improve crytpo::taproot error type
#1895
Conversation
Add 'use Error::*' locally to make the code more terse.
We provide a `From<secp255k1::Error>` impl so we do not need to explicitly convert the error return, just use `?`.
The rustdoc on the `taproot::Error::InvalidSighashType` is wrong, fix it.
This error type is only used in the `from_slice` function. Use prefix `Sig` because `taproot::FromSliceError` does not fully express how the error came about. Use specific identifier for the error, this aids usage but also prevents us later adding "random" other variants into this error and using it in other functions.
9bf5695 to
202d1cd
Compare
|
Concept ACK but maybe simply |
|
I agree its a bit long but I think the module is unusual in our code so a slightly long one here is better than non-uniform |
| .map_err(|_| Error::InvalidSighashType(*hash_ty))?; | ||
| let sig = | ||
| secp256k1::schnorr::Signature::from_slice(sig)?; | ||
| .map_err(|_| SigFromSliceError::InvalidSighashType(*hash_ty))?; |
There was a problem hiding this comment.
Maybe there should be From conversion?
There was a problem hiding this comment.
sighash::Error needs splitting up, its on my todo list. Then the SigFromSliceErorr can have a variant that holds the error returned here.
In case you are interested, I audited all errors yesterday and there are actually not that many that need splitting up, we are close to having good errors I believe. My list of modules that contain errors that need improving is:
- bip158
- bip32
- script/mod
- crypto/ecdsa
- crypto/key
- crypto/sighash
First three patches are preparatory cleanup, last patch renames
crypto::taproot::ErrortoSigFromSliceError. See commit log for justification of theSigprefix.Done as part of the great error cleanup.