fix(starknet_integration_tests): temporary fixes to make the e2e flow test pass

[yair-starkware]

No description provided.

[reviewable-StarkWare]

This change is 

[yair-starkware]

• test(starknet_integration_tests): test mempool syncing in e2e flow test #2628
• test(starknet_integration_tests): test proposer-validator setup in flow test #2348
• fix(starknet_integration_tests): temporary fixes to make the e2e flow test pass #2591 👈 (View in Graphite)
• fix(starknet_integration_tests): fix config when creating multiple nodes #2426
• refactor(starknet_integration_tests): separate sequencer setup from test setup #2299
• test(starknet_integration_tests): test commit blocks by running multiple heights #2242
• feat(tests_integration): one to many network config #2253
• main

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

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 34.59%. Comparing base (e3165c4) to head (15f96fe).
Report is 803 commits behind head on main.

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #2591      +/-   ##
==========================================
- Coverage   40.10%   34.59%   -5.51%     
==========================================
  Files          26      276     +250     
  Lines        1895    32180   +30285     
  Branches     1895    32180   +30285     
==========================================
+ Hits          760    11133   +10373     
- Misses       1100    20058   +18958     
- Partials       35      989     +954     

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

[alonh5]

Reviewed 3 of 3 files at r1, all commit messages.
Reviewable status: all files reviewed, 2 unresolved discussions (waiting on @dan-starkware, @matan-starkware, and @yair-starkware)

---

crates/starknet_integration_tests/tests/end_to_end_flow_test.rs line 42 at r1 (raw file):

    const LISTEN_TO_BROADCAST_MESSAGES_TIMEOUT: std::time::Duration =
        std::time::Duration::from_secs(50);

I think this could be less.

Suggestion:

15

---

crates/starknet_batcher/src/batcher.rs line 458 at r1 (raw file):

    let as_duration =
        time_to_deadline.to_std().map_err(|_| BatcherError::TimeToDeadlineError { deadline })?;
    // TODO(Matan): this is a temporary solution to the timeout issue.

@matan-starkware please ack
Also below

[matan-starkware]

Reviewed 3 of 3 files at r1, all commit messages.
Reviewable status: all files reviewed, 4 unresolved discussions (waiting on @alonh5, @dan-starkware, and @yair-starkware)

---

crates/starknet_batcher/src/batcher.rs line 458 at r1 (raw file):

Previously, alonh5 (Alon Haramati) wrote…

@matan-starkware please ack
Also below

I think we more likely want some constant buffer that can be configured. duration - buffer. Do I understand you want this to be part of the context?

---

crates/starknet_integration_tests/src/utils.rs line 94 at r1 (raw file):

    // TODO: Need to also add a channel for votes, in addition to the proposals channel.

    // TODO(Matan, Dan): set reasonable default timeouts.

The defaults are (3s, 1s, 1s). What was the issue with those times? We are aiming for 3s block time decentralized I thought, so we should make sure our tests can handle that, no?

Code quote:

    // TODO(Matan, Dan): set reasonable default timeouts.

---

crates/starknet_integration_tests/src/utils.rs line 107 at r1 (raw file):

                start_height: BlockNumber(1),
		// TODO(Matan, Dan): Set the right amount
                consensus_delay: Duration::from_secs(5),

:(

1s wasn't enough in the test? How are we handling startup for the nodes in the test?

Code quote:

                consensus_delay: Duration::from_secs(5),

[alonh5]

Reviewable status: all files reviewed, 3 unresolved discussions (waiting on @dan-starkware and @yair-starkware)

---

crates/starknet_batcher/src/batcher.rs line 458 at r1 (raw file):

Previously, matan-starkware wrote…

I think we more likely want some constant buffer that can be configured. duration - buffer. Do I understand you want this to be part of the context?

This is the simplest patch for now, so you and others can run the flow test. Yes, the context should decide how much time to give the batcher so there's enough time to reach a consensus with the block. a time buffer sounds good.

[yair-starkware]

Reviewable status: all files reviewed, 3 unresolved discussions (waiting on @alonh5, @dan-starkware, and @matan-starkware)

---

crates/starknet_integration_tests/src/utils.rs line 94 at r1 (raw file):

Previously, matan-starkware wrote…

The defaults are (3s, 1s, 1s). What was the issue with those times? We are aiming for 3s block time decentralized I thought, so we should make sure our tests can handle that, no?

Occasionally there was not enough time to get the whole flow done before the deadline (+ the buffer time added between block building and consensus)

---

crates/starknet_integration_tests/src/utils.rs line 107 at r1 (raw file):

Previously, matan-starkware wrote…

:(

1s wasn't enough in the test? How are we handling startup for the nodes in the test?

Also, we start the nodes in sequence to make sure they don't try to get the same ports (should be fixed in the future as well)

---

crates/starknet_integration_tests/tests/end_to_end_flow_test.rs line 42 at r1 (raw file):

Previously, alonh5 (Alon Haramati) wrote…

I think this could be less.

I prefer not to deal with sporadic failures. We shouldn't reach the deadline anyway, so I don't think setting a high number here is a problem.

[alonh5]

Reviewed 3 of 3 files at r2, all commit messages.
Reviewable status: all files reviewed, 2 unresolved discussions (waiting on @dan-starkware and @matan-starkware)

[yair-starkware]

Merge activity

• Dec 10, 7:43 AM EST: A user started a stack merge that includes this pull request via Graphite.
• Dec 10, 7:44 AM EST: A user merged this pull request with Graphite.