search

ethereum / tests

Common tests for all Ethereum implementations
MIT License
558 stars 319 forks source link

EIP-7523: Remove empty account from post-merge tests #1329

Closed petertdavies closed 9 months ago

petertdavies commented 10 months ago

This PR removes every empty account from the prestate of post merge tests in order to comply with EIP-7523. It should not be merged until that EIP leaves Draft status. I have not included filled tests.

Some obscure empty account edgecases (e.g. what if coinbase is an empty account) have only been ever tested by accident in unrelated tests. I don't seem any value in preserving these tests. If there is a desire to preserve them, new tests should be written.

winsvega commented 10 months ago

if it is only after merge, then the tests must be split. empty account cases must be covered before merge, after merge vectors change into not having empty accounts

winsvega commented 10 months ago

we really reduce test coverage by doing that in my opinion. even if we fetch the state not to have empty accounts, L2 remain vulnerable to this cases I think.

petertdavies commented 10 months ago

we really reduce test coverage by doing that in my opinion. even if we fetch the state not to have empty accounts, L2 remain vulnerable to this cases I think.

I am confident that empty accounts do not exist in the current state of any production EVM chain anywhere.

The creation of new empty accounts became impossible in the Spurious Dragon hardfork which forked on Mainnet in November 2016. This is the same hard fork that introduced chain ids. It is unlikely that anyone would have launched an EVM after this without including Spurious Dragon since they would have wanted to use chain ids.

The only chains that are anywhere near old enough to not have launched with Spurious Dragon enabled are Ethereum, Ethereum Classic and some Ethereum Classic testnets. Ethereum is dealt with by EIP-7523 and Ethereum Classic is not going to merge so the point is moot. In addition, a transaction was sent on Ethereum Classic to clear all empty accounts.

chfast commented 9 months ago

I'm very much in favor of this change. I can help reviewing the PR if that's needed.