Skip to content

Commit

Permalink
Merge branch 'main' into load-args-externally
Browse files Browse the repository at this point in the history
  • Loading branch information
fmoletta authored May 15, 2024
2 parents 69c20e9 + aecbb3f commit 6400d09
Show file tree
Hide file tree
Showing 6 changed files with 65 additions and 35 deletions.
4 changes: 4 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,10 @@

* feat: Load arguments into VM instead of creating them via instructions in cairo1-run [#1759](https://github.com/lambdaclass/cairo-vm/pull/1759)

#### [1.0.0-rc3] - 2024-05-14

* bugfix: Fix handling of return values wrapped in `PanicResult` in cairo1-run crate [#1763](https://github.com/lambdaclass/cairo-vm/pull/1763)

* refactor(BREAKING): Move the VM back to the CairoRunner [#1743](https://github.com/lambdaclass/cairo-vm/pull/1743)
* `CairoRunner` has a new public field `vm: VirtualMachine`
* `CairoRunner` no longer derives `Debug`
Expand Down
14 changes: 7 additions & 7 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

6 changes: 3 additions & 3 deletions Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -20,16 +20,16 @@ exclude = ["ensure-no_std"]
resolver = "2"

[workspace.package]
version = "1.0.0-rc2"
version = "1.0.0-rc3"
edition = "2021"
license = "Apache-2.0"
repository = "https://github.com/lambdaclass/cairo-vm/"
readme = "README.md"
keywords = ["starknet", "cairo", "vm", "wasm", "no_std"]

[workspace.dependencies]
cairo-vm = { path = "./vm", version = "1.0.0-rc2", default-features = false }
cairo-vm-tracer = { path = "./cairo-vm-tracer", version = "1.0.0-rc2", default-features = false }
cairo-vm = { path = "./vm", version = "1.0.0-rc3", default-features = false }
cairo-vm-tracer = { path = "./cairo-vm-tracer", version = "1.0.0-rc3", default-features = false }
mimalloc = { version = "0.1.37", default-features = false }
num-bigint = { version = "0.4", default-features = false, features = [
"serde",
Expand Down
2 changes: 1 addition & 1 deletion RELEASE.md
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@
- The versions must be the same.
- You need to update the workspace dependency `cairo-vm`, which
you can find in the root cargo manifest under the section `[workspace.dependencies]`.
- [Here](https://github.com/lambdaclass/cairo-rs/pull/1301/files) is an
- [Here](https://github.com/lambdaclass/cairo-vm/pull/1748/files) is an
example pull request with these changes.
- [ ] Run `cargo update` and `git add Cargo.lock`
- [ ] Update `CHANGELOG.md`:
Expand Down
68 changes: 46 additions & 22 deletions cairo1-run/src/cairo_run.rs
Original file line number Diff line number Diff line change
Expand Up @@ -225,10 +225,13 @@ pub fn cairo_run_program(
runner.end_run(false, false, &mut hint_processor)?;

let skip_output = cairo_run_config.proof_mode || cairo_run_config.append_return_values;

let result_inner_type_size =
result_inner_type_size(return_type_id, &sierra_program_registry, &type_sizes);
// Fetch return values
let return_values = fetch_return_values(
return_type_size,
return_type_id,
result_inner_type_size,
&runner.vm,
builtin_count,
skip_output,
Expand Down Expand Up @@ -735,9 +738,38 @@ fn get_function_builtins(
(builtins, builtin_offset)
}

// Returns the size of the T type in PanicResult::Ok(T) if applicable
// Returns None if the return_type_id is not a PanicResult
fn result_inner_type_size(
return_type_id: Option<&ConcreteTypeId>,
sierra_program_registry: &ProgramRegistry<CoreType, CoreLibfunc>,
type_sizes: &UnorderedHashMap<ConcreteTypeId, i16>,
) -> Option<i16> {
if return_type_id
.and_then(|id| {
id.debug_name
.as_ref()
.map(|name| name.starts_with("core::panics::PanicResult::"))
})
.unwrap_or_default()
{
let return_type_info =
get_info(sierra_program_registry, return_type_id.as_ref().unwrap()).unwrap();
// We already know info.long_id.generic_args[0] contains the Panic variant
let inner_args = &return_type_info.long_id.generic_args[1];
let inner_type = match inner_args {
GenericArg::Type(type_id) => type_id,
_ => unreachable!(),
};
type_sizes.get(inner_type).copied()
} else {
None
}
}

fn fetch_return_values(
return_type_size: i16,
return_type_id: Option<&ConcreteTypeId>,
result_inner_type_size: Option<i16>,
vm: &VirtualMachine,
builtin_count: i16,
fetch_from_output: bool,
Expand All @@ -754,15 +786,8 @@ fn fetch_return_values(
return_type_size as usize,
)?
};
// Check if this result is a Panic result
if return_type_id
.and_then(|id| {
id.debug_name
.as_ref()
.map(|name| name.starts_with("core::panics::PanicResult::"))
})
.unwrap_or_default()
{
// Handle PanicResult (we already checked if the type is a PanicResult when fetching the inner type size)
if let Some(inner_type_size) = result_inner_type_size {
// Check the failure flag (aka first return value)
if return_values.first() != Some(&MaybeRelocatable::from(0)) {
// In case of failure, extract the error from the return values (aka last two values)
Expand All @@ -784,10 +809,11 @@ fn fetch_return_values(
panic_data.iter().map(|c| *c.as_ref()).collect(),
));
} else {
if return_values.len() < 3 {
if return_values.len() < inner_type_size as usize {
return Err(Error::FailedToExtractReturnValues);
}
return_values = return_values[2..].to_vec()
return_values =
return_values[((return_type_size - inner_type_size).into_or_panic())..].to_vec()
}
}
Ok(return_values)
Expand Down Expand Up @@ -881,15 +907,13 @@ fn serialize_output_inner<'a>(
.expect("Missing return value")
.get_relocatable()
.expect("Array start_ptr not Relocatable");
// Arrays can come in two formats: either [start_ptr, end_ptr] or [end_ptr], with the start_ptr being implicit (base of the end_ptr's segment)
let (array_start, array_size ) = match return_values_iter.peek().and_then(|mr| mr.get_relocatable()) {
Some(array_end) if array_end.segment_index == array_start.segment_index && array_end.offset >= array_start.offset => {
// Pop the value we just peeked
return_values_iter.next();
(array_start, (array_end - array_start).unwrap())
}
_ => ((array_start.segment_index, 0).into(), array_start.offset),
};
let array_end = return_values_iter
.next()
.expect("Missing return value")
.get_relocatable()
.expect("Array end_ptr not Relocatable");
let array_size = (array_end - array_start).unwrap();

let array_data = vm.get_continuous_range(array_start, array_size).unwrap();
let mut array_data_iter = array_data.iter().peekable();
let array_elem_id = &info.ty;
Expand Down
6 changes: 4 additions & 2 deletions vm/src/vm/errors/cairo_run_errors.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,9 @@ use crate::types::errors::program_errors::ProgramError;
use crate::vm::errors::{
runner_errors::RunnerError, trace_errors::TraceError, vm_errors::VirtualMachineError,
};

// In case you need to add a CairoRunError enum variant
// Add it with #[error(transparent)]
// If not it can cause some performance regressions, like in https://github.com/lambdaclass/cairo-vm/pull/1720
#[derive(Debug, Error)]
pub enum CairoRunError {
#[error(transparent)]
Expand All @@ -22,6 +24,6 @@ pub enum CairoRunError {
MemoryError(#[from] MemoryError),
#[error(transparent)]
VmException(#[from] VmException),
#[error("Cairo Pie validation failed: {0}")]
#[error(transparent)]
CairoPieValidation(#[from] CairoPieValidationError),
}

0 comments on commit 6400d09

Please sign in to comment.