From b0073a1d00aba62239701316e7501f094757797f Mon Sep 17 00:00:00 2001 From: Federica Date: Thu, 23 Nov 2023 15:38:27 -0300 Subject: [PATCH 01/15] Add back RelocataedTrace struct --- vm/src/vm/trace/mod.rs | 15 +++++++++++---- 1 file changed, 11 insertions(+), 4 deletions(-) diff --git a/vm/src/vm/trace/mod.rs b/vm/src/vm/trace/mod.rs index faab7ae879..a2491cfd60 100644 --- a/vm/src/vm/trace/mod.rs +++ b/vm/src/vm/trace/mod.rs @@ -3,14 +3,21 @@ pub mod trace_entry { ///A trace entry for every instruction that was executed. ///Holds the register values before the instruction was executed. - /// Before relocation: - /// Register values are represented as their offsets, as their indexes will always be 0,1,1 respectively - /// The index of the last pc will not be equal to 0, but it is not appended to the trace - /// After relocation the value of each register will be a single integer + ///Register values for ap & fp are represented as their offsets, as their indexes will always be 1 #[derive(Debug, PartialEq, Eq, Deserialize, Serialize)] pub struct TraceEntry { pub pc: usize, pub ap: usize, pub fp: usize, } + + + /// A trace entry for every instruction that was executed. + /// Holds the register values before the instruction was executed, after going through the relocation process + #[derive(Debug, PartialEq, Eq, Deserialize, Serialize)] + pub struct RelocatedTraceEntry { + pub pc: usize, + pub ap: usize, + pub fp: usize, + } } From f85e52cce8dc4e7546c73bde9c94da2f7e8f7bc4 Mon Sep 17 00:00:00 2001 From: Federica Date: Thu, 23 Nov 2023 15:45:57 -0300 Subject: [PATCH 02/15] Move relocated trace back to runner --- vm/src/vm/runners/cairo_runner.rs | 37 ++++++++++++++++++++++++++++--- vm/src/vm/trace/mod.rs | 1 - 2 files changed, 34 insertions(+), 4 deletions(-) diff --git a/vm/src/vm/runners/cairo_runner.rs b/vm/src/vm/runners/cairo_runner.rs index b9507b4e75..9a7e4d0547 100644 --- a/vm/src/vm/runners/cairo_runner.rs +++ b/vm/src/vm/runners/cairo_runner.rs @@ -7,7 +7,9 @@ use crate::{ prelude::*, }, types::instance_definitions::keccak_instance_def::KeccakInstanceDef, - vm::runners::builtin_runner::SegmentArenaBuiltinRunner, + vm::{ + runners::builtin_runner::SegmentArenaBuiltinRunner, trace::trace_entry::RelocatedTraceEntry, + }, }; use crate::{ @@ -143,6 +145,7 @@ pub struct CairoRunner { pub original_steps: Option, pub relocated_memory: Vec>, pub exec_scopes: ExecutionScopes, + pub relocated_trace: Option>, } impl CairoRunner { @@ -184,6 +187,7 @@ impl CairoRunner { relocated_memory: Vec::new(), exec_scopes: ExecutionScopes::new(), execution_public_memory: if proof_mode { Some(Vec::new()) } else { None }, + relocated_trace: None, }) } @@ -756,12 +760,39 @@ impl CairoRunner { Ok(()) } + ///Relocates the VM's trace, turning relocatable registers to numbered ones + fn relocate_trace( + &mut self, + vm: &VirtualMachine, + relocation_table: &Vec, + ) -> Result<(), TraceError> { + if self.relocated_trace.is_some() { + return Err(TraceError::AlreadyRelocated); + } + + let trace = vm.trace.as_ref().ok_or(TraceError::TraceNotEnabled)?.iter(); + let mut relocated_trace = Vec::::with_capacity(trace.len()); + let segment_1_base = relocation_table + .get(1) + .ok_or(TraceError::NoRelocationFound)?; + + for entry in trace { + relocated_trace.push(RelocatedTraceEntry { + pc: entry.pc + 1, + ap: entry.ap + segment_1_base, + fp: entry.fp + segment_1_base, + }) + } + self.relocated_trace = Some(relocated_trace); + Ok(()) + } + /// Relocates the VM's memory, turning bidimensional indexes into contiguous numbers, and values /// into Felt252s. Uses the relocation_table to asign each index a number according to the value /// on its segment number. fn relocate_memory( &mut self, - vm: &mut VirtualMachine, + vm: &VirtualMachine, relocation_table: &Vec, ) -> Result<(), MemoryError> { if !(self.relocated_memory.is_empty()) { @@ -812,7 +843,7 @@ impl CairoRunner { } } - vm.relocate_trace(&relocation_table)?; + self.relocate_trace(vm, &relocation_table)?; vm.relocation_table = Some(relocation_table); Ok(()) } diff --git a/vm/src/vm/trace/mod.rs b/vm/src/vm/trace/mod.rs index a2491cfd60..ac249689f2 100644 --- a/vm/src/vm/trace/mod.rs +++ b/vm/src/vm/trace/mod.rs @@ -11,7 +11,6 @@ pub mod trace_entry { pub fp: usize, } - /// A trace entry for every instruction that was executed. /// Holds the register values before the instruction was executed, after going through the relocation process #[derive(Debug, PartialEq, Eq, Deserialize, Serialize)] From 9f093acf137cf453e131952f6ca0af00fa771fe9 Mon Sep 17 00:00:00 2001 From: Federica Date: Thu, 23 Nov 2023 16:01:47 -0300 Subject: [PATCH 03/15] Remove trace relocation from vm --- cairo-vm-cli/src/main.rs | 8 ++++++-- cairo1-run/src/main.rs | 6 ++++-- vm/src/air_public_input.rs | 4 ++-- vm/src/cairo_run.rs | 7 +++---- vm/src/vm/runners/cairo_runner.rs | 7 +++++-- vm/src/vm/trace/mod.rs | 2 +- vm/src/vm/vm_core.rs | 32 ------------------------------- 7 files changed, 21 insertions(+), 45 deletions(-) diff --git a/cairo-vm-cli/src/main.rs b/cairo-vm-cli/src/main.rs index 37ab8fb69f..4f3cbe1070 100644 --- a/cairo-vm-cli/src/main.rs +++ b/cairo-vm-cli/src/main.rs @@ -149,13 +149,17 @@ fn run(args: impl Iterator) -> Result<(), Error> { } if let Some(trace_path) = args.trace_file { - let relocated_trace = vm.get_relocated_trace()?; + let relocated_trace = cairo_runner + .relocated_trace + .as_ref() + .ok_or(Error::Trace(TraceError::TraceNotRelocated))? + .clone(); let trace_file = std::fs::File::create(trace_path)?; let mut trace_writer = FileWriter::new(io::BufWriter::with_capacity(3 * 1024 * 1024, trace_file)); - cairo_run::write_encoded_trace(relocated_trace, &mut trace_writer)?; + cairo_run::write_encoded_trace(&relocated_trace, &mut trace_writer)?; trace_writer.flush()?; } diff --git a/cairo1-run/src/main.rs b/cairo1-run/src/main.rs index ce711ca554..3e3103af24 100644 --- a/cairo1-run/src/main.rs +++ b/cairo1-run/src/main.rs @@ -294,12 +294,14 @@ fn run(args: impl Iterator) -> Result, Erro runner.relocate(&mut vm, true)?; if let Some(trace_path) = args.trace_file { - let relocated_trace = vm.get_relocated_trace()?; + let relocated_trace = runner + .relocated_trace + .ok_or(Error::Trace(TraceError::TraceNotRelocated))?; let trace_file = std::fs::File::create(trace_path)?; let mut trace_writer = FileWriter::new(io::BufWriter::with_capacity(3 * 1024 * 1024, trace_file)); - cairo_run::write_encoded_trace(relocated_trace, &mut trace_writer)?; + cairo_run::write_encoded_trace(&relocated_trace, &mut trace_writer)?; trace_writer.flush()?; } if let Some(memory_path) = args.memory_file { diff --git a/vm/src/air_public_input.rs b/vm/src/air_public_input.rs index 24cd0f08af..fa752e94fb 100644 --- a/vm/src/air_public_input.rs +++ b/vm/src/air_public_input.rs @@ -10,7 +10,7 @@ use crate::{ types::layout::CairoLayout, vm::{ errors::{trace_errors::TraceError, vm_errors::VirtualMachineError}, - trace::trace_entry::TraceEntry, + trace::trace_entry::RelocatedTraceEntry, }, }; @@ -73,7 +73,7 @@ impl<'a> PublicInput<'a> { dyn_layout_params: Option<&'a CairoLayout>, public_memory_addresses: &[(usize, usize)], memory_segment_addresses: HashMap<&'static str, (usize, usize)>, - trace: &[TraceEntry], + trace: &[RelocatedTraceEntry], rc_limits: (isize, isize), ) -> Result { let memory_entry = diff --git a/vm/src/cairo_run.rs b/vm/src/cairo_run.rs index e52312ee4a..050da98fd2 100644 --- a/vm/src/cairo_run.rs +++ b/vm/src/cairo_run.rs @@ -158,7 +158,7 @@ pub struct EncodeTraceError(usize, bincode::error::EncodeError); /// Bincode encodes to little endian by default and each trace entry is composed of /// 3 usize values that are padded to always reach 64 bit size. pub fn write_encoded_trace( - relocated_trace: &[crate::vm::trace::trace_entry::TraceEntry], + relocated_trace: &[crate::vm::trace::trace_entry::RelocatedTraceEntry], dest: &mut impl Writer, ) -> Result<(), EncodeTraceError> { for (i, entry) in relocated_trace.iter().enumerate() { @@ -318,11 +318,11 @@ mod tests { // relocate memory so we can dump it to file assert!(cairo_runner.relocate(&mut vm, false).is_ok()); - let trace_entries = vm.get_relocated_trace().unwrap(); + let trace_entries = cairo_runner.relocated_trace.unwrap(); let mut buffer = [0; 24]; let mut buff_writer = SliceWriter::new(&mut buffer); // write cairo_rs vm trace file - write_encoded_trace(trace_entries, &mut buff_writer).unwrap(); + write_encoded_trace(&trace_entries, &mut buff_writer).unwrap(); // compare that the original cairo vm trace file and cairo_rs vm trace files are equal assert_eq!(buffer, *expected_encoded_trace); @@ -369,6 +369,5 @@ mod tests { .run_until_pc(end, &mut vm, &mut hint_processor) .is_ok()); assert!(cairo_runner.relocate(&mut vm, false).is_ok()); - assert!(vm.get_relocated_trace().is_err()); } } diff --git a/vm/src/vm/runners/cairo_runner.rs b/vm/src/vm/runners/cairo_runner.rs index 9a7e4d0547..3e9ac395df 100644 --- a/vm/src/vm/runners/cairo_runner.rs +++ b/vm/src/vm/runners/cairo_runner.rs @@ -1307,7 +1307,10 @@ impl CairoRunner { dyn_layout, &vm.get_public_memory_addresses()?, vm.get_memory_segment_addresses()?, - vm.get_relocated_trace()?, + &self + .relocated_trace + .as_ref() + .ok_or(PublicInputError::EmptyTrace)?, self.get_perm_range_check_limits(vm) .ok_or(PublicInputError::NoRangeCheckLimits)?, ) @@ -2838,7 +2841,7 @@ mod tests { .segments .relocate_segments() .expect("Couldn't relocate after compute effective sizes"); - vm.relocate_trace(&rel_table).unwrap(); + cairo_runner.relocate_trace(&mut vm, &rel_table).unwrap(); let relocated_trace = vm.trace.unwrap(); assert_eq!(relocated_trace.len(), 12); assert_eq!( diff --git a/vm/src/vm/trace/mod.rs b/vm/src/vm/trace/mod.rs index ac249689f2..e729fc22f4 100644 --- a/vm/src/vm/trace/mod.rs +++ b/vm/src/vm/trace/mod.rs @@ -13,7 +13,7 @@ pub mod trace_entry { /// A trace entry for every instruction that was executed. /// Holds the register values before the instruction was executed, after going through the relocation process - #[derive(Debug, PartialEq, Eq, Deserialize, Serialize)] + #[derive(Debug, PartialEq, Eq, Deserialize, Serialize, Clone)] pub struct RelocatedTraceEntry { pub pc: usize, pub ap: usize, diff --git a/vm/src/vm/vm_core.rs b/vm/src/vm/vm_core.rs index 3bc833adad..d74b5be5c1 100644 --- a/vm/src/vm/vm_core.rs +++ b/vm/src/vm/vm_core.rs @@ -28,7 +28,6 @@ use felt::Felt252; use num_traits::{ToPrimitive, Zero}; use super::errors::runner_errors::RunnerError; -use super::errors::trace_errors::TraceError; use super::runners::builtin_runner::OUTPUT_BUILTIN_NAME; const MAX_TRACEBACK_ENTRIES: u32 = 20; @@ -80,7 +79,6 @@ pub struct VirtualMachine { pub(crate) trace: Option>, pub(crate) current_step: usize, pub(crate) rc_limits: Option<(isize, isize)>, - trace_relocated: bool, skip_instruction_execution: bool, run_finished: bool, instruction_cache: Vec>, @@ -112,7 +110,6 @@ impl VirtualMachine { segments: MemorySegmentManager::new(), rc_limits: None, run_finished: false, - trace_relocated: false, instruction_cache: Vec::new(), #[cfg(feature = "hooks")] hooks: Default::default(), @@ -965,34 +962,6 @@ impl VirtualMachine { Ok(()) } - ///Relocates the VM's trace, turning relocatable registers to numbered ones - pub fn relocate_trace(&mut self, relocation_table: &[usize]) -> Result<(), TraceError> { - if let Some(ref mut trace) = self.trace { - if self.trace_relocated { - return Err(TraceError::AlreadyRelocated); - } - let segment_1_base = relocation_table - .get(1) - .ok_or(TraceError::NoRelocationFound)?; - - trace.iter_mut().for_each(|entry| { - entry.pc += 1; - entry.ap += segment_1_base; - entry.fp += segment_1_base; - }); - self.trace_relocated = true; - } - Ok(()) - } - - pub fn get_relocated_trace(&self) -> Result<&Vec, TraceError> { - if self.trace_relocated { - self.trace.as_ref().ok_or(TraceError::TraceNotEnabled) - } else { - Err(TraceError::TraceNotRelocated) - } - } - /// Returns a list of addresses of memory cells that constitute the public memory. pub fn get_public_memory_addresses(&self) -> Result, VirtualMachineError> { if let Some(relocation_table) = &self.relocation_table { @@ -1128,7 +1097,6 @@ impl VirtualMachineBuilder { segments: self.segments, rc_limits: None, run_finished: self.run_finished, - trace_relocated: false, instruction_cache: Vec::new(), #[cfg(feature = "hooks")] hooks: self.hooks, From ab9eb1a105095a0f7da804784e3d71533048e4b4 Mon Sep 17 00:00:00 2001 From: Federica Date: Thu, 23 Nov 2023 16:16:10 -0300 Subject: [PATCH 04/15] Fix test --- vm/src/cairo_run.rs | 2 +- vm/src/tests/mod.rs | 6 +++--- vm/src/vm/runners/cairo_runner.rs | 31 ++++++++++++++++--------------- 3 files changed, 20 insertions(+), 19 deletions(-) diff --git a/vm/src/cairo_run.rs b/vm/src/cairo_run.rs index 050da98fd2..5d69be4cdf 100644 --- a/vm/src/cairo_run.rs +++ b/vm/src/cairo_run.rs @@ -248,7 +248,7 @@ mod tests { assert!(cairo_runner .run_until_pc(end, &mut vm, &mut hint_processor) .is_ok()); - assert!(cairo_runner.relocate(&mut vm, true).is_ok()); + assert!(dbg!(cairo_runner.relocate(&mut vm, true)).is_ok()); // `main` returns without doing nothing, but `not_main` sets `[ap]` to `1` // Memory location was found empirically and simply hardcoded assert_eq!(cairo_runner.relocated_memory[2], Some(Felt252::new(123))); diff --git a/vm/src/tests/mod.rs b/vm/src/tests/mod.rs index cecacbad8d..3e408ba6af 100644 --- a/vm/src/tests/mod.rs +++ b/vm/src/tests/mod.rs @@ -2,6 +2,7 @@ use crate::vm::errors::cairo_run_errors::CairoRunError; #[cfg(feature = "cairo-1-hints")] use crate::vm::runners::cairo_runner::RunResources; +use crate::vm::trace::trace_entry::RelocatedTraceEntry; #[cfg(feature = "cairo-1-hints")] use crate::{ hint_processor::cairo_1_hint_processor::hint_processor::Cairo1HintProcessor, @@ -22,7 +23,6 @@ use crate::stdlib::prelude::*; use crate::{ cairo_run::{cairo_run, CairoRunConfig}, hint_processor::builtin_hint_processor::builtin_hint_processor_definition::BuiltinHintProcessor, - vm::trace::trace_entry::TraceEntry, }; #[cfg(target_arch = "wasm32")] @@ -93,9 +93,9 @@ pub(self) fn run_program( let expected_trace: Vec<_> = trace .iter() .copied() - .map(|(pc, ap, fp)| TraceEntry { pc, ap, fp }) + .map(|(pc, ap, fp)| RelocatedTraceEntry { pc, ap, fp }) .collect(); - let trace = vm.get_relocated_trace().unwrap(); + let trace = runner.relocated_trace.as_ref().unwrap(); assert_eq!(trace.len(), expected_trace.len()); for (entry, expected) in trace.iter().zip(expected_trace.iter()) { assert_eq!(entry, expected); diff --git a/vm/src/vm/runners/cairo_runner.rs b/vm/src/vm/runners/cairo_runner.rs index 3e9ac395df..b90f4ab7f2 100644 --- a/vm/src/vm/runners/cairo_runner.rs +++ b/vm/src/vm/runners/cairo_runner.rs @@ -842,8 +842,9 @@ impl CairoRunner { return Err(TraceError::MemoryError(memory_error)); } } - - self.relocate_trace(vm, &relocation_table)?; + if vm.trace.is_some() { + self.relocate_trace(vm, &relocation_table)?; + } vm.relocation_table = Some(relocation_table); Ok(()) } @@ -2842,11 +2843,11 @@ mod tests { .relocate_segments() .expect("Couldn't relocate after compute effective sizes"); cairo_runner.relocate_trace(&mut vm, &rel_table).unwrap(); - let relocated_trace = vm.trace.unwrap(); + let relocated_trace = cairo_runner.relocated_trace.unwrap(); assert_eq!(relocated_trace.len(), 12); assert_eq!( relocated_trace[0], - TraceEntry { + RelocatedTraceEntry { pc: 5, ap: 18, fp: 18 @@ -2854,7 +2855,7 @@ mod tests { ); assert_eq!( relocated_trace[1], - TraceEntry { + RelocatedTraceEntry { pc: 6, ap: 19, fp: 18 @@ -2862,7 +2863,7 @@ mod tests { ); assert_eq!( relocated_trace[2], - TraceEntry { + RelocatedTraceEntry { pc: 8, ap: 20, fp: 18 @@ -2870,7 +2871,7 @@ mod tests { ); assert_eq!( relocated_trace[3], - TraceEntry { + RelocatedTraceEntry { pc: 1, ap: 22, fp: 22 @@ -2878,7 +2879,7 @@ mod tests { ); assert_eq!( relocated_trace[4], - TraceEntry { + RelocatedTraceEntry { pc: 2, ap: 22, fp: 22 @@ -2886,7 +2887,7 @@ mod tests { ); assert_eq!( relocated_trace[5], - TraceEntry { + RelocatedTraceEntry { pc: 4, ap: 23, fp: 22 @@ -2894,7 +2895,7 @@ mod tests { ); assert_eq!( relocated_trace[6], - TraceEntry { + RelocatedTraceEntry { pc: 10, ap: 23, fp: 18 @@ -2902,7 +2903,7 @@ mod tests { ); assert_eq!( relocated_trace[7], - TraceEntry { + RelocatedTraceEntry { pc: 12, ap: 24, fp: 18 @@ -2910,7 +2911,7 @@ mod tests { ); assert_eq!( relocated_trace[8], - TraceEntry { + RelocatedTraceEntry { pc: 1, ap: 26, fp: 26 @@ -2918,7 +2919,7 @@ mod tests { ); assert_eq!( relocated_trace[9], - TraceEntry { + RelocatedTraceEntry { pc: 2, ap: 26, fp: 26 @@ -2926,7 +2927,7 @@ mod tests { ); assert_eq!( relocated_trace[10], - TraceEntry { + RelocatedTraceEntry { pc: 4, ap: 27, fp: 26 @@ -2934,7 +2935,7 @@ mod tests { ); assert_eq!( relocated_trace[11], - TraceEntry { + RelocatedTraceEntry { pc: 14, ap: 27, fp: 18 From f61c82c5fb9beb69823e072346a5042a860c1d2b Mon Sep 17 00:00:00 2001 From: Federica Date: Thu, 23 Nov 2023 17:12:57 -0300 Subject: [PATCH 05/15] Update tests --- vm/src/utils.rs | 20 +++++-- vm/src/vm/runners/cairo_runner.rs | 99 +++++++++++++++++-------------- vm/src/vm/trace/mod.rs | 21 ++++++- vm/src/vm/vm_core.rs | 28 +++++---- 4 files changed, 105 insertions(+), 63 deletions(-) diff --git a/vm/src/utils.rs b/vm/src/utils.rs index 23d5e67dc1..389be4e5c6 100644 --- a/vm/src/utils.rs +++ b/vm/src/utils.rs @@ -413,7 +413,10 @@ pub mod test_utils { pub(crate) use non_continuous_ids_data; #[track_caller] - pub(crate) fn trace_check(actual: &[TraceEntry], expected: &[(usize, usize, usize)]) { + pub(crate) fn trace_check( + actual: &[TraceEntry], + expected: &[(crate::utils::Relocatable, usize, usize)], + ) { assert_eq!(actual.len(), expected.len()); for (entry, expected) in actual.iter().zip(expected.iter()) { assert_eq!(&(entry.pc, entry.ap, entry.fp), expected); @@ -704,22 +707,29 @@ mod test { fn assert_trace() { let trace = vec![ TraceEntry { - pc: 2, + pc: (0, 2).into(), ap: 7, fp: 1, }, TraceEntry { - pc: 5, + pc: (0, 5).into(), ap: 1, fp: 0, }, TraceEntry { - pc: 9, + pc: (0, 9).into(), ap: 5, fp: 7, }, ]; - trace_check(&trace, &[(2, 7, 1), (5, 1, 0), (9, 5, 7)]); + trace_check( + &trace, + &[ + ((0, 2).into(), 7, 1), + ((0, 5).into(), 1, 0), + ((0, 9).into(), 5, 7), + ], + ); } #[test] diff --git a/vm/src/vm/runners/cairo_runner.rs b/vm/src/vm/runners/cairo_runner.rs index b90f4ab7f2..7e7dfbbff5 100644 --- a/vm/src/vm/runners/cairo_runner.rs +++ b/vm/src/vm/runners/cairo_runner.rs @@ -8,7 +8,8 @@ use crate::{ }, types::instance_definitions::keccak_instance_def::KeccakInstanceDef, vm::{ - runners::builtin_runner::SegmentArenaBuiltinRunner, trace::trace_entry::RelocatedTraceEntry, + runners::builtin_runner::SegmentArenaBuiltinRunner, + trace::trace_entry::{relocate_trace_register, RelocatedTraceEntry}, }, }; @@ -778,7 +779,7 @@ impl CairoRunner { for entry in trace { relocated_trace.push(RelocatedTraceEntry { - pc: entry.pc + 1, + pc: relocate_trace_register(entry.pc, relocation_table)?, ap: entry.ap + segment_1_base, fp: entry.fp + segment_1_base, }) @@ -2144,7 +2145,13 @@ mod tests { assert_eq!(trace.len(), 5); trace_check( &trace, - &[(3, 2, 2), (5, 3, 2), (0, 5, 5), (2, 6, 5), (7, 6, 2)], + &[ + ((0, 3).into(), 2, 2), + ((0, 5).into(), 3, 2), + ((0, 0).into(), 5, 5), + ((0, 2).into(), 6, 5), + ((0, 7).into(), 6, 2), + ], ); } @@ -2221,16 +2228,16 @@ mod tests { trace_check( &trace, &[ - (8, 3, 3), - (9, 4, 3), - (11, 5, 3), - (0, 7, 7), - (1, 7, 7), - (3, 8, 7), - (4, 9, 7), - (5, 9, 7), - (7, 10, 7), - (13, 10, 3), + ((0, 8).into(), 3, 3), + ((0, 9).into(), 4, 3), + ((0, 11).into(), 5, 3), + ((0, 0).into(), 7, 7), + ((0, 1).into(), 7, 7), + ((0, 3).into(), 8, 7), + ((0, 4).into(), 9, 7), + ((0, 5).into(), 9, 7), + ((0, 7).into(), 10, 7), + ((0, 13).into(), 10, 3), ], ); //Check the range_check builtin segment @@ -2337,18 +2344,18 @@ mod tests { trace_check( &trace, &[ - (4, 3, 3), - (5, 4, 3), - (7, 5, 3), - (0, 7, 7), - (1, 7, 7), - (3, 8, 7), - (9, 8, 3), - (11, 9, 3), - (0, 11, 11), - (1, 11, 11), - (3, 12, 11), - (13, 12, 3), + ((0, 4).into(), 3, 3), + ((0, 5).into(), 4, 3), + ((0, 7).into(), 5, 3), + ((0, 0).into(), 7, 7), + ((0, 1).into(), 7, 7), + ((0, 3).into(), 8, 7), + ((0, 9).into(), 8, 3), + ((0, 11).into(), 9, 3), + ((0, 0).into(), 11, 11), + ((0, 1).into(), 11, 11), + ((0, 3).into(), 12, 11), + ((0, 13).into(), 12, 3), ], ); //Check that the output to be printed is correct @@ -2475,24 +2482,24 @@ mod tests { trace_check( &trace, &[ - (13, 4, 4), - (14, 5, 4), - (16, 6, 4), - (4, 8, 8), - (5, 8, 8), - (7, 9, 8), - (8, 10, 8), - (9, 10, 8), - (11, 11, 8), - (12, 12, 8), - (18, 12, 4), - (19, 13, 4), - (20, 14, 4), - (0, 16, 16), - (1, 16, 16), - (3, 17, 16), - (22, 17, 4), - (23, 18, 4), + ((0, 13).into(), 4, 4), + ((0, 14).into(), 5, 4), + ((0, 16).into(), 6, 4), + ((0, 4).into(), 8, 8), + ((0, 5).into(), 8, 8), + ((0, 7).into(), 9, 8), + ((0, 8).into(), 10, 8), + ((0, 9).into(), 10, 8), + ((0, 11).into(), 11, 8), + ((0, 12).into(), 12, 8), + ((0, 18).into(), 12, 4), + ((0, 19).into(), 13, 4), + ((0, 20).into(), 14, 4), + ((0, 0).into(), 16, 16), + ((0, 1).into(), 16, 16), + ((0, 3).into(), 17, 16), + ((0, 22).into(), 17, 4), + ((0, 23).into(), 18, 4), ], ); //Check the range_check builtin segment @@ -4007,7 +4014,7 @@ mod tests { 0x80FF_8000_0530u64 )))]]; vm.trace = Some(vec![TraceEntry { - pc: 0, + pc: (0, 0).into(), ap: 0, fp: 0, }]); @@ -4029,7 +4036,7 @@ mod tests { 0x80FF_8000_0530u64 )))]]; vm.trace = Some(vec![TraceEntry { - pc: 0, + pc: (0, 0).into(), ap: 0, fp: 0, }]); @@ -4097,7 +4104,7 @@ mod tests { 0x80FF_8000_0530u64 )))]]; vm.trace = Some(vec![TraceEntry { - pc: 0, + pc: (0, 0).into(), ap: 0, fp: 0, }]); diff --git a/vm/src/vm/trace/mod.rs b/vm/src/vm/trace/mod.rs index e729fc22f4..8f2a17cd36 100644 --- a/vm/src/vm/trace/mod.rs +++ b/vm/src/vm/trace/mod.rs @@ -1,12 +1,17 @@ pub mod trace_entry { use serde::{Deserialize, Serialize}; + use crate::{ + types::relocatable::Relocatable, + vm::errors::{memory_errors::MemoryError, trace_errors::TraceError}, + }; + ///A trace entry for every instruction that was executed. ///Holds the register values before the instruction was executed. ///Register values for ap & fp are represented as their offsets, as their indexes will always be 1 #[derive(Debug, PartialEq, Eq, Deserialize, Serialize)] pub struct TraceEntry { - pub pc: usize, + pub pc: Relocatable, pub ap: usize, pub fp: usize, } @@ -19,4 +24,18 @@ pub mod trace_entry { pub ap: usize, pub fp: usize, } + + pub fn relocate_trace_register( + value: Relocatable, + relocation_table: &Vec, + ) -> Result { + let segment_index: usize = value.segment_index.try_into().map_err(|_| { + TraceError::MemoryError(MemoryError::AddressInTemporarySegment(value.segment_index)) + })?; + + if relocation_table.len() <= segment_index { + return Err(TraceError::NoRelocationFound); + } + Ok(relocation_table[segment_index] + value.offset) + } } diff --git a/vm/src/vm/vm_core.rs b/vm/src/vm/vm_core.rs index d74b5be5c1..0ac788aa6d 100644 --- a/vm/src/vm/vm_core.rs +++ b/vm/src/vm/vm_core.rs @@ -393,7 +393,7 @@ impl VirtualMachine { if let Some(ref mut trace) = &mut self.trace { trace.push(TraceEntry { - pc: self.run_context.pc.offset, + pc: self.run_context.pc, ap: self.run_context.ap, fp: self.run_context.fp, }); @@ -2884,7 +2884,7 @@ mod tests { Ok(()) ); let trace = vm.trace.unwrap(); - trace_check(&trace, &[(0, 2, 2)]); + trace_check(&trace, &[((0, 0).into(), 2, 2)]); assert_eq!(vm.run_context.pc, Relocatable::from((3, 0))); assert_eq!(vm.run_context.ap, 2); @@ -2978,7 +2978,13 @@ mod tests { assert_eq!(trace.len(), 5); trace_check( &trace, - &[(3, 2, 2), (5, 3, 2), (0, 5, 5), (2, 6, 5), (7, 6, 2)], + &[ + ((0, 3).into(), 2, 2), + ((0, 5).into(), 3, 2), + ((0, 0).into(), 5, 5), + ((0, 2).into(), 6, 5), + ((0, 7).into(), 6, 2), + ], ); //Check that the following addresses have been accessed: // Addresses have been copied from python execution: @@ -3677,12 +3683,12 @@ mod tests { trace_check( &trace, &[ - (3, 2, 2), - (0, 4, 4), - (2, 5, 4), - (5, 5, 2), - (7, 6, 2), - (8, 6, 2), + ((0, 3).into(), 2, 2), + ((0, 0).into(), 4, 4), + ((0, 2).into(), 5, 4), + ((0, 5).into(), 5, 2), + ((0, 7).into(), 6, 2), + ((0, 8).into(), 6, 2), ], ); @@ -4197,7 +4203,7 @@ mod tests { }) .skip_instruction_execution(true) .trace(Some(vec![TraceEntry { - pc: 1, + pc: (0, 1).into(), ap: 1, fp: 1, }])); @@ -4239,7 +4245,7 @@ mod tests { assert_eq!( virtual_machine_from_builder.trace, Some(vec![TraceEntry { - pc: 1, + pc: (0, 1).into(), ap: 1, fp: 1, }]) From c47c41cc7992d4ffe6d56c3a877aa540ee1bddd8 Mon Sep 17 00:00:00 2001 From: Federica Date: Thu, 23 Nov 2023 17:15:05 -0300 Subject: [PATCH 06/15] clippy --- vm/src/vm/runners/cairo_runner.rs | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/vm/src/vm/runners/cairo_runner.rs b/vm/src/vm/runners/cairo_runner.rs index 7e7dfbbff5..ecc4ceab19 100644 --- a/vm/src/vm/runners/cairo_runner.rs +++ b/vm/src/vm/runners/cairo_runner.rs @@ -1309,8 +1309,7 @@ impl CairoRunner { dyn_layout, &vm.get_public_memory_addresses()?, vm.get_memory_segment_addresses()?, - &self - .relocated_trace + self.relocated_trace .as_ref() .ok_or(PublicInputError::EmptyTrace)?, self.get_perm_range_check_limits(vm) @@ -2604,7 +2603,7 @@ mod tests { .segments .relocate_segments() .expect("Couldn't relocate after compute effective sizes"); - assert_eq!(cairo_runner.relocate_memory(&mut vm, &rel_table), Ok(())); + assert_eq!(cairo_runner.relocate_memory(&vm, &rel_table), Ok(())); assert_eq!(cairo_runner.relocated_memory[0], None); assert_eq!( cairo_runner.relocated_memory[1], @@ -2710,7 +2709,7 @@ mod tests { .segments .relocate_segments() .expect("Couldn't relocate after compute effective sizes"); - assert_eq!(cairo_runner.relocate_memory(&mut vm, &rel_table), Ok(())); + assert_eq!(cairo_runner.relocate_memory(&vm, &rel_table), Ok(())); assert_eq!(cairo_runner.relocated_memory[0], None); assert_eq!( cairo_runner.relocated_memory[1], From def1bbeb7cab31c336e7196bbfe2f92e82ee9ca8 Mon Sep 17 00:00:00 2001 From: Federica Date: Thu, 23 Nov 2023 17:26:48 -0300 Subject: [PATCH 07/15] clippy --- cairo-vm-cli/src/main.rs | 5 ++--- vm/src/vm/runners/cairo_runner.rs | 2 +- 2 files changed, 3 insertions(+), 4 deletions(-) diff --git a/cairo-vm-cli/src/main.rs b/cairo-vm-cli/src/main.rs index 4f3cbe1070..6bfd81db4c 100644 --- a/cairo-vm-cli/src/main.rs +++ b/cairo-vm-cli/src/main.rs @@ -152,14 +152,13 @@ fn run(args: impl Iterator) -> Result<(), Error> { let relocated_trace = cairo_runner .relocated_trace .as_ref() - .ok_or(Error::Trace(TraceError::TraceNotRelocated))? - .clone(); + .ok_or(Error::Trace(TraceError::TraceNotRelocated))?; let trace_file = std::fs::File::create(trace_path)?; let mut trace_writer = FileWriter::new(io::BufWriter::with_capacity(3 * 1024 * 1024, trace_file)); - cairo_run::write_encoded_trace(&relocated_trace, &mut trace_writer)?; + cairo_run::write_encoded_trace(relocated_trace, &mut trace_writer)?; trace_writer.flush()?; } diff --git a/vm/src/vm/runners/cairo_runner.rs b/vm/src/vm/runners/cairo_runner.rs index ecc4ceab19..68bf5f2fa8 100644 --- a/vm/src/vm/runners/cairo_runner.rs +++ b/vm/src/vm/runners/cairo_runner.rs @@ -2848,7 +2848,7 @@ mod tests { .segments .relocate_segments() .expect("Couldn't relocate after compute effective sizes"); - cairo_runner.relocate_trace(&mut vm, &rel_table).unwrap(); + cairo_runner.relocate_trace(&vm, &rel_table).unwrap(); let relocated_trace = cairo_runner.relocated_trace.unwrap(); assert_eq!(relocated_trace.len(), 12); assert_eq!( From 94393f180bbb1fb08f20b2f252b7668d8182201f Mon Sep 17 00:00:00 2001 From: Federica Date: Thu, 23 Nov 2023 17:37:39 -0300 Subject: [PATCH 08/15] Remove dbg print + add changelog entry --- CHANGELOG.md | 5 +++++ vm/src/cairo_run.rs | 2 +- 2 files changed, 6 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index dd8e4f920e..dda51fcbf7 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,11 @@ #### Upcoming Changes +* feat: `Partially Revert Optimize trace relocation #906` [#492](https://github.com/lambdaclass/cairo-vm/pull/492) + + * Remove methods `VirtualMachine::get_relocated_trace`& `VirtualMachine::relocate_trace` + * Add `relocated_trace` field to `CairoRunner` + #### [0.9.1] - 2023-11-16 * chore: bump `cairo-lang-` dependencies to 2.3.1 [#1482](https://github.com/lambdaclass/cairo-vm/pull/1482), [#1483](https://github.com/lambdaclass/cairo-vm/pull/1483) diff --git a/vm/src/cairo_run.rs b/vm/src/cairo_run.rs index 5d69be4cdf..050da98fd2 100644 --- a/vm/src/cairo_run.rs +++ b/vm/src/cairo_run.rs @@ -248,7 +248,7 @@ mod tests { assert!(cairo_runner .run_until_pc(end, &mut vm, &mut hint_processor) .is_ok()); - assert!(dbg!(cairo_runner.relocate(&mut vm, true)).is_ok()); + assert!(cairo_runner.relocate(&mut vm, true).is_ok()); // `main` returns without doing nothing, but `not_main` sets `[ap]` to `1` // Memory location was found empirically and simply hardcoded assert_eq!(cairo_runner.relocated_memory[2], Some(Felt252::new(123))); From f3436d53630364da8871dfe15211ce1acc9f5f91 Mon Sep 17 00:00:00 2001 From: Federica Date: Thu, 23 Nov 2023 17:38:47 -0300 Subject: [PATCH 09/15] Add test check --- vm/src/cairo_run.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/vm/src/cairo_run.rs b/vm/src/cairo_run.rs index 050da98fd2..2980550270 100644 --- a/vm/src/cairo_run.rs +++ b/vm/src/cairo_run.rs @@ -369,5 +369,6 @@ mod tests { .run_until_pc(end, &mut vm, &mut hint_processor) .is_ok()); assert!(cairo_runner.relocate(&mut vm, false).is_ok()); + assert!(cairo_runner.relocated_trace.is_none()); } } From 1a81ed1b2feee68d4fd22dc305f8717779e49491 Mon Sep 17 00:00:00 2001 From: Federica Date: Thu, 23 Nov 2023 17:39:50 -0300 Subject: [PATCH 10/15] Add test check --- CHANGELOG.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index dda51fcbf7..7ef06566e4 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,10 +2,10 @@ #### Upcoming Changes -* feat: `Partially Revert Optimize trace relocation #906` [#492](https://github.com/lambdaclass/cairo-vm/pull/492) +* feat: Partially Revert `Optimize trace relocation #906` [#492](https://github.com/lambdaclass/cairo-vm/pull/492) * Remove methods `VirtualMachine::get_relocated_trace`& `VirtualMachine::relocate_trace` - * Add `relocated_trace` field to `CairoRunner` + * Add `relocated_trace` field to `CairoRunner` #### [0.9.1] - 2023-11-16 From 54c4ac2b781dcba966f1c6f6f4f0e4373086f437 Mon Sep 17 00:00:00 2001 From: Federica Date: Thu, 23 Nov 2023 17:59:59 -0300 Subject: [PATCH 11/15] Add missing wasm import --- vm/src/vm/trace/mod.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/vm/src/vm/trace/mod.rs b/vm/src/vm/trace/mod.rs index 8f2a17cd36..24c014056d 100644 --- a/vm/src/vm/trace/mod.rs +++ b/vm/src/vm/trace/mod.rs @@ -4,6 +4,7 @@ pub mod trace_entry { use crate::{ types::relocatable::Relocatable, vm::errors::{memory_errors::MemoryError, trace_errors::TraceError}, + stdlib::prelude::*, }; ///A trace entry for every instruction that was executed. From 200dd7a4017618ef9c6f511526e2514fb5228238 Mon Sep 17 00:00:00 2001 From: Federica Date: Fri, 24 Nov 2023 10:44:44 -0300 Subject: [PATCH 12/15] Fix pr num --- CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 8edbf8faa0..287c548aa2 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,7 +2,7 @@ #### Upcoming Changes -* feat: Partially Revert `Optimize trace relocation #906` [#492](https://github.com/lambdaclass/cairo-vm/pull/492) +* feat: Partially Revert `Optimize trace relocation #906` [#1492](https://github.com/lambdaclass/cairo-vm/pull/1492) * Remove methods `VirtualMachine::get_relocated_trace`& `VirtualMachine::relocate_trace` * Add `relocated_trace` field to `CairoRunner` From aad4dc9d08990f8e4e0df66017bf59f030d67036 Mon Sep 17 00:00:00 2001 From: Federica Date: Fri, 24 Nov 2023 17:43:26 -0300 Subject: [PATCH 13/15] Make relocate_trace public --- CHANGELOG.md | 2 +- vm/src/vm/runners/cairo_runner.rs | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 287c548aa2..b2123c23b9 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,7 +5,7 @@ * feat: Partially Revert `Optimize trace relocation #906` [#1492](https://github.com/lambdaclass/cairo-vm/pull/1492) * Remove methods `VirtualMachine::get_relocated_trace`& `VirtualMachine::relocate_trace` - * Add `relocated_trace` field to `CairoRunner` + * Add `relocated_trace` field & `relocate_trace` method to `CairoRunner` * feat: add debugging capabilities behind `print` feature flag. [#1476](https://github.com/lambdaclass/cairo-vm/pull/1476) diff --git a/vm/src/vm/runners/cairo_runner.rs b/vm/src/vm/runners/cairo_runner.rs index 68bf5f2fa8..1845833ec5 100644 --- a/vm/src/vm/runners/cairo_runner.rs +++ b/vm/src/vm/runners/cairo_runner.rs @@ -762,7 +762,7 @@ impl CairoRunner { } ///Relocates the VM's trace, turning relocatable registers to numbered ones - fn relocate_trace( + pub fn relocate_trace( &mut self, vm: &VirtualMachine, relocation_table: &Vec, From 5c517d8ee6157935418335eee992eca8c37a6dec Mon Sep 17 00:00:00 2001 From: Federica Date: Fri, 24 Nov 2023 17:45:30 -0300 Subject: [PATCH 14/15] List breaking changes --- CHANGELOG.md | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index b2123c23b9..c001a80f73 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,10 +2,11 @@ #### Upcoming Changes -* feat: Partially Revert `Optimize trace relocation #906` [#1492](https://github.com/lambdaclass/cairo-vm/pull/1492) +* 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` - * Add `relocated_trace` field & `relocate_trace` method to `CairoRunner` + * Remove methods `VirtualMachine::get_relocated_trace`& `VirtualMachine::relocate_trace`. + * Add `relocated_trace` field & `relocate_trace` method to `CairoRunner`. + * Swap `TraceEntry` for `RelocatedTraceEntry` type in `write_encoded_trace` & `PublicInput::new` signatures. * feat: add debugging capabilities behind `print` feature flag. [#1476](https://github.com/lambdaclass/cairo-vm/pull/1476) From 8ca276fc3e337b7f64a2afa1a33325a2380c7223 Mon Sep 17 00:00:00 2001 From: Federica Date: Fri, 24 Nov 2023 17:47:19 -0300 Subject: [PATCH 15/15] List pr purpose in changelog --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index c001a80f73..b2a21be759 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,7 @@ * Remove methods `VirtualMachine::get_relocated_trace`& `VirtualMachine::relocate_trace`. * Add `relocated_trace` field & `relocate_trace` method to `CairoRunner`. * Swap `TraceEntry` for `RelocatedTraceEntry` type in `write_encoded_trace` & `PublicInput::new` signatures. + * Now takes into account the program counter's segment index when building the execution trace instead of assuming it to be 0. * feat: add debugging capabilities behind `print` feature flag. [#1476](https://github.com/lambdaclass/cairo-vm/pull/1476)