Fix Script signature verification, boolean handling, and parsing

src/ch05_transactions/tx_output.rs

@@ -17,7 +17,7 @@ impl TxOut {
     }
 
     pub fn repr(&self) -> String {
-        format!("{}:{}", &self.amount, hex::encode(&self.script_pubkey))
+        format!("{}:{}", self.amount, hex::encode(&self.script_pubkey))
     }
 
     pub fn parse(data: &[u8], mut index: usize) -> (Self, usize) {

src/ch06_script/opcodes.rs

@@ -23,6 +23,16 @@ pub struct OpCodes {
 #[derive(Debug, Clone)]
 pub struct Element(pub Vec<u8>);
 
+impl Element {
+    /// Script booleans treat all-zero bytes and negative zero as false.
+    pub fn is_truthy(&self) -> bool {
+        self.0
+            .iter()
+            .enumerate()
+            .any(|(index, &byte)| byte != 0 && !(index == self.0.len() - 1 && byte == 0x80))
+    }
+}
+
 impl OpCodes {
     pub fn encode_num(num: i64) -> Element {
         if num == 0 {
@@ -319,8 +329,8 @@ impl OpCodes {
             return false;
         }
 
-        let signature_bytes = stack.pop().unwrap().0;
         let pubkey_bytes = stack.pop().unwrap().0;
+        let signature_bytes = stack.pop().unwrap().0;
 
         if signature_bytes.is_empty() {
             stack.push(Self::encode_num(0));
@@ -424,9 +434,7 @@ impl OpCodes {
         }
 
         let element = stack.pop().unwrap();
-        let value = Self::decode_num(element);
-
-        if value == 0 {
+        if !element.is_truthy() {
             return false;
         }
 

src/ch06_script/script.rs

@@ -1,7 +1,7 @@
 use std::{collections::VecDeque, io::Error, ops::Add};
 
 use crate::{
-    decode_varint, encode_varint,
+    encode_varint,
     opcodes::{Element, opcode_functions},
 };
 
@@ -33,62 +33,58 @@ impl Script {
     }
 
     pub fn parse(ser: &[u8]) -> Result<Self, Error> {
-        // ser[0] (or more bytes) is a varint giving the total byte length of the script body.
-        // varint_size is the number of bytes the varint itself occupies (1, 3, 5, or 9).
-        // end = varint_size + length, NOT just length — the loop must account for the offset.
-        let (length, varint_size) = decode_varint(ser, 0);
+        let invalid = || {
+            Error::new(
+                std::io::ErrorKind::InvalidData,
+                "Truncated or invalid script",
+            )
+        };
+        let prefix = *ser.first().ok_or_else(invalid)?;
+        let (length, prefix_size) = match prefix {
+            0xfd => (
+                u16::from_le_bytes(ser.get(1..3).ok_or_else(invalid)?.try_into().unwrap()) as u64,
+                3usize,
+            ),
+            0xfe => (
+                u32::from_le_bytes(ser.get(1..5).ok_or_else(invalid)?.try_into().unwrap()) as u64,
+                5usize,
+            ),
+            0xff => (
+                u64::from_le_bytes(ser.get(1..9).ok_or_else(invalid)?.try_into().unwrap()),
+                9usize,
+            ),
+            n => (n as u64, 1usize),
+        };
+        let length = usize::try_from(length).map_err(|_| invalid())?;
+        let end = prefix_size.checked_add(length).ok_or_else(invalid)?;
+        let body = ser.get(prefix_size..end).ok_or_else(invalid)?;
+        let mut index = 0usize;
         let mut commands = Vec::new();
-        let mut index: usize = varint_size;
-        let end = varint_size + length as usize;
-
-        while index < end {
-            // let current = &ser[index];
 
-            let current_byte = ser[index];
+        while index < body.len() {
+            let opcode = body[index];
             index += 1;
-            match current_byte {
-                // 0x01–0x4b: the byte itself is the number of data bytes to push
-                1..=75 => {
-                    let n = current_byte as usize;
-                    // commands.extend_from_slice(&ser[index..index+n]);
-                    commands.push(Cmd::Data((ser[index..index + n]).to_vec()));
-                    index += n;
-                }
-                // 0x4c = OP_PUSHDATA1: next 1 byte is the data length
+            let data_length = match opcode {
+                1..=75 => opcode as usize,
                 76 => {
-                    // let (data_length, _) = decode_varint(ser, index);
-                    let data_length = ser[index] as usize;
+                    let length = *body.get(index).ok_or_else(invalid)? as usize;
                     index += 1;
-                    // commands.extend_from_slice(&ser[index..index + data_length as usize]);
-                    // commands.push(Cmd::OpCode((ser[index..index + data_length]).to_vec()));
-                    let data = ser[index..index + data_length].to_vec();
-                    commands.push(Cmd::Data(data));
-                    index += data_length;
+                    length
                 }
-                // 0x4d = OP_PUSHDATA2: next 2 bytes (little-endian) are the data length
                 77 => {
-                    let data_length = u16::from_le_bytes(ser[index..index + 2].try_into().unwrap());
+                    let bytes = body.get(index..index + 2).ok_or_else(invalid)?;
                     index += 2;
-                    // commands.push(Cmd::OpCode(
-                    //     ser[index..index + data_length as usize].to_vec(),
-                    // ));
-                    let data = ser[index..index + data_length as usize].to_vec();
-                    commands.push(Cmd::Data(data));
-
-                    index += data_length as usize
+                    u16::from_le_bytes(bytes.try_into().unwrap()) as usize
                 }
-                // Everything else is an opcode
-                op_code => {
-                    println!("Pushing Operation");
-                    commands.push(Cmd::OpCode(op_code));
+                _ => {
+                    commands.push(Cmd::OpCode(opcode));
+                    continue;
                 }
             };
-        }
-        if index != end {
-            return Err(Error::new(
-                std::io::ErrorKind::InvalidData,
-                "Parsing script failed",
-            ));
+            let data_end = index.checked_add(data_length).ok_or_else(invalid)?;
+            let data = body.get(index..data_end).ok_or_else(invalid)?;
+            commands.push(Cmd::Data(data.to_vec()));
+            index = data_end;
         }
 
         Ok(Script { commands })
@@ -172,10 +168,6 @@ impl Script {
 
         let top = stack.pop().unwrap();
 
-        if top.0.is_empty() {
-            Ok(false)
-        } else {
-            Ok(true)
-        }
+        Ok(top.is_truthy())
     }
 }

tests/ch06_script_tests.rs

@@ -250,3 +250,93 @@ fn test_script_add_concatenates() {
     let combined = s1 + s2;
     assert_eq!(combined.commands.len(), 3);
 }
+
+#[test]
+fn test_script_boolean_encodings() {
+    for bytes in [vec![], vec![0], vec![0, 0], vec![0x80], vec![0, 0x80]] {
+        let script = Script::new(vec![Cmd::Data(bytes.clone())]);
+        assert!(!script.evaluate(&[]).unwrap(), "false encoding: {bytes:?}");
+        let verify = script + Script::new(vec![Cmd::OpCode(0x69), Cmd::OpCode(0x51)]);
+        assert!(!verify.evaluate(&[]).unwrap());
+    }
+    for bytes in [vec![1], vec![0x80, 0], vec![0, 1], vec![0xff; 16]] {
+        let script = Script::new(vec![Cmd::Data(bytes.clone())]);
+        assert!(script.evaluate(&[]).unwrap(), "true encoding: {bytes:?}");
+        let verify = script + Script::new(vec![Cmd::OpCode(0x69), Cmd::OpCode(0x51)]);
+        assert!(verify.evaluate(&[]).unwrap());
+    }
+}
+
+#[test]
+fn test_parse_rejects_truncated_scripts() {
+    let cases = [
+        vec![],
+        vec![0xfd],
+        vec![0xfd, 1],
+        vec![0xfe, 0, 0, 0],
+        vec![0xff, 0, 0, 0, 0, 0, 0, 0],
+        vec![2, 0x51],
+        vec![1, 2],
+        vec![2, 2, 0xaa],
+        vec![1, 76],
+        vec![2, 76, 1],
+        vec![1, 77],
+        vec![2, 77, 1],
+        vec![3, 77, 1, 0],
+        vec![0xff, 255, 255, 255, 255, 255, 255, 255, 255],
+        // Bytes beyond the declared body must not satisfy a data push.
+        vec![1, 1, 0xaa],
+        vec![2, 76, 1, 0xaa],
+        vec![3, 77, 1, 0, 0xaa],
+    ];
+    for input in cases {
+        assert!(Script::parse(&input).is_err(), "input: {input:?}");
+    }
+}
+
+#[test]
+fn test_parse_pushdata_boundaries() {
+    for length in [0usize, 1, 75, 76, 255, 256, 520] {
+        let script = Script::new(vec![Cmd::Data(vec![0xaa; length])]);
+        let parsed = Script::parse(&script.serialize()).unwrap();
+        assert_eq!(parsed.serialize(), script.serialize());
+    }
+    for prefix in [
+        vec![0xfd, 1, 0],
+        vec![0xfe, 1, 0, 0, 0],
+        vec![0xff, 1, 0, 0, 0, 0, 0, 0, 0],
+    ] {
+        let mut input = prefix;
+        input.extend_from_slice(&[0x51, 0x00]);
+        let script = Script::parse(&input).unwrap();
+        assert_eq!(script.commands.len(), 1);
+        assert!(script.evaluate(&[]).unwrap());
+    }
+}
+
+#[test]
+fn test_checksig_standard_stack_order() {
+    use secp256k1::{Message, PublicKey, Secp256k1, SecretKey};
+    let context = Secp256k1::new();
+    let secret = SecretKey::from_byte_array([1; 32]).unwrap();
+    let public = PublicKey::from_secret_key(&context, &secret);
+    let digest = [2; 32];
+    let mut signature = context
+        .sign_ecdsa(Message::from_digest(digest), &secret)
+        .serialize_der()
+        .to_vec();
+    signature.push(1); // SIGHASH_ALL
+    let script = Script::new(vec![
+        Cmd::Data(signature),
+        Cmd::Data(public.serialize().to_vec()),
+        Cmd::OpCode(0xac),
+    ]);
+    assert!(script.evaluate(&digest).unwrap());
+    assert!(!script.evaluate(&[3; 32]).unwrap());
+    let empty_signature = Script::new(vec![
+        Cmd::Data(vec![]),
+        Cmd::Data(public.serialize().to_vec()),
+        Cmd::OpCode(0xac),
+    ]);
+    assert!(!empty_signature.evaluate(&digest).unwrap());
+}