feat(starknet_integration_tests): struct to collect streamed txs

[yair-starkware]

No description provided.

[reviewable-StarkWare]

This change is 

[yair-starkware]

• feat(starknet_integration_tests): refactor e2e flow test #4787 : 2 dependent PRs (#5164 , #5223 )
• feat(starknet_integration_tests): struct to collect batched txs #4786
• feat(starknet_integration_tests): struct to collect streamed txs #4785 👈 (View in Graphite)
• refactor(starknet_integration_tests): copy listen_to_broadcasted_messages to test setup #4784
• main

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

[graphite-app]

Graphite Automations

"Yair - Auto-assign" took an action on this PR • (03/10/25)

1 assignee was added to this PR based on Yair's automation.

[alonh5]

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

---

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

/// Stores tx hashes aggregated by height and round.
#[derive(Debug, Default)]
pub struct AggregatedTransactions(Vec<(BlockNumber, (Round, Vec<TransactionHash>))>);

Create a named struct for this, instead of using .0/.1.

Suggestion:

RoundTransactions

---

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

    }

    pub fn get_latest_block_with_txs(&self) -> Option<BlockNumber> {

What is this for?

---

crates/starknet_integration_tests/src/utils.rs line 593 at r2 (raw file):

/// Assumes that rounds are monotonically increasing and that the last round is the chosen one.
#[derive(Debug, Default)]
pub struct AggregatedTransactions(Vec<(BlockNumber, (Round, Vec<TransactionHash>))>);

Do you even need to keep the whole history of blocks and rounds? Why not just store all txs together and track only the current block and round?

---

crates/starknet_integration_tests/src/utils.rs line 611 at r2 (raw file):

    }

    pub fn get_all_txs_so_far(&self) -> Vec<TransactionHash> {

Looks like here your collecting all the txs from all rounds and not just the last one like you said.

---

crates/starknet_integration_tests/src/utils.rs line 627 at r2 (raw file):

    }

    fn prepare_entry(&mut self, height: BlockNumber, round: Round) -> &mut Vec<TransactionHash> {

If you store it as a map instead of a vec with the keys being (block, round) you can just get the entry or create a new one. You're already checking these conditions in the validate function.

[Yael-Starkware]

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

---

crates/starknet_integration_tests/src/utils.rs line 593 at r2 (raw file):

/// Assumes that rounds are monotonically increasing and that the last round is the chosen one.
#[derive(Debug, Default)]
pub struct AggregatedTransactions(Vec<(BlockNumber, (Round, Vec<TransactionHash>))>);

why not use a HashMap and store only the most recent round for each block_number.

Code quote:

pub struct AggregatedTransactions(Vec<(BlockNumber, (Round, Vec<TransactionHash>))>);

---

crates/starknet_integration_tests/src/utils.rs line 624 at r2 (raw file):

        if *last_height == height && *last_round > round {
            panic!("Expected round to be greater or equal to the last round.");
        }

isn't it possible to receive old messages from a previous round?

Code quote:

        if *last_height > height {
            panic!("Expected height to be greater or equal to the last height with transactions.");
        }
        if *last_height == height && *last_round > round {
            panic!("Expected round to be greater or equal to the last round.");
        }

[yair-starkware]

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

---

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

Previously, alonh5 (Alon Haramati) wrote…

Create a named struct for this, instead of using .0/.1.

Done.

[yair-starkware]

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

---

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

Previously, yair-starkware (Yair) wrote…

Done.

Changed to keep only the last round

---

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

initially I thought about waiting on blocks instead of time

Removing for now

---

crates/starknet_integration_tests/src/utils.rs line 593 at r2 (raw file):

Previously, Yael-Starkware (YaelD) wrote…

why not use a HashMap and store only the most recent round for each block_number.

Changed, but using vec to keep sorted

---

crates/starknet_integration_tests/src/utils.rs line 593 at r2 (raw file):

Previously, alonh5 (Alon Haramati) wrote…

Do you even need to keep the whole history of blocks and rounds? Why not just store all txs together and track only the current block and round?

I guess I could do that, but initially I thought about waiting on blocks instead of time.
I think we might still want this so I prefer to keep the functionality.

---

crates/starknet_integration_tests/src/utils.rs line 611 at r2 (raw file):

Previously, alonh5 (Alon Haramati) wrote…

Looks like here your collecting all the txs from all rounds and not just the last one like you said.

Fixed by saving only the last round

---

crates/starknet_integration_tests/src/utils.rs line 624 at r2 (raw file):

Previously, Yael-Starkware (YaelD) wrote…

isn't it possible to receive old messages from a previous round?

It wasn't possible in the previous implementation

---

crates/starknet_integration_tests/src/utils.rs line 627 at r2 (raw file):

Previously, alonh5 (Alon Haramati) wrote…

If you store it as a map instead of a vec with the keys being (block, round) you can just get the entry or create a new one. You're already checking these conditions in the validate function.

Done

[yair-starkware]

crates/starknet_integration_tests/src/utils.rs line 593 at r2 (raw file):

Previously, yair-starkware (Yair) wrote…

Changed, but using vec to keep sorted

IndexMap

[alonh5]

Reviewed 1 of 2 files at r3, 1 of 1 files at r4, 1 of 1 files at r5, all commit messages.
Reviewable status: all files reviewed, 4 unresolved discussions (waiting on @ArniStarkware, @Yael-Starkware, and @yair-starkware)

---

crates/starknet_integration_tests/src/utils.rs line 593 at r2 (raw file):

Previously, yair-starkware (Yair) wrote…

I guess I could do that, but initially I thought about waiting on blocks instead of time.
I think we might still want this so I prefer to keep the functionality.

If you want to wait on blocks you still only need the current block, no?

It will simplify the other functions as well, please consider

---

crates/starknet_integration_tests/src/utils.rs line 611 at r2 (raw file):

Previously, yair-starkware (Yair) wrote…

Fixed by saving only the last round

Where? I still see add_transactions is called on every round in the next PR, and you're not removing old ones here.
You can maintain staged txs and clean it in the beginning of every round, only if the next txs are from the next block you then add them to the committed txs.

[Yael-Starkware]

Reviewed all commit messages.
Reviewable status: 1 of 2 files reviewed, 4 unresolved discussions (waiting on @alonh5, @ArniStarkware, and @yair-starkware)

---

crates/starknet_integration_tests/src/utils.rs line 624 at r2 (raw file):

Previously, yair-starkware (Yair) wrote…

It wasn't possible in the previous implementation

got it. do we want to verify this?

---

crates/starknet_integration_tests/src/utils.rs line 615 at r6 (raw file):

        let entry = self.0.entry(height).or_insert(RoundTransactions { round, tx_hashes: vec![] });
        entry.tx_hashes.extend_from_slice(tx_hashes);
        entry.round = round;

Suggestion:

        let entry = self.0.entry(height).or_default();
        entry.tx_hashes.extend_from_slice(tx_hashes);
        entry.round = round;

[yair-starkware]

Reviewable status: 1 of 2 files reviewed, 4 unresolved discussions (waiting on @alonh5, @ArniStarkware, and @Yael-Starkware)

---

crates/starknet_integration_tests/src/utils.rs line 593 at r2 (raw file):

Previously, alonh5 (Alon Haramati) wrote…

If you want to wait on blocks you still only need the current block, no?

It will simplify the other functions as well, please consider

Done.

---

crates/starknet_integration_tests/src/utils.rs line 611 at r2 (raw file):

Previously, alonh5 (Alon Haramati) wrote…

Where? I still see add_transactions is called on every round in the next PR, and you're not removing old ones here.
You can maintain staged txs and clean it in the beginning of every round, only if the next txs are from the next block you then add them to the committed txs.

Not relevant now (it was fixed by using index map with height as key)

---

crates/starknet_integration_tests/src/utils.rs line 624 at r2 (raw file):

Previously, Yael-Starkware (YaelD) wrote…

got it. do we want to verify this?

Checked

---

crates/starknet_integration_tests/src/utils.rs line 615 at r6 (raw file):

        let entry = self.0.entry(height).or_insert(RoundTransactions { round, tx_hashes: vec![] });
        entry.tx_hashes.extend_from_slice(tx_hashes);
        entry.round = round;

Not relevant now

[Yael-Starkware]

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

---

crates/starknet_integration_tests/src/utils.rs line 600 at r7 (raw file):

    // To remove from tx_hashes when a new round replaces the last round.
    last_round_tx_hashes: Vec<TransactionHash>,
}

