Skip to content

Commit

Permalink
Re-order error code blocks
Browse files Browse the repository at this point in the history
Audit the whole codebase and re-order code around error handling so that
it is all in a uniform layout in the file, specifically

1. Error declaration
2. impl Display ...
3. impl error::Error ...
4. All the From impls

Refactor only, no logic changes. All changes are just moving blocks of
code around.
  • Loading branch information
tcharding committed May 20, 2022
1 parent 2eac9de commit a5db0f0
Show file tree
Hide file tree
Showing 8 changed files with 206 additions and 206 deletions.
144 changes: 72 additions & 72 deletions src/interpreter/error.rs
Original file line number Diff line number Diff line change
Expand Up @@ -118,78 +118,6 @@ pub enum Error {
VerifyFailed,
}

/// A type of representing which keys errored during interpreter checksig evaluation
// Note that we can't use BitcoinKey because it is not public
#[derive(Clone, Copy, Debug, PartialEq, Eq, PartialOrd, Ord, Hash)]
pub enum PkEvalErrInner {
/// Full Key
FullKey(bitcoin::PublicKey),
/// XOnly Key
XOnlyKey(bitcoin::XOnlyPublicKey),
}

impl From<BitcoinKey> for PkEvalErrInner {
fn from(pk: BitcoinKey) -> Self {
match pk {
BitcoinKey::Fullkey(pk) => PkEvalErrInner::FullKey(pk),
BitcoinKey::XOnlyPublicKey(xpk) => PkEvalErrInner::XOnlyKey(xpk),
}
}
}

impl fmt::Display for PkEvalErrInner {
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
match self {
PkEvalErrInner::FullKey(pk) => pk.fmt(f),
PkEvalErrInner::XOnlyKey(xpk) => xpk.fmt(f),
}
}
}

#[doc(hidden)]
impl From<secp256k1::Error> for Error {
fn from(e: secp256k1::Error) -> Error {
Error::Secp(e)
}
}

#[doc(hidden)]
impl From<bitcoin::util::sighash::Error> for Error {
fn from(e: bitcoin::util::sighash::Error) -> Error {
Error::SighashError(e)
}
}

#[doc(hidden)]
impl From<bitcoin::EcdsaSigError> for Error {
fn from(e: bitcoin::EcdsaSigError) -> Error {
Error::EcdsaSig(e)
}
}

#[doc(hidden)]
impl From<bitcoin::SchnorrSigError> for Error {
fn from(e: bitcoin::SchnorrSigError) -> Error {
Error::SchnorrSig(e)
}
}

#[doc(hidden)]
impl From<crate::Error> for Error {
fn from(e: crate::Error) -> Error {
Error::Miniscript(e)
}
}

impl error::Error for Error {
fn cause(&self) -> Option<&dyn error::Error> {
match *self {
Error::Secp(ref err) => Some(err),
ref x => Some(x),
}
}
}

impl fmt::Display for Error {
fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
match *self {
Expand Down Expand Up @@ -263,3 +191,75 @@ 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),
}
}
}

#[doc(hidden)]
impl From<secp256k1::Error> for Error {
fn from(e: secp256k1::Error) -> Error {
Error::Secp(e)
}
}

#[doc(hidden)]
impl From<bitcoin::util::sighash::Error> for Error {
fn from(e: bitcoin::util::sighash::Error) -> Error {
Error::SighashError(e)
}
}

#[doc(hidden)]
impl From<bitcoin::EcdsaSigError> for Error {
fn from(e: bitcoin::EcdsaSigError) -> Error {
Error::EcdsaSig(e)
}
}

#[doc(hidden)]
impl From<bitcoin::SchnorrSigError> for Error {
fn from(e: bitcoin::SchnorrSigError) -> Error {
Error::SchnorrSig(e)
}
}

#[doc(hidden)]
impl From<crate::Error> for Error {
fn from(e: crate::Error) -> Error {
Error::Miniscript(e)
}
}

/// A type of representing which keys errored during interpreter checksig evaluation
// Note that we can't use BitcoinKey because it is not public
#[derive(Clone, Copy, Debug, PartialEq, Eq, PartialOrd, Ord, Hash)]
pub enum PkEvalErrInner {
/// Full Key
FullKey(bitcoin::PublicKey),
/// XOnly Key
XOnlyKey(bitcoin::XOnlyPublicKey),
}

