Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

core/vm: reject contract creation if the storage is non-empty #28912

Merged
merged 3 commits into from
Apr 8, 2024
Merged

core/vm: reject contract creation if the storage is non-empty #28912

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
ef12453
core/vm: reject contract creation if the storage is non-empty
rjl493456442 Feb 2, 2024
84527f3
tests: remove duplication
rjl493456442 Apr 8, 2024
d42b1f6
tests: avoid duplicate state-tests
holiman Apr 8, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 9 additions & 3 deletions core/vm/evm.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -439,13 +439,19 @@ func (evm *EVM) create(caller ContractRef, codeAndHash *codeAndHash, gas uint64,
if evm.chainRules.IsBerlin {
evm.StateDB.AddAddressToAccessList(address)
}
// Ensure there's no existing contract already at the designated address
// Ensure there's no existing contract already at the designated address.
// Account is regarded as existent if any of these three conditions is met:
// - the nonce is nonzero
// - the code is non-empty
// - the storage is non-empty
contractHash := evm.StateDB.GetCodeHash(address)
if evm.StateDB.GetNonce(address) != 0 || (contractHash != (common.Hash{}) && contractHash != types.EmptyCodeHash) {
storageRoot := evm.StateDB.GetStorageRoot(address)
if evm.StateDB.GetNonce(address) != 0 ||
(contractHash != (common.Hash{}) && contractHash != types.EmptyCodeHash) || // non-empty code
(storageRoot != (common.Hash{}) && storageRoot != types.EmptyRootHash) { // non-empty storage
if evm.Config.Tracer != nil && evm.Config.Tracer.OnGasChange != nil {
evm.Config.Tracer.OnGasChange(gas, 0, tracing.GasChangeCallFailedExecution)
}

return nil, common.Address{}, 0, ErrContractAddressCollision
}
// Create a new account on the state
Expand Down
1 change: 1 addition & 0 deletions core/vm/interface.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,7 @@ type StateDB interface {
GetCommittedState(common.Address, common.Hash) common.Hash
GetState(common.Address, common.Hash) common.Hash
SetState(common.Address, common.Hash, common.Hash)
GetStorageRoot(addr common.Address) common.Hash

GetTransientState(addr common.Address, key common.Hash) common.Hash
SetTransientState(addr common.Address, key, value common.Hash)
Expand Down
13 changes: 13 additions & 0 deletions tests/block_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,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/`)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Interesting.
The exec spec tests are in a different location; the reference tests are at ./testdata/BlockchainTests, and spec tests at ./spec-tests-fixtures/blockchain_tests.

But yes, the exec spec tests are also imported in to the reference tests. For state-tests, it's GeneralStateTests/Pyspecs (we should skip those!), for blockchain tests, it seems to be BlockchainTests/GeneralStateTests/Pyspecs. That is, those are already skipped due to

	// General state tests are 'exported' as blockchain tests, but we can run them natively.
	// For speedier CI-runs, the line below can be uncommented, so those are skipped.
	// For now, in hardfork-times (Berlin), we run the tests both as StateTests and
	// as blockchain tests, since the latter also covers things like receipt root
	bt.skipLoad(`^GeneralStateTests/`)


bt.walk(t, blockTestDir, func(t *testing.T, name string, test *BlockTest) {
execBlockTest(t, bt, test)
})
Expand All @@ -64,6 +69,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)
})
Expand Down
8 changes: 8 additions & 0 deletions tests/state_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,14 @@ 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`)

// Broken tests:
// EOF is not part of cancun
st.skipLoad(`^stEOF/`)
Expand Down
Loading