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

internal: fix downloader synchronising judgement #16238

Closed
wants to merge 2 commits into from

Conversation

rjl493456442
Copy link
Member

During the fast sync, the pivot point may move if the pivot block is stale. So the original judgement is inaccurate if the pivot point is moved.

We should judgement whether downloader is in work or not always by the synchronising status flag.

Copy link
Member

@karalabe karalabe left a comment

Choose a reason for hiding this comment

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

Please split the PR into two ones, one for the highest block fix, the other for the state commit fix.

The highest block issue is a good catch, but the fix is not in my opinion. Many outside processes will consider the client synced if the HighestBlock == CurrentBlock. We don't want to do that. Instead, lets fix syncStatsChainHeight. Imho the best way would be to add an extra clause inside processHeaders after https://github.com/ethereum/go-ethereum/blob/master/eth/downloader/downloader.go#L1297 to bump syncStatsChainHeight to the last header if a higher one was found.

@karalabe karalabe added this to the 1.8.3 milestone Mar 7, 2018
@karalabe
Copy link
Member

karalabe commented Mar 8, 2018

Superseded by #16280 and #16283.

@karalabe karalabe closed this Mar 8, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants