Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Cairo1] Fix GasBuiltin handling #1789

Merged
merged 9 commits into from
Jun 12, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,10 @@

#### Upcoming Changes

* fix: Handle `GasBuiltin` in cairo1-run crate [#1789](https://github.com/lambdaclass/cairo-vm/pull/1789)
* Load `initial_gas` into vm instead of creating it via instructions.
* Fix bug affecting programs with input arguments and gas builtin.

* fix: Change (de)serialization of CairoPie's `OutputBuiltinAdditionalData`'s `PublicMemoryPage` to vectors of length 2. [#1781](https://github.com/lambdaclass/cairo-vm/pull/1781)

* fix: Fixed deserialization issue when signature additional data is empty, and the name of the builtin range_check96 [#1785](https://github.com/lambdaclass/cairo-vm/pull/1785)
Expand Down
52 changes: 37 additions & 15 deletions cairo1-run/src/cairo_run.rs
Original file line number Diff line number Diff line change
Expand Up @@ -176,7 +176,6 @@
&casm_program,
&type_sizes,
main_func,
initial_gas,
&cairo_run_config,
)?;

Expand Down Expand Up @@ -254,7 +253,7 @@
cairo_run_config.trace_enabled,
)?;
let end = runner.initialize(cairo_run_config.proof_mode)?;
load_arguments(&mut runner, &cairo_run_config, main_func)?;
load_arguments(&mut runner, &cairo_run_config, main_func, initial_gas)?;

// Run it until the end / infinite loop in proof_mode
runner.run_until_pc(end, &mut hint_processor)?;
Expand Down Expand Up @@ -394,7 +393,7 @@
}

// Loads the input arguments into the execution segment, leaving the necessary gaps for the values that will be written by
// the instructions in the entry_code (produced by `create_entry_code`)
// the instructions in the entry_code (produced by `create_entry_code`). Also loads the initial gas if the GasBuiltin is present

