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

@@ -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<Result<Instruction<'a>, 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<Instruction<'a>, Error>;
 
     fn next(&mut self) -> Option<Result<Instruction<'a>, Error>> {
-        if self.data.is_empty() {
+        let &byte = self.data.next()?;
-            return None;
-        }
 
         // 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)) {
+                    // index acceess is safe because we checked the lenght above
-                        self.data = &[];
+                    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])));
+                let ret = Some(Ok(Instruction::PushBytes(&self.data.as_slice()[..n])));
-                self.data = &self.data[n + 1..];
+                self.data.nth(n.max(1) - 1);
                 ret
             }
             opcodes::Class::Ordinary(opcodes::Ordinary::OP_PUSHDATA1) => {
-                if self.data.len() < 2 {
+                self.next_push_data_len(1, 76)
-                    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
             }
             opcodes::Class::Ordinary(opcodes::Ordinary::OP_PUSHDATA2) => {
-                if self.data.len() < 3 {
+                self.next_push_data_len(2, 0x100)
-                    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
             }
             opcodes::Class::Ordinary(opcodes::Ordinary::OP_PUSHDATA4) => {
-                if self.data.len() < 5 {
+                self.next_push_data_len(4, 0x10000)
-                    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
             }
             // Everything else we can push right through
             _ => {
-                let ret = Some(Ok(Instruction::Op(opcodes::All::from(self.data[0]))));
+                let ret = Some(Ok(Instruction::Op(opcodes::All::from(byte))));
-                self.data = &self.data[1..];
                 ret
             }
         }