chore(starknet_batcher): set block timestamp in build block input

[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(starknet_batcher): use block info to create block info #2238 : 2 dependent PRs (#2267 , #2302 )
• chore(starknet_consensus_manager): set proposer address in propose block input #2346
• chore(starknet_batcher): set use_kzg_da flag in build block input #2345
• chore(starknet_batcher): set block timestamp in build block input #2342 👈 (View in Graphite)
• chore(starknet_batcher): set temp gas prices in propose block input #2341
• chore(starknet_batcher): verify block number on propose block #2340
• chore(starknet_batcher): add height as input #2339
• chore(starknet_batcher): add block info to block proposal input #2237
• chore(blockifier): move block info to starknet api #2246 : 1 other dependent PR (#2230 )
• chore(blockifier): move GasPrices to snapi #2245
• 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.

[reviewable-StarkWare]

This change is 

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 17.84%. Comparing base (37ca509) to head (c65933e).

Additional details and impacted files

@@                                  Coverage Diff                                  @@
##           arni/batcher/block_builder_factory/set_gas_prices    #2342      +/-   ##
=====================================================================================
+ Coverage                                              17.79%   17.84%   +0.04%     
=====================================================================================
  Files                                                    119      119              
  Lines                                                  13878    13886       +8     
  Branches                                               13878    13886       +8     
=====================================================================================
+ Hits                                                    2470     2478       +8     
  Misses                                                 11137    11137              
  Partials                                                 271      271              

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

[Yael-Starkware]

Reviewable status: 0 of 2 files reviewed, 2 unresolved discussions (waiting on @alonh5 and @ArniStarkware)

---

crates/starknet_batcher/src/block_builder.rs line 321 at r1 (raw file):

                    .try_into()
                    .map_err(BlockBuilderError::BadTimestamp)?,
            ),

why did you change it?
I think the previous version was easier to read.

This code will be deleted anyway, right?

Code quote:

            block_timestamp: BlockTimestamp(
                chrono::Utc::now()
                    .timestamp()
                    .try_into()
                    .map_err(BlockBuilderError::BadTimestamp)?,
            ),

---

crates/sequencing/papyrus_consensus_orchestrator/src/sequencer_consensus_context.rs line 163 at r1 (raw file):

                gas_prices: TEMPORARY_GAS_PRICES,
                block_timestamp: BlockTimestamp(
                    now.timestamp().try_into().expect("Failed to convert timestamp"),

same for the next change.

Suggestion:

chrono::Utc::now().timestamp().try_into().expect("Failed to convert timestamp"),

[alonh5]

Reviewed 2 of 2 files at r1, all commit messages.
Reviewable status: all files reviewed, 3 unresolved discussions (waiting on @ArniStarkware)

---

crates/starknet_batcher/src/block_builder.rs line 49 at r1 (raw file):

pub enum BlockBuilderError {
    #[error(transparent)]
    BadTimestamp(#[from] std::num::TryFromIntError),

Why this change?

[ArniStarkware]

crates/sequencing/papyrus_consensus_orchestrator/src/sequencer_consensus_context.rs line 163 at r1 (raw file):

Previously, Yael-Starkware (YaelD) wrote…

same for the next change.

So, Should I have two invocations of chorono::Utc::now() five lines apart? Once for deadline and once for block_timestamp.

In my opinion, it is bad - as these will be slightly different from each other - based on how long these lines take to execute.

[ArniStarkware]

Dismissed @alonh5 from a discussion.
Reviewable status: 0 of 2 files reviewed, 2 unresolved discussions (waiting on @alonh5 and @Yael-Starkware)

---

crates/starknet_batcher/src/block_builder.rs line 49 at r1 (raw file):

Previously, alonh5 (Alon Haramati) wrote…

Why this change?

As @Yael-Starkware pointed out - this will be deleted by the next PR anyway.
Reverted.

---

crates/starknet_batcher/src/block_builder.rs line 321 at r1 (raw file):

Previously, Yael-Starkware (YaelD) wrote…

why did you change it?
I think the previous version was easier to read.

This code will be deleted anyway, right?

1. Yes - the code will be deleted anyway. Reverted.
2. I think it is bad that a generic TryFromIntError is always converted to a BadTimestamp. (Again - in theory), what happens if another conversion from integer happens in the scope of Block-build, but has nothing to do with timestamp?

[alonh5]

Reviewed 2 of 2 files at r2, all commit messages.
Reviewable status: all files reviewed, 2 unresolved discussions (waiting on @Yael-Starkware)

[Yael-Starkware]

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

---

crates/sequencing/papyrus_consensus_orchestrator/src/sequencer_consensus_context.rs line 163 at r1 (raw file):

Previously, ArniStarkware (Arnon Hod) wrote…

So, Should I have two invocations of chorono::Utc::now() five lines apart? Once for deadline and once for block_timestamp.

In my opinion, it is bad - as these will be slightly different from each other - based on how long these lines take to execute.

oh sorry I missed the deadline usage. retracting.