feat(spec-spec, tests): Implement eip-8246 and testing scenario#2842
feat(spec-spec, tests): Implement eip-8246 and testing scenario#2842LouisTsai-Csie wants to merge 3 commits into
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## eips/amsterdam/eip-8246 #2842 +/- ##
===========================================================
- Coverage 90.01% 89.79% -0.23%
===========================================================
Files 539 487 -52
Lines 32618 28885 -3733
Branches 3030 2621 -409
===========================================================
- Hits 29361 25937 -3424
+ Misses 2699 2433 -266
+ Partials 558 515 -43
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
marioevz
left a comment
marioevz
left a comment
There was a problem hiding this comment.
Looks good overall. I think we need to take care of failing tests in ./tests/cancun/eip6780_selfdestruct before eventually merging into forks/amsterdam (if this gets included in a devnet).
|
|
||
| """ | ||
| account.nonce = Uint(0) | ||
| account.code_hash = EMPTY_CODE_HASH |
There was a problem hiding this comment.
Might be worth to double check whether this is sufficient to clear the code. cc @gurukamath as he probably knows the answer.
There was a problem hiding this comment.
Yes. This is equivalent to resetting the code.
| set_account(tx_state, address, None) | ||
|
|
||
|
|
||
| def preserve_account_balance(account: Account) -> None: |
There was a problem hiding this comment.
How about creating a convert_to_balance_only_account (or something similar) which has all the semantics of destroy_account minus the balance bit and simply calling that in fork.py?
|
|
||
| """ | ||
| account.nonce = Uint(0) | ||
| account.code_hash = EMPTY_CODE_HASH |
There was a problem hiding this comment.
Yes. This is equivalent to resetting the code.
| @@ -434,7 +434,7 @@ def destroy_account(tx_state: TransactionState, address: Address) -> None: | |||
| This function is made available exclusively for the ``SELFDESTRUCT`` | |||
There was a problem hiding this comment.
Docstring to be updated
| for address in tx_output.accounts_to_delete: | ||
| destroy_account(tx_state, address) | ||
| destroy_storage(tx_state, address) | ||
| modify_state(tx_state, address, preserve_account_balance) |
There was a problem hiding this comment.
If modify_state ends up calling destroy_account (zero-balance case), destroy_storage runs twice. Not a huge issue but something to be kept in mind
3 participants
🗒️ Description
Implement EIP-8246: https://forkcast.org/eips/8246
🔗 Related Issues or PRs
issue #2821
✅ Checklist
just statictype(scope):.mkdocs servelocally and verified the auto-generated docs for new tests in the Test Case Reference are correctly formatted.@ported_frommarker.Cute Animal Picture