Merge rust-bitcoin#3411: script: refactor push_int_unchecked and test…

… push_int overflow

a33bcd3 test: ensure push_int check i32::MIN of overflow error (Chris Hyunhum Cho)
c9988ba refactor: use match for OP_N push in push_int_unchecked (Chris Hyunhum Cho)

Pull request description:

  Follow up rust-bitcoin#3392

  c9988ba
  - refactor `push_int_unchecked` with match expression for cleaner code(many thanks for tcharding rust-bitcoin#3407).

  a33bcd3
  - ensure newly introduced safe `push_int` function as expected, testing if returns `Error::NumericOverflow` when `n` is `i32::MIN`

ACKs for top commit:
  tcharding:
    ACK a33bcd3
  apoelstra:
    ACK a33bcd3 successfully ran local tests

Tree-SHA512: 14f19d37f35b47e148b40c5017f0270c534c136d86be0c061cb476e1693130c5fc1bfc45a6f7c75a473022490c5f4e061cbc02640b1a616619ae721116e3cd54

bitcoin/src/blockdata/script/builder.rs

@@ -5,7 +5,7 @@ use core::fmt;
 use super::{opcode_to_verify, write_scriptint, Error, PushBytes, Script, ScriptBuf};
 use crate::locktime::absolute;
 use crate::opcodes::all::*;
-use crate::opcodes::{self, Opcode};
+use crate::opcodes::Opcode;
 use crate::prelude::Vec;
 use crate::script::{ScriptBufExt as _, ScriptBufExtPriv as _, ScriptExtPriv as _};
 use crate::Sequence;
@@ -46,20 +46,23 @@ impl Builder {
     ///
     /// Integers are encoded as little-endian signed-magnitude numbers, but there are dedicated
     /// opcodes to push some small integers.
-    /// It doesn't check whether the integer in the range of [-2^31 +1...2^31 -1].
+    ///
+    /// This function implements `CScript::push_int64` from Core `script.h`.
+    ///
+    /// > Numeric opcodes (OP_1ADD, etc) are restricted to operating on 4-byte integers.
+    /// > The semantics are subtle, though: operands must be in the range [-2^31 +1...2^31 -1],
+    /// > but results may overflow (and are valid as long as they are not used in a subsequent
+    /// > numeric operation). CScriptNum enforces those semantics by storing results as
+    /// > an int64 and allowing out-of-range values to be returned as a vector of bytes but
+    /// > throwing an exception if arithmetic is done or the result is interpreted as an integer.
+    ///
+    /// Does not check whether `n` is in the range of [-2^31 +1...2^31 -1].
     pub fn push_int_unchecked(self, n: i64) -> Builder {
-        // We can special-case -1, 1-16
-        if n == -1 || (1..=16).contains(&n) {
-            let opcode = Opcode::from((n - 1 + opcodes::OP_TRUE.to_u8() as i64) as u8);
-            self.push_opcode(opcode)
-        }
-        // We can also special-case zero
-        else if n == 0 {
-            self.push_opcode(opcodes::OP_0)
-        }
-        // Otherwise encode it as data
-        else {
-            self.push_int_non_minimal(n)
+        match n {
+            -1 => self.push_opcode(OP_PUSHNUM_NEG1),
+            0 => self.push_opcode(OP_PUSHBYTES_0),
+            1..=16 => self.push_opcode(Opcode::from(n as u8 + (OP_PUSHNUM_1.to_u8() - 1))),
+            _ => self.push_int_non_minimal(n),
         }
     }
 

bitcoin/src/blockdata/script/tests.rs

@@ -916,3 +916,9 @@ fn instruction_script_num_parse() {
         Some(Ok(Instruction::PushBytes(PushBytes::empty()))),
     );
 }
+
+#[test]
+fn script_push_int_overflow() {
+    // Only errors if `data == i32::MIN` (CScriptNum cannot have value -2^31).
+    assert_eq!(Builder::new().push_int(i32::MIN), Err(Error::NumericOverflow));
+}