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.

Sign up for GitHub

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

Jump to bottom

seed nodes keep disconnecting #839

Closed
offerm opened this issue Mar 18, 2019 · 3 comments
Closed

seed nodes keep disconnecting #839

offerm opened this issue Mar 18, 2019 · 3 comments
Assignees
@sangaman
Labels
critical bug
Milestone
1.0.0-alpha.11

Comments

@offerm
Copy link
Contributor

offerm commented Mar 18, 2019

the connection to the seed nodes is not stable.

log attached

xud.log
@sangaman
Copy link
Collaborator

It looks like your node is sending GetNodes requests but not getting any responses. I don't know why yet but I'll look into it further.

@kilrau kilrau added this to the 1.0.0-alpha.11 milestone Mar 18, 2019
@offerm
Copy link
Contributor Author

offerm commented Mar 18, 2019

@sangaman you are right. My node is sending getNodes request which is not being responded.

The getNodes packet is ignored due to a check in lib/p2p/Peer.ts:619 - isPacketSolicited(packet)

If this returns false the packet will be ignored and will lead to connection failure.

Now, isPacketSolicited(packet) returns false if the connection is not market open. Marking the connection as open is done only after the server sends getNodes.

So, if A connects to B and B receives a getNodes from A before B issues a getNodes itself the packet from A will be ignored and disconnected will happen.

The handshake code between A and B is not symmetric.
A sends sessionInit
B sends ack
A wait for the ack from A
A compute encryption key (CPU)
B sends sessionInit
A sends 'ackB wait for theack` from A
B compute encryption key (CPU)

While B is doing the last 2 operations A is already sending getNodes to B.

The new Encryption code increases the chance that the getNodes from A will arrive at B before B marks the connection as open

@sangaman
Copy link
Collaborator

Awesome, thanks for this analysis that makes a lot of sense. I should be able to come up with a fix tonight.

sangaman added a commit that referenced this issue Mar 19, 2019
@sangaman
fix(p2p): handle packets received during handshake
5910510 
This commit addresses an edge case bug whereby one peer finishes the
handshake before the other and begins sending regular packets. It is
possible for the other peer to attempt to process these regular packets
while it is executing asynchronous code to complete the handshake, which
results in the regular packets being discarded as "unsolicited."

This makes two changes:

1. Moves the logic to asynchronously send a `GetNodesPacket` until after
we mark the peer as `opened`.
2. Add an `initializing` flag that is set to true once we receive a
`SessionAckPacket`. This is an indicator that any subsequent packets we
receive should not be acted upon until the handshake is complete. If
we receive packets while `opened` is false but `initializing` is true,
we wait to see which of the `open` or `close` events fire first to
either accept or discard a packet, respectively.

Fixes #839.
@sangaman sangaman mentioned this issue Mar 19, 2019
Closed
@ghost ghost added the in progress label Mar 19, 2019
sangaman added a commit that referenced this issue Mar 19, 2019
@sangaman
fix(p2p): handle packets received during handshake
32ad2cc 
This commit addresses an edge case bug whereby one peer finishes the
handshake before the other and begins sending regular packets. It is
possible for the other peer to attempt to process these regular packets
while it is executing asynchronous code to complete the handshake, which
results in the regular packets being discarded as "unsolicited."

This makes two changes:

1. Moves the logic to asynchronously send a `GetNodesPacket` until after
we mark the peer as `opened`.
2. Add an `initializing` flag that is set to true once we receive a
`SessionAckPacket`. This is an indicator that any subsequent packets we
receive should not be acted upon until the handshake is complete. If
we receive packets while `opened` is false but `initializing` is true,
we wait to see which of the `open` or `close` events fire first to
either accept or discard a packet, respectively.

Fixes #839.
sangaman added a commit that referenced this issue Mar 19, 2019
@sangaman
fix(p2p): open peer before sending session ack
4bff4e6 
This commit addresses an edge case bug whereby one peer finishes the
handshake before the other and begins sending regular packets. It is
possible for the other peer to attempt to process these regular packets
while it is executing asynchronous code to complete the handshake, which
results in the regular packets being discarded as "unsolicited."

The key change is marking the peer as `opened` after receiving and
authenticating the `SessionInitPacket`, as well as validating the peer
(not banned, not ourselves, not an incompatible version, etc...), but
before we send the `SessionAckPacket`. This ensures that we are ready
to accept packets before the peer is ready to send packets.

This involves restructuring the open flow, splitting it into two methods
(`beginOpen` and `completeOpen`). The first establishes the socket
connection and begins the handshake. The second marks the peer as
`opened` and compelts the handshake. In between we validate the peer,
and afterwards we perform the routine for newly opened peers including
sharing and asking for open orders.

Fixes #839.
@sangaman sangaman mentioned this issue Mar 19, 2019
Merged
sangaman added a commit that referenced this issue Mar 19, 2019
@sangaman
fix(p2p): open peer before sending session ack
7a55046 
This commit addresses an edge case bug whereby one peer finishes the
handshake before the other and begins sending regular packets. It is
possible for the other peer to attempt to process these regular packets
while it is executing asynchronous code to complete the handshake, which
results in the regular packets being discarded as "unsolicited."

The key change is marking the peer as `opened` after receiving and
authenticating the `SessionInitPacket`, as well as validating the peer
(not banned, not ourselves, not an incompatible version, etc...), but
before we send the `SessionAckPacket`. This ensures that we are ready
to accept packets before the peer is ready to send packets.

This involves restructuring the open flow, splitting it into two methods
(`beginOpen` and `completeOpen`). The first establishes the socket
connection and begins the handshake. The second marks the peer as
`opened` and compelts the handshake. In between we validate the peer,
and afterwards we perform the routine for newly opened peers including
sharing and asking for open orders.

Fixes #839.
sangaman added a commit that referenced this issue Mar 19, 2019
@sangaman
fix(p2p): open peer before sending session ack
2e6e1e8 
This commit addresses an edge case bug whereby one peer finishes the
handshake before the other and begins sending regular packets. It is
possible for the other peer to attempt to process these regular packets
while it is executing asynchronous code to complete the handshake, which
results in the regular packets being discarded as "unsolicited."

The key change is marking the peer as `opened` after receiving and
authenticating the `SessionInitPacket`, as well as validating the peer
(not banned, not ourselves, not an incompatible version, etc...), but
before we send the `SessionAckPacket`. This ensures that we are ready
to accept packets before the peer is ready to send packets.

This involves restructuring the open flow, splitting it into two methods
(`beginOpen` and `completeOpen`). The first establishes the socket
connection and begins the handshake. The second marks the peer as
`opened` and compelts the handshake. In between we validate the peer,
and afterwards we perform the routine for newly opened peers including
sharing and asking for open orders.

Fixes #839.
@ghost ghost removed the in progress label Mar 20, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@sangaman sangaman

Labels
critical bug
Projects
None yet
Milestone
1.0.0-alpha.11
Development

Successfully merging a pull request may close this issue.

fix(p2p): handle packets received during handshake ExchangeUnion/xud
3 participants
@sangaman @offerm @kilrau