diff --git a/CHANGELOG.md b/CHANGELOG.md index b2a21be759..8ff4c0c326 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,16 @@ #### Upcoming Changes +* feat: Add HintProcessor::execute_hint_extensive + refactor hint_ranges [#1491](https://github.com/lambdaclass/cairo-vm/pull/1491) + + * Add trait method `HintProcessorLogic::execute_hint_extensive`: + * This method has a similar behaviour to `HintProcessorLogic::execute_hint` but it also returns a `HintExtension` (type alias for `HashMap>>`) that can be used to extend the current map of hints used by the VM. This behaviour achieves what the `vm_load_data` primitive does for cairo-lang, and is needed to implement os hints. + * This method is now used by the VM to execute hints instead of `execute_hint`, but it's default implementation calls `execute_hint`, so current implementors of the `HintProcessor` trait won't notice any change. + + * Signature changes: + * `pub fn step_hint(&mut self, hint_executor: &mut dyn HintProcessor, exec_scopes: &mut ExecutionScopes, hint_datas: &mut Vec>, constants: &HashMap) -> Result<(), VirtualMachineError>` -> `pub fn step_hint(&mut self, hint_processor: &mut dyn HintProcessor, exec_scopes: &mut ExecutionScopes, hint_datas: &mut Vec>, hint_ranges: &mut HashMap, constants: &HashMap) -> Result<(), VirtualMachineError>` + * `pub fn step(&mut self, hint_executor: &mut dyn HintProcessor, exec_scopes: &mut ExecutionScopes, hint_data: &[Box], constants: &HashMap) -> Result<(), VirtualMachineError>` -> `pub fn step(&mut self, hint_processor: &mut dyn HintProcessor, exec_scopes: &mut ExecutionScopes, hint_datas: &mut Vec>, hint_ranges: &mut HashMap, constants: &HashMap) -> Result<(), VirtualMachineError>` + * BREAKING: Partially Revert `Optimize trace relocation #906` [#1492](https://github.com/lambdaclass/cairo-vm/pull/1492) * Remove methods `VirtualMachine::get_relocated_trace`& `VirtualMachine::relocate_trace`. diff --git a/vm/src/hint_processor/hint_processor_definition.rs b/vm/src/hint_processor/hint_processor_definition.rs index 6e2912af3d..7ab8bd7973 100644 --- a/vm/src/hint_processor/hint_processor_definition.rs +++ b/vm/src/hint_processor/hint_processor_definition.rs @@ -6,6 +6,7 @@ use crate::serde::deserialize_program::OffsetValue; use crate::serde::deserialize_program::Reference; use crate::types::exec_scope::ExecutionScopes; use crate::types::instruction::Register; +use crate::types::relocatable::Relocatable; use crate::vm::errors::hint_errors::HintError; use crate::vm::errors::vm_errors::VirtualMachineError; use crate::vm::runners::cairo_runner::ResourceTracker; @@ -18,14 +19,13 @@ use felt::Felt252; use arbitrary::Arbitrary; pub trait HintProcessorLogic { - //Executes the hint which's data is provided by a dynamic structure previously created by compile_hint + // Executes the hint which's data is provided by a dynamic structure previously created by compile_hint + // Note: The method used by the vm to execute hints is `execute_hint_extensive`. + // The default implementation for `execute_hint_extensive` calls this method, so it can be ignored unless loading hints during the vm run is needed. + // If you chose to implement `execute_hint_extensive` instead of using the default implementation, then this method will not be used. fn execute_hint( &mut self, - //Proxy to VM, contains refrences to necessary data - //+ MemoryProxy, which provides the necessary methods to manipulate memory vm: &mut VirtualMachine, - //Proxy to ExecutionScopes, provides the necessary methods to manipulate the scopes and - //access current scope variables exec_scopes: &mut ExecutionScopes, //Data structure that can be downcasted to the structure generated by compile_hint hint_data: &Box, @@ -52,8 +52,29 @@ pub trait HintProcessorLogic { ids_data: get_ids_data(reference_ids, references)?, })) } + + // Executes the hint which's data is provided by a dynamic structure previously created by compile_hint + // Also returns a map of hints to be loaded after the current hint is executed + // Note: This is the method used by the vm to execute hints, + // if you chose to implement this method instead of using the default implementation, then `execute_hint` will not be used + fn execute_hint_extensive( + &mut self, + vm: &mut VirtualMachine, + exec_scopes: &mut ExecutionScopes, + //Data structure that can be downcasted to the structure generated by compile_hint + hint_data: &Box, + //Constant values extracted from the program specification. + constants: &HashMap, + ) -> Result { + self.execute_hint(vm, exec_scopes, hint_data, constants)?; + Ok(HintExtension::default()) + } } +// A map of hints that can be used to extend the current map of hints for the vm run +// The map matches the pc at which the hints should be executed to a vec of compiled hints (Outputed by HintProcessor::CompileHint) +pub type HintExtension = HashMap>>; + pub trait HintProcessor: HintProcessorLogic + ResourceTracker {} impl HintProcessor for T where T: HintProcessorLogic + ResourceTracker {} diff --git a/vm/src/types/program.rs b/vm/src/types/program.rs index 2ba347e6a2..b6ccf89a11 100644 --- a/vm/src/types/program.rs +++ b/vm/src/types/program.rs @@ -34,6 +34,8 @@ use std::path::Path; #[cfg(all(feature = "arbitrary", feature = "std"))] use arbitrary::{Arbitrary, Unstructured}; +use super::relocatable::Relocatable; + // NOTE: `Program` has been split in two containing some data that will be deep-copied // and some that will be allocated on the heap inside an `Arc<_>`. // This is because it has been reported that cloning the whole structure when creating @@ -106,7 +108,7 @@ impl<'a> Arbitrary<'a> for SharedProgramData { pub(crate) struct HintsCollection { hints: Vec, /// This maps a PC to the range of hints in `hints` that correspond to it. - hints_ranges: Vec, + pub(crate) hints_ranges: HashMap, } impl HintsCollection { @@ -122,7 +124,7 @@ impl HintsCollection { let Some((max_hint_pc, full_len)) = bounds else { return Ok(HintsCollection { hints: Vec::new(), - hints_ranges: Vec::new(), + hints_ranges: HashMap::new(), }); }; @@ -131,14 +133,14 @@ impl HintsCollection { } let mut hints_values = Vec::with_capacity(full_len); - let mut hints_ranges = vec![None; max_hint_pc + 1]; + let mut hints_ranges = HashMap::new(); 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_ranges.insert(Relocatable::from((0_isize, *pc)), range); hints_values.extend_from_slice(&hs[..]); } @@ -151,29 +153,20 @@ impl HintsCollection { 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() - } } impl From<&HintsCollection> for BTreeMap> { fn from(hc: &HintsCollection) -> Self { let mut hint_map = BTreeMap::new(); - for (i, r) in hc.hints_ranges.iter().enumerate() { - let Some(r) = r else { - continue; - }; - hint_map.insert(i, hc.hints[r.0..r.0 + r.1.get()].to_owned()); + for (pc, r) in hc.hints_ranges.iter() { + hint_map.insert(pc.offset, hc.hints[r.0..r.0 + r.1.get()].to_owned()); } hint_map } } -/// Represents a range of hints corresponding to a PC. -/// -/// Is [`None`] if the range is empty, and it is [`Some`] tuple `(start, length)` otherwise. -type HintRange = Option<(usize, NonZeroUsize)>; +/// Represents a range of hints corresponding to a PC as a tuple `(start, length)`. +pub type HintRange = (usize, NonZeroUsize); #[cfg_attr(all(feature = "arbitrary", feature = "std"), derive(Arbitrary))] #[derive(Clone, Debug, PartialEq, Eq)] @@ -425,19 +418,14 @@ 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 - } - }) - }) + self.hints_ranges.iter().filter_map(|(pc, (start, len))| { + let end = start + len.get(); + if end <= self.hints.len() { + Some((pc.offset, &self.hints[*start..end])) + } else { + None + } + }) } } @@ -493,7 +481,7 @@ mod tests { ); assert_eq!( program.shared_program_data.hints_collection.hints_ranges, - Vec::new() + HashMap::new() ); } @@ -539,7 +527,7 @@ mod tests { ); assert_eq!( program.shared_program_data.hints_collection.hints_ranges, - Vec::new() + HashMap::new() ); } @@ -600,12 +588,10 @@ mod tests { .hints_collection .hints_ranges .iter() - .enumerate() - .filter_map(|(pc, r)| r.map(|(s, l)| (pc, (s, s + l.get())))) - .map(|(pc, (s, e))| { + .map(|(pc, (s, l))| { ( - pc, - program.shared_program_data.hints_collection.hints[s..e].to_vec(), + pc.offset, + program.shared_program_data.hints_collection.hints[*s..(s + l.get())].to_vec(), ) }) .collect(); @@ -1250,7 +1236,7 @@ mod tests { fn default_program() { let hints_collection = HintsCollection { hints: Vec::new(), - hints_ranges: Vec::new(), + hints_ranges: HashMap::new(), }; let shared_program_data = SharedProgramData { diff --git a/vm/src/vm/runners/cairo_runner.rs b/vm/src/vm/runners/cairo_runner.rs index 1845833ec5..fdbdeae0ac 100644 --- a/vm/src/vm/runners/cairo_runner.rs +++ b/vm/src/vm/runners/cairo_runner.rs @@ -553,23 +553,21 @@ impl CairoRunner { hint_processor: &mut dyn HintProcessor, ) -> Result<(), VirtualMachineError> { let references = &self.program.shared_program_data.reference_manager; - let hint_data = self.get_hint_data(references, hint_processor)?; + let mut hint_data = self.get_hint_data(references, hint_processor)?; + let mut hint_ranges = self + .program + .shared_program_data + .hints_collection + .hints_ranges + .clone(); #[cfg(feature = "hooks")] vm.execute_before_first_step(self, &hint_data)?; while vm.run_context.pc != address && !hint_processor.consumed() { - let hint_data = &self - .program - .shared_program_data - .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, &mut self.exec_scopes, - hint_data, + &mut hint_data, + &mut hint_ranges, &self.program.constants, )?; hint_processor.consume_step(); @@ -590,26 +588,24 @@ impl CairoRunner { hint_processor: &mut dyn HintProcessor, ) -> Result<(), VirtualMachineError> { let references = &self.program.shared_program_data.reference_manager; - let hint_data = self.get_hint_data(references, hint_processor)?; + let mut hint_data = self.get_hint_data(references, hint_processor)?; + let mut hint_ranges = self + .program + .shared_program_data + .hints_collection + .hints_ranges + .clone(); for remaining_steps in (1..=steps).rev() { if self.final_pc.as_ref() == Some(&vm.run_context.pc) { return Err(VirtualMachineError::EndOfProgram(remaining_steps)); } - let hint_data = self - .program - .shared_program_data - .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, &mut self.exec_scopes, - hint_data, + &mut hint_data, + &mut hint_ranges, &self.program.constants, )?; } diff --git a/vm/src/vm/vm_core.rs b/vm/src/vm/vm_core.rs index 0ac788aa6d..bfe5ffea65 100644 --- a/vm/src/vm/vm_core.rs +++ b/vm/src/vm/vm_core.rs @@ -1,5 +1,6 @@ use crate::stdlib::{any::Any, borrow::Cow, collections::HashMap, prelude::*}; +use crate::types::program::HintRange; use crate::{ hint_processor::hint_processor_definition::HintProcessor, types::{ @@ -24,6 +25,7 @@ use crate::{ }; use core::cmp::Ordering; +use core::num::NonZeroUsize; use felt::Felt252; use num_traits::{ToPrimitive, Zero}; @@ -440,15 +442,34 @@ impl VirtualMachine { pub fn step_hint( &mut self, - hint_executor: &mut dyn HintProcessor, + hint_processor: &mut dyn HintProcessor, exec_scopes: &mut ExecutionScopes, - hint_data: &[Box], + hint_datas: &mut Vec>, + hint_ranges: &mut HashMap, constants: &HashMap, ) -> Result<(), VirtualMachineError> { - for (hint_index, hint_data) in hint_data.iter().enumerate() { - hint_executor - .execute_hint(self, exec_scopes, hint_data, constants) - .map_err(|err| VirtualMachineError::Hint(Box::new((hint_index, err))))? + // Check if there is a hint range for the current pc + if let Some((s, l)) = hint_ranges.get(&self.run_context.pc) { + // Re-binding to avoid mutability problems + let s = *s; + // Execute each hint for the given range + for idx in s..(s + l.get()) { + let hint_extension = hint_processor + .execute_hint_extensive( + self, + exec_scopes, + hint_datas.get(idx).ok_or(VirtualMachineError::Unexpected)?, + constants, + ) + .map_err(|err| VirtualMachineError::Hint(Box::new((idx - s, err))))?; + // Update the hint_ranges & hint_datas with the hints added by the executed hint + for (hint_pc, hints) in hint_extension { + if let Ok(len) = NonZeroUsize::try_from(hints.len()) { + hint_ranges.insert(hint_pc, (hint_datas.len(), len)); + hint_datas.extend(hints); + } + } + } } Ok(()) } @@ -480,18 +501,25 @@ impl VirtualMachine { pub fn step( &mut self, - hint_executor: &mut dyn HintProcessor, + hint_processor: &mut dyn HintProcessor, exec_scopes: &mut ExecutionScopes, - hint_data: &[Box], + hint_datas: &mut Vec>, + hint_ranges: &mut HashMap, constants: &HashMap, ) -> Result<(), VirtualMachineError> { - self.step_hint(hint_executor, exec_scopes, hint_data, constants)?; + self.step_hint( + hint_processor, + exec_scopes, + hint_datas, + hint_ranges, + constants, + )?; #[cfg(feature = "hooks")] - self.execute_pre_step_instruction(hint_executor, exec_scopes, hint_data, constants)?; + self.execute_pre_step_instruction(hint_processor, exec_scopes, hint_datas, constants)?; self.step_instruction()?; #[cfg(feature = "hooks")] - self.execute_post_step_instruction(hint_executor, exec_scopes, hint_data, constants)?; + self.execute_post_step_instruction(hint_processor, exec_scopes, hint_datas, constants)?; Ok(()) } @@ -2649,7 +2677,8 @@ mod tests { vm.step( &mut hint_processor, exec_scopes_ref!(), - &Vec::new(), + &mut Vec::new(), + &mut HashMap::new(), &HashMap::new(), ), Ok(()) @@ -2878,8 +2907,9 @@ mod tests { vm.step( &mut hint_processor, exec_scopes_ref!(), - &Vec::new(), - &HashMap::new() + &mut Vec::new(), + &mut HashMap::new(), + &HashMap::new(), ), Ok(()) ); @@ -2960,7 +2990,8 @@ mod tests { vm.step( &mut hint_processor, exec_scopes_ref!(), - &Vec::new(), + &mut Vec::new(), + &mut HashMap::new(), &HashMap::new() ), Ok(()) @@ -3062,7 +3093,8 @@ mod tests { vm.step( &mut hint_processor, exec_scopes_ref!(), - &Vec::new(), + &mut Vec::new(), + &mut HashMap::new(), &HashMap::new() ), Ok(()) @@ -3083,7 +3115,8 @@ mod tests { vm.step( &mut hint_processor, exec_scopes_ref!(), - &Vec::new(), + &mut Vec::new(), + &mut HashMap::new(), &HashMap::new() ), Ok(()) @@ -3105,7 +3138,8 @@ mod tests { vm.step( &mut hint_processor, exec_scopes_ref!(), - &Vec::new(), + &mut Vec::new(), + &mut HashMap::new(), &HashMap::new() ), Ok(()) @@ -3625,7 +3659,7 @@ mod tests { #[cfg_attr(target_arch = "wasm32", wasm_bindgen_test)] fn test_step_for_preset_memory_with_alloc_hint() { let mut vm = vm!(true); - let hint_data = vec![any_box!(HintProcessorData::new_default( + let mut hint_data = vec![any_box!(HintProcessorData::new_default( "memory[ap] = segments.add()".to_string(), HashMap::new(), ))]; @@ -3663,17 +3697,16 @@ mod tests { //Run Steps for _ in 0..6 { - let hint_data = if vm.run_context.pc == (0, 0).into() { - &hint_data[0..] - } else { - &hint_data[0..0] - }; assert_matches!( vm.step( &mut hint_processor, exec_scopes_ref!(), - hint_data, - &HashMap::new() + &mut hint_data, + &mut HashMap::from([( + Relocatable::from((0, 0)), + (0_usize, NonZeroUsize::new(1).unwrap()) + )]), + &HashMap::new(), ), Ok(()) );