Merge rust-bitcoin#2535: Move read_scriptint to PushBytes & create read_int function

d6ef16a update api (Divyansh Gupta)
a336ec0 refactor(script): move `read_scriptint` to `PushBytes` & create `read_int` function (Divyansh Gupta)

Pull request description:

  * Moved `read_scriptint` method to `Push_Bytes` struct
  * Created `Instruction::read_int` method which acts as a wrappper around this  function.

  Done as part of rust-bitcoin#1547

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

Tree-SHA512: e33df8adcb1c23351da303f6bad1ea4a8eae30e65943d230ae886183a01f970aecd0c8c8fd3a6c337cfe6dde1b7590778d88c308415e393f137065ef7da4b29c

Author: apoelstra

api/bitcoin/all-features.txt

@@ -7849,6 +7849,7 @@ pub fn bitcoin::blockdata::script::Instruction<'a>::eq(&self, other: &bitcoin::b
 pub fn bitcoin::blockdata::script::Instruction<'a>::fmt(&self, f: &mut core::fmt::Formatter<'_>) -> core::fmt::Result
 pub fn bitcoin::blockdata::script::Instruction<'a>::opcode(&self) -> core::option::Option<bitcoin::blockdata::opcodes::Opcode>
 pub fn bitcoin::blockdata::script::Instruction<'a>::push_bytes(&self) -> core::option::Option<&bitcoin::blockdata::script::PushBytes>
+pub fn bitcoin::blockdata::script::Instruction<'a>::read_int(&self) -> core::option::Option<i64>
 pub fn bitcoin::blockdata::script::Instruction<'a>::script_num(&self) -> core::option::Option<i64>
 pub fn bitcoin::blockdata::script::InstructionIndices<'a>::as_script(&self) -> &'a bitcoin::blockdata::script::Script
 pub fn bitcoin::blockdata::script::InstructionIndices<'a>::clone(&self) -> bitcoin::blockdata::script::InstructionIndices<'a>
@@ -7883,6 +7884,7 @@ pub fn bitcoin::blockdata::script::PushBytes::index(&self, index: usize) -> &Sel
 pub fn bitcoin::blockdata::script::PushBytes::is_empty(&self) -> bool
 pub fn bitcoin::blockdata::script::PushBytes::len(&self) -> usize
 pub fn bitcoin::blockdata::script::PushBytes::partial_cmp(&self, other: &bitcoin::blockdata::script::PushBytes) -> core::option::Option<core::cmp::Ordering>
+pub fn bitcoin::blockdata::script::PushBytes::read_scriptint(&self) -> core::result::Result<i64, bitcoin::blockdata::script::Error>
 pub fn bitcoin::blockdata::script::PushBytes::to_owned(&self) -> Self::Owned
 pub fn bitcoin::blockdata::script::PushBytesBuf::as_mut(&mut self) -> &mut bitcoin::blockdata::script::PushBytes
 pub fn bitcoin::blockdata::script::PushBytesBuf::as_mut_push_bytes(&mut self) -> &mut bitcoin::blockdata::script::PushBytes
@@ -8247,7 +8249,6 @@ pub fn bitcoin::blockdata::script::WScriptHash::serialize<S: serde::ser::Seriali
 pub fn bitcoin::blockdata::script::WScriptHash::to_byte_array(self) -> Self::Bytes
 pub fn bitcoin::blockdata::script::WScriptHash::to_raw_hash(self) -> bitcoin_hashes::sha256::Hash
 pub fn bitcoin::blockdata::script::read_scriptbool(v: &[u8]) -> bool
-pub fn bitcoin::blockdata::script::read_scriptint(v: &[u8]) -> core::result::Result<i64, bitcoin::blockdata::script::Error>
 pub fn bitcoin::blockdata::script::read_scriptint_non_minimal(v: &[u8]) -> core::result::Result<i64, bitcoin::blockdata::script::Error>
 pub fn bitcoin::blockdata::script::witness_program::Error::clone(&self) -> bitcoin::blockdata::script::witness_program::Error
 pub fn bitcoin::blockdata::script::witness_program::Error::eq(&self, other: &bitcoin::blockdata::script::witness_program::Error) -> bool
@@ -9260,7 +9261,6 @@ pub fn bitcoin::psbt::raw::ProprietaryKey<Subtype>::to_key(&self) -> bitcoin::ps
 pub fn bitcoin::psbt::raw::ProprietaryKey<Subtype>::try_from(key: bitcoin::psbt::raw::Key) -> core::result::Result<Self, Self::Error>
 pub fn bitcoin::script::PushBytesErrorReport::input_len(&self) -> usize
 pub fn bitcoin::script::read_scriptbool(v: &[u8]) -> bool
