Sitelet https://github.com/starkware-libs/cairo/commit/013c7edaf78773189c29350aa103a4fb8d0e4bc3
Skip to content

Commit 013c7ed

Browse files
eytan-starkwareeytan-starkware
committed
(bug fix): validate entry-point builtins are in canonical order in class-to-casm
A Starknet entry point's builtin order is only *assumed* to be the canonical order the OS expects: it is produced upstream by the `cairo-lang-starknet` plugin (which stamps `#[implicit_precedence(...)]`) plus the lowering-stage sort, and nothing re-checks it when a `ContractClass` is compiled to a `CasmContractClass`. A class whose entry points list builtins out of that order would compile here yet misbehave in the OS. Add `ENTRY_POINT_BUILTIN_ORDER` to `cairo-lang-starknet-classes`: the canonical order excluding the trailing gas/system pair (which is checked separately). `from_contract_class` now validates that each entry point's remaining builtins are a subsequence of it, returning `InvalidEntryPointSignatureWrongBuiltinsOrder` otherwise; gas or system threaded mid-list is rejected as `InvalidBuiltinType` since they are no longer in the valid mid-list set. The plugin's `IMPLICIT_PRECEDENCE` path list stays in the plugin (Cairo type paths don't belong in the sierra/casm layer) with a comment tying it to the check here. Tested with hand-written Sierra (the frontend always emits canonical order, so a violation can only be constructed by hand): a canonical entry point compiles; the same entry point with two builtins swapped, or with an extra gas/system mid-list, is rejected. All existing contract and plugin fixtures still compile unchanged.
1 parent daff9bc commit 013c7ed

3 files changed

Lines changed: 171 additions & 16 deletions

File tree

‎crates/cairo-lang-starknet-classes/src/casm_contract_class.rs‎

Lines changed: 28 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -94,6 +94,21 @@ pub enum StarknetSierraCompilationError {
9494
UnsupportedSierraVersion { version_in_contract: VersionId, version_of_compiler: VersionId },
9595
}
9696

97+
/// The canonical order entry-point builtins must appear in, as expected by the Starknet OS,
98+
/// excluding the trailing gas and system builtins (checked separately in
99+
/// [`CasmContractClass::from_contract_class`]).
100+
pub static ENTRY_POINT_BUILTIN_ORDER: [GenericTypeId; 9] = [
101+
PedersenType::ID,
102+
RangeCheckType::ID,
103+
BitwiseType::ID,
104+
EcOpType::ID,
105+
PoseidonType::ID,
106+
SegmentArenaType::ID,
107+
RangeCheck96Type::ID,
108+
AddModType::ID,
109+
MulModType::ID,
110+
];
111+
97112
fn skip_if_none<T>(opt_field: &Option<T>) -> bool {
98113
opt_field.is_none()
99114
}
@@ -475,19 +490,10 @@ impl CasmContractClass {
475490
None
476491
};
477492

478-
let builtin_types = UnorderedHashSet::<GenericTypeId>::from_iter([
479-
RangeCheckType::id(),
480-
BitwiseType::id(),
481-
PedersenType::id(),
482-
EcOpType::id(),
483-
PoseidonType::id(),
484-
SegmentArenaType::id(),
485-
GasBuiltinType::id(),
486-
SystemType::id(),
487-
RangeCheck96Type::id(),
488-
AddModType::id(),
489-
MulModType::id(),
490-
]);
493+
// The set of valid entry-point builtin types, excluding the trailing gas and system
494+
// builtins (checked separately below).
495+
let builtin_types: UnorderedHashSet<GenericTypeId> =
496+
ENTRY_POINT_BUILTIN_ORDER.iter().cloned().collect();
491497

