py-evm icon indicating copy to clipboard operation
py-evm copied to clipboard

Do an audit to check for more instances of account_exists

Open lithp opened this issue 6 years ago • 3 comments

https://github.com/ethereum/py-evm/pull/1644 involved a change to how py-evm decides whether an account exists:

def extcodehash(computation: BaseComputation) -> None:
    """
    Return the code hash for a given address.
    EIP: https://github.com/ethereum/EIPs/blob/master/EIPS/eip-1052.md
    """
    account = force_bytes_to_address(computation.stack_pop(type_hint=constants.BYTES))
    account_db = computation.state.account_db


-     if not account_db.account_exists(account):
+    if account_db.account_is_empty(account):
        computation.stack_push(constants.NULL_BYTE)
    else:
        computation.stack_push(account_db.get_code_hash(account))

This change is specific to extcodehash but the general idea should always apply. Under the state cleaning rules from EIP-161 any account which is empty will be deleted, and should be treated as if it does not exist.

I'm not sure whether this means that account_exists should always return the same things that account_is_empty would, but this feels like an easy source of bugs. All other instances of account_exists should be checked to see if they make the same mistake which was fixed here.

lithp avatar Jan 10 '19 02:01 lithp

I just did a quick run-through, and everywhere that uses it seems to be doing it correctly.

This place in particular would break if account_exists always returned not account_is_empty:

https://github.com/ethereum/py-evm/blob/32834ab5d2b6ae509509b620cde9290506eec7bd/eth/vm/forks/spurious_dragon/state.py#L32-L35

Maybe we should rename it account_technically_exists() (only 10% joking :)). A silly name might encourage people to go look at the docstring, which should direct most use cases to invoke account_is_empty().

carver avatar Mar 24 '19 00:03 carver

How about account_present_in_trie

pipermerriam avatar Mar 24 '19 01:03 pipermerriam

How about account_present_in_trie

Yeah, that's way better

carver avatar Mar 27 '19 16:03 carver