peer: Remove superfluous dup version check. #1248
Conversation
davecgh
force-pushed
the
peer_remove_superfluous_dup_version_check
branch
2 times, most recently
from
42463e1 to
0837320
Compare
peer/peer_test.go
Outdated
| } | ||
| inConn, outConn := pipe( | ||
| &conn{laddr: "10.0.0.1:8333", raddr: "10.0.0.2:8333"}, | ||
| &conn{laddr: "10.0.0.2:8333", raddr: "10.0.0.1:8333"}, |
There was a problem hiding this comment.
any reason to use the bitcoin ports here?
There was a problem hiding this comment.
Nope. It was copy paste from the setup in other tests. I'll update it for the default dcrd port, but it really doesn't matter because these are all mock connections that use pipe writers to communicate and thus aren't really opening the port anyway.
davecgh
force-pushed
the
peer_remove_superfluous_dup_version_check
branch
from
0837320 to
e5cae21
Compare
peer/peer_test.go
Outdated
| }() | ||
| select { | ||
| case <-disconnected: | ||
| close(disconnected) |
There was a problem hiding this comment.
Actually, this would panic if the disconnect took more than a second to complete. Would be better to buffer the channel and not close it, or use a select to send if ready.
There was a problem hiding this comment.
How so? It will only be closed once it reads from the channel. Otherwise, the test will fatal out and take the blocked goroutine with it.
There was a problem hiding this comment.
Nevertheless, I'll go ahead and buffer the channel and remove the close though since it doesn't add any value.
davecgh
force-pushed
the
peer_remove_superfluous_dup_version_check
branch
from
e5cae21 to
e8c1599
Compare
This removes an impossible to hit duplicate version check and adds a test to ensure the behavior is correct. Previously, the check was necessary because the handler was a part of the input handler loop, however, the initial negotiation logic was refactored some time ago to be synchronous and thus it is no longer possible to receive a second version message in the refactored code path.
davecgh
force-pushed
the
peer_remove_superfluous_dup_version_check
branch
from
e8c1599 to
da74b4b
Compare
This relies on PR #1247.
This removes an impossible to hit duplicate version check and adds a test to ensure the behavior is correct.
Previously, the check was necessary because the handler was a part of the input handler loop, however, the initial negotiation logic was refactored some time ago to be synchronous and thus it is no longer possible to receive a second version message in the refactored code path.