test: add consensus to e2e flow test

[dan-starkware]

No description provided.

[reviewable-StarkWare]

This change is 

[dan-starkware]

• test: add consensus to e2e flow test #1811 👈
• feat(consensus): send proposal content #1810
• feat(consensus): run network manager in consensus #1809
• chore(consensus): add network config #1808
• chore: rename proposal init and add log #1807
• chore: add target in fmt layer #1806
• fix(network): fix secret key length in test util #1774
• main

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

Join @dan-starkware and the rest of your teammates on Graphite

[github-actions]

Artifacts upload triggered. View details here

[codecov]

Codecov Report

Attention: Patch coverage is 92.59259% with 2 lines in your changes missing coverage. Please review.

Project coverage is 76.39%. Comparing base (e3165c4) to head (671118d).
Report is 332 commits behind head on main.

Files with missing lines	Patch %	Lines
...us_orchestrator/src/sequencer_consensus_context.rs	60.00%	1 Missing and 1 partial ⚠️

Additional details and impacted files

@@             Coverage Diff             @@
##             main    #1811       +/-   ##
===========================================
+ Coverage   40.10%   76.39%   +36.28%     
===========================================
  Files          26      373      +347     
  Lines        1895    39845    +37950     
  Branches     1895    39845    +37950     
===========================================
+ Hits          760    30438    +29678     
- Misses       1100     7138     +6038     
- Partials       35     2269     +2234     

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

[github-actions]

Artifacts upload triggered. View details here

[Itay-Tsabary-Starkware]

Reviewable status: 0 of 7 files reviewed, 4 unresolved discussions (waiting on @dan-starkware)

---

crates/tests-integration/src/integration_test_utils.rs line 72 at r1 (raw file):

}

fn create_consensus_manager_config()

Current name is confusing, as there are other create_X_config that just return a config object, while this variant also returns the channels. Could you please rename?

Code quote:

create_consensus_manager_config

---

crates/tests-integration/tests/end_to_end_test.rs line 44 at r1 (raw file):

        .expect("listen to broadcasted messages should finish in time");
    });
    join_handle.await.expect("Task should succeed");

Please wrap the "await for something to happen within a timeout" as a utility function (or add a TODO, can include my name as well).

Code quote:

    let join_handle = tokio::spawn(async move {
        tokio::time::timeout(
            LISTEN_TO_BROADCAST_MESSAGES_TIMEOUT,
            listen_to_broadcasted_messages(
                mock_running_system.consensus_proposals_channels,
                &expected_batched_tx_hashes,
            ),
        )
        .await
        .expect("listen to broadcasted messages should finish in time");
    });
    join_handle.await.expect("Task should succeed");

---

crates/tests-integration/tests/end_to_end_test.rs line 51 at r1 (raw file):

    expected_batched_tx_hashes: &[TransactionHash],
) {
    // TODO(Dan, Guy): retrieve chian ID (from the config?).

Typo

Code quote:

chian

---

crates/tests-integration/tests/end_to_end_test.rs line 51 at r1 (raw file):

    expected_batched_tx_hashes: &[TransactionHash],
) {
    // TODO(Dan, Guy): retrieve chian ID (from the config?).

I'd suggest modifying IntegrationTestSetup to hold it as a member, and instantiate the value using StorageTestSetup.

Code quote:

(from the config?)

---

crates/tests-integration/tests/end_to_end_test.rs line 71 at r1 (raw file):

            );
        }
    }

I didn't review this part, I suggest asking someone more familiar with the consensus to do so. If there's no on else I'll dig into this though, please lmk.

Code quote:

    let mut broadcasted_messages_receiver =
        consensus_proposals_channels.broadcasted_messages_receiver;
    let mut received_tx_hashes = vec![];
    while received_tx_hashes.len() < expected_batched_tx_hashes.len() {
        let (message, _broadcasted_message_metadata) = broadcasted_messages_receiver
            .next()
            .await
            .unwrap_or_else(|| panic!("Expected to receive a message from the broadcast topic"));
        if let ProposalPart::Transactions(transactions) = message.unwrap() {
            received_tx_hashes.append(
                &mut transactions
                    .transactions
                    .iter()
                    .map(|tx| tx.calculate_transaction_hash(&chain_id).unwrap())
                    .collect(),
            );
        }
    }