/* Example of execution segment before running the main function:
Before calling this function (after runner.initialize):
Expand Down Expand Up @@ -441,19 +440,27 @@
builtin_base_0
builtin_base_1
(*2) segment_arena_ptr + 3 (segment_arena base)
(*4) initial_gas
(*3) arg_0
(*3) arg_1
]
(*1) if output builtin is added (if either proof_mode or append_return_values is enabled)
(*2) if segment arena is present
(*3) if args are used
(*4) if gas builtin is present
*/
fn load_arguments(
runner: &mut CairoRunner,
cairo_run_config: &Cairo1RunConfig,
main_func: &Function,
initial_gas: usize,
) -> Result<(), Error> {
if cairo_run_config.args.is_empty() {
let got_gas_builtin = main_func
.signature
.param_types
.iter()
.any(|ty| ty.debug_name.as_ref().is_some_and(|n| n == "GasBuiltin"));
if cairo_run_config.args.is_empty() && !got_gas_builtin {
// Nothing to be done
return Ok(());
}
Expand All @@ -470,12 +477,21 @@
// * segment_arena_ptr
// * info_segment_ptr
// * 0
// * segment_arena_ptr + 3
let mut ap_offset = runner.get_program().builtins_len();
if cairo_run_config.copy_to_output() {
ap_offset += runner.get_program().builtins_len() - 1;
}
if got_segment_arena {
ap_offset += 3;
ap_offset += 4;
}
// Load initial gas if GasBuiltin is present
if got_gas_builtin {
runner.vm.insert_value(
(runner.vm.get_ap() + ap_offset).map_err(VirtualMachineError::Math)?,
Felt252::from(initial_gas),
)?;

Check warning on line 493 in cairo1-run/src/cairo_run.rs

View check run for this annotation

Codecov / codecov/patch

cairo1-run/src/cairo_run.rs#L493

Added line #L493 was not covered by tests
ap_offset += 1;
}
for arg in cairo_run_config.args {
match arg {
Expand Down Expand Up @@ -516,11 +532,20 @@
casm_program: &CairoProgram,
type_sizes: &UnorderedHashMap<ConcreteTypeId, i16>,
func: &Function,
initial_gas: usize,
config: &Cairo1RunConfig,
) -> Result<(CasmContext, Vec<BuiltinName>), Error> {
let copy_to_output_builtin = config.copy_to_output();
let signature = &func.signature;
let got_segment_arena = signature.param_types.iter().any(|ty| {
get_info(sierra_program_registry, ty)
.map(|x| x.long_id.generic_id == SegmentArenaType::ID)
.unwrap_or_default()
});
let got_gas_builtin = signature.param_types.iter().any(|ty| {
get_info(sierra_program_registry, ty)
.map(|x| x.long_id.generic_id == GasBuiltinType::ID)
.unwrap_or_default()
});
// The builtins in the formatting expected by the runner.
let (builtins, builtin_offset) =
get_function_builtins(&signature.param_types, copy_to_output_builtin);
Expand All @@ -538,11 +563,6 @@
let offset: i16 = 2 + builtins.len().into_or_panic::<i16>();
ctx.add_var(CellExpression::Deref(deref!([fp - offset])))
});
let got_segment_arena = signature.param_types.iter().any(|ty| {
get_info(sierra_program_registry, ty)
.map(|x| x.long_id.generic_id == SegmentArenaType::ID)
.unwrap_or_default()
});
if copy_to_output_builtin {
// Leave a gap to write the builtin final pointers
// We write them on a fixed cells relative to the starting FP pointer so we don't lose them after serializing outputs
Expand Down Expand Up @@ -583,9 +603,9 @@
ap += 1;
};
} else if generic_ty == &GasBuiltinType::ID {
// We already loaded the inital gas so we just advance AP
casm_build_extend! {ctx,
const initial_gas = initial_gas;
tempvar _gas = initial_gas;
ap += 1;
};
} else {
let ty_size = type_sizes[ty];
Expand Down Expand Up @@ -703,8 +723,10 @@
// We lost the output_ptr var after re-scoping, so we need to create it again
// The last instruction will write the last output ptr so we can find it in [ap - 1]
let output_ptr = ctx.add_var(CellExpression::Deref(deref!([ap - 1])));
// len(builtins - output) + len(builtins) + if segment_arena: segment_arena_ptr + info_ptr + 0 + (segment_arena_ptr + 3)
let offset = (2 * builtins.len() - 1 + 4 * got_segment_arena as usize) as i16;
// len(builtins - output) + len(builtins) + if segment_arena: segment_arena_ptr + info_ptr + 0 + (segment_arena_ptr + 3) + (gas_builtin)
let offset = (2 * builtins.len() - 1
+ 4 * got_segment_arena as usize
+ got_gas_builtin as usize) as i16;
let array_start_ptr = ctx.add_var(CellExpression::Deref(deref!([fp + offset])));
let array_end_ptr = ctx.add_var(CellExpression::Deref(deref!([fp + offset + 1])));
casm_build_extend! {ctx,
Expand Down
7 changes: 7 additions & 0 deletions cairo1-run/src/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -414,6 +414,13 @@ mod tests {
#[case("bitwise.cairo", "11772", "[11772]", None, None)]
#[case("factorial.cairo", "3628800", "[3628800]", None, None)]
#[case("fibonacci.cairo", "89", "[89]", None, None)]
#[case(
"with_input/dict_with_input.cairo",
"[17 18]",
"[17 18]",
Some("[17 18]"),
Some("[17 18]")
)]

fn test_run_progarm(
#[case] program: &str,
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
fn main(input:Array<felt252>) -> Array<felt252> {
let _elements: Felt252Dict<Nullable<Span<u8>>> = Default::default();
input
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
fn main(input:Array<felt252>) -> Array<felt252> {
let _elements: Felt252Dict<Nullable<Span<u8>>> = Default::default();
input
}
Loading