-pub fn bitcoin::script::read_scriptint(v: &[u8]) -> core::result::Result<i64, bitcoin::blockdata::script::Error>
 pub fn bitcoin::script::read_scriptint_non_minimal(v: &[u8]) -> core::result::Result<i64, bitcoin::blockdata::script::Error>
 pub fn bitcoin::script::write_scriptint(out: &mut [u8; 8], n: i64) -> usize
 pub fn bitcoin::sighash::Annex<'a>::as_bytes(&self) -> &[u8]

api/bitcoin/default-features.txt

@@ -7465,6 +7465,7 @@ pub fn bitcoin::blockdata::script::Instruction<'a>::eq(&self, other: &bitcoin::b
 pub fn bitcoin::blockdata::script::Instruction<'a>::fmt(&self, f: &mut core::fmt::Formatter<'_>) -> core::fmt::Result
 pub fn bitcoin::blockdata::script::Instruction<'a>::opcode(&self) -> core::option::Option<bitcoin::blockdata::opcodes::Opcode>
 pub fn bitcoin::blockdata::script::Instruction<'a>::push_bytes(&self) -> core::option::Option<&bitcoin::blockdata::script::PushBytes>
+pub fn bitcoin::blockdata::script::Instruction<'a>::read_int(&self) -> core::option::Option<i64>
 pub fn bitcoin::blockdata::script::Instruction<'a>::script_num(&self) -> core::option::Option<i64>
 pub fn bitcoin::blockdata::script::InstructionIndices<'a>::as_script(&self) -> &'a bitcoin::blockdata::script::Script
 pub fn bitcoin::blockdata::script::InstructionIndices<'a>::clone(&self) -> bitcoin::blockdata::script::InstructionIndices<'a>
@@ -7499,6 +7500,7 @@ pub fn bitcoin::blockdata::script::PushBytes::index(&self, index: usize) -> &Sel
 pub fn bitcoin::blockdata::script::PushBytes::is_empty(&self) -> bool
 pub fn bitcoin::blockdata::script::PushBytes::len(&self) -> usize
 pub fn bitcoin::blockdata::script::PushBytes::partial_cmp(&self, other: &bitcoin::blockdata::script::PushBytes) -> core::option::Option<core::cmp::Ordering>
+pub fn bitcoin::blockdata::script::PushBytes::read_scriptint(&self) -> core::result::Result<i64, bitcoin::blockdata::script::Error>
 pub fn bitcoin::blockdata::script::PushBytes::to_owned(&self) -> Self::Owned
 pub fn bitcoin::blockdata::script::PushBytesBuf::as_mut(&mut self) -> &mut bitcoin::blockdata::script::PushBytes
 pub fn bitcoin::blockdata::script::PushBytesBuf::as_mut_push_bytes(&mut self) -> &mut bitcoin::blockdata::script::PushBytes
@@ -7852,7 +7854,6 @@ pub fn bitcoin::blockdata::script::WScriptHash::partial_cmp(&self, other: &bitco
 pub fn bitcoin::blockdata::script::WScriptHash::to_byte_array(self) -> Self::Bytes
 pub fn bitcoin::blockdata::script::WScriptHash::to_raw_hash(self) -> bitcoin_hashes::sha256::Hash
 pub fn bitcoin::blockdata::script::read_scriptbool(v: &[u8]) -> bool
-pub fn bitcoin::blockdata::script::read_scriptint(v: &[u8]) -> core::result::Result<i64, bitcoin::blockdata::script::Error>
 pub fn bitcoin::blockdata::script::read_scriptint_non_minimal(v: &[u8]) -> core::result::Result<i64, bitcoin::blockdata::script::Error>
 pub fn bitcoin::blockdata::script::witness_program::Error::clone(&self) -> bitcoin::blockdata::script::witness_program::Error
 pub fn bitcoin::blockdata::script::witness_program::Error::eq(&self, other: &bitcoin::blockdata::script::witness_program::Error) -> bool
@@ -8770,7 +8771,6 @@ pub fn bitcoin::psbt::raw::ProprietaryKey<Subtype>::to_key(&self) -> bitcoin::ps
 pub fn bitcoin::psbt::raw::ProprietaryKey<Subtype>::try_from(key: bitcoin::psbt::raw::Key) -> core::result::Result<Self, Self::Error>
 pub fn bitcoin::script::PushBytesErrorReport::input_len(&self) -> usize
 pub fn bitcoin::script::read_scriptbool(v: &[u8]) -> bool
-pub fn bitcoin::script::read_scriptint(v: &[u8]) -> core::result::Result<i64, bitcoin::blockdata::script::Error>
 pub fn bitcoin::script::read_scriptint_non_minimal(v: &[u8]) -> core::result::Result<i64, bitcoin::blockdata::script::Error>
 pub fn bitcoin::script::write_scriptint(out: &mut [u8; 8], n: i64) -> usize
 pub fn bitcoin::sighash::Annex<'a>::as_bytes(&self) -> &[u8]

api/bitcoin/no-features.txt

@@ -6821,6 +6821,7 @@ pub fn bitcoin::blockdata::script::Instruction<'a>::eq(&self, other: &bitcoin::b
 pub fn bitcoin::blockdata::script::Instruction<'a>::fmt(&self, f: &mut core::fmt::Formatter<'_>) -> core::fmt::Result
 pub fn bitcoin::blockdata::script::Instruction<'a>::opcode(&self) -> core::option::Option<bitcoin::blockdata::opcodes::Opcode>
 pub fn bitcoin::blockdata::script::Instruction<'a>::push_bytes(&self) -> core::option::Option<&bitcoin::blockdata::script::PushBytes>
