-
Notifications
You must be signed in to change notification settings - Fork 772
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
Reset selfdestruct on revert #392
Conversation
@mattdean-digicatapult now another state test fails :/ |
Noooooo, I thought I had it. Will check, thanks @rmeissner |
Hang on @rmeissner, how did you get a failure? I've just replayed this commit on top of the eip-1052 branch and I'm getting no failures |
I looked at the results of the ci: https://circleci.com/gh/ethereumjs/ethereumjs-vm/3060?utm_campaign=vcs-integration-link&utm_medium=referral&utm_source=github-build-link |
So there's 14 failures for that build and 15 for master https://circleci.com/gh/ethereumjs/ethereumjs-vm/3012. The difference being the 14 fixed in #326 right? |
Ahh I forgot that EXTCODEHASH was not merged :D .... so yeah we have ... all tests should be fixed if both PRs are merged :D |
Awesome, more than happy BTW to rebase this on top of the merged EXTCODEHASH changes and get the first fully passing build in a long time |
Just merged EXTCODEHASH. 😄 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good.
Fixes test TouchToEmptyAccountRevert3 on Constantinople
9144221
to
70dd74a
Compare
rebased, sorry @holgerd77 |
WOHOOOOO all tests pass gz @mattdean-digicatapult @holgerd77 |
Whew, that was a long way. Congrats, everyone!!! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good.
Fixes test TouchToEmptyAccountRevert3 on Constantinople.
Once #326 is merged this should mean all the tests now pass 🎉