refactor(blockifier): remove enforce fee from actual fee

[avivg-starkware]

This change is 

[codecov]

Codecov Report

Attention: Patch coverage is 75.00000% with 1 line in your changes missing coverage. Please review.

Project coverage is 69.13%. Comparing base (e3165c4) to head (8a1bdd4).
Report is 85 commits behind head on main.

Files with missing lines	Patch %	Lines
crates/blockifier/src/concurrency/fee_utils.rs	50.00%	0 Missing and 1 partial ⚠️

Additional details and impacted files

@@             Coverage Diff             @@
##             main     #835       +/-   ##
===========================================
+ Coverage   40.10%   69.13%   +29.02%     
===========================================
  Files          26      100       +74     
  Lines        1895    13447    +11552     
  Branches     1895    13447    +11552     
===========================================
+ Hits          760     9296     +8536     
- Misses       1100     3749     +2649     
- Partials       35      402      +367     

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

[meship-starkware]

Reviewed 3 of 3 files at r1, 1 of 8 files at r2, 9 of 9 files at r3, all commit messages.
Reviewable status: all files reviewed, 5 unresolved discussions (waiting on @avivg-starkware and @noaov1)

---

crates/blockifier/src/concurrency/fee_utils.rs line 66 at r3 (raw file):

        //     // Fee(0),
        //     "Transaction with no fee transfer info must have zero fee."
        // )

Remove this code.

Code quote:

        // assert_eq!(
        //     //aviv: changes:
        //     // tx_execution_info.receipt.fee,
        //     // Fee(0),
        //     "Transaction with no fee transfer info must have zero fee."
        // )

---

crates/blockifier/src/fee/actual_cost.rs line 107 at r3 (raw file):

        // } else {
        //     Fee(0)
        // };

Remove this code.

Code quote:

        // L1 handler transactions are not charged an L2 fee but it is compared to the L1 fee.
        // PREV:
        // let fee = if tx_context.tx_info.enforce_fee() || tx_type == TransactionType::L1Handler {
        //     tx_context.tx_info.get_fee_by_gas_vector(&tx_context.block_context.block_info, gas)
        // } else {
        //     Fee(0)
        // };

---

crates/blockifier/src/fee/actual_cost.rs line 108 at r3 (raw file):

        //     Fee(0)
        // };
        let fee = if tx_type == TransactionType::Declare

Please document this line

Suggestion:

// To be backward compatible with the meaning of enforce fee in Declare V0. 
let fee = if tx_type == TransactionType::Declare

---

crates/blockifier/src/transaction/account_transaction.rs line 362 at r3 (raw file):

        concurrency_mode: bool,
    ) -> TransactionExecutionResult<Option<CallInfo>> {
        // aviv: if !charge_fee || actual_fee == Fee(0) {

Remove all aviv comments.

---

crates/blockifier/src/transaction/transactions_test.rs line 1281 at r3 (raw file):

                FeatureContract::ERC20(CairoVersion::Cairo0).get_class_hash(),
            )
        };

I think it is a bit cleaner this way

Suggestion:

    let (expected_actual_fee, expected_fee_transfer_call_info) =
        if tx_version == TransactionVersion::ZERO {
            (Fee(0), None)
        } else {
            let expected_actual_fee = actual_execution_info.receipt.resources.calculate_tx_fee(block_context, fee_type).unwrap();
            (
                expected_actual_fee,
                expected_fee_transfer_call_info(
                    tx_context,
                    sender_address,
                    expected_actual_fee,
                    FeatureContract::ERC20(CairoVersion::Cairo0).get_class_hash(),
                ),
            )
        };

[meship-starkware]

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

[meship-starkware]

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

[avivg-starkware]

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

---

crates/blockifier/src/concurrency/fee_utils.rs line 66 at r3 (raw file):

Previously, meship-starkware (Meshi Peled) wrote…

Remove this code.

Done.

---

crates/blockifier/src/transaction/account_transaction.rs line 362 at r3 (raw file):

Previously, meship-starkware (Meshi Peled) wrote…

Remove all aviv comments.

Done

---

crates/blockifier/src/transaction/transactions_test.rs line 1281 at r3 (raw file):

Previously, meship-starkware (Meshi Peled) wrote…

