Preserve richer errors on the Rust side

For now, the underlying errors are discarded when comparing against the
C++ results, but there are corresponding changes on the C++ side in a
separate branch.

fuzz/fuzz_targets/compare.rs

@@ -20,5 +20,6 @@ fuzz_target!(|tup: (i64, bool, &[u8], &[u8], u32)| {
         sig,
         testing::repair_flags(VerificationFlags::from_bits_truncate(flags)),
     );
-    assert_eq!(ret.0, ret.1);
+    assert_eq!(ret.0, ret.1.clone().map_err(testing::normalize_error),
+               "original Rust result: {:?}", ret.1);
 });

src/lib.rs

@@ -24,11 +24,11 @@ pub use zcash_script::*;
 /// A tag to indicate that the C++ implementation of zcash_script should be used.
 pub enum Cxx {}
 
-impl From<zcash_script_error_t> for Error {
+impl<'a> From<zcash_script_error_t> for Error<'a> {
     #[allow(non_upper_case_globals)]
-    fn from(err_code: zcash_script_error_t) -> Error {
+    fn from(err_code: zcash_script_error_t) -> Self {
         match err_code {
-            zcash_script_error_t_zcash_script_ERR_OK => Error::Ok,
+            zcash_script_error_t_zcash_script_ERR_OK => Error::Ok(None),
             zcash_script_error_t_zcash_script_ERR_VERIFY_SCRIPT => Error::VerifyScript,
             unknown => Error::Unknown(unknown.into()),
         }
@@ -64,14 +64,14 @@ extern "C" fn sighash_callback(
 
 /// This steals a bit of the wrapper code from zebra_script, to provide the API that they want.
 impl ZcashScript for Cxx {
-    fn verify_callback(
+    fn verify_callback<'a>(
         sighash: SighashCalculator,
         lock_time: i64,
         is_final: bool,
         script_pub_key: &[u8],
         signature_script: &[u8],
         flags: VerificationFlags,
-    ) -> Result<(), Error> {
+    ) -> Result<(), Error<'a>> {
         let mut err = 0;
 
         // SAFETY: The `script` fields are created from a valid Rust `slice`.
@@ -131,14 +131,14 @@ fn check_legacy_sigop_count_script<T: ZcashScript, U: ZcashScript>(
 /// Runs both the C++ and Rust implementations of `ZcashScript::verify_callback` and returns both
 /// results. This is more useful for testing than the impl that logs a warning if the results differ
 /// and always returns the C++ result.
-pub fn check_verify_callback<T: ZcashScript, U: ZcashScript>(
+pub fn check_verify_callback<'a, T: ZcashScript, U: ZcashScript>(
     sighash: SighashCalculator,
     lock_time: i64,
     is_final: bool,
     script_pub_key: &[u8],
     script_sig: &[u8],
     flags: VerificationFlags,
-) -> (Result<(), Error>, Result<(), Error>) {
+) -> (Result<(), Error<'a>>, Result<(), Error<'a>>) {
     (
         T::verify_callback(
             sighash,
@@ -173,14 +173,14 @@ impl<T: ZcashScript, U: ZcashScript> ZcashScript for (T, U) {
         cxx
     }
 
-    fn verify_callback(
+    fn verify_callback<'a>(
         sighash: SighashCalculator,
         lock_time: i64,
         is_final: bool,
         script_pub_key: &[u8],
         script_sig: &[u8],
         flags: VerificationFlags,
-    ) -> Result<(), Error> {
+    ) -> Result<(), Error<'a>> {
         let (cxx, rust) = check_verify_callback::<T, U>(
             sighash,
             lock_time,
@@ -207,6 +207,14 @@ impl<T: ZcashScript, U: ZcashScript> ZcashScript for (T, U) {
 pub mod testing {
     use super::*;
 
+    /// Convert errors that don’t exist in the C++ code into the cases that do.
+    pub fn normalize_error(err: Error) -> Error {
+        match err {
+            Error::Ok(Some(_)) => Error::Ok(None),
+            _ => err,
+        }
+    }
+
     /// Ensures that flags represent a supported state. This avoids crashes in the C++ code, which
     /// break various tests.
     pub fn repair_flags(flags: VerificationFlags) -> VerificationFlags {
@@ -271,7 +279,7 @@ mod tests {
             flags,
         );
 
-        assert_eq!(ret.0, ret.1);
+        assert_eq!(ret.0, ret.1.map_err(normalize_error));
         assert!(ret.0.is_ok());
     }
 
@@ -292,8 +300,12 @@ mod tests {
             flags,
         );
 
-        assert_eq!(ret.0, ret.1);
-        assert_eq!(ret.0, Err(Error::Ok));
+        assert_eq!(ret.0, ret.1.clone().map_err(normalize_error));
+        // Checks the Rust result, because we have more information on the Rust side.
+        assert_eq!(
+            ret.1,
+            Err(Error::Ok(Some(script_error::ScriptError::EvalFalse)))
+        );
     }
 
     #[test]
@@ -313,8 +325,12 @@ mod tests {
             flags,
         );
 
-        assert_eq!(ret.0, ret.1);
-        assert_eq!(ret.0, Err(Error::Ok));
+        assert_eq!(ret.0, ret.1.clone().map_err(normalize_error));
+        // Checks the Rust result, because we have more information on the Rust side.
+        assert_eq!(
+            ret.1,
+            Err(Error::Ok(Some(script_error::ScriptError::EvalFalse)))
+        );
     }
 
     proptest! {
@@ -340,7 +356,8 @@ mod tests {
                 &sig[..],
                 repair_flags(VerificationFlags::from_bits_truncate(flags)),
             );
-            prop_assert_eq!(ret.0, ret.1);
+            prop_assert_eq!(ret.0, ret.1.clone().map_err(normalize_error),
+                            "original Rust result: {:?}", ret.1);
         }
 
         /// Similar to `test_arbitrary_scripts`, but ensures the `sig` only contains pushes.
@@ -363,7 +380,8 @@ mod tests {
                 repair_flags(VerificationFlags::from_bits_truncate(flags))
                     | VerificationFlags::SigPushOnly,
             );
-            prop_assert_eq!(ret.0, ret.1);
+            prop_assert_eq!(ret.0, ret.1.clone().map_err(normalize_error),
+                            "original Rust result: {:?}", ret.1);
         }
     }
 }

src/zcash_script.rs

@@ -2,15 +2,16 @@ use std::num::TryFromIntError;
 
 use super::interpreter::*;
 use super::script::*;
+use super::script_error::*;
 
 /// This maps to `zcash_script_error_t`, but most of those cases aren’t used any more. This only
 /// replicates the still-used cases, and then an `Unknown` bucket for anything else that might
 /// happen.
-#[derive(Copy, Clone, Debug, PartialEq, Eq)]
+#[derive(Clone, Debug, PartialEq, Eq)]
 #[repr(u32)]
-pub enum Error {
+pub enum Error<'a> {
     /// Any failure that results in the script being invalid.
-    Ok = 0,
+    Ok(Option<ScriptError<'a>>) = 0,
     /// An exception was caught.
     VerifyScript = 7,
     /// The script size can’t fit in a `u32`, as required by the C++ code.
@@ -40,14 +41,14 @@ pub trait ZcashScript {
     ///  - flags: the script verification flags to use.
     ///
     ///  Note that script verification failure is indicated by `Err(Error::Ok)`.
-    fn verify_callback(
+    fn verify_callback<'a>(
         sighash_callback: SighashCalculator,
         lock_time: i64,
         is_final: bool,
         script_pub_key: &[u8],
         script_sig: &[u8],
         flags: VerificationFlags,
-    ) -> Result<(), Error>;
+    ) -> Result<(), Error<'a>>;
 
     /// Returns the number of transparent signature operations in the input or
     /// output script pointed to by script.
@@ -64,14 +65,14 @@ impl ZcashScript for Rust {
         Ok(cscript.get_sig_op_count(false))
     }
 
-    fn verify_callback(
+    fn verify_callback<'a>(
         sighash: SighashCalculator,
         lock_time: i64,
         is_final: bool,
         script_pub_key: &[u8],
         script_sig: &[u8],
         flags: VerificationFlags,
-    ) -> Result<(), Error> {
+    ) -> Result<(), Error<'a>> {
         let lock_time_num = ScriptNum(lock_time);
         verify_script(
             &Script(script_sig),
@@ -83,6 +84,6 @@ impl ZcashScript for Rust {
                 is_final,
             },
         )
-        .map_err(|_| Error::Ok)
+        .map_err(|e| Error::Ok(Some(e)))
     }
 }