This repository has been archived by the owner on Jul 1, 2021. It is now read-only.
Connect to unsynced peers, and trinity peers #618
Merged
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
Trinity peers were not communicating DAO headers during handshake, so trinity would not connect to another trinity node. Solution was to immediately add peer to pool during boot, and then kick it back out if it fails the DAO check. We were previously failing any peer who couldn't answer with the DAO fork header. That means that unsynced peers would be rejected. Modified to accept unsynced peers for now.
carver
force-pushed
the
connect-to-syncing-peers
branch
from
f184da5 to
7dc6403
Compare
pipermerriam
approved these changes
May 17, 2019
trinity/protocol/common/boot.py
Outdated
| f"{self.peer} failed to return DAO fork check headers" | ||
| self.logger.debug( | ||
| f"{self.peer} returned only {headers!r} at DAO fork. " | ||
| "Assuming peer is syncing, for now..." |
There was a problem hiding this comment.
I'd advocate for this to fall back to request the peer's announced HEAD block and to check whether the returned block number is less than the DAO headers. It seems we should be able to actually verify whether a peer responded because they don't have the headers.
There was a problem hiding this comment.
How about something like 7ac9344 ?
pipermerriam
approved these changes
May 18, 2019
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
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.
Trinity peers were not communicating DAO headers during handshake, so trinity would not connect to another trinity node. (lol, am I the first one to try this?)
Why? I think it's because all incoming requests were buffered, including the request for the DAO headers. So they both asked each other for the DAO headers and never answered because they were waiting for the other to respond.
Current solution is to immediately add peer to pool during boot, and then kick it back out if it fails the DAO check. At least, that's the solution when I connected my two trinity peers together.
Bonus: We were previously failing any peer who couldn't answer with the DAO
fork header. That means that unsynced peers would be rejected. This modifies the peer boot to accept unsynced peers for now.
To-Do
Cute Animal Picture