Address Str4d’s comments on #171

src/interpreter.rs

@@ -1,18 +1,34 @@
-bitflags::bitflags! {
-    /// The different SigHash types, as defined in <https://zips.z.cash/zip-0143>
-    ///
-    /// TODO: This is currently defined as `i32` to match the `c_int` constants in this package, but
-    ///       should use librustzcash’s `u8` constants once we’ve removed the C++.
-    #[derive(Copy, Clone, Debug, PartialEq, Eq)]
-    pub struct HashType: i32 {
-        /// Sign all the outputs
-        const All = 1;
-        /// Sign none of the outputs - anyone can spend
-        const None = 2;
-        /// Sign one of the outputs - anyone can spend the rest
-        const Single = 3;
-        /// Anyone can add inputs to this transaction
-        const AnyoneCanPay = 0x80;
+#[derive(Copy, Clone, Debug, PartialEq, Eq)]
+pub enum SignedOutputs {
+    /// Sign all the outputs
+    All,
+    /// Sign one of the outputs - anyone can spend the rest
+    Single,
+    /// Sign none of the outputs - anyone can spend
+    None,
+}
+
+/// The different SigHash types, as defined in <https://zips.z.cash/zip-0143>
+#[derive(Copy, Clone, Debug, PartialEq, Eq)]
+pub struct HashType {
+    pub signed_outputs: SignedOutputs,
+    /// Anyone can add inputs to this transaction
+    pub anyone_can_pay: bool,
+}
+
+impl HashType {
+    pub fn from_bits(bits: i32) -> Option<Self> {
+        let msigned_outputs = match (bits & 2 != 0, bits & 1 != 0) {
+            (false, false) => None,
+            (false, true) => Some(SignedOutputs::All),
+            (true, false) => Some(SignedOutputs::None),
+            (true, true) => Some(SignedOutputs::Single),
+        };
+
+        msigned_outputs.map(|signed_outputs| HashType {
+            signed_outputs,
+            anyone_can_pay: bits & 0x80 != 0,
+        })
     }
 }
 

src/lib.rs

@@ -47,7 +47,9 @@ extern "C" fn sighash_callback(
     let ctx = ctx as *const SighashCalculator;
     // SAFETY: `ctx` is a valid `SighashCalculator` passed to `verify_callback` which forwards it to
     // the `CallbackTransactionSignatureChecker`.
-    if let Some(sighash) = unsafe { *ctx }(script_code_vec, HashType::from_bits_retain(hash_type)) {
+    if let Some(sighash) =
+        HashType::from_bits(hash_type).and_then(|ht| unsafe { *ctx }(script_code_vec, ht))
+    {
         assert_eq!(sighash_out_len, sighash.len().try_into().unwrap());
         // SAFETY: `sighash_out` is a valid buffer created in
         // `CallbackTransactionSignatureChecker::CheckSig`.

src/zcash_script.rs

@@ -6,12 +6,11 @@ use super::interpreter::*;
 /// replicates the still-used cases, and then an `Unknown` bucket for anything else that might
 /// happen.
 #[derive(Copy, Clone, Debug, PartialEq, Eq)]
-#[repr(u32)]
 pub enum Error {
     /// Any failure that results in the script being invalid.
-    Ok = 0,
+    Ok,
     /// An exception was caught.
-    VerifyScript = 7,
+    VerifyScript,
     /// The script size can’t fit in a `u32`, as required by the C++ code.
     InvalidScriptSize(TryFromIntError),
     /// Some other failure value recovered from C++.
@@ -21,7 +20,9 @@ pub enum Error {
     Unknown(i64),
 }
 
-/// All signature hashes are 32 bits, since they are necessarily produced by SHA256.
+/// All signature hashes are 32 bytes, since they are either:
+/// - a SHA-256 output (for v1 or v2 transactions).
+/// - a BLAKE2b-256 output (for v3 and above transactions).
 pub const SIGHASH_SIZE: usize = 32;
 
 /// A function which is called to obtain the sighash.
@@ -44,7 +45,7 @@ pub trait ZcashScript {
     ///  the transaction itself. In particular, the sighash for the spend
     ///  is obtained using a callback function.
     ///
-    ///  - sighash_callback: a callback function which is called to obtain the sighash.
+    ///  - sighash: a callback function which is called to obtain the sighash.
     ///  - n_lock_time: the lock time of the transaction being validated.
     ///  - is_final: a boolean indicating whether the input being validated is final
     ///    (i.e. its sequence number is 0xFFFFFFFF).