fix(sierra-gas): include RangeCheck96 in ConstCost::into_full_cost_iter - #10038
Conversation
|
This change is |
This stack of pull requests is managed by Graphite. Learn more about stacking. |
PR SummaryMedium Risk Overview
Reviewed by Cursor Bugbot for commit b4cd067. Bugbot is set up for automated code reviews on this repo. Configure here. |
ConstCost::into_full_cost_iter emitted Step/Hole/RangeCheck but omitted range_checks96. In runtime-cost mode this iterator feeds GasInfo::variable_values, so the RangeCheck96 component (e.g. from circuit libfuncs) was silently dropped, under-counting gas for any code that uses it. Add the missing entry. Regenerates the metadata_computation e2e golden (rc96 costs now propagate). SIERRA_UPDATE_NO_CHANGE_TAG=Has no effect on produced code.
49b5b44 to
b4cd067
Compare
eytan-starkware
left a comment
There was a problem hiding this comment.
@eytan-starkware reviewed 2 files and all commit messages, and made 1 comment.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on orizi and TomerStarkware).
crates/cairo-lang-sierra-gas/src/compute_costs.rs line 807 at r1 (raw file):
(CostTokenType::Hole, self.holes.into()), (CostTokenType::RangeCheck, self.range_checks.into()), (CostTokenType::RangeCheck96, self.range_checks96.into()),
Can this be used once so we see it doesnt crash?
orizi
left a comment
There was a problem hiding this comment.
@orizi made 1 comment.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on eytan-starkware and TomerStarkware).
crates/cairo-lang-sierra-gas/src/compute_costs.rs line 807 at r1 (raw file):
Previously, eytan-starkware wrote…
Can this be used once so we see it doesnt crash?
this map is used only for testing - unlike Const and the pre-cost map - so no actual testable usage.
TomerStarkware
left a comment
There was a problem hiding this comment.
@TomerStarkware reviewed all commit messages and made 1 comment.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on eytan-starkware).
eytan-starkware
left a comment
There was a problem hiding this comment.
@eytan-starkware made 1 comment and resolved 1 discussion.
Reviewable status:complete! all files reviewed, all discussions resolved (waiting on orizi).
Summary
RangeCheck96is now included as a tracked cost token type in thePostCostTypeEximplementation forConstCost, alongside the existingStep,Hole, andRangeCheckentries. This ensuresrange_checks96fromConstCostis properly surfaced during gas cost computation.Type of change
Please check one:
Why is this change needed?
RangeCheck96was already a recognizedCostTokenTypebut was not being emitted by thePostCostTypeExiterator forConstCost. This meant 96-bit range check costs were silently omitted from gas solutions derived fromConstCost.What was the behavior or documentation before?
Gas solution maps did not include a
RangeCheck96entry when computed viaConstCost::post_cost_type_ex.What is the behavior or documentation after?
Gas solution maps now include
RangeCheck96: 0(or the appropriate non-zero value) in all cost breakdowns, consistent with how other token types are reported.Related issue or discussion (if any)
Additional context
E2E test snapshots for
metadata_computationhave been updated to reflect the newly emittedRangeCheck96field in all gas solution maps.