(refactor): validate entry-point signatures before compilation in class-to-casm - #10221
Conversation
|
This change is |
This stack of pull requests is managed by Graphite. Learn more about stacking. |
PR SummaryLow Risk Overview Signature validation is split from CASM entry-point assembly: Reviewed by Cursor Bugbot for commit 46939ec. Bugbot is set up for automated code reviews on this repo. Configure here. |
orizi
left a comment
There was a problem hiding this comment.
@orizi reviewed all commit messages and made 2 comments.
Reviewable status: 0 of 1 files reviewed, 2 unresolved discussions (waiting on eytan-starkware and TomerStarkware).
crates/cairo-lang-starknet-classes/src/casm_contract_class.rs line 587 at r1 (raw file):
} Ok::<Vec<CasmContractEntryPoint>, StarknetSierraCompilationError>(entry_points) };
Suggestion:
let as_casm_entry_points =
|contract_entry_points: Vec<ContractEntryPoint>,
infos: Vec<(StatementIdx, &FunctionId, Vec<String>)>| -> Vec<CasmContractEntryPoint> {
zip_eq(contract_entry_points, infos).map(
|(contract_entry_point, (statement_id, function_id, builtins))| {
let code_offset = cairo_program
.debug_info
.sierra_statement_info
.get(statement_id.0)
.ok_or(StarknetSierraCompilationError::EntryPointError)?
.start_offset;
assert_eq!(
metadata.gas_info.function_costs[function_id],
CostTokenMap::from_iter([(CostTokenType::Const, ENTRY_POINT_COST as i64)]),
"Unexpected entry point cost."
);
CasmContractEntryPoint {
selector: contract_entry_point.selector,
offset: code_offset,
builtins,
}
}).collect()
};crates/cairo-lang-starknet-classes/src/casm_contract_class.rs line 623 at r1 (raw file):
constructor_infos, )?, },
Suggestion:
entry_points_by_type: CasmContractEntryPoints {
external: as_casm_entry_points(
contract_class.entry_points_by_type.external,
external_infos,
),
l1_handler: as_casm_entry_points(
contract_class.entry_points_by_type.l1_handler,
l1_handler_infos,
),
constructor: as_casm_entry_points(
contract_class.entry_points_by_type.constructor,
constructor_infos,
),
},…ss-to-casm Move the entry-point signature checks (return/panic shape, input == output builtins, felt252-span argument, builtin types, trailing gas/system) out of the post-compilation `as_casm_entry_point` into an `entry_point_builtins` closure that runs over all entry points before the (much heavier) sierra-to-casm compilation. `as_casm_entry_point` reuses it for the builtin names and keeps only the compilation-dependent parts (code offset, cost assert). This reports signature errors without paying for compilation, and also fixes a potential panic: an out-of-range entry-point function index used to be indexed unchecked when building the metadata config; validation now precedes it.
d5c0814 to
46939ec
Compare
orizi
left a comment
There was a problem hiding this comment.
@orizi reviewed 1 file and all commit messages, made 1 comment, and resolved 2 discussions.
Reviewable status:complete! all files reviewed, all discussions resolved (waiting on TomerStarkware).
Summary
Type of change
Please check one:
Why is this change needed?
What was the behavior or documentation before?
What is the behavior or documentation after?
Related issue or discussion (if any)
Additional context