diff --git a/CHANGELOG.md b/CHANGELOG.md index 732599102f..373eeffa0b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,6 +4,8 @@ #### [0.8.6] - 2023-8-11 +* refactor: combine `Program.hints` and `Program.hints_ranges` into custom collection [#1366](https://github.com/lambdaclass/cairo-vm/pull/1366) + * fix: Fix div_mod [#1383](https://github.com/lambdaclass/cairo-vm/pull/1383) * Fixes `div_mod` function so that it behaves like the cairo-lang version diff --git a/vm/src/serde/deserialize_program.rs b/vm/src/serde/deserialize_program.rs index ee361a7d91..c15b974f06 100644 --- a/vm/src/serde/deserialize_program.rs +++ b/vm/src/serde/deserialize_program.rs @@ -19,7 +19,7 @@ use crate::{ types::{ errors::program_errors::ProgramError, instruction::Register, - program::{Program, SharedProgramData}, + program::{HintsCollection, Program, SharedProgramData}, relocatable::MaybeRelocatable, }, vm::runners::builtin_runner::{ @@ -483,13 +483,11 @@ pub fn parse_program_json( } } - let (hints, hints_ranges) = - Program::flatten_hints(&program_json.hints, program_json.data.len())?; + let hints_collection = HintsCollection::new(&program_json.hints, program_json.data.len())?; let shared_program_data = SharedProgramData { data: program_json.data, - hints, - hints_ranges, + hints_collection, main: entrypoint_pc, start, end, @@ -864,20 +862,11 @@ mod tests { } fn get_hints_as_map(program: &Program) -> HashMap> { - let (hints, ranges) = ( - &program.shared_program_data.hints, - &program.shared_program_data.hints_ranges, - ); - let mut hints_map = HashMap::new(); - - for (pc, range) in ranges.iter().enumerate() { - let Some((start, len)) = range else { - continue; - }; - // Associate the PC with its corresponding hints by mapping them - // to the elements in the proper range converted to vec. - hints_map.insert(pc, hints[*start..start + len.get()].to_vec()); - } + let hints_collection = &program.shared_program_data.hints_collection; + let hints_map: HashMap> = hints_collection + .iter() + .map(|(pc, hints)| (pc, hints.to_vec())) + .collect(); hints_map } diff --git a/vm/src/types/program.rs b/vm/src/types/program.rs index aefd91c7b0..5c3c8959d5 100644 --- a/vm/src/types/program.rs +++ b/vm/src/types/program.rs @@ -51,9 +51,7 @@ use arbitrary::{Arbitrary, Unstructured}; #[derive(Clone, Default, Debug, PartialEq, Eq)] pub(crate) struct SharedProgramData { pub(crate) data: Vec, - pub(crate) hints: Vec, - /// This maps a PC to the range of hints in `hints` that correspond to it. - pub(crate) hints_ranges: Vec, + pub(crate) hints_collection: HintsCollection, pub(crate) main: Option, //start and end labels will only be used in proof-mode pub(crate) start: Option, @@ -66,7 +64,7 @@ pub(crate) struct SharedProgramData { #[cfg(all(feature = "arbitrary", feature = "std"))] impl<'a> Arbitrary<'a> for SharedProgramData { - /// Create an arbitary [`SharedProgramData`] using `flatten_hints` to generate `hints` and + /// Create an arbitary [`SharedProgramData`] using `HintsCollection::new` to generate `hints` and /// `hints_ranges` fn arbitrary(u: &mut Unstructured<'a>) -> arbitrary::Result { let mut data = Vec::new(); @@ -81,12 +79,11 @@ impl<'a> Arbitrary<'a> for SharedProgramData { } let raw_hints = BTreeMap::>::arbitrary(u)?; - let (hints, hints_ranges) = Program::flatten_hints(&raw_hints, data.len()) + let hints_collection = HintsCollection::new(&raw_hints, data.len()) .map_err(|_| arbitrary::Error::IncorrectFormat)?; Ok(SharedProgramData { data, - hints, - hints_ranges, + hints_collection, main: Option::::arbitrary(u)?, start: Option::::arbitrary(u)?, end: Option::::arbitrary(u)?, @@ -98,6 +95,61 @@ impl<'a> Arbitrary<'a> for SharedProgramData { } } +#[derive(Clone, Default, Debug, PartialEq, Eq)] +pub(crate) struct HintsCollection { + hints: Vec, + /// This maps a PC to the range of hints in `hints` that correspond to it. + hints_ranges: Vec, +} + +impl HintsCollection { + pub(crate) fn new( + hints: &BTreeMap>, + program_length: usize, + ) -> Result { + let bounds = hints + .iter() + .map(|(pc, hs)| (*pc, hs.len())) + .reduce(|(max_hint_pc, full_len), (pc, len)| (max_hint_pc.max(pc), full_len + len)); + + let Some((max_hint_pc, full_len)) = bounds else { + return Ok(HintsCollection { + hints: Vec::new(), + hints_ranges: Vec::new(), + }); + }; + + if max_hint_pc >= program_length { + return Err(ProgramError::InvalidHintPc(max_hint_pc, program_length)); + } + + let mut hints_values = Vec::with_capacity(full_len); + let mut hints_ranges = vec![None; max_hint_pc + 1]; + + for (pc, hs) in hints.iter().filter(|(_, hs)| !hs.is_empty()) { + let range = ( + hints_values.len(), + NonZeroUsize::new(hs.len()).expect("empty vecs already filtered"), + ); + hints_ranges[*pc] = Some(range); + hints_values.extend_from_slice(&hs[..]); + } + + Ok(HintsCollection { + hints: hints_values, + hints_ranges, + }) + } + + pub fn iter_hints(&self) -> impl Iterator { + self.hints.iter() + } + + pub fn get_hint_range_for_pc(&self, pc: usize) -> Option { + self.hints_ranges.get(pc).cloned() + } +} + /// Represents a range of hints corresponding to a PC. /// /// Is [`None`] if the range is empty, and it is [`Some`] tuple `(start, length)` otherwise. @@ -135,15 +187,14 @@ impl Program { } let hints: BTreeMap<_, _> = hints.into_iter().collect(); - let (hints, hints_ranges) = Self::flatten_hints(&hints, data.len())?; + let hints_collection = HintsCollection::new(&hints, data.len())?; let shared_program_data = SharedProgramData { data, main, start: None, end: None, - hints, - hints_ranges, + hints_collection, error_message_attributes, instruction_locations, identifiers, @@ -156,38 +207,6 @@ impl Program { }) } - pub(crate) fn flatten_hints( - hints: &BTreeMap>, - program_length: usize, - ) -> Result<(Vec, Vec), ProgramError> { - let bounds = hints - .iter() - .map(|(pc, hs)| (*pc, hs.len())) - .reduce(|(max_hint_pc, full_len), (pc, len)| (max_hint_pc.max(pc), full_len + len)); - - let Some((max_hint_pc, full_len)) = bounds else { - return Ok((Vec::new(), Vec::new())); - }; - - if max_hint_pc >= program_length { - return Err(ProgramError::InvalidHintPc(max_hint_pc, program_length)); - } - - let mut hints_values = Vec::with_capacity(full_len); - let mut hints_ranges = vec![None; max_hint_pc + 1]; - - for (pc, hs) in hints.iter().filter(|(_, hs)| !hs.is_empty()) { - let range = ( - hints_values.len(), - NonZeroUsize::new(hs.len()).expect("empty vecs already filtered"), - ); - hints_ranges[*pc] = Some(range); - hints_values.extend_from_slice(&hs[..]); - } - - Ok((hints_values, hints_ranges)) - } - #[cfg(feature = "std")] pub fn from_file(path: &Path, entrypoint: Option<&str>) -> Result { let file_content = std::fs::read(path)?; @@ -310,6 +329,25 @@ impl TryFrom for Program { } } +#[cfg(test)] +impl HintsCollection { + pub fn iter(&self) -> impl Iterator { + self.hints_ranges + .iter() + .enumerate() + .filter_map(|(pc, range)| { + range.and_then(|(start, len)| { + let end = start + len.get(); + if end <= self.hints.len() { + Some((pc, &self.hints[start..end])) + } else { + None + } + }) + }) + } +} + #[cfg(test)] mod tests { use super::*; @@ -354,8 +392,14 @@ mod tests { assert_eq!(program.shared_program_data.data, data); assert_eq!(program.shared_program_data.main, None); assert_eq!(program.shared_program_data.identifiers, HashMap::new()); - assert_eq!(program.shared_program_data.hints, Vec::new()); - assert_eq!(program.shared_program_data.hints_ranges, Vec::new()); + assert_eq!( + program.shared_program_data.hints_collection.hints, + Vec::new() + ); + assert_eq!( + program.shared_program_data.hints_collection.hints_ranges, + Vec::new() + ); } #[test] @@ -412,11 +456,17 @@ mod tests { let program_hints: HashMap<_, _> = program .shared_program_data + .hints_collection .hints_ranges .iter() .enumerate() .filter_map(|(pc, r)| r.map(|(s, l)| (pc, (s, s + l.get())))) - .map(|(pc, (s, e))| (pc, program.shared_program_data.hints[s..e].to_vec())) + .map(|(pc, (s, e))| { + ( + pc, + program.shared_program_data.hints_collection.hints[s..e].to_vec(), + ) + }) .collect(); assert_eq!(program_hints, hints); } @@ -1020,10 +1070,14 @@ mod tests { #[test] #[cfg_attr(target_arch = "wasm32", wasm_bindgen_test)] fn default_program() { - let shared_program_data = SharedProgramData { - data: Vec::new(), + let hints_collection = HintsCollection { hints: Vec::new(), hints_ranges: Vec::new(), + }; + + let shared_program_data = SharedProgramData { + data: Vec::new(), + hints_collection, main: None, start: None, end: None, diff --git a/vm/src/utils.rs b/vm/src/utils.rs index 4f3f5eff9d..23d5e67dc1 100644 --- a/vm/src/utils.rs +++ b/vm/src/utils.rs @@ -246,7 +246,8 @@ pub mod test_utils { } pub(crate) use cairo_runner; - pub(crate) use crate::stdlib::sync::Arc; + pub(crate) use crate::stdlib::{collections::BTreeMap, sync::Arc}; + pub(crate) use crate::types::program::HintsCollection; pub(crate) use crate::types::program::Program; pub(crate) use crate::types::program::SharedProgramData; macro_rules! program { @@ -258,8 +259,7 @@ pub mod test_utils { ( $( $builtin_name: expr ),* ) => {{ let shared_program_data = SharedProgramData { data: crate::stdlib::vec::Vec::new(), - hints: crate::stdlib::vec::Vec::new(), - hints_ranges: crate::stdlib::vec::Vec::new(), + hints_collection: HintsCollection::new(&BTreeMap::new(), 0).unwrap(), main: None, start: None, end: None, @@ -344,13 +344,12 @@ pub mod test_utils { impl From for Program { fn from(val: ProgramFlat) -> Self { // NOTE: panics if hints have PCs higher than the program length - let (hints, hints_ranges) = - Program::flatten_hints(&val.hints, val.data.len()).expect("hints are valid"); + let hints_collection = + HintsCollection::new(&val.hints, val.data.len()).expect("hints are valid"); Program { shared_program_data: Arc::new(SharedProgramData { data: val.data, - hints, - hints_ranges, + hints_collection, main: val.main, start: val.start, end: val.end, @@ -604,6 +603,7 @@ pub mod test_utils { mod test { use crate::hint_processor::hint_processor_definition::HintProcessorLogic; use crate::stdlib::{cell::RefCell, collections::HashMap, rc::Rc, string::String, vec::Vec}; + use crate::types::program::HintsCollection; use crate::{ hint_processor::{ builtin_hint_processor::{ @@ -925,8 +925,7 @@ mod test { fn program_macro() { let shared_data = SharedProgramData { data: Vec::new(), - hints: Vec::new(), - hints_ranges: Vec::new(), + hints_collection: HintsCollection::new(&BTreeMap::new(), 0).unwrap(), main: None, start: None, end: None, @@ -950,8 +949,7 @@ mod test { fn program_macro_with_builtin() { let shared_data = SharedProgramData { data: Vec::new(), - hints: Vec::new(), - hints_ranges: Vec::new(), + hints_collection: HintsCollection::new(&BTreeMap::new(), 0).unwrap(), main: None, start: None, end: None, @@ -976,8 +974,7 @@ mod test { fn program_macro_custom_definition() { let shared_data = SharedProgramData { data: Vec::new(), - hints: Vec::new(), - hints_ranges: Vec::new(), + hints_collection: HintsCollection::new(&BTreeMap::new(), 0).unwrap(), main: Some(2), start: None, end: None, diff --git a/vm/src/vm/runners/cairo_runner.rs b/vm/src/vm/runners/cairo_runner.rs index a4811a5a78..7d55208a97 100644 --- a/vm/src/vm/runners/cairo_runner.rs +++ b/vm/src/vm/runners/cairo_runner.rs @@ -515,8 +515,8 @@ impl CairoRunner { ) -> Result>, VirtualMachineError> { self.program .shared_program_data - .hints - .iter() + .hints_collection + .iter_hints() .map(|hint| { hint_executor .compile_hint( @@ -549,12 +549,14 @@ impl CairoRunner { #[cfg(feature = "hooks")] vm.execute_before_first_step(self, &hint_data)?; while vm.run_context.pc != address && !hint_processor.consumed() { - let hint_data = self + let hint_data = &self .program .shared_program_data - .hints_ranges - .get(vm.run_context.pc.offset) - .and_then(|r| r.and_then(|(s, l)| hint_data.get(s..s + l.get()))) + .hints_collection + .get_hint_range_for_pc(vm.run_context.pc.offset) + .and_then(|range| { + range.and_then(|(start, length)| hint_data.get(start..start + length.get())) + }) .unwrap_or(&[]); vm.step( hint_processor, @@ -590,9 +592,11 @@ impl CairoRunner { let hint_data = self .program .shared_program_data - .hints_ranges - .get(vm.run_context.pc.offset) - .and_then(|r| r.and_then(|(s, l)| hint_data.get(s..s + l.get()))) + .hints_collection + .get_hint_range_for_pc(vm.run_context.pc.offset) + .and_then(|range| { + range.and_then(|(start, length)| hint_data.get(start..start + length.get())) + }) .unwrap_or(&[]); vm.step( hint_processor,