Merge rust-bitcoin/rust-bitcoin#1895: Improve `crytpo::taproot` error type

202d1cd581 Rename taproot::Error to SigFromSliceError (Tobin C. Harding)
29678cb82b Correctly document InvalidSighashType variant (Tobin C. Harding)
13d5c0536b Remove explicit error conversion (Tobin C. Harding)
d86517ae4f taproot: Use error variants locally (Tobin C. Harding)

Pull request description:

  First three patches are preparatory cleanup, last patch renames `crypto::taproot::Error` to `SigFromSliceError`. See commit log for justification of the `Sig` prefix.

  Done as part of the great error cleanup.

ACKs for top commit:
  apoelstra:
    ACK 202d1cd581
  Kixunil:
    ACK 202d1cd581

Tree-SHA512: 87aef07d2a3518c68c070e348d2331a9fbf1dc5cd36fd4d966607ddb0531eca9dc6be08f1923f941d33973f173b915490de9ef0cad724cce7bf108cdc8a82af0

bitcoin/src/crypto/taproot.rs

@@ -26,23 +26,21 @@ pub struct Signature {
 
 impl Signature {
     /// Deserialize from slice
-    pub fn from_slice(sl: &[u8]) -> Result<Self, Error> {
+    pub fn from_slice(sl: &[u8]) -> Result<Self, SigFromSliceError> {
         match sl.len() {
             64 => {
                 // default type
-                let sig =
+                let sig = secp256k1::schnorr::Signature::from_slice(sl)?;
-                    secp256k1::schnorr::Signature::from_slice(sl).map_err(Error::Secp256k1)?;
                 Ok(Signature { sig, hash_ty: TapSighashType::Default })
             }
             65 => {
                 let (hash_ty, sig) = sl.split_last().expect("Slice len checked == 65");
                 let hash_ty = TapSighashType::from_consensus_u8(*hash_ty)
-                    .map_err(|_| Error::InvalidSighashType(*hash_ty))?;
+                    .map_err(|_| SigFromSliceError::InvalidSighashType(*hash_ty))?;
-                let sig =
+                let sig = secp256k1::schnorr::Signature::from_slice(sig)?;
-                    secp256k1::schnorr::Signature::from_slice(sig).map_err(Error::Secp256k1)?;
                 Ok(Signature { sig, hash_ty })
             }
-            len => Err(Error::InvalidSignatureSize(len)),
+            len => Err(SigFromSliceError::InvalidSignatureSize(len)),
         }
     }
 
@@ -59,11 +57,13 @@ impl Signature {
     }
 }
 
-/// A taproot sig related error.
+/// An error constructing a [`taproot::Signature`] from a byte slice.
+///
+/// [`taproot::Signature`]: crate::crypto::taproot::Signature
 #[derive(Clone, PartialEq, Eq, PartialOrd, Ord, Hash, Debug)]
 #[non_exhaustive]
-pub enum Error {
+pub enum SigFromSliceError {
-    /// Base58 encoding error
+    /// Invalid signature hash type.
     InvalidSighashType(u8),
     /// Signature has valid size but does not parse correctly
     Secp256k1(secp256k1::Error),
@@ -71,22 +71,23 @@ pub enum Error {
     InvalidSignatureSize(usize),
 }
 
-impl fmt::Display for Error {
+impl fmt::Display for SigFromSliceError {
     fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
+        use SigFromSliceError::*;
+
         match *self {
-            Error::InvalidSighashType(hash_ty) =>
+            InvalidSighashType(hash_ty) => write!(f, "invalid signature hash type {}", hash_ty),
-                write!(f, "invalid signature hash type {}", hash_ty),
+            Secp256k1(ref e) =>
-            Error::Secp256k1(ref e) =>
                 write_err!(f, "taproot signature has correct len but is malformed"; e),
-            Error::InvalidSignatureSize(sz) => write!(f, "invalid taproot signature size: {}", sz),
+            InvalidSignatureSize(sz) => write!(f, "invalid taproot signature size: {}", sz),
         }
     }
 }
 
 #[cfg(feature = "std")]
-impl std::error::Error for Error {
+impl std::error::Error for SigFromSliceError {
     fn source(&self) -> Option<&(dyn std::error::Error + 'static)> {
-        use self::Error::*;
+        use SigFromSliceError::*;
 
         match self {
             Secp256k1(e) => Some(e),
@@ -95,6 +96,6 @@ impl std::error::Error for Error {
     }
 }
 
-impl From<secp256k1::Error> for Error {
+impl From<secp256k1::Error> for SigFromSliceError {
-    fn from(e: secp256k1::Error) -> Error { Error::Secp256k1(e) }
+    fn from(e: secp256k1::Error) -> Self { SigFromSliceError::Secp256k1(e) }
 }

bitcoin/src/psbt/error.rs

@@ -85,7 +85,7 @@ pub enum Error {
     /// Parsing error indicating invalid ECDSA signatures
     InvalidEcdsaSignature(crate::crypto::ecdsa::Error),
     /// Parsing error indicating invalid taproot signatures
-    InvalidTaprootSignature(crate::crypto::taproot::Error),
+    InvalidTaprootSignature(crate::crypto::taproot::SigFromSliceError),
     /// Parsing error indicating invalid control block
     InvalidControlBlock,
     /// Parsing error indicating invalid leaf version

bitcoin/src/psbt/serialize.rs

@@ -258,10 +258,12 @@ impl Serialize for taproot::Signature {
 
 impl Deserialize for taproot::Signature {
     fn deserialize(bytes: &[u8]) -> Result<Self, Error> {
+        use taproot::SigFromSliceError::*;
+
         taproot::Signature::from_slice(bytes).map_err(|e| match e {
-            taproot::Error::InvalidSighashType(flag) => Error::NonStandardSighashType(flag as u32),
+            InvalidSighashType(flag) => Error::NonStandardSighashType(flag as u32),
-            taproot::Error::InvalidSignatureSize(_) => Error::InvalidTaprootSignature(e),
+            InvalidSignatureSize(_) => Error::InvalidTaprootSignature(e),
-            taproot::Error::Secp256k1(..) => Error::InvalidTaprootSignature(e),
+            Secp256k1(..) => Error::InvalidTaprootSignature(e),
         })
     }
 }

bitcoin/src/taproot.rs

@@ -17,7 +17,7 @@ use secp256k1::{self, Scalar, Secp256k1};
 use crate::consensus::Encodable;
 use crate::crypto::key::{TapTweak, TweakedPublicKey, UntweakedPublicKey, XOnlyPublicKey};
 // Re-export these so downstream only has to use one `taproot` module.
-pub use crate::crypto::taproot::{Error, Signature};
+pub use crate::crypto::taproot::{SigFromSliceError, Signature};
 use crate::prelude::*;
 use crate::{io, Script, ScriptBuf};