-
Notifications
You must be signed in to change notification settings - Fork 65
blockifier: convert builtins_to_gas to cairo_primitives_to_gas #12462
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
base: avivg/blockifier/get_cairo_primitive_cost
Are you sure you want to change the base?
blockifier: convert builtins_to_gas to cairo_primitives_to_gas #12462
Conversation
|
Warning This pull request is not mergeable via GitHub because a downstack PR is open. Once all requirements are satisfied, merge this PR as a stack on Graphite.
This stack of pull requests is managed by Graphite. Learn more about stacking. |
dfeb234 to
48bc7b3
Compare
f43a78f to
e9e6ef3
Compare
48bc7b3 to
8469e09
Compare
6b6c67b to
ddde9e6
Compare
ddde9e6 to
2c59ece
Compare
4a50c7f to
31ccc82
Compare
avivg-starkware
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@avivg-starkware made 3 comments.
Reviewable status: 0 of 3 files reviewed, 3 unresolved discussions (waiting on @noaov1).
crates/blockifier/src/bouncer.rs line 693 at r2 (raw file):
) -> GasAmount { // TODO(AvivG): To support opcodes in the computation, pass cairo_primitive_counter_map to this // function
implemented in next PR
Code quote:
// TODO(AvivG): To support opcodes in the computation, pass cairo_primitive_counter_map to this
// function
crates/blockifier/src/bouncer.rs line 709 at r2 (raw file):
versioned_constants: &VersionedConstants, ) -> GasAmount { // TODO(AvivG): To support opcodes in the computation, resources should include opcode counters.
Will be implemented as part of converting ExecutionResources to ExtendedExecutionResources
Code quote:
// TODO(AvivG): To support opcodes in the computation, resources should include opcode counters.
crates/blockifier/src/bouncer.rs line 746 at r2 (raw file):
// NOTE: 'blake' is currently the only supported opcode, by being included in the // builtin_gas_costs. cairo_primitives_gas_costs: &BuiltinGasCosts,
I don't really like this, however, I'm not sure if it's worth making drastic changes. nothing enforces that all entries in CairoPrimitiveCounterMap exist in BuiltinGasCosts
What do you think about adding a test for that sake?
Code quote:
pub fn cairo_primitives_to_gas(
cairo_primitives_counters: &CairoPrimitiveCounterMap,
// NOTE: 'blake' is currently the only supported opcode, by being included in the
// builtin_gas_costs.
cairo_primitives_gas_costs: &BuiltinGasCosts,
31ccc82 to
f0672b5
Compare

Note
Medium Risk
Touches core gas accounting used for block capacity checks; while largely a refactor, mistakes could change fee/proving-gas calculations and bouncer behavior.
Overview
Switches proving-gas and VM-resources gas accounting from builtin-only counters to a generalized Cairo primitive counter map (builtins + opcodes), renaming
builtins_to_gastocairo_primitives_to_gasand routing gas-weight lookup throughBuiltinGasCosts::get_cairo_primitive_gas_cost.Updates bouncer gas computations (
vm_resources_to_gas,sierra_gas_to_steps_gas, proving-gas calculation) and adjusts tests accordingly; current support for opcodes is limited toblake, with TODOs noting that true opcode counting still needs to be wired into resource tracking.Written by Cursor Bugbot for commit f0672b5. This will update automatically on new commits. Configure here.