Skip to content

Commit 2f8c8a0

Browse files
authored
[cairo1] Fix system builtin as implicit (#2207)
* Auxiliary function * Add cairo programs * Add test case * Update changelog * Fix output segment offset * Add test * Add tests * Remove unnecessary test * Fix changelog typo * Use common func * Add comment * Fix clippy * Update comment
1 parent d5f7fd8 commit 2f8c8a0

File tree

9 files changed

+74
-24
lines changed

9 files changed

+74
-24
lines changed

CHANGELOG.md

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,8 @@
22

33
#### Upcoming Changes
44

5+
* Fix bug affecting cairo1 programs with input and System builtin [#2207](https://github.com/lambdaclass/cairo-vm/pull/2207)
6+
57
* chore: Pin generic-array version to 0.14.7 or lower. [#2227](https://github.com/lambdaclass/cairo-vm/pull/2227)
68

79
* feat: Add support for WASM with Cairo 1 [#2216](https://github.com/lambdaclass/cairo-vm/pull/2216)

cairo1-run/src/cairo_run.rs

Lines changed: 25 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -54,6 +54,11 @@ use itertools::{chain, Itertools};
5454
use num_bigint::{BigInt, Sign};
5555
use num_traits::{cast::ToPrimitive, Zero};
5656

57+
// Necessary memory gaps for the values created on the entry code of each implicit builtin
58+
const SEGMENT_ARENA_GAPS: usize = 4;
59+
const GAS_BUILTIN_GAPS: usize = 1;
60+
const SYSTEM_BUILTIN_GAPS: usize = 1;
61+
5762
/// Representation of a cairo argument
5863
/// Can consist of a single Felt or an array of Felts
5964
#[derive(Debug, Clone)]
@@ -469,20 +474,13 @@ fn load_arguments(
469474
cairo_run_config: &Cairo1RunConfig,
470475
main_func: &Function,
471476
) -> Result<(), Error> {
472-
let got_gas_builtin = main_func
473-
.signature
474-
.param_types
475-
.iter()
476-
.any(|ty| ty.debug_name.as_ref().is_some_and(|n| n == "GasBuiltin"));
477+
let got_gas_builtin = got_implicit_builtin(&main_func.signature.param_types, "GasBuiltin");
477478
if cairo_run_config.args.is_empty() && !got_gas_builtin {
478479
// Nothing to be done
479480
return Ok(());
480481
}
481-
let got_segment_arena = main_func
482-
.signature
483-
.param_types
484-
.iter()
485-
.any(|ty| ty.debug_name.as_ref().is_some_and(|n| n == "SegmentArena"));
482+
let got_segment_arena = got_implicit_builtin(&main_func.signature.param_types, "SegmentArena");
483+
let got_system_builtin = got_implicit_builtin(&main_func.signature.param_types, "System");
486484
// This AP correction represents the memory slots taken up by the values created by `create_entry_code`:
487485
// These include:
488486
// * The builtin bases (not including output)
@@ -497,10 +495,13 @@ fn load_arguments(
497495
ap_offset += runner.get_program().builtins_len() - 1;
498496
}
499497
if got_segment_arena {
500-
ap_offset += 4;
498+
ap_offset += SEGMENT_ARENA_GAPS;
501499
}
502500
if got_gas_builtin {
503-
ap_offset += 1;
501+
ap_offset += GAS_BUILTIN_GAPS;
502+
}
503+
if got_system_builtin {
504+
ap_offset += SYSTEM_BUILTIN_GAPS;
504505
}
505506
for arg in cairo_run_config.args {
506507
match arg {
@@ -547,16 +548,9 @@ fn create_entry_code(
547548
) -> Result<(CasmContext, Vec<BuiltinName>), Error> {
548549
let copy_to_output_builtin = config.copy_to_output();
549550
let signature = &func.signature;
550-
let got_segment_arena = signature.param_types.iter().any(|ty| {
551-
get_info(sierra_program_registry, ty)
552-
.map(|x| x.long_id.generic_id == SegmentArenaType::ID)
553-
.unwrap_or_default()
554-
});
555-
let got_gas_builtin = signature.param_types.iter().any(|ty| {
556-
get_info(sierra_program_registry, ty)
557-
.map(|x| x.long_id.generic_id == GasBuiltinType::ID)
558-
.unwrap_or_default()
559-
});
551+
let got_segment_arena = got_implicit_builtin(&signature.param_types, "SegmentArena");
552+
let got_gas_builtin = got_implicit_builtin(&signature.param_types, "GasBuiltin");
553+
let got_system_builtin = got_implicit_builtin(&signature.param_types, "System");
560554
// The builtins in the formatting expected by the runner.
561555
let (builtins, builtin_offset) =
562556
get_function_builtins(&signature.param_types, copy_to_output_builtin);
@@ -738,10 +732,12 @@ fn create_entry_code(
738732
// We lost the output_ptr var after re-scoping, so we need to create it again
739733
// The last instruction will write the last output ptr so we can find it in [ap - 1]
740734
let output_ptr = ctx.add_var(CellExpression::Deref(deref!([ap - 1])));
741-
// len(builtins - output) + len(builtins) + if segment_arena: segment_arena_ptr + info_ptr + 0 + (segment_arena_ptr + 3) + (gas_builtin)
735+
// len(builtins - output) + len(builtins) + if segment_arena: segment_arena_ptr +
736+
// info_ptr + 0 + (segment_arena_ptr + 3) + (gas_builtin) + (system_builtin)
742737
let offset = (2 * builtins.len() - 1
743738
+ 4 * got_segment_arena as usize
744-
+ got_gas_builtin as usize) as i16;
739+
+ got_gas_builtin as usize
740+
+ got_system_builtin as usize) as i16;
745741
let array_start_ptr = ctx.add_var(CellExpression::Deref(deref!([fp + offset])));
746742
let array_end_ptr = ctx.add_var(CellExpression::Deref(deref!([fp + offset + 1])));
747743
casm_build_extend! {ctx,
@@ -1008,6 +1004,11 @@ fn check_only_array_felt_return_type(
10081004
_ => false,
10091005
}
10101006
}
1007+
fn got_implicit_builtin(param_types: &[ConcreteTypeId], builtin_name: &str) -> bool {
1008+
param_types
1009+
.iter()
1010+
.any(|ty| ty.debug_name.as_ref().is_some_and(|n| n == builtin_name))
1011+
}
10111012

10121013
fn is_panic_result(return_type_id: Option<&ConcreteTypeId>) -> bool {
10131014
return_type_id

cairo1-run/src/main.rs

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -454,6 +454,27 @@ mod tests {
454454
None,
455455
None
456456
)]
457+
#[case(
458+
"with_input/implicit_system_builtin.cairo",
459+
"[1 2]",
460+
"[1 2]",
461+
Some("[1 2]"),
462+
Some("[1 2]")
463+
)]
464+
#[case(
465+
"with_input/implicit_gas_builtin.cairo",
466+
"[1 2]",
467+
"[1 2]",
468+
Some("[1 2]"),
469+
Some("[1 2]")
470+
)]
471+
#[case(
472+
"with_input/system_segment_gas.cairo",
473+
"[1 2]",
474+
"[1 2]",
475+
Some("[1 2]"),
476+
Some("[1 2]")
477+
)]
457478
fn test_run_program(
458479
#[case] program: &str,
459480
#[case] expected_output: &str,
Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,4 @@
1+
fn main(input: Array<felt252>) -> Array<felt252> {
2+
core::internal::require_implicit::<GasBuiltin>();
3+
input
4+
}
Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,4 @@
1+
fn main(input: Array<felt252>) -> Array<felt252> {
2+
core::internal::require_implicit::<System>();
3+
input
4+
}
Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
fn main(input:Array<felt252>) -> Array<felt252> {
2+
core::internal::require_implicit::<System>();
3+
let _elements: Felt252Dict<Nullable<Span<u8>>> = Default::default();
4+
input
5+
}
Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,4 @@
1+
fn main(input: Array<felt252>) -> Array<felt252> {
2+
core::internal::require_implicit::<GasBuiltin>();
3+
input
4+
}
Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,4 @@
1+
fn main(input: Array<felt252>) -> Array<felt252> {
2+
core::internal::require_implicit::<System>();
3+
input
4+
}
Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
fn main(input:Array<felt252>) -> Array<felt252> {
2+
core::internal::require_implicit::<System>();
3+
let _elements: Felt252Dict<Nullable<Span<u8>>> = Default::default();
4+
input
5+
}

0 commit comments

Comments
 (0)