I think it is a bit cleaner this way

Done

---

crates/blockifier/src/fee/actual_cost.rs line 107 at r3 (raw file):

Previously, meship-starkware (Meshi Peled) wrote…

Remove this code.

Done.

[meship-starkware]

Reviewed 6 of 15 files at r7, all commit messages.
Reviewable status: 8 of 17 files reviewed, 2 unresolved discussions (waiting on @avivg-starkware and @noaov1)

---

crates/starknet_api/src/transaction.rs line 1098 at r8 (raw file):

            l1_data_gas: ResourceBounds { max_amount: 0, max_price_per_unit: 1 },
        };
        Self::AllResources(resource_bounds)

Why is it part of this PR?

Code quote:

        // Self::L1Gas(ResourceBounds { max_amount: 0, max_price_per_unit: 1 })
        // Aviv: suggestion?
        let resource_bounds = AllResourceBounds {
            l1_gas: ResourceBounds { max_amount: 0, max_price_per_unit: 1 },
            l2_gas: ResourceBounds { max_amount: 0, max_price_per_unit: 1 },
            l1_data_gas: ResourceBounds { max_amount: 0, max_price_per_unit: 1 },
        };
        Self::AllResources(resource_bounds)

[avivg-starkware]

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

---

crates/blockifier/src/transaction/transactions_test.rs line 1395 at r15 (raw file):

Previously, noaov1 (Noa Oved) wrote…

Why is it needed now?

for the case of V0 where charge_fee = false

[avivg-starkware]

python PR: https://github.com/starkware-industries/starkware/pull/36062

[meship-starkware]

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

[meship-starkware]

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

[noaov1]

Reviewed 5 of 8 files at r16, 1 of 2 files at r17, 1 of 1 files at r19, all commit messages.
Reviewable status: 19 of 20 files reviewed, 2 unresolved discussions (waiting on @avivg-starkware and @meship-starkware)

---

crates/blockifier/src/transaction/account_transaction.rs line 468 at r19 (raw file):

        if !charge_fee {
            // Fee charging is not enforced in some tests.
            return Ok(None);

I think we should revert this change. If the actual fee equals 0 there is no need to charge fee.
When can this happen?
Only in declare V0.
(and please leave here the "transaction simulation").

Code quote:

        if !charge_fee {
            // Fee charging is not enforced in some tests.
            return Ok(None);

---

crates/blockifier/src/transaction/transactions_test.rs line 1395 at r15 (raw file):

Previously, avivg-starkware wrote…

for the case of V0 where charge_fee = false

See my comment above.

[avivg-starkware]

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

---

crates/blockifier/src/transaction/transactions_test.rs line 1395 at r15 (raw file):

Previously, noaov1 (Noa Oved) wrote…

See my comment above.

removed

---

crates/blockifier/src/transaction/account_transaction.rs line 468 at r19 (raw file):

Previously, noaov1 (Noa Oved) wrote…

I think we should revert this change. If the actual fee equals 0 there is no need to charge fee.
When can this happen?
Only in declare V0.
(and please leave here the "transaction simulation").

Done.

[noaov1]

Reviewed 8 of 8 files at r20, all commit messages.
Reviewable status: 19 of 20 files reviewed, all discussions resolved (waiting on @meship-starkware)

[noaov1]

Reviewed 3 of 3 files at r21, all commit messages.
Reviewable status: 19 of 20 files reviewed, all discussions resolved (waiting on @meship-starkware)

[github-actions]

Artifacts upload triggered. View details here

[github-actions]

Artifacts upload triggered. View details here

[noaov1]

Reviewed 1 of 8 files at r16, 3 of 3 files at r22, all commit messages.
Reviewable status: complete! all files reviewed, all discussions resolved (waiting on @meship-starkware)

[github-actions]

Artifacts upload triggered. View details here

[avivg-starkware]

Reviewed 1 of 9 files at r6, 2 of 15 files at r7, 3 of 17 files at r12, 5 of 10 files at r13, 1 of 8 files at r16, 4 of 8 files at r20, 1 of 3 files at r21, 1 of 3 files at r22, 2 of 2 files at r23, all commit messages.
Reviewable status: complete! all files reviewed, all discussions resolved (waiting on @meship-starkware)