From 0dd6ab6adec44b5724aedfe02bc43723d7cca39c Mon Sep 17 00:00:00 2001 From: Federica Date: Wed, 25 Jan 2023 11:34:47 -0300 Subject: [PATCH 1/8] Add conversion to VmException to run_from_entrypoint --- src/vm/runners/cairo_runner.rs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/vm/runners/cairo_runner.rs b/src/vm/runners/cairo_runner.rs index 18d0d4de31..332fb688a2 100644 --- a/src/vm/runners/cairo_runner.rs +++ b/src/vm/runners/cairo_runner.rs @@ -18,7 +18,7 @@ use crate::{ vm::{ errors::{ memory_errors::MemoryError, runner_errors::RunnerError, trace_errors::TraceError, - vm_errors::VirtualMachineError, + vm_errors::VirtualMachineError, vm_exception::VmException, cairo_run_errors::CairoRunError, }, security::verify_secure_runner, trace::get_perm_range_check_limits, @@ -959,7 +959,7 @@ impl CairoRunner { verify_secure: bool, vm: &mut VirtualMachine, hint_processor: &mut dyn HintProcessor, - ) -> Result<(), VirtualMachineError> { + ) -> Result<(), CairoRunError> { let stack = args .iter() .map(|arg| vm.segments.gen_cairo_arg(arg, &mut vm.memory)) @@ -969,7 +969,7 @@ impl CairoRunner { self.initialize_vm(vm)?; - self.run_until_pc(end, vm, hint_processor)?; + self.run_until_pc(end, vm, hint_processor).map_err(|err| VmException::from_vm_error(&self, &vm, err))?; self.end_run(true, false, vm, hint_processor)?; if verify_secure { From 4c35a6381ca702d4a79740652d2983e78f10e3d3 Mon Sep 17 00:00:00 2001 From: Federica Date: Wed, 25 Jan 2023 12:20:40 -0300 Subject: [PATCH 2/8] Add test case + make VmException fields public --- src/vm/errors/vm_exception.rs | 10 +++---- src/vm/runners/cairo_runner.rs | 53 ++++++++++++++++++++++++++++------ 2 files changed, 49 insertions(+), 14 deletions(-) diff --git a/src/vm/errors/vm_exception.rs b/src/vm/errors/vm_exception.rs index 7811c13eeb..ba8c4af230 100644 --- a/src/vm/errors/vm_exception.rs +++ b/src/vm/errors/vm_exception.rs @@ -20,11 +20,11 @@ use crate::{ use super::vm_errors::VirtualMachineError; #[derive(Debug, PartialEq, Error)] pub struct VmException { - pc: usize, - inst_location: Option, - inner_exc: VirtualMachineError, - error_attr_value: Option, - traceback: Option, + pub pc: usize, + pub inst_location: Option, + pub inner_exc: VirtualMachineError, + pub error_attr_value: Option, + pub traceback: Option, } impl VmException { diff --git a/src/vm/runners/cairo_runner.rs b/src/vm/runners/cairo_runner.rs index 332fb688a2..b8e692938e 100644 --- a/src/vm/runners/cairo_runner.rs +++ b/src/vm/runners/cairo_runner.rs @@ -951,7 +951,6 @@ impl CairoRunner { Ok(()) } - #[allow(clippy::too_many_arguments)] pub fn run_from_entrypoint( &mut self, entrypoint: usize, @@ -4121,8 +4120,8 @@ mod tests { 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( + + let error = cairo_runner.run_from_entrypoint( main_entrypoint, &[ &mayberelocatable!(2).into(), @@ -4131,9 +4130,8 @@ mod tests { true, &mut vm, &mut hint_processor, - ), - Ok(()), - ); + ); + assert!(error.is_ok()); let mut new_cairo_runner = cairo_runner!(program); let mut new_vm = vm!(true); //this true expression dictates that the trace is enabled @@ -4150,7 +4148,7 @@ mod tests { .pc .unwrap(); - assert_eq!( + let result = new_cairo_runner.run_from_entrypoint( fib_entrypoint, &[ @@ -4160,9 +4158,8 @@ mod tests { true, &mut new_vm, &mut hint_processor, - ), - Ok(()), ); + assert!(result.is_ok()); } #[test] @@ -4178,4 +4175,42 @@ mod tests { let value = vec![MaybeRelocatable::from((0, 0))]; assert_eq!(expected, value.into()) } + + #[test] + fn run_from_entrypoint_csubstitute_error_message_test() { + let program = + Program::from_file(Path::new("cairo_programs/bad_programs/error_msg_function.json"), None).unwrap(); + let mut cairo_runner = cairo_runner!(program); + let mut vm = vm!(true); //this true expression dictates that the trace is enabled + let mut hint_processor = BuiltinHintProcessor::new_empty(); + + //this entrypoint tells which function to run in the cairo program + let main_entrypoint = program + .identifiers + .get("__main__.main") + .unwrap() + .pc + .unwrap(); + + vm.accessed_addresses = Some(Vec::new()); + cairo_runner.initialize_builtins(&mut vm).unwrap(); + cairo_runner.initialize_segments(&mut vm, None); + + let result = cairo_runner.run_from_entrypoint( + main_entrypoint, + &[ + ], + true, + &mut vm, + &mut hint_processor, + ); + match result { + Err(CairoRunError::VmException(exception)) => { + assert_eq!(exception.error_attr_value, Some(String::from("Error message: Test error\n")) ) + }, + Err(_) => panic!("Wrong error returned, expected VmException"), + Ok(_) => panic!("Expected run to fail"), + } + } + } From 02acbc2d594c9c0448e5b5c36ab731cb39e9242f Mon Sep 17 00:00:00 2001 From: Federica Date: Wed, 25 Jan 2023 12:21:53 -0300 Subject: [PATCH 3/8] clippy + fmt --- src/vm/runners/cairo_runner.rs | 81 ++++++++++++++++++---------------- 1 file changed, 43 insertions(+), 38 deletions(-) diff --git a/src/vm/runners/cairo_runner.rs b/src/vm/runners/cairo_runner.rs index b8e692938e..c6dd509508 100644 --- a/src/vm/runners/cairo_runner.rs +++ b/src/vm/runners/cairo_runner.rs @@ -17,8 +17,9 @@ use crate::{ utils::is_subsequence, vm::{ errors::{ - memory_errors::MemoryError, runner_errors::RunnerError, trace_errors::TraceError, - vm_errors::VirtualMachineError, vm_exception::VmException, cairo_run_errors::CairoRunError, + cairo_run_errors::CairoRunError, memory_errors::MemoryError, + runner_errors::RunnerError, trace_errors::TraceError, vm_errors::VirtualMachineError, + vm_exception::VmException, }, security::verify_secure_runner, trace::get_perm_range_check_limits, @@ -968,7 +969,8 @@ impl CairoRunner { self.initialize_vm(vm)?; - self.run_until_pc(end, vm, hint_processor).map_err(|err| VmException::from_vm_error(&self, &vm, err))?; + self.run_until_pc(end, vm, hint_processor) + .map_err(|err| VmException::from_vm_error(self, vm, err))?; self.end_run(true, false, vm, hint_processor)?; if verify_secure { @@ -4120,17 +4122,17 @@ mod tests { vm.accessed_addresses = Some(Vec::new()); cairo_runner.initialize_builtins(&mut vm).unwrap(); cairo_runner.initialize_segments(&mut vm, None); - - let error = cairo_runner.run_from_entrypoint( - main_entrypoint, - &[ - &mayberelocatable!(2).into(), - &MaybeRelocatable::from((2, 0)).into() - ], //range_check_ptr - true, - &mut vm, - &mut hint_processor, - ); + + let error = cairo_runner.run_from_entrypoint( + main_entrypoint, + &[ + &mayberelocatable!(2).into(), + &MaybeRelocatable::from((2, 0)).into(), + ], //range_check_ptr + true, + &mut vm, + &mut hint_processor, + ); assert!(error.is_ok()); let mut new_cairo_runner = cairo_runner!(program); @@ -4148,16 +4150,15 @@ mod tests { .pc .unwrap(); - let result = - new_cairo_runner.run_from_entrypoint( - fib_entrypoint, - &[ - &mayberelocatable!(2).into(), - &MaybeRelocatable::from((2, 0)).into() - ], - true, - &mut new_vm, - &mut hint_processor, + let result = new_cairo_runner.run_from_entrypoint( + fib_entrypoint, + &[ + &mayberelocatable!(2).into(), + &MaybeRelocatable::from((2, 0)).into(), + ], + true, + &mut new_vm, + &mut hint_processor, ); assert!(result.is_ok()); } @@ -4178,8 +4179,11 @@ mod tests { #[test] fn run_from_entrypoint_csubstitute_error_message_test() { - let program = - Program::from_file(Path::new("cairo_programs/bad_programs/error_msg_function.json"), None).unwrap(); + let program = Program::from_file( + Path::new("cairo_programs/bad_programs/error_msg_function.json"), + None, + ) + .unwrap(); let mut cairo_runner = cairo_runner!(program); let mut vm = vm!(true); //this true expression dictates that the trace is enabled let mut hint_processor = BuiltinHintProcessor::new_empty(); @@ -4195,22 +4199,23 @@ mod tests { vm.accessed_addresses = Some(Vec::new()); cairo_runner.initialize_builtins(&mut vm).unwrap(); cairo_runner.initialize_segments(&mut vm, None); - - let result = cairo_runner.run_from_entrypoint( - main_entrypoint, - &[ - ], - true, - &mut vm, - &mut hint_processor, - ); + + let result = cairo_runner.run_from_entrypoint( + main_entrypoint, + &[], + true, + &mut vm, + &mut hint_processor, + ); match result { Err(CairoRunError::VmException(exception)) => { - assert_eq!(exception.error_attr_value, Some(String::from("Error message: Test error\n")) ) - }, + assert_eq!( + exception.error_attr_value, + Some(String::from("Error message: Test error\n")) + ) + } Err(_) => panic!("Wrong error returned, expected VmException"), Ok(_) => panic!("Expected run to fail"), } } - } From 20ecbe219836973dbf497a2283a595edd9901eed Mon Sep 17 00:00:00 2001 From: Federica Date: Wed, 25 Jan 2023 12:23:14 -0300 Subject: [PATCH 4/8] Add test program --- cairo_programs/bad_programs/error_msg_function.cairo | 12 ++++++++++++ 1 file changed, 12 insertions(+) create mode 100644 cairo_programs/bad_programs/error_msg_function.cairo diff --git a/cairo_programs/bad_programs/error_msg_function.cairo b/cairo_programs/bad_programs/error_msg_function.cairo new file mode 100644 index 0000000000..bb366e620f --- /dev/null +++ b/cairo_programs/bad_programs/error_msg_function.cairo @@ -0,0 +1,12 @@ +func test_error_message() { + with_attr error_message("Test error") { + assert 1 = 0; + } + return (); +} + + +func main() { + test_error_message(); + return(); +} From 8d23b82c0bdcb87780217906794d195e46d5b3d1 Mon Sep 17 00:00:00 2001 From: Federica Date: Wed, 25 Jan 2023 12:23:33 -0300 Subject: [PATCH 5/8] fmt --- cairo_programs/bad_programs/error_msg_function.cairo | 1 - 1 file changed, 1 deletion(-) diff --git a/cairo_programs/bad_programs/error_msg_function.cairo b/cairo_programs/bad_programs/error_msg_function.cairo index bb366e620f..86ed5f1888 100644 --- a/cairo_programs/bad_programs/error_msg_function.cairo +++ b/cairo_programs/bad_programs/error_msg_function.cairo @@ -5,7 +5,6 @@ func test_error_message() { return (); } - func main() { test_error_message(); return(); From b964ff220aa143e2e3f5f5dddc41e1fdea6b3ad1 Mon Sep 17 00:00:00 2001 From: Federica Date: Wed, 25 Jan 2023 12:33:53 -0300 Subject: [PATCH 6/8] Add changelog entry --- CHANGELOG.md | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index ce9af82697..de2c147c5f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,11 @@ #### Upcoming Changes +* Add `VmException` to `CairoRunner::run_from_entrypoint`[#775](https://github.com/lambdaclass/cairo-rs/pull/775) + * Public Api Changes: + * Change error return type of `CairoRunner::run_from_entrypoint` to `CairoRunError`. + * Convert `VirtualMachineError`s outputed during the vm run to `VmException` in `CairoRunner::run_from_entrypoint`. + * 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` From 0c795f3eae1eb136c9b403132199611c6d716c38 Mon Sep 17 00:00:00 2001 From: Federica Date: Wed, 25 Jan 2023 12:34:50 -0300 Subject: [PATCH 7/8] Update changelog entry --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index de2c147c5f..a07893f8a6 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,6 +6,7 @@ * Public Api Changes: * Change error return type of `CairoRunner::run_from_entrypoint` to `CairoRunError`. * Convert `VirtualMachineError`s outputed during the vm run to `VmException` in `CairoRunner::run_from_entrypoint`. + * Make `VmException` fields public * Use CairoArg enum instead of Any in CairoRunner::run_from_entrypoint [#686](https://github.com/lambdaclass/cairo-rs/pull/686) * Public Api changes: From 15584a082dc2c03c8a64f202d25ac0a3dddf8946 Mon Sep 17 00:00:00 2001 From: fmoletta <99273364+fmoletta@users.noreply.github.com> Date: Thu, 26 Jan 2023 18:42:07 -0300 Subject: [PATCH 8/8] Clippy --- src/vm/runners/builtin_runner/keccak.rs | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/src/vm/runners/builtin_runner/keccak.rs b/src/vm/runners/builtin_runner/keccak.rs index 9283474703..20ef0a5fcd 100644 --- a/src/vm/runners/builtin_runner/keccak.rs +++ b/src/vm/runners/builtin_runner/keccak.rs @@ -679,8 +679,10 @@ mod tests { ((0, 35), 0) ]; - let mut keccak_instance = KeccakInstanceDef::default(); - keccak_instance._state_rep = vec![1; 8]; + let keccak_instance = KeccakInstanceDef { + _state_rep: vec![1; 8], + ..Default::default() + }; let builtin = KeccakBuiltinRunner::new(&keccak_instance, true); let result = builtin.deduce_memory_cell(&Relocatable::from((0, 25)), &memory);