chore(blockifier): move GasPrices to snapi #2245

ArniStarkware · 2024-11-24T12:09:41Z

No description provided.

reviewable-StarkWare · 2024-11-24T12:09:48Z

This change is

ArniStarkware · 2024-11-24T12:09:53Z

chore(blockifier): move block info to starknet api #2246 : 2 dependent PRs (#2230 , #2237 )
chore(blockifier): move GasPrices to snapi #2245 👈 (View in Graphite)
feat(blockifier): derive serialize for block info #2244
chore(blockifier): rename the getters for the gas prices struct #2269
chore(blockifier): separate create and validate logic on gas prices #2243
main

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

github-actions · 2024-11-24T12:15:39Z

Artifacts upload workflows:

codecov · 2024-11-24T12:31:17Z

Codecov Report

Attention: Patch coverage is 94.73684% with 3 lines in your changes missing coverage. Please review.

Project coverage is 53.32%. Comparing base (e3165c4) to head (4444d1b).
Report is 618 commits behind head on main.

Files with missing lines	Patch %	Lines
crates/blockifier/src/blockifier/block.rs	94.59%	2 Missing ⚠️
crates/starknet_api/src/block.rs	93.75%	1 Missing ⚠️

Additional details and impacted files

@@             Coverage Diff             @@
##             main    #2245       +/-   ##
===========================================
+ Coverage   40.10%   53.32%   +13.21%     
===========================================
  Files          26      302      +276     
  Lines        1895    34106    +32211     
  Branches     1895    34106    +32211     
===========================================
+ Hits          760    18187    +17427     
- Misses       1100    14637    +13537     
- Partials       35     1282     +1247

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

github-actions · 2024-11-24T12:32:09Z

Benchmark movements:
tree_computation_flow performance regressed!
tree_computation_flow time: [34.504 ms 34.994 ms 35.569 ms]
change: [+1.6275% +2.9988% +4.9672%] (p = 0.00 < 0.05)
Performance has regressed.
Found 9 outliers among 100 measurements (9.00%)
9 (9.00%) high severe

alonh5

Reviewed 7 of 10 files at r1, 1 of 3 files at r2, 2 of 2 files at r3, all commit messages.
Reviewable status: all files reviewed, 3 unresolved discussions (waiting on @aner-starkware, @ArniStarkware, and @dorimedini-starkware)

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

    let expected_eth_l2_gas_price = VersionedConstants::latest_constants()
        .convert_l1_to_l2_gas_price_round_up(
            gas_prices.gas_price_vector(&FeeType::Eth).l1_gas_price.into(),

When the call is trivial, it's better to access the field directly instead of using this method. (copied comment from previous PR).

Code quote:

gas_price_vector(&FeeType::Eth)

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

}