impl From<BitcoinKey> for PkEvalErrInner {
fn from(pk: BitcoinKey) -> Self {
match pk {
BitcoinKey::Fullkey(pk) => PkEvalErrInner::FullKey(pk),
BitcoinKey::XOnlyPublicKey(xpk) => PkEvalErrInner::XOnlyKey(xpk),
}
}
}

impl fmt::Display for PkEvalErrInner {
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
match self {
PkEvalErrInner::FullKey(pk) => pk.fmt(f),
PkEvalErrInner::XOnlyKey(xpk) => xpk.fmt(f),
}
}
}
118 changes: 59 additions & 59 deletions src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -575,65 +575,6 @@ pub enum Error {
TrNoExplicitScript,
}

#[doc(hidden)]
impl<Pk, Ctx> From<miniscript::types::Error<Pk, Ctx>> for Error
where
Pk: MiniscriptKey,
Ctx: ScriptContext,
{
fn from(e: miniscript::types::Error<Pk, Ctx>) -> Error {
Error::TypeCheck(e.to_string())
}
}

#[doc(hidden)]
impl From<policy::LiftError> for Error {
fn from(e: policy::LiftError) -> Error {
Error::LiftError(e)
}
}

#[doc(hidden)]
impl From<miniscript::context::ScriptContextError> for Error {
fn from(e: miniscript::context::ScriptContextError) -> Error {
Error::ContextError(e)
}
}

#[doc(hidden)]
impl From<miniscript::analyzable::AnalysisError> for Error {
fn from(e: miniscript::analyzable::AnalysisError) -> Error {
Error::AnalysisError(e)
}
}

#[doc(hidden)]
impl From<bitcoin::secp256k1::Error> for Error {
fn from(e: bitcoin::secp256k1::Error) -> Error {
Error::Secp(e)
}
}

#[doc(hidden)]
impl From<bitcoin::util::address::Error> for Error {
fn from(e: bitcoin::util::address::Error) -> Error {
Error::AddrError(e)
}
}

fn errstr(s: &str) -> Error {
Error::Unexpected(s.to_owned())
}

impl error::Error for Error {
fn cause(&self) -> Option<&dyn error::Error> {
match *self {
Error::BadPubkey(ref e) => Some(e),
_ => None,
}
}
}

// https://github.com/sipa/miniscript/pull/5 for discussion on this number
const MAX_RECURSION_DEPTH: u32 = 402;
// https://github.com/bitcoin/bips/blob/master/bip-0141.mediawiki
Expand Down Expand Up @@ -717,6 +658,61 @@ 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,
}
}
}

#[doc(hidden)]
impl<Pk, Ctx> From<miniscript::types::Error<Pk, Ctx>> for Error
where
Pk: MiniscriptKey,
Ctx: ScriptContext,
{
fn from(e: miniscript::types::Error<Pk, Ctx>) -> Error {
Error::TypeCheck(e.to_string())
}
}

#[doc(hidden)]
impl From<policy::LiftError> for Error {
fn from(e: policy::LiftError) -> Error {
Error::LiftError(e)
}
}

#[doc(hidden)]
impl From<miniscript::context::ScriptContextError> for Error {
fn from(e: miniscript::context::ScriptContextError) -> Error {
Error::ContextError(e)
}
}

#[doc(hidden)]
impl From<miniscript::analyzable::AnalysisError> for Error {
fn from(e: miniscript::analyzable::AnalysisError) -> Error {
Error::AnalysisError(e)
}
}

#[doc(hidden)]
impl From<bitcoin::secp256k1::Error> for Error {
fn from(e: bitcoin::secp256k1::Error) -> Error {
Error::Secp(e)
}
}

#[doc(hidden)]
impl From<bitcoin::util::address::Error> for Error {
fn from(e: bitcoin::util::address::Error) -> Error {
Error::AddrError(e)
}
}

#[doc(hidden)]
#[cfg(feature = "compiler")]
impl From<crate::policy::compiler::CompilerError> for Error {
Expand All @@ -732,6 +728,10 @@ impl From<policy::concrete::PolicyError> for Error {
}
}

fn errstr(s: &str) -> Error {
Error::Unexpected(s.to_owned())
}

