test(blockifier): parametrize test_max_fee_exceeds_balance by resource bounds types

[dorimedini-starkware]

This change is 

[dorimedini-starkware]

• test(blockifier): migrate remaining tests to all resource bounds (except execution flavors test) #1342
• test(blockifier): parametrize more tests in transactions_test by resource bounds types #1337
• test(blockifier): parametrize test_actual_fee_gt_resource_bounds by resource bounds types #1330
• test(blockifier): parametrize test_max_fee_exceeds_balance by resource bounds types #1322 👈
• test(blockifier): parametrize test_invoke_tx by resource bounds types #1321
• test(blockifier): update test_revert_on_resource_overuse to include new resource bounds #1314
• test(blockifier): update test_insufficient_max_fee_reverts to include new resource bounds #1313
• test(blockifier): l1 bounds -> all bounds (or parametrize) in account transaction test #1312
• test(blockifier): test enforce_fee flag with new resource bounds #1311
• test(blockifier): l1 bounds -> all bounds in stack trace test #1310
• test(blockifier): l1 bounds -> all bounds in worker logic test #1309
• test(blockifier): l1 bounds -> all bounds in versioned state test #1308
• test(blockifier): add all resource bounds to stateful validator test #1307
• refactor(blockifier): refactor default testing resource bounds #1306
• main

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

Join @dorimedini-starkware and the rest of your teammates on Graphite

[codecov]

Codecov Report

Attention: Patch coverage is 22.22222% with 14 lines in your changes missing coverage. Please review.

Project coverage is 77.35%. Comparing base (e3165c4) to head (21653a6).
Report is 126 commits behind head on main.

Files with missing lines	Patch %	Lines
crates/starknet_api/src/transaction.rs	0.00%	14 Missing ⚠️

Additional details and impacted files

@@             Coverage Diff             @@
##             main    #1322       +/-   ##
===========================================
+ Coverage   40.10%   77.35%   +37.24%     
===========================================
  Files          26      100       +74     
  Lines        1895    13457    +11562     
  Branches     1895    13457    +11562     
===========================================
+ Hits          760    10409     +9649     
- Misses       1100     2591     +1491     
- Partials       35      457      +422     

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

[github-actions]

Artifacts upload triggered. View details here

[github-actions]

Artifacts upload triggered. View details here

[github-actions]

Artifacts upload triggered. View details here

[github-actions]

Artifacts upload triggered. View details here

[github-actions]

Artifacts upload triggered. View details here

[yoavGrs]

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

---

crates/blockifier/src/transaction/transactions_test.rs line 841 at r2 (raw file):

}

fn assert_failure_if_resource_bounds_exceed_balance(

No if

Suggestion:

assert_resource_bounds_exceed_balance_failure

---

crates/blockifier/src/transaction/transactions_test.rs line 881 at r2 (raw file):

                    TransactionExecutionError::TransactionPreValidationError(
                        TransactionPreValidationError::TransactionFeeError(
                            TransactionFeeError::ResourcesBoundsExceedBalance {

It is better if this error object contains an AllResourceBounds object.
Non blocking.

Code quote:

ResourcesBoundsExceedBalance

---

crates/blockifier/src/transaction/transactions_test.rs line 955 at r2 (raw file):

        &[(account_contract, 1), (test_contract, 1)],
    );
    let account_contract_address = account_contract.get_instance_address(0);

Suggestion.

Suggestion:

sender_address

---

crates/blockifier/src/transaction/transactions_test.rs line 956 at r2 (raw file):

    );
    let account_contract_address = account_contract.get_instance_address(0);
    let default_args = invoke_tx_args! {

Suggestion:

default_invoke_args

---

crates/blockifier/src/transaction/transactions_test.rs line 960 at r2 (raw file):

        calldata: create_trivial_calldata(test_contract.get_instance_address(0)
    )};
    let invalid_max_fee = Fee(BALANCE.0 + 1);

Move under V1 Invoke.

Code quote:

let invalid_max_fee = Fee(BALANCE.0 + 1);

---

crates/blockifier/src/transaction/transactions_test.rs line 1047 at r2 (raw file):

                base_resource_bounds
            ));
            base_resource_bounds.l1_gas.max_amount.0 -= 10;

