Implement EIP6780 #2612
Implement EIP6780 #2612
Conversation
Codecov Report
Additional details and impacted files
Flags with carried forward coverage won't be shown. Click here to find out more. |
| @@ -122,6 +122,11 @@ export class DefaultStateManager extends BaseStateManager implements StateManage | |||
| }) | |||
| } | |||
|
|
|||
| async deleteAccount(address: Address) { | |||
| delete this._storageTries[address.buf.toString('hex')] | |||
There was a problem hiding this comment.
In the case that a contract gets selfdestructed, this method gets called. However, before this change, it would not clear the storage trie cache. This means that while the contract should have empty storage, StateManager thinks the storage has not been wiped.
Does this lead to a consensus bug if you selfdestruct a CREATE2 contract and then CREATE2 the contract again? If EVM reads storage, does this contract then have storage or is it empty? It actually is, if we create a contract we ensure we wipe the storage first.
There was a problem hiding this comment.
Sorry, not so deep into this stuff, but I would assume that what's in this cache doesn't matter since the storage root in the account is set to the empty root? 🤔 Haven't looked into the code though.
There was a problem hiding this comment.
The problem was this:
- Create a contract, which sets storage slot 1 to value 1. Selfdestruct this contract
- If you
getAccounton the contract, it correctly returns an account which has nonce 0, balance 0, codeHash empty, storage empty - However, if you
getContractStorageon that address, and that key, it will returnBuffer < 01 >. This is because the storage trie cache was not deleted, so storage trie still points to the contract storage state right before the selfdestruct.
This thus means that in some very exotic situations, if test runners (ganache, hardhat) try to test certain expected values, StateManager thus returns the wrongly cached values (so if you inspect StateManager it reports the wrong values). You could thus write a test which will not match the expected values due to not deleting the cached trie.
jochem-brouwer
changed the base branch from
master
to
statemanager-cache-rewrite-new
|
I guess it likely makes sense to put this on hold a bit and proceed once we have a version of #2592 squashed and rebased towards |
|
Followed-up by #2733 |
See -> #2733
Implements EIP6780. This is considered for inclusion by ACD.Test currently fails, and I have no idea why, the storage root of the contract is the empty storage root, but somehow the contract still returns that the slot is non-empty.