From d961b9c4eeef02fa9c0f664e505e862bd366327b Mon Sep 17 00:00:00 2001 From: junderw Date: Fri, 2 Jun 2023 16:32:29 -0700 Subject: [PATCH] Fix minor comments on count_sigops PR --- bitcoin/src/blockdata/opcodes.rs | 49 +++++++++++++++++++----- bitcoin/src/blockdata/script/borrowed.rs | 14 +++++-- 2 files changed, 51 insertions(+), 12 deletions(-) diff --git a/bitcoin/src/blockdata/opcodes.rs b/bitcoin/src/blockdata/opcodes.rs index e75a5ed8..c35a2aee 100644 --- a/bitcoin/src/blockdata/opcodes.rs +++ b/bitcoin/src/blockdata/opcodes.rs @@ -153,7 +153,7 @@ all_opcodes! { OP_PUSHNUM_NEG1 => 0x4f, "Push the array `0x81` onto the stack."; OP_RESERVED => 0x50, "Synonym for OP_RETURN."; OP_PUSHNUM_1 => 0x51, "Push the array `0x01` onto the stack."; - OP_PUSHNUM_2 => 0x52, "the array `0x02` onto the stack."; + OP_PUSHNUM_2 => 0x52, "Push the array `0x02` onto the stack."; OP_PUSHNUM_3 => 0x53, "Push the array `0x03` onto the stack."; OP_PUSHNUM_4 => 0x54, "Push the array `0x04` onto the stack."; OP_PUSHNUM_5 => 0x55, "Push the array `0x05` onto the stack."; @@ -419,15 +419,8 @@ impl All { /// # Returns /// /// Returns `None` if `self` is not a PUSHNUM. - /// - /// # Examples - /// - /// ``` - /// use bitcoin::opcodes::all::*; - /// assert_eq!(OP_PUSHNUM_5.decode_pushnum().expect("pushnum"), 5) - /// ``` #[inline] - pub const fn decode_pushnum(self) -> Option { + pub(crate) const fn decode_pushnum(self) -> Option { const START: u8 = OP_PUSHNUM_1.code; const END: u8 = OP_PUSHNUM_16.code; match self.code { @@ -577,6 +570,44 @@ mod tests { assert_eq!(s, " OP_NOP"); } + #[test] + fn decode_pushnum() { + // Test all possible opcodes + // - Sanity check + assert_eq!(OP_PUSHNUM_1.code, 0x51_u8); + assert_eq!(OP_PUSHNUM_16.code, 0x60_u8); + for i in 0x00..=0xff_u8 { + let expected = match i { + // OP_PUSHNUM_1 ..= OP_PUSHNUM_16 + 0x51..=0x60 => Some(i - 0x50), + _ => None, + }; + assert_eq!(All::from(i).decode_pushnum(), expected); + } + + // Test the named opcode constants + // - This is the OP right before PUSHNUMs start + assert!(OP_RESERVED.decode_pushnum().is_none()); + assert_eq!(OP_PUSHNUM_1.decode_pushnum().expect("pushnum"), 1); + assert_eq!(OP_PUSHNUM_2.decode_pushnum().expect("pushnum"), 2); + assert_eq!(OP_PUSHNUM_3.decode_pushnum().expect("pushnum"), 3); + assert_eq!(OP_PUSHNUM_4.decode_pushnum().expect("pushnum"), 4); + assert_eq!(OP_PUSHNUM_5.decode_pushnum().expect("pushnum"), 5); + assert_eq!(OP_PUSHNUM_6.decode_pushnum().expect("pushnum"), 6); + assert_eq!(OP_PUSHNUM_7.decode_pushnum().expect("pushnum"), 7); + assert_eq!(OP_PUSHNUM_8.decode_pushnum().expect("pushnum"), 8); + assert_eq!(OP_PUSHNUM_9.decode_pushnum().expect("pushnum"), 9); + assert_eq!(OP_PUSHNUM_10.decode_pushnum().expect("pushnum"), 10); + assert_eq!(OP_PUSHNUM_11.decode_pushnum().expect("pushnum"), 11); + assert_eq!(OP_PUSHNUM_12.decode_pushnum().expect("pushnum"), 12); + assert_eq!(OP_PUSHNUM_13.decode_pushnum().expect("pushnum"), 13); + assert_eq!(OP_PUSHNUM_14.decode_pushnum().expect("pushnum"), 14); + assert_eq!(OP_PUSHNUM_15.decode_pushnum().expect("pushnum"), 15); + assert_eq!(OP_PUSHNUM_16.decode_pushnum().expect("pushnum"), 16); + // - This is the OP right after PUSHNUMs end + assert!(OP_NOP.decode_pushnum().is_none()); + } + #[test] fn classify_test() { let op174 = OP_CHECKMULTISIG; diff --git a/bitcoin/src/blockdata/script/borrowed.rs b/bitcoin/src/blockdata/script/borrowed.rs index f64ae326..5f1c747e 100644 --- a/bitcoin/src/blockdata/script/borrowed.rs +++ b/bitcoin/src/blockdata/script/borrowed.rs @@ -318,7 +318,7 @@ impl Script { crate::Amount::from_sat(sats) } - /// Count the sigops for this Script using accurate counting. + /// Counts the sigops for this Script using accurate counting. /// /// In Bitcoin Core, there are two ways to count sigops, "accurate" and "legacy". /// This method uses "accurate" counting. This means that OP_CHECKMULTISIG and its @@ -332,9 +332,13 @@ impl Script { /// (Note: taproot scripts don't count toward the sigop count of the block, /// nor do they have CHECKMULTISIG operations. This function does not count OP_CHECKSIGADD, /// so do not use this to try and estimate if a taproot script goes over the sigop budget.) + /// + /// # Errors + /// + /// If the Script is not able to be parsed to completion. pub fn count_sigops(&self) -> Result { self.count_sigops_internal(true) } - /// Count the sigops for this Script using legacy counting. + /// Counts the sigops for this Script using legacy counting. /// /// In Bitcoin Core, there are two ways to count sigops, "accurate" and "legacy". /// This method uses "legacy" counting. This means that OP_CHECKMULTISIG and its @@ -346,6 +350,10 @@ impl Script { /// (Note: taproot scripts don't count toward the sigop count of the block, /// nor do they have CHECKMULTISIG operations. This function does not count OP_CHECKSIGADD, /// so do not use this to try and estimate if a taproot script goes over the sigop budget.) + /// + /// # Errors + /// + /// If the Script is not able to be parsed to completion. pub fn count_sigops_legacy(&self) -> Result { self.count_sigops_internal(false) } @@ -364,7 +372,7 @@ impl Script { match (accurate, pushnum_cache) { (true, Some(pushnum)) => { // Add the number of pubkeys in the multisig as sigop count - n += pushnum as usize; + n += usize::from(pushnum); } _ => { // MAX_PUBKEYS_PER_MULTISIG from Bitcoin Core