Skip to content

Commit 97c8005

Browse files
KixunilKixunil
committed
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: rust-bitcoin/rust-bitcoin#662 (review)
1 parent a2138aa commit 97c8005

File tree

1 file changed

+47
-81
lines changed

1 file changed

+47
-81
lines changed

‎src/blockdata/script.rs

Lines changed: 47 additions & 81 deletions
Original file line numberDiff line numberDiff line change
@@ -582,15 +582,15 @@ impl Script {
582582
/// To force minimal pushes, use [`Self::instructions_minimal`].
583583
pub fn instructions(&self) -> Instructions {
584584
Instructions {
585-
data: &self.0[..],
585+
data: self.0.iter(),
586586
enforce_minimal: false,
587587
}
588588
}
589589

590590
/// Iterates over the script in the form of `Instruction`s while enforcing minimal pushes.
591591
pub fn instructions_minimal(&self) -> Instructions {
592592
Instructions {
593-
data: &self.0[..],
593+
data: self.0.iter(),
594594
enforce_minimal: true,
595595
}
596596
}
@@ -729,113 +729,79 @@ pub enum Instruction<'a> {
729729

730730
/// Iterator over a script returning parsed opcodes.
731731
pub struct Instructions<'a> {
732-
data: &'a [u8],
732+
data: ::core::slice::Iter<'a, u8>,
733733
enforce_minimal: bool,
734734
}
735735

