chore(blockifier): fix default strk l2 gas price test util

[ArniStarkware]

No description provided.

[ArniStarkware]

Warning

This pull request is not mergeable via GitHub because a downstack PR is open. Once all requirements are satisfied, merge this PR as a stack on Graphite.
Learn more

• chore(blockifier): fix default strk l2 gas price test util #2444 👈 (View in Graphite)
• chore(starknet_api): move gas price related test consts to starknet api #2443 : 1 other dependent PR (#2445 )
• main

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

[reviewable-StarkWare]

This change is 

[github-actions]

Artifacts upload workflows:

• Started at 2024-12-04T09:57:18Z
• Started at 2024-12-09T09:42:31Z

[ArniStarkware]

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

---

crates/blockifier/src/versioned_constants_test.rs line 213 at r1 (raw file):

    assert_eq!(expected_l2_gas_price.get(), result_l2_gas_price.get());
}

I'm not sure we want to add this test.

1. In theory, we are testing the convert function here. In practice, we test that the test util is correct.
2. Each time versioned constants change, we must change this value.
3. a. I am not sure it is interesting to keep the test util accurate for this conversion.

Code quote:

#[rstest]
#[case::strk(DEFAULT_STRK_L1_GAS_PRICE, DEFAULT_STRK_L2_GAS_PRICE)]
fn test_convert_l1_to_l2_gas_price_round_up(
    #[case] l1_gas_price: NonzeroGasPrice,
    #[case] expected_l2_gas_price: NonzeroGasPrice,
) {
    let result_l2_gas_price = NonzeroGasPrice::new(
        VersionedConstants::latest_constants()
            .convert_l1_to_l2_gas_price_round_up(l1_gas_price.into()),
    )
    .unwrap();

    assert_eq!(expected_l2_gas_price.get(), result_l2_gas_price.get());
}

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Please upload report for BASE (arni/test_utils/gas_prices/move_consts_to_snapi@cbd8129). Learn more about missing BASE report.

Additional details and impacted files

@@                                Coverage Diff                                 @@
##             arni/test_utils/gas_prices/move_consts_to_snapi    #2444   +/-   ##
==================================================================================
  Coverage                                                   ?   51.56%           
==================================================================================
  Files                                                      ?      291           
  Lines                                                      ?    33227           
  Branches                                                   ?    33227           
==================================================================================
  Hits                                                       ?    17133           
  Misses                                                     ?    14951           
  Partials                                                   ?     1143           

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

[github-actions]

Benchmark movements:
tree_computation_flow performance regressed!
tree_computation_flow time: [34.585 ms 34.879 ms 35.216 ms]
change: [+1.0484% +1.9830% +2.9618%] (p = 0.00 < 0.05)
Performance has regressed.
Found 20 outliers among 100 measurements (20.00%)
4 (4.00%) high mild
16 (16.00%) high severe

[alonh5]

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

---

crates/blockifier/src/versioned_constants_test.rs line 213 at r1 (raw file):

Previously, ArniStarkware (Arnon Hod) wrote…

I'm not sure we want to add this test.

1. In theory, we are testing the convert function here. In practice, we test that the test util is correct.
2. Each time versioned constants change, we must change this value.
3. a. I am not sure it is interesting to keep the test util accurate for this conversion.

When you say test util, do you mean the DEFAULT_STRK_L2_GAS_PRICE constant?
I agree, I think we can remove this test and restore the value. In that case this PR can be closed right?

[ArniStarkware]

Closed due to no need.