/// The size of an encoding of a number in Script
pub fn script_num_size(n: usize) -> usize {
match n {
Expand Down
32 changes: 16 additions & 16 deletions src/miniscript/decode.rs
Original file line number Diff line number Diff line change
Expand Up @@ -41,15 +41,6 @@ pub trait ParseableKey: Sized + ToPublicKey + private::Sealed {
fn from_slice(sl: &[u8]) -> Result<Self, KeyParseError>;
}

/// Decoding error while parsing keys
#[derive(Debug, PartialEq, Eq, PartialOrd, Ord, Hash)]
pub enum KeyParseError {
/// Bitcoin PublicKey parse error
FullKeyParseError(bitcoin::util::key::Error),
/// Xonly key parse Error
XonlyKeyParseError(bitcoin::secp256k1::Error),
}

impl ParseableKey for bitcoin::PublicKey {
fn from_slice(sl: &[u8]) -> Result<Self, KeyParseError> {
bitcoin::PublicKey::from_slice(sl).map_err(KeyParseError::FullKeyParseError)
Expand All @@ -63,13 +54,13 @@ impl ParseableKey for bitcoin::secp256k1::XOnlyPublicKey {
}
}

impl error::Error for KeyParseError {
fn cause(&self) -> Option<&(dyn error::Error + 'static)> {
match self {
KeyParseError::FullKeyParseError(e) => Some(e),
KeyParseError::XonlyKeyParseError(e) => Some(e),
}
}
/// Decoding error while parsing keys
#[derive(Debug, PartialEq, Eq, PartialOrd, Ord, Hash)]
pub enum KeyParseError {
/// Bitcoin PublicKey parse error
FullKeyParseError(bitcoin::util::key::Error),
/// Xonly key parse Error
XonlyKeyParseError(bitcoin::secp256k1::Error),
}

impl fmt::Display for KeyParseError {
Expand All @@ -81,6 +72,15 @@ impl fmt::Display for KeyParseError {
}
}

impl error::Error for KeyParseError {
fn cause(&self) -> Option<&(dyn error::Error + 'static)> {
match self {
KeyParseError::FullKeyParseError(e) => Some(e),
KeyParseError::XonlyKeyParseError(e) => Some(e),
}
}
}

/// Private Mod to prevent downstream from implementing this public trait
mod private {

Expand Down
12 changes: 6 additions & 6 deletions src/miniscript/types/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -104,12 +104,6 @@ pub struct Error<Pk: MiniscriptKey, Ctx: ScriptContext> {
pub error: ErrorKind,
}

impl<Pk: MiniscriptKey, Ctx: ScriptContext> error::Error for Error<Pk, Ctx> {
fn cause(&self) -> Option<&dyn error::Error> {
None
}
}

impl<Pk: MiniscriptKey, Ctx: ScriptContext> fmt::Display for Error<Pk, Ctx> {
fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
match self.error {
Expand Down Expand Up @@ -219,6 +213,12 @@ impl<Pk: MiniscriptKey, Ctx: ScriptContext> fmt::Display for Error<Pk, Ctx> {
}
}

impl<Pk: MiniscriptKey, Ctx: ScriptContext> error::Error for Error<Pk, Ctx> {
fn cause(&self) -> Option<&dyn error::Error> {
None
}
}

/// Structure representing the type of a Miniscript fragment, including all
/// properties relevant to the main codebase
#[derive(Copy, Clone, PartialEq, Eq, PartialOrd, Ord, Debug, Hash)]
Expand Down
4 changes: 2 additions & 2 deletions src/policy/compiler.rs
Original file line number Diff line number Diff line change
Expand Up @@ -62,8 +62,6 @@ pub enum CompilerError {
PolicyError(policy::concrete::PolicyError),
}

impl error::Error for CompilerError {}

impl fmt::Display for CompilerError {
fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
match *self {
Expand All @@ -81,6 +79,8 @@ impl fmt::Display for CompilerError {
}
}

impl error::Error for CompilerError {}

#[doc(hidden)]
impl From<policy::concrete::PolicyError> for CompilerError {
fn from(e: policy::concrete::PolicyError) -> CompilerError {
Expand Down
Loading

0 comments on commit a5db0f0

Please sign in to comment.