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

Revert storage after selfdestruct #1866

Merged
merged 3 commits into from Oct 31, 2019

Conversation

@carver
Copy link
Contributor

carver commented Oct 30, 2019

What was wrong?

Fix #1865

How was it fixed?

Two fixes for two different scenarios:

  • If creating the storage DB for the first time just to wipe it, then make sure it has the original storage root hash.
  • If completely resetting the journal, make sure that any current clear (like a storage wipe) gets fully reset (which means it should read through to the underlying if the key is missing from the journal) See more commentary in the PR

To-Do

  • Clean up commit history

Cute Animal Picture

put a cute animal picture link inside the parentheses

@carver carver changed the title Revert selfdestruct Revert storage after selfdestruct Oct 30, 2019
@carver carver requested a review from cburgdorf Oct 30, 2019
@carver carver referenced this pull request Oct 30, 2019
@@ -74,3 +74,31 @@ def test_set_storage_input_validation(state, address, slot, new_value):
def test_delete_storage_input_validation(state):
with pytest.raises(ValidationError):
state.delete_storage(INVALID_ADDRESS)


# TODO candidate to build a core ethereum test (surprising that this case wasn't covered, already!)

This comment has been minimized.

Copy link
@cburgdorf

cburgdorf Oct 31, 2019

Contributor

Should we turn this into an issue at ethereum/tests and link to this PR?

This comment has been minimized.

Copy link
@pipermerriam

pipermerriam Oct 31, 2019

Member

cc @holiman , just noting that this wasn't covered in consensus tests and maybe interesting to test other clients, we'll work on generating a consensus style test for this.

This comment has been minimized.

Copy link
@carver

carver Oct 31, 2019

Author Contributor

I just wrote it up in #1867

Copy link
Contributor

cburgdorf left a comment

Yay! It took me a while to understand what's going on but the comments where very helpful 👍

@carver carver force-pushed the carver:revert-selfdestruct branch from 87ef75b to 750f82b Oct 31, 2019
@carver carver merged commit 83d50fa into ethereum:master Oct 31, 2019
20 checks passed
20 checks passed
ci/circleci: py36-benchmark Your tests passed on CircleCI!
Details
ci/circleci: py36-core Your tests passed on CircleCI!
Details
ci/circleci: py36-database Your tests passed on CircleCI!
Details
ci/circleci: py36-docs Your tests passed on CircleCI!
Details
ci/circleci: py36-lint Your tests passed on CircleCI!
Details
ci/circleci: py36-native-blockchain-byzantium Your tests passed on CircleCI!
Details
ci/circleci: py36-native-blockchain-constantinople Your tests passed on CircleCI!
Details
ci/circleci: py36-native-blockchain-frontier Your tests passed on CircleCI!
Details
ci/circleci: py36-native-blockchain-homestead Your tests passed on CircleCI!
Details
ci/circleci: py36-native-blockchain-petersburg Your tests passed on CircleCI!
Details
ci/circleci: py36-native-blockchain-spurious_dragon Your tests passed on CircleCI!
Details
ci/circleci: py36-native-blockchain-tangerine_whistle Your tests passed on CircleCI!
Details
ci/circleci: py36-native-blockchain-transition Your tests passed on CircleCI!
Details
ci/circleci: py36-transactions Your tests passed on CircleCI!
Details
ci/circleci: py36-vm Your tests passed on CircleCI!
Details
ci/circleci: py37-core Your tests passed on CircleCI!
Details
ci/circleci: py37-database Your tests passed on CircleCI!
Details
ci/circleci: py37-lint Your tests passed on CircleCI!
Details
ci/circleci: py37-transactions Your tests passed on CircleCI!
Details
ci/circleci: py37-vm Your tests passed on CircleCI!
Details
@carver carver deleted the carver:revert-selfdestruct branch Oct 31, 2019
@holiman

This comment has been minimized.

Copy link

holiman commented Nov 1, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
4 participants
You can’t perform that action at this time.