736+
impl<'a> Instructions<'a> {
737+
fn next_push_data_len(&mut self, len: usize, max: usize) -> Option<Result<Instruction<'a>, Error>> {
738+
let n = match read_uint_iter(&mut self.data, len) {
739+
Ok(n) => n,
740+
// We do exhaustive matching to not forget to handle new variants if we extend
741+
// `UintError` type.
742+
// Overflow actually means early end of script (script is definitely shorter
743+
// than `usize::max_value()`)
744+
Err(UintError::EarlyEndOfScript) | Err(UintError::NumericOverflow) => {
745+
let data_len = self.data.len();
746+
self.data.nth(data_len); // Kill iterator so that it does not return an infinite stream of errors
747+
return Some(Err(Error::EarlyEndOfScript));
748+
},
749+
};
750+
if self.enforce_minimal && n < max {
751+
let data_len = self.data.len();
752+
self.data.nth(data_len); // Kill iterator so that it does not return an infinite stream of errors
753+
return Some(Err(Error::NonMinimalPush));
754+
}
755+
let ret = Some(Ok(Instruction::PushBytes(&self.data.as_slice()[..n])));
756+
self.data.nth(n.max(1) - 1);
757+
ret
758+
}
759+
}
760+
736761
impl<'a> Iterator for Instructions<'a> {
737762
type Item = Result<Instruction<'a>, Error>;
738763

739764
fn next(&mut self) -> Option<Result<Instruction<'a>, Error>> {
740-
if self.data.is_empty() {
741-
return None;
742-
}
765+
let &byte = self.data.next()?;
743766

744767
// classify parameter does not really matter here since we are only using
745768
// it for pushes and nums
746-
match opcodes::All::from(self.data[0]).classify(opcodes::ClassifyContext::Legacy) {
769+
match opcodes::All::from(byte).classify(opcodes::ClassifyContext::Legacy) {
747770
opcodes::Class::PushBytes(n) => {
771+
// make sure safety argument holds across refactorings
772+
let n: u32 = n;
773+
// casting is safe because we don't support 16-bit architectures
748774
let n = n as usize;
749-
if self.data.len() < n + 1 {
750-
self.data = &[]; // Kill iterator so that it does not return an infinite stream of errors
775+
776+
if self.data.len() < n {
777+
let data_len = self.data.len();
778+
self.data.nth(data_len); // Kill iterator so that it does not return an infinite stream of errors
751779
return Some(Err(Error::EarlyEndOfScript));
752780
}
753781
if self.enforce_minimal {
754-
if n == 1 && (self.data[1] == 0x81 || (self.data[1] > 0 && self.data[1] <= 16)) {
755-
self.data = &[];
782+
// index acceess is safe because we checked the lenght above
783+
if n == 1 && (self.data.as_slice()[0] == 0x81 || (self.data.as_slice()[0] > 0 && self.data.as_slice()[0] <= 16)) {
784+
let data_len = self.data.len();
785+
self.data.nth(data_len); // Kill iterator so that it does not return an infinite stream of errors
756786
return Some(Err(Error::NonMinimalPush));
757787
}
758788
}
759-
let ret = Some(Ok(Instruction::PushBytes(&self.data[1..n+1])));
760-
self.data = &self.data[n + 1..];
789+
let ret = Some(Ok(Instruction::PushBytes(&self.data.as_slice()[..n])));
790+
self.data.nth(n.max(1) - 1);
761791
ret
762792
}
763793
opcodes::Class::Ordinary(opcodes::Ordinary::OP_PUSHDATA1) => {
764-
if self.data.len() < 2 {
765-
self.data = &[];
766-
return Some(Err(Error::EarlyEndOfScript));
767-
}
768-
let n = match read_uint(&self.data[1..], 1) {
769-
Ok(n) => n,
770-
Err(e) => {
771-
self.data = &[];
772-
return Some(Err(e));
773-
}
774-
};
775-
if self.data.len() < n + 2 {
776-
self.data = &[];
777-
return Some(Err(Error::EarlyEndOfScript));
778-
}
779-
if self.enforce_minimal && n < 76 {
780-
self.data = &[];
781-
return Some(Err(Error::NonMinimalPush));
782-
}
783-
let ret = Some(Ok(Instruction::PushBytes(&self.data[2..n+2])));
784-
self.data = &self.data[n + 2..];
785-
ret
794+
self.next_push_data_len(1, 76)
786795
}
787796
opcodes::Class::Ordinary(opcodes::Ordinary::OP_PUSHDATA2) => {
788-
if self.data.len() < 3 {
789-
self.data = &[];
790-
return Some(Err(Error::EarlyEndOfScript));
791-
}
792-
let n = match read_uint(&self.data[1..], 2) {
793-
Ok(n) => n,
794-
Err(e) => {
795-
self.data = &[];
796-
return Some(Err(e));
797-
}
798-
};
799-
if self.enforce_minimal && n < 0x100 {
800-
self.data = &[];
801-
return Some(Err(Error::NonMinimalPush));
802-
}
803-
if self.data.len() < n + 3 {
804-
self.data = &[];
805-
return Some(Err(Error::EarlyEndOfScript));
806-
}
807-
let ret = Some(Ok(Instruction::PushBytes(&self.data[3..n + 3])));
808-
self.data = &self.data[n + 3..];
809-
ret
797+
self.next_push_data_len(2, 0x100)
810798
}
811799
opcodes::Class::Ordinary(opcodes::Ordinary::OP_PUSHDATA4) => {
812-
if self.data.len() < 5 {
813-
self.data = &[];
814-
return Some(Err(Error::EarlyEndOfScript));
815-
}
816-
let n = match read_uint(&self.data[1..], 4) {
817-
Ok(n) => n,
818-
Err(e) => {
819-
self.data = &[];
820-
return Some(Err(e));
821-
}
822-
};
823-
if self.enforce_minimal && n < 0x10000 {
824-
self.data = &[];
825-
return Some(Err(Error::NonMinimalPush));
826-
}
827-
if self.data.len() < n + 5 {
828-
self.data = &[];
829-
return Some(Err(Error::EarlyEndOfScript));
830-
}
831-
let ret = Some(Ok(Instruction::PushBytes(&self.data[5..n + 5])));
832-
self.data = &self.data[n + 5..];
833-
ret
800+
self.next_push_data_len(4, 0x10000)
834801
}
835802
// Everything else we can push right through
836803
_ => {
837-
let ret = Some(Ok(Instruction::Op(opcodes::All::from(self.data[0]))));
838-
self.data = &self.data[1..];
804+
let ret = Some(Ok(Instruction::Op(opcodes::All::from(byte))));
839805
ret
840806
}
841807
}

0 commit comments

Comments
 (0)