From 654b2772b883a9c42e6629649f5237974718c0f6 Mon Sep 17 00:00:00 2001 From: Tobin Harding Date: Tue, 25 Jan 2022 09:24:10 +1100 Subject: [PATCH 1/6] Add passing unit tests for read_scriptbool In preparation for refactoring `read_scriptbool` add passing unit tests. --- src/blockdata/script.rs | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) diff --git a/src/blockdata/script.rs b/src/blockdata/script.rs index 88406702..dd3c99a1 100644 --- a/src/blockdata/script.rs +++ b/src/blockdata/script.rs @@ -1448,5 +1448,23 @@ mod test { assert!(instructions.next().is_none()); assert!(instructions.next().is_none()); } + + #[test] + fn read_scriptbool_zero_is_false() { + let v: Vec = vec![0x00, 0x00, 0x00, 0x00]; + assert!(!read_scriptbool(&v)); + + let v: Vec = vec![0x00, 0x00, 0x00, 0x80]; // With sign bit set. + assert!(!read_scriptbool(&v)); + } + + #[test] + fn read_scriptbool_non_zero_is_true() { + let v: Vec = vec![0x01, 0x00, 0x00, 0x00]; + assert!(read_scriptbool(&v)); + + let v: Vec = vec![0x01, 0x00, 0x00, 0x80]; // With sign bit set. + assert!(read_scriptbool(&v)); + } } From 373ea89a9a1b3dc4828cf4801419e1a30ff4901c Mon Sep 17 00:00:00 2001 From: Tobin Harding Date: Tue, 25 Jan 2022 08:55:29 +1100 Subject: [PATCH 2/6] Simplify read_scriptbool Refactor and simplify the logical operators in `read_scriptbool`. Refactor only, no logic changes. --- src/blockdata/script.rs | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/src/blockdata/script.rs b/src/blockdata/script.rs index dd3c99a1..ca68eea3 100644 --- a/src/blockdata/script.rs +++ b/src/blockdata/script.rs @@ -239,9 +239,12 @@ pub fn read_scriptint(v: &[u8]) -> Result { /// else as true", except that the overflow rules don't apply. #[inline] pub fn read_scriptbool(v: &[u8]) -> bool { - !(v.is_empty() || - ((v[v.len() - 1] == 0 || v[v.len() - 1] == 0x80) && - v.iter().rev().skip(1).all(|&w| w == 0))) + let last = match v.last() { + Some(last) => *last, + None => return false, + }; + + !((last == 0x00 || last == 0x80) && v.iter().rev().skip(1).all(|&b| b == 0)) } /// Read a script-encoded unsigned integer From f5512c4931423b6bc6f1f0496d424a0bd4ccdc91 Mon Sep 17 00:00:00 2001 From: Tobin Harding Date: Tue, 25 Jan 2022 09:43:17 +1100 Subject: [PATCH 3/6] Refactor is_p2pkh Refactor with the aim of simplifying `is_p2kh`. This function is covered sufficiently by current unit tests. Refactor only, no logic changes. --- src/blockdata/script.rs | 17 +++++++++++------ 1 file changed, 11 insertions(+), 6 deletions(-) diff --git a/src/blockdata/script.rs b/src/blockdata/script.rs index ca68eea3..adc702f7 100644 --- a/src/blockdata/script.rs +++ b/src/blockdata/script.rs @@ -440,12 +440,17 @@ impl Script { /// Checks whether a script pubkey is a p2pk output #[inline] pub fn is_p2pk(&self) -> bool { - (self.0.len() == 67 && - self.0[0] == opcodes::all::OP_PUSHBYTES_65.into_u8() && - self.0[66] == opcodes::all::OP_CHECKSIG.into_u8()) - || (self.0.len() == 35 && - self.0[0] == opcodes::all::OP_PUSHBYTES_33.into_u8() && - self.0[34] == opcodes::all::OP_CHECKSIG.into_u8()) + match self.len() { + 67 => { + self.0[0] == opcodes::all::OP_PUSHBYTES_65.into_u8() + && self.0[66] == opcodes::all::OP_CHECKSIG.into_u8() + } + 35 => { + self.0[0] == opcodes::all::OP_PUSHBYTES_33.into_u8() + && self.0[34] == opcodes::all::OP_CHECKSIG.into_u8() + } + _ => false + } } /// Checks whether a script pubkey is a Segregated Witness (segwit) program. From e54a2d653b27b35920e5de37fd16bf6e6f84cde6 Mon Sep 17 00:00:00 2001 From: Tobin Harding Date: Tue, 25 Jan 2022 09:57:48 +1100 Subject: [PATCH 4/6] Put && operator at front of line In an effort to make code containing multi-line logical AND clearer to read put the operator at the start of the line. --- src/blockdata/block.rs | 4 ++-- src/blockdata/script.rs | 38 +++++++++++++++++++------------------- 2 files changed, 21 insertions(+), 21 deletions(-) diff --git a/src/blockdata/block.rs b/src/blockdata/block.rs index 8ca1fa0c..28aeda59 100644 --- a/src/blockdata/block.rs +++ b/src/blockdata/block.rs @@ -195,8 +195,8 @@ impl Block { // commitment is in the last output that starts with below magic if let Some(pos) = coinbase.output.iter() .rposition(|o| { - o.script_pubkey.len () >= 38 && - o.script_pubkey[0..6] == [0x6a, 0x24, 0xaa, 0x21, 0xa9, 0xed] }) { + o.script_pubkey.len () >= 38 + && o.script_pubkey[0..6] == [0x6a, 0x24, 0xaa, 0x21, 0xa9, 0xed] }) { let commitment = WitnessCommitment::from_slice(&coinbase.output[pos].script_pubkey.as_bytes()[6..38]).unwrap(); // witness reserved value is in coinbase input witness let witness_vec: Vec<_> = coinbase.input[0].witness.iter().collect(); diff --git a/src/blockdata/script.rs b/src/blockdata/script.rs index adc702f7..80c5c580 100644 --- a/src/blockdata/script.rs +++ b/src/blockdata/script.rs @@ -420,21 +420,21 @@ impl Script { /// Checks whether a script pubkey is a p2sh output #[inline] pub fn is_p2sh(&self) -> bool { - self.0.len() == 23 && - self.0[0] == opcodes::all::OP_HASH160.into_u8() && - self.0[1] == opcodes::all::OP_PUSHBYTES_20.into_u8() && - self.0[22] == opcodes::all::OP_EQUAL.into_u8() + self.0.len() == 23 + && self.0[0] == opcodes::all::OP_HASH160.into_u8() + && self.0[1] == opcodes::all::OP_PUSHBYTES_20.into_u8() + && self.0[22] == opcodes::all::OP_EQUAL.into_u8() } /// Checks whether a script pubkey is a p2pkh output #[inline] pub fn is_p2pkh(&self) -> bool { - self.0.len() == 25 && - self.0[0] == opcodes::all::OP_DUP.into_u8() && - self.0[1] == opcodes::all::OP_HASH160.into_u8() && - self.0[2] == opcodes::all::OP_PUSHBYTES_20.into_u8() && - self.0[23] == opcodes::all::OP_EQUALVERIFY.into_u8() && - self.0[24] == opcodes::all::OP_CHECKSIG.into_u8() + self.0.len() == 25 + && self.0[0] == opcodes::all::OP_DUP.into_u8() + && self.0[1] == opcodes::all::OP_HASH160.into_u8() + && self.0[2] == opcodes::all::OP_PUSHBYTES_20.into_u8() + && self.0[23] == opcodes::all::OP_EQUALVERIFY.into_u8() + && self.0[24] == opcodes::all::OP_CHECKSIG.into_u8() } /// Checks whether a script pubkey is a p2pk output @@ -476,25 +476,25 @@ impl Script { /// Checks whether a script pubkey is a p2wsh output #[inline] pub fn is_v0_p2wsh(&self) -> bool { - self.0.len() == 34 && - self.witness_version() == Some(WitnessVersion::V0) && - self.0[1] == opcodes::all::OP_PUSHBYTES_32.into_u8() + self.0.len() == 34 + && self.witness_version() == Some(WitnessVersion::V0) + && self.0[1] == opcodes::all::OP_PUSHBYTES_32.into_u8() } /// Checks whether a script pubkey is a p2wpkh output #[inline] pub fn is_v0_p2wpkh(&self) -> bool { - self.0.len() == 22 && - self.witness_version() == Some(WitnessVersion::V0) && - self.0[1] == opcodes::all::OP_PUSHBYTES_20.into_u8() + self.0.len() == 22 + && self.witness_version() == Some(WitnessVersion::V0) + && self.0[1] == opcodes::all::OP_PUSHBYTES_20.into_u8() } /// Checks whether a script pubkey is a P2TR output #[inline] pub fn is_v1_p2tr(&self) -> bool { - self.0.len() == 34 && - self.witness_version() == Some(WitnessVersion::V1) && - self.0[1] == opcodes::all::OP_PUSHBYTES_32.into_u8() + self.0.len() == 34 + && self.witness_version() == Some(WitnessVersion::V1) + && self.0[1] == opcodes::all::OP_PUSHBYTES_32.into_u8() } /// Check if this is an OP_RETURN output From 4b6e86658dd408f64da881c79f66efcd15d6e964 Mon Sep 17 00:00:00 2001 From: Tobin Harding Date: Tue, 25 Jan 2022 10:07:43 +1100 Subject: [PATCH 5/6] Refactor is_provably_unspendable Refactor with the aim of making the code easier to read. Code path is covered by current unit tests. Refactor only, no logic changes. --- src/blockdata/script.rs | 19 +++++++++++++++---- 1 file changed, 15 insertions(+), 4 deletions(-) diff --git a/src/blockdata/script.rs b/src/blockdata/script.rs index 80c5c580..f71e9a02 100644 --- a/src/blockdata/script.rs +++ b/src/blockdata/script.rs @@ -499,14 +499,25 @@ impl Script { /// Check if this is an OP_RETURN output pub fn is_op_return (&self) -> bool { - !self.0.is_empty() && (opcodes::All::from(self.0[0]) == opcodes::all::OP_RETURN) + match self.0.first() { + Some(b) => *b == opcodes::all::OP_RETURN.into_u8(), + None => false + } } /// Whether a script can be proven to have no satisfying input pub fn is_provably_unspendable(&self) -> bool { - !self.0.is_empty() && - (opcodes::All::from(self.0[0]).classify(opcodes::ClassifyContext::Legacy) == opcodes::Class::ReturnOp || - opcodes::All::from(self.0[0]).classify(opcodes::ClassifyContext::Legacy) == opcodes::Class::IllegalOp) + use blockdata::opcodes::Class::{ReturnOp, IllegalOp}; + + match self.0.first() { + Some(b) => { + let first = opcodes::All::from(*b); + let class = first.classify(opcodes::ClassifyContext::Legacy); + + class == ReturnOp || class == IllegalOp + }, + None => false, + } } /// Gets the minimum value an output with this script should have in order to be From df7bb03a67519b0e7eba77a0a149aeab3dafc715 Mon Sep 17 00:00:00 2001 From: Tobin Harding Date: Wed, 26 Jan 2022 16:52:41 +1100 Subject: [PATCH 6/6] Simplify read_scriptbool Simplify `read_scriptbool` by doing: - Use `split_last` to get at the last element - Mask the last byte against ^0x80 instead of using two equality statements --- src/blockdata/script.rs | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) diff --git a/src/blockdata/script.rs b/src/blockdata/script.rs index f71e9a02..81d7568b 100644 --- a/src/blockdata/script.rs +++ b/src/blockdata/script.rs @@ -239,12 +239,10 @@ pub fn read_scriptint(v: &[u8]) -> Result { /// else as true", except that the overflow rules don't apply. #[inline] pub fn read_scriptbool(v: &[u8]) -> bool { - let last = match v.last() { - Some(last) => *last, - None => return false, - }; - - !((last == 0x00 || last == 0x80) && v.iter().rev().skip(1).all(|&b| b == 0)) + match v.split_last() { + Some((last, rest)) => !((last & !0x80 == 0x00) && rest.iter().all(|&b| b == 0)), + None => false, + } } /// Read a script-encoded unsigned integer