Fix: Script::count_sigops parsing should not return a Result

This commit is contained in:
junderw 2023-09-17 11:32:42 -07:00
parent e70836c871
commit 026a55809e
No known key found for this signature in database
GPG Key ID: B256185D3A971908
2 changed files with 20 additions and 28 deletions

View File

@ -350,11 +350,7 @@ impl Script {
/// (Note: taproot scripts don't count toward the sigop count of the block, /// (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, /// 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.) /// so do not use this to try and estimate if a taproot script goes over the sigop budget.)
/// pub fn count_sigops(&self) -> usize { self.count_sigops_internal(true) }
/// # 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) }
/// Counts the sigops for this Script using legacy counting. /// Counts the sigops for this Script using legacy counting.
/// ///
@ -368,20 +364,14 @@ impl Script {
/// (Note: taproot scripts don't count toward the sigop count of the block, /// (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, /// 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.) /// so do not use this to try and estimate if a taproot script goes over the sigop budget.)
/// pub fn count_sigops_legacy(&self) -> usize { self.count_sigops_internal(false) }
/// # 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)
}
fn count_sigops_internal(&self, accurate: bool) -> Result<usize, super::Error> { fn count_sigops_internal(&self, accurate: bool) -> usize {
let mut n = 0; let mut n = 0;
let mut pushnum_cache = None; let mut pushnum_cache = None;
for inst in self.instructions() { for inst in self.instructions() {
match inst? { match inst {
Instruction::Op(opcode) => { Ok(Instruction::Op(opcode)) => {
match opcode { match opcode {
OP_CHECKSIG | OP_CHECKSIGVERIFY => { OP_CHECKSIG | OP_CHECKSIGVERIFY => {
n += 1; n += 1;
@ -404,13 +394,15 @@ impl Script {
} }
} }
} }
Instruction::PushBytes(_) => { Ok(Instruction::PushBytes(_)) => {
pushnum_cache = None; pushnum_cache = None;
} }
// In Bitcoin Core it does `if (!GetOp(pc, opcode)) break;`
Err(_) => break,
} }
} }
Ok(n) n
} }
/// Iterates over the script instructions. /// Iterates over the script instructions.

View File

@ -616,7 +616,7 @@ fn test_script_get_sigop_count() {
.push_opcode(OP_EQUAL) .push_opcode(OP_EQUAL)
.into_script() .into_script()
.count_sigops(), .count_sigops(),
Ok(0) 0
); );
assert_eq!( assert_eq!(
Builder::new() Builder::new()
@ -627,7 +627,7 @@ fn test_script_get_sigop_count() {
.push_opcode(OP_CHECKSIG) .push_opcode(OP_CHECKSIG)
.into_script() .into_script()
.count_sigops(), .count_sigops(),
Ok(1) 1
); );
assert_eq!( assert_eq!(
Builder::new() Builder::new()
@ -639,7 +639,7 @@ fn test_script_get_sigop_count() {
.push_opcode(OP_PUSHNUM_1) .push_opcode(OP_PUSHNUM_1)
.into_script() .into_script()
.count_sigops(), .count_sigops(),
Ok(1) 1
); );
let multi = Builder::new() let multi = Builder::new()
.push_opcode(OP_PUSHNUM_1) .push_opcode(OP_PUSHNUM_1)
@ -649,8 +649,8 @@ fn test_script_get_sigop_count() {
.push_opcode(OP_PUSHNUM_3) .push_opcode(OP_PUSHNUM_3)
.push_opcode(OP_CHECKMULTISIG) .push_opcode(OP_CHECKMULTISIG)
.into_script(); .into_script();
assert_eq!(multi.count_sigops(), Ok(3)); assert_eq!(multi.count_sigops(), 3);
assert_eq!(multi.count_sigops_legacy(), Ok(20)); assert_eq!(multi.count_sigops_legacy(), 20);
let multi_verify = Builder::new() let multi_verify = Builder::new()
.push_opcode(OP_PUSHNUM_1) .push_opcode(OP_PUSHNUM_1)
.push_slice([3; 33]) .push_slice([3; 33])
@ -660,8 +660,8 @@ fn test_script_get_sigop_count() {
.push_opcode(OP_CHECKMULTISIGVERIFY) .push_opcode(OP_CHECKMULTISIGVERIFY)
.push_opcode(OP_PUSHNUM_1) .push_opcode(OP_PUSHNUM_1)
.into_script(); .into_script();
assert_eq!(multi_verify.count_sigops(), Ok(3)); assert_eq!(multi_verify.count_sigops(), 3);
assert_eq!(multi_verify.count_sigops_legacy(), Ok(20)); assert_eq!(multi_verify.count_sigops_legacy(), 20);
let multi_nopushnum_pushdata = Builder::new() let multi_nopushnum_pushdata = Builder::new()
.push_opcode(OP_PUSHNUM_1) .push_opcode(OP_PUSHNUM_1)
.push_slice([3; 33]) .push_slice([3; 33])
@ -669,8 +669,8 @@ fn test_script_get_sigop_count() {
.push_slice([3; 33]) .push_slice([3; 33])
.push_opcode(OP_CHECKMULTISIG) .push_opcode(OP_CHECKMULTISIG)
.into_script(); .into_script();
assert_eq!(multi_nopushnum_pushdata.count_sigops(), Ok(20)); assert_eq!(multi_nopushnum_pushdata.count_sigops(), 20);
assert_eq!(multi_nopushnum_pushdata.count_sigops_legacy(), Ok(20)); assert_eq!(multi_nopushnum_pushdata.count_sigops_legacy(), 20);
let multi_nopushnum_op = Builder::new() let multi_nopushnum_op = Builder::new()
.push_opcode(OP_PUSHNUM_1) .push_opcode(OP_PUSHNUM_1)
.push_slice([3; 33]) .push_slice([3; 33])
@ -678,8 +678,8 @@ fn test_script_get_sigop_count() {
.push_opcode(OP_DROP) .push_opcode(OP_DROP)
.push_opcode(OP_CHECKMULTISIG) .push_opcode(OP_CHECKMULTISIG)
.into_script(); .into_script();
assert_eq!(multi_nopushnum_op.count_sigops(), Ok(20)); assert_eq!(multi_nopushnum_op.count_sigops(), 20);
assert_eq!(multi_nopushnum_op.count_sigops_legacy(), Ok(20)); assert_eq!(multi_nopushnum_op.count_sigops_legacy(), 20);
} }
#[test] #[test]