chore(blockifier): add test_proving_gas_minus_sierra_gas_equals_builtin_gas

[avivg-starkware]

0.13.6

[reviewable-StarkWare]

This change is 

[github-actions]

Artifacts upload workflows:

• Started at 2025-07-01T08:23:19Z
• Started at 2025-07-01T08:25:01Z
• Started at 2025-07-01T09:43:08Z
• Started at 2025-07-01T09:49:08Z
• Started at 2025-07-01T09:50:38Z
• Started at 2025-07-01T09:55:53Z
• Started at 2025-07-01T10:08:03Z
• Started at 2025-07-01T10:10:41Z
• Started at 2025-07-03T14:09:35Z
• Started at 2025-07-06T08:00:15Z
• Started at 2025-07-06T08:04:24Z
• Started at 2025-07-07T12:41:55Z

[avivg-starkware]

• chore(blockifier): add test_proving_gas_minus_sierra_gas_equals_builtin_gas #7686 👈 (View in Graphite)
• main-v0.13.6

This stack of pull requests is managed by Graphite. Learn more about stacking.

[meship-starkware]

Reviewed 1 of 1 files at r2, all commit messages.
Reviewable status: 1 of 2 files reviewed, all discussions resolved (waiting on @noaov1)

[meship-starkware]

Reviewed 1 of 2 files at r1.
Reviewable status: all files reviewed, 5 unresolved discussions (waiting on @noaov1)

---

crates/blockifier/src/bouncer_test.rs line 313 at r2 (raw file):

    #[case] executed_class_hashes: HashSet<ClassHash>,
    // Builtin counters for CASM hash computation.
    #[case] additional_os_resources: HashMap<BuiltinName, usize>,

This is for both Casm and Patricia, right?

Code quote:

 #[case] additional_os_resources: HashMap<BuiltinName, usize>,

---

crates/blockifier/src/bouncer_test.rs line 319 at r2 (raw file):

    // Transaction resources builtins.
    let mut builtins = HashMap::from([(BuiltinName::range_check, 2), (BuiltinName::pedersen, 1)]);

Suggestion:

tx_builtin_counters

---

crates/blockifier/src/bouncer_test.rs line 337 at r2 (raw file):

        &state,
        &executed_class_hashes,
        0,

Suggestion:

       // n_visited_storage_entries
       0,

---

crates/blockifier/src/bouncer_test.rs line 339 at r2 (raw file):

        0,
        &tx_resources,
        &StateMaps::default().keys(),

Suggestion:

 // state changes keys
 &StateMaps::default().keys(),

---

crates/blockifier/src/bouncer_test.rs line 364 at r2 (raw file):

            let swo_gas_total = u64_from_usize(swo_gas * count);
            let stone_gas_total = stone_gas * u64_from_usize(*count);

Consider using a checked mul for a better error in case of overflow. It's a test, so it's not mandatory, but I think an overflow might be hard to catch.

Code quote:

            let swo_gas_total = u64_from_usize(swo_gas * count);
            let stone_gas_total = stone_gas * u64_from_usize(*count);

[noaov1]

Reviewed 1 of 2 files at r1, all commit messages.
Reviewable status: all files reviewed, 10 unresolved discussions (waiting on @avivg-starkware and @meship-starkware)

---

crates/blockifier/src/bouncer_test.rs line 313 at r2 (raw file):

Previously, meship-starkware (Meshi Peled) wrote…

This is for both Casm and Patricia, right?

Should be. Note that since n_visited_storage_entries = 0 we get trivial patricia update resources.

---

crates/blockifier/src/bouncer_test.rs line 250 at r2 (raw file):

/// Verifies that the difference between proving gas and Sierra gas is fully explained
/// by the builtin gas delta (between Stone and Stwo). Covers combinations of OS computation

I'm confused.
proving_gas - sierra_gas = builtins_proving_gas,
no?

Code quote:

/// Verifies that the difference between proving gas and Sierra gas is fully explained
/// by the builtin gas delta (between Stone and Stwo). Covers combinations of OS computation

---

