Skip to content

Commit

Permalink
Merge branch 'main' into perf/compact_memory_cell_main
Browse files Browse the repository at this point in the history
  • Loading branch information
fmoletta authored Apr 30, 2024
2 parents d33a3c7 + 0df3f34 commit b304f9a
Show file tree
Hide file tree
Showing 19 changed files with 297 additions and 182 deletions.
40 changes: 40 additions & 0 deletions .github/workflows/rust.yml
Original file line number Diff line number Diff line change
Expand Up @@ -724,3 +724,43 @@ jobs:

- name: Run script
run: ./vm/src/tests/compare_factorial_outputs_all_layouts.sh

compare-run-from-cairo-pie-all-outputs:
name: Compare all outputs from running Cairo PIEs
needs: [ build-programs, build-release, run-cairo-release ]
runs-on: ubuntu-22.04
steps:
- name: Checkout
uses: actions/checkout@v3

- name: Python3 Build
uses: actions/setup-python@v4
with:
python-version: '3.9'
cache: 'pip'

- name: Install cairo-lang and deps
run: pip install -r requirements.txt

- name: Fetch release binary
uses: actions/cache/restore@v3
with:
key: cli-bin-rel-${{ github.sha }}
path: target/release/cairo-vm-cli
fail-on-cache-miss: true

- name: Fetch traces for cairo-vm
uses: actions/cache/restore@v3
with:
path: |
cairo_programs/**/*.memory
cairo_programs/**/*.trace
cairo_programs/**/*.air_public_input
cairo_programs/**/*.air_private_input
cairo_programs/**/*.pie.zip
key: cairo_test_programs-release-trace-cache-${{ github.sha }}
fail-on-cache-miss: true

- name: Run comparison
run: ./vm/src/tests/compare_all_pie_outputs.sh

