Repository: rust-bitcoin/rust-bitcoin
Pull request #1890 (2023-06-01 02:24:05)

[script] Add method get_sigop_count
Planning to also add methods for the various parts of Transaction etc. to eventually allow for easier sigops calculation.

Bare multisig is making a comeback, which is causing a large amount of transactions' effective vSizes (for fee calculation) to be dependent on the sigop count.

This is a first step at making those transactions easier to estimate fees for / template blocks for etc.

junderw: A preview of what's to come:

https://github.com/junderw/rust-bitcoin/compare/feat/getsigopcount...feat/wip-sigop-tx

This is very rough, and I'm assuming it will need to be behind the consensus flag and needs tests... but this is a rough WIP idea of where I'm trying to go with this.

Eventually, we want Esplora to return sigop based vSize as well (if it's higher).

But for now (this PR) I'm putting it off until we discuss here.
junderw: Continuing on the thread of things that aren't rust-like, perhaps we should make two methods, `get_sigop_count` and `get_sigop_count_legacy` which used accurate = true and false respectively.

Either that or create a bool-enum that's more descriptive, like `Accurate, Legacy`... any thoughts?
apoelstra: I'd prefer two methods to an enum, but I don't feel strongly.
tcharding: Now we definitely want a single commit, this is going to be annoying for others to review :)
junderw: Done.
tcharding: hmmm, I don't really like the co-authored-by tag but because you put it in it forced me to look a lot closer at the code so I guess that's a win. (All I did was give some review suggestions so I totally don't need attribution, but thanks for the thought.)

To the best of my knowledge this PR mirrors the behavior in Core `CScript::GetSigOpCount`.

ACK 85e4672f2ba07c9a7f1380f0b3a4ceaeffdb3fa2

junderw: I think I just invented a new underhanded way to get a review! hahahahahah /s
Kixunil: I like changing it to two methods, I was thinking about it before but forgot to mention it. Bit more bikeshedding: in Rust we tend to *not* use the `get_` prefix, so maybe get rid of it? And maybe even rename the method to `count_sigops` to signal this has linear complexity? 

Patch:
diff --git a/bitcoin/src/blockdata/opcodes.rs b/bitcoin/src/blockdata/opcodes.rs
index 76ddbce8..e75a5ed8 100644
--- a/bitcoin/src/blockdata/opcodes.rs
+++ b/bitcoin/src/blockdata/opcodes.rs
@@ -411,6 +411,30 @@ pub fn classify(self, ctx: ClassifyContext) -> Class {
     /// Encodes [`All`] as a byte.
     #[inline]
     pub const fn to_u8(self) -> u8 { self.code }
+
+    /// Encodes PUSHNUM [`All`] as a `u8` representing its number (1-16).
+    ///
+    /// Does not convert `OP_FALSE` to 0. Only `1` to `OP_PUSHNUM_16` are covered.
+    ///
+    /// # 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> {
+        const START: u8 = OP_PUSHNUM_1.code;
+        const END: u8 = OP_PUSHNUM_16.code;
+        match self.code {
+            START..=END => Some(self.code - START + 1),
+            _ => None,
+        }
+    }
 }
 
 impl From<u8> for All {
diff --git a/bitcoin/src/blockdata/script/borrowed.rs b/bitcoin/src/blockdata/script/borrowed.rs
index aad2f557..f64ae326 100644
--- a/bitcoin/src/blockdata/script/borrowed.rs
+++ b/bitcoin/src/blockdata/script/borrowed.rs
@@ -318,6 +318,75 @@ pub fn dust_value(&self) -> crate::Amount {
         crate::Amount::from_sat(sats)
     }
 
+    /// Count 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
+    /// verify variant count for N sigops where N is the number of pubkeys used in the
+    /// multisig. However, it will count for 20 sigops if CHECKMULTISIG is not preceeded by an
+    /// OP_PUSHNUM from 1 - 16 (this would be an invalid script)
+    ///
+    /// Bitcoin Core uses accurate counting for sigops contained within redeemScripts (P2SH)
+    /// and witnessScripts (P2WSH) only. It uses legacy for sigops in scriptSigs and scriptPubkeys.
+    ///
+    /// (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.)
+    pub fn count_sigops(&self) -> Result<usize, super::Error> { self.count_sigops_internal(true) }
+
+    /// Count 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
+    /// verify variant count for 20 sigops.
+    ///
+    /// Bitcoin Core uses legacy counting for sigops contained within scriptSigs and
+    /// scriptPubkeys. It uses accurate for redeemScripts (P2SH) and witnessScripts (P2WSH).
+    ///
+    /// (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.)
+    pub fn count_sigops_legacy(&self) -> Result<usize, super::Error> {
+        self.count_sigops_internal(false)
+    }
+
+    fn count_sigops_internal(&self, accurate: bool) -> Result<usize, super::Error> {
+        let mut n = 0;
+        let mut pushnum_cache = None;
+        for inst in self.instructions() {
+            match inst? {
+                Instruction::Op(opcode) => {
+                    match opcode {
+                        OP_CHECKSIG | OP_CHECKSIGVERIFY => {
+                            n += 1;
+                        }
+                        OP_CHECKMULTISIG | OP_CHECKMULTISIGVERIFY => {
+                            match (accurate, pushnum_cache) {
+                                (true, Some(pushnum)) => {
+                                    // Add the number of pubkeys in the multisig as sigop count
+                                    n += pushnum as usize;
+                                }
+                                _ => {
+                                    // MAX_PUBKEYS_PER_MULTISIG from Bitcoin Core
+                                    // https://github.com/bitcoin/bitcoin/blob/v25.0/src/script/script.h#L29-L30
+                                    n += 20;
+                                }
+                            }
+                        }
+                        _ => {
+                            pushnum_cache = opcode.decode_pushnum();
+                        }
+                    }
+                }
+                Instruction::PushBytes(_) => {
+                    pushnum_cache = None;
+                }
+            }
+        }
+
+        Ok(n)
+    }
+
     /// Iterates over the script instructions.
     ///
     /// Each returned item is a nested enum covering opcodes, datapushes and errors.


Test patch:
diff --git a/bitcoin/src/blockdata/script/tests.rs b/bitcoin/src/blockdata/script/tests.rs
index 7b4a68a3..c112e25f 100644
--- a/bitcoin/src/blockdata/script/tests.rs
+++ b/bitcoin/src/blockdata/script/tests.rs
@@ -607,6 +607,82 @@ fn defult_dust_value_tests() {
     assert_eq!(script_p2pkh.dust_value(), crate::Amount::from_sat(546));
 }
 
+#[test]
+fn test_script_get_sigop_count() {
+    assert_eq!(
+        Builder::new()
+            .push_opcode(OP_DUP)
+            .push_opcode(OP_HASH160)
+            .push_slice([42; 20])
+            .push_opcode(OP_EQUAL)
+            .into_script()
+            .count_sigops(),
+        Ok(0)
+    );
+    assert_eq!(
+        Builder::new()
+            .push_opcode(OP_DUP)
+            .push_opcode(OP_HASH160)
+            .push_slice([42; 20])
+            .push_opcode(OP_EQUALVERIFY)
+            .push_opcode(OP_CHECKSIG)
+            .into_script()
+            .count_sigops(),
+        Ok(1)
+    );
+    assert_eq!(
+        Builder::new()
+            .push_opcode(OP_DUP)
+            .push_opcode(OP_HASH160)
+            .push_slice([42; 20])
+            .push_opcode(OP_EQUALVERIFY)
+            .push_opcode(OP_CHECKSIGVERIFY)
+            .push_opcode(OP_PUSHNUM_1)
+            .into_script()
+            .count_sigops(),
+        Ok(1)
+    );
+    let multi = Builder::new()
+        .push_opcode(OP_PUSHNUM_1)
+        .push_slice([3; 33])
+        .push_slice([3; 33])
+        .push_slice([3; 33])
+        .push_opcode(OP_PUSHNUM_3)
+        .push_opcode(OP_CHECKMULTISIG)
+        .into_script();
+    assert_eq!(multi.count_sigops(), Ok(3));
+    assert_eq!(multi.count_sigops_legacy(), Ok(20));
+    let multi_verify = Builder::new()
+        .push_opcode(OP_PUSHNUM_1)
+        .push_slice([3; 33])
+        .push_slice([3; 33])
+        .push_slice([3; 33])
+        .push_opcode(OP_PUSHNUM_3)
+        .push_opcode(OP_CHECKMULTISIGVERIFY)
+        .push_opcode(OP_PUSHNUM_1)
+        .into_script();
+    assert_eq!(multi_verify.count_sigops(), Ok(3));
+    assert_eq!(multi_verify.count_sigops_legacy(), Ok(20));
+    let multi_nopushnum_pushdata = Builder::new()
+        .push_opcode(OP_PUSHNUM_1)
+        .push_slice([3; 33])
+        .push_slice([3; 33])
+        .push_slice([3; 33])
+        .push_opcode(OP_CHECKMULTISIG)
+        .into_script();
+    assert_eq!(multi_nopushnum_pushdata.count_sigops(), Ok(20));
+    assert_eq!(multi_nopushnum_pushdata.count_sigops_legacy(), Ok(20));
+    let multi_nopushnum_op = Builder::new()
+        .push_opcode(OP_PUSHNUM_1)
+        .push_slice([3; 33])
+        .push_slice([3; 33])
+        .push_opcode(OP_DROP)
+        .push_opcode(OP_CHECKMULTISIG)
+        .into_script();
+    assert_eq!(multi_nopushnum_op.count_sigops(), Ok(20));
+    assert_eq!(multi_nopushnum_op.count_sigops_legacy(), Ok(20));
+}
+
 #[test]
 #[cfg(feature = "serde")]
 fn test_script_serde_human_and_not() {