crates/blockifier/src/bouncer_test.rs line 325 at r2 (raw file):

            sierra_gas: GasAmount(100),
            tx_vm_resources: ExecutionResources {
                builtin_instance_counter: builtins.clone(),

Note that, in theory, if we have sierra gas, then
builtin_instance_counter ! = builtins.

Code quote:

                builtin_instance_counter: builtins.clone(),

---

crates/blockifier/src/bouncer_test.rs line 354 at r2 (raw file):

        .iter()
        .map(|(name, count)| {
            let swo_gas = block_context.bouncer_config.builtin_weights.builtin_weight(name);

Maybe proving_gas is more consistent with the code.

Suggestion:

 let stwo_gas = block_context.bouncer_config.builtin_weights.builtin_weight(name);

---

crates/blockifier/src/bouncer_test.rs line 361 at r2 (raw file):

                .builtins
                .get_builtin_gas_cost(name)
                .unwrap_or_else(|_| panic!("Expected gas cost for builtin: {}", name));

It's informative enough

Suggestion:

.expect();

---

crates/blockifier/src/bouncer_test.rs line 366 at r2 (raw file):

            let stone_gas_total = stone_gas * u64_from_usize(*count);

            swo_gas_total - stone_gas_total

Note that this can be negative

Code quote:

            swo_gas_total - stone_gas_total

[avivg-starkware]

crates/blockifier/src/bouncer_test.rs line 366 at r2 (raw file):

Previously, noaov1 (Noa Oved) wrote…

Note that this can be negative

all gas prices of stow are currently >= stone , is this to be changed?

[avivg-starkware]

Reviewable status: 1 of 2 files reviewed, 4 unresolved discussions (waiting on @meship-starkware and @noaov1)

---

crates/blockifier/src/bouncer_test.rs line 313 at r2 (raw file):

Previously, noaov1 (Noa Oved) wrote…

Should be. Note that since n_visited_storage_entries = 0 we get trivial patricia update resources.

see refactor below ``n_visited_storage_entries != 0ifcasm_hash_computation_builtins` is not empty

[avivg-starkware]

crates/blockifier/src/bouncer_test.rs line 325 at r2 (raw file):

Previously, noaov1 (Noa Oved) wrote…

Note that, in theory, if we have sierra gas, then
builtin_instance_counter ! = builtins.

changed the Sierra to ZERO

[avivg-starkware]

crates/blockifier/src/bouncer_test.rs line 250 at r2 (raw file):

Previously, noaov1 (Noa Oved) wrote…

I'm confused.
proving_gas - sierra_gas = builtins_proving_gas,
no?

IIUC your question - you're right - the goal of this test is to confirm exactly that:
proving_gas - sierra_gas == builtins_proving_gas,
while covering all possible sources of builtins_proving_gas: transaction builtins, OS, and CASM computation.

I wrote the test to catch any potential drift if builtin accounting changes or if gas costs are misapplied across Stone/Stwo.
Essentially, it verifies that this relationship continues to hold.

[meship-starkware]

Reviewed 1 of 1 files at r5, all commit messages.
Reviewable status: all files reviewed, 3 unresolved discussions (waiting on @noaov1)

[noaov1]

Reviewed all commit messages.
Reviewable status: all files reviewed, 3 unresolved discussions (waiting on @avivg-starkware)

---

crates/blockifier/src/bouncer_test.rs line 366 at r2 (raw file):

Previously, avivg-starkware wrote…

all gas prices of stow are currently >= stone , is this to be changed?

This can change

[avivg-starkware]

Reviewable status: all files reviewed, 3 unresolved discussions (waiting on @noaov1)

---

crates/blockifier/src/bouncer_test.rs line 366 at r2 (raw file):

Previously, noaov1 (Noa Oved) wrote…

This can change

added doc

[noaov1]

Reviewed 1 of 1 files at r5.
Reviewable status: all files reviewed, 5 unresolved discussions (waiting on @avivg-starkware)

---

crates/blockifier/src/bouncer_test.rs line 254 at r5 (raw file):

/// Covers combinations of OS computation builtins and CASM hash computation builtins.
#[rstest]
#[case::no_os_no_casm(

Not sure if it is better

Suggestion:

#[case::no_tx_overhead_no_os_additional_cost(

---

crates/blockifier/src/bouncer_test.rs line 312 at r5 (raw file):

    #[case] contract_instances: &[(FeatureContract, u16)],
    #[case] os_vm_resources: ExecutionResources,
    #[case] executed_class_hashes: HashSet<ClassHash>,

Can this be deduced from contract_instances?
Same question for casm_hash_computation_builtins

Code quote:

    #[case] executed_class_hashes: HashSet<ClassHash>,

---

crates/blockifier/src/bouncer_test.rs line 335 at r5 (raw file):

    };
    // If we simulate CASM hash computation, we assume storage entries were visited.
    let n_visited_storage_entries = if casm_hash_computation_builtins.is_empty() { 0 } else { 1 };

Suggestion:

    // The os additional resources contains both patricia updates and 
    // CASM hash computation. 
    let n_visited_storage_entries = if casm_hash_computation_builtins.is_empty() { 0 } else { 1 };

---

crates/blockifier/src/bouncer_test.rs line 349 at r5 (raw file):

    .unwrap();

    // Combine TX + OS + CASM builtin usage.

What about the patricia builtins?

Suggestion:

// Combine TX + TX overhaed (OS) + CASM and patricia builtin usage.

---

crates/blockifier/src/bouncer_test.rs line 364 at r5 (raw file):

                .builtins
                .get_builtin_gas_cost(name)
                .expect("");

I meant unwrap(), sorry.

Suggestion:

                .get_builtin_gas_cost(name)
                .unwrap();

[avivg-starkware]

Reviewable status: 1 of 2 files reviewed, 3 unresolved discussions (waiting on @meship-starkware and @noaov1)

---

crates/blockifier/src/bouncer_test.rs line 254 at r5 (raw file):

Previously, noaov1 (Noa Oved) wrote…

Not sure if it is better

What do you think of the current names?

---

crates/blockifier/src/bouncer_test.rs line 312 at r5 (raw file):

Previously, noaov1 (Noa Oved) wrote…

Can this be deduced from contract_instances?
Same question for casm_hash_computation_builtins

Done.

---

crates/blockifier/src/bouncer_test.rs line 349 at r5 (raw file):

Previously, noaov1 (Noa Oved) wrote…

What about the patricia builtins?

Done.

---

crates/blockifier/src/bouncer_test.rs line 364 at r5 (raw file):

Previously, noaov1 (Noa Oved) wrote…

I meant unwrap(), sorry.

my bad

[noaov1]

Reviewed 1 of 1 files at r6.
Reviewable status: all files reviewed (commit messages unreviewed), 4 unresolved discussions (waiting on @avivg-starkware)

---

crates/blockifier/src/bouncer_test.rs line 255 at r6 (raw file):

#[rstest]
#[case::tx_builtins_only(&[], ExecutionResources::default())]
#[case::tx_builtins_plus_os_builtins(

Suggestion:

tx_builtins_plus_os_tx_builtins

---

crates/blockifier/src/bouncer_test.rs line 264 at r6 (raw file):

    },
)]
#[case::tx_builtins_plus_casm_hash_computation(

Suggestion:

tx_builtins_plus_os_additional_cost

---

crates/blockifier/src/bouncer_test.rs line 299 at r6 (raw file):

        HashMap::new()
    } else {
        HashMap::from([(BuiltinName::poseidon, 11699), (BuiltinName::pedersen, 2205)])

Please try to avoid hard-coded numbers. Consider using get_particia_update_resources and estimate_casm_hash_computation_resources.

Code quote:

        HashMap::from([(BuiltinName::poseidon, 11699), (BuiltinName::pedersen, 2205)])

[avivg-starkware]

Reviewable status: 1 of 2 files reviewed, 2 unresolved discussions (waiting on @noaov1)

---

crates/blockifier/src/bouncer_test.rs line 299 at r6 (raw file):

Previously, noaov1 (Noa Oved) wrote…

Please try to avoid hard-coded numbers. Consider using get_particia_update_resources and estimate_casm_hash_computation_resources.

Done.

[meship-starkware]

Reviewed 1 of 1 files at r7, all commit messages.
Reviewable status: all files reviewed, 2 unresolved discussions (waiting on @noaov1)

[noaov1]

Reviewed 1 of 1 files at r7, all commit messages.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @avivg-starkware)

---

crates/blockifier/src/bouncer_test.rs line 290 at r7 (raw file):

) {
    use crate::bouncer::get_particia_update_resources;
    use crate::transaction::objects::ExecutionResourcesTraits;

Please move to the top of the file.

Code quote:

    use crate::bouncer::get_particia_update_resources;
    use crate::transaction::objects::ExecutionResourcesTraits;

[avivg-starkware]

crates/blockifier/src/bouncer_test.rs line 290 at r7 (raw file):

Previously, noaov1 (Noa Oved) wrote…

Please move to the top of the file.

thank you!!

[meship-starkware]

Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @noaov1)

[meship-starkware]

Reviewed 1 of 1 files at r8, all commit messages.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @noaov1)