+pub fn bitcoin::blockdata::script::Instruction<'a>::read_int(&self) -> core::option::Option<i64>
 pub fn bitcoin::blockdata::script::Instruction<'a>::script_num(&self) -> core::option::Option<i64>
 pub fn bitcoin::blockdata::script::InstructionIndices<'a>::as_script(&self) -> &'a bitcoin::blockdata::script::Script
 pub fn bitcoin::blockdata::script::InstructionIndices<'a>::clone(&self) -> bitcoin::blockdata::script::InstructionIndices<'a>
@@ -6855,6 +6856,7 @@ pub fn bitcoin::blockdata::script::PushBytes::index(&self, index: usize) -> &Sel
 pub fn bitcoin::blockdata::script::PushBytes::is_empty(&self) -> bool
 pub fn bitcoin::blockdata::script::PushBytes::len(&self) -> usize
 pub fn bitcoin::blockdata::script::PushBytes::partial_cmp(&self, other: &bitcoin::blockdata::script::PushBytes) -> core::option::Option<core::cmp::Ordering>
+pub fn bitcoin::blockdata::script::PushBytes::read_scriptint(&self) -> core::result::Result<i64, bitcoin::blockdata::script::Error>
 pub fn bitcoin::blockdata::script::PushBytes::to_owned(&self) -> Self::Owned
 pub fn bitcoin::blockdata::script::PushBytesBuf::as_mut(&mut self) -> &mut bitcoin::blockdata::script::PushBytes
 pub fn bitcoin::blockdata::script::PushBytesBuf::as_mut_push_bytes(&mut self) -> &mut bitcoin::blockdata::script::PushBytes
@@ -7207,7 +7209,6 @@ pub fn bitcoin::blockdata::script::WScriptHash::partial_cmp(&self, other: &bitco
 pub fn bitcoin::blockdata::script::WScriptHash::to_byte_array(self) -> Self::Bytes
 pub fn bitcoin::blockdata::script::WScriptHash::to_raw_hash(self) -> bitcoin_hashes::sha256::Hash
 pub fn bitcoin::blockdata::script::read_scriptbool(v: &[u8]) -> bool
-pub fn bitcoin::blockdata::script::read_scriptint(v: &[u8]) -> core::result::Result<i64, bitcoin::blockdata::script::Error>
 pub fn bitcoin::blockdata::script::read_scriptint_non_minimal(v: &[u8]) -> core::result::Result<i64, bitcoin::blockdata::script::Error>
 pub fn bitcoin::blockdata::script::witness_program::Error::clone(&self) -> bitcoin::blockdata::script::witness_program::Error
 pub fn bitcoin::blockdata::script::witness_program::Error::eq(&self, other: &bitcoin::blockdata::script::witness_program::Error) -> bool
@@ -7910,7 +7911,6 @@ pub fn bitcoin::psbt::raw::ProprietaryKey<Subtype>::to_key(&self) -> bitcoin::ps
 pub fn bitcoin::psbt::raw::ProprietaryKey<Subtype>::try_from(key: bitcoin::psbt::raw::Key) -> core::result::Result<Self, Self::Error>
 pub fn bitcoin::script::PushBytesErrorReport::input_len(&self) -> usize
 pub fn bitcoin::script::read_scriptbool(v: &[u8]) -> bool
-pub fn bitcoin::script::read_scriptint(v: &[u8]) -> core::result::Result<i64, bitcoin::blockdata::script::Error>
 pub fn bitcoin::script::read_scriptint_non_minimal(v: &[u8]) -> core::result::Result<i64, bitcoin::blockdata::script::Error>
 pub fn bitcoin::script::write_scriptint(out: &mut [u8; 8], n: i64) -> usize
 pub fn bitcoin::sighash::Annex<'a>::as_bytes(&self) -> &[u8]

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!(Ok(-i), read_scriptint(&build_scriptint(-i)));
+        assert_eq!(
+            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!(read_scriptint(&build_scriptint(-(1 << 31))).is_err());
+    assert!(PushBytes::read_scriptint(
+        <&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!(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!(
+        PushBytes::read_scriptint(<[_; 2] as AsRef<PushBytes>>::as_ref(&[0x80, 0x00])),
+        Ok(0x80)
+    );
+    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));