EIP-2681: Limit account nonce to 2^64-1 - Create Test Cases#454
EIP-2681: Limit account nonce to 2^64-1 - Create Test Cases#454SamWilsn merged 2 commits intoethereum:masterfrom
Conversation
Codecov Report
@@ Coverage Diff @@
## master #454 +/- ##
==========================================
+ Coverage 79.36% 79.45% +0.08%
==========================================
Files 187 187
Lines 9864 9864
==========================================
+ Hits 7829 7837 +8
+ Misses 2035 2027 -8
Flags with carried forward coverage won't be shown. Click here to find out more.
Continue to review full report at Codecov.
|
| json_data = json.load(fp)[f"{test_name}_{network}"] | ||
| data = json.load(fp) | ||
|
|
||
| # Some newer test files have for example _d0g0v0_ |
There was a problem hiding this comment.
Do we know what these blobs mean?
cc. @lightclient
There was a problem hiding this comment.
I am not entirely sure. But they do appear in both, the legacy as well as non-legacy tests. In the non-legacy tests, there is a slight discrepancy that this pattern does not appear in the file name but appears in the keys within, which is why I had to implement this pattern matching
| # Some newer test files have for example _d0g0v0_ | ||
| # between test_name and network | ||
| keys_to_search = re.compile( | ||
| f"{test_name}_d[0-9]g[0-9]v[0-9]_{network}" |
There was a problem hiding this comment.
All the regex meta-characters that we have passed in will be appropriately handled by re.compile. And no meta-characters appear in the test_name as far as I have seen. So, we should be ok.
There was a problem hiding this comment.
Instead of using the same function for legacy tests and the new tests, would it make sense to have a separate function for handling the new tests?
voith
left a comment
voith
left a comment
There was a problem hiding this comment.
LGTM, Besides the one comment that I left.
| # Some newer test files have for example _d0g0v0_ | ||
| # between test_name and network | ||
| keys_to_search = re.compile( | ||
| f"{test_name}_d[0-9]g[0-9]v[0-9]_{network}" |
There was a problem hiding this comment.
Instead of using the same function for legacy tests and the new tests, would it make sense to have a separate function for handling the new tests?
|
@voith @SamWilsn Thanks for the feedback. After considering the points that you raised, I did a detailed analysis of the Here are the important points
I say we just keep it simple and handle it all with the same regex I have already tested this and it picks up exactly all the tests that we've been running thus far. Let me know what you think and I'll update the PR accordingly. |
|
Updated as described in the comment above |
|
I think, for the time being, I'm fine with using a single function for both styles of test. |
* feat(fw): Add sha256 hash to fixtures * feat: add hasher script * changelog * src/ethereum_test_tools/test_filling: add `_info/hash` to framework tests. (ethereum#16) * fix(entry): Add short flags * docs: add hasher to `verifying_changes.md` --------- Co-authored-by: spencer <[email protected]>
* feat(fw): Add sha256 hash to fixtures * feat: add hasher script * changelog * src/ethereum_test_tools/test_filling: add `_info/hash` to framework tests. (#16) * fix(entry): Add short flags * docs: add hasher to `verifying_changes.md` --------- Co-authored-by: spencer <[email protected]>
5 participants
(closes #401 )
What was wrong?
The Non-Legacy tests were not implemented.
These tests include ones for exceptional halting of CREATE opcode when the nonce is high
Related to Issue #401
How was it fixed?
Added the High Nonce tests out of
tests/fixtures/BlockchainTests/GeneralStateTests/Cute Animal Picture