Merge rust-bitcoin/rust-bitcoin#987: Implement std::error::Error for the new MSRV

97a5bb1 Implement std::error::source codebase wide (Tobin C. Harding)
0a9191b Add parenthesis around left hand side of companion (Tobin C. Harding)
7cf8af2 Put Error impl block below Display (Tobin C. Harding)
2384712 Re-order Display match arms (Tobin C. Harding)

Pull request description:

  Now that we have MSRV of 1.41.1 we should use `source` instead of `cause`. Audit the whole codebase and implement `source` for _every_ error type we have.

  The first three patches are preparatory cleanup, patch 3 is particularly shameful (adds parenthesis to make my editor work).

  CC @Kixunil because he is championing the error stuff.

ACKs for top commit:
  apoelstra:
    ACK 97a5bb1

Tree-SHA512: 46313a28929445f32e01e30ca3b0246b30bc9d5e43db5754d4b441e9c30d3e427efaf247100eb6b452f98beec5a4fcde1daba7943a772114aa34f78ab52cbc60

Author: apoelstra

src/blockdata/block.rs

@@ -355,7 +355,16 @@ impl fmt::Display for Bip34Error {
 }
 
 #[cfg(feature = "std")]
-impl ::std::error::Error for Bip34Error {}
+#[cfg_attr(docsrs, doc(cfg(feature = "std")))]
+impl std::error::Error for Bip34Error {
+    fn source(&self) -> Option<&(dyn std::error::Error + 'static)> {
+        use self::Bip34Error::*;
+
+        match self {
+            Unsupported | NotPresent | UnexpectedPush(_) => None,
+        }
+    }
+}
 
 #[cfg(test)]
 mod tests {

src/blockdata/script.rs

@@ -188,7 +188,20 @@ impl fmt::Display for Error {
 
 #[cfg(feature = "std")]
 #[cfg_attr(docsrs, doc(cfg(feature = "std")))]
-impl ::std::error::Error for Error {}
+impl std::error::Error for Error {
+    fn source(&self) -> Option<&(dyn std::error::Error + 'static)> {
+        use self::Error::*;
+
+        match self {
+            NonMinimalPush
+            | EarlyEndOfScript
+            | NumericOverflow
+            | BitcoinConsensus(_) // TODO: This should return `Some` but bitcoinconsensus::Error does not implement Error.
+            | UnknownSpentOutput(_)
+            | SerializationError => None,
+        }
+    }
+}
 
 // Our internal error proves that we only return these two cases from `read_uint_iter`.
 // Since it's private we don't bother with trait impls besides From.

src/blockdata/transaction.rs

@@ -27,7 +27,6 @@ use crate::prelude::*;
 
 use crate::io;
 use core::{fmt, str, default::Default};
-#[cfg(feature = "std")] use std::error;
 
 use crate::hashes::{self, Hash, sha256d};
 use crate::hashes::hex::FromHex;
@@ -142,12 +141,14 @@ impl fmt::Display for ParseOutPointError {
 
 #[cfg(feature = "std")]
 #[cfg_attr(docsrs, doc(cfg(feature = "std")))]
-impl error::Error for ParseOutPointError {
-    fn cause(&self) -> Option<&dyn  error::Error> {
-        match *self {
-            ParseOutPointError::Txid(ref e) => Some(e),
-            ParseOutPointError::Vout(ref e) => Some(e),
-            _ => None,
+impl std::error::Error for ParseOutPointError {
+    fn source(&self) -> Option<&(dyn std::error::Error + 'static)> {
+        use self::ParseOutPointError::*;
+
+        match self {
+            Txid(e) => Some(e),
+            Vout(e) => Some(e),
+            Format | TooLong | VoutNotCanonical => None,
         }
     }
 }
@@ -733,7 +734,11 @@ impl fmt::Display for NonStandardSighashType {
 
 #[cfg(feature = "std")]
 #[cfg_attr(docsrs, doc(cfg(feature = "std")))]
-impl error::Error for NonStandardSighashType {}
+impl std::error::Error for NonStandardSighashType {
+    fn source(&self) -> Option<&(dyn std::error::Error + 'static)> {
+        None
+    }
+}
 
 /// Legacy Hashtype of an input's signature
 #[deprecated(since = "0.28.0", note = "Please use [`EcdsaSighashType`] instead")]
@@ -886,9 +891,13 @@ impl fmt::Display for SighashTypeParseError {
     }
 }
 
-#[cfg_attr(docsrs, doc(cfg(feature = "std")))]
 #[cfg(feature = "std")]
-impl ::std::error::Error for SighashTypeParseError {}
+#[cfg_attr(docsrs, doc(cfg(feature = "std")))]
+impl std::error::Error for SighashTypeParseError {
+    fn source(&self) -> Option<&(dyn std::error::Error + 'static)> {
+        None
+    }
+}
 
 #[cfg(test)]
 mod tests {

src/consensus/encode.rs

@@ -30,7 +30,6 @@
 use crate::prelude::*;
 
 use core::{fmt, mem, u32, convert::From};
-#[cfg(feature = "std")] use std::error;
 
 use crate::hashes::{sha256d, Hash, sha256};
 use crate::hash_types::{BlockHash, FilterHash, TxMerkleNode, FilterHeader};
@@ -106,18 +105,20 @@ impl fmt::Display for Error {
 
 #[cfg(feature = "std")]
 #[cfg_attr(docsrs, doc(cfg(feature = "std")))]
-impl ::std::error::Error for Error {
-    fn cause(&self) -> Option<&dyn  error::Error> {
-        match *self {
-            Error::Io(ref e) => Some(e),
-            Error::Psbt(ref e) => Some(e),
-            Error::UnexpectedNetworkMagic { .. }
-            | Error::OversizedVectorAllocation { .. }
-            | Error::InvalidChecksum { .. }
-            | Error::NonMinimalVarInt
-            | Error::UnknownNetworkMagic(..)
-            | Error::ParseFailed(..)
-            | Error::UnsupportedSegwitFlag(..) => None,
+impl std::error::Error for Error {
+    fn source(&self) -> Option<&(dyn std::error::Error + 'static)> {
+        use self::Error::*;
+
+        match self {
+            Io(e) => Some(e),
+            Psbt(e) => Some(e),
+            UnexpectedNetworkMagic { .. }
+            | OversizedVectorAllocation { .. }
+            | InvalidChecksum { .. }
+            | NonMinimalVarInt
+            | UnknownNetworkMagic(_)
+            | ParseFailed(_)
+            | UnsupportedSegwitFlag(_) => None,
         }
     }
 }

src/network/message.rs

@@ -111,9 +111,13 @@ impl fmt::Display for CommandStringError {
     }
 }
 
-#[cfg_attr(docsrs, doc(cfg(feature = "std")))]
 #[cfg(feature = "std")]
-impl ::std::error::Error for CommandStringError {}
+#[cfg_attr(docsrs, doc(cfg(feature = "std")))]
+impl std::error::Error for CommandStringError {
+    fn source(&self) -> Option<&(dyn std::error::Error + 'static)> {
+        None
+    }
+}
 
 /// A Network message
 #[derive(Clone, Debug, PartialEq, Eq)]

src/network/mod.rs

@@ -20,7 +20,6 @@
 
 use crate::io;
 use core::fmt;
-#[cfg(feature = "std")] use std::error;
 
 pub mod constants;
 
@@ -79,11 +78,13 @@ impl From<io::Error> for Error {
 
 #[cfg(feature = "std")]
 #[cfg_attr(docsrs, doc(cfg(feature = "std")))]
-impl error::Error for Error {
-    fn cause(&self) -> Option<&dyn error::Error> {
-        match *self {
-            Error::Io(ref e) => Some(e),
-            Error::SocketMutexPoisoned | Error::SocketNotConnectedToPeer => None,
+impl std::error::Error for Error {
+    fn source(&self) -> Option<&(dyn std::error::Error + 'static)> {
+        use self::Error::*;
+
+        match self {
+            Io(e) => Some(e),
+            SocketMutexPoisoned | SocketNotConnectedToPeer => None,
         }
     }
 }

src/util/address.rs

@@ -37,7 +37,6 @@ use crate::prelude::*;
 use core::fmt;
 use core::num::ParseIntError;
 use core::str::FromStr;
-#[cfg(feature = "std")] use std::error;
 
 use secp256k1::{Secp256k1, Verification, XOnlyPublicKey};
 use bech32;
@@ -104,13 +103,22 @@ impl fmt::Display for Error {
 
 #[cfg(feature = "std")]
 #[cfg_attr(docsrs, doc(cfg(feature = "std")))]
-impl ::std::error::Error for Error {
-    fn cause(&self) -> Option<&dyn  error::Error> {
-        match *self {
-            Error::Base58(ref e) => Some(e),
-            Error::Bech32(ref e) => Some(e),
-            Error::UnparsableWitnessVersion(ref e) => Some(e),
-            _ => None,
+impl std::error::Error for Error {
+    fn source(&self) -> Option<&(dyn std::error::Error + 'static)> {
+        use self::Error::*;
+
+        match self {
+            Base58(e) => Some(e),
+            Bech32(e) => Some(e),
+            UnparsableWitnessVersion(e) => Some(e),
+            EmptyBech32Payload
+            | InvalidBech32Variant { .. }
+            | InvalidWitnessVersion(_)
+            | MalformedWitnessVersion
+            | InvalidWitnessProgramLength(_)
+            | InvalidSegwitV0ProgramLength(_)
+            | UncompressedPubkey
+            | ExcessiveScriptSize => None,
         }
     }
 }

src/util/amount.rs

@@ -192,7 +192,7 @@ impl fmt::Display for ParseAmountError {
 
 #[cfg(feature = "std")]
 #[cfg_attr(docsrs, doc(cfg(feature = "std")))]
-impl ::std::error::Error for ParseAmountError {}
+impl std::error::Error for ParseAmountError {}
 
 fn is_too_precise(s: &str, precision: usize) -> bool {
     s.contains('.') || precision >= s.len() || s.chars().rev().take(precision).any(|d| d != '0')

src/util/base58.rs

@@ -57,8 +57,8 @@ impl fmt::Display for Error {
             Error::BadByte(b) => write!(f, "invalid base58 character 0x{:x}", b),
             Error::BadChecksum(exp, actual) => write!(f, "base58ck checksum 0x{:x} does not match expected 0x{:x}", actual, exp),
             Error::InvalidLength(ell) => write!(f, "length {} invalid for this base58 type", ell),
-            Error::InvalidAddressVersion(ref v) => write!(f, "address version {} is invalid for this base58 type", v),
             Error::InvalidExtendedKeyVersion(ref v) => write!(f, "extended key version {:#04x?} is invalid for this base58 type", v),
+            Error::InvalidAddressVersion(ref v) => write!(f, "address version {} is invalid for this base58 type", v),
             Error::TooShort(_) => write!(f, "base58ck data not even long enough for a checksum"),
             Error::Secp256k1(ref e) => fmt::Display::fmt(&e, f),
             Error::Hex(ref e) => write!(f, "Hexadecimal decoding error: {}", e)
@@ -68,7 +68,22 @@ impl fmt::Display for Error {
 
 #[cfg(feature = "std")]
 #[cfg_attr(docsrs, doc(cfg(feature = "std")))]
-impl ::std::error::Error for Error {}
+impl std::error::Error for Error {
+    fn source(&self) -> Option<&(dyn std::error::Error + 'static)> {
+        use self::Error::*;
+
+        match self {
+            BadByte(_)
+            | BadChecksum(_, _)
+            | InvalidLength(_)
+            | InvalidExtendedKeyVersion(_)
+            | InvalidAddressVersion(_)
+            | TooShort(_) => None,
+            Secp256k1(e) => Some(e),
+            Hex(e) => Some(e),
+        }
+    }
+}
 
 /// Vector-like object that holds the first 100 elements on the stack. If more space is needed it
 /// will be allocated on the heap.

src/util/bip158.rs

@@ -74,10 +74,6 @@ pub enum Error {
     Io(io::Error),
 }
 
-#[cfg(feature = "std")]
-#[cfg_attr(docsrs, doc(cfg(feature = "std")))]
-impl ::std::error::Error for Error {}
-
 impl Display for Error {
     fn fmt(&self, f: &mut Formatter) -> Result<(), fmt::Error> {
         match *self {
@@ -87,6 +83,20 @@ impl Display for Error {
     }
 }
 
+#[cfg(feature = "std")]
+#[cfg_attr(docsrs, doc(cfg(feature = "std")))]
+impl std::error::Error for Error {
+    fn source(&self) -> Option<&(dyn std::error::Error + 'static)> {
+        use self::Error::*;
+
+        match self {
+            UtxoMissing(_) => None,
+            Io(e) => Some(e),
+        }
+    }
+}
+
+
 impl From<io::Error> for Error {
     fn from(io: io::Error) -> Self {
         Error::Io(io)