chore(blockifier_reexecution): add blockifier reexecution test to ci

[aner-starkware]

No description provided.

[reviewable-StarkWare]

This change is 

[github-actions]

Artifacts upload triggered. View details here

[github-actions]

Artifacts upload triggered. View details here

[github-actions]

Artifacts upload triggered. View details here

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 42.14%. Comparing base (e3165c4) to head (eb9fc34).
Report is 561 commits behind head on main.

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #1857      +/-   ##
==========================================
+ Coverage   40.10%   42.14%   +2.04%     
==========================================
  Files          26      204     +178     
  Lines        1895    23943   +22048     
  Branches     1895    23943   +22048     
==========================================
+ Hits          760    10091    +9331     
- Misses       1100    13386   +12286     
- Partials       35      466     +431     

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

---

🚨 Try these New Features:

• Flaky Tests Detection - Detect and resolve failed and flaky tests
• JS Bundle Analysis - Avoid shipping oversized bundles

[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

[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

[aner-starkware]

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

---

.github/workflows/blockifier_reexecution_ci.yml line 9 at r1 (raw file):

Previously, dorimedini-starkware wrote…

do we want to run this CI on post-merge? I doubt it; it should only block PRs

Done.

---

.github/workflows/blockifier_reexecution_ci.yml line 11 at r1 (raw file):

Previously, dorimedini-starkware wrote…

irrelevant (copied from somewhere?)

Done.

---

.github/workflows/blockifier_reexecution_ci.yml line 22 at r1 (raw file):

Previously, dorimedini-starkware wrote…

move this down (sort this list)

Done.

---

.github/workflows/blockifier_reexecution_ci.yml line 26 at r1 (raw file):

Previously, dorimedini-starkware wrote…

why should these three trigger this workflow?

I basically copied everything from the blockifier CI, reasoning that if there's a reason to run the blockifier checks, then reexecution checks should also be run

---

.github/workflows/blockifier_reexecution_ci.yml line 44 at r1 (raw file):

Previously, dorimedini-starkware wrote…

see above - don't run this workflow on push events at all, and remove this if

Done.

---

.github/workflows/blockifier_reexecution_ci.yml line 52 at r1 (raw file):

Previously, dorimedini-starkware wrote…

the point with public access was to allow running this CI without secrets.
otherwise, external PRs from external devs cannot run the CI.
why do you need this for read access?

That's not what I recall; look at your request in the conversation.

---

.github/workflows/blockifier_reexecution_ci.yml line 57 at r1 (raw file):

Previously, dorimedini-starkware wrote…

I can see that the reexecution itself did not run. the raw json tests ran, but that's it.

once you fix this, please check which is faster: with or without --release.

1. how much time required for compilation in both cases?
2. how much time required for running the test in both cases?

OK, will check.

[aner-starkware]

Previously, dorimedini-starkware wrote…

we now need a README.md file for this crate. Can you write one?
(it can be a separate PR)
the readme should cover:

1. All CLI commands and their meaning
2. How to run the RPC tests (using TEST_URL=<ENDPOINT>)

OK, will do it in a separate PR.

[aner-starkware]

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

---

crates/blockifier_reexecution/resources/offline_reexecution_files_prefix line 1 at r1 (raw file):

Previously, dorimedini-starkware wrote…

not that it matters much, but... where is this value from?

Hash of some commit; probably already amended that commit or rebased, but the hash doesn't change (since it's the folder - need to change it manually)

---

crates/blockifier_reexecution/src/state_reader/raw_rpc_json_test.rs line 170 at r1 (raw file):

Previously, dorimedini-starkware wrote…

also blocking because blocking self sometimes doesn't actually work

Done.

[aner-starkware]

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

---

.github/workflows/blockifier_ci.yml line 57 at r1 (raw file):

Previously, dorimedini-starkware wrote…

I think this is still worth keeping. I know reexecution feature always activates this serde feature but we shouldn't assume this will be true forever IMO.

Done.

[aner-starkware]

.github/workflows/blockifier_reexecution_ci.yml line 57 at r1 (raw file):

Previously, aner-starkware wrote…

OK, will check.

Actually, I think it was reversed - the reexecution tests ran, but all other tests did not. Now all tests run... But I'll split the tests file (in a separate PR), so that it will make more sense.

[dorimedini-starkware]

Reviewed 2 of 3 files at r2, 1 of 1 files at r3, 1 of 1 files at r4, all commit messages.
Reviewable status: all files reviewed, 4 unresolved discussions (waiting on @aner-starkware and @AvivYossef-starkware)

---

.github/workflows/blockifier_reexecution_ci.yml line 26 at r1 (raw file):

Previously, aner-starkware wrote…

I basically copied everything from the blockifier CI, reasoning that if there's a reason to run the blockifier checks, then reexecution checks should also be run

If there's a reason to run blockifier checks, then yes,
but

1. changes in the blockifier CI are changes in the checks, not changes in the crate, so no reason to trigger reexecution checks
2. native blockifier is not a dependency of reexecution, so changes in the native blockifier shouldn't trigger reexecution - the blockifier CI also runs native blockifier stuff (actually, only the upload_artifacts workflow should really be triggered on changes to native blockifier, we can probably update the blockifier CI triggers to reflect this).

[dorimedini-starkware]

Reviewable status: all files reviewed, 4 unresolved discussions (waiting on @aner-starkware and @AvivYossef-starkware)

---

.github/workflows/blockifier_reexecution_ci.yml line 57 at r1 (raw file):

Previously, aner-starkware wrote…

Actually, I think it was reversed - the reexecution tests ran, but all other tests did not. Now all tests run... But I'll split the tests file (in a separate PR), so that it will make more sense.

without --release, compilation takes 3:20 and reexecution seems to take ~15 minutes.
also, I don't see that the 0_13_1_1 case finished, but the tests seem to have passed... why?
also, 15 minutes is a bit much, please try again with --release... hopefully it should decrease the runtime dramatically

[aner-starkware]

.github/workflows/blockifier_reexecution_ci.yml line 57 at r1 (raw file):

Previously, dorimedini-starkware wrote…

without --release, compilation takes 3:20 and reexecution seems to take ~15 minutes.
also, I don't see that the 0_13_1_1 case finished, but the tests seem to have passed... why?
also, 15 minutes is a bit much, please try again with --release... hopefully it should decrease the runtime dramatically

Without --release: compilation 4 minutes, tests 14 minutes (total: 18 minutes); this solution isn't scalable, if you test reexecution on more blocks then it will grow significantly...

With --release: compilation 10 minutes, tests 1 minute (total 11 minutes); this solution is more scalable since you can add many tests and it won't dramatically increase the time

[dorimedini-starkware]

Reviewable status: all files reviewed, 4 unresolved discussions (waiting on @aner-starkware and @AvivYossef-starkware)

---

.github/workflows/blockifier_reexecution_ci.yml line 57 at r1 (raw file):

Previously, aner-starkware wrote…

Without --release: compilation 4 minutes, tests 14 minutes (total: 18 minutes); this solution isn't scalable, if you test reexecution on more blocks then it will grow significantly...

With --release: compilation 10 minutes, tests 1 minute (total 11 minutes); this solution is more scalable since you can add many tests and it won't dramatically increase the time

cool! let's go with --release then

[aner-starkware]

.github/workflows/blockifier_reexecution_ci.yml line 57 at r1 (raw file):

Previously, dorimedini-starkware wrote…

cool! let's go with --release then

test state_reader::raw_rpc_json_test::test_block_reexecution::case_03_v_0_13_1_1 ... ok
Seems to have finished...

[dorimedini-starkware]

Reviewable status: all files reviewed, 4 unresolved discussions (waiting on @aner-starkware and @AvivYossef-starkware)

---

.github/workflows/blockifier_reexecution_ci.yml line 57 at r1 (raw file):

Previously, aner-starkware wrote…

test state_reader::raw_rpc_json_test::test_block_reexecution::case_03_v_0_13_1_1 ... ok
Seems to have finished...

ah, missed that..

[aner-starkware]

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

---

.github/workflows/blockifier_reexecution_ci.yml line 26 at r1 (raw file):

Previously, dorimedini-starkware wrote…

If there's a reason to run blockifier checks, then yes,
but

1. changes in the blockifier CI are changes in the checks, not changes in the crate, so no reason to trigger reexecution checks
2. native blockifier is not a dependency of reexecution, so changes in the native blockifier shouldn't trigger reexecution - the blockifier CI also runs native blockifier stuff (actually, only the upload_artifacts workflow should really be triggered on changes to native blockifier, we can probably update the blockifier CI triggers to reflect this).

Done.

---

.github/workflows/blockifier_reexecution_ci.yml line 31 at r1 (raw file):

Previously, dorimedini-starkware wrote…

why? reexecution doesn't depend on native blockifier

Done.

---

.github/workflows/blockifier_reexecution_ci.yml line 57 at r1 (raw file):

Previously, dorimedini-starkware wrote…

ah, missed that..

Done.

[dorimedini-starkware]

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

---

.github/workflows/blockifier_reexecution_ci.yml line 52 at r1 (raw file):

Previously, aner-starkware wrote…

That's not what I recall; look at your request in the conversation.

discussed offline, we want a secret-less solution..

[dorimedini-starkware]

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

[aner-starkware]

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

---

.github/workflows/blockifier_reexecution_ci.yml line 52 at r1 (raw file):

Previously, dorimedini-starkware wrote…

discussed offline, we want a secret-less solution..

In the end continuing with secrets for now?

[dorimedini-starkware]

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

---

.github/workflows/blockifier_reexecution_ci.yml line 52 at r1 (raw file):

Previously, aner-starkware wrote…

In the end continuing with secrets for now?

yes

[dorimedini-starkware]

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