Implement error::Error for all error types

Now that we have an MSRV of 1.41.1 we can implement `std::Error` by way of the `cause` method. Audit the whole codebase and implement `cause` for all error types, matching explicitly on enum variants so we don't forget to update the impls if/when the error enums get modified.
yancyribbens · May 20, 2022 · 5935d86 · 5935d86
1 parent a5db0f0
commit 5935d86
Show file tree

Hide file tree

Showing 9 changed files with 232 additions and 16 deletions.
diff --git a/src/descriptor/key.rs b/src/descriptor/key.rs
@@ -219,7 +219,11 @@ impl fmt::Display for DescriptorKeyParseError {
     }
 }
 
-impl error::Error for DescriptorKeyParseError {}
+impl error::Error for DescriptorKeyParseError {
+    fn source(&self) -> Option<&(dyn std::error::Error + 'static)> {
+        None
+    }
+}
 
 impl fmt::Display for DescriptorPublicKey {
     fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
@@ -369,7 +373,15 @@ impl fmt::Display for ConversionError {
     }
 }
 
-impl error::Error for ConversionError {}
+impl error::Error for ConversionError {
+    fn source(&self) -> Option<&(dyn std::error::Error + 'static)> {
+        use self::ConversionError::*;
+
+        match self {
+            Wildcard | HardenedChild | HardenedWildcard => None,
+        }
+    }
+}
 
 impl DescriptorPublicKey {
     /// The fingerprint of the master key associated with this key, `0x00000000` if none.

diff --git a/src/interpreter/error.rs b/src/interpreter/error.rs
@@ -194,9 +194,46 @@ impl fmt::Display for Error {
 
 impl error::Error for Error {
     fn cause(&self) -> Option<&dyn error::Error> {
-        match *self {
-            Error::Secp(ref err) => Some(err),
-            ref x => Some(x),
+        use self::Error::*;
+
+        match self {
+            AbsoluteLocktimeNotMet(_)
+            | CannotInferTrDescriptors
+            | ControlBlockVerificationError
+            | CouldNotEvaluate
+            | ExpectedPush
+            | HashPreimageLengthMismatch
+            | IncorrectPubkeyHash
+            | IncorrectScriptHash
+            | IncorrectWPubkeyHash
+            | IncorrectWScriptHash
+            | InsufficientSignaturesMultiSig
+            | InvalidEcdsaSignature(_)
+            | InvalidSchnorrSignature(_)
+            | InvalidSchnorrSighashType(_)
+            | NonStandardSighash(_)
+            | MissingExtraZeroMultiSig
+            | MultiSigEvaluationError
+            | NonEmptyWitness
+            | NonEmptyScriptSig
+            | PubkeyParseError
+            | XOnlyPublicKeyParseError
+            | PkEvaluationError(_)
+            | PkHashVerifyFail(_)
+            | RelativeLocktimeNotMet(_)
+            | ScriptSatisfactionError
+            | TapAnnexUnsupported
+            | UncompressedPubkey
+            | UnexpectedStackBoolean
+            | UnexpectedStackEnd
+            | UnexpectedStackElementPush
+            | VerifyFailed => None,
+            ControlBlockParse(e) => Some(e),
+            EcdsaSig(e) => Some(e),
+            Miniscript(e) => Some(e),
+            Secp(e) => Some(e),
+            SchnorrSig(e) => Some(e),
+            SighashError(e) => Some(e),
         }
     }
 }

diff --git a/src/lib.rs b/src/lib.rs
@@ -660,9 +660,51 @@ impl fmt::Display for Error {
 
 impl error::Error for Error {
     fn cause(&self) -> Option<&dyn error::Error> {
-        match *self {
-            Error::BadPubkey(ref e) => Some(e),
-            _ => None,
+        use self::Error::*;
+
+        match self {
+            InvalidOpcode(_)
+            | NonMinimalVerify(_)
+            | InvalidPush(_)
+            | CmsTooManyKeys(_)
+            | MultiATooManyKeys(_)
+            | Unprintable(_)
+            | ExpectedChar(_)
+            | UnexpectedStart
+            | Unexpected(_)
+            | MultiColon(_)
+            | MultiAt(_)
+            | AtOutsideOr(_)
+            | LikelyFalse
+            | UnknownWrapper(_)
+            | NonTopLevel(_)
+            | Trailing(_)
+            | MissingHash(_)
+            | MissingSig(_)
+            | RelativeLocktimeNotMet(_)
+            | AbsoluteLocktimeNotMet(_)
+            | CouldNotSatisfy
+            | TypeCheck(_)
+            | BadDescriptor(_)
+            | MaxRecursiveDepthExceeded
+            | ScriptSizeTooLarge
+            | NonStandardBareScript
+            | ImpossibleSatisfaction
+            | BareDescriptorAddr
+            | TaprootSpendInfoUnavialable
+            | TrNoScriptCode
+            | TrNoExplicitScript => None,
+            Script(e) => Some(e),
+            AddrError(e) => Some(e),
+            BadPubkey(e) => Some(e),
+            Secp(e) => Some(e),
+            #[cfg(feature = "compiler")]
+            CompilerError(e) => Some(e),
+            PolicyError(e) => Some(e),
+            LiftError(e) => Some(e),
+            ContextError(e) => Some(e),
+            AnalysisError(e) => Some(e),
+            PubKeyCtxError(e, _) => Some(e),
         }
     }
 }

diff --git a/src/miniscript/analyzable.rs b/src/miniscript/analyzable.rs
@@ -66,7 +66,19 @@ impl fmt::Display for AnalysisError {
     }
 }
 
-impl error::Error for AnalysisError {}
+impl error::Error for AnalysisError {
+    fn cause(&self) -> Option<&dyn error::Error> {
+        use self::AnalysisError::*;
+
+        match self {
+            SiglessBranch
+            | RepeatedPubkeys
+            | BranchExceedResouceLimits
+            | HeightTimelockCombination
+            | Malleable => None,
+        }
+    }
+}
 
 impl<Pk: MiniscriptKey, Ctx: ScriptContext> Miniscript<Pk, Ctx> {
     /// Whether all spend paths of miniscript require a signature

diff --git a/src/miniscript/context.rs b/src/miniscript/context.rs
@@ -12,7 +12,7 @@
 // If not, see <http://creativecommons.org/publicdomain/zero/1.0/>.
 //
 
-use std::{fmt, hash};
+use std::{error, fmt, hash};
 
 use bitcoin;
 use bitcoin::blockdata::constants::MAX_BLOCK_WEIGHT;
@@ -75,6 +75,31 @@ pub enum ScriptContextError {
     MultiANotAllowed,
 }
 
+impl error::Error for ScriptContextError {
+    fn cause(&self) -> Option<&dyn error::Error> {
+        use self::ScriptContextError::*;
+
+        match self {
+            MalleablePkH
+            | MalleableOrI
+            | MalleableDupIf
+            | CompressedOnly(_)
+            | XOnlyKeysNotAllowed(_, _)
+            | UncompressedKeysNotAllowed
+            | MaxWitnessItemssExceeded { .. }
+            | MaxOpCountExceeded
+            | MaxWitnessScriptSizeExceeded
+            | MaxRedeemScriptSizeExceeded
+            | MaxScriptSigSizeExceeded
+            | ImpossibleSatisfaction
+            | TaprootMultiDisabled
+            | StackSizeLimitExceeded { .. }
+            | CheckMultiSigLimitExceeded
+            | MultiANotAllowed => None,
+        }
+    }
+}
+
 impl fmt::Display for ScriptContextError {
     fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
         match *self {

diff --git a/src/policy/compiler.rs b/src/policy/compiler.rs
@@ -79,7 +79,16 @@ impl fmt::Display for CompilerError {
     }
 }
 
-impl error::Error for CompilerError {}
+impl error::Error for CompilerError {
+    fn cause(&self) -> Option<&dyn error::Error> {
+        use self::CompilerError::*;
+
+        match self {
+            TopLevelNonSafe | ImpossibleNonMalleableCompilation | LimitsExceeded => None,
+            PolicyError(e) => Some(e),
+        }
+    }
+}
 
 #[doc(hidden)]
 impl From<policy::concrete::PolicyError> for CompilerError {

diff --git a/src/policy/concrete.rs b/src/policy/concrete.rs
@@ -132,7 +132,24 @@ impl fmt::Display for PolicyError {
     }
 }
 
-impl error::Error for PolicyError {}
+impl error::Error for PolicyError {
+    fn cause(&self) -> Option<&dyn error::Error> {
+        use self::PolicyError::*;
+
+        match self {
+            NonBinaryArgAnd
+            | NonBinaryArgOr
+            | IncorrectThresh
+            | ZeroTime
+            | TimeTooFar
+            | InsufficientArgsforAnd
+            | InsufficientArgsforOr
+            | EntailmentMaxTerminals
+            | HeightTimelockCombination
+            | DuplicatePubKeys => None,
+        }
+    }
+}
 
 impl<Pk: MiniscriptKey> Policy<Pk> {
     /// Flatten the [`Policy`] tree structure into a Vector of tuple `(leaf script, leaf probability)`

diff --git a/src/policy/mod.rs b/src/policy/mod.rs
@@ -81,7 +81,11 @@ impl fmt::Display for LiftError {
 
 impl error::Error for LiftError {
     fn cause(&self) -> Option<&dyn error::Error> {
-        None
+        use self::LiftError::*;
+
+        match self {
+            HeightTimelockCombination | BranchExceedResourceLimits => None,
+        }
     }
 }
 

diff --git a/src/psbt/mod.rs b/src/psbt/mod.rs
@@ -82,7 +82,16 @@ impl fmt::Display for Error {
     }
 }
 
-impl error::Error for Error {}
+impl error::Error for Error {
+    fn cause(&self) -> Option<&dyn error::Error> {
+        use self::Error::*;
+
+        match self {
+            InputError(e, _) => Some(e),
+            WrongInputCount { .. } | InputIdxOutofBounds { .. } => None,
+        }
+    }
+}
 
 /// Error type for Pbst Input
 #[derive(Debug)]
@@ -148,6 +157,32 @@ pub enum InputError {
     },
 }
 
+impl error::Error for InputError {
+    fn cause(&self) -> Option<&dyn error::Error> {
+        use self::InputError::*;
+
+        match self {
+            CouldNotSatisfyTr
+            | InvalidRedeemScript { .. }
+            | InvalidWitnessScript { .. }
+            | InvalidSignature { .. }
+            | MissingRedeemScript
+            | MissingWitness
+            | MissingPubkey
+            | MissingWitnessScript
+            | MissingUtxo
+            | NonEmptyWitnessScript
+            | NonEmptyRedeemScript
+            | NonStandardSighashType(_)
+            | WrongSighashFlag { .. } => None,
+            SecpErr(e) => Some(e),
+            KeyErr(e) => Some(e),
+            Interpreter(e) => Some(e),
+            MiniscriptError(e) => Some(e),
+        }
+    }
+}
+
 impl fmt::Display for InputError {
     fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
         match *self {
@@ -1069,7 +1104,16 @@ impl fmt::Display for UtxoUpdateError {
     }
 }
 
-impl error::Error for UtxoUpdateError {}
+impl error::Error for UtxoUpdateError {
+    fn cause(&self) -> Option<&dyn error::Error> {
+        use self::UtxoUpdateError::*;
+
+        match self {
+            IndexOutOfBounds(_, _) | MissingInputUtxo | UtxoCheck | MismatchedScriptPubkey => None,
+            DerivationError(e) => Some(e),
+        }
+    }
+}
 
 /// Return error type for [`PsbtExt::sighash_msg`]
 #[derive(Debug, PartialEq, Eq, PartialOrd, Ord, Hash, Clone, Copy)]
@@ -1110,7 +1154,21 @@ impl fmt::Display for SighashError {
     }
 }
 
-impl error::Error for SighashError {}
+impl error::Error for SighashError {
+    fn cause(&self) -> Option<&dyn error::Error> {
+        use self::SighashError::*;
+
+        match self {
+            IndexOutOfBounds(_, _)
+            | MissingInputUtxo
+            | MissingSpendUtxos
+            | InvalidSighashType
+            | MissingWitnessScript
+            | MissingRedeemScript => None,
+            SighashComputationError(e) => Some(e),
+        }
+    }
+}
 
 impl From<bitcoin::util::sighash::Error> for SighashError {
     fn from(e: bitcoin::util::sighash::Error) -> Self {