Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

chore(starknet_batcher): set use_kzg_da flag in build block input #2345

Conversation

ArniStarkware
Copy link
Contributor

No description provided.

@reviewable-StarkWare
Copy link

This change is Reviewable

Copy link

codecov bot commented Nov 28, 2024

Codecov Report

Attention: Patch coverage is 66.66667% with 1 line in your changes missing coverage. Please review.

Project coverage is 27.34%. Comparing base (e3165c4) to head (ef7066c).
Report is 676 commits behind head on main.

Files with missing lines Patch % Lines
crates/starknet_api/src/block.rs 0.00% 1 Missing ⚠️
Additional details and impacted files
@@             Coverage Diff             @@
##             main    #2345       +/-   ##
===========================================
- Coverage   40.10%   27.34%   -12.77%     
===========================================
  Files          26      119       +93     
  Lines        1895    14036    +12141     
  Branches     1895    14036    +12141     
===========================================
+ Hits          760     3838     +3078     
- Misses       1100     9863     +8763     
- Partials       35      335      +300     

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

@ArniStarkware ArniStarkware force-pushed the arni/batcher/block_builder_factory/set_block_timestamp branch from a21cac0 to 3bb7ed9 Compare November 28, 2024 15:34
@ArniStarkware ArniStarkware force-pushed the arni/batcher/block_builder_factory/set_use_kzg_da_in_build_block_input branch from 0b1faae to ef924ab Compare November 28, 2024 15:34
Copy link
Collaborator

@alonh5 alonh5 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

:lgtm:

Reviewed 2 of 2 files at r1, all commit messages.
Reviewable status: :shipit: complete! all files reviewed, all discussions resolved (waiting on @ArniStarkware)

@ArniStarkware ArniStarkware force-pushed the arni/batcher/block_builder_factory/set_block_timestamp branch from 3bb7ed9 to c65933e Compare December 1, 2024 12:43
@ArniStarkware ArniStarkware changed the title chore(batcher): set use_kzg_da flag in build block input chore(starknet_batcher): set use_kzg_da flag in build block input Dec 1, 2024
@ArniStarkware ArniStarkware force-pushed the arni/batcher/block_builder_factory/set_use_kzg_da_in_build_block_input branch 2 times, most recently from f3ada2d to f84a046 Compare December 1, 2024 13:52
@ArniStarkware ArniStarkware force-pushed the arni/batcher/block_builder_factory/set_block_timestamp branch from c65933e to 5e58a58 Compare December 2, 2024 09:23
@ArniStarkware ArniStarkware force-pushed the arni/batcher/block_builder_factory/set_use_kzg_da_in_build_block_input branch from f84a046 to 4299eb1 Compare December 2, 2024 09:23
@ArniStarkware ArniStarkware force-pushed the arni/batcher/block_builder_factory/set_block_timestamp branch from 5e58a58 to 83b3f3e Compare December 2, 2024 13:23
@ArniStarkware ArniStarkware force-pushed the arni/batcher/block_builder_factory/set_use_kzg_da_in_build_block_input branch from 4299eb1 to afd4c28 Compare December 2, 2024 13:24
@ArniStarkware ArniStarkware force-pushed the arni/batcher/block_builder_factory/set_block_timestamp branch from 83b3f3e to 3d7428d Compare December 2, 2024 14:00
@ArniStarkware ArniStarkware force-pushed the arni/batcher/block_builder_factory/set_use_kzg_da_in_build_block_input branch from afd4c28 to 17c0fe1 Compare December 2, 2024 14:01
@ArniStarkware ArniStarkware force-pushed the arni/batcher/block_builder_factory/set_block_timestamp branch from 3d7428d to 5e5126a Compare December 2, 2024 14:33
@ArniStarkware ArniStarkware force-pushed the arni/batcher/block_builder_factory/set_use_kzg_da_in_build_block_input branch from 17c0fe1 to 0b3f15b Compare December 2, 2024 14:33
@ArniStarkware ArniStarkware changed the base branch from arni/batcher/block_builder_factory/set_block_timestamp to arni/batcher/block_builder_factory/set_gas_prices December 2, 2024 14:33
@ArniStarkware ArniStarkware force-pushed the arni/batcher/block_builder_factory/set_gas_prices branch 2 times, most recently from 2a4a5b5 to 6cfb9f4 Compare December 2, 2024 15:05
@ArniStarkware ArniStarkware force-pushed the arni/batcher/block_builder_factory/set_use_kzg_da_in_build_block_input branch from 0b3f15b to 4d6fbbd Compare December 2, 2024 15:05
@ArniStarkware ArniStarkware force-pushed the arni/batcher/block_builder_factory/set_gas_prices branch from 6cfb9f4 to 7426fef Compare December 2, 2024 15:27
@ArniStarkware ArniStarkware force-pushed the arni/batcher/block_builder_factory/set_use_kzg_da_in_build_block_input branch from 4d6fbbd to 38e59dc Compare December 2, 2024 15:27
@ArniStarkware ArniStarkware force-pushed the arni/batcher/block_builder_factory/set_gas_prices branch from 7426fef to c2e4423 Compare December 3, 2024 10:59
@ArniStarkware ArniStarkware force-pushed the arni/batcher/block_builder_factory/set_use_kzg_da_in_build_block_input branch from 38e59dc to fd84f65 Compare December 3, 2024 10:59
Copy link
Contributor

@matan-starkware matan-starkware left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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


crates/sequencing/papyrus_consensus_orchestrator/src/sequencer_consensus_context.rs line 175 at r4 (raw file):

                    now.timestamp().try_into().expect("Failed to convert timestamp"),
                ),
                use_kzg_da: true,

Will this become variable in the future? If so where does the info come from?

Code quote:

                use_kzg_da: true,

Copy link
Collaborator

@alonh5 alonh5 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewed 1 of 1 files at r3, 1 of 1 files at r4, all commit messages.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @matan-starkware)


crates/sequencing/papyrus_consensus_orchestrator/src/sequencer_consensus_context.rs line 175 at r4 (raw file):

Previously, matan-starkware wrote…

Will this become variable in the future? If so where does the info come from?

This should always be true unless the blob data price will be higher than regular data storage, which could happen. Hard coding true for now is fine.

Copy link
Contributor Author

ArniStarkware commented Dec 3, 2024

Merge activity

  • Dec 3, 7:31 AM EST: A user started a stack merge that includes this pull request via Graphite.
  • Dec 3, 7:33 AM EST: Graphite rebased this pull request as part of a merge.
  • Dec 3, 7:52 AM EST: A user merged this pull request with Graphite.

@ArniStarkware ArniStarkware changed the base branch from arni/batcher/block_builder_factory/set_gas_prices to graphite-base/2345 December 3, 2024 12:31
@ArniStarkware ArniStarkware changed the base branch from graphite-base/2345 to main December 3, 2024 12:32
@ArniStarkware ArniStarkware force-pushed the arni/batcher/block_builder_factory/set_use_kzg_da_in_build_block_input branch from fd84f65 to ef7066c Compare December 3, 2024 12:32
@ArniStarkware ArniStarkware merged commit b7934f4 into main Dec 3, 2024
19 checks passed
@github-actions github-actions bot locked and limited conversation to collaborators Dec 5, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants