feat(blockifier): compute allocation cost

[yoavGrs]

No description provided.

[reviewable-StarkWare]

This change is 

[yoavGrs]

• fix(blockifier): merge state diff with squash #2310 : 2 dependent PRs (#2456 , #2471 )
• feat(blockifier): add comprehensive state diff versioned constant #2407
• feat(blockifier): compute allocation cost #2301 👈 (View in Graphite)
• feat(starknet_api): checked mul for gas vector #2300
• feat(blockifier): two allocation_cost gas vectors #2293
• feat(blockifier): implement the allocated keys logic #2150
• feat(blockifier): add struct StateChangesCountForFee #2175
• feat(blockifier): add n_allocated_keys #2149
• feat(blockifier): add allocation cost versioned constant #2200
• main

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

[github-actions]

Artifacts upload workflows:

• Started at 2024-11-27T08:48:19Z
• Started at 2024-11-27T13:23:43Z
• Started at 2024-11-27T13:52:04Z
• Started at 2024-11-27T14:27:57Z
• Started at 2024-12-02T09:14:50Z
• Started at 2024-12-02T12:46:19Z
• Started at 2024-12-03T09:15:39Z
• Started at 2024-12-03T09:15:20Z
• Started at 2024-12-03T09:15:39Z
• Started at 2024-12-03T09:16:09Z
• Started at 2024-12-03T09:16:15Z
• Started at 2024-12-03T09:28:26Z

[codecov]

Codecov Report

Attention: Patch coverage is 60.00000% with 10 lines in your changes missing coverage. Please review.

Project coverage is 68.07%. Comparing base (e3165c4) to head (e281147).
Report is 666 commits behind head on main.

Files with missing lines	Patch %	Lines
crates/blockifier/src/fee/resources.rs	58.33%	8 Missing and 2 partials ⚠️

Additional details and impacted files

@@             Coverage Diff             @@
##             main    #2301       +/-   ##
===========================================
+ Coverage   40.10%   68.07%   +27.96%     
===========================================
  Files          26      109       +83     
  Lines        1895    14314    +12419     
  Branches     1895    14314    +12419     
===========================================
+ Hits          760     9744     +8984     
- Misses       1100     4156     +3056     
- Partials       35      414      +379     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[nimrod-starkware]

Reviewed 3 of 6 files at r1.
Reviewable status: 3 of 6 files reviewed, 2 unresolved discussions (waiting on @yoavGrs)

---

crates/blockifier/src/fee/resources.rs line 206 at r1 (raw file):

                 to {da_gas_cost:?}",
            )
        })

Do we want to panic in that case? Isn't it better to return something like GasVector::MAX?

Code quote:

    pub fn to_gas_vector(&self, use_kzg_da: bool, allocation_cost: &AllocationCost) -> GasVector {
        let n_allocated_keys = self.state_changes_for_fee.n_allocated_keys;
        let allocation_gas_vector = allocation_cost.get_cost(use_kzg_da);
        let total_allocation_cost =
            allocation_gas_vector.checked_mul(n_allocated_keys).unwrap_or_else(|| {
                panic!(
                    "State resources to gas vector overflowed: tried to multiply \
                     {allocation_gas_vector:?} by {n_allocated_keys:?}",
                )
            });
        let da_gas_cost =
            get_da_gas_cost(&self.state_changes_for_fee.state_changes_count, use_kzg_da);
        total_allocation_cost.checked_add(da_gas_cost).unwrap_or_else(|| {
            panic!(
                "State resources to gas vector overflowed: tried to add {total_allocation_cost:?} \
                 to {da_gas_cost:?}",
            )
        })

---

crates/blockifier/src/transaction/transactions_test.rs line 2308 at r1 (raw file):

        true => GasVector::from_l1_data_gas(160_u32.into()),
        false => GasVector::from_l1_gas(2203_u32.into()),
    };

If you set block_context.versioned_constants.allocation_cost = AllocationCost::ZERO, then the values are equal?

Code quote:

    let expected_gas = match use_kzg_da {
        true => GasVector {
            l1_gas: 17988_u32.into(),
            l1_data_gas: 160_u32.into(),
            l2_gas: 0_u32.into(),
        },
        false => GasVector::from_l1_gas(19682_u32.into()),
    };

    let expected_da_gas = match use_kzg_da {
        true => GasVector::from_l1_data_gas(160_u32.into()),
        false => GasVector::from_l1_gas(2203_u32.into()),
    };

[yoavGrs]

Reviewable status: 3 of 6 files reviewed, 2 unresolved discussions (waiting on @nimrod-starkware)

---

crates/blockifier/src/fee/resources.rs line 206 at r1 (raw file):

