core, tests: don't create account if deployment destination is existent

core/state/statedb.go

@@ -618,16 +618,16 @@ func (s *StateDB) setStateObject(object *stateObject) {
 func (s *StateDB) getOrNewStateObject(addr common.Address) *stateObject {
 	stateObject := s.getStateObject(addr)
 	if stateObject == nil {
-		stateObject, _ = s.createObject(addr)
+		stateObject = s.createObject(addr)
 	}
 	return stateObject
 }
 
 // createObject creates a new state object. If there is an existing account with
 // the given address, it is overwritten and returned as the second return value.
-func (s *StateDB) createObject(addr common.Address) (newobj, prev *stateObject) {
-	prev = s.getDeletedStateObject(addr) // Note, prev might have been deleted, we need that!
-	newobj = newObject(s, addr, nil)
+func (s *StateDB) createObject(addr common.Address) *stateObject {
+	prev := s.getDeletedStateObject(addr) // Note, prev might have been deleted, we need that!
+	newobj := newObject(s, addr, nil)
 	if prev == nil {
 		s.journal.append(createObjectChange{account: &addr})
 	} else {
@@ -659,27 +659,15 @@ func (s *StateDB) createObject(addr common.Address) (newobj, prev *stateObject)
 		delete(s.storagesOrigin, prev.address)
 	}
 	s.setStateObject(newobj)
-	if prev != nil && !prev.deleted {
-		return newobj, prev
-	}
-	return newobj, nil
+	return newobj
 }
 
-// CreateAccount explicitly creates a state object. If a state object with the address
-// already exists the balance is carried over to the new account.
-//
-// CreateAccount is called during the EVM CREATE operation. The situation might arise that
-// a contract does the following:
-//
-//  1. sends funds to sha(account ++ (nonce + 1))
-//  2. tx_create(sha(account ++ nonce)) (note that this gets the address of 1)
-//
-// Carrying over the balance ensures that Ether doesn't disappear.
+// 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) {
-	newObj, prev := s.createObject(addr)
-	if prev != nil {
-		newObj.setBalance(prev.data.Balance)
-	}
+	s.createObject(addr)
 }
 
 // Copy creates a deep, independent copy of the state.

core/vm/evm.go

@@ -433,8 +433,9 @@ func (evm *EVM) create(caller ContractRef, codeAndHash *codeAndHash, gas uint64,
 		return nil, common.Address{}, gas, ErrNonceUintOverflow
 	}
 	evm.StateDB.SetNonce(caller.Address(), nonce+1)
-	// We add this to the access list _before_ taking a snapshot. Even if the creation fails,
-	// the access-list change should not be rolled back
+
+	// We add this to the access list _before_ taking a snapshot. Even if the
+	// creation fails, the access-list change should not be rolled back.
 	if evm.chainRules.IsBerlin {
 		evm.StateDB.AddAddressToAccessList(address)
 	}
@@ -443,9 +444,14 @@ func (evm *EVM) create(caller ContractRef, codeAndHash *codeAndHash, gas uint64,
 	if evm.StateDB.GetNonce(address) != 0 || (contractHash != (common.Hash{}) && contractHash != types.EmptyCodeHash) {
 		return nil, common.Address{}, 0, ErrContractAddressCollision
 	}
-	// Create a new account on the state
+	// Create a new account on the state only if the object was not present.
+	// It might be possible the contract code is deployed to a pre-existent
+	// account with non-zero balance and potential non-empty storage. If so,
+	// inherit the leftover balance and storage instead of clearing it.
 	snapshot := evm.StateDB.Snapshot()
+	if !evm.StateDB.Exist(address) {
 		evm.StateDB.CreateAccount(address)
+	}
 	if evm.chainRules.IsEIP158 {
 		evm.StateDB.SetNonce(address, 1)
 	}

core/vm/interface.go

@@ -60,6 +60,7 @@ type StateDB interface {
 	// Exist reports whether the given account exists in state.
 	// Notably this should also return true for self-destructed accounts.
 	Exist(common.Address) bool
+
 	// Empty returns whether the given account is empty. Empty
 	// is defined according to EIP161 (balance = nonce = code = 0).
 	Empty(common.Address) bool

tests/block_test.go

@@ -50,6 +50,11 @@ 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) {
 		if runtime.GOARCH == "386" && runtime.GOOS == "windows" && rand.Int63()%2 == 0 {
 			t.Skip("test (randomly) skipped on 32-bit windows")
@@ -68,6 +73,14 @@ func TestExecutionSpecBlocktests(t *testing.T) {
 	}
 	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)
 	})

tests/state_test.go

@@ -55,6 +55,18 @@ func initMatcher(st *testMatcher) {
 	// Uses 1GB RAM per tested fork
 	st.skipLoad(`^stStaticCall/static_Call1MB`)
 
+	// These tests fail as of https://github.com/ethereum/go-ethereum/pull/28666, since we
+	// no longer delete "leftover storage" when deploying a contract.
+	st.skipLoad(`^stSStoreTest/InitCollision\.json`)
+	st.skipLoad(`^stRevertTest/RevertInCreateInInit\.json`)
+	st.skipLoad(`^stExtCodeHash/dynamicAccountOverwriteEmpty\.json`)
+	st.skipLoad(`^stCreate2/create2collisionStorage\.json`)
+	st.skipLoad(`^stCreate2/RevertInCreateInInitCreate2\.json`)
+	st.skipLoad(`^stRevertTest/RevertInCreateInInit\.json`)
+	st.skipLoad(`^stExtCodeHash/dynamicAccountOverwriteEmpty\.json`)
+	st.skipLoad(`^stCreate2/create2collisionStorage\.json`)
+	st.skipLoad(`^stCreate2/RevertInCreateInInitCreate2\.json`)
+
 	// Broken tests:
 	// EOF is not part of cancun
 	st.skipLoad(`^stEOF/`)