Merge rust-bitcoin/rust-bitcoin#2042: Split Prevouts errors out into specific error types

e21ee381bc Split Prevouts errors out into specific error types (Tobin C. Harding)

Pull request description:

  Done as part of the great error clean up.

  Currently we are returning a general `Error` from `Prevouts` functions, this is un-informative, we can do better by returning specific types that indicate the exact error path.

ACKs for top commit:
  Kixunil:
    ACK e21ee381bc
  apoelstra:
    ACK e21ee381bc

Tree-SHA512: 2a4900f9e31584ad2b6faafa17ea98742fff9206ee1bf77ed29624e0c7b05e655b3b6bf3710e2da26b0b2b8bd5eb36fdd81decbb1f55b41f153f0fbcc4a9165e

bitcoin/src/crypto/sighash.rs

@@ -15,6 +15,7 @@ use core::borrow::{Borrow, BorrowMut};
 use core::{fmt, str};
 
 use hashes::{hash_newtype, sha256, sha256d, sha256t_hash_newtype, Hash};
+use internals::write_err;
 
 use crate::blockdata::witness::Witness;
 use crate::consensus::{encode, Encodable};
@@ -223,15 +224,15 @@ pub enum Error {
 
     /// There are mismatches in the number of prevouts provided compared to the number of inputs in
     /// the transaction.
-    PrevoutsSize,
+    PrevoutsSize(PrevoutsSizeError),
 
     /// Requested a prevout index which is greater than the number of prevouts provided or a
     /// [`Prevouts::One`] with different index.
-    PrevoutIndex,
+    PrevoutsIndex(PrevoutsIndexError),
 
     /// A single prevout has been provided but all prevouts are needed unless using
     /// `SIGHASH_ANYONECANPAY`.
-    PrevoutKind,
+    PrevoutsKind(PrevoutsKindError),
 
     /// Annex must be at least one byte long and the first bytes must be `0x50`.
     WrongAnnex,
@@ -251,9 +252,9 @@ impl fmt::Display for Error {
             Io(error_kind) => write!(f, "writer errored: {:?}", error_kind),
             IndexOutOfInputsBounds { index, inputs_size } => write!(f, "requested index ({}) is greater or equal than the number of transaction inputs ({})", index, inputs_size),
             SingleWithoutCorrespondingOutput { index, outputs_size } => write!(f, "SIGHASH_SINGLE for input ({}) haven't a corresponding output (#outputs:{})", index, outputs_size),
-            PrevoutsSize => write!(f, "number of supplied prevouts differs from the number of inputs in transaction"),
+            PrevoutsSize(ref e) => write_err!(f, "prevouts size"; e),
-            PrevoutIndex => write!(f, "the index requested is greater than available prevouts or different from the provided [Provided::Anyone] index"),
+            PrevoutsIndex(ref e) => write_err!(f, "prevouts index"; e),
-            PrevoutKind => write!(f, "a single prevout has been provided but all prevouts are needed without `ANYONECANPAY`"),
+            PrevoutsKind(ref e) => write_err!(f, "prevouts kind"; e),
             WrongAnnex => write!(f, "annex must be at least one byte long and the first bytes must be `0x50`"),
             InvalidSighashType(hash_ty) => write!(f, "Invalid taproot signature hash type : {} ", hash_ty),
             NotP2wpkhScript => write!(f, "script is not a script pubkey for a p2wpkh output"),
@@ -267,12 +268,12 @@ impl std::error::Error for Error {
         use Error::*;
 
         match *self {
+            PrevoutsSize(ref e) => Some(e),
+            PrevoutsIndex(ref e) => Some(e),
+            PrevoutsKind(ref e) => Some(e),
             Io(_)
             | IndexOutOfInputsBounds { .. }
             | SingleWithoutCorrespondingOutput { .. }
-            | PrevoutsSize
-            | PrevoutIndex
-            | PrevoutKind
             | WrongAnnex
             | InvalidSighashType(_)
             | NotP2wpkhScript => None,
@@ -284,36 +285,114 @@ impl From<io::Error> for Error {
     fn from(e: io::Error) -> Self { Error::Io(e.kind()) }
 }
 
+impl From<PrevoutsSizeError> for Error {
+    fn from(e: PrevoutsSizeError) -> Self { Self::PrevoutsSize(e) }
+}
+
+impl From<PrevoutsKindError> for Error {
+    fn from(e: PrevoutsKindError) -> Self { Self::PrevoutsKind(e) }
+}
+
+impl From<PrevoutsIndexError> for Error {
+    fn from(e: PrevoutsIndexError) -> Self { Self::PrevoutsIndex(e) }
+}
+
 impl<'u, T> Prevouts<'u, T>
 where
     T: Borrow<TxOut>,
 {
-    fn check_all(&self, tx: &Transaction) -> Result<(), Error> {
+    fn check_all(&self, tx: &Transaction) -> Result<(), PrevoutsSizeError> {
         if let Prevouts::All(prevouts) = self {
             if prevouts.len() != tx.input.len() {
-                return Err(Error::PrevoutsSize);
+                return Err(PrevoutsSizeError);
             }
         }
         Ok(())
     }
 
-    fn get_all(&self) -> Result<&[T], Error> {
+    fn get_all(&self) -> Result<&[T], PrevoutsKindError> {
         match self {
             Prevouts::All(prevouts) => Ok(*prevouts),
-            _ => Err(Error::PrevoutKind),
+            _ => Err(PrevoutsKindError),
         }
     }
 
-    fn get(&self, input_index: usize) -> Result<&TxOut, Error> {
+    fn get(&self, input_index: usize) -> Result<&TxOut, PrevoutsIndexError> {
         match self {
             Prevouts::One(index, prevout) =>
                 if input_index == *index {
                     Ok(prevout.borrow())
                 } else {
-                    Err(Error::PrevoutIndex)
+                    Err(PrevoutsIndexError::InvalidOneIndex)
                 },
-            Prevouts::All(prevouts) =>
+            Prevouts::All(prevouts) => prevouts
-                prevouts.get(input_index).map(|x| x.borrow()).ok_or(Error::PrevoutIndex),
+                .get(input_index)
+                .map(|x| x.borrow())
+                .ok_or(PrevoutsIndexError::InvalidAllIndex),
+        }
+    }
+}
+
+/// The number of supplied prevouts differs from the number of inputs in the transaction.
+#[derive(Debug, Clone, PartialEq, Eq)]
+#[non_exhaustive]
+pub struct PrevoutsSizeError;
+
+impl fmt::Display for PrevoutsSizeError {
+    fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
+        write!(f, "number of supplied prevouts differs from the number of inputs in transaction")
+    }
+}
+
+#[cfg(feature = "std")]
+impl std::error::Error for PrevoutsSizeError {
+    fn source(&self) -> Option<&(dyn std::error::Error + 'static)> { None }
+}
+
+/// A single prevout was been provided but all prevouts are needed without `ANYONECANPAY`.
+#[derive(Debug, Clone, PartialEq, Eq)]
+#[non_exhaustive]
+pub struct PrevoutsKindError;
+
+impl fmt::Display for PrevoutsKindError {
+    fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
+        write!(f, "single prevout provided but all prevouts are needed without `ANYONECANPAY`")
+    }
+}
+
+#[cfg(feature = "std")]
+impl std::error::Error for PrevoutsKindError {
+    fn source(&self) -> Option<&(dyn std::error::Error + 'static)> { None }
+}
+
+/// [`Prevouts`] index related errors.
+#[derive(Debug, Clone, PartialEq, Eq)]
+#[non_exhaustive]
+pub enum PrevoutsIndexError {
+    /// Invalid index when accessing a [`Prevouts::One`] kind.
+    InvalidOneIndex,
+    /// Invalid index when accessing a [`Prevouts::All`] kind.
+    InvalidAllIndex,
+}
+
+impl fmt::Display for PrevoutsIndexError {
+    fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
+        use PrevoutsIndexError::*;
+
+        match *self {
+            InvalidOneIndex => write!(f, "invalid index when accessing a Prevouts::One kind"),
+            InvalidAllIndex => write!(f, "invalid index when accessing a Prevouts::All kind"),
+        }
+    }
+}
+
+#[cfg(feature = "std")]
+impl std::error::Error for PrevoutsIndexError {
+    fn source(&self) -> Option<&(dyn std::error::Error + 'static)> {
+        use PrevoutsIndexError::*;
+
+        match *self {
+            InvalidOneIndex | InvalidAllIndex => None,
         }
     }
 }
@@ -1484,23 +1563,23 @@ mod tests {
         let empty_prevouts : Prevouts<TxOut> = Prevouts::All(&empty_vec);
         assert_eq!(
             c.taproot_signature_hash(0, &empty_prevouts, None, None, TapSighashType::All),
-            Err(Error::PrevoutsSize)
+            Err(Error::PrevoutsSize(PrevoutsSizeError))
         );
         let two = vec![TxOut::NULL, TxOut::NULL];
         let too_many_prevouts = Prevouts::All(&two);
         assert_eq!(
             c.taproot_signature_hash(0, &too_many_prevouts, None, None, TapSighashType::All),
-            Err(Error::PrevoutsSize)
+            Err(Error::PrevoutsSize(PrevoutsSizeError))
         );
         let tx_out = TxOut::NULL;
         let prevout = Prevouts::One(1, &tx_out);
         assert_eq!(
             c.taproot_signature_hash(0, &prevout, None, None, TapSighashType::All),
-            Err(Error::PrevoutKind)
+            Err(Error::PrevoutsKind(PrevoutsKindError))
         );
         assert_eq!(
             c.taproot_signature_hash(0, &prevout, None, None, TapSighashType::AllPlusAnyoneCanPay),
-            Err(Error::PrevoutIndex)
+            Err(Error::PrevoutsIndex(PrevoutsIndexError::InvalidOneIndex))
         );
         assert_eq!(
             c.taproot_signature_hash(10, &prevout, None, None, TapSighashType::AllPlusAnyoneCanPay),