From 1f55edf718fb0fed099f2f22a6735276229408f5 Mon Sep 17 00:00:00 2001 From: Martin Habovstiak Date: Sat, 2 Oct 2021 20:27:39 +0200 Subject: [PATCH] Use iterator in `blockdata::script::Instructions` This refactors `blockdata::script::Instructions` to use `::core::slice::Iter<'a, u8>` instead of `&'a [u8]` to better express the intention and to avoid some slicing mistakes. Similarly to a previous change this uses a macro to deduplicate the common logic and the new `read_uint_iter` internal function to automatically advance the iterator. Addresses: https://github.com/rust-bitcoin/rust-bitcoin/pull/662#pullrequestreview-768320603 --- src/blockdata/script.rs | 128 +++++++++++++++------------------------- 1 file changed, 47 insertions(+), 81 deletions(-) diff --git a/src/blockdata/script.rs b/src/blockdata/script.rs index dd448fd8..57431150 100644 --- a/src/blockdata/script.rs +++ b/src/blockdata/script.rs @@ -582,7 +582,7 @@ impl Script { /// To force minimal pushes, use [`Self::instructions_minimal`]. pub fn instructions(&self) -> Instructions { Instructions { - data: &self.0[..], + data: self.0.iter(), enforce_minimal: false, } } @@ -590,7 +590,7 @@ impl Script { /// Iterates over the script in the form of `Instruction`s while enforcing minimal pushes. pub fn instructions_minimal(&self) -> Instructions { Instructions { - data: &self.0[..], + data: self.0.iter(), enforce_minimal: true, } } @@ -729,113 +729,79 @@ pub enum Instruction<'a> { /// Iterator over a script returning parsed opcodes. pub struct Instructions<'a> { - data: &'a [u8], + data: ::core::slice::Iter<'a, u8>, enforce_minimal: bool, } +impl<'a> Instructions<'a> { + fn next_push_data_len(&mut self, len: usize, max: usize) -> Option, Error>> { + let n = match read_uint_iter(&mut self.data, len) { + Ok(n) => n, + // We do exhaustive matching to not forget to handle new variants if we extend + // `UintError` type. + // Overflow actually means early end of script (script is definitely shorter + // than `usize::max_value()`) + Err(UintError::EarlyEndOfScript) | Err(UintError::NumericOverflow) => { + let data_len = self.data.len(); + self.data.nth(data_len); // Kill iterator so that it does not return an infinite stream of errors + return Some(Err(Error::EarlyEndOfScript)); + }, + }; + if self.enforce_minimal && n < max { + let data_len = self.data.len(); + self.data.nth(data_len); // Kill iterator so that it does not return an infinite stream of errors + return Some(Err(Error::NonMinimalPush)); + } + let ret = Some(Ok(Instruction::PushBytes(&self.data.as_slice()[..n]))); + self.data.nth(n.max(1) - 1); + ret + } +} + impl<'a> Iterator for Instructions<'a> { type Item = Result, Error>; fn next(&mut self) -> Option, Error>> { - if self.data.is_empty() { - return None; - } + let &byte = self.data.next()?; // classify parameter does not really matter here since we are only using // it for pushes and nums - match opcodes::All::from(self.data[0]).classify(opcodes::ClassifyContext::Legacy) { + match opcodes::All::from(byte).classify(opcodes::ClassifyContext::Legacy) { opcodes::Class::PushBytes(n) => { + // make sure safety argument holds across refactorings + let n: u32 = n; + // casting is safe because we don't support 16-bit architectures 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 + + if self.data.len() < n { + let data_len = self.data.len(); + self.data.nth(data_len); // Kill iterator so that it does not return an infinite stream of errors 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 = &[]; + // index acceess is safe because we checked the lenght above + if n == 1 && (self.data.as_slice()[0] == 0x81 || (self.data.as_slice()[0] > 0 && self.data.as_slice()[0] <= 16)) { + let data_len = self.data.len(); + self.data.nth(data_len); // Kill iterator so that it does not return an infinite stream of errors return Some(Err(Error::NonMinimalPush)); } } - let ret = Some(Ok(Instruction::PushBytes(&self.data[1..n+1]))); - self.data = &self.data[n + 1..]; + let ret = Some(Ok(Instruction::PushBytes(&self.data.as_slice()[..n]))); + self.data.nth(n.max(1) - 1); ret } opcodes::Class::Ordinary(opcodes::Ordinary::OP_PUSHDATA1) => { - if self.data.len() < 2 { - self.data = &[]; - return Some(Err(Error::EarlyEndOfScript)); - } - let n = match read_uint(&self.data[1..], 1) { - Ok(n) => n, - Err(e) => { - self.data = &[]; - return Some(Err(e)); - } - }; - if self.data.len() < n + 2 { - self.data = &[]; - return Some(Err(Error::EarlyEndOfScript)); - } - if self.enforce_minimal && n < 76 { - self.data = &[]; - return Some(Err(Error::NonMinimalPush)); - } - let ret = Some(Ok(Instruction::PushBytes(&self.data[2..n+2]))); - self.data = &self.data[n + 2..]; - ret + self.next_push_data_len(1, 76) } opcodes::Class::Ordinary(opcodes::Ordinary::OP_PUSHDATA2) => { - if self.data.len() < 3 { - self.data = &[]; - return Some(Err(Error::EarlyEndOfScript)); - } - let n = match read_uint(&self.data[1..], 2) { - Ok(n) => n, - Err(e) => { - self.data = &[]; - return Some(Err(e)); - } - }; - if self.enforce_minimal && n < 0x100 { - self.data = &[]; - return Some(Err(Error::NonMinimalPush)); - } - if self.data.len() < n + 3 { - self.data = &[]; - return Some(Err(Error::EarlyEndOfScript)); - } - let ret = Some(Ok(Instruction::PushBytes(&self.data[3..n + 3]))); - self.data = &self.data[n + 3..]; - ret + self.next_push_data_len(2, 0x100) } opcodes::Class::Ordinary(opcodes::Ordinary::OP_PUSHDATA4) => { - if self.data.len() < 5 { - self.data = &[]; - return Some(Err(Error::EarlyEndOfScript)); - } - let n = match read_uint(&self.data[1..], 4) { - Ok(n) => n, - Err(e) => { - self.data = &[]; - return Some(Err(e)); - } - }; - if self.enforce_minimal && n < 0x10000 { - self.data = &[]; - return Some(Err(Error::NonMinimalPush)); - } - if self.data.len() < n + 5 { - self.data = &[]; - return Some(Err(Error::EarlyEndOfScript)); - } - let ret = Some(Ok(Instruction::PushBytes(&self.data[5..n + 5]))); - self.data = &self.data[n + 5..]; - ret + self.next_push_data_len(4, 0x10000) } // Everything else we can push right through _ => { - let ret = Some(Ok(Instruction::Op(opcodes::All::from(self.data[0])))); - self.data = &self.data[1..]; + let ret = Some(Ok(Instruction::Op(opcodes::All::from(byte)))); ret } }