1 change: 1 addition & 0 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@
**/*.air_public_input
**/*.air_private_input
**/*.pie.zip
**/*.pie
**/*.swp
bench/results
.python-version
Expand Down
4 changes: 4 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,8 @@
* perf: use a more compact representation for `MemoryCell` [#1672](https://github.com/lambdaclass/cairo-vm/pull/1672)
* BREAKING: `Memory::get_value` will now always return `Cow::Owned` variants, code that relied on `Cow::Borrowed` may break

* feat: Add `--run_from_cairo_pie` to `cairo-vm-cli` + workflow [#1730](https://github.com/lambdaclass/cairo-vm/pull/1730)

* Serialize directly into writer in `CairoPie::write_zip_file`[#1736](https://github.com/lambdaclass/cairo-vm/pull/1736)

* feat: Add support for cairo1 run with segements arena validation.
Expand All @@ -26,6 +28,8 @@
* Add `CairoPie` methods `run_validity_checks` & `check_pie_compatibility`
* Add `Program` method `from_stripped_program`

* bugfix: Don't assume outer deref when fetching integer values from references[#1732](https://github.com/lambdaclass/cairo-vm/pull/1732)

* feat: Implement `extend_additional_data` for `BuiltinRunner`[#1726](https://github.com/lambdaclass/cairo-vm/pull/1726)

* BREAKING: Set dynamic params as null by default on air public input [#1716](https://github.com/lambdaclass/cairo-vm/pull/1716)
Expand Down
4 changes: 4 additions & 0 deletions Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -329,6 +329,9 @@ compare_air_private_input: $(CAIRO_RS_AIR_PRIVATE_INPUT) $(CAIRO_AIR_PRIVATE_INP
compare_pie: $(CAIRO_RS_PIE) $(CAIRO_PIE)
cd vm/src/tests; ./compare_vm_state.sh pie

compare_all_pie_outputs: $(CAIRO_RS_PIE)
cd vm/src/tests; ./compare_all_pie_outputs.sh

# Run with nightly enable the `doc_cfg` feature wich let us provide clear explaination about which parts of the code are behind a feature flag
docs:
RUSTDOCFLAGS="--cfg docsrs" cargo +nightly doc --verbose --release --locked --no-deps --all-features --open
Expand All @@ -338,6 +341,7 @@ clean:
rm -f $(TEST_DIR)/*.memory
rm -f $(TEST_DIR)/*.trace
rm -f $(TEST_DIR)/*.pie.zip
rm -f $(TEST_DIR)/*.pie
rm -f $(BENCH_DIR)/*.json
rm -f $(BAD_TEST_DIR)/*.json
rm -f $(PRINT_TEST_DIR)/*.json
Expand Down
4 changes: 3 additions & 1 deletion README.md
Original file line number Diff line number Diff line change
Expand Up @@ -174,10 +174,12 @@ The cairo-vm-cli supports the following optional arguments:

- `--air_private_input <AIR_PRIVATE_INPUT>`: Receives the name of a file and outputs the AIR private inputs into it. Can only be used if proof_mode, trace_file & memory_file are also enabled.

- `--cairo_pie_output <CAIRO_PIE_OUTPUT>`: Receives the name of a file and outputs the Cairo PIE into it. Can only be used if proof_mode, is not enabled.
- `--cairo_pie_output <CAIRO_PIE_OUTPUT>`: Receives the name of a file and outputs the Cairo PIE into it. Can only be used if proof_mode is not enabled.

- `--allow_missing_builtins`: Disables the check that all builtins used by the program need to be included in the selected layout. Enabled by default when in proof_mode.

- `run_from_cairo_pie`: Runs a Cairo PIE instead of a compiled json file. The name of the file will be the first argument received by the CLI (as if it were to run a normal compiled program). Can only be used if proof_mode is not enabled.

For example, to obtain the air public inputs from a fibonacci program run, we can run :

```bash
Expand Down
41 changes: 30 additions & 11 deletions cairo-vm-cli/src/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -10,8 +10,10 @@ use cairo_vm::types::layout_name::LayoutName;
use cairo_vm::vm::errors::cairo_run_errors::CairoRunError;
use cairo_vm::vm::errors::trace_errors::TraceError;
use cairo_vm::vm::errors::vm_errors::VirtualMachineError;
use cairo_vm::vm::runners::cairo_pie::CairoPie;
#[cfg(feature = "with_tracer")]
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")]
Expand Down Expand Up @@ -68,6 +70,13 @@ struct Args {
#[structopt(long = "tracer")]
#[cfg(feature = "with_tracer")]
tracer: bool,
#[structopt(
long = "run_from_cairo_pie",
// We need to add these air_private_input & air_public_input or else
// passing run_from_cairo_pie + either of these without proof_mode will not fail
conflicts_with_all = ["proof_mode", "air_private_input", "air_public_input"]
)]
run_from_cairo_pie: bool,
}

#[derive(Debug, Error)]
Expand Down Expand Up @@ -153,7 +162,7 @@ fn run(args: impl Iterator<Item = String>) -> Result<(), Error> {
let args = Args::try_parse_from(args)?;

let trace_enabled = args.trace_file.is_some() || args.air_public_input.is_some();
let mut hint_executor = BuiltinHintProcessor::new_empty();

let cairo_run_config = cairo_run::CairoRunConfig {
entrypoint: &args.entrypoint,
trace_enabled,
Expand All @@ -165,16 +174,26 @@ fn run(args: impl Iterator<Item = String>) -> Result<(), Error> {
..Default::default()
};

let program_content = std::fs::read(args.filename).map_err(Error::IO)?;

let (cairo_runner, mut vm) =
match cairo_run::cairo_run(&program_content, &cairo_run_config, &mut hint_executor) {
Ok(runner) => runner,
Err(error) => {
eprintln!("{error}");
return Err(Error::Runner(error));
}
};
let (cairo_runner, mut vm) = match {
if args.run_from_cairo_pie {
let pie = CairoPie::read_zip_file(&args.filename)?;
let mut hint_processor = BuiltinHintProcessor::new(
Default::default(),
RunResources::new(pie.execution_resources.n_steps),
);
cairo_run::cairo_run_pie(&pie, &cairo_run_config, &mut hint_processor)
} else {
let program_content = std::fs::read(args.filename).map_err(Error::IO)?;
let mut hint_processor = BuiltinHintProcessor::new_empty();
cairo_run::cairo_run(&program_content, &cairo_run_config, &mut hint_processor)
}
} {
Ok(runner) => runner,
Err(error) => {
eprintln!("{error}");
return Err(Error::Runner(error));
}
};

if args.print_output {
let mut output_buffer = "Program Output:\n".to_string();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -367,7 +367,7 @@ mod tests {
assert_matches!(
run_hint!(vm, ids_data, hint_code),
Err(HintError::IdentifierNotRelocatable(bx))
if *bx == ("output".to_string(), (1,0).into())
if bx.as_ref() == "output"
);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1003,7 +1003,7 @@ mod tests {
assert_matches!(
run_hint!(vm, ids_data, hint_code),
Err(HintError::IdentifierNotInteger(bx))
if *bx == ("len".to_string(), (1,1).into())
if bx.as_ref() == "len"
);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -270,7 +270,7 @@ mod tests {
let ids_data = ids_data!["array_ptr", "elm_size", "n_elms", "index", "key"];
assert_matches!(
run_hint!(vm, ids_data, hint_code::FIND_ELEMENT),
Err(HintError::IdentifierNotInteger(bx)) if *bx == ("key".to_string(), (1,4).into())
Err(HintError::UnknownIdentifier(bx)) if bx.as_ref() == "key"
);
}

Expand All @@ -283,7 +283,7 @@ mod tests {
)]));
assert_matches!(
run_hint!(vm, ids_data, hint_code::FIND_ELEMENT),
Err(HintError::IdentifierNotInteger(bx)) if *bx == ("elm_size".to_string(), (1,1).into())
Err(HintError::IdentifierNotInteger(bx)) if bx.as_ref() == "elm_size"
);
}

Expand Down Expand Up @@ -321,7 +321,7 @@ mod tests {
init_vm_ids_data(HashMap::from([("n_elms".to_string(), relocatable)]));
assert_matches!(
run_hint!(vm, ids_data, hint_code::FIND_ELEMENT),
Err(HintError::IdentifierNotInteger(bx)) if *bx == ("n_elms".to_string(), (1,2).into())
Err(HintError::IdentifierNotInteger(bx)) if bx.as_ref() == "n_elms"
);
}

Expand Down Expand Up @@ -364,7 +364,7 @@ mod tests {
init_vm_ids_data(HashMap::from([("key".to_string(), relocatable)]));
assert_matches!(
run_hint!(vm, ids_data, hint_code::FIND_ELEMENT),
Err(HintError::IdentifierNotInteger(bx)) if *bx == ("key".to_string(), (1,4).into())
Err(HintError::IdentifierNotInteger(bx)) if bx.as_ref() == "key"
);
}

Expand Down Expand Up @@ -403,7 +403,7 @@ mod tests {
)]));
assert_matches!(
run_hint!(vm, ids_data, hint_code::SEARCH_SORTED_LOWER),
Err(HintError::IdentifierNotInteger(bx)) if *bx == ("elm_size".to_string(), (1,1).into())
Err(HintError::IdentifierNotInteger(bx)) if bx.as_ref() == "elm_size"
);
}

Expand All @@ -429,7 +429,7 @@ mod tests {
)]));
assert_matches!(
run_hint!(vm, ids_data, hint_code::SEARCH_SORTED_LOWER),
Err(HintError::IdentifierNotInteger(bx)) if *bx == ("n_elms".to_string(), (1,2).into())
Err(HintError::IdentifierNotInteger(bx)) if bx.as_ref() == "n_elms"
);
}

Expand Down
29 changes: 12 additions & 17 deletions vm/src/hint_processor/builtin_hint_processor/hint_utils.rs
Original file line number Diff line number Diff line change
Expand Up @@ -48,12 +48,10 @@ pub fn get_ptr_from_var_name(
match get_ptr_from_reference(vm, reference, ap_tracking) {
// Map internal errors into more descriptive variants
Ok(val) => Ok(val),
Err(HintError::WrongIdentifierTypeInternal(var_addr)) => Err(
HintError::IdentifierNotRelocatable(Box::new((var_name.to_string(), *var_addr))),
),
_ => Err(HintError::UnknownIdentifier(
var_name.to_string().into_boxed_str(),
Err(HintError::WrongIdentifierTypeInternal) => Err(HintError::IdentifierNotRelocatable(
Box::<str>::from(var_name),
)),
_ => Err(HintError::UnknownIdentifier(Box::<str>::from(var_name))),
}
}

Expand All @@ -77,7 +75,7 @@ pub fn get_relocatable_from_var_name(
ids_data
.get(var_name)
.and_then(|x| compute_addr_from_reference(x, vm, ap_tracking))
.ok_or_else(|| HintError::UnknownIdentifier(var_name.to_string().into_boxed_str()))
.ok_or_else(|| HintError::UnknownIdentifier(Box::<str>::from(var_name)))
}

//Gets the value of a variable name.
Expand All @@ -93,12 +91,10 @@ pub fn get_integer_from_var_name(
match get_integer_from_reference(vm, reference, ap_tracking) {
// Map internal errors into more descriptive variants
Ok(val) => Ok(val),
Err(HintError::WrongIdentifierTypeInternal(var_addr)) => Err(
HintError::IdentifierNotInteger(Box::new((var_name.to_string(), *var_addr))),
),
_ => Err(HintError::UnknownIdentifier(
var_name.to_string().into_boxed_str(),
)),
Err(HintError::WrongIdentifierTypeInternal) => {
Err(HintError::IdentifierNotInteger(Box::<str>::from(var_name)))
}
_ => Err(HintError::UnknownIdentifier(Box::<str>::from(var_name))),
}
}

Expand All @@ -111,7 +107,7 @@ pub fn get_maybe_relocatable_from_var_name<'a>(
) -> Result<MaybeRelocatable, HintError> {
let reference = get_reference_from_var_name(var_name, ids_data)?;
get_maybe_relocatable_from_reference(vm, reference, ap_tracking)
.ok_or_else(|| HintError::UnknownIdentifier(var_name.to_string().into_boxed_str()))
.ok_or_else(|| HintError::UnknownIdentifier(Box::<str>::from(var_name)))
}

pub fn get_reference_from_var_name<'a>(
Expand All @@ -120,7 +116,7 @@ pub fn get_reference_from_var_name<'a>(
) -> Result<&'a HintReference, HintError> {
ids_data
.get(var_name)
.ok_or_else(|| HintError::UnknownIdentifier(var_name.to_string().into_boxed_str()))
.ok_or_else(|| HintError::UnknownIdentifier(Box::<str>::from(var_name)))
}

pub fn get_constant_from_var_name<'a>(
Expand All @@ -137,7 +133,6 @@ pub fn get_constant_from_var_name<'a>(
#[cfg(test)]
mod tests {
use super::*;
use crate::stdlib::string::ToString;

use crate::{
hint_processor::hint_processor_definition::HintReference,
Expand Down Expand Up @@ -218,7 +213,7 @@ mod tests {

assert_matches!(
get_ptr_from_var_name("value", &vm, &ids_data, &ApTracking::new()),
Err(HintError::IdentifierNotRelocatable(bx)) if *bx == ("value".to_string(), (1,0).into())
Err(HintError::IdentifierNotRelocatable(bx)) if bx.as_ref() == "value"
);
}

Expand Down Expand Up @@ -274,7 +269,7 @@ mod tests {

assert_matches!(
get_integer_from_var_name("value", &vm, &ids_data, &ApTracking::new()),
Err(HintError::IdentifierNotInteger(bx)) if *bx == ("value".to_string(), (1,0).into())
Err(HintError::IdentifierNotInteger(bx)) if bx.as_ref() == "value"
);
}
}
Loading

0 comments on commit b304f9a

Please sign in to comment.