fix(batcher): treat deadline as error in validate flow

[Yael-Starkware]

No description provided.

[reviewable-StarkWare]

This change is 

[Yael-Starkware]

• fix(batcher): treat deadline as error in validate flow #2148 👈 (View in Graphite)
• fix(batcher): fix send_proposal_content return status and add the relevant tests #2053 : 2 other dependent PRs (#2126 , #2162 )
• feat(batcher): add validate flow #2050
• refactor(batcher): extract the deadline function in order to share with the validate flow #2049
• refactor(batcher): add input channel size config #2042
• refactor(batcher): add batcher creation func to test #2046
• feat(batcher): add validate flow to proposal manager #1951
• refactor(batcher): add propose_tx_provider fixture #1996
• refactor(batcher): change BuildProposalError to GenerateProposalError #1986
• refactor(batcher): extract common logic into a function for validate flow #1985
• main

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

[codecov]

Codecov Report

Attention: Patch coverage is 71.42857% with 4 lines in your changes missing coverage. Please review.

Project coverage is 5.26%. Comparing base (e3165c4) to head (fa0b49a).
Report is 511 commits behind head on main.

Files with missing lines	Patch %	Lines
crates/starknet_batcher/src/block_builder.rs	71.42%	3 Missing and 1 partial ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main   #2148       +/-   ##
==========================================
- Coverage   40.10%   5.26%   -34.85%     
==========================================
  Files          26     145      +119     
  Lines        1895   17068    +15173     
  Branches     1895   17068    +15173     
==========================================
+ Hits          760     898      +138     
- Misses       1100   16094    +14994     
- Partials       35      76       +41     

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

---

🚨 Try these New Features:

• Flaky Tests Detection - Detect and resolve failed and flaky tests
• JS Bundle Analysis - Avoid shipping oversized bundles

[Yael-Starkware]

Reviewable status: 0 of 3 files reviewed, 1 unresolved discussion (waiting on @alonh5 and @yair-starkware)

---

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

    StreamTransactionsError(#[from] tokio::sync::mpsc::error::SendError<Transaction>),
    #[error("Build block with fail_on_err mode, failed on error {}.", _0)]
    FailOnError(String),

do you think an enum would be better than a string here?

#[derive(Debug)]
pub enum FailCause {
DeadlineReached,
BlockFull,
TransactionError(err),
}

Code quote:

FailOnError(String)

[alonh5]

Reviewed 3 of 3 files at r1, all commit messages.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @Yael-Starkware and @yair-starkware)

---

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

Previously, Yael-Starkware (YaelD) wrote…

do you think an enum would be better than a string here?

#[derive(Debug)]
pub enum FailCause {
DeadlineReached,
BlockFull,
TransactionError(err),
}

Both are fine, but I think an enum would be better.

[Yael-Starkware]

Reviewable status: 0 of 3 files reviewed, 1 unresolved discussion (waiting on @alonh5 and @yair-starkware)

---

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

Previously, alonh5 (Alon Haramati) wrote…

Both are fine, but I think an enum would be better.

Done.

[alonh5]

Reviewed 2 of 3 files at r2, 1 of 1 files at r3, all commit messages.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @Yael-Starkware and @yair-starkware)

---

crates/starknet_batcher/src/block_builder.rs line 71 at r3 (raw file):

        match self {
            FailOnErrorCause::BlockFull => write!(f, "Block is full"),
            FailOnErrorCause::DeadlineReached => write!(f, "Deadline was reached"),

Suggestion:

has been reached

[yair-starkware]

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

---

a discussion (no related file):
Can we use test cases in the tests that check fail_on_error

---

crates/starknet_batcher/src/block_builder_test.rs line 154 at r3 (raw file):

fn mock_transaction_executor_with_delay(
    input_txs: Vec<Transaction>,

Not blocking, but IIUC this change is not necessary (and it is advised to use slices in arguments)

Code quote:

Vec<T

---

crates/starknet_batcher/src/block_builder.rs line 52 at r3 (raw file):

    #[error(transparent)]
    StreamTransactionsError(#[from] tokio::sync::mpsc::error::SendError<Transaction>),
    #[error("Build block with fail_on_err mode, failed on error {0}.")]

activated

Code quote:

 mode

---

crates/starknet_batcher/src/block_builder.rs line 61 at r3 (raw file):

#[derive(Debug)]
pub enum FailOnErrorCause {

You can derive Error and use error(transaparent) in BlockBuilderError

Code quote:

pub enum FailOnErrorCause {

[yair-starkware]

Reviewed 1 of 3 files at r2, 1 of 1 files at r3.
Reviewable status: all files reviewed, 5 unresolved discussions (waiting on @Yael-Starkware)

[Yael-Starkware]

Reviewable status: 1 of 3 files reviewed, 1 unresolved discussion (waiting on @alonh5 and @yair-starkware)

---

a discussion (no related file):

Previously, yair-starkware (Yair) wrote…

Can we use test cases in the tests that check fail_on_error

done.

---

crates/starknet_batcher/src/block_builder.rs line 52 at r3 (raw file):

Previously, yair-starkware (Yair) wrote…

activated

done

---

crates/starknet_batcher/src/block_builder.rs line 61 at r3 (raw file):

Previously, yair-starkware (Yair) wrote…

You can derive Error and use error(transaparent) in BlockBuilderError

Done.

---

crates/starknet_batcher/src/block_builder_test.rs line 154 at r3 (raw file):

Previously, yair-starkware (Yair) wrote…

Not blocking, but IIUC this change is not necessary (and it is advised to use slices in arguments)

done.

---

crates/starknet_batcher/src/block_builder.rs line 71 at r3 (raw file):

        match self {
            FailOnErrorCause::BlockFull => write!(f, "Block is full"),
            FailOnErrorCause::DeadlineReached => write!(f, "Deadline was reached"),

done

[alonh5]

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

[yair-starkware]

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