consider changing the names.
you are adding transactions to the "last_round_tx_hashes" during the current round, so I think it is more clear to look at it as the current round.

Suggestion:

pub struct AggregatedTransactions {
    pub latest_block_number: BlockNumber,
    pub round: u32,
    pub accumulated_tx_hashes: Vec<TransactionHash>,
    // To remove from tx_hashes when a new round replaces the current round.
    current_round_tx_hashes: Vec<TransactionHash>,
}

[yair-starkware]

Reviewable status: 1 of 2 files reviewed, 3 unresolved discussions (waiting on @alonh5, @ArniStarkware, and @Yael-Starkware)

---

crates/starknet_integration_tests/src/utils.rs line 600 at r7 (raw file):

Previously, Yael-Starkware (YaelD) wrote…

consider changing the names.
you are adding transactions to the "last_round_tx_hashes" during the current round, so I think it is more clear to look at it as the current round.

Done.

[alonh5]

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

---

crates/starknet_integration_tests/src/utils.rs line 593 at r2 (raw file):

Previously, yair-starkware (Yair) wrote…

Done.

Nice, thanks

---

crates/starknet_integration_tests/src/utils.rs line 611 at r7 (raw file):

        self.validate_coherent_height_and_round(height, round);
        if self.latest_block_number == height && self.round < round {
            self.tx_hashes.retain(|tx_hash| !self.last_round_tx_hashes.contains(tx_hash));

Can you store the current round tx and only add them to self.tx_hashes if you advance to the next height?
The way it is now, if the last round is sent but not accepted the test could still pass.
Also it will make this code more explicit and readable, there should be explicit logic for these cases:

1. Same round - accumulate round txs
2. Same height new round - discard last round and restart accumulation
3. New height - add round txs to tx hashes

[Yael-Starkware]

Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @ArniStarkware and @yair-starkware)

---

crates/starknet_integration_tests/src/utils.rs line 600 at r7 (raw file):

Previously, yair-starkware (Yair) wrote…

Done.

I also suggested to rename the tx_hashes to accumulated_tx_hashes. non blocking.

[yair-starkware]

Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @alonh5 and @ArniStarkware)

---

crates/starknet_integration_tests/src/utils.rs line 600 at r7 (raw file):

Previously, Yael-Starkware (YaelD) wrote…

I also suggested to rename the tx_hashes to accumulated_tx_hashes. non blocking.

Done

---

crates/starknet_integration_tests/src/utils.rs line 611 at r7 (raw file):

Previously, alonh5 (Alon Haramati) wrote…

Can you store the current round tx and only add them to self.tx_hashes if you advance to the next height?
The way it is now, if the last round is sent but not accepted the test could still pass.
Also it will make this code more explicit and readable, there should be explicit logic for these cases:

1. Same round - accumulate round txs
2. Same height new round - discard last round and restart accumulation
3. New height - add round txs to tx hashes

Done.

[alonh5]

Nice!

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

[Yael-Starkware]

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