refactor: share benchmark code with simple flow test

[avi-starkware]

This change is 

[avi-starkware]

crates/blockifier/Cargo.toml line 57 at r1 (raw file):

glob.workspace = true
pretty_assertions.workspace = true
rand.workspace = true

I wanted to keep it here, but rust doesn't let me. Are test utils not considered "dev" for the purpose of dependencies?

Code quote:

rand.workspace = true

[codecov-commenter]

Codecov Report

Attention: Patch coverage is 93.05556% with 5 lines in your changes missing coverage. Please review.

Project coverage is 78.18%. Comparing base (0abafd7) to head (a6fff18).

Files	Patch %	Lines
...s/blockifier/src/test_utils/transfers_generator.rs	93.05%	5 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #1917      +/-   ##
==========================================
+ Coverage   78.02%   78.18%   +0.15%     
==========================================
  Files          61       62       +1     
  Lines        8756     8828      +72     
  Branches     8756     8828      +72     
==========================================
+ Hits         6832     6902      +70     
- Misses       1484     1486       +2     
  Partials      440      440              

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

[avi-starkware]

+reviewer:@OriStarkware

[OriStarkware]

Reviewed all commit messages.
Reviewable status: 0 of 6 files reviewed, 2 unresolved discussions (waiting on @avi-starkware, @noaov1, and @Yoni-Starkware)

a discussion (no related file):
Can you split this into a code-moving PR and a code-changing PR?

---

[avi-starkware]

Previously, OriStarkware wrote…

Can you split this into a code-moving PR and a code-changing PR?

Done.

The code change happens in a preliminary PR: #1919 .
The current PR moves the code, and introduces the file transfers_flow_test.rs.

[OriStarkware]

Reviewed 4 of 6 files at r1, 1 of 1 files at r2, all commit messages.
Reviewable status: 5 of 6 files reviewed, 2 unresolved discussions (waiting on @avi-starkware, @noaov1, and @Yoni-Starkware)

---

crates/blockifier/Cargo.toml line 57 at r1 (raw file):

Previously, avi-starkware wrote…

I wanted to keep it here, but rust doesn't let me. Are test utils not considered "dev" for the purpose of dependencies?

Please check that

---

crates/blockifier/src/blockifier/transfers_flow_test.rs line 5 at r2 (raw file):

