diff --git a/CHANGELOG.md b/CHANGELOG.md index d6396b439b..ce9af82697 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,15 @@ #### Upcoming Changes +* Use CairoArg enum instead of Any in CairoRunner::run_from_entrypoint [#686](https://github.com/lambdaclass/cairo-rs/pull/686) + * Public Api changes: + * Remove `Result` from `MaybeRelocatable::mod_floor`, it now returns a `MaybeRelocatable` + * Add struct `CairoArg` + * Change `arg` argument of `CairoRunner::run_from_entrypoint` from `Vec<&dyn Any>` to `&[&CairoArg]` + * Remove argument `typed_args` from `CairoRunner::run_from_entrypoint` + * Remove no longer used method `gen_typed_arg` from `VirtualMachine` & `MemorySegmentManager` + * Add methods `MemorySegmentManager::gen_cairo_arg` & `MemorySegmentManager::write_simple_args` as typed counterparts to `MemorySegmentManager::gen_arg` & `MemorySegmentManager::write_arg` + #### [0.1.1] - 2023-01-11 * Add input file contents to traceback [#666](https://github.com/lambdaclass/cairo-rs/pull/666/files) diff --git a/src/vm/runners/cairo_runner.rs b/src/vm/runners/cairo_runner.rs index 866acb22bf..8c15613a9e 100644 --- a/src/vm/runners/cairo_runner.rs +++ b/src/vm/runners/cairo_runner.rs @@ -22,7 +22,7 @@ use crate::{ }, security::verify_secure_runner, trace::get_perm_range_check_limits, - vm_memory::{memory::RelocateValue, memory_segments::gen_typed_args}, + vm_memory::memory::RelocateValue, { runners::builtin_runner::{ BitwiseBuiltinRunner, BuiltinRunner, EcOpBuiltinRunner, HashBuiltinRunner, @@ -44,6 +44,24 @@ use std::{ use super::builtin_runner::KeccakBuiltinRunner; +#[derive(Clone, Debug, Eq, PartialEq)] +pub enum CairoArg { + Single(MaybeRelocatable), + Array(Vec), +} + +impl From for CairoArg { + fn from(other: MaybeRelocatable) -> Self { + CairoArg::Single(other) + } +} + +impl From> for CairoArg { + fn from(other: Vec) -> Self { + CairoArg::Array(other) + } +} + pub struct CairoRunner { pub(crate) program: Program, layout: CairoLayout, @@ -937,28 +955,15 @@ impl CairoRunner { pub fn run_from_entrypoint( &mut self, entrypoint: usize, - args: Vec<&dyn Any>, - typed_args: bool, + args: &[&CairoArg], verify_secure: bool, - _apply_modulo_to_args: bool, vm: &mut VirtualMachine, hint_processor: &mut dyn HintProcessor, ) -> Result<(), VirtualMachineError> { - let stack = if typed_args { - if args.len() != 1 { - return Err(VirtualMachineError::InvalidArgCount(1, args.len())); - } - - gen_typed_args(args)? - } else { - let mut stack = Vec::new(); - for arg in args { - stack.push(vm.segments.gen_arg(arg, &mut vm.memory)?); - } - - stack - }; - + let stack = args + .iter() + .map(|arg| vm.segments.gen_cairo_arg(arg, &mut vm.memory)) + .collect::, VirtualMachineError>>()?; let return_fp = vm.segments.add(&mut vm.memory); let end = self.initialize_function_entrypoint(vm, entrypoint, stack, return_fp.into())?; @@ -3333,116 +3338,6 @@ mod tests { ); } - /// Test that the call to .run_from_entrypoint() with args.count() != 1 when - /// typed_args is true fails. - #[test] - fn run_from_entrypoint_typed_args_invalid_arg_count() { - let program = - Program::from_file(Path::new("cairo_programs/not_main.json"), Some("main")).unwrap(); - let mut cairo_runner = cairo_runner!(program); - let mut vm = vm!(); - let mut hint_processor = BuiltinHintProcessor::new_empty(); - - let entrypoint = program - .identifiers - .get("__main__.not_main") - .unwrap() - .pc - .unwrap(); - assert_eq!( - cairo_runner.run_from_entrypoint( - entrypoint, - vec![], - true, - true, - true, - &mut vm, - &mut hint_processor, - ), - Err(VirtualMachineError::InvalidArgCount(1, 0)), - ); - assert_eq!( - cairo_runner.run_from_entrypoint( - entrypoint, - vec![&mayberelocatable!(0), &mayberelocatable!(1)], - true, - true, - true, - &mut vm, - &mut hint_processor, - ), - Err(VirtualMachineError::InvalidArgCount(1, 2)), - ); - } - - /// Test that the call to .run_from_entrypoint() with args.count() == 1 when - /// typed_args is true succeeds. - #[test] - fn run_from_entrypoint_typed_args() { - let program = - Program::from_file(Path::new("cairo_programs/not_main.json"), Some("main")).unwrap(); - let mut cairo_runner = cairo_runner!(program); - let mut vm = vm!(); - let mut hint_processor = BuiltinHintProcessor::new_empty(); - - let entrypoint = program - .identifiers - .get("__main__.not_main") - .unwrap() - .pc - .unwrap(); - - vm.accessed_addresses = Some(Vec::new()); - cairo_runner.initialize_builtins(&mut vm).unwrap(); - cairo_runner.initialize_segments(&mut vm, None); - assert_eq!( - cairo_runner.run_from_entrypoint( - entrypoint, - vec![&mayberelocatable!(0)], - true, - true, - true, - &mut vm, - &mut hint_processor, - ), - Ok(()), - ); - } - - /// Test that the call to .run_from_entrypoint() when typed_args is false - /// succeeds. - #[test] - fn run_from_entrypoint_untyped_args() { - let program = - Program::from_file(Path::new("cairo_programs/not_main.json"), Some("main")).unwrap(); - let mut cairo_runner = cairo_runner!(program); - let mut vm = vm!(); - let mut hint_processor = BuiltinHintProcessor::new_empty(); - - let entrypoint = program - .identifiers - .get("__main__.not_main") - .unwrap() - .pc - .unwrap(); - - vm.accessed_addresses = Some(Vec::new()); - cairo_runner.initialize_builtins(&mut vm).unwrap(); - cairo_runner.initialize_segments(&mut vm, None); - assert_eq!( - cairo_runner.run_from_entrypoint( - entrypoint, - vec![], - false, - true, - true, - &mut vm, - &mut hint_processor, - ), - Ok(()), - ); - } - #[test] fn finalize_segments_run_not_ended() { let program = program!(); @@ -4229,9 +4124,10 @@ mod tests { assert_eq!( cairo_runner.run_from_entrypoint( main_entrypoint, - vec![&mayberelocatable!(2), &MaybeRelocatable::from((2, 0))], //range_check_ptr - false, - true, + &vec![ + &mayberelocatable!(2).into(), + &MaybeRelocatable::from((2, 0)).into() + ], //range_check_ptr true, &mut vm, &mut hint_processor, @@ -4257,9 +4153,10 @@ mod tests { assert_eq!( new_cairo_runner.run_from_entrypoint( fib_entrypoint, - vec![&mayberelocatable!(2), &MaybeRelocatable::from((2, 0))], - false, - true, + &vec![ + &mayberelocatable!(2).into(), + &MaybeRelocatable::from((2, 0)).into() + ], true, &mut new_vm, &mut hint_processor, @@ -4267,4 +4164,18 @@ mod tests { Ok(()), ); } + + #[test] + fn cairo_arg_from_single() { + let expected = CairoArg::Single(MaybeRelocatable::from((0, 0))); + let value = MaybeRelocatable::from((0, 0)); + assert_eq!(expected, value.into()) + } + + #[test] + fn cairo_arg_from_array() { + let expected = CairoArg::Array(vec![MaybeRelocatable::from((0, 0))]); + let value = vec![MaybeRelocatable::from((0, 0))]; + assert_eq!(expected, value.into()) + } } diff --git a/src/vm/vm_core.rs b/src/vm/vm_core.rs index 1d0e72a33e..6a761dfaeb 100644 --- a/src/vm/vm_core.rs +++ b/src/vm/vm_core.rs @@ -24,8 +24,6 @@ use felt::Felt; use num_traits::{ToPrimitive, Zero}; use std::{any::Any, borrow::Cow, collections::HashMap, ops::Add}; -use super::vm_memory::memory_segments::gen_typed_args; - const MAX_TRACEBACK_ENTRIES: u32 = 20; #[derive(PartialEq, Eq, Debug)] @@ -963,13 +961,6 @@ impl VirtualMachine { self.memory.add_relocation_rule(src_ptr, dst_ptr) } - pub fn gen_typed_args( - &self, - args: Vec<&dyn Any>, - ) -> Result, VirtualMachineError> { - gen_typed_args(args) - } - pub fn gen_arg(&mut self, arg: &dyn Any) -> Result { self.segments.gen_arg(arg, &mut self.memory) } @@ -3772,50 +3763,6 @@ mod tests { ); } - /// Test that the call to .gen_arg() with any other argument returns a not - /// implemented error. - #[test] - fn gen_arg_not_implemented() { - let mut vm = vm!(); - - assert_eq!(vm.gen_arg(&""), Err(VirtualMachineError::NotImplemented),); - } - - #[test] - fn gen_typed_args_empty() { - assert_eq!(gen_typed_args(vec![]), Ok(vec![])); - } - - /// Test that the call to .gen_typed_args() with an unsupported vector - /// returns a not implemented error. - #[test] - fn gen_typed_args_not_implemented() { - assert_eq!( - gen_typed_args(vec![&0usize]), - Err(VirtualMachineError::NotImplemented), - ); - } - - /// Test that the call to .gen_typed_args() with a Vec - /// with a relocatables returns the original contents. - #[test] - fn gen_typed_args_relocatable_slice() { - assert_eq!( - gen_typed_args(vec![&[ - mayberelocatable!(0, 0), - mayberelocatable!(0, 1), - mayberelocatable!(0, 2), - ] - .into_iter() - .collect::>(),]), - Ok(vec![ - mayberelocatable!(0, 0), - mayberelocatable!(0, 1), - mayberelocatable!(0, 2), - ]), - ); - } - /// Test that compute_effective_sizes() works as intended. #[test] fn compute_effective_sizes() { diff --git a/src/vm/vm_memory/memory_segments.rs b/src/vm/vm_memory/memory_segments.rs index 5275aadbd7..0065fc50ee 100644 --- a/src/vm/vm_memory/memory_segments.rs +++ b/src/vm/vm_memory/memory_segments.rs @@ -1,3 +1,4 @@ +use crate::vm::runners::cairo_runner::CairoArg; use crate::{ types::relocatable::{MaybeRelocatable, Relocatable}, utils::from_relocatable_to_indexes, @@ -6,7 +7,6 @@ use crate::{ vm_memory::memory::Memory, }, }; - use std::{ any::Any, cmp, @@ -130,6 +130,21 @@ impl MemorySegmentManager { } } + pub fn gen_cairo_arg( + &mut self, + arg: &CairoArg, + memory: &mut Memory, + ) -> Result { + match arg { + CairoArg::Single(value) => Ok(value.clone()), + CairoArg::Array(values) => { + let base = self.add(memory); + self.load_data(memory, &base.into(), values)?; + Ok(base.into()) + } + } + } + pub fn write_arg( &mut self, memory: &mut Memory, @@ -230,22 +245,6 @@ impl MemorySegmentManager { } } -pub fn gen_typed_args(args: Vec<&dyn Any>) -> Result, VirtualMachineError> { - let mut cairo_args = Vec::new(); - for arg in args { - if let Some(value) = arg.downcast_ref::() { - cairo_args.push(value.into()); - } else if let Some(value) = arg.downcast_ref::>() { - let value = value.iter().map(|x| x as &dyn Any).collect::>(); - cairo_args.extend(gen_typed_args(value)?.into_iter()); - } else { - return Err(VirtualMachineError::NotImplemented); - } - } - - Ok(cairo_args) -} - impl Default for MemorySegmentManager { fn default() -> Self { Self::new() @@ -259,6 +258,7 @@ mod tests { use crate::{relocatable, utils::test_utils::*}; use felt::{Felt, NewFelt}; use num_traits::Num; + use std::vec; #[test] fn add_segment_no_size() { @@ -808,43 +808,6 @@ mod tests { ); } - /// Test that the call to .gen_typed_args() with an empty vector returns an - /// empty vector. - #[test] - fn gen_typed_args_empty() { - assert_eq!(gen_typed_args(vec![]), Ok(vec![])); - } - - /// Test that the call to .gen_typed_args() with an unsupported vector - /// returns a not implemented error. - #[test] - fn gen_typed_args_not_implemented() { - assert_eq!( - gen_typed_args(vec![&0usize]), - Err(VirtualMachineError::NotImplemented), - ); - } - - /// Test that the call to .gen_typed_args() with a Vec - /// with a relocatables returns the original contents. - #[test] - fn gen_typed_args_relocatable_slice() { - assert_eq!( - gen_typed_args(vec![&[ - mayberelocatable!(0, 0), - mayberelocatable!(0, 1), - mayberelocatable!(0, 2), - ] - .into_iter() - .collect::>(),],), - Ok(vec![ - mayberelocatable!(0, 0), - mayberelocatable!(0, 1), - mayberelocatable!(0, 2), - ]), - ); - } - #[test] fn finalize_no_size_nor_memory_no_change() { let mut segments = MemorySegmentManager::new(); @@ -881,4 +844,39 @@ mod tests { ); assert_eq!(segments.segment_sizes, HashMap::from([(0, 42)])); } + + #[test] + fn gen_cairo_arg_single() { + let mut memory_segment_manager = MemorySegmentManager::new(); + let mut vm = vm!(); + + assert_eq!( + memory_segment_manager.gen_cairo_arg(&mayberelocatable!(1234).into(), &mut vm.memory), + Ok(mayberelocatable!(1234)), + ); + } + + #[test] + fn gen_cairo_arg_array() { + let mut memory_segment_manager = MemorySegmentManager::new(); + let mut vm = vm!(); + + assert_eq!( + memory_segment_manager.gen_cairo_arg( + &vec![ + mayberelocatable!(0), + mayberelocatable!(1), + mayberelocatable!(2), + mayberelocatable!(3), + mayberelocatable!(0, 0), + mayberelocatable!(0, 1), + mayberelocatable!(0, 2), + mayberelocatable!(0, 3), + ] + .into(), + &mut vm.memory, + ), + Ok(mayberelocatable!(0, 0)), + ); + } }