diff --git a/CHANGELOG.md b/CHANGELOG.md index 91ef388a56..3dac6b636f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,14 @@ #### Upcoming Changes +* fix(BREAKING): Use program builtins in `initialize_main_entrypoint` & `read_return_values`[#1703](https://github.com/lambdaclass/cairo-vm/pull/1703) + * `initialize_main_entrypoint` now iterates over the program builtins when building the stack & inserts 0 for any missing builtin + * `read_return_values` now only computes the final stack of the builtins in the program + * BREAKING: `read_return_values` now takes a boolean argument `allow_missing_builtins` + * Added method `BuiltinRunner::identifier` to get the `BuiltinName` of each builtin + * BREAKING: `OutputBuiltinRunner::get_public_memory` now takes a reference to `MemorySegmentManager` + * BREAKING: method `VirtualMachine::get_memory_segment_addresses` moved to `CairoRunner::get_memory_segment_addresses` + * feat(BREAKING): Add range_check96 builtin[#1698](https://github.com/lambdaclass/cairo-vm/pull/1698) * Add the new `range_check96` builtin to the `all_cairo` layout. * `RangeCheckBuiltinRunner` changes: diff --git a/cairo-vm-cli/src/main.rs b/cairo-vm-cli/src/main.rs index f8fc6d0d62..ddb97c068a 100644 --- a/cairo-vm-cli/src/main.rs +++ b/cairo-vm-cli/src/main.rs @@ -338,7 +338,6 @@ mod tests { #[values(false, true)] memory_file: bool, #[values(false, true)] mut trace_file: bool, #[values(false, true)] proof_mode: bool, - #[values(false, true)] secure_run: bool, #[values(false, true)] print_output: bool, #[values(false, true)] entrypoint: bool, #[values(false, true)] air_public_input: bool, @@ -371,9 +370,6 @@ mod tests { if trace_file { args.extend_from_slice(&["--trace_file".to_string(), "/dev/null".to_string()]); } - if secure_run { - args.extend_from_slice(&["--secure_run".to_string(), "true".to_string()]); - } if print_output { args.extend_from_slice(&["--print_output".to_string()]); } diff --git a/cairo_programs/pedersen_extra_builtins.cairo b/cairo_programs/pedersen_extra_builtins.cairo new file mode 100644 index 0000000000..513ce93ddb --- /dev/null +++ b/cairo_programs/pedersen_extra_builtins.cairo @@ -0,0 +1,11 @@ +%builtins output pedersen range_check ecdsa bitwise ec_op + +from starkware.cairo.common.cairo_builtins import HashBuiltin +from starkware.cairo.common.hash import hash2 + +func main{output_ptr: felt*, pedersen_ptr: HashBuiltin*, range_check_ptr, ecdsa_ptr, bitwise_ptr, ec_op_ptr}() { + let (seed) = hash2{hash_ptr=pedersen_ptr}(0, 0); + assert [output_ptr] = seed; + let output_ptr = output_ptr + 1; + return (); +} diff --git a/vm/src/cairo_run.rs b/vm/src/cairo_run.rs index b0b4a53c60..07c3a29c77 100644 --- a/vm/src/cairo_run.rs +++ b/vm/src/cairo_run.rs @@ -100,7 +100,7 @@ pub fn cairo_run_program( )?; vm.verify_auto_deductions()?; - cairo_runner.read_return_values(&mut vm)?; + cairo_runner.read_return_values(&mut vm, allow_missing_builtins)?; if cairo_run_config.proof_mode { cairo_runner.finalize_segments(&mut vm)?; } @@ -135,6 +135,10 @@ pub fn cairo_run_fuzzed_program( .secure_run .unwrap_or(!cairo_run_config.proof_mode); + let allow_missing_builtins = cairo_run_config + .allow_missing_builtins + .unwrap_or(cairo_run_config.proof_mode); + let mut cairo_runner = CairoRunner::new( &program, cairo_run_config.layout, @@ -143,12 +147,7 @@ pub fn cairo_run_fuzzed_program( let mut vm = VirtualMachine::new(cairo_run_config.trace_enabled); - let _end = cairo_runner.initialize( - &mut vm, - cairo_run_config - .allow_missing_builtins - .unwrap_or(cairo_run_config.proof_mode), - )?; + let _end = cairo_runner.initialize(&mut vm, allow_missing_builtins)?; let res = match cairo_runner.run_until_steps(steps_limit, &mut vm, hint_executor) { Err(VirtualMachineError::EndOfProgram(_remaining)) => Ok(()), // program ran OK but ended before steps limit @@ -160,7 +159,7 @@ pub fn cairo_run_fuzzed_program( cairo_runner.end_run(false, false, &mut vm, hint_executor)?; vm.verify_auto_deductions()?; - cairo_runner.read_return_values(&mut vm)?; + cairo_runner.read_return_values(&mut vm, allow_missing_builtins)?; if cairo_run_config.proof_mode { cairo_runner.finalize_segments(&mut vm)?; } diff --git a/vm/src/tests/cairo_run_test.rs b/vm/src/tests/cairo_run_test.rs index 60bb867a6e..bf44200dd3 100644 --- a/vm/src/tests/cairo_run_test.rs +++ b/vm/src/tests/cairo_run_test.rs @@ -1071,6 +1071,40 @@ fn cairo_run_print_dict_array() { run_program_simple(program_data); } +#[test] +fn run_program_allow_missing_builtins() { + let program_data = include_bytes!("../../../cairo_programs/pedersen_extra_builtins.json"); + let config = CairoRunConfig { + allow_missing_builtins: Some(true), + layout: "small", // The program logic only uses builtins in the small layout but contains builtins outside of it + ..Default::default() + }; + assert!(crate::cairo_run::cairo_run( + program_data, + &config, + &mut BuiltinHintProcessor::new_empty() + ) + .is_ok()) +} + +#[test] +fn run_program_allow_missing_builtins_proof() { + let program_data = + include_bytes!("../../../cairo_programs/proof_programs/pedersen_extra_builtins.json"); + let config = CairoRunConfig { + proof_mode: true, + allow_missing_builtins: Some(true), + layout: "small", // The program logic only uses builtins in the small layout but contains builtins outside of it + ..Default::default() + }; + assert!(crate::cairo_run::cairo_run( + program_data, + &config, + &mut BuiltinHintProcessor::new_empty() + ) + .is_ok()) +} + #[test] #[cfg(feature = "mod_builtin")] fn cairo_run_mod_builtin() { @@ -1174,15 +1208,17 @@ fn run_program_with_custom_mod_builtin_params( .unwrap(); vm.verify_auto_deductions().unwrap(); - cairo_runner.read_return_values(&mut vm).unwrap(); + cairo_runner.read_return_values(&mut vm, false).unwrap(); if cairo_run_config.proof_mode { cairo_runner.finalize_segments(&mut vm).unwrap(); } - let security_res = verify_secure_runner(&cairo_runner, true, None, &mut vm); - if let Some(error) = security_error { - assert!(security_res.is_err()); - assert!(security_res.err().unwrap().to_string().contains(error)); - return; + if !cairo_run_config.proof_mode { + let security_res = verify_secure_runner(&cairo_runner, true, None, &mut vm); + if let Some(error) = security_error { + assert!(security_res.is_err()); + assert!(security_res.err().unwrap().to_string().contains(error)); + return; + } + security_res.unwrap(); } - security_res.unwrap(); } diff --git a/vm/src/vm/errors/runner_errors.rs b/vm/src/vm/errors/runner_errors.rs index ac59da83e9..b0961d69df 100644 --- a/vm/src/vm/errors/runner_errors.rs +++ b/vm/src/vm/errors/runner_errors.rs @@ -122,6 +122,10 @@ pub enum RunnerError { FillMemoryCoudNotFillTable(usize, usize), #[error("{}: {}", (*.0).0, (*.0).1)] ModBuiltinSecurityCheck(Box<(&'static str, String)>), + #[error("{0} is missing")] + MissingBuiltin(&'static str), + #[error("The stop pointer of the missing builtin {0} must be 0")] + MissingBuiltinStopPtrNotZero(&'static str), } #[cfg(test)] diff --git a/vm/src/vm/runners/builtin_runner/mod.rs b/vm/src/vm/runners/builtin_runner/mod.rs index e8f2e9502e..75cc0c8983 100644 --- a/vm/src/vm/runners/builtin_runner/mod.rs +++ b/vm/src/vm/runners/builtin_runner/mod.rs @@ -1,5 +1,6 @@ use crate::air_private_input::PrivateInput; use crate::math_utils::safe_div_usize; +use crate::serde::deserialize_program::BuiltinName; use crate::stdlib::prelude::*; use crate::types::relocatable::{MaybeRelocatable, Relocatable}; use crate::vm::errors::memory_errors::{self, InsufficientAllocatedCellsError, MemoryError}; @@ -412,6 +413,22 @@ impl BuiltinRunner { } } + pub fn identifier(&self) -> BuiltinName { + match self { + BuiltinRunner::Bitwise(_) => BuiltinName::bitwise, + BuiltinRunner::EcOp(_) => BuiltinName::ec_op, + BuiltinRunner::Hash(_) => BuiltinName::pedersen, + BuiltinRunner::RangeCheck(_) => BuiltinName::range_check, + BuiltinRunner::RangeCheck96(_) => BuiltinName::range_check96, + BuiltinRunner::Output(_) => BuiltinName::output, + BuiltinRunner::Keccak(_) => BuiltinName::keccak, + BuiltinRunner::Signature(_) => BuiltinName::ecdsa, + BuiltinRunner::Poseidon(_) => BuiltinName::poseidon, + BuiltinRunner::SegmentArena(_) => BuiltinName::segment_arena, + BuiltinRunner::Mod(b) => b.identifier(), + } + } + pub fn run_security_checks(&self, vm: &VirtualMachine) -> Result<(), VirtualMachineError> { if let BuiltinRunner::Output(_) | BuiltinRunner::SegmentArena(_) = self { return Ok(()); diff --git a/vm/src/vm/runners/builtin_runner/modulo.rs b/vm/src/vm/runners/builtin_runner/modulo.rs index dc4e69f8c1..924aa65a28 100644 --- a/vm/src/vm/runners/builtin_runner/modulo.rs +++ b/vm/src/vm/runners/builtin_runner/modulo.rs @@ -1,6 +1,7 @@ use crate::{ air_private_input::{ModInput, ModInputInstance, ModInputMemoryVars, PrivateInput}, math_utils::{div_mod_unsigned, safe_div_usize}, + serde::deserialize_program::BuiltinName, stdlib::{ borrow::Cow, collections::BTreeMap, @@ -117,6 +118,13 @@ impl ModBuiltinRunner { } } + pub fn identifier(&self) -> BuiltinName { + match self.builtin_type { + ModBuiltinType::Mul => BuiltinName::mul_mod, + ModBuiltinType::Add => BuiltinName::add_mod, + } + } + pub fn initialize_segments(&mut self, segments: &mut MemorySegmentManager) { self.base = segments.add().segment_index as usize; // segments.add() always returns a positive index self.zero_segment_index = segments.add_zero_segment(self.zero_segment_size) @@ -725,7 +733,7 @@ mod tests { runner .end_run(false, false, &mut vm, &mut hint_processor) .unwrap(); - runner.read_return_values(&mut vm).unwrap(); + runner.read_return_values(&mut vm, false).unwrap(); runner.finalize_segments(&mut vm).unwrap(); let air_private_input = runner.get_air_private_input(&vm); diff --git a/vm/src/vm/runners/builtin_runner/output.rs b/vm/src/vm/runners/builtin_runner/output.rs index 393b0b7c1d..c6283d3756 100644 --- a/vm/src/vm/runners/builtin_runner/output.rs +++ b/vm/src/vm/runners/builtin_runner/output.rs @@ -164,10 +164,11 @@ impl OutputBuiltinRunner { Ok(()) } - pub fn get_public_memory(&self) -> Result, RunnerError> { - let size = self - .stop_ptr - .ok_or(RunnerError::NoStopPointer(Box::new(OUTPUT_BUILTIN_NAME)))?; + pub fn get_public_memory( + &self, + segments: &MemorySegmentManager, + ) -> Result, RunnerError> { + let size = self.get_used_cells(segments)?; let mut public_memory: Vec<(usize, usize)> = (0..size).map(|i| (i, 0)).collect(); for (page_id, page) in self.pages.iter() { @@ -590,9 +591,10 @@ mod tests { ) .unwrap(); - builtin.stop_ptr = Some(7); + let mut segments = MemorySegmentManager::new(); + segments.segment_used_sizes = Some(vec![7]); - let public_memory = builtin.get_public_memory().unwrap(); + let public_memory = builtin.get_public_memory(&segments).unwrap(); assert_eq!( public_memory, vec![(0, 0), (1, 0), (2, 1), (3, 1), (4, 2), (5, 2), (6, 2)] diff --git a/vm/src/vm/runners/cairo_runner.rs b/vm/src/vm/runners/cairo_runner.rs index 914f0e32c5..f405e672af 100644 --- a/vm/src/vm/runners/cairo_runner.rs +++ b/vm/src/vm/runners/cairo_runner.rs @@ -525,9 +525,19 @@ impl CairoRunner { vm: &mut VirtualMachine, ) -> Result { let mut stack = Vec::new(); - - for builtin_runner in vm.builtin_runners.iter() { - stack.append(&mut builtin_runner.initial_stack()); + { + let builtin_runners = vm + .builtin_runners + .iter() + .map(|b| (b.identifier(), b)) + .collect::>(); + for builtin_id in &self.program.builtins { + if let Some(builtin_runner) = builtin_runners.get(builtin_id) { + stack.append(&mut builtin_runner.initial_stack()); + } else { + stack.push(Felt252::ZERO.into()) + } + } } if self.is_proof_mode() { @@ -1120,7 +1130,7 @@ impl CairoRunner { .get_used_cells_and_allocated_size(vm) .map_err(RunnerError::FinalizeSegements)?; if let BuiltinRunner::Output(output_builtin) = builtin_runner { - let public_memory = output_builtin.get_public_memory()?; + let public_memory = output_builtin.get_public_memory(&vm.segments)?; vm.segments .finalize(Some(size), builtin_runner.base(), Some(&public_memory)) } else { @@ -1268,14 +1278,33 @@ impl CairoRunner { Ok(()) } - pub fn read_return_values(&mut self, vm: &mut VirtualMachine) -> Result<(), RunnerError> { + pub fn read_return_values( + &mut self, + vm: &mut VirtualMachine, + allow_missing_builtins: bool, + ) -> Result<(), RunnerError> { if !self.run_ended { return Err(RunnerError::ReadReturnValuesNoEndRun); } let mut pointer = vm.get_ap(); - for builtin_runner in vm.builtin_runners.iter_mut().rev() { - let new_pointer = builtin_runner.final_stack(&vm.segments, pointer)?; - pointer = new_pointer; + for builtin_id in self.program.builtins.iter().rev() { + if let Some(builtin_runner) = vm + .builtin_runners + .iter_mut() + .find(|b| b.identifier() == *builtin_id) + { + let new_pointer = builtin_runner.final_stack(&vm.segments, pointer)?; + pointer = new_pointer; + } else { + if !allow_missing_builtins { + return Err(RunnerError::MissingBuiltin(builtin_id.name())); + } + pointer.offset = pointer.offset.saturating_sub(1); + + if !vm.get_integer(pointer)?.is_zero() { + return Err(RunnerError::MissingBuiltinStopPtrNotZero(builtin_id.name())); + } + } } if self.segments_finalized { return Err(RunnerError::FailedAddingReturnValues); @@ -1441,7 +1470,7 @@ impl CairoRunner { layout_name, dyn_layout, &vm.get_public_memory_addresses()?, - vm.get_memory_segment_addresses()?, + self.get_memory_segment_addresses(vm)?, self.relocated_trace .as_ref() .ok_or(PublicInputError::EmptyTrace)?, @@ -1457,6 +1486,41 @@ impl CairoRunner { } AirPrivateInput(private_inputs) } + + pub fn get_memory_segment_addresses( + &self, + vm: &VirtualMachine, + ) -> Result, VirtualMachineError> { + let relocation_table = vm + .relocation_table + .as_ref() + .ok_or(MemoryError::UnrelocatedMemory)?; + + let relocate = |segment: (usize, usize)| -> Result<(usize, usize), VirtualMachineError> { + let (index, stop_ptr_offset) = segment; + let base = relocation_table + .get(index) + .ok_or(VirtualMachineError::RelocationNotFound(index))?; + Ok((*base, base + stop_ptr_offset)) + }; + + vm.builtin_runners + .iter() + .map(|builtin| -> Result<_, VirtualMachineError> { + let (base, stop_ptr) = builtin.get_memory_segment_addresses(); + let stop_ptr = if self.program.builtins.contains(&builtin.identifier()) { + stop_ptr.ok_or_else(|| RunnerError::NoStopPointer(Box::new(builtin.name())))? + } else { + stop_ptr.unwrap_or_default() + }; + + Ok(( + builtin.name().strip_suffix("_builtin").unwrap_or_default(), + relocate((base, stop_ptr))?, + )) + }) + .collect() + } } #[derive(Clone, Debug, Eq, PartialEq)] @@ -3268,6 +3332,7 @@ mod tests { // Swap the first and second builtins (first should be `output`). vm.builtin_runners.swap(0, 1); + cairo_runner.program.builtins.swap(0, 1); cairo_runner.initialize_segments(&mut vm, None); @@ -4671,7 +4736,7 @@ mod tests { let mut vm = vm!(); //Check values written by first call to segments.finalize() - assert_eq!(cairo_runner.read_return_values(&mut vm), Ok(())); + assert_eq!(cairo_runner.read_return_values(&mut vm, false), Ok(())); assert_eq!( cairo_runner .execution_public_memory @@ -4693,7 +4758,7 @@ mod tests { cairo_runner.run_ended = false; let mut vm = vm!(); assert_eq!( - cairo_runner.read_return_values(&mut vm), + cairo_runner.read_return_values(&mut vm, false), Err(RunnerError::ReadReturnValuesNoEndRun) ); } @@ -4712,7 +4777,7 @@ mod tests { cairo_runner.segments_finalized = true; let mut vm = vm!(); assert_eq!( - cairo_runner.read_return_values(&mut vm), + cairo_runner.read_return_values(&mut vm, false), Err(RunnerError::FailedAddingReturnValues) ); } @@ -4740,7 +4805,7 @@ mod tests { vm.set_ap(1); vm.segments.segment_used_sizes = Some(vec![0, 1, 0]); //Check values written by first call to segments.finalize() - assert_eq!(cairo_runner.read_return_values(&mut vm), Ok(())); + assert_eq!(cairo_runner.read_return_values(&mut vm, false), Ok(())); let output_builtin = match &vm.builtin_runners[0] { BuiltinRunner::Output(runner) => runner, _ => unreachable!(), @@ -4771,7 +4836,7 @@ mod tests { vm.set_ap(1); vm.segments.segment_used_sizes = Some(vec![1, 1, 0]); //Check values written by first call to segments.finalize() - assert_eq!(cairo_runner.read_return_values(&mut vm), Ok(())); + assert_eq!(cairo_runner.read_return_values(&mut vm, false), Ok(())); let output_builtin = match &vm.builtin_runners[0] { BuiltinRunner::Output(runner) => runner, _ => unreachable!(), @@ -4809,13 +4874,13 @@ mod tests { // We use 5 as bitwise builtin's segment size as a bitwise instance is 5 cells vm.segments.segment_used_sizes = Some(vec![0, 2, 0, 5]); //Check values written by first call to segments.finalize() - assert_eq!(cairo_runner.read_return_values(&mut vm), Ok(())); + assert_eq!(cairo_runner.read_return_values(&mut vm, false), Ok(())); let output_builtin = match &vm.builtin_runners[0] { BuiltinRunner::Output(runner) => runner, _ => unreachable!(), }; assert_eq!(output_builtin.stop_ptr, Some(0)); - assert_eq!(cairo_runner.read_return_values(&mut vm), Ok(())); + assert_eq!(cairo_runner.read_return_values(&mut vm, false), Ok(())); let bitwise_builtin = match &vm.builtin_runners[1] { BuiltinRunner::Bitwise(runner) => runner, _ => unreachable!(), diff --git a/vm/src/vm/vm_core.rs b/vm/src/vm/vm_core.rs index dc8818e81d..eac5544536 100644 --- a/vm/src/vm/vm_core.rs +++ b/vm/src/vm/vm_core.rs @@ -1064,40 +1064,6 @@ impl VirtualMachine { } } - pub fn get_memory_segment_addresses( - &self, - ) -> Result, VirtualMachineError> { - let relocation_table = self - .relocation_table - .as_ref() - .ok_or(MemoryError::UnrelocatedMemory)?; - - let relocate = |segment: (usize, usize)| -> Result<(usize, usize), VirtualMachineError> { - let (index, stop_ptr_offset) = segment; - let base = relocation_table - .get(index) - .ok_or(VirtualMachineError::RelocationNotFound(index))?; - Ok((*base, base + stop_ptr_offset)) - }; - - self.builtin_runners - .iter() - .map(|builtin| -> Result<_, VirtualMachineError> { - let addresses = - if let (base, Some(stop_ptr)) = builtin.get_memory_segment_addresses() { - (base, stop_ptr) - } else { - return Err(RunnerError::NoStopPointer(Box::new(builtin.name())).into()); - }; - - Ok(( - builtin.name().strip_suffix("_builtin").unwrap_or_default(), - relocate(addresses)?, - )) - }) - .collect() - } - #[doc(hidden)] pub fn builtins_final_stack_from_stack_pointer_dict( &mut self,