refactor(script): move `read_scriptint` to `PushBytes` & create `read_int` function

* Moved read_scriptint method to Push_Bytes struct
 * Created Instruction::read_int method
fix #1547

bitcoin/src/blockdata/block.rs

@@ -19,6 +19,7 @@ use crate::consensus::{encode, Decodable, Encodable, Params};
 use crate::internal_macros::{impl_consensus_encoding, impl_hashencode};
 use crate::pow::{CompactTarget, Target, Work};
 use crate::prelude::*;
+use crate::script::PushBytes;
 use crate::{merkle_tree, VarInt};
 
 hashes::hash_newtype! {
@@ -378,7 +379,7 @@ impl Block {
         match push.map_err(|_| Bip34Error::NotPresent)? {
             script::Instruction::PushBytes(b) => {
                 // Check that the number is encoded in the minimal way.
-                let h = script::read_scriptint(b.as_bytes())
+                let h = PushBytes::read_scriptint(b)
                     .map_err(|_e| Bip34Error::UnexpectedPush(b.as_bytes().to_vec()))?;
                 if h < 0 {
                     Err(Bip34Error::NegativeHeight)

bitcoin/src/blockdata/script/instruction.rs

@@ -61,6 +61,27 @@ impl<'a> Instruction<'a> {
             Instruction::PushBytes(bytes) => ScriptBuf::reserved_len_for_slice(bytes.len()),
         }
     }
+
+    /// Reads an integer from an Instruction,
+    /// returning Some(i64) for valid opcodes or pushed bytes, otherwise None
+    pub fn read_int(&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 PushBytes::read_scriptint(bytes) {
+                Ok(v) => Some(v),
+                _ => None,
+            },
+        }
+    }
 }
 
 /// Iterator over a script returning parsed opcodes.

bitcoin/src/blockdata/script/mod.rs

@@ -155,54 +155,10 @@ pub fn write_scriptint(out: &mut [u8; 8], n: i64) -> usize {
     len
 }
 
-/// Decodes an integer in script(minimal CScriptNum) format.
-///
-/// Notice that this fails on overflow: the result is the same as in
-/// bitcoind, that only 4-byte signed-magnitude values may be read as
-/// numbers. They can be added or subtracted (and a long time ago,
-/// multiplied and divided), and this may result in numbers which
-/// can't be written out in 4 bytes or less. This is ok! The number
-/// just can't be read as a number again.
-/// This is a bit crazy and subtle, but it makes sense: you can load
-/// 32-bit numbers and do anything with them, which back when mult/div
-/// was allowed, could result in up to a 64-bit number. We don't want
-/// overflow since that's surprising --- and we don't want numbers that
-/// don't fit in 64 bits (for efficiency on modern processors) so we
-/// simply say, anything in excess of 32 bits is no longer a number.
-/// This is basically a ranged type implementation.
-///
-/// This code is based on the `CScriptNum` constructor in Bitcoin Core (see `script.h`).
-pub fn read_scriptint(v: &[u8]) -> Result<i64, Error> {
-    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 {
-        // 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
-        // is +-255, which encode to 0xff00 and 0xff80 respectively.
-        // (big-endian).
-        if v.len() <= 1 || (v[v.len() - 2] & 0x80) == 0 {
-            return Err(Error::NonMinimalPush);
-        }
-    }
-
-    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
+/// See [`push_bytes::PushBytes::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() {

bitcoin/src/blockdata/script/push_bytes.rs

@@ -19,8 +19,10 @@ mod primitive {
     };
 
     use super::PushBytesError;
+    use crate::blockdata::script::Error;
     #[allow(unused)]
     use crate::prelude::*;
+    use crate::script::scriptint_parse;
 
     #[cfg(any(target_pointer_width = "16", target_pointer_width = "32"))]
     fn check_limit(_: usize) -> Result<(), PushBytesError> { Ok(()) }
@@ -73,6 +75,50 @@ mod primitive {
 
         /// Returns the underlying mutbale bytes.
         pub fn as_mut_bytes(&mut self) -> &mut [u8] { &mut self.0 }
+
+        /// Decodes an integer in script(minimal CScriptNum) format.
+        ///
+        /// Notice that this fails on overflow: the result is the same as in
+        /// bitcoind, that only 4-byte signed-magnitude values may be read as
+        /// numbers. They can be added or subtracted (and a long time ago,
+        /// multiplied and divided), and this may result in numbers which
+        /// can't be written out in 4 bytes or less. This is ok! The number
+        /// just can't be read as a number again.
+        /// This is a bit crazy and subtle, but it makes sense: you can load
+        /// 32-bit numbers and do anything with them, which back when mult/div
+        /// was allowed, could result in up to a 64-bit number. We don't want
+        /// overflow since that's surprising --- and we don't want numbers that
+        /// don't fit in 64 bits (for efficiency on modern processors) so we
+        /// simply say, anything in excess of 32 bits is no longer a number.
+        /// This is basically a ranged type implementation.
+        ///
+        /// This code is based on the `CScriptNum` constructor in Bitcoin Core (see `script.h`).
+        pub fn read_scriptint(&self) -> Result<i64, Error> {
+            let last = match self.as_bytes().last() {
+                Some(last) => last,
+                None => return Ok(0),
+            };
+            if self.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 {
+                // 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
+                // is +-255, which encode to 0xff00 and 0xff80 respectively.
+                // (big-endian).
+                if self.len() <= 1 || (self[self.len() - 2] & 0x80) == 0 {
+                    return Err(Error::NonMinimalPush);
+                }
+            }
+
+            Ok(scriptint_parse(self.as_bytes()))
+        }
     }
 
     macro_rules! delegate_index {

bitcoin/src/blockdata/script/tests.rs

@@ -310,23 +310,51 @@ fn scriptint_round_trip() {
         -((1 << 31) - 1),
     ];
     for &i in test_vectors.iter() {
-        assert_eq!(Ok(i), read_scriptint(&build_scriptint(i)));
+        assert_eq!(
-        assert_eq!(Ok(-i), read_scriptint(&build_scriptint(-i)));
+            Ok(i),
+            PushBytes::read_scriptint(
+                <&PushBytes>::try_from(build_scriptint(i).as_slice()).unwrap()
+            )
+        );
+        assert_eq!(
+            Ok(-i),
+            PushBytes::read_scriptint(
+                <&PushBytes>::try_from(build_scriptint(-i).as_slice()).unwrap()
+            )
+        );
         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!(PushBytes::read_scriptint(
-    assert!(read_scriptint(&build_scriptint(-(1 << 31))).is_err());
+        <&PushBytes>::try_from(build_scriptint(1 << 31).as_slice()).unwrap()
+    )
+    .is_err());
+    assert!(PushBytes::read_scriptint(
+        <&PushBytes>::try_from(build_scriptint(-(1 << 31)).as_slice()).unwrap()
+    )
+    .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]
 fn non_minimal_scriptints() {
-    assert_eq!(read_scriptint(&[0x80, 0x00]), Ok(0x80));
+    assert_eq!(
-    assert_eq!(read_scriptint(&[0xff, 0x00]), Ok(0xff));
+        PushBytes::read_scriptint(<[_; 2] as AsRef<PushBytes>>::as_ref(&[0x80, 0x00])),
-    assert_eq!(read_scriptint(&[0x8f, 0x00, 0x00]), Err(Error::NonMinimalPush));
+        Ok(0x80)
-    assert_eq!(read_scriptint(&[0x7f, 0x00]), Err(Error::NonMinimalPush));
+    );
+    assert_eq!(
+        PushBytes::read_scriptint(<[_; 2] as AsRef<PushBytes>>::as_ref(&[0xff, 0x00])),
+        Ok(0xff)
+    );
+    assert_eq!(
+        PushBytes::read_scriptint(<[_; 3] as AsRef<PushBytes>>::as_ref(&[0x8f, 0x00, 0x00])),
+        Err(Error::NonMinimalPush)
+    );
+    assert_eq!(
+        PushBytes::read_scriptint(<[_; 2] as AsRef<PushBytes>>::as_ref(&[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));

fuzz/fuzz_targets/bitcoin/deserialize_script.rs

@@ -23,7 +23,7 @@ fn do_test(data: &[u8]) {
                     // reserialized as numbers. (For -1 through 16, this will use special ops; for
                     // others it'll just reserialize them as pushes.)
                     if bytes.len() == 1 && bytes[0] != 0x80 && bytes[0] != 0x00 {
-                        if let Ok(num) = script::read_scriptint(bytes.as_bytes()) {
+                        if let Ok(num) = script::PushBytes::read_scriptint(bytes) {
                             b = b.push_int(num);
                         } else {
                             b = b.push_slice(bytes);