-
Notifications
You must be signed in to change notification settings - Fork 2k
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
Fix invalid sync request #16779
Fix invalid sync request #16779
Conversation
9d2666b
to
4b63b1b
Compare
This pull request has conflicts, please resolve those before we can evaluate the pull request. |
1 similar comment
This pull request has conflicts, please resolve those before we can evaluate the pull request. |
4b63b1b
to
ef55951
Compare
Conflicts have been resolved. A maintainer will review the pull request shortly. |
Pull Request Test Coverage Report for Build 6787966429Warning: This coverage report may be inaccurate.We've detected an issue with your CI configuration that might affect the accuracy of this pull request's coverage report.
💛 - Coveralls |
…tly fall into the short-sync case because the current logic (mistakenly) assumes the heavier peak has a greater height. This patch adds the additional condition that the heavier peak also has a greater height. This avoids sending an invalid block request to the peer.
ef55951
to
ff86f5d
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
aok
Purpose:
When exposed to a heavier peak with a lower height than our current peak, we currently fall into the short-sync (catch-up) case because the current logic (mistakenly) assumes the heavier peak has a greater height. This results in us sending an invalid block request to the peer, which in turn responds with a
None
message.The log looks like this:
In the above test our (lighter) peak height is 1593 and the other (heavier) peak height is 1077. We end up requesting block 1593 from the peer.
This patch adds the additional condition that the heavier peak also has a greater or equal height, as it appears was the intention to begin with. This avoids sending an invalid block request to the peer and we immediately fall back to a long sync.
Current Behavior:
We send an invalid request, handle the resulting error and then initiate the long sync.
New Behavior:
We initiate the long sync right away.