add EIP 6780: only SELFDESTRUCT in same transaction #819
Conversation
Codecov ReportAttention:
Additional details and impacted files@@ Coverage Diff @@
## forks/cancun #819 +/- ##
================================================
- Coverage 69.96% 69.64% -0.32%
================================================
Files 610 611 +1
Lines 34295 34898 +603
================================================
+ Hits 23993 24306 +313
- Misses 10302 10592 +290
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
| # mark beneficiary as touched | ||
| if account_exists_and_is_empty(evm.env.state, beneficiary): | ||
| evm.touched_accounts.add(beneficiary) |
There was a problem hiding this comment.
Should the beneficiary account be touched (in the EIP-158 sense) in both cases? (poking @petertdavies since he knows more about that topic.)
There was a problem hiding this comment.
Yes, this should be outside the if block.
| evm.touched_accounts.add(beneficiary) | ||
|
|
||
| # HALT the execution | ||
| evm.running = False |
There was a problem hiding this comment.
Hm, I think this'll also lead to an issue. If evm.running is not set to False, then you need to increment the program counter.
There was a problem hiding this comment.
Execution of the stack frame should end and return regardless of whether the contract was marked for deletion. So this should also be outside the if block.
SamWilsn
force-pushed
the
forks/cancun
branch
from
673ccec to
fcf2cd6
Compare
SamWilsn
force-pushed
the
selfdestruct-removal
branch
from
03a2844 to
c7802d4
Compare
| @@ -522,15 +523,18 @@ def selfdestruct(evm: Evm) -> None: | |||
| # beneficiary). | |||
| set_account_balance(evm.env.state, originator, U256(0)) | |||
There was a problem hiding this comment.
Balance transfer also has to only happen under certain conditions i.e. if the contract was created in the same transaction or if the beneficiary and originator are different.
gurukamath
force-pushed
the
selfdestruct-removal
branch
from
c7802d4 to
d84a075
Compare
|
@SamWilsn @petertdavies Have updated the implementation |
gurukamath
force-pushed
the
selfdestruct-removal
branch
from
d84a075 to
972fa5c
Compare
| # register account for deletion only if it was created | ||
| # in the same transaction | ||
| if originator in evm.env.state.created_accounts: | ||
| evm.accounts_to_delete.add(originator) |
There was a problem hiding this comment.
Do we still need to zero the balance to support selfdestruct(this) in the same transaction?
gurukamath
force-pushed
the
selfdestruct-removal
branch
from
4d75baa to
85cc3e5
Compare
This is my first shot at implementing EIP 6780 in EELS. I'm no Python expert, so this will probably require an extra cautious look-over.
Cute Animal Picture