From c9988ba8cb352d4a4c37d23083345e05de665a89 Mon Sep 17 00:00:00 2001 From: Chris Hyunhum Cho Date: Thu, 26 Sep 2024 09:56:12 +0900 Subject: [PATCH 1/2] refactor: use match for OP_N push in push_int_unchecked --- bitcoin/src/blockdata/script/builder.rs | 31 ++++++++++++++----------- 1 file changed, 17 insertions(+), 14 deletions(-) diff --git a/bitcoin/src/blockdata/script/builder.rs b/bitcoin/src/blockdata/script/builder.rs index ebcac35b07..4c71c4aa5d 100644 --- a/bitcoin/src/blockdata/script/builder.rs +++ b/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), } } From a33bcd3654640ec7aa1cbf13fc9cd8b1be0d757d Mon Sep 17 00:00:00 2001 From: Chris Hyunhum Cho Date: Thu, 26 Sep 2024 10:05:44 +0900 Subject: [PATCH 2/2] test: ensure push_int check i32::MIN of overflow error --- bitcoin/src/blockdata/script/tests.rs | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/bitcoin/src/blockdata/script/tests.rs b/bitcoin/src/blockdata/script/tests.rs index a92c1caea4..480769c419 100644 --- a/bitcoin/src/blockdata/script/tests.rs +++ b/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)); +}