#[test]
pub fn transfers_flow_test() {
    let n_chunks = 100;

Why do we need so many chunks?

[avi-starkware]

crates/blockifier/src/blockifier/transfers_flow_test.rs line 5 at r2 (raw file):

Previously, OriStarkware wrote…

Why do we need so many chunks?

This is just an arbitrary number. i'll reduce it to 10.

[OriStarkware]

Reviewed 2 of 2 files at r3, all commit messages.
Reviewable status: 5 of 6 files reviewed, 1 unresolved discussion (waiting on @avi-starkware, @noaov1, and @Yoni-Starkware)

[OriStarkware]

Reviewable status: 5 of 6 files reviewed, all discussions resolved (waiting on @noaov1 and @Yoni-Starkware)

[OriStarkware]

Reviewed 1 of 6 files at r1.
Reviewable status: complete! all files reviewed, all discussions resolved (waiting on @noaov1 and @Yoni-Starkware)

[noaov1]

Reviewed 3 of 6 files at r1, 1 of 1 files at r2, 2 of 2 files at r3, all commit messages.
Reviewable status: all files reviewed, 3 unresolved discussions (waiting on @avi-starkware and @Yoni-Starkware)

---

crates/blockifier/Cargo.toml line 57 at r1 (raw file):

Previously, OriStarkware wrote…

Please check that

Can you please explain?

---

crates/blockifier/src/blockifier/transfers_flow_test.rs line 5 at r3 (raw file):

#[test]
pub fn transfers_flow_test() {
    let n_chunks = 10;

Given the new implantation of execute_txs (execute a stream of transactions and divides it to chunks) consider running it one time.

Code quote:

    let n_chunks = 10;

---

crates/blockifier/src/test_utils/transfers_simulator.rs line 25 at r3 (raw file):

const CHUNK_SIZE: usize = 10;
const RANDOMIZATION_SEED: u64 = 0;
const CHARGE_FEE: bool = false;

Can you please remind me why it's off? In the flow test we might want to charge fee.

Code quote:

const CHARGE_FEE: bool = false;

---

crates/blockifier/src/test_utils/transfers_simulator.rs line 26 at r3 (raw file):

const RANDOMIZATION_SEED: u64 = 0;
const CHARGE_FEE: bool = false;
const TRANSACTION_VERSION: TransactionVersion = TransactionVersion(StarkFelt::ONE);

Suggestion:

const TRANSACTION_VERSION: TransactionVersion = TransactionVersion(StarkFelt::THREE);

[avi-starkware]

crates/blockifier/Cargo.toml line 57 at r1 (raw file):

Previously, noaov1 (Noa Oved) wrote…

Can you please explain?

Test code is compiled when the create is built, so it is not considered a dev-dependency when we use it in test-utils as opposed to when we use it in the benchmark.

[avi-starkware]

Reviewable status: 2 of 6 files reviewed, 3 unresolved discussions (waiting on @noaov1, @OriStarkware, and @Yoni-Starkware)

---

crates/blockifier/src/blockifier/transfers_flow_test.rs line 5 at r3 (raw file):

Previously, noaov1 (Noa Oved) wrote…

Given the new implantation of execute_txs (execute a stream of transactions and divides it to chunks) consider running it one time.

Done.

---

crates/blockifier/src/test_utils/transfers_simulator.rs line 25 at r3 (raw file):

Previously, noaov1 (Noa Oved) wrote…

Can you please remind me why it's off? In the flow test we might want to charge fee.

This is a block of code I moved from elsewhere. I suggest not changing anything in this file in this PR, and doing it in the next PR: #1921.

If I make changes here, anything changed in the previous PR will mess everything up in this PR.

---

crates/blockifier/src/test_utils/transfers_simulator.rs line 26 at r3 (raw file):

const RANDOMIZATION_SEED: u64 = 0;
const CHARGE_FEE: bool = false;
const TRANSACTION_VERSION: TransactionVersion = TransactionVersion(StarkFelt::ONE);

See previous comment.

[avi-starkware]

Reviewable status: 2 of 6 files reviewed, 2 unresolved discussions (waiting on @noaov1, @OriStarkware, and @Yoni-Starkware)

---

crates/blockifier/src/test_utils/transfers_simulator.rs line 25 at r3 (raw file):

Previously, avi-starkware wrote…

This is a block of code I moved from elsewhere. I suggest not changing anything in this file in this PR, and doing it in the next PR: #1921.

If I make changes here, anything changed in the previous PR will mess everything up in this PR.

I changed it to true in #1921.

---

crates/blockifier/src/test_utils/transfers_simulator.rs line 26 at r3 (raw file):

Previously, avi-starkware wrote…

See previous comment.

Done in #1921.

[OriStarkware]

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

[noaov1]

Reviewed 3 of 4 files at r4, 1 of 1 files at r5, all commit messages.
Reviewable status: complete! all files reviewed, all discussions resolved (waiting on @Yoni-Starkware)

---

crates/blockifier/Cargo.toml line 57 at r1 (raw file):

Previously, avi-starkware wrote…

Test code is compiled when the create is built, so it is not considered a dev-dependency when we use it in test-utils as opposed to when we use it in the benchmark.

Does it have some negative affect?

[OriStarkware]

Reviewable status: complete! all files reviewed, all discussions resolved (waiting on @Yoni-Starkware)

The base branch was changed.

[avi-starkware]

crates/blockifier/Cargo.toml line 57 at r1 (raw file):

Previously, noaov1 (Noa Oved) wrote…

Does it have some negative affect?

I changed it to be the same as rstest:

1. set optional = true.
2. add rand to the feature testing.
3. add rand to dev-dependencies.

This ensures that rand is available for cargo test and cargo bench, but is not included in the production build.

[noaov1]

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

---

crates/blockifier/Cargo.toml line 57 at r1 (raw file):

Previously, avi-starkware wrote…

I changed it to be the same as rstest:

1. set optional = true.
2. add rand to the feature testing.
3. add rand to dev-dependencies.

This ensures that rand is available for cargo test and cargo bench, but is not included in the production build.

Nice!

[noaov1]

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