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 "core/state: mark account as dirty when resetObject occurs" #271

Merged
merged 3 commits into from
Nov 21, 2023

Conversation

PlasmaPower
Copy link
Collaborator

This reverts commit 15bd21f.

This worked for upstream go-ethereum, but some ways we manipulate the state means this changes behavior for us

joshuacolvin0
joshuacolvin0 previously approved these changes Nov 21, 2023
Copy link
Member

@joshuacolvin0 joshuacolvin0 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@tsahee
Copy link
Contributor

tsahee commented Nov 21, 2023

seems fine.. but need to understand that test failure (I re-ran it to see if it's just flakiness)

@PlasmaPower
Copy link
Collaborator Author

Hmm yeah this seems to have broken something. I'll take a look tomorrow

@PlasmaPower
Copy link
Collaborator Author

The failing test is in state_fuzz_test.go and they say "We're mostly adding this change to appease the fuzzer" so I think it might be intended and we should just disable that test.

@PlasmaPower
Copy link
Collaborator Author

I've skipped that test. I've been syncing a node overnight with this change and it agrees with the rest of the network, so I think it's working (definitely working better than the previous alpha).

Copy link
Member

@joshuacolvin0 joshuacolvin0 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@joshuacolvin0 joshuacolvin0 merged commit f4eec6e into master Nov 21, 2023
3 checks passed
@joshuacolvin0 joshuacolvin0 deleted the dont-dirty-reset-object branch November 21, 2023 16:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
s CLA signed
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants