Skip to content

Commit

Permalink
fix: Use program builtins in initialize_main_entrypoint & `read_ret…
Browse files Browse the repository at this point in the history
…urn_values` (#1703)

* Ignore pesky pie zip files

* Add zero segment

* Handle zero segment when counting memory holes

* Add Changelog entry

* Fix macro

* First draft of runner & instance_def

* Complete struct

* Impl initialize_segments + read_n_words_value

* Implement read_inputs

* Implement read_memory_vars

* Impl fill_inputs

* Impl fill_offsets

* Impl write_n_words_value

* Implement fill_value

* Implement fill_memory

* Integrate mod builtin under feature flag

* Add test file

* Impl hint

* Push hint impl file

* Add quick impl of run_additional_security_checks

* Some fixes

* Fixes

* Fix bugs

* Temp testing code

* Refactor: Use a struct to represent inputs

* Refactor: use a struct to represent memory vars

* Refactor: Use a constant N_WORDS

* Refactor: use fixed N_WORDS size arrays to ensure safe indexing

* Store prime as NonZeroFelt

* Refactor: Used a fixed-size array for shift_powers

* Clippy

* Clippy

* Small improvments

* Refactor: Use only offsets_ptr instead of all inputs in fill_offsets

* Refactor(Simplification): Reduce the return type of read_memory_vars to only a b & c as the other values are not used

* Fix(Refactor: Use BigUint to represent values as we have to accomodate values up to U384

* Refactor: Optimize fill_offsets loop so clippy does not complain about it

* Add a fill_memory wrapper on the vm to avoid cloning the builtin runners

* Fixes

* Check batch size

* Refactors: Reduce duplication in mod_builtin_fill_memory & combine loops in fill_value

* Safety: Use saturating sub

* Refactor: Avoid creating empty structs in fill_memory

* Add integration test

* Add error handling + integration tests

* Fix bugged shift_powers creation

* Improve error formatting

* Simplify error string generation

* Fix test

* Add test using large batch size + hint impl for circuits using large batch size

* Add test with large batch size

* Fix hint impl + rename module

* Improve error handling of run_additional_security_checks

* Remove unused method BuiltinRunner::get_memory_accesses

* Impl mark_accessed_memory

* Mark cells created by fill_memory as accessed during execution

* Remove mark_accessed_memory

* Finalize zero segment

* WIP: Move final_stack to enum impl

* Remove unused methods from builtin runners

* Remove commented code

* Get rid of empty impls in builtin runners

* Move get_memory_segment_addresses impl to enum impl

* Impl builtin methods

* Add placeholder value

* Fix cells_per_instance

* Remove temp testing code

* Run modulo builtin security checks when running builtin security checks

* Use realistic value for mod builtin ratio

* Draft(private inputs)

* Impl air_private_input

* Update private input structure

* Improve struct compatibility for serialization

* Relocate zero_segment_address + Use BtreeMap keep batches sorted

* Fix field names

* Clippy

* Remove unused attribute from modulo module + limit feature gated code

* Fix test files EOF

* Relocate ptrs when creating mod private input

* Remove feature

* Update requirements.txt

* Update program + add changelog entry

* Impl new hints

* Adjust recursive_large_output params

* Update tests

* Remove allow(unused)

* Push bench file

* Add non-std imports

* Remove unused func

* Remove dead code

* Box error contents to reduce errir size

* Avoid needless to_string when reporting errors

* Copy common lib modules so that we can run mod builtin tests in CI

* Add mod_builtin to special features so we can run the tests in CI

* Fix file

* Fix get_used_instances for segment arena

* Fix test

* Add temp fix

* Clippy

* fmt

* Add no-std import

* Add no-std import

* Push changelog

* Push deleted comment

* fmt

* Fix typo

* Compile with proof mode on CI + add air_private_input tests

* Run proof mode integration tests

* Push symlinks for convenience as these will be moved soon

* Fix test

* Fix test value

* Fix test value

* Fix

* Use rea data in layoyts + adjust tests accordingly

* Update air priv input tests value (Accounting for increased ratios in layout)

* Remove feature mod_builtin

* Revert "Remove feature mod_builtin"

This reverts commit 5cc921c.

* Remove dbg print

* Fix read_return_values

* Fix initialize_main_entrypoint & read_return_values

* Add test

* Fix OutputBuiltinRunner::get_public_memory

* Fix air public input generation

* Fix tests

* Fix EOF

* Add checks

* Fix tests

* Fix order

* Add CHANGELOG entry

* Fix

* Re,ove unused import

* Fix

* Update CHANGELOG.md

* Fix test

* fmt

* Fix
  • Loading branch information
fmoletta authored Apr 10, 2024
1 parent ca3b8ab commit 6de8f56
Show file tree
Hide file tree
Showing 11 changed files with 188 additions and 76 deletions.
8 changes: 8 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down
4 changes: 0 additions & 4 deletions cairo-vm-cli/src/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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()]);
}
Expand Down
11 changes: 11 additions & 0 deletions cairo_programs/pedersen_extra_builtins.cairo
Original file line number Diff line number Diff line change
@@ -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 ();
}
15 changes: 7 additions & 8 deletions vm/src/cairo_run.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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)?;
}
Expand Down Expand Up @@ -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,
Expand All @@ -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
Expand All @@ -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)?;
}
Expand Down
50 changes: 43 additions & 7 deletions vm/src/tests/cairo_run_test.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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() {
Expand Down Expand Up @@ -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();
}
4 changes: 4 additions & 0 deletions vm/src/vm/errors/runner_errors.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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)]
Expand Down
17 changes: 17 additions & 0 deletions vm/src/vm/runners/builtin_runner/mod.rs
Original file line number Diff line number Diff line change
@@ -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};
Expand Down Expand Up @@ -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(());
Expand Down
10 changes: 9 additions & 1 deletion vm/src/vm/runners/builtin_runner/modulo.rs
Original file line number Diff line number Diff line change
@@ -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,
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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);
Expand Down
14 changes: 8 additions & 6 deletions vm/src/vm/runners/builtin_runner/output.rs
Original file line number Diff line number Diff line change
Expand Up @@ -164,10 +164,11 @@ impl OutputBuiltinRunner {
Ok(())
}

pub fn get_public_memory(&self) -> Result<Vec<(usize, usize)>, RunnerError> {
let size = self
.stop_ptr
.ok_or(RunnerError::NoStopPointer(Box::new(OUTPUT_BUILTIN_NAME)))?;
pub fn get_public_memory(
&self,
segments: &MemorySegmentManager,
) -> Result<Vec<(usize, usize)>, 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() {
Expand Down Expand Up @@ -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)]
Expand Down
Loading

0 comments on commit 6de8f56

Please sign in to comment.