chore(blockifier): dedup test util l1 handler creator

[ArniStarkware]

No description provided.

[reviewable-StarkWare]

This change is 

[ArniStarkware]

• test(blockifier): uncover an error in test_l1_handler in the case of transaction version three #2146
• chore(blockifier): dedup test util l1 handler creator #2131 👈 (View in Graphite)
• main

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

[github-actions]

Artifacts upload triggered. View details here

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 77.38%. Comparing base (e3165c4) to head (8b70d13).
Report is 488 commits behind head on main.

Additional details and impacted files

@@             Coverage Diff             @@
##             main    #2131       +/-   ##
===========================================
+ Coverage   40.10%   77.38%   +37.27%     
===========================================
  Files          26      108       +82     
  Lines        1895    13966    +12071     
  Branches     1895    13966    +12071     
===========================================
+ Hits          760    10807    +10047     
- Misses       1100     2698     +1598     
- Partials       35      461      +426     

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

---

🚨 Try these New Features:

• Flaky Tests Detection - Detect and resolve failed and flaky tests
• JS Bundle Analysis - Avoid shipping oversized bundles

[ArniStarkware]

Reviewable status: 0 of 3 files reviewed, all discussions resolved (waiting on @giladchase and @yair-starkware)

---

crates/starknet_api/src/test_utils/l1_handler.rs line 55 at r1 (raw file):

    // if tx_version != TransactionVersion::THREE {
    //     panic!("Unsupported transaction version: {:?}.", l1_handler_tx_args.version);
    // }

@giladchase - How critical is this validation? The test crates/blockifier/src/transaction/transactions_test.rs --- test_l1_handler fails with TransactionVersion::THREE.

This implies both Versions should be accounted for and tested for... I am working on this PR.

Code quote:

    // TODO(Arni): Re-enable this validation.
    // if tx_version != TransactionVersion::THREE {
    //     panic!("Unsupported transaction version: {:?}.", l1_handler_tx_args.version);
    // }

[github-actions]

Artifacts upload triggered. View details here

[giladchase]

Reviewable status: 0 of 3 files reviewed, all discussions resolved (waiting on @yair-starkware)

---

crates/starknet_api/src/test_utils/l1_handler.rs line 55 at r1 (raw file):

How critical is this validation

AFAIK we don't intend to support version < 3, if you heard differently then remove the validation, otherwise it's probably better to keep it and fix the source.

crates/blockifier/src/transaction/transactions_test.rs --- test_l1_handler fails with TransactionVersion::THREE.

Is this a typo? I don't understand how this can happen, the conditional raises only if transactionVersion is not THREE 🤔 .

[github-actions]

Artifacts upload triggered. View details here

[ArniStarkware]

Reviewable status: 0 of 3 files reviewed, all discussions resolved (waiting on @yair-starkware)

---

crates/starknet_api/src/test_utils/l1_handler.rs line 55 at r1 (raw file):

Previously, giladchase wrote…

How critical is this validation

AFAIK we don't intend to support version < 3, if you heard differently then remove the validation, otherwise it's probably better to keep it and fix the source.

crates/blockifier/src/transaction/transactions_test.rs --- test_l1_handler fails with TransactionVersion::THREE.

Is this a typo? I don't understand how this can happen, the conditional raises only if transactionVersion is not THREE 🤔 .

I will remove this assertion. This test util can be shared with the blockifier test utils, that uses - and assumes the version is 0.
You can see it in this PR.

This is not a typo: In #2146, there is one test case that fails. The test case is of Version::THREE, with USE kzg.

---

crates/blockifier/src/test_utils/l1_handler.rs line 24 at r2 (raw file):

        paid_fee_on_l1: l1_fee,
        ..Default::default()
    })

@giladchase see code dedup. (using the snapi's test util).
See the version zero.

Code quote:

    executable_l1_handler_tx(L1HandlerTxArgs {
        version: TransactionVersion::ZERO,
        contract_address,
        entry_point_selector: selector_from_name("l1_handler_set_value"),
        calldata,
        paid_fee_on_l1: l1_fee,
        ..Default::default()
    })

[github-actions]

Artifacts upload triggered. View details here

[giladchase]

Reviewable status: 0 of 3 files reviewed, all discussions resolved (waiting on @yair-starkware)

---

crates/starknet_api/src/test_utils/l1_handler.rs line 55 at r1 (raw file):

Previously, ArniStarkware (Arnon Hod) wrote…

I will remove this assertion. This test util can be shared with the blockifier test utils, that uses - and assumes the version is 0.
You can see it in this PR.

This is not a typo: In #2146, there is one test case that fails. The test case is of Version::THREE, with USE kzg.

Oh right, ya kill it with fire , tnx!

[giladchase]

Reviewable status: 0 of 3 files reviewed, all discussions resolved (waiting on @yair-starkware)

---

crates/starknet_api/src/test_utils/l1_handler.rs line 55 at r1 (raw file):

Previously, giladchase wrote…

Oh right, ya kill it with fire , tnx!

I mean delete it altogether, no need to reenable if it if the version3 assumption is incorrect

[ArniStarkware]

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