pub fn gas_prices(

Suggestion:

validated_gas_prices

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

#[derive(Clone, Copy, Hash, EnumIter, Eq, PartialEq)]
pub enum FeeType {

This isn't a tx field, it's only used in gas prices, right? Wouldn't it be better in the same file with gas prices?

ArniStarkware · 2024-11-25T14:46:25Z

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

}

pub fn gas_prices(

Done.

alonh5

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

github-actions · 2024-11-28T07:18:23Z

Benchmark movements:
tree_computation_flow performance improved 😺
tree_computation_flow time: [33.870 ms 33.905 ms 33.943 ms]
change: [-6.8623% -5.3657% -4.0491%] (p = 0.00 < 0.05)
Performance has improved.
Found 8 outliers among 100 measurements (8.00%)
7 (7.00%) high mild
1 (1.00%) high severe

full_committer_flow performance improved 😺
full_committer_flow time: [30.298 ms 30.519 ms 30.929 ms]
change: [-4.1438% -3.3774% -1.5684%] (p = 0.00 < 0.05)
Performance has improved.
Found 7 outliers among 100 measurements (7.00%)
5 (5.00%) high mild
2 (2.00%) high severe

alonh5

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

github-actions · 2024-11-28T09:26:49Z

Benchmark movements:
tree_computation_flow performance improved 😺
tree_computation_flow time: [33.858 ms 33.908 ms 33.967 ms]
change: [-5.2011% -3.7148% -2.4141%] (p = 0.00 < 0.05)
Performance has improved.
Found 6 outliers among 100 measurements (6.00%)
2 (2.00%) high mild
4 (4.00%) high severe

full_committer_flow performance improved 😺
full_committer_flow time: [30.364 ms 30.409 ms 30.462 ms]
change: [-2.4239% -2.1967% -1.9667%] (p = 0.00 < 0.05)
Performance has improved.
Found 3 outliers among 100 measurements (3.00%)
1 (1.00%) high mild
2 (2.00%) high severe

github-actions · 2024-11-28T09:51:31Z

Benchmark movements:
full_committer_flow performance improved 😺
full_committer_flow time: [30.313 ms 30.370 ms 30.433 ms]
change: [-2.1262% -1.9083% -1.6552%] (p = 0.00 < 0.05)
Performance has improved.
Found 7 outliers among 100 measurements (7.00%)
4 (4.00%) high mild
3 (3.00%) high severe

github-actions · 2024-11-28T10:08:04Z

Benchmark movements:
tree_computation_flow performance improved 😺
tree_computation_flow time: [34.061 ms 34.113 ms 34.178 ms]
change: [-6.0905% -4.5395% -3.1440%] (p = 0.00 < 0.05)
Performance has improved.
Found 7 outliers among 100 measurements (7.00%)
1 (1.00%) high mild
6 (6.00%) high severe

full_committer_flow performance improved 😺
full_committer_flow time: [30.502 ms 30.613 ms 30.758 ms]
change: [-2.2568% -1.8885% -1.4481%] (p = 0.00 < 0.05)
Performance has improved.
Found 7 outliers among 100 measurements (7.00%)
4 (4.00%) high mild
3 (3.00%) high severe

ArniStarkware · 2024-11-28T10:18:04Z

Merge activity

Nov 28, 5:18 AM EST: A user merged this pull request with Graphite.

This was referenced Nov 24, 2024

chore(blockifier): separate create and validate logic on gas prices #2243

Merged

feat(blockifier): derive serialize for block info #2244

Merged

chore(blockifier): move block info to starknet api #2246

Merged

ArniStarkware requested review from alonh5, aner-starkware and dorimedini-starkware November 24, 2024 12:10

ArniStarkware marked this pull request as ready for review November 24, 2024 12:10

ArniStarkware force-pushed the arni/block_info/derive_serialize branch from 71289c1 to 10c393b Compare November 24, 2024 12:21

ArniStarkware force-pushed the arni/gas_prices/move_to_snapi branch from bfa1ba1 to 6a958f8 Compare November 24, 2024 12:21

ArniStarkware force-pushed the arni/block_info/derive_serialize branch from 10c393b to 73a5586 Compare November 24, 2024 14:49

ArniStarkware force-pushed the arni/gas_prices/move_to_snapi branch from 6a958f8 to f5c93b6 Compare November 24, 2024 14:49

ArniStarkware force-pushed the arni/block_info/derive_serialize branch from 73a5586 to 68c849c Compare November 25, 2024 11:43

ArniStarkware force-pushed the arni/gas_prices/move_to_snapi branch from f5c93b6 to f63bc29 Compare November 25, 2024 11:43

ArniStarkware mentioned this pull request Nov 25, 2024

chore(blockifier): rename the getters for the gas prices struct #2269

Merged

ArniStarkware force-pushed the arni/block_info/derive_serialize branch from 68c849c to 2038d5b Compare November 25, 2024 11:49

ArniStarkware force-pushed the arni/gas_prices/move_to_snapi branch from f63bc29 to d0e9ff5 Compare November 25, 2024 11:49

alonh5 requested changes Nov 25, 2024

View reviewed changes

ArniStarkware force-pushed the arni/block_info/derive_serialize branch from 2038d5b to 47f6640 Compare November 25, 2024 14:44

ArniStarkware force-pushed the arni/gas_prices/move_to_snapi branch from d0e9ff5 to 2a0a57d Compare November 25, 2024 14:44

alonh5 approved these changes Nov 27, 2024

View reviewed changes

ArniStarkware force-pushed the arni/block_info/derive_serialize branch from a3c1d51 to 0fa29fe Compare November 28, 2024 07:06

ArniStarkware force-pushed the arni/gas_prices/move_to_snapi branch from 0b66a48 to 33cef26 Compare November 28, 2024 07:06

ArniStarkware requested a review from alonh5 November 28, 2024 07:59

alonh5 approved these changes Nov 28, 2024

View reviewed changes

ArniStarkware force-pushed the arni/block_info/derive_serialize branch from 0fa29fe to d128a84 Compare November 28, 2024 09:16

ArniStarkware force-pushed the arni/gas_prices/move_to_snapi branch from 33cef26 to fdf402c Compare November 28, 2024 09:17

ArniStarkware force-pushed the arni/block_info/derive_serialize branch 2 times, most recently from 6acf4b7 to 61e2b40 Compare November 28, 2024 09:40

ArniStarkware force-pushed the arni/gas_prices/move_to_snapi branch from fdf402c to 3d7b232 Compare November 28, 2024 09:40

ArniStarkware changed the base branch from arni/block_info/derive_serialize to graphite-base/2245 November 28, 2024 09:56

ArniStarkware force-pushed the graphite-base/2245 branch from 61e2b40 to 680df8d Compare November 28, 2024 09:56

ArniStarkware force-pushed the arni/gas_prices/move_to_snapi branch from 3d7b232 to 7669ae6 Compare November 28, 2024 09:56

ArniStarkware changed the base branch from graphite-base/2245 to main November 28, 2024 09:57

chore(blockifier): move GasPrices to snapi

4444d1b

ArniStarkware force-pushed the arni/gas_prices/move_to_snapi branch from 7669ae6 to 4444d1b Compare November 28, 2024 09:57

ArniStarkware merged commit d7198bd into main Nov 28, 2024
23 checks passed

ArniStarkware deleted the arni/gas_prices/move_to_snapi branch November 28, 2024 13:46

github-actions bot locked and limited conversation to collaborators Dec 1, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

chore(blockifier): move GasPrices to snapi #2245

chore(blockifier): move GasPrices to snapi #2245

ArniStarkware commented Nov 24, 2024

reviewable-StarkWare commented Nov 24, 2024

ArniStarkware commented Nov 24, 2024 •

edited

Loading

github-actions bot commented Nov 24, 2024 •

edited

Loading

codecov bot commented Nov 24, 2024 •

edited

Loading

github-actions bot commented Nov 24, 2024

alonh5 left a comment

ArniStarkware commented Nov 25, 2024

alonh5 left a comment

github-actions bot commented Nov 28, 2024

alonh5 left a comment

github-actions bot commented Nov 28, 2024

github-actions bot commented Nov 28, 2024

github-actions bot commented Nov 28, 2024

ArniStarkware commented Nov 28, 2024

chore(blockifier): move GasPrices to snapi #2245

chore(blockifier): move GasPrices to snapi #2245

Conversation

ArniStarkware commented Nov 24, 2024

reviewable-StarkWare commented Nov 24, 2024

ArniStarkware commented Nov 24, 2024 • edited Loading

github-actions bot commented Nov 24, 2024 • edited Loading

codecov bot commented Nov 24, 2024 • edited Loading

Codecov Report

github-actions bot commented Nov 24, 2024

alonh5 left a comment

Choose a reason for hiding this comment

ArniStarkware commented Nov 25, 2024

alonh5 left a comment

Choose a reason for hiding this comment

github-actions bot commented Nov 28, 2024

alonh5 left a comment

Choose a reason for hiding this comment

github-actions bot commented Nov 28, 2024

github-actions bot commented Nov 28, 2024

github-actions bot commented Nov 28, 2024

ArniStarkware commented Nov 28, 2024

Merge activity

ArniStarkware commented Nov 24, 2024 •

edited

Loading

github-actions bot commented Nov 24, 2024 •

edited

Loading

codecov bot commented Nov 24, 2024 •

edited

Loading