Skip to content

Commit

Permalink
Preserve richer errors on the Rust side
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
sellout committed Oct 21, 2024
1 parent a16d6ad commit 4c34b3e
Show file tree
Hide file tree
Showing 3 changed files with 36 additions and 13 deletions.
3 changes: 2 additions & 1 deletion fuzz/fuzz_targets/compare.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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);
});
38 changes: 28 additions & 10 deletions src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ mod cxx;
mod external;
mod interpreter;
mod script;
mod script_error;
pub mod script_error;
mod zcash_script;

use std::os::raw::{c_int, c_uint, c_void};
Expand All @@ -26,9 +26,9 @@ pub enum Cxx {}

impl From<zcash_script_error_t> for Error {
#[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()),
}
Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -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());
}

Expand All @@ -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.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]
Expand All @@ -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.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! {
Expand All @@ -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.map_err(normalize_error),
"original Rust result: {:?}", ret.1);
}

/// Similar to `test_arbitrary_scripts`, but ensures the `sig` only contains pushes.
Expand All @@ -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.map_err(normalize_error),
"original Rust result: {:?}", ret.1);
}
}
}
8 changes: 6 additions & 2 deletions src/zcash_script.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ 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
Expand All @@ -10,7 +11,10 @@ use super::script::*;
#[repr(u32)]
pub enum Error {
/// Any failure that results in the script being invalid.
Ok = 0,
///
/// __NB__: This is in `Option` because this type is used by both the C++ and Rust
/// implementations, but the C++ impl doesn’t yet expose the original error.
Ok(Option<ScriptError>) = 0,
/// An exception was caught.
VerifyScript = 7,
/// The script size can’t fit in a `u32`, as required by the C++ code.
Expand Down Expand Up @@ -84,6 +88,6 @@ impl ZcashScript for Rust {
is_final,
},
)
.map_err(|_| Error::Ok)
.map_err(|e| Error::Ok(Some(e)))
}
}

0 comments on commit 4c34b3e

Please sign in to comment.