From fea09a0a940484f096cb51ae44de89dc115892b0 Mon Sep 17 00:00:00 2001 From: Steven Roose Date: Thu, 23 Jan 2020 01:28:12 +0000 Subject: [PATCH] Improve the Instructions iterator for scripts - Rename the `iter` method to `instructions`. - Add `instructions_minimal` for minimal-enforced iteration. - Iterator has `Result` as items. --- fuzz/fuzz_targets/deserialize_script.rs | 11 +-- src/blockdata/script.rs | 99 +++++++++++++------------ src/util/contracthash.rs | 8 +- 3 files changed, 64 insertions(+), 54 deletions(-) diff --git a/fuzz/fuzz_targets/deserialize_script.rs b/fuzz/fuzz_targets/deserialize_script.rs index e7c36de0..63ab573c 100644 --- a/fuzz/fuzz_targets/deserialize_script.rs +++ b/fuzz/fuzz_targets/deserialize_script.rs @@ -8,13 +8,14 @@ use bitcoin::consensus::encode; fn do_test(data: &[u8]) { let s: Result = encode::deserialize(data); if let Ok(script) = s { - let _: Vec = script.iter(false).collect(); - let enforce_min: Vec = script.iter(true).collect(); + let _: Result, script::Error> = script.instructions().collect(); let mut b = script::Builder::new(); - for ins in enforce_min { - match ins { - script::Instruction::Error(_) => return, + for ins in script.instructions_minimal() { + if ins.is_err() { + return; + } + match ins.ok().unwrap() { script::Instruction::Op(op) => { b = b.push_opcode(op); } script::Instruction::PushBytes(bytes) => { // Any one-byte pushes, except -0, which can be interpreted as numbers, should be diff --git a/src/blockdata/script.rs b/src/blockdata/script.rs index d71b936c..3dcd9d85 100644 --- a/src/blockdata/script.rs +++ b/src/blockdata/script.rs @@ -329,10 +329,21 @@ impl Script { /// opcodes, datapushes and errors. At most one error will be returned and then the /// iterator will end. To instead iterate over the script as sequence of bytes, treat /// it as a slice using `script[..]` or convert it to a vector using `into_bytes()`. - pub fn iter(&self, enforce_minimal: bool) -> Instructions { + /// + /// To force minimal pushes, use [instructions_minimal]. + pub fn instructions(&self) -> Instructions { Instructions { data: &self.0[..], - enforce_minimal: enforce_minimal, + enforce_minimal: false, + } + } + + /// Iterate over the script in the form of `Instruction`s while enforcing + /// minimal pushes. + pub fn instructions_minimal(&self) -> Instructions { + Instructions { + data: &self.0[..], + enforce_minimal: true, } } @@ -437,8 +448,6 @@ pub enum Instruction<'a> { PushBytes(&'a [u8]), /// Some non-push opcode Op(opcodes::All), - /// An opcode we were unable to parse - Error(Error) } /// Iterator over a script returning parsed opcodes @@ -448,9 +457,9 @@ pub struct Instructions<'a> { } impl<'a> Iterator for Instructions<'a> { - type Item = Instruction<'a>; + type Item = Result, Error>; - fn next(&mut self) -> Option> { + fn next(&mut self) -> Option, Error>> { if self.data.is_empty() { return None; } @@ -460,93 +469,93 @@ impl<'a> Iterator for Instructions<'a> { let n = n as usize; if self.data.len() < n + 1 { self.data = &[]; // Kill iterator so that it does not return an infinite stream of errors - return Some(Instruction::Error(Error::EarlyEndOfScript)); + return Some(Err(Error::EarlyEndOfScript)); } if self.enforce_minimal { if n == 1 && (self.data[1] == 0x81 || (self.data[1] > 0 && self.data[1] <= 16)) { self.data = &[]; - return Some(Instruction::Error(Error::NonMinimalPush)); + return Some(Err(Error::NonMinimalPush)); } } - let ret = Some(Instruction::PushBytes(&self.data[1..n+1])); + let ret = Some(Ok(Instruction::PushBytes(&self.data[1..n+1]))); self.data = &self.data[n + 1..]; ret } opcodes::Class::Ordinary(opcodes::Ordinary::OP_PUSHDATA1) => { if self.data.len() < 2 { self.data = &[]; - return Some(Instruction::Error(Error::EarlyEndOfScript)); + return Some(Err(Error::EarlyEndOfScript)); } let n = match read_uint(&self.data[1..], 1) { Ok(n) => n, Err(e) => { self.data = &[]; - return Some(Instruction::Error(e)); + return Some(Err(e)); } }; if self.data.len() < n + 2 { self.data = &[]; - return Some(Instruction::Error(Error::EarlyEndOfScript)); + return Some(Err(Error::EarlyEndOfScript)); } if self.enforce_minimal && n < 76 { self.data = &[]; - return Some(Instruction::Error(Error::NonMinimalPush)); + return Some(Err(Error::NonMinimalPush)); } - let ret = Some(Instruction::PushBytes(&self.data[2..n+2])); + let ret = Some(Ok(Instruction::PushBytes(&self.data[2..n+2]))); self.data = &self.data[n + 2..]; ret } opcodes::Class::Ordinary(opcodes::Ordinary::OP_PUSHDATA2) => { if self.data.len() < 3 { self.data = &[]; - return Some(Instruction::Error(Error::EarlyEndOfScript)); + return Some(Err(Error::EarlyEndOfScript)); } let n = match read_uint(&self.data[1..], 2) { Ok(n) => n, Err(e) => { self.data = &[]; - return Some(Instruction::Error(e)); + return Some(Err(e)); } }; if self.enforce_minimal && n < 0x100 { self.data = &[]; - return Some(Instruction::Error(Error::NonMinimalPush)); + return Some(Err(Error::NonMinimalPush)); } if self.data.len() < n + 3 { self.data = &[]; - return Some(Instruction::Error(Error::EarlyEndOfScript)); + return Some(Err(Error::EarlyEndOfScript)); } - let ret = Some(Instruction::PushBytes(&self.data[3..n + 3])); + let ret = Some(Ok(Instruction::PushBytes(&self.data[3..n + 3]))); self.data = &self.data[n + 3..]; ret } opcodes::Class::Ordinary(opcodes::Ordinary::OP_PUSHDATA4) => { if self.data.len() < 5 { self.data = &[]; - return Some(Instruction::Error(Error::EarlyEndOfScript)); + return Some(Err(Error::EarlyEndOfScript)); } let n = match read_uint(&self.data[1..], 4) { Ok(n) => n, Err(e) => { self.data = &[]; - return Some(Instruction::Error(e)); + return Some(Err(e)); } }; if self.enforce_minimal && n < 0x10000 { self.data = &[]; - return Some(Instruction::Error(Error::NonMinimalPush)); + return Some(Err(Error::NonMinimalPush)); } if self.data.len() < n + 5 { self.data = &[]; - return Some(Instruction::Error(Error::EarlyEndOfScript)); + return Some(Err(Error::EarlyEndOfScript)); } - let ret = Some(Instruction::PushBytes(&self.data[5..n + 5])); + let ret = Some(Ok(Instruction::PushBytes(&self.data[5..n + 5]))); self.data = &self.data[n + 5..]; ret } // Everything else we can push right through _ => { - let ret = Some(Instruction::Op(opcodes::All::from(self.data[0]))); + let ret = Some(Ok(Instruction::Op(opcodes::All::from(self.data[0])))); self.data = &self.data[1..]; ret } @@ -676,8 +685,8 @@ impl Default for Builder { impl From> for Builder { fn from(v: Vec) -> Builder { let script = Script(v.into_boxed_slice()); - let last_op = match script.iter(false).last() { - Some(Instruction::Op(op)) => Some(op), + let last_op = match script.instructions().last() { + Some(Ok(Instruction::Op(op))) => Some(op), _ => None, }; Builder(script.into_bytes(), last_op) @@ -1009,31 +1018,31 @@ mod test { let minimal = hex_script!("0169b2"); // minimal let nonminimal_alt = hex_script!("026900b2"); // non-minimal number but minimal push (should be OK) - let v_zero: Vec = zero.iter(true).collect(); - let v_zeropush: Vec = zeropush.iter(true).collect(); + let v_zero: Result, Error> = zero.instructions_minimal().collect(); + let v_zeropush: Result, Error> = zeropush.instructions_minimal().collect(); - let v_min: Vec = minimal.iter(true).collect(); - let v_nonmin: Vec = nonminimal.iter(true).collect(); - let v_nonmin_alt: Vec = nonminimal_alt.iter(true).collect(); - let slop_v_min: Vec = minimal.iter(false).collect(); - let slop_v_nonmin: Vec = nonminimal.iter(false).collect(); - let slop_v_nonmin_alt: Vec = nonminimal_alt.iter(false).collect(); + let v_min: Result, Error> = minimal.instructions_minimal().collect(); + let v_nonmin: Result, Error> = nonminimal.instructions_minimal().collect(); + let v_nonmin_alt: Result, Error> = nonminimal_alt.instructions_minimal().collect(); + let slop_v_min: Result, Error> = minimal.instructions().collect(); + let slop_v_nonmin: Result, Error> = nonminimal.instructions().collect(); + let slop_v_nonmin_alt: Result, Error> = nonminimal_alt.instructions().collect(); assert_eq!( - v_zero, + v_zero.unwrap(), vec![ Instruction::PushBytes(&[]), ] ); assert_eq!( - v_zeropush, + v_zeropush.unwrap(), vec![ Instruction::PushBytes(&[0]), ] ); assert_eq!( - v_min, + v_min.clone().unwrap(), vec![ Instruction::PushBytes(&[105]), Instruction::Op(opcodes::OP_NOP3), @@ -1041,23 +1050,21 @@ mod test { ); assert_eq!( - v_nonmin, - vec![ - Instruction::Error(Error::NonMinimalPush), - ] + v_nonmin.err().unwrap(), + Error::NonMinimalPush ); assert_eq!( - v_nonmin_alt, + v_nonmin_alt.clone().unwrap(), vec![ Instruction::PushBytes(&[105, 0]), Instruction::Op(opcodes::OP_NOP3), ] ); - assert_eq!(v_min, slop_v_min); - assert_eq!(v_min, slop_v_nonmin); - assert_eq!(v_nonmin_alt, slop_v_nonmin_alt); + assert_eq!(v_min.clone().unwrap(), slop_v_min.unwrap()); + assert_eq!(v_min.unwrap(), slop_v_nonmin.unwrap()); + assert_eq!(v_nonmin_alt.unwrap(), slop_v_nonmin_alt.unwrap()); } #[test] diff --git a/src/util/contracthash.rs b/src/util/contracthash.rs index 539944dd..345e9bc4 100644 --- a/src/util/contracthash.rs +++ b/src/util/contracthash.rs @@ -227,8 +227,11 @@ pub fn untemplate(script: &script::Script) -> Result<(Template, Vec), } let mut mode = Mode::SeekingKeys; - for instruction in script.iter(false) { - match instruction { + for instruction in script.instructions() { + if let Err(e) = instruction { + return Err(Error::Script(e)); + } + match instruction.unwrap() { script::Instruction::PushBytes(data) => { let n = data.len(); ret = match PublicKey::from_slice(data) { @@ -275,7 +278,6 @@ pub fn untemplate(script: &script::Script) -> Result<(Template, Vec), } ret = ret.push_opcode(op); } - script::Instruction::Error(e) => { return Err(Error::Script(e)); } } } Ok((Template::from(&ret[..]), retkeys))