Previously, nimrod-starkware wrote…

Do we want to panic in that case? Isn't it better to return something like GasVector::MAX?

Yes, I want to panic.
Like in the function to_gas_vector of StarknetResources that calls to this function.
We don't want to lose fees.

---

crates/blockifier/src/transaction/transactions_test.rs line 2308 at r1 (raw file):

Previously, nimrod-starkware wrote…

If you set block_context.versioned_constants.allocation_cost = AllocationCost::ZERO, then the values are equal?

Yes, with AllocationCost::ZERO the test passes without changes.

[nimrod-starkware]

Reviewable status: 3 of 6 files reviewed, 1 unresolved discussion (waiting on @dorimedini-starkware and @yoavGrs)

---

crates/blockifier/src/transaction/transactions_test.rs line 2308 at r1 (raw file):

Previously, yoavGrs wrote…

Yes, with AllocationCost::ZERO the test passes without changes.

I would prefer to set block_context.versioned_constants.allocation_cost = AllocationCost::ZERO, and keep values as they were

[dorimedini-starkware]

Reviewed 3 of 6 files at r1, all commit messages.
Reviewable status: all files reviewed, 3 unresolved discussions (waiting on @nimrod-starkware and @yoavGrs)

---

crates/blockifier/src/transaction/transactions_test.rs line 2308 at r1 (raw file):

Previously, nimrod-starkware wrote…

I would prefer to set block_context.versioned_constants.allocation_cost = AllocationCost::ZERO, and keep values as they were

this is wrong: the DA gas in the receipt should not change, regardless of allocation cost

---

crates/blockifier/src/fee/resources.rs line 207 at r1 (raw file):

            )
        })
    }

where do we compute the da_gas in the TransactionReceipt?
please make sure it is NOT state_resource.to_gas_vector... if it is, please update it (maybe add a to_da_gas_vector method in this struct?)

Suggestion:

    pub fn to_gas_vector(&self, use_kzg_da: bool, allocation_cost: &AllocationCost) -> GasVector {
        let n_allocated_keys = self.state_changes_for_fee.n_allocated_keys;
        let allocation_gas_vector = allocation_cost.get_cost(use_kzg_da);
        let total_allocation_cost =
            allocation_gas_vector.checked_mul(n_allocated_keys).unwrap_or_else(|| {
                panic!(
                    "State resources to gas vector overflowed: tried to multiply \
                     {allocation_gas_vector:?} by {n_allocated_keys:?}",
                )
            });
        let da_gas_cost = self.to_da_gas_vector(use_kzg_da);
        total_allocation_cost.checked_add(da_gas_cost).unwrap_or_else(|| {
            panic!(
                "State resources to gas vector overflowed: tried to add {total_allocation_cost:?} \
                 to {da_gas_cost:?}",
            )
        })
    }

---

crates/blockifier/src/transaction/transactions_test.rs line 593 at r1 (raw file):

        receipt: TransactionReceipt {
            fee: expected_actual_fee,
            da_gas,

this is wrong - the da_gas you compute includes allocation costs.
please fix the computation of the TransactionReceipt (maybe many tests will fail... but you will see which ones at least)

Code quote:

da_gas,

[yoavGrs]

Reviewable status: 1 of 6 files reviewed, 3 unresolved discussions (waiting on @dorimedini-starkware and @nimrod-starkware)

---

crates/blockifier/src/fee/resources.rs line 207 at r1 (raw file):

Previously, dorimedini-starkware wrote…

where do we compute the da_gas in the TransactionReceipt?
please make sure it is NOT state_resource.to_gas_vector... if it is, please update it (maybe add a to_da_gas_vector method in this struct?)

Done.

---

crates/blockifier/src/transaction/transactions_test.rs line 593 at r1 (raw file):

Previously, dorimedini-starkware wrote…

this is wrong - the da_gas you compute includes allocation costs.
please fix the computation of the TransactionReceipt (maybe many tests will fail... but you will see which ones at least)

Done.

---

crates/blockifier/src/transaction/transactions_test.rs line 2308 at r1 (raw file):

Previously, dorimedini-starkware wrote…

this is wrong: the DA gas in the receipt should not change, regardless of allocation cost

Done.

[dorimedini-starkware]

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

[nimrod-starkware]

Reviewable status: complete! all files reviewed, all discussions resolved (waiting on @yoavGrs)

[dorimedini-starkware]

Reviewable status: complete! all files reviewed, all discussions resolved (waiting on @yoavGrs)

[dorimedini-starkware]

Reviewed 1 of 1 files at r3, all commit messages.
Reviewable status: complete! all files reviewed, all discussions resolved (waiting on @yoavGrs)