refactor: Move the VM back to the CairoRunner

CHANGELOG.md

@@ -2,6 +2,56 @@
 
 #### Upcoming Changes
 
+* refactor(BREAKING): Move the VM back to the CairoRunner [#1743](https://github.com/lambdaclass/cairo-vm/pull/1743)
+  * `CairoRunner` has a new public field `vm: VirtualMachine`
+  * `CairoRunner` no longer derives `Debug`
+  * `CairoRunner` methods `new_v2` & `new` take an extra boolean argument `trace_enabled`.
+  * Functions `cairo_run` , `cairo_run_program` & `cairo_run_fuzzed_program` from `vm` crate and `cairo_run_program` from `cairo1-run` crate now retun only `CairoRunner` instead of `(CairoRunner, VirtualMachine)`
+  * `CairoRunner` methods no longer take a reference to `VirtualMachine`. Methods that took an immutable reference to self and a mutable reference to the VM now take a mutable reference to self. Affected methods:
+    * `initialize`
+    * `initialize_builtins`
+    * `initialize_all_builtins`
+    * `initialize_segments`
+    * `initialize_state`
+    * `initialize_function_entrypoint`
+    * `initialize_state`
+    * `initialize_main_entrypoint`
+    * `initialize_vm`
+    * `run_until_pc`
+    * `run_for_steps`
+    * `run_until_steps`
+    * `run_until_power_of_2`
+    * `get_perm_range_check_limits`
+    * `check_range_check_usage`
+    * `get_memory_holes`
+    * `check_diluted_check_usage`
+    * `end_run`
+    * `relocate_trace`
+    * `relocate_memory`
+    * `relocate`
+    * `get_builtin_segments_info`
+    * `get_builtin_segments_info_for_pie`
+    * `get_execution_resources`
+    * `finalize_segments`
+    * `run_from_entrypoint`
+    * `check_used_cells`
+    * `check_memory_usage`
+    * `initialize_function_runner_cairo_1`
+    * `initialize_function_runner`
+    * `read_return_values`
+    * `get_builtins_final_stack`
+    * `get_cairo_pie`
+    * `get_air_public_input`
+    * `get_air_private_input`
+    * `get_memory_segment_addresses`
+  * Functions & methods taking a reference to `CairoRunner` & `VirtualMachine` now only take a reference to `CairoRunner`:
+    * `start_tracer`
+    * `VmException::from_vm_error`
+    * `get_error_attr_value`
+    * `get_traceback`
+    * `verify_secure_runner`
+  * [hooks feature] `BeforeFirstStepHookFunc` dyn Fn no longer takes a mutable reference to `CairoRunner`, along with `VirtualMachine::execute_before_first_step`.
+
 * `cairo1-run` CLI: Allow loading arguments from file[#1739](https://github.com/lambdaclass/cairo-vm/pull/1739)
 
 * BREAKING: Remove unused `CairoRunner` field `original_steps`[#1742](https://github.com/lambdaclass/cairo-vm/pull/1742)

README.md

@@ -209,9 +209,7 @@ When running a Cairo program directly using the Cairo-vm repository you would fi
 2. Instantiate the VM, the cairo_runner, the hint processor, and the entrypoint
 
   ```rust
-  let mut vm = VirtualMachine::new(false);
-
-  let mut cairo_runner = CairoRunner::new(&program, LayoutName::all_cairo, false);
+  let mut cairo_runner = CairoRunner::new(&program, LayoutName::all_cairo, false, false);
 
   let mut hint_processor = BuiltinHintProcessor::new_empty();
 
@@ -224,8 +222,8 @@ When running a Cairo program directly using the Cairo-vm repository you would fi
 3. Lastly, initialize the builtins and segments.
 
   ```rust
-  cairo_runner.initialize_builtins(&mut vm)?;
-  cairo_runner.initialize_segments(&mut vm, None);
+  cairo_runner.initialize_builtins(false)?;
+  cairo_runner.initialize_segments(None);
   ```
 
 When using cairo-vm with the Starknet devnet there are additional parameters that are part of the OS context passed on to the `run_from_entrypoint` method that we do not have here when using it directly. These parameters are, for example, initial stacks of the builtins, which are the base of each of them and are needed as they are the implicit arguments of the function.
@@ -238,7 +236,6 @@ When using cairo-vm with the Starknet devnet there are additional parameters tha
                 &MaybeRelocatable::from((2,0)).into() //this would be the output_ptr for example if our cairo function uses it
                 ],
             false,
-            &mut vm,
             &mut hint_processor,
         );
 ```

bench/criterion_benchmark.rs

@@ -1,6 +1,6 @@
 use cairo_vm::{
     types::{layout_name::LayoutName, program::Program},
-    vm::{runners::cairo_runner::CairoRunner, vm_core::VirtualMachine},
+    vm::runners::cairo_runner::CairoRunner,
 };
 use criterion::{black_box, criterion_group, criterion_main, BatchSize, Criterion};
 
@@ -31,6 +31,7 @@ fn build_many_runners(c: &mut Criterion) {
                     black_box(&program),
                     black_box(LayoutName::starknet_with_keccak),
                     black_box(false),
+                    black_box(false),
                 )
                 .unwrap(),
             );
@@ -44,15 +45,8 @@ fn load_program_data(c: &mut Criterion) {
     let program = Program::from_bytes(program.as_slice(), Some("main")).unwrap();
     c.bench_function("initialize", |b| {
         b.iter_batched(
-            || {
-                (
-                    CairoRunner::new(&program, LayoutName::starknet_with_keccak, false).unwrap(),
-                    VirtualMachine::new(false),
-                )
-            },
-            |(mut runner, mut vm)| {
-                _ = black_box(runner.initialize(black_box(&mut vm), false).unwrap())
-            },
+            || CairoRunner::new(&program, LayoutName::starknet_with_keccak, false, false).unwrap(),
+            |mut runner| _ = black_box(runner.initialize(false).unwrap()),
             BatchSize::SmallInput,
         )
     });

bench/iai_benchmark.rs

@@ -3,7 +3,7 @@ use iai_callgrind::main;
 
 use cairo_vm::{
     types::{layout_name::LayoutName, program::Program},
-    vm::{runners::cairo_runner::CairoRunner, vm_core::VirtualMachine},
+    vm::runners::cairo_runner::CairoRunner,
 };
 
 use mimalloc::MiMalloc;
@@ -31,26 +31,29 @@ fn parse_program_helper() -> Program {
 #[inline(never)]
 fn build_runner() {
     let program = parse_program_helper();
-    let runner =
-        CairoRunner::new(black_box(&program), LayoutName::starknet_with_keccak, false).unwrap();
+    let runner = CairoRunner::new(
+        black_box(&program),
+        LayoutName::starknet_with_keccak,
+        false,
+        false,
+    )
+    .unwrap();
     core::mem::drop(black_box(runner));
 }
 
 #[export_name = "helper::build_runner"]
 #[inline(never)]
-fn build_runner_helper() -> (CairoRunner, VirtualMachine) {
+fn build_runner_helper() -> CairoRunner {
     //Picked the biggest one at the time of writing
     let program = include_bytes!("../cairo_programs/benchmarks/keccak_integration_benchmark.json");
     let program = Program::from_bytes(program.as_slice(), Some("main")).unwrap();
-    let runner = CairoRunner::new(&program, LayoutName::starknet_with_keccak, false).unwrap();
-    let vm = VirtualMachine::new(false);
-    (runner, vm)
+    CairoRunner::new(&program, LayoutName::starknet_with_keccak, false, false).unwrap()
 }
 
 #[inline(never)]
 fn load_program_data() {
-    let (mut runner, mut vm) = build_runner_helper();
-    _ = black_box(runner.initialize(black_box(&mut vm), false).unwrap());
+    let mut runner = build_runner_helper();
+    _ = black_box(runner.initialize(false).unwrap());
 }
 
 main!(

cairo-vm-cli/src/main.rs

@@ -15,8 +15,6 @@ use cairo_vm::vm::runners::cairo_pie::CairoPie;
 use cairo_vm::vm::runners::cairo_runner::CairoRunner;
 use cairo_vm::vm::runners::cairo_runner::RunResources;
 #[cfg(feature = "with_tracer")]
-use cairo_vm::vm::vm_core::VirtualMachine;
-#[cfg(feature = "with_tracer")]
 use cairo_vm_tracer::error::trace_data_errors::TraceDataError;
 #[cfg(feature = "with_tracer")]
 use cairo_vm_tracer::tracer::run_tracer;
@@ -134,8 +132,9 @@ impl FileWriter {
 }
 
 #[cfg(feature = "with_tracer")]
-fn start_tracer(cairo_runner: &CairoRunner, vm: &VirtualMachine) -> Result<(), TraceDataError> {
-    let relocation_table = vm
+fn start_tracer(cairo_runner: &CairoRunner) -> Result<(), TraceDataError> {
+    let relocation_table = cairo_runner
+        .vm
         .relocate_segments()
         .map_err(TraceDataError::FailedToGetRelocationTable)?;
     let instruction_locations = cairo_runner
@@ -174,7 +173,7 @@ fn run(args: impl Iterator<Item = String>) -> Result<(), Error> {
         ..Default::default()
     };
 
-    let (cairo_runner, mut vm) = match {
+    let mut cairo_runner = match {
         if args.run_from_cairo_pie {
             let pie = CairoPie::read_zip_file(&args.filename)?;
             let mut hint_processor = BuiltinHintProcessor::new(
@@ -197,7 +196,7 @@ fn run(args: impl Iterator<Item = String>) -> Result<(), Error> {
 
     if args.print_output {
         let mut output_buffer = "Program Output:\n".to_string();
-        vm.write_output(&mut output_buffer)?;
+        cairo_runner.vm.write_output(&mut output_buffer)?;
         print!("{output_buffer}");
     }
 
@@ -225,13 +224,13 @@ fn run(args: impl Iterator<Item = String>) -> Result<(), Error> {
     }
 
     if let Some(file_path) = args.air_public_input {
-        let json = cairo_runner.get_air_public_input(&vm)?.serialize_json()?;
+        let json = cairo_runner.get_air_public_input()?.serialize_json()?;
         std::fs::write(file_path, json)?;
     }
 
     #[cfg(feature = "with_tracer")]
     if args.tracer {
-        start_tracer(&cairo_runner, &vm)?;
+        start_tracer(&cairo_runner)?;
     }
 
     if let (Some(file_path), Some(ref trace_file), Some(ref memory_file)) =
@@ -252,7 +251,7 @@ fn run(args: impl Iterator<Item = String>) -> Result<(), Error> {
             .to_string();
 
         let json = cairo_runner
-            .get_air_private_input(&vm)
+            .get_air_private_input()
             .to_serializable(trace_path, memory_path)
             .serialize_json()
             .map_err(PublicInputError::Serde)?;
@@ -262,7 +261,7 @@ fn run(args: impl Iterator<Item = String>) -> Result<(), Error> {
     if let Some(ref file_name) = args.cairo_pie_output {
         let file_path = Path::new(file_name);
         cairo_runner
-            .get_cairo_pie(&vm)
+            .get_cairo_pie()
             .map_err(CairoRunError::Runner)?
             .write_zip_file(file_path)?
     }

cairo1-run/src/cairo_run.rs

@@ -109,19 +109,11 @@ impl Default for Cairo1RunConfig<'_> {
 }
 
 // Runs a Cairo 1 program
-// Returns the runner & VM after execution + the return values + the serialized return values (if serialize_output is enabled)
+// Returns the runner after execution + the return values + the serialized return values (if serialize_output is enabled)
 pub fn cairo_run_program(
     sierra_program: &SierraProgram,
     cairo_run_config: Cairo1RunConfig,
-) -> Result<
-    (
-        CairoRunner,
-        VirtualMachine,
-        Vec<MaybeRelocatable>,
-        Option<String>,
-    ),
-    Error,
-> {
+) -> Result<(CairoRunner, Vec<MaybeRelocatable>, Option<String>), Error> {
     let metadata = calc_metadata_ap_change_only(sierra_program)
         .map_err(|_| VirtualMachineError::Unexpected)?;
     let sierra_program_registry = ProgramRegistry::<CoreType, CoreLibfunc>::new(sierra_program)?;
@@ -215,32 +207,36 @@ pub fn cairo_run_program(
         RunnerMode::ExecutionMode
     };
 
-    let mut runner = CairoRunner::new_v2(&program, cairo_run_config.layout, runner_mode)?;
-    let mut vm = VirtualMachine::new(cairo_run_config.trace_enabled);
-    let end = runner.initialize(&mut vm, cairo_run_config.proof_mode)?;
+    let mut runner = CairoRunner::new_v2(
+        &program,
+        cairo_run_config.layout,
+        runner_mode,
+        cairo_run_config.trace_enabled,
+    )?;
+    let end = runner.initialize(cairo_run_config.proof_mode)?;
 
     // Run it until the end / infinite loop in proof_mode
-    runner.run_until_pc(end, &mut vm, &mut hint_processor)?;
+    runner.run_until_pc(end, &mut hint_processor)?;
     if cairo_run_config.proof_mode {
-        runner.run_for_steps(1, &mut vm, &mut hint_processor)?;
+        runner.run_for_steps(1, &mut hint_processor)?;
     }
 
-    runner.end_run(false, false, &mut vm, &mut hint_processor)?;
+    runner.end_run(false, false, &mut hint_processor)?;
 
     let skip_output = cairo_run_config.proof_mode || cairo_run_config.append_return_values;
     // Fetch return values
     let return_values = fetch_return_values(
         return_type_size,
         return_type_id,
-        &vm,
+        &runner.vm,
         builtin_count,
         skip_output,
     )?;
 
     let serialized_output = if cairo_run_config.serialize_output {
         Some(serialize_output(
             &return_values,
-            &mut vm,
+            &mut runner.vm,
             return_type_id,
             &sierra_program_registry,
             &type_sizes,
@@ -253,12 +249,15 @@ pub fn cairo_run_program(
     if cairo_run_config.finalize_builtins {
         if skip_output {
             // Set stop pointer for each builtin
-            vm.builtins_final_stack_from_stack_pointer_dict(
+            runner.vm.builtins_final_stack_from_stack_pointer_dict(
                 &builtins
                     .iter()
                     .enumerate()
                     .map(|(i, builtin)| {
-                        (*builtin, (vm.get_ap() - (builtins.len() - 1 - i)).unwrap())
+                        (
+                            *builtin,
+                            (runner.vm.get_ap() - (builtins.len() - 1 - i)).unwrap(),
+                        )
                     })
                     .collect(),
                 false,
@@ -267,20 +266,20 @@ pub fn cairo_run_program(
             finalize_builtins(
                 &main_func.signature.ret_types,
                 &type_sizes,
-                &mut vm,
+                &mut runner.vm,
                 builtin_count,
             )?;
         }
 
         // Build execution public memory
         if cairo_run_config.proof_mode {
-            runner.finalize_segments(&mut vm)?;
+            runner.finalize_segments()?;
         }
     }
 
-    runner.relocate(&mut vm, true)?;
+    runner.relocate(true)?;
 
-    Ok((runner, vm, return_values, serialized_output))
+    Ok((runner, return_values, serialized_output))
 }
 
 #[allow(clippy::type_complexity)]
@@ -1217,7 +1216,7 @@ mod tests {
             ..Default::default()
         };
         // Run program
-        let (_, vm, return_values, _) =
+        let (runner, return_values, _) =
             cairo_run_program(&sierra_program, cairo_run_config).unwrap();
         // When the return type is a PanicResult, we remove the panic wrapper when returning the ret values
         // And handle the panics returning an error, so we need to add it here
@@ -1230,14 +1229,16 @@ mod tests {
         };
         // Check that the output segment contains the return values
         // The output builtin will always be the first builtin, so we know it's segment is 2
-        let output_builtin_segment = vm
+        let output_builtin_segment = runner
+            .vm
             .get_continuous_range((2, 0).into(), return_values.len())
             .unwrap();
         // While this test can make sure that the return values are the same as the output segment values, as the code that fetches return values
         // takes them from the output segment we can't be sure that these return values are correct, for this we use the integration tests in the main.rs file
         assert_eq!(output_builtin_segment, return_values, "{}", filename);
         // Just for consistency, we will check that there are no values in the output segment after the return values
-        assert!(vm
+        assert!(runner
+            .vm
             .get_maybe(&Relocatable::from((2_isize, return_values.len())))
             .is_none());
     }