search

ethereum / execution-specs

Specification for the Execution Layer. Tracking network upgrades.
Creative Commons Zero v1.0 Universal
869 stars 240 forks source link

Address collision when storage exists: spec vs impl mismatch #1028

Open xrchz opened 2 weeks ago

xrchz commented 2 weeks ago

Metadata

What was wrong?

The Python execution specs consider a create message to fail due to address collision if the account in question has nonce or code: https://github.com/ethereum/execution-specs/blob/98d6ddaaa709a2b7d0cd642f4cfcdadc8c0808e1/src/ethereum/cancun/vm/interpreter.py#L110

But the tests and client implementation seems to consider it a collision also if the account has non-empty storage.

Sources

EIP specifying the geth behaviour: https://eips.ethereum.org/EIPS/eip-7610

Geth check: https://github.com/ethereum/go-ethereum/blob/a1093d98eb3260f2abf340903c2d968b2b891c11/core/vm/evm.go#L460

Test relying on the geth behaviour: https://github.com/ethereum/tests/blob/4c87ebbf024a3e9ec842bcce90564f629a3bcd82/BlockchainTests/GeneralStateTests/stRevertTest/RevertInCreateInInit_Paris.json

Additional Context

If the python spec is corrected, there are a few simplifications also possible, e.g. no need to delete the storage when creating an account nor to keep track of created accounts.

d-xo commented 2 weeks ago

Looks this this was only merged into the prague hard fork branch: https://github.com/ethereum/execution-specs/pull/999.

I guess since this EIP was retroactively applied it would be better to merge it directly to master (as was done for the 3607 implementation).

SamWilsn commented 2 weeks ago

EIP-7610 doesn't seem to be Final yet. Has this been agreed upon by ACD?

chfast commented 2 weeks ago

EIP-7610 doesn't seem to be Final yet. Has this been agreed upon by ACD?

It was, I confirmed this multiple times. However, I have issues with implementing this in Erigon, where its difficult to answer the question if an account has any storage.