chore(blockifier_reexecution): add parallel offline reexecution

[aner-starkware]

No description provided.

[reviewable-StarkWare]

This change is 

[github-actions]

Artifacts upload triggered. View details here

[codecov]

Codecov Report

Attention: Patch coverage is 0% with 69 lines in your changes missing coverage. Please review.

Project coverage is 76.88%. Comparing base (e3165c4) to head (15e7ba3).
Report is 366 commits behind head on main.

Files with missing lines	Patch %	Lines
crates/blockifier_reexecution/src/main.rs	0.00%	67 Missing ⚠️
...s/blockifier_reexecution/src/state_reader/utils.rs	0.00%	2 Missing ⚠️

Additional details and impacted files

@@             Coverage Diff             @@
##             main    #1981       +/-   ##
===========================================
+ Coverage   40.10%   76.88%   +36.77%     
===========================================
  Files          26      373      +347     
  Lines        1895    39742    +37847     
  Branches     1895    39742    +37847     
===========================================
+ Hits          760    30555    +29795     
- Misses       1100     6913     +5813     
- Partials       35     2274     +2239     

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

[dorimedini-starkware]

Reviewed 5 of 5 files at r1, all commit messages.
Reviewable status: all files reviewed, 4 unresolved discussions (waiting on @aner-starkware and @AvivYossef-starkware)

---

crates/blockifier_reexecution/src/main.rs line 92 at r1 (raw file):

        /// get_block_numbers_for_reexecution().
        #[clap(long, short = 'b', num_args = 0..)]
        block_numbers: Vec<u64>,

shouldn't the type be Option<Vec<u64>>?

Code quote:

        /// Block numbers. If not specified, blocks are retrieved from
        /// get_block_numbers_for_reexecution().
        #[clap(long, short = 'b', num_args = 0..)]
        block_numbers: Vec<u64>,

---

crates/blockifier_reexecution/src/main.rs line 168 at r1 (raw file):

                let full_file_path =
                    format!("{directory_path}/block_{block_number}/reexecution_data.json");
                tokio::task::spawn_blocking(move || {

please add a comment like this over every spawn_blocking line

Suggestion:

// RPC calls are "synchronous IO" (see
// [here](https://stackoverflow.com/questions/74547541/when-should-you-use-tokios-spawn-blocking)
// for details).
tokio::task::spawn_blocking(move || {

---

crates/blockifier_reexecution/src/main.rs line 197 at r1 (raw file):

                0 => get_block_numbers_for_reexecution(),
                _ => block_numbers.into_iter().map(BlockNumber).collect(),
            };

change above to Option and then you can do this

Suggestion:

            let block_numbers = block_numbers
                .map(|block_numbers| block_numbers.into_iter().map(BlockNumber).collect())
                .unwrap_or(get_block_numbers_for_reexecution();

---

crates/blockifier_reexecution/src/main.rs line 209 at r1 (raw file):

                    println!("Reexecution test for block {block} passed successfully.");
                }));
            }

non-blocking

Suggestion:

            let threads: Vec<JoinHandle<_>> = block_numbers
                .iter()
                .map(|block| {
                let full_file_path =
                    format!("{directory_path}/block_{block}/reexecution_data.json");
                tokio::task::spawn(async move {
                    reexecute_and_verify_correctness(
                        OfflineConsecutiveStateReaders::new_from_file(&full_file_path).unwrap(),
                    );
                    println!("Reexecution test for block {block} passed successfully.");
                });
                })
                .collect()

[github-actions]

Artifacts upload triggered. View details here

[aner-starkware]

Reviewable status: 4 of 5 files reviewed, 3 unresolved discussions (waiting on @AvivYossef-starkware and @dorimedini-starkware)

---

crates/blockifier_reexecution/src/main.rs line 92 at r1 (raw file):

Previously, dorimedini-starkware wrote…

shouldn't the type be Option<Vec<u64>>?

Done.

---

crates/blockifier_reexecution/src/main.rs line 168 at r1 (raw file):

Previously, dorimedini-starkware wrote…

please add a comment like this over every spawn_blocking line

Done.

---

crates/blockifier_reexecution/src/main.rs line 197 at r1 (raw file):

Previously, dorimedini-starkware wrote…

change above to Option and then you can do this

Done.

---

crates/blockifier_reexecution/src/main.rs line 209 at r1 (raw file):

Previously, dorimedini-starkware wrote…

non-blocking

I think it's a bit strange to spawn inside a mapping, I prefer it to be clearer when new threads are spawned.

[dorimedini-starkware]

Reviewed 1 of 1 files at r2, all commit messages.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @aner-starkware and @AvivYossef-starkware)

---

crates/blockifier_reexecution/src/main.rs line 204 at r2 (raw file):

            let block_numbers = block_numbers
                .map(|block_numbers| block_numbers.into_iter().map(BlockNumber).collect())
                .unwrap_or(get_block_numbers_for_reexecution());

duplicated logic - please put in a function and use twice

Suggestion:

            let block_numbers = parse_block_numbers_arg(block_numbers);