Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add VmException to CairoRunner::run_from_entrypoint #775

Merged
merged 11 commits into from
Jan 27, 2023
24 changes: 24 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,12 @@

#### 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`.
* Make `VmException` fields public

* Fix `BuiltinRunner::final_stack` and remove quick fix [#778](https://github.com/lambdaclass/cairo-rs/pull/778)
* Public Api changes:
* Various changes to public `BuiltinRunner` method's signatures:
Expand All @@ -22,12 +28,30 @@
* `EcOpBuiltinRunner::deduce_memory_cell` now returns the values of the point coordinates instead of the indices when a `PointNotOnCurve` error is returned

* Refactor `Refactor verify_secure_runner` [#768](https://github.com/lambdaclass/cairo-rs/pull/768)
<<<<<<< HEAD
* Public Api changes:
* Remove builtin name from the return value of `BuiltinRunner::get_memory_segment_addresses`
* Simplify the return value of `CairoRunner::get_builtin_segments_info` to `Vec<(usize, usize)>`
* CairoRunner::read_return_values now receives a mutable reference to VirtualMachine
* Bugfixes:
* CairoRunner::read_return_values now updates the `stop_ptr` of each builtin after calling `BuiltinRunner::final_stack`
>>>>>>> 195f9ce1eaaa66093207078525e5158e78ce0590
=======
Public Api changes:
* Remove builtin name from the return value of `BuiltinRunner::get_memory_segment_addresses`
* Simplify the return value of `CairoRunner::get_builtin_segments_info` to `Vec<(usize, usize)>`
* CairoRunner::read_return_values now receives a mutable reference to VirtualMachine
Bugfixes:
* CairoRunner::read_return_values now updates the `stop_ptr` of each builtin after calling `BuiltinRunner::final_stack`

* Refactor `Refactor verify_secure_runner` [#768](https://github.com/lambdaclass/cairo-rs/pull/768)
Public Api changes:
* Remove builtin name from the return value of `BuiltinRunner::get_memory_segment_addresses`
* Simplify the return value of `CairoRunner::get_builtin_segments_info` to `Vec<(usize, usize)>`
* CairoRunner::read_return_values now receives a mutable reference to VirtualMachine
Bugfixes:
* CairoRunner::read_return_values now updates the `stop_ptr` of each builtin after calling `BuiltinRunner::final_stack`
>>>>>>> 15584a082dc2c03c8a64f202d25ac0a3dddf8946

* Use CairoArg enum instead of Any in CairoRunner::run_from_entrypoint [#686](https://github.com/lambdaclass/cairo-rs/pull/686)
* Public Api changes:
Expand Down
11 changes: 11 additions & 0 deletions cairo_programs/bad_programs/error_msg_function.cairo
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
func test_error_message() {
with_attr error_message("Test error") {
assert 1 = 0;
}
return ();
}

func main() {
test_error_message();
return();
}
10 changes: 5 additions & 5 deletions src/vm/errors/vm_exception.rs
Original file line number Diff line number Diff line change
Expand Up @@ -20,11 +20,11 @@ use crate::{
use super::vm_errors::VirtualMachineError;
#[derive(Debug, PartialEq, Error)]
pub struct VmException {
pc: usize,
inst_location: Option<Location>,
inner_exc: VirtualMachineError,
error_attr_value: Option<String>,
traceback: Option<String>,
pub pc: usize,
pub inst_location: Option<Location>,
pub inner_exc: VirtualMachineError,
pub error_attr_value: Option<String>,
pub traceback: Option<String>,
}

impl VmException {
Expand Down
98 changes: 69 additions & 29 deletions src/vm/runners/cairo_runner.rs
Original file line number Diff line number Diff line change
Expand Up @@ -17,8 +17,9 @@ use crate::{
utils::is_subsequence,
vm::{
errors::{
memory_errors::MemoryError, runner_errors::RunnerError, trace_errors::TraceError,
vm_errors::VirtualMachineError,
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,
Expand Down Expand Up @@ -950,15 +951,14 @@ impl CairoRunner {
Ok(())
}

#[allow(clippy::too_many_arguments)]
pub fn run_from_entrypoint(
&mut self,
entrypoint: usize,
args: &[&CairoArg],
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))
Expand All @@ -968,7 +968,8 @@ 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 {
Expand Down Expand Up @@ -4234,19 +4235,18 @@ 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(
main_entrypoint,
&[
&mayberelocatable!(2).into(),
&MaybeRelocatable::from((2, 0)).into()
], //range_check_ptr
true,
&mut vm,
&mut hint_processor,
),
Ok(()),

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);
let mut new_vm = vm!(true); //this true expression dictates that the trace is enabled
Expand All @@ -4263,19 +4263,17 @@ mod tests {
.pc
.unwrap();

assert_eq!(
new_cairo_runner.run_from_entrypoint(
fib_entrypoint,
&[
&mayberelocatable!(2).into(),
&MaybeRelocatable::from((2, 0)).into()
],
true,
&mut new_vm,
&mut hint_processor,
),
Ok(()),
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());
}

#[test]
Expand All @@ -4292,6 +4290,48 @@ mod tests {
assert_eq!(expected, value.into())
}

#[test]
fn run_from_entrypoint_substitute_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"),
}
}

#[test]
fn get_builtins_final_stack_range_check_builtin() {
let program = Program::from_file(
Expand Down