✨ feat(tests): EIP-7928 test cases for EIP-2930 transactions #2167
Conversation
raxhvl
added
scope:tests
raxhvl
changed the title
fselmo
force-pushed
the
feat/eip-7928/test-eip-2930
branch
from
94a7cc5 to
996efd8
Compare
There was a problem hiding this comment.
Hey @raxhvl, not a whole lot here. Looks great to me. Just one comment for a fix before we merge. I fixed the rebase issues and the linting after rebasing.
Do you think it makes sense to keep these as a separate file? I'm not against it but I don't see a whole lot of these cases building up. I want to make sure we start toward a good organization for these test cases. Curious on your thoughts.
tests/amsterdam/eip7928_block_level_access_lists/test_block_access_lists_eip2930_tx.py
Outdated
Show resolved
Hide resolved
|
Im okay moving this file, should we create a generic test for all BAL x other eip tests? something like |
I think we can make a better decision as the tests grow. I don't love the generic name either because some tests in the main file might also touch on EIPs, etc. For now, let's stick to the for-better-or-for-worse convention of being OK with many tests being in the "main" file (e.g. blob_txs, set_code_txs). |
🧹 chore(tests): Move to new test file ✨ feat(tests): EIP-7928test_bal_2930_slot_listed_and_unlisted_reads ✨ feat(tests): EIP-7928 test_bal_2930_slot_listed_and_unlisted_writes ✨ feat(tests): EIP-7928 test_bal_2930_slot_listed_but_untouched
fselmo
force-pushed
the
feat/eip-7928/test-eip-2930
branch
from
0c6bf2b to
8471ae5
Compare
fselmo
force-pushed
the
feat/eip-7928/test-eip-2930
branch
from
a954e50 to
817ad0f
Compare
|
Hey @raxhvl, your comment here made me want to double check all cases and I found another validation issue. This time it affected a test in the latest release. I think we should be good now 🤞🏼. The empty account changes expectation was not being validated properly. Wanted to flag the fix in this commit where there was a balance transfer present, so not entirely empty expectation. edit: Actually, the filling here should be fine. Since this was on the validation side of things and the specs were correct, we still use the correct BAL in our filled test... it's just the expectation validation that needed the change. Client teams should be OK. The BAL will just change to not include the balance transfer in the future. |
🗒️ Description
Test case for effects EIP-2930 transactions on EIP-7928.
cc: @nerolation
✅ Checklist
toxchecks to avoid unnecessary CI fails, see also Code Standards and Enabling Pre-commit Checks:uvx --with=tox-uv tox -e lint,typecheck,spellcheck,markdownlinttype(scope):.