Skip to content
This repository has been archived by the owner on Nov 6, 2020. It is now read-only.

Limit sync reorganization to 20 blocks #3509

Merged
merged 1 commit into from
Nov 18, 2016
Merged

Limit sync reorganization to 20 blocks #3509

merged 1 commit into from
Nov 18, 2016

Conversation

arkpar
Copy link
Collaborator

@arkpar arkpar commented Nov 18, 2016

No description provided.

@arkpar arkpar added A0-pleasereview 🤓 Pull request needs code review. B0-patch labels Nov 18, 2016
@rphmeier
Copy link
Contributor

What's the motivation for this change?

@arkpar
Copy link
Collaborator Author

arkpar commented Nov 18, 2016

Prevents us from searching for common block indefinitely.
This was lost somehow during refactoring.

@rphmeier rphmeier added A8-looksgood 🦄 Pull request is reviewed well. and removed A0-pleasereview 🤓 Pull request needs code review. labels Nov 18, 2016
@rphmeier
Copy link
Contributor

It does feel a bit fragile though. Limiting reorganization amount is supposed to be managed by pruning history, so a hard limit at 20 feels overly restrictive. Can't the sync service just check if the newly-given block is actually useful?

@arkpar
Copy link
Collaborator Author

arkpar commented Nov 18, 2016

PV63 does not provide means to check if the NewHashes hash is useful. We need to download the header first and then find what chain it belongs to

@arkpar arkpar merged commit 15702a8 into master Nov 18, 2016
@arkpar arkpar deleted the sync-fix branch November 18, 2016 16:45
@arkpar arkpar restored the sync-fix branch November 18, 2016 16:49
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
A8-looksgood 🦄 Pull request is reviewed well.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants