-
Notifications
You must be signed in to change notification settings - Fork 207
feat(l1): bump zkevm EF-tests fixtures to v0.3.3 and extend stateless witness coverage to all for_amsterdam tests #6527
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from 7 commits
9d48e68
189b8a6
5a597e6
ad031d5
e216333
3571da7
5bd9c88
56439dd
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1 +1 @@ | ||
| https://github.com/ethereum/execution-spec-tests/releases/download/zkevm%40v0.3.0/fixtures_zkevm.tar.gz | ||
| https://github.com/ethereum/execution-spec-tests/releases/download/zkevm%40v0.3.3/fixtures_zkevm.tar.gz |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -21,20 +21,41 @@ const SKIPPED_BASE: &[&str] = &[ | |
| ]; | ||
|
|
||
| // Extra skips added only for prover backends. | ||
| #[cfg(feature = "sp1")] | ||
| #[cfg(all(feature = "sp1", not(feature = "stateless")))] | ||
| const EXTRA_SKIPS: &[&str] = &[ | ||
| // I believe these tests fail because of how much stress they put into the zkVM, they probably cause an OOM though this should be checked | ||
| "static_Call50000", | ||
| "Return50000", | ||
| "static_Call1MB1024Calldepth", | ||
| ]; | ||
| #[cfg(not(feature = "sp1"))] | ||
| #[cfg(feature = "stateless")] | ||
| const EXTRA_SKIPS: &[&str] = &[ | ||
| // zkevm@v0.3.3 tolerance tests: the fixture's `statelessOutputBytes` declares `valid = 1` | ||
| // because the executed path does not actually consume the malformed/extra/missing witness | ||
| // entry, but our RpcExecutionWitness conversion eagerly validates the full witness and | ||
| // rejects it. Re-enable once the witness conversion is lazy per EIP-8025 §Tolerance. | ||
| "validation_headers_malformed_rlp_header", | ||
| "validation_headers_missing_oldest_blockhash_ancestor", | ||
| "validation_headers_missing_parent_header", | ||
| "validation_state_extra_unused_trie_node", | ||
| // zkevm@v0.3.3 rejection tests: `statelessOutputBytes` declares `valid = 0` so the guest | ||
| // program must reject the deliberately-incomplete witness, but our stateless path runs | ||
| // to completion instead of detecting the missing entry. Re-enable once the witness | ||
| // completeness checks land (missing delegation/external-code bytecodes, non-contiguous | ||
| // header chain detection). | ||
| "validation_codes_missing_delegated_code_on_insufficient_balance_call", | ||
| "validation_codes_missing_external_code_read_target", | ||
| "validation_codes_missing_redelegation_old_marker", | ||
| "validation_codes_missing_sender_delegation_marker", | ||
| "validation_headers_non_contiguous_chain", | ||
| ]; | ||
| #[cfg(not(any(feature = "sp1", feature = "stateless")))] | ||
| const EXTRA_SKIPS: &[&str] = &[]; | ||
|
Comment on lines
-31
to
60
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. if sp1 is set but stateless isn't this won't compile, and if sp1 implies stateless then why specify both?
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
|
||
|
|
||
| // Select backend | ||
| #[cfg(feature = "stateless")] | ||
| const BACKEND: Option<BackendType> = Some(BackendType::Exec); | ||
| #[cfg(feature = "sp1")] | ||
| #[cfg(all(feature = "sp1", not(feature = "stateless")))] | ||
| const BACKEND: Option<BackendType> = Some(BackendType::SP1); | ||
| #[cfg(not(any(feature = "sp1", feature = "stateless")))] | ||
| const BACKEND: Option<BackendType> = None; | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
cfg(feature = "sp1")andcfg(feature = "stateless")aren't mutually exclusive at the cfg level — if both are enabled together, this file fails to compile (two definitions ofEXTRA_SKIPS). Either gate the third arm withcfg(all(not(feature="sp1"), not(feature="stateless")))(which it already does vianot(any(...))) AND add acompile_error!incfg(all(feature="sp1", feature="stateless"))to make the exclusivity explicit. Currently those features may be exclusive in practice, but the constraint isn't documented anywhere.Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
compile_error!is at lines 6–7. In ad031d5 I gated thesp1arms withnot(feature = "stateless")so the both-on combo now fails with only that diagnostic (previously two duplicate-definition errors rode along with it).