[dan-starkware]

Reviewable status: 0 of 7 files reviewed, 3 unresolved discussions (waiting on @Itay-Tsabary-Starkware)

---

crates/tests-integration/src/integration_test_utils.rs line 72 at r1 (raw file):

Previously, Itay-Tsabary-Starkware wrote…

Current name is confusing, as there are other create_X_config that just return a config object, while this variant also returns the channels. Could you please rename?

Done.

---

crates/tests-integration/tests/end_to_end_test.rs line 44 at r1 (raw file):

Previously, Itay-Tsabary-Starkware wrote…

Please wrap the "await for something to happen within a timeout" as a utility function (or add a TODO, can include my name as well).

Done.

---

crates/tests-integration/tests/end_to_end_test.rs line 51 at r1 (raw file):

Previously, Itay-Tsabary-Starkware wrote…

I'd suggest modifying IntegrationTestSetup to hold it as a member, and instantiate the value using StorageTestSetup.

Done.

[github-actions]

Artifacts upload triggered. View details here

[Itay-Tsabary-Starkware]

Reviewed 5 of 7 files at r1, 2 of 2 files at r2, all commit messages.
Reviewable status: complete! all files reviewed, all discussions resolved (waiting on @dan-starkware)

[github-actions]

Artifacts upload triggered. View details here

[github-actions]

Artifacts upload triggered. View details here

[github-actions]

Artifacts upload triggered. View details here

[github-actions]

Artifacts upload triggered. View details here

[github-actions]

Artifacts upload triggered. View details here

[matan-starkware]

Reviewed 7 of 7 files at r7, all commit messages.
Reviewable status: all files reviewed, 2 unresolved discussions (waiting on @dan-starkware, @guy-starkware, and @Itay-Tsabary-Starkware)

---

crates/tests-integration/tests/end_to_end_test.rs line 39 at r5 (raw file):

Previously, dan-starkware wrote…

Done.

I don't see any change, which is fine for this PR, just noting since you wrote "Done"

---

crates/tests-integration/tests/end_to_end_test.rs line 68 at r5 (raw file):

Previously, dan-starkware wrote…

Why? This is a utility for testing.

macros are harder to work with in the IDE, you don't get nice type hints, and jumping to the definition doesn't bring you to the actual function used, still requires more sleuthing. They also make compile times slower.

I prefer functions when there is a 1 to 1 replacement.

[github-actions]

Artifacts upload triggered. View details here

[dan-starkware]

Reviewable status: 5 of 13 files reviewed, 1 unresolved discussion (waiting on @guy-starkware, @Itay-Tsabary-Starkware, and @matan-starkware)

---

crates/tests-integration/tests/end_to_end_test.rs line 39 at r5 (raw file):

Previously, matan-starkware wrote…

I don't see any change, which is fine for this PR, just noting since you wrote "Done"

I meant ACK

[github-actions]

Artifacts upload triggered. View details here

[matan-starkware]

Reviewed 7 of 8 files at r8, 1 of 1 files at r9, all commit messages.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @guy-starkware and @Itay-Tsabary-Starkware)

[github-actions]

Artifacts upload triggered. View details here

[Itay-Tsabary-Starkware]

Reviewed 1 of 6 files at r5, 3 of 7 files at r7, 4 of 8 files at r8, 1 of 1 files at r10, all commit messages.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @guy-starkware)

[github-actions]

Artifacts upload triggered. View details here

[ShahakShama]

Reviewed 1 of 6 files at r5, 2 of 7 files at r7, 1 of 8 files at r8, 10 of 10 files at r11, all commit messages.
Reviewable status: all files reviewed, 2 unresolved discussions (waiting on @dan-starkware and @guy-starkware)

[ShahakShama]

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

[ShahakShama]

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

talked f2f

[ShahakShama]

Reviewable status: all files reviewed, 2 unresolved discussions (waiting on @graphite-app[bot] and @guy-starkware)

[yair-starkware]

Dismissed @graphite-app[bot] from a discussion.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @guy-starkware)

[yair-starkware]

Dismissed @guy-starkware from a discussion.
Reviewable status: complete! all files reviewed, all discussions resolved (waiting on @guy-starkware)