Merge rust-bitcoin/rust-bitcoin#2081: Feature: Instruction can read the script number

cd15c746cb Feature: Instruction can read the script number (junderw)

Pull request description:

  This new method on `Instruction` will allow for interpreting the Instruction as a script number (i32)

ACKs for top commit:
  tcharding:
    ACK cd15c746cb
  apoelstra:
    ACK cd15c746cb

Tree-SHA512: 597add5ab28b53b7f61633b2e2005bed2c8fe4777575c9498c9a5431e4b020cf8ddf02509b1b4c1ba455e879e024d7d034636ab4863cca42567ed087b648abc3

bitcoin/src/blockdata/script/instruction.rs

@@ -31,6 +31,31 @@ impl<'a> Instruction<'a> {
         }
     }
 
+    /// Returns the number interpretted by the script parser
+    /// if it can be coerced into a number.
+    ///
+    /// This does not require the script num to be minimal.
+    pub fn script_num(&self) -> Option<i64> {
+        match self {
+            Instruction::Op(op) => {
+                let v = op.to_u8();
+                match v {
+                    // OP_PUSHNUM_1 ..= OP_PUSHNUM_16
+                    0x51..=0x60 => Some(v as i64 - 0x50),
+                    // OP_PUSHNUM_NEG1
+                    0x4f => Some(-1),
+                    _ => None,
+                }
+            }
+            Instruction::PushBytes(bytes) => {
+                match super::read_scriptint_non_minimal(bytes.as_bytes()) {
+                    Ok(v) => Some(v),
+                    _ => None,
+                }
+            }
+        }
+    }
+
     /// Returns the number of bytes required to encode the instruction in script.
     pub(super) fn script_serialized_len(&self) -> usize {
         match self {

bitcoin/src/blockdata/script/mod.rs

@@ -170,20 +170,19 @@ pub fn write_scriptint(out: &mut [u8; 8], n: i64) -> usize {
 ///
 /// This code is based on the `CScriptNum` constructor in Bitcoin Core (see `script.h`).
 pub fn read_scriptint(v: &[u8]) -> Result<i64, Error> {
-    let len = v.len();
-    if len > 4 {
-        return Err(Error::NumericOverflow);
-    }
     let last = match v.last() {
         Some(last) => last,
         None => return Ok(0),
     };
+    if v.len() > 4 {
+        return Err(Error::NumericOverflow);
+    }
     // Comment and code copied from Bitcoin Core:
     // https://github.com/bitcoin/bitcoin/blob/447f50e4aed9a8b1d80e1891cda85801aeb80b4e/src/script/script.h#L247-L262
     // If the most-significant-byte - excluding the sign bit - is zero
     // then we're not minimal. Note how this test also rejects the
     // negative-zero encoding, 0x80.
-    if (last & 0x7f) == 0 {
+    if (*last & 0x7f) == 0 {
         // One exception: if there's more than one byte and the most
         // significant bit of the second-most-significant-byte is set
         // it would conflict with the sign bit. An example of this case
@@ -194,12 +193,33 @@ pub fn read_scriptint(v: &[u8]) -> Result<i64, Error> {
         }
     }
 
+    Ok(scriptint_parse(v))
+}
+
+/// Decodes an integer in script format without non-minimal error.
+///
+/// The overflow error for slices over 4 bytes long is still there.
+/// See [`read_scriptint`] for a description of some subtleties of
+/// this function.
+pub fn read_scriptint_non_minimal(v: &[u8]) -> Result<i64, Error> {
+    if v.is_empty() {
+        return Ok(0);
+    }
+    if v.len() > 4 {
+        return Err(Error::NumericOverflow);
+    }
+
+    Ok(scriptint_parse(v))
+}
+
+// Caller to guarantee that `v` is not empty.
+fn scriptint_parse(v: &[u8]) -> i64 {
     let (mut ret, sh) = v.iter().fold((0, 0), |(acc, sh), n| (acc + ((*n as i64) << sh), sh + 8));
-    if v[len - 1] & 0x80 != 0 {
+    if v[v.len() - 1] & 0x80 != 0 {
         ret &= (1 << (sh - 1)) - 1;
         ret = -ret;
     }
-    Ok(ret)
+    ret
 }
 
 /// Decodes a boolean.

bitcoin/src/blockdata/script/tests.rs

@@ -312,9 +312,13 @@ fn scriptint_round_trip() {
     for &i in test_vectors.iter() {
         assert_eq!(Ok(i), read_scriptint(&build_scriptint(i)));
         assert_eq!(Ok(-i), read_scriptint(&build_scriptint(-i)));
+        assert_eq!(Ok(i), read_scriptint_non_minimal(&build_scriptint(i)));
+        assert_eq!(Ok(-i), read_scriptint_non_minimal(&build_scriptint(-i)));
     }
     assert!(read_scriptint(&build_scriptint(1 << 31)).is_err());
     assert!(read_scriptint(&build_scriptint(-(1 << 31))).is_err());
+    assert!(read_scriptint_non_minimal(&build_scriptint(1 << 31)).is_err());
+    assert!(read_scriptint_non_minimal(&build_scriptint(-(1 << 31))).is_err());
 }
 
 #[test]
@@ -323,6 +327,11 @@ fn non_minimal_scriptints() {
     assert_eq!(read_scriptint(&[0xff, 0x00]), Ok(0xff));
     assert_eq!(read_scriptint(&[0x8f, 0x00, 0x00]), Err(Error::NonMinimalPush));
     assert_eq!(read_scriptint(&[0x7f, 0x00]), Err(Error::NonMinimalPush));
+
+    assert_eq!(read_scriptint_non_minimal(&[0x80, 0x00]), Ok(0x80));
+    assert_eq!(read_scriptint_non_minimal(&[0xff, 0x00]), Ok(0xff));
+    assert_eq!(read_scriptint_non_minimal(&[0x8f, 0x00, 0x00]), Ok(0x8f));
+    assert_eq!(read_scriptint_non_minimal(&[0x7f, 0x00]), Ok(0x7f));
 }
 
 #[test]
@@ -769,3 +778,49 @@ fn read_scriptbool_non_zero_is_true() {
     let v: Vec<u8> = vec![0x01, 0x00, 0x00, 0x80]; // With sign bit set.
     assert!(read_scriptbool(&v));
 }
+
+#[test]
+fn instruction_script_num_parse() {
+    let push_bytes = [
+        (PushBytesBuf::from([]), Some(0)),
+        (PushBytesBuf::from([0x00]), Some(0)),
+        (PushBytesBuf::from([0x01]), Some(1)),
+        // Check all the negative 1s
+        (PushBytesBuf::from([0x81]), Some(-1)),
+        (PushBytesBuf::from([0x01, 0x80]), Some(-1)),
+        (PushBytesBuf::from([0x01, 0x00, 0x80]), Some(-1)),
+        (PushBytesBuf::from([0x01, 0x00, 0x00, 0x80]), Some(-1)),
+        // Check all the negative 0s
+        (PushBytesBuf::from([0x80]), Some(0)),
+        (PushBytesBuf::from([0x00, 0x80]), Some(0)),
+        (PushBytesBuf::from([0x00, 0x00, 0x80]), Some(0)),
+        (PushBytesBuf::from([0x00, 0x00, 0x00, 0x80]), Some(0)),
+        // Too long
+        (PushBytesBuf::from([0x01, 0x00, 0x00, 0x00, 0x80]), None),
+        // Check the position of all the bytes
+        (PushBytesBuf::from([0xef, 0xbe, 0xad, 0x5e]), Some(0x5eadbeef)),
+        // Add negative
+        (PushBytesBuf::from([0xef, 0xbe, 0xad, 0xde]), Some(-0x5eadbeef)),
+    ];
+    let ops = [
+        (Instruction::Op(opcodes::all::OP_PUSHDATA4), None),
+        (Instruction::Op(opcodes::all::OP_PUSHNUM_NEG1), Some(-1)),
+        (Instruction::Op(opcodes::all::OP_RESERVED), None),
+        (Instruction::Op(opcodes::all::OP_PUSHNUM_1), Some(1)),
+        (Instruction::Op(opcodes::all::OP_PUSHNUM_16), Some(16)),
+        (Instruction::Op(opcodes::all::OP_NOP), None),
+    ];
+    for (input, expected) in &push_bytes {
+        assert_eq!(Instruction::PushBytes(input).script_num(), *expected);
+    }
+    for (input, expected) in &ops {
+        assert_eq!(input.script_num(), *expected);
+    }
+
+    // script_num() is predicated on OP_0/OP_FALSE (0x00)
+    // being treated as an empty PushBytes
+    assert_eq!(
+        Script::from_bytes(&[0x00]).instructions().next(),
+        Some(Ok(Instruction::PushBytes(PushBytes::empty()))),
+    );
+}