fix(blockifier): enforce fee for allresourcebounds when l1 gas is 0

[aner-starkware]

This change is 

[codecov]

Codecov Report

Attention: Patch coverage is 84.37500% with 10 lines in your changes missing coverage. Please review.

Project coverage is 75.52%. Comparing base (b0cfe82) to head (641d5ce).
Report is 49 commits behind head on main.

Files with missing lines	Patch %	Lines
crates/blockifier/src/versioned_constants.rs	52.94%	8 Missing ⚠️
crates/blockifier/src/transaction/objects.rs	92.00%	2 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #846      +/-   ##
==========================================
+ Coverage   74.18%   75.52%   +1.33%     
==========================================
  Files         359       87     -272     
  Lines       36240    11120   -25120     
  Branches    36240    11120   -25120     
==========================================
- Hits        26886     8398   -18488     
+ Misses       7220     2311    -4909     
+ Partials     2134      411    -1723     

Flag	Coverage Δ
	75.52% <84.37%> (+1.33%)	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

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

[nimrod-starkware]

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

[dorimedini-starkware]

Reviewed 1 of 1 files at r1, all commit messages.
Reviewable status: all files reviewed, 2 unresolved discussions (waiting on @aner-starkware, @nimrod-starkware, and @TzahiTaub)

---

crates/blockifier/src/transaction/objects.rs line 120 at r1 (raw file):

                        + u128::from(l1_data_gas.max_amount) * l1_data_gas.max_price_per_unit
                        > 0
                }

I think ValidResourceBounds should have a max_possible_fee method that returns the scalar product of the amounts and the prices; this will replace the two match arms here

Code quote:

                ValidResourceBounds::L1Gas(l1_bounds) => {
                    let max_amount: u128 = l1_bounds.max_amount.into();
                    max_amount * l1_bounds.max_price_per_unit > 0
                }
                ValidResourceBounds::AllResources(AllResourceBounds {
                    l1_gas,
                    l2_gas,
                    l1_data_gas,
                }) => {
                    u128::from(l1_gas.max_amount) * l1_gas.max_price_per_unit
                        + u128::from(l2_gas.max_amount) * l2_gas.max_price_per_unit
                        + u128::from(l1_data_gas.max_amount) * l1_data_gas.max_price_per_unit
                        > 0
                }

---

crates/blockifier/src/transaction/objects.rs line 121 at r1 (raw file):

                        > 0
                }
            },

flatten the match please (or, see comment above)

Suggestion:

            TransactionInfo::Current(ValidResourceBounds::L1Gas(l1_bounds)) =>  {
                let max_amount: u128 = l1_bounds.max_amount.into();
                max_amount * l1_bounds.max_price_per_unit > 0
            }
            TransactionInfo::Current(ValidResourceBounds::AllResources(AllResourceBounds {
                    l1_gas,
                    l2_gas,
                    l1_data_gas,
            })) => {
                u128::from(l1_gas.max_amount) * l1_gas.max_price_per_unit
                    + u128::from(l2_gas.max_amount) * l2_gas.max_price_per_unit
                    + u128::from(l1_data_gas.max_amount) * l1_data_gas.max_price_per_unit
                    > 0
            },

[aner-starkware]

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

---

crates/blockifier/src/transaction/objects.rs line 120 at r1 (raw file):

Previously, dorimedini-starkware wrote…

I think ValidResourceBounds should have a max_possible_fee method that returns the scalar product of the amounts and the prices; this will replace the two match arms here

Done.

---

crates/blockifier/src/transaction/objects.rs line 121 at r1 (raw file):

Previously, dorimedini-starkware wrote…

flatten the match please (or, see comment above)

Took the above option.

[github-actions]

Benchmark movements:
tree_computation_flow performance improved 😺
tree_computation_flow time: [66.775 ms 66.876 ms 67.006 ms]
change: [-9.0514% -5.8887% -3.0941%] (p = 0.00 < 0.05)
Performance has improved.
Found 3 outliers among 100 measurements (3.00%)
3 (3.00%) high severe

[github-actions]

Benchmark movements:
tree_computation_flow performance improved 😺
tree_computation_flow time: [66.527 ms 66.982 ms 67.648 ms]
change: [-8.9707% -5.5627% -2.2541%] (p = 0.00 < 0.05)
Performance has improved.
Found 6 outliers among 100 measurements (6.00%)
6 (6.00%) high severe

[dorimedini-starkware]

Reviewed 2 of 2 files at r3, all commit messages.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @aner-starkware, @nimrod-starkware, and @TzahiTaub)

---

crates/starknet_api/src/transaction.rs line 998 at r3 (raw file):

            }
        }
    }

wrap in this type, for clarity

Suggestion:

    pub fn max_possible_fee(&self) -> Fee {
        Fee(match self {
            ValidResourceBounds::L1Gas(l1_bounds) => {
                let max_amount: u128 = l1_bounds.max_amount.into();
                max_amount * l1_bounds.max_price_per_unit
            }
            ValidResourceBounds::AllResources(AllResourceBounds {
                l1_gas,
                l2_gas,
                l1_data_gas,
            }) => {
                u128::from(l1_gas.max_amount) * l1_gas.max_price_per_unit
                    + u128::from(l2_gas.max_amount) * l2_gas.max_price_per_unit
                    + u128::from(l1_data_gas.max_amount) * l1_data_gas.max_price_per_unit
            }
        })
    }

[aner-starkware]

Reviewable status: 0 of 2 files reviewed, 1 unresolved discussion (waiting on @dorimedini-starkware, @nimrod-starkware, and @TzahiTaub)

---

crates/starknet_api/src/transaction.rs line 998 at r3 (raw file):

Previously, dorimedini-starkware wrote…

wrap in this type, for clarity

Done. Though the name fn name is probably clear enough ;)

[dorimedini-starkware]

Reviewed 2 of 2 files at r4, all commit messages.
Reviewable status: complete! all files reviewed, all discussions resolved (waiting on @nimrod-starkware and @TzahiTaub)

---

crates/starknet_api/src/transaction.rs line 998 at r3 (raw file):

Previously, aner-starkware wrote…

Done. Though the name fn name is probably clear enough ;)

it's clear, but type enforcement is better.
it enforces that we don't perform addition/subtraction on different numeric types, for example (basic physics, can't add meters to seconds :) )

[dorimedini-starkware]

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