Merge rust-bitcoin/rust-bitcoin#1898: Fix comments for #1890

d961b9c4ee Fix minor comments on count_sigops PR (junderw)

Pull request description:

  Fixing some comments that were left on #1890

ACKs for top commit:
  yancyribbens:
    ACK d961b9c4ee
  apoelstra:
    ACK d961b9c4ee
  tcharding:
    ACK d961b9c4ee

Tree-SHA512: caa04428eb7c09915964e4a7bae2d1fca2426317f3620d16e73e992269a99d7adb3d360affb954a173835661a9960cf760d29ae9861816b1a898c01428b0f2d6

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<u8> {
+    pub(crate) const fn decode_pushnum(self) -> Option<u8> {
         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;

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<usize, super::Error> { 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<usize, super::Error> {
         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