Skip to content
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

Add 'peerconnect' event from Pool #38

Merged
merged 1 commit into from
Feb 18, 2015

Conversation

throughnothing
Copy link
Contributor

This could be useful to some consumers. I'm using it to detect nodes that are connected, but slow to respond with their verack commands, to disconnect these slow nodes and get different nodes in my pool.

@braydonf
Copy link
Contributor

The connect event should likely have a separate test, as the "new addrs" test is only testing for handling of "addr" messages.

@throughnothing
Copy link
Contributor Author

@braydonf yup, was already working on a better test, that will test peerready, peerdisconnect, and peerconnect messages (all of which weren't really tested). Just pushed.

@throughnothing throughnothing force-pushed the peerconnect-event branch 2 times, most recently from 8b4a57f to 3ed6931 Compare February 16, 2015 22:17
@coveralls
Copy link

Coverage Status

Coverage increased (+0.03%) to 91.95% when pulling 3ed6931 on throughnothing:peerconnect-event into e197345 on bitpay:master.

@@ -100,4 +100,40 @@ describe('Pool', function() {

});

it('should propagate connect, ready, and disconnect peer evenst', function(done) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There is a typo and and extra space.

@braydonf
Copy link
Contributor

Interesting, I thought there may have been a test for disconnect, but it doesn't look like it. It should test to make sure that the peer has been removed from _connectedPeers, but that can be done in another PR.

@throughnothing
Copy link
Contributor Author

Good catch, updated.

@coveralls
Copy link

Coverage Status

Coverage increased (+0.03%) to 91.95% when pulling e0b58e2 on throughnothing:peerconnect-event into e197345 on bitpay:master.

@braydonf
Copy link
Contributor

LGTM

@maraoz
Copy link
Contributor

maraoz commented Feb 18, 2015

great contribution, thanks @throughnothing !

maraoz added a commit that referenced this pull request Feb 18, 2015
@maraoz maraoz merged commit 872dc52 into bitpay:master Feb 18, 2015
@throughnothing throughnothing deleted the peerconnect-event branch February 22, 2015 01:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants