error: split up top-level error type into DecodeError and FinalizeError

Essentially, DecodeError is used when constructing a program from the
bit encoding and FinalizeError is used when constructing a program using
the library.

Along the way, tighten up some error returns for functions that return
e.g. crate::Error when they actually only ever yield type errors.

We drop the Error::Policy variant which was never used. The old Error
enum now has a single variant, which annoyingly is used by the
Haskell-generated code so we can't remove it without roundtripping
through upstream. Will try to do this before the next release but it's
not a huge priority.

In the next commit we'll add a ParseError which wraps DecodeError and
adds some string-encoding-related variants.

Author: apoelstra

jets-bench/src/data_structures.rs

@@ -9,7 +9,7 @@ use simplicity::{
     hashes::Hash,
     hex::FromHex,
     types::Final,
-    BitIter, Error, Value,
+    BitIter, EarlyEndOfStreamError, Value,
 };
 
 /// Engine to compute SHA256 hash function.
@@ -54,7 +54,7 @@ impl SimplicityCtx8 {
 /// # Panics:
 ///
 /// Panics if the length of the slice is >= 2^(n + 1) bytes
-pub fn var_len_buf_from_slice(v: &[u8], mut n: usize) -> Result<Value, Error> {
+pub fn var_len_buf_from_slice(v: &[u8], mut n: usize) -> Result<Value, EarlyEndOfStreamError> {
     // Simplicity consensus rule for n < 16 while reading buffers.
     assert!(n < 16);
     assert!(v.len() < (1 << (n + 1)));

src/lib.rs

@@ -79,76 +79,96 @@ pub fn leaf_version() -> elements::taproot::LeafVersion {
     elements::taproot::LeafVersion::from_u8(0xbe).expect("constant leaf version")
 }
 
-/// Error type for simplicity
+/// Error decoding a program from the bit encoding.
 #[non_exhaustive]
 #[derive(Debug)]
-pub enum Error {
+pub enum DecodeError {
     /// Decoder error
     Decode(decode::Error),
     /// A disconnect node was *not* populated at redeem time
     DisconnectRedeemTime,
     /// Type-checking error
     Type(types::Error),
-    // Execution error
-    Execution(bit_machine::ExecutionError),
-    /// Tried to parse a jet but the name wasn't recognized
-    InvalidJetName(String),
-    /// Policy error
-    #[cfg(feature = "elements")]
-    Policy(policy::Error),
 }
 
-impl fmt::Display for Error {
+impl fmt::Display for DecodeError {
     fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
         match self {
-            Error::Decode(ref e) => fmt::Display::fmt(e, f),
-            Error::DisconnectRedeemTime => {
+            Self::Decode(ref e) => fmt::Display::fmt(e, f),
+            Self::DisconnectRedeemTime => {
                 f.write_str("disconnect node had one child (redeem time); must have two")
             }
-            Error::Type(ref e) => fmt::Display::fmt(e, f),
-            Error::Execution(ref e) => fmt::Display::fmt(e, f),
-            Error::InvalidJetName(s) => write!(f, "unknown jet `{}`", s),
-            #[cfg(feature = "elements")]
-            Error::Policy(ref e) => fmt::Display::fmt(e, f),
+            Self::Type(ref e) => fmt::Display::fmt(e, f),
         }
     }
 }
 
-impl std::error::Error for Error {
+impl std::error::Error for DecodeError {
     fn source(&self) -> Option<&(dyn std::error::Error + 'static)> {
-        match *self {
-            Error::Decode(ref e) => Some(e),
-            Error::DisconnectRedeemTime => None,
-            Error::Type(ref e) => Some(e),
-            Error::Execution(ref e) => Some(e),
-            Error::InvalidJetName(..) => None,
-            #[cfg(feature = "elements")]
-            Error::Policy(ref e) => Some(e),
+        match self {
+            Self::Decode(ref e) => Some(e),
+            Self::DisconnectRedeemTime => None,
+            Self::Type(ref e) => Some(e),
         }
     }
 }
 
-impl From<crate::decode::Error> for Error {
-    fn from(e: crate::decode::Error) -> Error {
-        Error::Decode(e)
+/// Error finalizing a program (i.e. typechecking and pruning).
+#[non_exhaustive]
+#[derive(Debug)]
+pub enum FinalizeError {
+    /// A disconnect node was *not* populated at redeem time
+    DisconnectRedeemTime,
+    // Execution error
+    Execution(bit_machine::ExecutionError),
+    /// Type-checking error
+    Type(types::Error),
+}
+
+impl fmt::Display for FinalizeError {
+    fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
+        match self {
+            Self::DisconnectRedeemTime => {
+                f.write_str("disconnect node had one child (redeem time); must have two")
+            }
+            Self::Execution(ref e) => fmt::Display::fmt(e, f),
+            Self::Type(ref e) => fmt::Display::fmt(e, f),
+        }
     }
 }
 
-impl From<EarlyEndOfStreamError> for Error {
-    fn from(e: EarlyEndOfStreamError) -> Error {
-        Error::Decode(e.into())
+impl std::error::Error for FinalizeError {
+    fn source(&self) -> Option<&(dyn std::error::Error + 'static)> {
+        match self {
+            Self::DisconnectRedeemTime => None,
+            Self::Execution(ref e) => Some(e),
+            Self::Type(ref e) => Some(e),
+        }
     }
 }
 
-impl From<crate::types::Error> for Error {
-    fn from(e: crate::types::Error) -> Error {
-        Error::Type(e)
+/// Error type for simplicity
+// FIXME we should collapse this error to its single variant; but need to update
+// autogenerated code to do so, so we leave it be for now.
+#[non_exhaustive]
+#[derive(Debug)]
+pub enum Error {
+    /// Tried to parse a jet but the name wasn't recognized
+    InvalidJetName(String),
+}
+
+impl fmt::Display for Error {
+    fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
+        match self {
+            Error::InvalidJetName(s) => write!(f, "unknown jet `{}`", s),
+        }
     }
 }
 
-#[cfg(feature = "elements")]
-impl From<policy::Error> for Error {
-    fn from(e: policy::Error) -> Error {
-        Error::Policy(e)
+impl std::error::Error for Error {
+    fn source(&self) -> Option<&(dyn std::error::Error + 'static)> {
+        match *self {
+            Error::InvalidJetName(..) => None,
+        }
     }
 }

src/node/commit.rs

@@ -4,7 +4,7 @@ use crate::dag::{DagLike, MaxSharing, NoSharing, PostOrderIterItem};
 use crate::jet::Jet;
 use crate::types::arrow::{Arrow, FinalArrow};
 use crate::{encode, types, Value};
-use crate::{Amr, BitIter, BitWriter, Cmr, Error, Ihr, Imr};
+use crate::{Amr, BitIter, BitWriter, Cmr, DecodeError, Ihr, Imr};
 
 use super::{
     Construct, ConstructData, ConstructNode, Constructible, Converter, Inner, Marker, NoDisconnect,
@@ -248,15 +248,17 @@ impl<J: Jet> CommitNode<J> {
     /// or the witness is provided by other means.
     ///
     /// If the serialization contains the witness data, then use [`RedeemNode::decode()`].
-    pub fn decode<I: Iterator<Item = u8>>(bits: BitIter<I>) -> Result<Arc<Self>, Error> {
+    pub fn decode<I: Iterator<Item = u8>>(bits: BitIter<I>) -> Result<Arc<Self>, DecodeError> {
+        use crate::decode;
+
         // 1. Decode program with out witnesses.
-        let construct = crate::ConstructNode::decode(bits)?;
-        let program = construct.finalize_types()?;
+        let construct = crate::ConstructNode::decode(bits).map_err(DecodeError::Decode)?;
+        let program = construct.finalize_types().map_err(DecodeError::Type)?;
         // 2. Do sharing check, using incomplete IHRs
         if program.as_ref().is_shared_as::<MaxSharing<Commit<J>>>() {
             Ok(program)
         } else {
-            Err(Error::Decode(crate::decode::Error::SharingNotMaximal))
+            Err(DecodeError::Decode(decode::Error::SharingNotMaximal))
         }
     }
 

src/node/construct.rs

@@ -3,7 +3,7 @@
 use crate::dag::{InternalSharing, PostOrderIterItem};
 use crate::jet::Jet;
 use crate::types::{self, arrow::Arrow};
-use crate::{encode, BitIter, BitWriter, Cmr, FailEntropy, RedeemNode, Value, Word};
+use crate::{encode, BitIter, BitWriter, Cmr, FailEntropy, FinalizeError, RedeemNode, Value, Word};
 
 use std::io;
 use std::marker::PhantomData;
@@ -73,19 +73,20 @@ impl<J: Jet> ConstructNode<J> {
     /// certainly what you want, since the resulting `CommitNode` cannot be further
     /// composed, and needs to be 1->1 to go on-chain. But if you don't, call
     /// [`Self::finalize_types_non_program`] instead.
-    pub fn finalize_types(&self) -> Result<Arc<CommitNode<J>>, crate::Error> {
+    pub fn finalize_types(&self) -> Result<Arc<CommitNode<J>>, types::Error> {
         self.set_arrow_to_program()?;
         self.finalize_types_non_program()
     }
 
     /// Convert a [`ConstructNode`] to a [`CommitNode`] by finalizing all of the types.
     ///
     /// Does *not* sets the source and target type of this node to unit.
-    pub fn finalize_types_non_program(&self) -> Result<Arc<CommitNode<J>>, crate::Error> {
+    pub fn finalize_types_non_program(&self) -> Result<Arc<CommitNode<J>>, types::Error> {
         struct FinalizeTypes<J: Jet>(PhantomData<J>);
 
         impl<J: Jet> Converter<Construct<J>, Commit<J>> for FinalizeTypes<J> {
-            type Error = crate::Error;
+            type Error = types::Error;
+
             fn convert_witness(
                 &mut self,
                 _: &PostOrderIterItem<&ConstructNode<J>>,
@@ -109,9 +110,7 @@ impl<J: Jet> ConstructNode<J> {
                 inner: Inner<&Arc<CommitNode<J>>, J, &NoDisconnect, &NoWitness>,
             ) -> Result<Arc<CommitData<J>>, Self::Error> {
                 let converted_data = inner.map(|node| node.cached_data());
-                CommitData::new(&data.node.data.arrow, converted_data)
-                    .map(Arc::new)
-                    .map_err(crate::Error::from)
+                CommitData::new(&data.node.data.arrow, converted_data).map(Arc::new)
             }
         }
 
@@ -130,11 +129,11 @@ impl<J: Jet> ConstructNode<J> {
     /// ## See
     ///
     /// [`RedeemNode::prune`]
-    pub fn finalize_unpruned(&self) -> Result<Arc<RedeemNode<J>>, crate::Error> {
+    pub fn finalize_unpruned(&self) -> Result<Arc<RedeemNode<J>>, FinalizeError> {
         struct Finalizer<J>(PhantomData<J>);
 
         impl<J: Jet> Converter<Construct<J>, Redeem<J>> for Finalizer<J> {
-            type Error = crate::Error;
+            type Error = FinalizeError;
 
             fn convert_witness(
                 &mut self,
@@ -159,7 +158,12 @@ impl<J: Jet> ConstructNode<J> {
                     // didn't provide any witness data. However, this is only the case for the
                     // most trivial programs. The only place where we must be careful is our
                     // unit tests, which tend to include these kinds of trivial programs.
-                    let ty = data.node.arrow().target.finalize()?;
+                    let ty = data
+                        .node
+                        .arrow()
+                        .target
+                        .finalize()
+                        .map_err(FinalizeError::Type)?;
                     Ok(Value::zero(&ty))
                 }
             }
@@ -173,7 +177,7 @@ impl<J: Jet> ConstructNode<J> {
                 if let Some(child) = maybe_converted {
                     Ok(Arc::clone(child))
                 } else {
-                    Err(crate::Error::DisconnectRedeemTime)
+                    Err(FinalizeError::DisconnectRedeemTime)
                 }
             }
 
@@ -187,7 +191,7 @@ impl<J: Jet> ConstructNode<J> {
                     .map_disconnect(|node| node.cached_data())
                     .map_witness(Value::shallow_clone);
                 Ok(Arc::new(RedeemData::new(
-                    data.node.arrow().finalize()?,
+                    data.node.arrow().finalize().map_err(FinalizeError::Type)?,
                     converted_data,
                 )))
             }
@@ -211,9 +215,9 @@ impl<J: Jet> ConstructNode<J> {
     pub fn finalize_pruned(
         &self,
         env: &J::Environment,
-    ) -> Result<Arc<RedeemNode<J>>, crate::Error> {
+    ) -> Result<Arc<RedeemNode<J>>, FinalizeError> {
         let unpruned = self.finalize_unpruned()?;
-        unpruned.prune(env).map_err(crate::Error::Execution)
+        unpruned.prune(env).map_err(FinalizeError::Execution)
     }
 
     /// Decode a Simplicity expression from bits, without witness data.
@@ -409,7 +413,7 @@ mod tests {
 
         assert!(matches!(
             node.finalize_types_non_program(),
-            Err(crate::Error::Type(types::Error::OccursCheck { .. })),
+            Err(types::Error::OccursCheck { .. }),
         ));
     }
 
@@ -430,7 +434,7 @@ mod tests {
 
         assert!(matches!(
             comp2.finalize_types_non_program(),
-            Err(crate::Error::Type(types::Error::OccursCheck { .. })),
+            Err(types::Error::OccursCheck { .. }),
         ));
     }
 
@@ -458,7 +462,7 @@ mod tests {
 
         assert!(matches!(
             comp8.finalize_types_non_program(),
-            Err(crate::Error::Type(types::Error::OccursCheck { .. })),
+            Err(types::Error::OccursCheck { .. }),
         ));
     }
 
@@ -560,9 +564,6 @@ mod tests {
 
     #[test]
     fn regression_286_2() {
-        use crate::types::Error as TypeError;
-        use crate::Error;
-
         // This one is smaller because it starts with a witness node which has a large type.
         // This is a bit easier to grok but can't be serialized as a complete/valid program
         // without providing the witness data, which limits its ability to share with the
@@ -586,6 +587,6 @@ mod tests {
 
         // In #286 we incorrectly succeed finalizing the types, and then encode a bad program.
         let err = c10.finalize_types().unwrap_err();
-        assert!(matches!(err, Error::Type(TypeError::OccursCheck { .. })));
+        assert!(matches!(err, types::Error::OccursCheck { .. }));
     }
 }

src/node/convert.rs

@@ -169,7 +169,7 @@ impl<W: Iterator<Item = Value>> SimpleFinalizer<W> {
 }
 
 impl<W: Iterator<Item = Value>, J: Jet> Converter<Commit<J>, Redeem<J>> for SimpleFinalizer<W> {
-    type Error = crate::Error;
+    type Error = crate::FinalizeError;
 
     fn convert_witness(
         &mut self,
@@ -188,7 +188,7 @@ impl<W: Iterator<Item = Value>, J: Jet> Converter<Commit<J>, Redeem<J>> for Simp
         _: Option<&Arc<RedeemNode<J>>>,
         _: &NoDisconnect,
     ) -> Result<Arc<RedeemNode<J>>, Self::Error> {
-        Err(crate::Error::DisconnectRedeemTime)
+        Err(crate::FinalizeError::DisconnectRedeemTime)
     }
 
     fn convert_data(