miniscript: move lexer errors into their own type

Also clean up the names and format messages for them.

Author: apoelstra

src/lib.rs

@@ -131,8 +131,6 @@ mod util;
 use core::{fmt, hash, str};
 
 use bitcoin::hashes::{hash160, ripemd160, sha256, Hash};
-use bitcoin::hex::DisplayHex;
-use bitcoin::{script, Opcode};
 
 pub use crate::blanket_traits::FromStrKey;
 pub use crate::descriptor::{DefiniteDescriptorKey, Descriptor, DescriptorPublicKey};
@@ -436,15 +434,8 @@ pub trait ForEachKey<Pk: MiniscriptKey> {
 
 #[derive(Debug)]
 pub enum Error {
-    /// Opcode appeared which is not part of the script subset
-    InvalidOpcode(Opcode),
-    /// Some opcode occurred followed by `OP_VERIFY` when it had
-    /// a `VERIFY` version that should have been used instead
-    NonMinimalVerify(String),
-    /// Push was illegal in some context
-    InvalidPush(Vec<u8>),
-    /// rust-bitcoin script error
-    Script(script::Error),
+    /// Error when lexing a bitcoin Script.
+    ScriptLexer(crate::miniscript::lex::Error),
     /// rust-bitcoin address error
     AddrError(bitcoin::address::ParseError),
     /// rust-bitcoin p2sh address error
@@ -519,12 +510,7 @@ const MAX_RECURSION_DEPTH: u32 = 402;
 impl fmt::Display for Error {
     fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
         match *self {
-            Error::InvalidOpcode(op) => write!(f, "invalid opcode {}", op),
-            Error::NonMinimalVerify(ref tok) => write!(f, "{} VERIFY", tok),
-            Error::InvalidPush(ref push) => {
-                write!(f, "invalid push {:x}", push.as_hex())
-            },
-            Error::Script(ref e) => fmt::Display::fmt(e, f),
+            Error::ScriptLexer(ref e) => e.fmt(f),
             Error::AddrError(ref e) => fmt::Display::fmt(e, f),
             Error::AddrP2shError(ref e) => fmt::Display::fmt(e, f),
             Error::UnexpectedStart => f.write_str("unexpected start of script"),
@@ -576,10 +562,7 @@ impl std::error::Error for Error {
         use self::Error::*;
 
         match self {
-            InvalidOpcode(_)
-            | NonMinimalVerify(_)
-            | InvalidPush(_)
-            | UnexpectedStart
+            UnexpectedStart
             | Unexpected(_)
             | UnknownWrapper(_)
             | NonTopLevel(_)
@@ -593,7 +576,7 @@ impl std::error::Error for Error {
             | BareDescriptorAddr
             | TrNoScriptCode
             | MultipathDescLenMismatch => None,
-            Script(e) => Some(e),
+            ScriptLexer(e) => Some(e),
             AddrError(e) => Some(e),
             AddrP2shError(e) => Some(e),
             Secp(e) => Some(e),
@@ -614,6 +597,11 @@ impl std::error::Error for Error {
     }
 }
 
+#[doc(hidden)]
+impl From<miniscript::lex::Error> for Error {
+    fn from(e: miniscript::lex::Error) -> Error { Error::ScriptLexer(e) }
+}
+
 #[doc(hidden)]
 impl From<miniscript::types::Error> for Error {
     fn from(e: miniscript::types::Error) -> Error { Error::TypeCheck(e.to_string()) }

src/miniscript/lex.rs

@@ -10,7 +10,6 @@ use core::fmt;
 use bitcoin::blockdata::{opcodes, script};
 use bitcoin::hex::DisplayHex as _;
 
-use super::Error;
 use crate::prelude::*;
 
 /// Atom of a tokenized version of a script
@@ -187,7 +186,7 @@ pub fn lex(script: &'_ script::Script) -> Result<Vec<Token>, Error> {
                     Some(op @ &Token::Equal)
                     | Some(op @ &Token::CheckSig)
                     | Some(op @ &Token::CheckMultiSig) => {
-                        return Err(Error::NonMinimalVerify(format!("{:?}", op)))
+                        return Err(Error::NonMinimalVerify(*op));
                     }
                     _ => {}
                 }
@@ -220,8 +219,8 @@ pub fn lex(script: &'_ script::Script) -> Result<Vec<Token>, Error> {
                         Ok(v) if v >= 0 => {
                             ret.push(Token::Num(v as u32));
                         }
-                        Ok(_) => return Err(Error::InvalidPush(bytes.to_owned().into())),
-                        Err(e) => return Err(Error::Script(e)),
+                        Ok(n) => return Err(Error::NegativeInt { bytes: bytes.to_owned(), n }),
+                        Err(err) => return Err(Error::InvalidInt { bytes: bytes.to_owned(), err }),
                     }
                 }
             }
@@ -281,3 +280,53 @@ pub fn lex(script: &'_ script::Script) -> Result<Vec<Token>, Error> {
     }
     Ok(ret)
 }
+
+/// Lexer error.
+#[derive(Debug, Clone, PartialEq, Eq)]
+pub enum Error {
+    /// Parsed a negative number.
+    InvalidInt {
+        /// The bytes of the push that were attempted to be parsed.
+        bytes: bitcoin::script::PushBytesBuf,
+        /// The error that occured.
+        err: bitcoin::script::Error,
+    },
+    /// Parsed an opcode outside of the Miniscript language.
+    InvalidOpcode(bitcoin::Opcode),
+    /// Parsed a negative number.
+    NegativeInt {
+        /// The bytes of the push that were parsed to a negative number.
+        bytes: bitcoin::script::PushBytesBuf,
+        /// The resulting number.
+        n: i64,
+    },
+    /// Non-minimal verify (e.g. `CHECKSIG VERIFY` in place of `CHECKSIGVERIFY`).
+    NonMinimalVerify(Token),
+    /// Error iterating through script.
+    Script(bitcoin::script::Error),
+}
+
+impl fmt::Display for Error {
+    fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
+        match *self {
+            Self::Script(ref e) => e.fmt(f),
+            Self::InvalidInt { ref bytes, ref err } => write!(f, "push {} of length {} is not a key, hash or minimal integer: {}", bytes.as_bytes().as_hex(), bytes.len(), err),
+            Self::InvalidOpcode(ref op) => write!(f, "found opcode {} which does not occur in Miniscript", op),
+            Self::NegativeInt { ref bytes, n } => write!(f, "push {} of length {} parses as a negative number {} which does not occur in Miniscript", bytes.as_bytes().as_hex(), bytes.len(), n),
+            Self::NonMinimalVerify(ref op) => write!(f, "found {} VERIFY (should be one opcode, {}VERIFY)", op, op),
+        }
+    }
+}
+
+#[cfg(feature = "std")]
+impl std::error::Error for Error {
+    fn cause(&self) -> Option<&(dyn std::error::Error + 'static)> {
+        match *self {
+            Self::InvalidInt { ref err, .. } => Some(err),
+            Self::InvalidOpcode(..) => None,
+            Self::NegativeInt { .. } => None,
+            Self::NonMinimalVerify(..) => None,
+            Self::Script(ref e) => Some(e),
+        }
+    }
+}