diff --git a/CHANGELOG.md b/CHANGELOG.md index d0cd50c54b..1b543a741c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,8 @@ #### Upcoming Changes +* fix(BREAKING): Fix no trace padding flow in proof mode [#1909](https://github.com/lambdaclass/cairo-vm/pull/1909) + * feat: implement `kzg` data availability hints [#1887](https://github.com/lambdaclass/cairo-vm/pull/1887) #### [2.0.0-rc3] - 2024-12-26 diff --git a/bench/criterion_benchmark.rs b/bench/criterion_benchmark.rs index d473fe61f6..86a032df56 100644 --- a/bench/criterion_benchmark.rs +++ b/bench/criterion_benchmark.rs @@ -33,6 +33,7 @@ fn build_many_runners(c: &mut Criterion) { black_box(None), black_box(false), black_box(false), + black_box(false), ) .unwrap(), ); @@ -53,6 +54,7 @@ fn load_program_data(c: &mut Criterion) { None, false, false, + false, ) .unwrap() }, diff --git a/bench/iai_benchmark.rs b/bench/iai_benchmark.rs index 441f79b5bc..1b7a4c67a3 100644 --- a/bench/iai_benchmark.rs +++ b/bench/iai_benchmark.rs @@ -37,6 +37,7 @@ fn build_runner() { None, false, false, + false, ) .unwrap(); core::mem::drop(black_box(runner)); @@ -54,6 +55,7 @@ fn build_runner_helper() -> CairoRunner { None, false, false, + false, ) .unwrap() } diff --git a/cairo1-run/src/cairo_run.rs b/cairo1-run/src/cairo_run.rs index b196bda230..e0b417c321 100644 --- a/cairo1-run/src/cairo_run.rs +++ b/cairo1-run/src/cairo_run.rs @@ -258,6 +258,7 @@ pub fn cairo_run_program( cairo_run_config.dynamic_layout_params.clone(), runner_mode, cairo_run_config.trace_enabled, + false, )?; let end = runner.initialize(cairo_run_config.proof_mode)?; load_arguments(&mut runner, &cairo_run_config, main_func, initial_gas)?; diff --git a/hint_accountant/src/main.rs b/hint_accountant/src/main.rs index dcb5ca05a2..d639dd3e4f 100644 --- a/hint_accountant/src/main.rs +++ b/hint_accountant/src/main.rs @@ -49,7 +49,7 @@ fn run() { whitelists.push(whitelist_file.allowed_hint_expressions); } } - let mut vm = VirtualMachine::new(false); + let mut vm = VirtualMachine::new(false, false); let mut hint_executor = BuiltinHintProcessor::new_empty(); let (ap_tracking_data, reference_ids, references, mut exec_scopes, constants) = ( ApTracking::default(), diff --git a/vm/src/cairo_run.rs b/vm/src/cairo_run.rs index 0e61856a3b..1243ef1d0f 100644 --- a/vm/src/cairo_run.rs +++ b/vm/src/cairo_run.rs @@ -34,6 +34,9 @@ pub struct CairoRunConfig<'a> { pub dynamic_layout_params: Option, pub proof_mode: bool, pub secure_run: Option, + // Disable padding of the trace to accommodate the expected ratios-n_steps relationships w.r.t + // the layout. It does, however, pad the number of builtin instances to the next power of 2 + // compared to their values at the end of the execution, but doesn't modify n_steps. pub disable_trace_padding: bool, pub allow_missing_builtins: Option, } @@ -75,6 +78,7 @@ pub fn cairo_run_program_with_initial_scope( cairo_run_config.dynamic_layout_params.clone(), cairo_run_config.proof_mode, cairo_run_config.trace_enabled, + cairo_run_config.disable_trace_padding, )?; cairo_runner.exec_scopes = exec_scopes; @@ -162,6 +166,7 @@ pub fn cairo_run_pie( cairo_run_config.dynamic_layout_params.clone(), false, cairo_run_config.trace_enabled, + cairo_run_config.disable_trace_padding, )?; let end = cairo_runner.initialize(allow_missing_builtins)?; @@ -235,6 +240,7 @@ pub fn cairo_run_fuzzed_program( cairo_run_config.dynamic_layout_params.clone(), cairo_run_config.proof_mode, cairo_run_config.trace_enabled, + cairo_run_config.disable_trace_padding, )?; let _end = cairo_runner.initialize(allow_missing_builtins)?; diff --git a/vm/src/hint_processor/builtin_hint_processor/secp/cairo0_hints.rs b/vm/src/hint_processor/builtin_hint_processor/secp/cairo0_hints.rs index 6fec9cb10f..5e894588da 100644 --- a/vm/src/hint_processor/builtin_hint_processor/secp/cairo0_hints.rs +++ b/vm/src/hint_processor/builtin_hint_processor/secp/cairo0_hints.rs @@ -372,7 +372,7 @@ mod tests { #[test] #[cfg_attr(target_arch = "wasm32", wasm_bindgen_test)] fn test_is_on_curve_2() { - let mut vm = VirtualMachine::new(false); + let mut vm = VirtualMachine::new(false, false); vm.set_fp(1); let ids_data = non_continuous_ids_data![("is_on_curve", -1)]; vm.segments = segments![((1, 0), 1)]; @@ -404,7 +404,7 @@ mod tests { #[test] #[cfg_attr(target_arch = "wasm32", wasm_bindgen_test)] fn test_compute_q_mod_prime() { - let mut vm = VirtualMachine::new(false); + let mut vm = VirtualMachine::new(false, false); let ap_tracking = ApTracking::default(); @@ -431,7 +431,7 @@ mod tests { #[test] #[cfg_attr(target_arch = "wasm32", wasm_bindgen_test)] fn test_compute_ids_high_low() { - let mut vm = VirtualMachine::new(false); + let mut vm = VirtualMachine::new(false, false); let value = BigInt::from(25); let shift = BigInt::from(12); @@ -501,7 +501,7 @@ mod tests { #[test] #[cfg_attr(target_arch = "wasm32", wasm_bindgen_test)] fn test_r1_get_point_from_x() { - let mut vm = VirtualMachine::new(false); + let mut vm = VirtualMachine::new(false, false); vm.set_fp(10); let ids_data = non_continuous_ids_data![("x", -10), ("v", -7)]; @@ -560,7 +560,7 @@ mod tests { #[test] #[cfg_attr(target_arch = "wasm32", wasm_bindgen_test)] fn test_reduce_value() { - let mut vm = VirtualMachine::new(false); + let mut vm = VirtualMachine::new(false, false); //Initialize fp vm.run_context.fp = 10; @@ -616,7 +616,7 @@ mod tests { #[test] #[cfg_attr(target_arch = "wasm32", wasm_bindgen_test)] fn test_reduce_x() { - let mut vm = VirtualMachine::new(false); + let mut vm = VirtualMachine::new(false, false); //Initialize fp vm.run_context.fp = 10; diff --git a/vm/src/tests/cairo_run_test.rs b/vm/src/tests/cairo_run_test.rs index d847852929..c2dea2f4b2 100644 --- a/vm/src/tests/cairo_run_test.rs +++ b/vm/src/tests/cairo_run_test.rs @@ -1185,6 +1185,7 @@ fn run_program_with_custom_mod_builtin_params( cairo_run_config.dynamic_layout_params, cairo_run_config.proof_mode, cairo_run_config.trace_enabled, + cairo_run_config.disable_trace_padding, ) .unwrap(); diff --git a/vm/src/tests/mod.rs b/vm/src/tests/mod.rs index 9141c3a0f9..ecf44b2c71 100644 --- a/vm/src/tests/mod.rs +++ b/vm/src/tests/mod.rs @@ -116,6 +116,7 @@ fn run_cairo_1_entrypoint( None, false, false, + false, ) .unwrap(); @@ -221,6 +222,7 @@ fn run_cairo_1_entrypoint_with_run_resources( None, false, false, + false, ) .unwrap(); diff --git a/vm/src/utils.rs b/vm/src/utils.rs index 760c5201ad..6a6a284ee6 100644 --- a/vm/src/utils.rs +++ b/vm/src/utils.rs @@ -234,7 +234,7 @@ pub mod test_utils { macro_rules! vm_with_range_check { () => {{ - let mut vm = VirtualMachine::new(false); + let mut vm = VirtualMachine::new(false, false); vm.builtin_runners = vec![ $crate::vm::runners::builtin_runner::RangeCheckBuiltinRunner::<8>::new( Some(8), @@ -255,12 +255,13 @@ pub mod test_utils { None, false, false, + false, ) .unwrap() }; ($program:expr, $layout:expr) => { crate::vm::runners::cairo_runner::CairoRunner::new( - &$program, $layout, None, false, false, + &$program, $layout, None, false, false, false, ) .unwrap() }; @@ -271,6 +272,7 @@ pub mod test_utils { None, $proof_mode, false, + false, ) .unwrap() }; @@ -281,6 +283,7 @@ pub mod test_utils { None, $proof_mode, $trace_enabled, + false, ) .unwrap() }; @@ -405,11 +408,11 @@ pub mod test_utils { macro_rules! vm { () => {{ - crate::vm::vm_core::VirtualMachine::new(false) + crate::vm::vm_core::VirtualMachine::new(false, false) }}; ($use_trace:expr) => {{ - crate::vm::vm_core::VirtualMachine::new($use_trace) + crate::vm::vm_core::VirtualMachine::new($use_trace, false) }}; } pub(crate) use vm; diff --git a/vm/src/vm/runners/builtin_runner/mod.rs b/vm/src/vm/runners/builtin_runner/mod.rs index 2374f43f3d..358ca34e13 100644 --- a/vm/src/vm/runners/builtin_runner/mod.rs +++ b/vm/src/vm/runners/builtin_runner/mod.rs @@ -532,17 +532,35 @@ impl BuiltinRunner { Ok((used, used)) } _ => { - let used = self.get_used_cells(&vm.segments)?; - let size = self.get_allocated_memory_units(vm)?; - if used > size { - return Err(InsufficientAllocatedCellsError::BuiltinCells(Box::new(( - self.name(), - used, - size, - ))) - .into()); + let used_cells = self.get_used_cells(&vm.segments)?; + if vm.disable_trace_padding { + // If trace padding is disabled, we pad the used cells to still ensure that the + // number of instances is a power of 2. + let num_instances = self.get_used_instances(&vm.segments)?; + let padded_used_cells = if num_instances > 0 { + let n_output_cells = + self.cells_per_instance() as usize - self.n_input_cells() as usize; + assert!( + used_cells + n_output_cells + >= (self.cells_per_instance() as usize) * num_instances + ); + num_instances.next_power_of_two() * self.cells_per_instance() as usize + } else { + 0 + }; + Ok((used_cells, padded_used_cells)) + } else { + let size = self.get_allocated_memory_units(vm)?; + if used_cells > size { + return Err(InsufficientAllocatedCellsError::BuiltinCells(Box::new(( + self.name(), + used_cells, + size, + ))) + .into()); + } + Ok((used_cells, size)) } - Ok((used, size)) } } } @@ -693,11 +711,13 @@ impl From for BuiltinRunner { #[cfg(test)] mod tests { use super::*; + use crate::cairo_run::{cairo_run, CairoRunConfig}; use crate::hint_processor::builtin_hint_processor::builtin_hint_processor_definition::BuiltinHintProcessor; use crate::relocatable; use crate::types::builtin_name::BuiltinName; use crate::types::instance_definitions::mod_instance_def::ModInstanceDef; use crate::types::instance_definitions::LowRatio; + use crate::types::layout_name::LayoutName; use crate::types::program::Program; use crate::utils::test_utils::*; use crate::vm::errors::memory_errors::InsufficientAllocatedCellsError; @@ -877,6 +897,87 @@ mod tests { assert_eq!(builtin.get_allocated_memory_units(&cairo_runner.vm), Ok(5)); } + #[test] + #[cfg_attr(target_arch = "wasm32", wasm_bindgen_test)] + fn compare_proof_mode_with_and_without_disable_trace_padding() { + let program_data = + include_bytes!("../../../../../cairo_programs/proof_programs/pedersen_test.json"); + + let config_false = CairoRunConfig { + disable_trace_padding: false, + proof_mode: true, + layout: LayoutName::all_cairo, + ..Default::default() + }; + let mut hint_processor_false = BuiltinHintProcessor::new_empty(); + let runner_false = + cairo_run(program_data, &config_false, &mut hint_processor_false).unwrap(); + let last_step_false = runner_false.vm.current_step; + + assert!(last_step_false.is_power_of_two()); + + let config_true = CairoRunConfig { + disable_trace_padding: true, + proof_mode: true, + layout: LayoutName::all_cairo, + ..Default::default() + }; + let mut hint_processor_true = BuiltinHintProcessor::new_empty(); + let runner_true = cairo_run(program_data, &config_true, &mut hint_processor_true).unwrap(); + let last_step_true = runner_true.vm.current_step; + + // Ensure the last step is not a power of two - true for this specific program, not always. + assert!(!last_step_true.is_power_of_two()); + + assert!(last_step_true < last_step_false); + + let builtin_runners_false = &runner_false.vm.builtin_runners; + let builtin_runners_true = &runner_true.vm.builtin_runners; + assert_eq!(builtin_runners_false.len(), builtin_runners_true.len()); + // Compare allocated instances for each pair of builtin runners. + for (builtin_runner_false, builtin_runner_true) in builtin_runners_false + .iter() + .zip(builtin_runners_true.iter()) + { + assert_eq!(builtin_runner_false.name(), builtin_runner_true.name()); + match builtin_runner_false { + BuiltinRunner::Output(_) | BuiltinRunner::SegmentArena(_) => { + continue; + } + _ => {} + } + let (_, allocated_size_false) = builtin_runner_false + .get_used_cells_and_allocated_size(&runner_false.vm) + .unwrap(); + let (_, allocated_size_true) = builtin_runner_true + .get_used_cells_and_allocated_size(&runner_true.vm) + .unwrap(); + let n_allocated_instances_false = safe_div_usize( + allocated_size_false, + builtin_runner_false.cells_per_instance() as usize, + ) + .unwrap(); + let n_allocated_instances_true = safe_div_usize( + allocated_size_true, + builtin_runner_true.cells_per_instance() as usize, + ) + .unwrap(); + assert!( + n_allocated_instances_false.is_power_of_two() || n_allocated_instances_false == 0 + ); + assert!( + n_allocated_instances_true.is_power_of_two() || n_allocated_instances_true == 0 + ); + // Checks that the number of allocated instances is diffrent when trace padding is + // enabled/disabled. Holds for this specific program, not always (that is, in other + // programs, padding may be of size 0, or the same). + assert!( + n_allocated_instances_true == 0 + || n_allocated_instances_true != n_allocated_instances_false + ); + } + } + #[test] #[cfg_attr(target_arch = "wasm32", wasm_bindgen_test)] fn get_allocated_memory_units_ec_op_with_items() { diff --git a/vm/src/vm/runners/builtin_runner/modulo.rs b/vm/src/vm/runners/builtin_runner/modulo.rs index 085e3549b6..ef11096535 100644 --- a/vm/src/vm/runners/builtin_runner/modulo.rs +++ b/vm/src/vm/runners/builtin_runner/modulo.rs @@ -817,7 +817,7 @@ mod tests { let mut hint_processor = BuiltinHintProcessor::new_empty(); let program = Program::from_bytes(program_data, Some("main")).unwrap(); let mut runner = - CairoRunner::new(&program, LayoutName::all_cairo, None, true, false).unwrap(); + CairoRunner::new(&program, LayoutName::all_cairo, None, true, false, false).unwrap(); let end = runner.initialize(false).unwrap(); // Modify add_mod & mul_mod params diff --git a/vm/src/vm/runners/cairo_runner.rs b/vm/src/vm/runners/cairo_runner.rs index 65209c35de..266a3eb1e5 100644 --- a/vm/src/vm/runners/cairo_runner.rs +++ b/vm/src/vm/runners/cairo_runner.rs @@ -176,6 +176,7 @@ impl CairoRunner { dynamic_layout_params: Option, mode: RunnerMode, trace_enabled: bool, + disable_trace_padding: bool, ) -> Result { let cairo_layout = match layout { LayoutName::plain => CairoLayout::plain_instance(), @@ -197,7 +198,7 @@ impl CairoRunner { }; Ok(CairoRunner { program: program.clone(), - vm: VirtualMachine::new(trace_enabled), + vm: VirtualMachine::new(trace_enabled, disable_trace_padding), layout: cairo_layout, final_pc: None, program_base: None, @@ -226,6 +227,7 @@ impl CairoRunner { dynamic_layout_params: Option, proof_mode: bool, trace_enabled: bool, + disable_trace_padding: bool, ) -> Result { if proof_mode { Self::new_v2( @@ -234,6 +236,7 @@ impl CairoRunner { dynamic_layout_params, RunnerMode::ProofModeCanonical, trace_enabled, + disable_trace_padding, ) } else { Self::new_v2( @@ -242,6 +245,7 @@ impl CairoRunner { dynamic_layout_params, RunnerMode::ExecutionMode, trace_enabled, + disable_trace_padding, ) } } diff --git a/vm/src/vm/vm_core.rs b/vm/src/vm/vm_core.rs index 5862aee093..c04fd3721e 100644 --- a/vm/src/vm/vm_core.rs +++ b/vm/src/vm/vm_core.rs @@ -89,6 +89,8 @@ pub struct VirtualMachine { pub(crate) rc_limits: Option<(isize, isize)>, skip_instruction_execution: bool, run_finished: bool, + // This flag is a parallel to the one in `struct CairoRunConfig`. + pub(crate) disable_trace_padding: bool, instruction_cache: Vec>, #[cfg(feature = "test_utils")] pub(crate) hooks: crate::vm::hooks::Hooks, @@ -96,7 +98,7 @@ pub struct VirtualMachine { } impl VirtualMachine { - pub fn new(trace_enabled: bool) -> VirtualMachine { + pub fn new(trace_enabled: bool, disable_trace_padding: bool) -> VirtualMachine { let run_context = RunContext { pc: Relocatable::from((0, 0)), ap: 0, @@ -118,6 +120,7 @@ impl VirtualMachine { segments: MemorySegmentManager::new(), rc_limits: None, run_finished: false, + disable_trace_padding, instruction_cache: Vec::new(), #[cfg(feature = "test_utils")] hooks: Default::default(), @@ -1255,6 +1258,7 @@ impl VirtualMachineBuilder { #[cfg(feature = "test_utils")] hooks: self.hooks, relocation_table: None, + disable_trace_padding: false, } } } @@ -1444,7 +1448,7 @@ mod tests { op1: MaybeRelocatable::Int(Felt252::from(10)), }; - let mut vm = VirtualMachine::new(false); + let mut vm = VirtualMachine::new(false, false); vm.run_context.pc = Relocatable::from((0, 4)); vm.run_context.ap = 5; vm.run_context.fp = 6;