Code duplication.
Move te logic into a macro

Suggestion:

            macro_rules! resource_overdraft {
                ($max_amount:expr) => {
                    $max_amount.0 += 10;
                    v3_txs_invalid_bounds_invoke_case!(ValidResourceBounds::AllResources(
                        base_resource_bounds
                    ));
                    v3_txs_invalid_bounds_declare_case!(ValidResourceBounds::AllResources(
                        base_resource_bounds
                    ));
                    $max_amount.0 -= 10;
                };
            }

            resource_overdraft!(base_resource_bounds.l1_gas.max_amount);

[dorimedini-starkware]

Reviewable status: 0 of 1 files reviewed, 4 unresolved discussions (waiting on @yoavGrs)

---

crates/blockifier/src/transaction/transactions_test.rs line 841 at r2 (raw file):

Previously, yoavGrs wrote…

No if

Done.

---

crates/blockifier/src/transaction/transactions_test.rs line 956 at r2 (raw file):

    );
    let account_contract_address = account_contract.get_instance_address(0);
    let default_args = invoke_tx_args! {

Done.

---

crates/blockifier/src/transaction/transactions_test.rs line 960 at r2 (raw file):

Previously, yoavGrs wrote…

Move under V1 Invoke.

Done.

---

crates/blockifier/src/transaction/transactions_test.rs line 1047 at r2 (raw file):

Previously, yoavGrs wrote…

Code duplication.
Move te logic into a macro

Done.

[github-actions]

Artifacts upload triggered. View details here

[dorimedini-starkware]

Reviewable status: 0 of 4 files reviewed, 5 unresolved discussions (waiting on @yoavGrs)

a discussion (no related file):
py side pr (change in error formatting, need to check nothing breaks)

---

[github-actions]

Artifacts upload triggered. View details here

[yoavGrs]

Reviewed 2 of 4 files at r4, 1 of 1 files at r5, all commit messages.
Reviewable status: 3 of 4 files reviewed, 2 unresolved discussions (waiting on @dorimedini-starkware)

---

crates/blockifier/src/transaction/transactions_test.rs line 904 at r5 (raw file):

                        && l1_data_max_amount == l1_data_gas
                        && l1_data_max_price == l1_data_gas_price
                );

Compare AllResourceBounds objects

Suggestion:

            ValidResourceBounds::AllResources(resources) => {
                assert_matches!(
                    tx_error,
                    TransactionExecutionError::TransactionPreValidationError(
                        TransactionPreValidationError::TransactionFeeError(
                            TransactionFeeError::ResourcesBoundsExceedBalance {
                                bounds: resource_bounds,
                                ..
                            }
                        )
                    )
                    if resources == resource_bounds
                );

[dorimedini-starkware]

Reviewable status: 3 of 4 files reviewed, 2 unresolved discussions (waiting on @yoavGrs)

---

crates/blockifier/src/transaction/transactions_test.rs line 904 at r5 (raw file):

Previously, yoavGrs wrote…

Compare AllResourceBounds objects

yes!! much better

[github-actions]

Artifacts upload triggered. View details here

[dorimedini-starkware]

Reviewable status: 3 of 4 files reviewed, 1 unresolved discussion (waiting on @yoavGrs)

a discussion (no related file):

Previously, dorimedini-starkware wrote…

py side pr (change in error formatting, need to check nothing breaks)

passes

---

[yoavGrs]

Reviewed 1 of 1 files at r6, all commit messages.
Reviewable status: complete! all files reviewed, all discussions resolved (waiting on @dorimedini-starkware)