-
Notifications
You must be signed in to change notification settings - Fork 412
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
Partially fix hive test #4417
Partially fix hive test #4417
Conversation
Which hive tests are partially fixed thanks to this PR? |
I think it was one of the |
Ok, now I remember I think. We don't do any validation in beacon header sync, so no chain is connected here. But during MergeBlockDownloader, a block failed, which mean it stop suggesting at that point, invalid chain is not connected to its descendent as no validation was attempted. Then another NP of it's descendent also fail block validation, but since its not connected, it's last valid block is its not-validated parent block instead of the parent of the first non-valid block. |
Ok, I think I understood issue, but what will happen if we restart client in this case? |
I'm guessing, we will have the same issue. |
ok, so this PR will improve our behaviour, but do you have any thoughts what we can do after restart? |
Not at the moment |
Changes:
Types of changes
What types of changes does your code introduce?
Put an
x
in the boxes that applyTesting
Requires testing
In case you checked yes, did you write tests??
Comments about testing , should you have some (optional)
Further comments (optional)
If this is a relatively large or complex change, kick off the discussion by explaining why you chose the solution you did and what alternatives you considered, etc...