492498
let as_casm_entry_point = |contract_entry_point: ContractEntryPoint| {
493499
let Some(function) = program.funcs.get(contract_entry_point.function_idx) else {
@@ -519,7 +525,7 @@ impl CasmContractClass {
519525
require(type_resolver.is_valid_entry_point_return_type(panic_result))
520526
.ok_or(StarknetSierraCompilationError::InvalidEntryPointSignature)?;
521527

522-
for type_id in input_builtins {
528+
for type_id in builtins {
523529
if !builtin_types.contains(type_resolver.get_generic_id(type_id)) {
524530
return Err(StarknetSierraCompilationError::InvalidBuiltinType(
525531
type_id.clone(),
@@ -536,6 +542,14 @@ impl CasmContractClass {
536542
);
537543
}
538544

545+
// Iterator-based subsequence check: the entry point's builtins (excluding the trailing
546+
// gas and system, verified above) must be a subsequence of the canonical order.
547+
let mut canonical = ENTRY_POINT_BUILTIN_ORDER.iter();
548+
require(builtins.iter().all(|type_id| {
549+
canonical.any(|generic_id| generic_id == type_resolver.get_generic_id(type_id))
550+
}))
551+
.ok_or(StarknetSierraCompilationError::InvalidEntryPointSignatureWrongBuiltinsOrder)?;
552+
539553
let builtins = builtins
540554
.iter()
541555
.map(|type_id| match type_resolver.get_generic_id(type_id).0.as_str() {

‎crates/cairo-lang-starknet-classes/src/casm_contract_class_test.rs‎

Lines changed: 141 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -4,20 +4,27 @@ use std::collections::HashSet;
44
use std::fs;
55
use std::io::BufReader;
66

7+
use cairo_lang_sierra::ProgramParser;
78
use cairo_lang_sierra::ids::GenericLibfuncId;
9+
use cairo_lang_sierra_generator::canonical_id_replacer::CanonicalReplacer;
10+
use cairo_lang_sierra_generator::replace_ids::SierraIdReplacer;
811
use cairo_lang_test_utils::compare_contents_or_fix_with_path;
912
use cairo_lang_test_utils::parse_test_file::TestRunnerResult;
1013
use cairo_lang_utils::ordered_hash_map::OrderedHashMap;
14+
use indoc::formatdoc;
1115
use itertools::Itertools;
16+
use num_bigint::BigUint;
1217
use starknet_types_core::felt::Felt as Felt252;
1318
use test_case::test_case;
1419

1520
use crate::allowed_libfuncs::{
1621
BUILTIN_AUDITED_LIBFUNCS_LIST, ListSelector, lookup_allowed_libfuncs_list,
1722
};
18-
use crate::casm_contract_class::{BigUintAsHex, CasmContractClass};
23+
use crate::casm_contract_class::{BigUintAsHex, CasmContractClass, StarknetSierraCompilationError};
1924
use crate::compiler_version::current_sierra_version_id;
20-
use crate::contract_class::ContractClass;
25+
use crate::contract_class::{
26+
ContractClass, ContractEntryPoint, ContractEntryPoints, ExtractedSierraProgram,
27+
};
2128
use crate::felt252_serde::{Felt252SerdeError, sierra_from_felt252s};
2229
use crate::test_utils::get_example_file_path;
2330

@@ -34,6 +41,138 @@ fn test_casm_contract_from_contract_class_failure(name: &str) {
3441
);
3542
}
3643

44+
/// Hand-written Sierra for a contract with a single entry point. `RangeCheck` is fixed as the
45+
/// first builtin (it feeds `withdraw_gas`, so the entry-point cost is properly accounted); the
46+
/// next two builtins are `builtin_a` then `builtin_b`, which are just threaded through. Vary
47+
/// those two to control the builtin order the check sees. Hand-written on purpose: the frontend
48+
/// always emits builtins in the canonical order, so a non-canonical order can only be built here.
49+
fn entry_point_sierra(builtin_a: &str, builtin_b: &str) -> String {
50+
formatdoc! {"
51+
type RangeCheck = RangeCheck [storable: true, drop: false, dup: false, zero_sized: false];
52+
type Bitwise = Bitwise [storable: true, drop: false, dup: false, zero_sized: false];
53+
type EcOp = EcOp [storable: true, drop: false, dup: false, zero_sized: false];
54+
type GasBuiltin = GasBuiltin [storable: true, drop: false, dup: false, zero_sized: false];
55+
type System = System [storable: true, drop: false, dup: false, zero_sized: false];
56+
type felt252 = felt252 [storable: true, drop: true, dup: true, zero_sized: false];
57+
type Array<felt252> = Array<felt252> [storable: true, drop: true, dup: false, zero_sized: false];
58+
type Snapshot<Array<felt252>> = Snapshot<Array<felt252>> [storable: true, drop: true, dup: true, zero_sized: false];
59+
type core::array::Span::<core::felt252> = Struct<ut@[782572820229152792105145177694740816763001980856532159945905090893343825762], Snapshot<Array<felt252>>> [storable: true, drop: true, dup: true, zero_sized: false];
60+
type Tuple<core::array::Span::<core::felt252>> = Struct<ut@[1325343513152088812341467750635149026053683136611136091911357178651207272643], core::array::Span::<core::felt252>> [storable: true, drop: true, dup: true, zero_sized: false];
61+
type core::panics::Panic = Struct<ut@[640126984585624630990013944782631102820301644699864366139839615702772668018]> [storable: true, drop: true, dup: true, zero_sized: true];
62+
type Tuple<core::panics::Panic, Array<felt252>> = Struct<ut@[1325343513152088812341467750635149026053683136611136091911357178651207272643], core::panics::Panic, Array<felt252>> [storable: true, drop: true, dup: false, zero_sized: false];
63+
type core::panics::PanicResult::<(core::array::Span::<core::felt252>,)> = Enum<ut@[270671131472959732993844072583327084608513343873724697777364695367457417702], Tuple<core::array::Span::<core::felt252>>, Tuple<core::panics::Panic, Array<felt252>>> [storable: true, drop: true, dup: false, zero_sized: false];
64+
65+
libfunc revoke_ap_tracking = revoke_ap_tracking;
66+
libfunc withdraw_gas = withdraw_gas;
67+
libfunc branch_align = branch_align;
68+
libfunc redeposit_gas = redeposit_gas;
69+
libfunc struct_construct<Tuple<core::array::Span::<core::felt252>>> = struct_construct<Tuple<core::array::Span::<core::felt252>>>;
70+
libfunc enum_init<core::panics::PanicResult::<(core::array::Span::<core::felt252>,)>, 0> = enum_init<core::panics::PanicResult::<(core::array::Span::<core::felt252>,)>, 0>;
71+
libfunc drop<core::array::Span::<core::felt252>> = drop<core::array::Span::<core::felt252>>;
72+
libfunc array_new<felt252> = array_new<felt252>;
73+
libfunc struct_construct<core::panics::Panic> = struct_construct<core::panics::Panic>;
74+
libfunc struct_construct<Tuple<core::panics::Panic, Array<felt252>>> = struct_construct<Tuple<core::panics::Panic, Array<felt252>>>;
75+
libfunc enum_init<core::panics::PanicResult::<(core::array::Span::<core::felt252>,)>, 1> = enum_init<core::panics::PanicResult::<(core::array::Span::<core::felt252>,)>, 1>;
76+
libfunc store_temp<RangeCheck> = store_temp<RangeCheck>;
77+
libfunc store_temp_a = store_temp<{builtin_a}>;
78+
libfunc store_temp_b = store_temp<{builtin_b}>;
79+
libfunc store_temp<GasBuiltin> = store_temp<GasBuiltin>;
80+
libfunc store_temp<System> = store_temp<System>;
81+
libfunc store_temp<core::panics::PanicResult::<(core::array::Span::<core::felt252>,)>> = store_temp<core::panics::PanicResult::<(core::array::Span::<core::felt252>,)>>;
82+
83+
F0:
84+
revoke_ap_tracking() -> ();
85+
withdraw_gas([0], [3]) {{ fallthrough([6], [7]) F0_B0([8], [9]) }};
86+
branch_align() -> ();
87+
redeposit_gas([7]) -> ([10]);
88+
struct_construct<Tuple<core::array::Span::<core::felt252>>>([5]) -> ([11]);
89+
enum_init<core::panics::PanicResult::<(core::array::Span::<core::felt252>,)>, 0>([11]) -> ([12]);
90+
store_temp<RangeCheck>([6]) -> ([6]);
91+
store_temp_a([1]) -> ([1]);
92+
store_temp_b([2]) -> ([2]);
93+
store_temp<GasBuiltin>([10]) -> ([10]);
94+
store_temp<System>([4]) -> ([4]);
95+
store_temp<core::panics::PanicResult::<(core::array::Span::<core::felt252>,)>>([12]) -> ([12]);
96+
return([6], [1], [2], [10], [4], [12]);
97+
F0_B0:
98+
branch_align() -> ();
99+
drop<core::array::Span::<core::felt252>>([5]) -> ();
100+
array_new<felt252>() -> ([13]);
101+
struct_construct<core::panics::Panic>() -> ([14]);
102+
struct_construct<Tuple<core::panics::Panic, Array<felt252>>>([14], [13]) -> ([15]);
103+
enum_init<core::panics::PanicResult::<(core::array::Span::<core::felt252>,)>, 1>([15]) -> ([16]);
104+
store_temp<RangeCheck>([8]) -> ([8]);
105+
store_temp_a([1]) -> ([1]);
106+
store_temp_b([2]) -> ([2]);
107+
store_temp<GasBuiltin>([9]) -> ([9]);
108+
store_temp<System>([4]) -> ([4]);
109+
store_temp<core::panics::PanicResult::<(core::array::Span::<core::felt252>,)>>([16]) -> ([16]);
110+
return([8], [1], [2], [9], [4], [16]);
111+
112+
test::contract::__wrapper__entry@F0([0]: RangeCheck, [1]: {builtin_a}, [2]: {builtin_b}, [3]: GasBuiltin, [4]: System, [5]: core::array::Span::<core::felt252>) -> (RangeCheck, {builtin_a}, {builtin_b}, GasBuiltin, System, core::panics::PanicResult::<(core::array::Span::<core::felt252>,)>);
113+
"}
114+
}
115+
116+
/// Compiles a hand-written Sierra contract (single external entry point at function 0) to CASM.
117+
fn compile_hand_written(
118+
sierra_text: &str,
119+
) -> Result<CasmContractClass, StarknetSierraCompilationError> {
120+
// `ProgramParser` assigns hash-based ids to named types, but the entry-point `TypeResolver`
121+
// indexes type declarations positionally, as in a real (felt-deserialized) contract class.
122+
// Canonicalize the ids to sequential with the shared `CanonicalReplacer`.
123+
let program = ProgramParser::new().parse(sierra_text).unwrap();
124+
let program = CanonicalReplacer::from_program(&program).apply(&program);
125+
let extracted = ExtractedSierraProgram {
126+
program,
127+
sierra_version: current_sierra_version_id(),
128+
compiler_version: current_sierra_version_id(),
129+
};
130+
// `from_contract_class` takes the Sierra program from `extracted`; from the `ContractClass` it
131+
// only reads `entry_points_by_type`, so the other fields are left empty.
132+
let contract = ContractClass {
133+
sierra_program: vec![],
134+
sierra_program_debug_info: None,
135+
contract_class_version: "0.1.0".to_string(),
136+
entry_points_by_type: ContractEntryPoints {
137+
external: vec![ContractEntryPoint { selector: BigUint::from(1u32), function_idx: 0 }],
138+
l1_handler: vec![],
139+
constructor: vec![],
140+
},
141+
abi: None,
142+
};
143+
CasmContractClass::from_contract_class(contract, extracted, false, usize::MAX)
144+
}
145+
146+
/// An entry point whose builtins are in the canonical order (RangeCheck, Bitwise, EcOp) compiles
147+
/// successfully.
148+
#[test]
149+
fn test_entry_point_canonical_builtin_order_accepted() {
150+
assert!(compile_hand_written(&entry_point_sierra("Bitwise", "EcOp")).is_ok());
151+
}
152+
153+
/// The same entry point with two builtins swapped (EcOp before Bitwise, violating the canonical
154+
/// order) is rejected. This order cannot come from the frontend, so it can only be exercised with
155+
/// hand-written Sierra.
156+
#[test]
157+
fn test_entry_point_non_canonical_builtin_order_rejected() {
158+
assert_eq!(
159+
compile_hand_written(&entry_point_sierra("EcOp", "Bitwise")).unwrap_err(),
160+
StarknetSierraCompilationError::InvalidEntryPointSignatureWrongBuiltinsOrder,
161+
);
162+
}
163+
164+
/// Gas and system builtins are only allowed in their fixed trailing slots — an extra `System`
165+
/// (or `GasBuiltin`) threaded mid-list is rejected even though the trailing pair is in place.
166+
#[test]
167+
fn test_entry_point_mid_list_gas_or_system_rejected() {
168+
for mid in ["System", "GasBuiltin"] {
169+
assert!(matches!(
170+
compile_hand_written(&entry_point_sierra("Bitwise", mid)).unwrap_err(),
171+
StarknetSierraCompilationError::InvalidBuiltinType(_),
172+
));
173+
}
174+
}
175+
37176
/// Tests that the CASM compiled from a contract in the contract_crate is the same as in
38177
/// <test_case>.compiled_contract_class.json.
39178
#[test_case("account__account")]

‎crates/cairo-lang-starknet/src/plugin/consts.rs‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -65,6 +65,8 @@ pub(super) const L1_HANDLER_FIRST_PARAM_NAME: &str = "from_address";
6565
pub(super) const CALLDATA_PARAM_NAME: &str = "__calldata__";
6666

6767
/// Starknet OS required implicit precedence.
68+
/// Checked in `cairo-lang-starknet-classes` during contract-class-to-CASM compilation (see
69+
/// `ENTRY_POINT_BUILTIN_ORDER` there).
6870
pub(super) const IMPLICIT_PRECEDENCE: &[&str] = &[
6971
"core::pedersen::Pedersen",
7072
"core::RangeCheck",

0 commit comments

Comments
 (0)