diff --git a/core/state/journal.go b/core/state/journal.go index 89f0cd6ce0..224b147c22 100644 --- a/core/state/journal.go +++ b/core/state/journal.go @@ -128,6 +128,9 @@ type ( account *common.Address prevcode, prevhash []byte } + destructibleChange struct { + account common.Address + } // Changes to other state values. refundChange struct { @@ -254,6 +257,20 @@ func (ch codeChange) copy() journalEntry { } } +func (ch destructibleChange) revert(s *StateDB) { + s.getStateObject(ch.account).destructible = false +} + +func (ch destructibleChange) dirtied() *common.Address { + return nil // destruct-eligible flag is not considered as dirty +} + +func (ch destructibleChange) copy() journalEntry { + return destructibleChange{ + account: ch.account, + } +} + func (ch storageChange) revert(s *StateDB) { s.getStateObject(*ch.account).setState(ch.key, ch.prevalue) } diff --git a/core/state/state_object.go b/core/state/state_object.go index 2aeb83206c..e52a21dadd 100644 --- a/core/state/state_object.go +++ b/core/state/state_object.go @@ -55,7 +55,7 @@ type stateObject struct { trie Trie // storage trie, which becomes non-nil on first access code []byte // contract bytecode, which gets set when code is loaded - originStorage Storage // Storage cache of original entries to dedup rewrites + originStorage Storage // Storage cache of original entries to de-duplicate rewrites pendingStorage Storage // Storage entries that need to be flushed to disk, at the end of an entire block dirtyStorage Storage // Storage entries that have been modified in the current transaction execution, reset for every transaction @@ -66,8 +66,11 @@ type stateObject struct { // account is still accessible in the scope of same transaction. selfDestructed bool - // Flag whether the object was created in the current transaction - created bool + // This is an EIP-6780 flag indicating if the object is eligible for + // self-destruct. Potential scenarios as follows: + // - object is created in the current transaction + // - object was previously existent and is being deployed in current transaction + destructible bool } // empty returns whether the account is considered empty. @@ -77,10 +80,6 @@ func (s *stateObject) empty() bool { // newObject creates a state object. func newObject(db *StateDB, address common.Address, acct *types.StateAccount) *stateObject { - var ( - origin = acct - created = acct == nil // true if the account was not existent - ) if acct == nil { acct = types.NewEmptyStateAccount() } @@ -88,12 +87,11 @@ func newObject(db *StateDB, address common.Address, acct *types.StateAccount) *s db: db, address: address, addrHash: crypto.Keccak256Hash(address[:]), - origin: origin, + origin: acct, data: *acct, originStorage: make(Storage), pendingStorage: make(Storage), dirtyStorage: make(Storage), - created: created, } } @@ -246,6 +244,7 @@ func (s *stateObject) finalise(prefetch bool) { if len(s.dirtyStorage) > 0 { s.dirtyStorage = make(Storage) } + s.destructible = false // unset the flag at the end of transaction } // updateTrie is responsible for persisting cached storage changes into the @@ -450,7 +449,7 @@ func (s *stateObject) deepCopy(db *StateDB) *stateObject { obj.dirtyStorage = s.dirtyStorage.Copy() obj.dirtyCode = s.dirtyCode obj.selfDestructed = s.selfDestructed - obj.created = s.created + obj.destructible = s.destructible return obj } @@ -530,6 +529,16 @@ func (s *stateObject) setNonce(nonce uint64) { s.data.Nonce = nonce } +func (s *stateObject) SetDestructible() { + if s.destructible { + return // might be possible in fuzzing + } + s.db.journal.append(destructibleChange{ + account: s.address, + }) + s.destructible = true +} + func (s *stateObject) CodeHash() []byte { return s.data.CodeHash } diff --git a/core/state/statedb.go b/core/state/statedb.go index d9f6cc3430..ff516d5438 100644 --- a/core/state/statedb.go +++ b/core/state/statedb.go @@ -497,7 +497,7 @@ func (s *StateDB) Selfdestruct6780(addr common.Address) { if stateObject == nil { return } - if stateObject.created { + if stateObject.destructible { s.SelfDestruct(addr) } } @@ -659,7 +659,17 @@ func (s *StateDB) createObject(addr common.Address) *stateObject { // 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) + obj := s.createObject(addr) + obj.SetDestructible() +} + +// SetDestructible marks the object with specific address as destructible. +func (s *StateDB) SetDestructible(addr common.Address) { + obj := s.getStateObject(addr) + if obj == nil { + return // might be possible in fuzzing + } + obj.SetDestructible() } // Copy creates a deep, independent copy of the state. @@ -808,7 +818,6 @@ func (s *StateDB) Finalise(deleteEmptyObjects bool) { delete(s.accountsOrigin, obj.address) // Clear out any previously updated account data (may be recreated via a resurrect) delete(s.storagesOrigin, obj.address) // Clear out any previously updated storage data (may be recreated via a resurrect) } else { - obj.created = false obj.finalise(true) // Prefetch slots in the background s.markUpdate(addr) } diff --git a/core/state/statedb_test.go b/core/state/statedb_test.go index c4ba63c3a5..ce1fb31f9f 100644 --- a/core/state/statedb_test.go +++ b/core/state/statedb_test.go @@ -391,6 +391,18 @@ func newTestAction(addr common.Address, r *rand.Rand) testAction { s.SelfDestruct(addr) }, }, + { + name: "SelfDestruct6780", + fn: func(a testAction, s *StateDB) { + s.Selfdestruct6780(addr) + }, + }, + { + name: "SetDestructible", + fn: func(a testAction, s *StateDB) { + s.SetDestructible(addr) + }, + }, { name: "AddRefund", fn: func(a testAction, s *StateDB) { diff --git a/core/vm/evm.go b/core/vm/evm.go index 07cb8f51bb..d04eef2366 100644 --- a/core/vm/evm.go +++ b/core/vm/evm.go @@ -461,6 +461,11 @@ func (evm *EVM) create(caller ContractRef, codeAndHash *codeAndHash, gas uint64, snapshot := evm.StateDB.Snapshot() if !evm.StateDB.Exist(address) { evm.StateDB.CreateAccount(address) + } else { + // The account with the designated address previously existed but is + // still eligible for deployment. Explicitly set it as destructible + // to adhere to EIP-6780. + evm.StateDB.SetDestructible(address) } if evm.chainRules.IsEIP158 { evm.StateDB.SetNonce(address, 1) diff --git a/core/vm/interface.go b/core/vm/interface.go index 30742e96de..0300e25b47 100644 --- a/core/vm/interface.go +++ b/core/vm/interface.go @@ -56,6 +56,7 @@ type StateDB interface { SelfDestruct(common.Address) HasSelfDestructed(common.Address) bool + SetDestructible(addr common.Address) Selfdestruct6780(common.Address) diff --git a/tests/block_test.go b/tests/block_test.go index be4b6ee37d..75bb8255f5 100644 --- a/tests/block_test.go +++ b/tests/block_test.go @@ -49,11 +49,6 @@ func TestBlockchain(t *testing.T) { // using 4.6 TGas bt.skipLoad(`.*randomStatetest94.json.*`) - // The tests under Pyspecs are the ones that are published as execution-spect tests. - // We run these tests separately, no need to _also_ run them as part of the - // reference tests. - bt.skipLoad(`^Pyspecs/`) - bt.walk(t, blockTestDir, func(t *testing.T, name string, test *BlockTest) { execBlockTest(t, bt, test) }) @@ -68,15 +63,6 @@ func TestExecutionSpecBlocktests(t *testing.T) { t.Skipf("directory %s does not exist", executionSpecBlockchainTestDir) } bt := new(testMatcher) - - // These tests fail as of https://github.com/ethereum/go-ethereum/pull/28666, since we - // no longer delete "leftover storage" when deploying a contract. - bt.skipLoad(`^cancun/eip6780_selfdestruct/selfdestruct/self_destructing_initcode_create_tx.json`) - bt.skipLoad(`^cancun/eip6780_selfdestruct/selfdestruct/self_destructing_initcode.json`) - bt.skipLoad(`^cancun/eip6780_selfdestruct/selfdestruct/recreate_self_destructed_contract_different_txs.json`) - bt.skipLoad(`^cancun/eip6780_selfdestruct/selfdestruct/delegatecall_from_new_contract_to_pre_existing_contract.json`) - bt.skipLoad(`^cancun/eip6780_selfdestruct/selfdestruct/create_selfdestruct_same_tx.json`) - bt.walk(t, executionSpecBlockchainTestDir, func(t *testing.T, name string, test *BlockTest) { execBlockTest(t, bt, test) })