build(blockifier): add l2_gas_price to blockinfo

[aner-starkware]

This change is 

[codecov]

Codecov Report

Attention: Patch coverage is 65.38462% with 18 lines in your changes missing coverage. Please review.

Project coverage is 76.40%. Comparing base (72fa342) to head (a03834c).
Report is 4 commits behind head on main.

Files	Patch %	Lines
crates/native_blockifier/src/py_state_diff.rs	25.00%	14 Missing and 4 partials ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #463      +/-   ##
==========================================
- Coverage   76.42%   76.40%   -0.03%     
==========================================
  Files         349      349              
  Lines       36936    36965      +29     
  Branches    36936    36965      +29     
==========================================
+ Hits        28228    28242      +14     
- Misses       6384     6396      +12     
- Partials     2324     2327       +3     

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

[yair-starkware]

Reviewable status: 0 of 18 files reviewed, 1 unresolved discussion (waiting on @aner-starkware and @ShahakShama)

---

crates/papyrus_execution/src/lib.rs line 363 at r2 (raw file):

            NonZeroU128::new(l1_data_gas_price.price_in_fri.0).unwrap_or(NonZeroU128::MIN),
            NonZeroU128::MIN,
            NonZeroU128::MIN,

I think these should come from the pending_data/block_header same way the l1 gas is

Code quote:

            NonZeroU128::MIN,
            NonZeroU128::MIN,

[aner-starkware]

Reviewable status: 0 of 18 files reviewed, 1 unresolved discussion (waiting on @ShahakShama)

---

crates/papyrus_execution/src/lib.rs line 363 at r2 (raw file):

Previously, yair-starkware (Yair) wrote…

I think these should come from the pending_data/block_header same way the l1 gas is

@ShahakShama told me to leave a TODO for this for now (see the first revision - it doesn't compile)

[dorimedini-starkware]

Reviewed 3 of 18 files at r1, 1 of 15 files at r2, 2 of 2 files at r3, all commit messages.
Reviewable status: 6 of 18 files reviewed, 5 unresolved discussions (waiting on @aner-starkware, @ShahakShama, and @TzahiTaub)

---

crates/blockifier/src/blockifier/block.rs line 55 at r3 (raw file):

                .expect("L1 to L2 price conversion error (Rust side).")
        {
            warn!("eth_l2_gas_price does not match expected!")

I suggest you output some numbers here: expected price, actual price, and the ratio
X2 (also below)

Code quote:

warn!("eth_l2_gas_price does not match expected!")

---

crates/gateway/src/rpc_objects.rs line 94 at r3 (raw file):

                parse_gas_price(self.l1_data_gas_price.price_in_fri)?,
                NonZeroU128::MIN,
                NonZeroU128::MIN,

shouldn't this be part of the block header @ShahakShama ?

Code quote:

                parse_gas_price(self.l1_data_gas_price.price_in_fri)?,
                NonZeroU128::MIN,
                NonZeroU128::MIN,

---

crates/native_blockifier/src/py_state_diff.rs line 216 at r3 (raw file):

                block_info.l2_gas_price.price_in_wei.try_into().map_err(|_| {
                    NativeBlockifierInputError::InvalidNativeBlockifierInputError(
                        InvalidNativeBlockifierInputError::InvalidGasPriceWei(

add a variant for this

Suggestion:

InvalidL2GasPriceWei

---

crates/native_blockifier/src/py_state_diff.rs line 223 at r3 (raw file):

                block_info.l2_gas_price.price_in_fri.try_into().map_err(|_| {
                    NativeBlockifierInputError::InvalidNativeBlockifierInputError(
                        InvalidNativeBlockifierInputError::InvalidGasPriceFri(

ditto

Suggestion:

InvalidL2GasPriceFri

[dorimedini-starkware]

Reviewed 12 of 15 files at r2.
Reviewable status: all files reviewed, 5 unresolved discussions (waiting on @aner-starkware, @ShahakShama, and @TzahiTaub)

[aner-starkware]

Reviewable status: 14 of 18 files reviewed, 6 unresolved discussions (waiting on @dorimedini-starkware, @ShahakShama, and @TzahiTaub)

a discussion (no related file):
Python side: https://reviewable.io/reviews/starkware-industries/starkware/35733#-

---

---

crates/blockifier/src/blockifier/block.rs line 55 at r3 (raw file):

Previously, dorimedini-starkware wrote…

I suggest you output some numbers here: expected price, actual price, and the ratio
X2 (also below)

Why print the ratio? Did the rest.

[dorimedini-starkware]

Reviewed 4 of 4 files at r4, 1 of 1 files at r5, all commit messages.
Reviewable status: all files reviewed, 11 unresolved discussions (waiting on @aner-starkware, @ShahakShama, and @TzahiTaub)

---

crates/blockifier/src/blockifier/block.rs line 55 at r5 (raw file):

                max_recursion_depth: 0,
                versioned_constants_base_overrides: None,
            })

why this function...? you are not passing overrides from the caller, why call this overriding method?

Code quote:

            VersionedConstants::get_versioned_constants(VersionedConstantsOverrides {
                validate_max_n_steps: 0,
                max_recursion_depth: 0,
                versioned_constants_base_overrides: None,
            })

---

crates/blockifier/src/blockifier/block.rs line 60 at r5 (raw file):

            warn!(
                "eth_l2_gas_price does not match expected! eth_l2_gas_price:{}, expected:{}.",
                eth_l2_gas_price, expected_eth_l2_gas_price

can you inline the values? more readable

Suggestion:

                "eth_l2_gas_price does not match expected! eth_l2_gas_price: {eth_l2_gas_price}, expected: {expected}."

---

crates/blockifier/src/blockifier/block.rs line 68 at r5 (raw file):

                max_recursion_depth: 0,
                versioned_constants_base_overrides: None,
            })

same Q

Code quote:

            VersionedConstants::get_versioned_constants(VersionedConstantsOverrides {
                validate_max_n_steps: 0,
                max_recursion_depth: 0,
                versioned_constants_base_overrides: None,
            })

---

crates/blockifier/src/blockifier/block.rs line 73 at r5 (raw file):

            warn!(
                "strk_l2_gas_price does not match expected! strk_l2_gas_price:{}, expected:{}.",
                strk_l2_gas_price, expected_strk_l2_gas_price

inline printed values (see above)

Code quote:

                "strk_l2_gas_price does not match expected! strk_l2_gas_price:{}, expected:{}.",
                strk_l2_gas_price, expected_strk_l2_gas_price

---

crates/blockifier/src/fee/fee_test.rs line 152 at r5 (raw file):

        .try_into()
        .unwrap(),
    );

why use this getter with no overrides, instead of latest_constants or something without args?

Code quote:

        VersionedConstants::get_versioned_constants(VersionedConstantsOverrides {
            validate_max_n_steps: 0,
            max_recursion_depth: 0,
            versioned_constants_base_overrides: None,
        })
        .l1_to_l2_gas_price_conversion(DEFAULT_ETH_L1_GAS_PRICE)
        .try_into()
        .unwrap(),
        VersionedConstants::get_versioned_constants(VersionedConstantsOverrides {
            validate_max_n_steps: 0,
            max_recursion_depth: 0,
            versioned_constants_base_overrides: None,
        })
        .l1_to_l2_gas_price_conversion(gas_price)
        .try_into()
        .unwrap(),
    );

---

crates/blockifier/src/test_utils/struct_impls.rs line 180 at r5 (raw file):

                    versioned_constants_base_overrides: None,
                })
                .l1_to_l2_gas_price_conversion(DEFAULT_STRK_L1_GAS_PRICE)

use latest_constants - it's equivalent, without the explicit overrides

Code quote:

                VersionedConstants::get_versioned_constants(VersionedConstantsOverrides {
                    validate_max_n_steps: 0,
                    max_recursion_depth: 0,
                    versioned_constants_base_overrides: None,
                })
                .l1_to_l2_gas_price_conversion(DEFAULT_ETH_L1_GAS_PRICE)
                .try_into()
                .unwrap(),
                VersionedConstants::get_versioned_constants(VersionedConstantsOverrides {
                    validate_max_n_steps: 0,
                    max_recursion_depth: 0,
                    versioned_constants_base_overrides: None,
                })
                .l1_to_l2_gas_price_conversion(DEFAULT_STRK_L1_GAS_PRICE)

---

crates/native_blockifier/src/py_state_diff.rs line 179 at r5 (raw file):

                    },
                )
                .l1_to_l2_gas_price_conversion(DEFAULT_STRK_L1_GAS_PRICE),

use latest_constants

Code quote:

                price_in_wei: VersionedConstants::get_versioned_constants(
                    VersionedConstantsOverrides {
                        validate_max_n_steps: 0,
                        max_recursion_depth: 0,
                        versioned_constants_base_overrides: None,
                    },
                )
                .l1_to_l2_gas_price_conversion(DEFAULT_ETH_L1_GAS_PRICE),
                price_in_fri: VersionedConstants::get_versioned_constants(
                    VersionedConstantsOverrides {
                        validate_max_n_steps: 0,
                        max_recursion_depth: 0,
                        versioned_constants_base_overrides: None,
                    },
                )
                .l1_to_l2_gas_price_conversion(DEFAULT_STRK_L1_GAS_PRICE),

[aner-starkware]

Reviewable status: 15 of 19 files reviewed, 11 unresolved discussions (waiting on @dorimedini-starkware, @ShahakShama, and @TzahiTaub)

---

crates/blockifier/src/blockifier/block.rs line 55 at r5 (raw file):

Previously, dorimedini-starkware wrote…

why this function...? you are not passing overrides from the caller, why call this overriding method?

Sorry, misunderstanding on my part - I thought you asked for this change.

---

crates/blockifier/src/blockifier/block.rs line 60 at r5 (raw file):

Previously, dorimedini-starkware wrote…

can you inline the values? more readable

Done.

---

crates/blockifier/src/blockifier/block.rs line 68 at r5 (raw file):

Previously, dorimedini-starkware wrote…

same Q

Done.

---

crates/blockifier/src/blockifier/block.rs line 73 at r5 (raw file):

Previously, dorimedini-starkware wrote…

inline printed values (see above)

Done.

---

crates/blockifier/src/fee/fee_test.rs line 152 at r5 (raw file):

Previously, dorimedini-starkware wrote…

why use this getter with no overrides, instead of latest_constants or something without args?

Sorry, misunderstanding on my part - I thought you asked for this change.

---

crates/blockifier/src/test_utils/struct_impls.rs line 180 at r5 (raw file):

Previously, dorimedini-starkware wrote…

use latest_constants - it's equivalent, without the explicit overrides

Done.

---

crates/native_blockifier/src/py_state_diff.rs line 216 at r3 (raw file):

Previously, dorimedini-starkware wrote…

add a variant for this

Done.

---

crates/native_blockifier/src/py_state_diff.rs line 223 at r3 (raw file):

Previously, dorimedini-starkware wrote…

ditto

Done.

---

crates/native_blockifier/src/py_state_diff.rs line 179 at r5 (raw file):

Previously, dorimedini-starkware wrote…

use latest_constants

Done.

[dorimedini-starkware]

Reviewed 3 of 4 files at r6, 1 of 1 files at r7, all commit messages.
Reviewable status: all files reviewed, 2 unresolved discussions (waiting on @aner-starkware, @ShahakShama, and @TzahiTaub)

[dorimedini-starkware]

Reviewable status: all files reviewed, 2 unresolved discussions (waiting on @aner-starkware, @ShahakShama, and @TzahiTaub)

---

crates/gateway/src/rpc_objects.rs line 94 at r3 (raw file):

Previously, dorimedini-starkware wrote…

shouldn't this be part of the block header @ShahakShama ?

add the TODO here as well? (guessing it's related to the below discussion)

[TzahiTaub]

Reviewed 1 of 15 files at r2, 1 of 1 files at r5, 3 of 4 files at r6.
Reviewable status: 15 of 19 files reviewed, 2 unresolved discussions (waiting on @aner-starkware, @dorimedini-starkware, and @ShahakShama)

[TzahiTaub]

crates/gateway/src/rpc_objects.rs line 94 at r3 (raw file):

Previously, dorimedini-starkware wrote…

add the TODO here as well? (guessing it's related to the below discussion)

Added. This is a part of the mempool project. @elintul FYI

[dorimedini-starkware]

Reviewed 4 of 4 files at r8, all commit messages.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @ShahakShama)

[dorimedini-starkware]

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