Chain repair fixes #1163
Merged
Chain repair fixes #1163
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
* core: define and test chain reparation cornercases * core: write up a variety of set-head tests * core, eth: unify chain rollbacks, handle all the cases * core: make linter smile * core: remove commented out legacy code * core, eth/downloader: fix review comments * core: revert a removed recovery mechanism
mrsmkl
requested review from
asaj,
kevjue,
timmoreton and
tkporter
as code owners
…ain into mrsmkl/long-repair
trianglesphere
suggested changes
Oct 9, 2020
There was a problem hiding this comment.
There's a couple small changes, and then the larger question of what to do with the tests. As noted, I think that removing tests with a side chain that's longer than the canonical chain could be valid, but I'd like more a little more though on that.
Once tests are figured out I think that this PR is good to go. It syncs with main net just fine in several modes.
|
Looks like some test cases are quite flaky, I'll check if it's the same in the master. Example: |
Unfortunately this seems to be a new flaky test. On my machine I don't find them to be that flaky, but I don't love adding new flaky tests. |
trianglesphere
approved these changes
Oct 15, 2020
oneeman
pushed a commit
that referenced
this pull request
Nov 25, 2020
Note: These changes overwrote the changes in eth/sync.go which we had made in #1163 (which were an adapted cherry-pick of a later chain repair commit from upstream). In the next commit I will apply the corresponding changes changes from the upstream chain repair commit, which are now directly applicable without having to be tweaked. Conflicts: eth/backend.go eth/handler.go eth/sync.go
oneeman
pushed a commit
that referenced
this pull request
Nov 25, 2020
For #1163, these changes had to be adapted because we had an older version of the `eth/sync.go`. The commit before this one merged in the upstream changes that were missing, and which overwrote the adapted chain repair fix changes. So this commit reapplies those changes from the upstream chain repair fix commit, since they now apply as is without any issues.
Merged
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description
Cherry-picked this:
ethereum/go-ethereum#21409
Other changes
Made some changes to get
SetHeadworking in lightest mode. Also doesn't start bloom indexer in lightest mode.Tested
Did some sync tests in addition to new unit tests.
Related issues
Backwards compatibility