Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

stateManager.setStateRoot doesn't clear the _storageTries cache. #444

Closed
davidmurdoch opened this issue Feb 21, 2019 · 4 comments · Fixed by #445
Closed

stateManager.setStateRoot doesn't clear the _storageTries cache. #444

davidmurdoch opened this issue Feb 21, 2019 · 4 comments · Fixed by #445

Comments

@davidmurdoch
Copy link
Contributor

#417 was about the _cache which was fixed in #420. This issue is about _storageTries https://github.com/ethereumjs/ethereumjs-vm/blob/c842d101108d6acdd9ca82ab1de274ba3b16e5b2/lib/state/stateManager.js#L38

which I think should also get cleared when the stateRoot is set.

We ran into the issue with Ganache when a user creates a snapshot (which just means we record the block number at the time snapshot was called), then modifies contract storage, reverts the snapshot (we just set the state root back to the way it was when the snapshot was created), then modifies contract storage again.

PR and tests to follow.

@s1na
Copy link
Contributor

s1na commented Feb 25, 2019

What you're describing seems to be really similar to what commit and revert in stateManager are supposed to do. Have you checked them out by any chance? revert does clear both the state cache and the contract storage cache.

@davidmurdoch
Copy link
Contributor Author

Because we can't checkpoint() before calling runBlock (since: eddd9db#diff-bbc749b6f8c7177c7960ea582daf8f63R371) there are many situations where commit/revert will no longer work (as of v2.5.0).

Clearing the storage cache when it becomes invalid seems like an appropriate fix to me, as the accounts cache is currently cleared when it becomes invalid: 09b8bfb.

@holgerd77
Copy link
Member

//cc @mattdean-digicatapult

@mattdean-digicatapult
Copy link
Contributor

This looks like a bug exactly as described by @davidmurdoch. The simplest solution is as described; just clear the storage caches in setStateRoot whenever we clear _cache.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants