[evm] implement EIP-6780 SELFDESTRUCT only in same transaction #4218
Conversation
dustinxie
requested review from
CoderZhi,
Liuhaai,
envestcc and
a team
as code owners
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #4218 +/- ##
==========================================
+ Coverage 76.51% 76.69% +0.18%
==========================================
Files 340 340
Lines 29273 29303 +30
==========================================
+ Hits 22397 22475 +78
+ Misses 5761 5719 -42
+ Partials 1115 1109 -6 ☔ View full report in Codecov by Sentry. |
dustinxie
force-pushed
the
check
branch
2 times, most recently
from
0f33d6d
to
12dba38
Compare
|
|
| @@ -286,7 +307,7 @@ func (stateDB *StateDBAdapter) AddBalance(evmAddr common.Address, a256 *uint256. | |||
| if contract, ok := stateDB.cachedContract[addrHash]; ok { | |||
| state = contract.SelfState() | |||
| } else { | |||
| state, err = accountutil.LoadOrCreateAccount(stateDB.sm, addr, stateDB.accountCreationOpts()...) | |||
| state, _, err = accountutil.LoadOrCreateAccount(stateDB.sm, addr, stateDB.accountCreationOpts()...) | |||
There was a problem hiding this comment.
if contract address could be calculated, is it possible that AddBalance function is called and create the account before the cooresponding contract is created? if so, should this contract address be considered as "created in the same transaction"?
There was a problem hiding this comment.
checked ethereum code, it does not detect collision and will overwrite the existing account
// CreateAccount explicitly creates a new state object, assuming that the
// account did not previously exist in the state. If the account already
// exists, this function will silently overwrite it which might lead to a
// consensus bug eventually.
func (s *StateDB) CreateAccount(addr common.Address) {
s.createObject(addr)
}
our code will check and only create a new account if it does not exist yet. In this aspect, we are different from ethereum (and maybe better since we don't overwrite?)
|
| _, err = accountutil.LoadOrCreateAccount(stateDB.sm, addr, stateDB.accountCreationOpts()...) | ||
| if stateDB.assertError(err, "Failed to create account.", zap.Error(err), zap.String("address", evmAddr.Hex())) { | ||
| return | ||
| } |
There was a problem hiding this comment.
if the address already exist in cache, no need to create it
this is same as Ethereum's code behavior
There was a problem hiding this comment.
unit test doesn't cover this case
Description
Please include a summary of the changes and the related issue. Please also include relevant motivation and context. List any dependencies that are required for this change.
Fixes #(issue)
Type of change
Please delete options that are not relevant.
How Has This Been Tested?
Please describe the tests that you ran to verify your changes. Provide instructions so we can reproduce. Please also list any relevant details for your test configuration
Test Configuration:
Checklist: