fix(ci): repair EE spec test workflows#1836
Conversation
|
Commit: 48d8dda SP1 Execution Results
|
Codecov Report❌ Patch coverage is
@@ Coverage Diff @@
## main #1836 +/- ##
==========================================
- Coverage 79.68% 79.62% -0.06%
==========================================
Files 673 674 +1
Lines 74661 74996 +335
==========================================
+ Hits 59495 59718 +223
- Misses 15166 15278 +112
Flags with carried forward coverage won't be shown. Click here to find out more.
... and 38 files with indirect coverage changes 🚀 New features to boost your workflow:
|
voidash
force-pushed
the
fix/ee-spec-ci-workflows
branch
from
1e97350 to
5141526
Compare
voidash
requested review from
alexhui01,
prajwolrg,
purusang and
storopoli
as code owners
| for _i in $(seq 1 60); do | ||
| if curl -sf -X POST http://localhost:30303 \ | ||
| -H "Content-Type: application/json" \ | ||
| -d '{"jsonrpc":"2.0","method":"eth_blockNumber","params":[],"id":1}' > /dev/null 2>&1; then | ||
| echo "alpen-client RPC is up" | ||
| exit 0 | ||
| fi | ||
| sleep 2 | ||
| done |
There was a problem hiding this comment.
clever, should we move this to a standalone script in contrib/ so we can have full shellcheck lints on it?
| --tx-wait-timeout 30 \ | ||
| --tx-wait-timeout 120 \ |
There was a problem hiding this comment.
running it multiple times. For the first time, 30 didn't work but 120 worked for me so...
| -m state_test \ | ||
| --fork=Shanghai \ | ||
| --fork=Prague \ | ||
| "--deselect=tests/frontier/opcodes/test_call.py::test_call_memory_expands_on_early_revert[fork_Prague-state_test]" \ |
There was a problem hiding this comment.
let's do it properly -- https://github.com/alpenlabs/execution-spec-tests/blob/main/skip_tests.yaml
| working-directory: docker | ||
| run: | | ||
| chmod +x test_ee_proof.sh | ||
| ./test_ee_proof.sh local |
There was a problem hiding this comment.
the whole purpose of this entire pipeline, at least the way I designed it, was to have our EE proofs (for now, chunk proofs) to be generated against the EF tests load (e.g. against the blocks with all these crazy transactions).
without this, we are quite literally testing pure Reth EVM against EF tests (which is pointless).
IMO, the essense of the effort should be directed towards restoring that logic (and re-adopting it for chunk proofs).
| # Execution-spec tests only need a sequencer RPC endpoint. Keep this | ||
| # isolated from L1 DA/prover services so EEST failures reflect EE | ||
| # execution behavior rather than Bitcoin publishing. | ||
| "alpen_eest": AlpenClientEnv(fullnode_count=0, enable_l1_da=False), |
There was a problem hiding this comment.
I dont know why we'd want a different alpen client for tests.
IMO we want exactly the same env (with the same exact binary under the hood as for the usual ee tests)
| # Alpen-client (EE) environments | ||
| "alpen_ee": AlpenClientEnv(enable_l1_da=True), | ||
| # Execution-spec tests only need a sequencer RPC endpoint. Keep this | ||
| # isolated from L1 DA/prover services so EEST failures reflect EE |
There was a problem hiding this comment.
with l1 da disabled, I'm not sure if the batch would ever be "proven", even in native (according to the batch lifecycle, something should be posted on l1 before it transitions to proved).
|
|
||
| Options: | ||
| --baseline-file FILE File containing byte offset captured before EEST. | ||
| --log-file FILE Alpen-client service.log to inspect. |
There was a problem hiding this comment.
i'd actually prefer a cleaner assertion path... a test-only RPC (if it doesnt exist yet) exposing parts that you need (chunk/batch/proof status, right from the storage) would not hurt.
we are overly relying on logs, and maintaining such fragile thing would be a nightmare.
P.S. we used to have getProofStatus or smth similar (at TN1 era) to actually assert stuff. Similar thing (whatever bits you need -- proof or chunk or batch status) can be easily exposed and used reliably.
There was a problem hiding this comment.
can you provide a summary of what's happening here (as well as why we need changes)?
frankly, I dont see a single reason to change ee binary as part of this work (besides perhaps adding test only RPC to live-introspect some specific state/storage bits).
4 participants
Summary
strata-zkvm-hostsoptional foralpen-clientso the native sequencer-only CI build works withoutsp1.tests/frontier/opcodes/test_call.py::test_call_memory_expands_on_early_revert[fork_Prague-state_test].