From ef12453d14faefbf14af3a81f7e76b57ad05506a Mon Sep 17 00:00:00 2001 From: Gary Rong Date: Fri, 2 Feb 2024 11:57:34 +0800 Subject: [PATCH 1/3] core/vm: reject contract creation if the storage is non-empty --- core/vm/evm.go | 12 +++++++++--- core/vm/interface.go | 1 + tests/block_test.go | 13 +++++++++++++ tests/state_test.go | 12 ++++++++++++ 4 files changed, 35 insertions(+), 3 deletions(-) diff --git a/core/vm/evm.go b/core/vm/evm.go index 25b5bc84e8bd..36bbf0d3da67 100644 --- a/core/vm/evm.go +++ b/core/vm/evm.go @@ -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 diff --git a/core/vm/interface.go b/core/vm/interface.go index d7028cc7c7e3..30742e96de2b 100644 --- a/core/vm/interface.go +++ b/core/vm/interface.go @@ -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) diff --git a/tests/block_test.go b/tests/block_test.go index 1ba84f5f24b6..be4b6ee37da6 100644 --- a/tests/block_test.go +++ b/tests/block_test.go @@ -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/`) + bt.walk(t, blockTestDir, func(t *testing.T, name string, test *BlockTest) { execBlockTest(t, bt, test) }) @@ -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) }) diff --git a/tests/state_test.go b/tests/state_test.go index 6ec5c9d857bc..852f2c0bee97 100644 --- a/tests/state_test.go +++ b/tests/state_test.go @@ -54,6 +54,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/`) From 84527f3d782a2e8e33b6bec3a2d4aeb4a018255c Mon Sep 17 00:00:00 2001 From: Gary Rong Date: Mon, 8 Apr 2024 14:57:00 +0800 Subject: [PATCH 2/3] tests: remove duplication --- tests/state_test.go | 4 ---- 1 file changed, 4 deletions(-) diff --git a/tests/state_test.go b/tests/state_test.go index 852f2c0bee97..10f4e4ee19d3 100644 --- a/tests/state_test.go +++ b/tests/state_test.go @@ -61,10 +61,6 @@ func initMatcher(st *testMatcher) { 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 From d42b1f6a2c427ea3b0fea0116157b4fb065dda16 Mon Sep 17 00:00:00 2001 From: Martin Holst Swende Date: Mon, 8 Apr 2024 13:22:25 +0200 Subject: [PATCH 3/3] tests: avoid duplicate state-tests --- tests/block_test.go | 5 ----- tests/state_test.go | 5 +++++ 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/tests/block_test.go b/tests/block_test.go index be4b6ee37da6..43e3d99b3e8c 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) }) diff --git a/tests/state_test.go b/tests/state_test.go index 10f4e4ee19d3..6f53b88722d6 100644 --- a/tests/state_test.go +++ b/tests/state_test.go @@ -65,6 +65,11 @@ func initMatcher(st *testMatcher) { // Broken tests: // EOF is not part of cancun st.skipLoad(`^stEOF/`) + + // The tests under Pyspecs are the ones that are published as execution-spec tests. + // We run these tests separately, no need to _also_ run them as part of the + // reference tests. + st.skipLoad(`^Pyspecs/`) } func TestState(t *testing.T) {