apollo_integration_tests: use validate_tx_count accross integration tests

[ArniStarkware]

No description provided.

[reviewable-StarkWare]

This change is 

[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

• apollo_integration_tests: simplify scenario arg in end to end flow #11505
• apollo_integration_tests: split end to end flow test into 5 scenarios #11504
• apollo_integration_tests: use validate_tx_count accross integration tests #11503 👈 (View in Graphite)
• apollo_integration_tests: clean old todos #11502
• apollo_integration_tests: rename reverted_l1_handler_tx_flow_test file #11501
• main-v0.14.1-committer

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

[Itay-Tsabary-Starkware]

@Itay-Tsabary-Starkware made 2 comments.
Reviewable status: 0 of 3 files reviewed, 2 unresolved discussions (waiting on @ArniStarkware).

---

crates/apollo_integration_tests/tests/common/mod.rs line 253 at r1 (raw file):

    );
    tx_hashes.to_vec()
}

I'm confused regarding this fn:

AFAIU the callers to this fn have a Vec<TransactionHash> element, which they pass as a slice to the fn, only for the fn to convert it back to a vector.

This seems inefficient.

Also - why does a validate fn have a return value? I'd expect it to either assert if the validation fails, or do nothing instead.

Code quote:

pub fn validate_tx_count(
    tx_hashes: &[TransactionHash],
    expected_count: usize,
) -> Vec<TransactionHash> {
    let tx_hashes_len = tx_hashes.len();
    assert_eq!(
        tx_hashes_len, expected_count,
        "Expected {expected_count} txs, but found {tx_hashes_len} txs.",
    );
    tx_hashes.to_vec()
}

---

crates/apollo_integration_tests/tests/test_many.rs line 27 at r1 (raw file):

}

const N_TXS: usize = 15;

Please place at the top of the module (after the use statements)

Code quote:

const N_TXS: usize = 15;

[ArniStarkware]

@ArniStarkware made 2 comments.
Reviewable status: 0 of 3 files reviewed, 2 unresolved discussions (waiting on @Itay-Tsabary-Starkware).

---

crates/apollo_integration_tests/tests/test_many.rs line 27 at r1 (raw file):

Previously, Itay-Tsabary-Starkware wrote…

Please place at the top of the module (after the use statements)

Done.

---

crates/apollo_integration_tests/tests/common/mod.rs line 253 at r1 (raw file):

Previously, Itay-Tsabary-Starkware wrote…

I'm confused regarding this fn:

AFAIU the callers to this fn have a Vec<TransactionHash> element, which they pass as a slice to the fn, only for the fn to convert it back to a vector.

This seems inefficient.

Also - why does a validate fn have a return value? I'd expect it to either assert if the validation fails, or do nothing instead.

Added a todo to simplify this inefficient "validation" (which, as you said, does more than validate.)

This is the parameter for TestScenario:

sequencer/crates/apollo_integration_tests/tests/common/mod.rs

Line 179 in bd2f4fb

pub test_tx_hashes_fn: TestTxHashesFn,

The only non-trivial usage of this parameter is:

sequencer/crates/apollo_integration_tests/tests/end_to_end_flow_test.rs

Lines 85 to 95 in bf5e216

	fn test_multiple_account_txs(tx_hashes: &[TransactionHash]) -> Vec<TransactionHash> {
	// Return the transaction hashes in the order they should be given by the mempool:
	// Transactions from the same account are ordered by nonce; otherwise, higher tips are given
	// priority.
	assert!(
	tx_hashes.len() == 3,
	"Unexpected number of transactions sent in the test scenario. Found {} transactions",
	tx_hashes.len()
	);
	vec![tx_hashes[2], tx_hashes[0], tx_hashes[1]]
	}

[Itay-Tsabary-Starkware]

@Itay-Tsabary-Starkware reviewed 3 files and all commit messages, made 1 comment, and resolved 2 discussions.
Reviewable status: complete! all files reviewed, all discussions resolved (waiting on @ArniStarkware).