crowetic/qortal
1
0
Fork 0
forked from Qortal/qortal
Code Pull Requests Activity

Improve inbound peer handshaking

Browse Source 
If a node accepts a connection from an inbound peer
then remote peer will send RESPONSE first
and local node would previously change handshaking state
to COMPLETED while computing their own RESPONSE.

This meant that the local node would sometimes also start
sending post-handshake messages to the remote peer,
e.g. TRANSACTION_SIGNATURES.

Remote peer is only expecting a RESPONSE message, so would
close connection.

So we introduce an extra handshaking state "RESPONDING" for use
by local node while they compute RESPONSE in a separate thread.
Once the RESPONSE has been sent, local node moves to COMPLETED
state and called onHandshakeCompleted() as per usual.

Note that the code path when connecting outbound to a remote peer
is not changed, and the RESPONDING state is not used.

Also in this commit:

Network.onPeerReady now bypasses call to onMessage and instead
calls onHandshakingMessage() directly to avoid race condition
where peer's handshake status could change between
onPeerReady's caller and onMessage() calling peer.getHandshakeStatus()
This commit is contained in:
catbref 2020-06-30 13:37:14 +01:00
parent e05fcd6655
commit 469bf2a63e
2 changed files with 30 additions and 5 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

31
src/main/java/org/qortal/network/Handshake.java
View File

@ -164,6 +164,11 @@ public enum Handshake {
peer.setPeersNodeId(Crypto.toNodeAddress(peersPublicKey));
// For inbound peers, we need to go into interim holding state while we compute RESPONSE
if (!peer.isOutbound())
return RESPONDING;
// Handshake completed!
return COMPLETED;
}
@ -184,22 +189,42 @@ public enum Handshake {
Message responseMessage = new ResponseMessage(nonce, data);
if (!peer.sendMessage(responseMessage))
peer.disconnect("failed to send RESPONSE");
// For inbound peers, we should actually be in RESPONDING state.
// So we need to do the extra work to move to COMPLETED state.
if (!peer.isOutbound()) {
peer.setHandshakeStatus(COMPLETED);
Network.getInstance().onHandshakeCompleted(peer);
}
});
responseThread.setDaemon(true);
responseThread.start();
}
},
COMPLETED(null) {
// Interim holding state while we compute RESPONSE to send to inbound peer
RESPONDING(null) {
@Override
public Handshake onMessage(Peer peer, Message message) {
// Handshake completed
// Should never be called
return null;
}
@Override
public void action(Peer peer) {
// Note: this is only called when we've made outbound connection
// Should never be called
}
},
COMPLETED(null) {
@Override
public Handshake onMessage(Peer peer, Message message) {
// Should never be called
return null;
}
@Override
public void action(Peer peer) {
// Note: this is only called if we've made outbound connection
}
};

4
src/main/java/org/qortal/network/Network.java
View File

@ -635,7 +635,7 @@ public class Network {
/** Called when Peer's thread has setup and is ready to process messages */
public void onPeerReady(Peer peer) {
this.onMessage(peer, null);
onHandshakingMessage(peer, null, Handshake.STARTED);
}
public void onDisconnect(Peer peer) {
@ -777,7 +777,7 @@ public class Network {
opportunisticMergePeers(peer.toString(), peerV2Addresses);
}
private void onHandshakeCompleted(Peer peer) {
/*pacakge*/ void onHandshakeCompleted(Peer peer) {
LOGGER.debug(String.format("Handshake completed with peer %s", peer));
// Are we already connected to this peer?